Re: [PATCH V4 2/2] xen/arm: Add i.MX lpuart early printk support

[Julien Grall <julien@xxxxxxx>]

Hi,

On 11/04/2022 07:24, Michal Orzel wrote:
> On 08.04.2022 16:09, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >   xen/arch/arm/Kconfig.debug              | 14 +++++++
> >   xen/arch/arm/arm64/debug-imx-lpuart.inc | 52 +++++++++++++++++++++++++
> >   2 files changed, 66 insertions(+)
> >   create mode 100644 xen/arch/arm/arm64/debug-imx-lpuart.inc
> >
> > diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> > index 35ccd13273..842d768280 100644
> > --- a/xen/arch/arm/Kconfig.debug
> > +++ b/xen/arch/arm/Kconfig.debug
> > @@ -58,6 +58,16 @@ choice
> >                         This option is preferred over the platform specific
> >                         options; the platform specific options are deprecated
> >                         and will soon be removed.
> > +       config EARLY_UART_CHOICE_IMX_LPUART
> > +               select EARLY_UART_IMX_LPUART
> > +               depends on ARM_64
> > +               bool "Early printk via i.MX LPUART"
> > +               help
> > +                       Say Y here if you wish the early printk to direct their
> > +                       output to a i.MX LPUART. You can use this option to
> > +                       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_MESON
> >                 select EARLY_UART_MESON
> >                 depends on ARM_64
> > @@ -186,6 +196,9 @@ config EARLY_UART_CADENCE
> >   config EARLY_UART_EXYNOS4210
> >         select EARLY_PRINTK
> >         bool
> > +config EARLY_UART_IMX_LPUART
> > +       select EARLY_PRINTK
> > +       bool
> >   config EARLY_UART_MESON
> >         select EARLY_PRINTK
> >         bool
> > @@ -283,6 +296,7 @@ config EARLY_PRINTK_INC
> >         default "debug-8250.inc" if EARLY_UART_8250
> >         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-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-imx-lpuart.inc 
> > b/xen/arch/arm/arm64/debug-imx-lpuart.inc
> > new file mode 100644
> > index 0000000000..f68252da86
> > --- /dev/null
> > +++ b/xen/arch/arm/arm64/debug-imx-lpuart.inc
> > @@ -0,0 +1,52 @@
> > +/*
> > + * xen/arch/arm/arm64/debug-imx-lpuart.inc
> > + *
> > + * i.MX8QM specific debug code
> > + *
> > + * Peng Fan <peng.fan@xxxxxxx>
> > + * Copyright 2022 NXP
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * 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.
> > + */
> > +
> > +#include <asm/imx-lpuart.h>
> > +
> > +.macro early_uart_init wb wc wd
> > +/* Already initialized in bootloader */
> > +.endm
> > +
> FWICS, early_uart_init call is protected with CONFIG_EARLY_UART_INIT that is set
> only for PL011 UART. So I think there is no need to implement this empty macro,
> unless we want to keep the same macros across the files no matter whether in 
> use or not.
> I think you need to ask Julien.
The call to early_uart_init is indeed protected by #ifdef 
CONFIG_EARLY_UART_INIT. The goal was to avoid having all debug-*.inc to 
implement dummy helper.
So I would prefer if early_uart_init is not implemented for 
debug-imx-lpuart.inc.
Cheers,

--
Julien Grall