[U-Boot] [PATCH 1/2] serial: add skipping init option

[Simon Glass]

Hi Jun,

On Wed, 27 Mar 2019 at 03:55, Jun Nie <jun.nie at linaro.org> wrote:
>
> Lukasz Majewski <lukma at denx.de> 于2019年3月27日周三 下午5:34写道：
> >
> > Hi Simon, Jun,
> >
> > > Hi Jun,
> > >
> > > On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie at linaro.org> wrote:
> > > >
> > > > Simon Glass <sjg at chromium.org> 于2019年3月22日周五 下午3:56写道：
> > > > >
> > > > > Hi Jun,
> > > > >
> > > > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie at linaro.org>
> > > > > wrote:
> > > > > >
> > > > > > add skipping init option to avoid corrupt data in console
> > > > > > if serial is already initilized when u-boot start its excution.
> > > > > >
> > > > > > Signed-off-by: Jun Nie <jun.nie at linaro.org>
> > > > > > ---
> > > > > >  drivers/serial/Kconfig | 6 ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > Could we use a device-tree property for this? Some UARTs have a
> > > > > 'skip-init' property.
> > > > >
> > > > > Regards,
> > > > > Simon
> > > >
> > > > Hi Simon,
> > > >
> > > > It is a good suggestion. But device tree is board specific, while
> > > > the config is boot flow specific. For ATF -> OPTEE -> U-BOOT case,
> > > > initialization should be skipped. Without earlier initialization
> > > > from firmware, U-boot should do it by itself. So I need find a
> > > > solution that is defconfig specific, instead of device tree
> > > > specific.
> > >
> > > So you want something that is flow-specific, I think. Your thinking is
> > > that the defconfig specifies this. Is that because certain boards
> > > require ATF/Optee?
> > >
> > > Should we consider having a way to tell U-Boot that it is running
> > > from Optee?
> > >
> > > Could the UART driver detect that the UART is already inited?
> >
> > Isn't this issue similar to:
> > http://patchwork.ozlabs.org/patch/820824/
> >
> > and
> >
> > http://patchwork.ozlabs.org/patch/865963/
> >
> My case is that console does not emit any log when OPTEE runs into u-boot.
> I already root caused that there is a bug in uart clock handling.
> UART1_CLK_ROOT
> is hard coded in imx_get_uartclk() of arch/arm/mach-imx/mx7/clock.c
> If the console serial is not UART1, 0 may be returned and console does not work.
>
> But when I add an IMX_UART_PORT config option in Kconfig to dynamic configure
> the CLK_ROOT ID as below, the expansion result is
> UARTCONFIG_IMX_UART_PORT_CLK_ROOT instead of  UART1_CLK_ROOT.
> It seems macro expansion happens before the inclusion of defconfig. Do you see
> any better idea other than add the definition in
> include/configs/pico-imx7d.h directly?
>
> #define IMX_UART_CLK_ROOT     UART##CONFIG_IMX_UART_PORT##_CLK_ROOT
>
> Current plan is to add below change so that platform can define
> different clock root other
> than UART1_CLK_ROOT.
>
> +++ b/arch/arm/mach-imx/mx7/clock.c
> @@ -51,9 +51,12 @@ static u32 get_ipg_clk(void)
>         return get_ahb_clk() / 2;
>  }
>
> +#ifndef UART_CLK_ROOT
> +#define UART_CLK_ROOT UART1_CLK_ROOT
> +#endif
>  u32 imx_get_uartclk(void)
>  {
> -       return get_root_clk(UART1_CLK_ROOT);
> +       return get_root_clk(UART_CLK_ROOT);
>  }
>

I'm not very knowledgeable on this platform. This seems OK to me but
you should send a patch for iMX people to review.

Regards,
Simon