Re: [Xen-devel] [RFC PATCH 1/2] xen/arm: Add Amlogic S905 SoC early printk support

[Julien Grall <julien.grall@xxxxxxx>]

Hi Amit,

On 07/08/18 18:07, Amit Singh Tomar wrote:
> Signed-off-by: Amit Singh Tomar <amittomer25@xxxxxxxxx>
> ---
>   docs/misc/arm/early-printk.txt     |  1 +
>   xen/arch/arm/Rules.mk              |  1 +
>   xen/arch/arm/arm64/debug-meson.inc | 50 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 52 insertions(+)
>   create mode 100644 xen/arch/arm/arm64/debug-meson.inc
>
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index f765f59..2aa9528 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -41,6 +41,7 @@ the name of the machine:
>     - juno: printk with pl011 on Juno platform
>     - lager: printk with SCIF0 on Renesas R-Car H2 processors
>     - midway: printk with the pl011 on Calxeda Midway processors
> +  - meson: printk with the MESON for Amlogic S905 SoCs
>     - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs
>     - omap5432: printk with UART3 on TI OMAP5432 processors
>     - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index f264592..d4fabdc 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -36,6 +36,7 @@ EARLY_PRINTK_hikey960       := pl011,0xfff32000
>   EARLY_PRINTK_juno           := pl011,0x7ff80000
>   EARLY_PRINTK_lager          := scif,0xe6e60000
>   EARLY_PRINTK_midway         := pl011,0xfff36000
> +EARLY_PRINTK_meson          := meson,0xc81004c0
I would prefer if no new alias are added. The same could be achieved 
with CONFIG_EARLY_PRINTK=meson,0xc81004c0.
This could be documented on the wiki.

Cheers,

>   EARLY_PRINTK_mvebu          := mvebu,0xd0012000
>   EARLY_PRINTK_omap5432       := 8250,0x48020000,2
>   EARLY_PRINTK_rcar3          := scif,0xe6e88000
> diff --git a/xen/arch/arm/arm64/debug-meson.inc 
> b/xen/arch/arm/arm64/debug-meson.inc
> new file mode 100644
> index 0000000..d5507d3
> --- /dev/null
> +++ b/xen/arch/arm/arm64/debug-meson.inc
> @@ -0,0 +1,50 @@
> +/*
> + * xen/arch/arm/arm64/debug-meson.inc
> + *
> + * MESON specific debug code.
> + *
> + * Copyright (c) 2018, Amit Singh Tomar <amittomer25@xxxxxxxxx>.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define UART_STATUS_REG     0x0c
> +#define UART_TX_REG         0x00
I would prefer if we stick with the spec name. So UART_TX_REG should be 
renamed UART_WFIFO_REG.
Also, it might be worth considering to prefix them with AML_ so it is 
easy to find them on lookup.
> +
Looking at the earlyconsole implementation in Linux, it seems that TX 
needs to be enabled first (see meson_uart_enable_tx_engine).
Is it now done in the firmware?

> +/*
> + * MESON UART wait UART to be ready to transmit
> + * xb: register which contains the UART base address
> + * c: scratch register
> + */
> +.macro early_uart_ready xb c
> +1:
> +        ldrh   w\c, [\xb, #UART_STATUS_REG] /* status register */
> +        tst    w\c, #(1 << 21)              /* Check TXFIFO FULL bit */
Please define 1 << 21 rather than hardcoding it.

> +        b.ne   1b                           /* Wait for the UART to be ready */
> +.endm
> +
> +/*
> + * MESON UART transmit character
> + * xb: register which contains the UART base address
> + * wt: register which contains the character to transmit
> + */
> +.macro early_uart_transmit xb wt
> +       strb  \wt, [\xb, #UART_TX_REG]
> +.endm
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel