Re: [PATCH v1 2/5] xen/arm: Add NXP LINFlexD UART early printk support

[Julien Grall <julien@xxxxxxx>]

Hi,

On 10/09/2024 15:34, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
>
> This adds support for early printk debug via the NXP LINFlexD
> UART controller.
>
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> Signed-off-by: Peter van der Perk <peter.vander.perk@xxxxxxx>
> ---
>   xen/arch/arm/Kconfig.debug           | 14 +++++++
>   xen/arch/arm/arm64/debug-linflex.inc | 58 ++++++++++++++++++++++++++++
>   2 files changed, 72 insertions(+)
>   create mode 100644 xen/arch/arm/arm64/debug-linflex.inc
>
> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> index eec860e88e..a309f67f90 100644
> --- a/xen/arch/arm/Kconfig.debug
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -68,6 +68,16 @@ choice
>                         provide the parameters for the i.MX LPUART rather than
>                         selecting one of the platform specific options below if
>                         you know the parameters for the port.
> +       config EARLY_UART_CHOICE_LINFLEX
> +               select EARLY_UART_LINFLEX
> +               depends on ARM_64
> +               bool "Early printk via NXP LINFlexD UART"
> +               help
> +                       Say Y here if you wish the early printk to direct their
> +                       output to an NXP LINFlexD UART. You can use this option 
> to
> +                       provide the parameters for the NXP LINFlexD UART rather 
> than
> +                       selecting one of the platform specific options below if
> +                       you know the parameters for the port.
>         config EARLY_UART_CHOICE_MESON
>                 select EARLY_UART_MESON
>                 depends on ARM_64
> @@ -199,6 +209,9 @@ config EARLY_UART_EXYNOS4210
>   config EARLY_UART_IMX_LPUART
>         select EARLY_PRINTK
>         bool
> +config EARLY_UART_LINFLEX
> +       select EARLY_PRINTK
> +       bool
>   config EARLY_UART_MESON
>         select EARLY_PRINTK
>         bool
> @@ -304,6 +317,7 @@ config EARLY_PRINTK_INC
>         default "debug-cadence.inc" if EARLY_UART_CADENCE
>         default "debug-exynos4210.inc" if EARLY_UART_EXYNOS4210
>         default "debug-imx-lpuart.inc" if EARLY_UART_IMX_LPUART
> +       default "debug-linflex.inc" if EARLY_UART_LINFLEX
>         default "debug-meson.inc" if EARLY_UART_MESON
>         default "debug-mvebu.inc" if EARLY_UART_MVEBU
>         default "debug-pl011.inc" if EARLY_UART_PL011
> diff --git a/xen/arch/arm/arm64/debug-linflex.inc 
> b/xen/arch/arm/arm64/debug-linflex.inc
> new file mode 100644
> index 0000000000..6ed704e104
> --- /dev/null
> +++ b/xen/arch/arm/arm64/debug-linflex.inc
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
GPL-2.0 is deprecated and replaced by GPL-2.0-only.

> +/*
> + * xen/arch/arm/arm64/debug-linflex.inc
> + *
> + * NXP LINFlexD UART specific debug code
> + *
> + * Andrei Cherechesu <andrei.cherechesu@xxxxxxx>
> + * Copyright 2018, 2021, 2023-2024 NXP
> + */
> +
> +#include <asm/asm_defns.h>
> +#include <asm/linflex-uart.h>
> +
> +/*
> + * wait LINFlexD UART to be ready to transmit
> + * xb: register which contains the UART base address
> + * c: scratch register number
> + */
> +.macro early_uart_ready xb, c
> +    ldr   w\c, [\xb, #UARTCR]       /* <= Control Register */
> +    and   w\c, w\c, #UARTCR_TFBM    /* Check Buffer/FIFO (0/1) Mode */
> +    cbz   w\c, 2f                   /* Buffer Mode => return */
> +1:
> +    ldrb  w\c, [\xb, #UARTSR]       /* <= Status Register */
> +    tst   w\c, #UARTSR_DTFTFF       /* FIFO Mode => Check DTF bit */
> +    b.ne  1b
> +2:
> +.endm
> +
> +/*
> + * LINFlexD 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, #BDRL]
> +
> +    ldr   \wt, [\xb, #UARTCR]       /* <= Control Register */
> +    and   \wt, \wt, #UARTCR_TFBM    /* Check Buffer/FIFO (0/1) Mode */
> +    cbnz  \wt, 2f                   /* FIFO Mode => goto exit */
> +
> +3:  /* Buffer Mode */
> +    ldrb  \wt, [\xb, #UARTSR]       /* <= Status Register */
> +    and   \wt, \wt, #UARTSR_DTFTFF  /* Check Transmission Completed */
> +    cbz   \wt, 3b
> +
> +    ldr   \wt, [\xb, #UARTSR]       /* <= Status Register */
> +    orr   \wt, \wt, #UARTSR_DTFTFF  /* Clear DTF bit */
> +    str   \wt, [\xb, #UARTSR]
> +2:
> +.endm
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
> \ No newline at end of file
The file should end with a newline.

I haven't looked at the specifics of the UART. But the code integration 
in Xen LGTM. So with the two comments addressed:
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall