RE: [PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Dear Wolfgang,
> From: Wolfgang Denk
> Sent: jeudi 23 avril 2020 22:40
>
> Dear Patrick,
>
> In message <8970fb86c1374d1999ff656c2a327...@sfhdag6node3.st.com>
> you wrote:
> >
[...]
> > > > /* enable console uart printing */
> > > > preloader_console_init();
> > > >
> > > > + if (clk || reset || pinctrl)
> > > > + printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > > > + __func__, clk, reset, pinctrl);
> > > > +
> > >
> > > This change makes little sense to me/ If you want error messages,
> > > then just turn above debug() into printf(), and be done with it.
> > > As an additional benefit so see at once which step failed.
> >
> > In this patch, I try to display error as soon as console is available
> > (after preloader_console_init) , if after one driver initialization
> > (clk, reset, pincontrol) failing.
> >
> > Change debug to printf only works only if CONFIG_DEBUG_UART is
> > activated (not by default) and board_debug_uart_init() exist to
> > configure the needed UART TX pins.
>
> Maybe you can remember an error code so you can tell the user which of the
> steps
> failed? That would be more useful, then.
After check, I agree with you first comment.
Because console over serial can work only if the driver required for uart
driver (clk, reset, pincontrol) can probe.
So CONFIG_DEBUG_UART is mandatory to identify the failing step and debug() is
enough in this case.
In fact my initial issue was only the simple return (and SPL boot continue)
when a error was detected: I replace them by hang() calls.
That simplify the implementation in V2:
[PATCH v2 02/12] arm: stm32mp: spl: update error management in board_init_f
http://patchwork.ozlabs.org/project/uboot/patch/20200422142834.v2.2.I703cbd885066981e3bab374021d5578dce7cb035@changeid/
Regards
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
> I know engineers. They love to change things. - Dr. McCoy
Patrick
Re: [PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Dear Patrick,
In message <8970fb86c1374d1999ff656c2a327...@sfhdag6node3.st.com> you wrote:
>
> > > - ret = uclass_get_device(UCLASS_CLK, 0, );
> > > - if (ret) {
> > > - debug("Clock init failed: %d\n", ret);
> > > - return;
> > > - }
> > > + clk = uclass_get_device(UCLASS_CLK, 0, );
> > > + if (clk)
> > > + debug("%s: Clock init failed: %d\n", __func__, clk);
> > >
> > > - ret = uclass_get_device(UCLASS_RESET, 0, );
> > > - if (ret) {
> > > - debug("Reset init failed: %d\n", ret);
> > > - return;
> > > - }
> > > + reset = uclass_get_device(UCLASS_RESET, 0, );
> > > + if (reset)
> > > + debug("%s: Reset init failed: %d\n", __func__, reset);
> > >
> > > - ret = uclass_get_device(UCLASS_PINCTRL, 0, );
> > > - if (ret) {
> > > - debug("%s: Cannot find pinctrl device\n", __func__);
> > > - return;
> > > - }
> > > + pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, );
> > > + if (pinctrl)
> > > + debug("%s: Cannot find pinctrl device: %d\n",
> > > + __func__, pinctrl);
> > >
> > > /* enable console uart printing */
> > > preloader_console_init();
> > >
> > > + if (clk || reset || pinctrl)
> > > + printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > > +__func__, clk, reset, pinctrl);
> > > +
> >
> > This change makes little sense to me/ If you want error messages, then
> > just turn
> > above debug() into printf(), and be done with it.
> > As an additional benefit so see at once which step failed.
>
> In this patch, I try to display error as soon as console is available
> (after preloader_console_init) , if after one driver initialization
> (clk, reset, pincontrol) failing.
>
> Change debug to printf only works only if CONFIG_DEBUG_UART
> is activated (not by default) and board_debug_uart_init() exist to configure
> the needed UART TX pins.
Maybe you can remember an error code so you can tell the user which
of the steps failed? That would be more useful, then.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I know engineers. They love to change things. - Dr. McCoy
RE: [PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Dear Wolfgang,
> From: Wolfgang Denk
> Sent: mercredi 1 avril 2020 13:30
>
> Dear Patrick Delaunay,
>
> In message
> <20200331180330.6.I41a641a07fd12da45b392920fc3407e608926396@changeid>
> you wrote:
> > Update board_init_f and try to display error message when console is
> > available.
> >
> > This patch adds trace to debug a spl boot issue when DEBUG and
> > DEBUG_UART is not activated, after uart probe.
> >
> > Signed-off-by: Patrick Delaunay
> > ---
> >
> > arch/arm/mach-stm32mp/spl.c | 33 -
> > 1 file changed, 16 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> > index ca4231cd0d..dfdb5bb7e9 100644
> > --- a/arch/arm/mach-stm32mp/spl.c
> > +++ b/arch/arm/mach-stm32mp/spl.c
> > @@ -79,37 +79,36 @@ void spl_display_print(void) void
> > board_init_f(ulong dummy) {
> > struct udevice *dev;
> > - int ret;
> > + int ret, clk, reset, pinctrl;
> >
> > arch_cpu_init();
> >
> > ret = spl_early_init();
> > if (ret) {
> > - debug("spl_early_init() failed: %d\n", ret);
> > + debug("%s: spl_early_init() failed: %d\n", __func__, ret);
> > hang();
> > }
> >
> > - ret = uclass_get_device(UCLASS_CLK, 0, );
> > - if (ret) {
> > - debug("Clock init failed: %d\n", ret);
> > - return;
> > - }
> > + clk = uclass_get_device(UCLASS_CLK, 0, );
> > + if (clk)
> > + debug("%s: Clock init failed: %d\n", __func__, clk);
> >
> > - ret = uclass_get_device(UCLASS_RESET, 0, );
> > - if (ret) {
> > - debug("Reset init failed: %d\n", ret);
> > - return;
> > - }
> > + reset = uclass_get_device(UCLASS_RESET, 0, );
> > + if (reset)
> > + debug("%s: Reset init failed: %d\n", __func__, reset);
> >
> > - ret = uclass_get_device(UCLASS_PINCTRL, 0, );
> > - if (ret) {
> > - debug("%s: Cannot find pinctrl device\n", __func__);
> > - return;
> > - }
> > + pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, );
> > + if (pinctrl)
> > + debug("%s: Cannot find pinctrl device: %d\n",
> > + __func__, pinctrl);
> >
> > /* enable console uart printing */
> > preloader_console_init();
> >
> > + if (clk || reset || pinctrl)
> > + printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > + __func__, clk, reset, pinctrl);
> > +
>
> This change makes little sense to me/ If you want error messages, then just
> turn
> above debug() into printf(), and be done with it.
> As an additional benefit so see at once which step failed.
In this patch, I try to display error as soon as console is available
(after preloader_console_init) , if after one driver initialization
(clk, reset, pincontrol) failing.
Change debug to printf only works only if CONFIG_DEBUG_UART
is activated (not by default) and board_debug_uart_init() exist to configure
the needed UART TX pins.
At least I need to remove the return and change them to hang() to interrupt SPL
execution if one probe failed to detect issue
I spent some time for this kind of issue: clock probe failed without any trace.
> Best regards,
>
> Wolfgang Denk
>
> --
Regards
Patrick
Re: [PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Dear Patrick Delaunay,
In message
<20200331180330.6.I41a641a07fd12da45b392920fc3407e608926396@changeid> you wrote:
> Update board_init_f and try to display error message
> when console is available.
>
> This patch adds trace to debug a spl boot issue when DEBUG
> and DEBUG_UART is not activated, after uart probe.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> arch/arm/mach-stm32mp/spl.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index ca4231cd0d..dfdb5bb7e9 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -79,37 +79,36 @@ void spl_display_print(void)
> void board_init_f(ulong dummy)
> {
> struct udevice *dev;
> - int ret;
> + int ret, clk, reset, pinctrl;
>
> arch_cpu_init();
>
> ret = spl_early_init();
> if (ret) {
> - debug("spl_early_init() failed: %d\n", ret);
> + debug("%s: spl_early_init() failed: %d\n", __func__, ret);
> hang();
> }
>
> - ret = uclass_get_device(UCLASS_CLK, 0, );
> - if (ret) {
> - debug("Clock init failed: %d\n", ret);
> - return;
> - }
> + clk = uclass_get_device(UCLASS_CLK, 0, );
> + if (clk)
> + debug("%s: Clock init failed: %d\n", __func__, clk);
>
> - ret = uclass_get_device(UCLASS_RESET, 0, );
> - if (ret) {
> - debug("Reset init failed: %d\n", ret);
> - return;
> - }
> + reset = uclass_get_device(UCLASS_RESET, 0, );
> + if (reset)
> + debug("%s: Reset init failed: %d\n", __func__, reset);
>
> - ret = uclass_get_device(UCLASS_PINCTRL, 0, );
> - if (ret) {
> - debug("%s: Cannot find pinctrl device\n", __func__);
> - return;
> - }
> + pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, );
> + if (pinctrl)
> + debug("%s: Cannot find pinctrl device: %d\n",
> + __func__, pinctrl);
>
> /* enable console uart printing */
> preloader_console_init();
>
> + if (clk || reset || pinctrl)
> + printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> +__func__, clk, reset, pinctrl);
> +
This change makes little sense to me/ If you want error messages,
then just turn above debug() into printf(), and be done with it.
As an additional benefit so see at once which step failed.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
A Chairman was as necessary to a Board planet as the zero was in
mathematics, but being a zero had big disadvantages...
- Terry Pratchett, _The Dark Side of the Sun_
Re: [PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Hi Patrick
On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Update board_init_f and try to display error message
> when console is available.
>
> This patch adds trace to debug a spl boot issue when DEBUG
> and DEBUG_UART is not activated, after uart probe.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> arch/arm/mach-stm32mp/spl.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index ca4231cd0d..dfdb5bb7e9 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -79,37 +79,36 @@ void spl_display_print(void)
> void board_init_f(ulong dummy)
> {
> struct udevice *dev;
> - int ret;
> + int ret, clk, reset, pinctrl;
>
> arch_cpu_init();
>
> ret = spl_early_init();
> if (ret) {
> - debug("spl_early_init() failed: %d\n", ret);
> + debug("%s: spl_early_init() failed: %d\n", __func__, ret);
> hang();
> }
>
> - ret = uclass_get_device(UCLASS_CLK, 0, );
> - if (ret) {
> - debug("Clock init failed: %d\n", ret);
> - return;
> - }
> + clk = uclass_get_device(UCLASS_CLK, 0, );
> + if (clk)
> + debug("%s: Clock init failed: %d\n", __func__, clk);
>
> - ret = uclass_get_device(UCLASS_RESET, 0, );
> - if (ret) {
> - debug("Reset init failed: %d\n", ret);
> - return;
> - }
> + reset = uclass_get_device(UCLASS_RESET, 0, );
> + if (reset)
> + debug("%s: Reset init failed: %d\n", __func__, reset);
>
> - ret = uclass_get_device(UCLASS_PINCTRL, 0, );
> - if (ret) {
> - debug("%s: Cannot find pinctrl device\n", __func__);
> - return;
> - }
> + pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, );
> + if (pinctrl)
> + debug("%s: Cannot find pinctrl device: %d\n",
> + __func__, pinctrl);
>
> /* enable console uart printing */
> preloader_console_init();
>
> + if (clk || reset || pinctrl)
> + printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> +__func__, clk, reset, pinctrl);
> +
> ret = uclass_get_device(UCLASS_RAM, 0, );
> if (ret) {
> printf("DRAM init failed: %d\n", ret);
Reviewed-by: Patrice Chotard
Thanks
[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f
Update board_init_f and try to display error message
when console is available.
This patch adds trace to debug a spl boot issue when DEBUG
and DEBUG_UART is not activated, after uart probe.
Signed-off-by: Patrick Delaunay
---
arch/arm/mach-stm32mp/spl.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index ca4231cd0d..dfdb5bb7e9 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -79,37 +79,36 @@ void spl_display_print(void)
void board_init_f(ulong dummy)
{
struct udevice *dev;
- int ret;
+ int ret, clk, reset, pinctrl;
arch_cpu_init();
ret = spl_early_init();
if (ret) {
- debug("spl_early_init() failed: %d\n", ret);
+ debug("%s: spl_early_init() failed: %d\n", __func__, ret);
hang();
}
- ret = uclass_get_device(UCLASS_CLK, 0, );
- if (ret) {
- debug("Clock init failed: %d\n", ret);
- return;
- }
+ clk = uclass_get_device(UCLASS_CLK, 0, );
+ if (clk)
+ debug("%s: Clock init failed: %d\n", __func__, clk);
- ret = uclass_get_device(UCLASS_RESET, 0, );
- if (ret) {
- debug("Reset init failed: %d\n", ret);
- return;
- }
+ reset = uclass_get_device(UCLASS_RESET, 0, );
+ if (reset)
+ debug("%s: Reset init failed: %d\n", __func__, reset);
- ret = uclass_get_device(UCLASS_PINCTRL, 0, );
- if (ret) {
- debug("%s: Cannot find pinctrl device\n", __func__);
- return;
- }
+ pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, );
+ if (pinctrl)
+ debug("%s: Cannot find pinctrl device: %d\n",
+ __func__, pinctrl);
/* enable console uart printing */
preloader_console_init();
+ if (clk || reset || pinctrl)
+ printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
+ __func__, clk, reset, pinctrl);
+
ret = uclass_get_device(UCLASS_RAM, 0, );
if (ret) {
printf("DRAM init failed: %d\n", ret);
--
2.17.1
