Message ID | 1553239204-9732-1-git-send-email-jun.nie@linaro.org |
---|---|
State | Changes Requested |
Delegated to: | Stefano Babic |
Headers | show |
Series | [U-Boot,1/2] serial: add skipping init option | expand |
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
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
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
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@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@denx.de
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@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@denx.de
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
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
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
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
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(+)