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

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

[Jun Nie]

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@linaro.org>
---
 drivers/serial/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 887cd68..7aeaffb 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -459,6 +459,12 @@ config DEBUG_UART_NS16550_CHECK_ENABLED
 	  Note that this does not work for every ns16550-compatible UART and
 	  so has to be enabled carefully or you might notice lost characters.
 
+config UART_SKIP_INIT
+       bool "Skip UART initialization"
+       help
+         Select this if the UART you want to use for output is already
+         initialized by the time U-Boot starts its execution.
+
 config ALTERA_JTAG_UART
 	bool "Altera JTAG UART support"
 	depends on DM_SERIAL
-- 
2.7.4

[U-Boot] [PATCH 2/2] imx: serial_mxc: skip uart initilization

[Jun Nie]

Skip uart initilization if CONFIG_UART_SKIP_INIT is defined.
That's means earlier firmware already initialized the uart.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/serial/serial_mxc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index 476df25..01c97a4 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -231,9 +231,11 @@ static int mxc_serial_tstc(void)
  */
 static int mxc_serial_init(void)
 {
+#ifndef CONFIG_UART_SKIP_INIT
 	_mxc_serial_init(mxc_base, false);
 
 	serial_setbrg();
+#endif
 
 	return 0;
 }
@@ -276,7 +278,9 @@ static int mxc_serial_probe(struct udevice *dev)
 {
 	struct mxc_serial_platdata *plat = dev->platdata;
 
+#ifndef CONFIG_UART_SKIP_INIT
 	_mxc_serial_init(plat->reg, plat->use_dte);
+#endif
 
 	return 0;
 }
-- 
2.7.4

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

[Simon Glass]

Hi Jun,

On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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

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

[Jun Nie]

Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
>
> Hi Jun,
>
> On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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.

Regards.
Jun

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

[Simon Glass]

Hi Jun,

On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
>
> Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
> >
> > Hi Jun,
> >
> > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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?

Regards,
Simon

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

[Lukasz Majewski]

Hi Simon, Jun,

> Hi Jun,
> 
> On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
> >
> > Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：  
> > >
> > > Hi Jun,
> > >
> > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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/

> 
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190327/a0f278e7/attachment.sig>

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

[Jun Nie]

Lukasz Majewski <lukma@denx.de> 于2019年3月27日周三 下午5:34写道：
>
> Hi Simon, Jun,
>
> > Hi Jun,
> >
> > On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
> > >
> > > Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
> > > >
> > > > Hi Jun,
> > > >
> > > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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);
 }

> >
> > Regards,
> > Simon
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de

[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@linaro.org> wrote:
>
> Lukasz Majewski <lukma@denx.de> 于2019年3月27日周三 下午5:34写道：
> >
> > Hi Simon, Jun,
> >
> > > Hi Jun,
> > >
> > > On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
> > > >
> > > > Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
> > > > >
> > > > > Hi Jun,
> > > > >
> > > > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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

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

[Simon Glass]

Hi Lukasz,

On Wed, 27 Mar 2019 at 03:34, Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Simon, Jun,
>
> > Hi Jun,
> >
> > On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
> > >
> > > Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
> > > >
> > > > Hi Jun,
> > > >
> > > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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/
>

Yes. But in the case of this patch I'm hoping we don't need it.

Regards,
Simon

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

[Jun Nie]

Simon Glass <sjg@chromium.org> 于2019年3月31日周日 上午5:19写道：
>
> Hi Lukasz,
>
> On Wed, 27 Mar 2019 at 03:34, Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Simon, Jun,
> >
> > > Hi Jun,
> > >
> > > On Fri, 22 Mar 2019 at 16:02, Jun Nie <jun.nie@linaro.org> wrote:
> > > >
> > > > Simon Glass <sjg@chromium.org> 于2019年3月22日周五 下午3:56写道：
> > > > >
> > > > > Hi Jun,
> > > > >
> > > > > On Fri, 22 Mar 2019 at 15:20, Jun Nie <jun.nie@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@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/
> >
>
> Yes. But in the case of this patch I'm hoping we don't need it.
>
> Regards,
> Simon

Right, we do not need patch here to skip the initialization. I already
add the fix in imx clock side to imx patch set. Thank for suggestion!

Jun