Re: Boot regression on am335x-hs-evm

[LABBE Corentin <clabbe@baylibre.com>]

Le Fri, Jun 10, 2022 at 11:01:47AM -0400, Tom Rini a écrit :
> On Fri, Jun 10, 2022 at 04:51:12PM +0200, LABBE Corentin wrote:
> > Le Fri, Jun 10, 2022 at 08:16:10AM -0400, Tom Rini a écrit :
> > > On Fri, Jun 10, 2022 at 11:59:23AM +0200, LABBE Corentin wrote:
> > > > Hello
> > > > 
> > > > I hit a boot regression on am335x-hs-evm.
> > > > On current uboot, the board does not boot at all.
> > > > This board uses both MLO and u-boot.img and only MLO was the problem.
> > > > 
> > > > After a bisect, I found that e41651fffda7 ("dm: Support parent devices with of-platdata") was the problem.
> > > > Reverting this patch lead to a success boot.
> > > > 
> > > > I cutdown the revert to a minimal fix:
> > > > --- a/drivers/core/lists.c
> > > > +++ b/drivers/core/lists.c
> > > > @@ -120,6 +120,7 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
> > > >                 int ret;
> > > >  
> > > >                 ret = bind_drivers_pass(parent, pre_reloc_only);
> > > > +               return ret;
> > > >                 if (!ret)
> > > >                         break;
> > > >                 if (ret != -EAGAIN && !result)
> > > > 
> > > > I cannot debug further since printf() is not working at this stage.
> > > > 
> > > > Since I wanted to know which error was badly handled, I tried to do this:
> > > > --- a/arch/arm/mach-omap2/sec-common.c
> > > > +++ b/arch/arm/mach-omap2/sec-common.c
> > > > @@ -111,6 +111,8 @@ static u32 find_sig_start(char *image, size_t size)
> > > >         return 0;
> > > >  }
> > > >  
> > > > +extern int errorcount;
> > > > +
> > > >  int secure_boot_verify_image(void **image, size_t *size)
> > > >  {
> > > >         int result = 1;
> > > > @@ -178,6 +180,7 @@ auth_exit:
> > > >          * via YMODEM. This is done to avoid disturbing the YMODEM serial
> > > >          * protocol transactions.
> > > >          */
> > > > +       printf("ERRORCOUNT %d\n", errorcount);
> > > >         if (!(IS_ENABLED(CONFIG_SPL_BUILD) &&
> > > >               IS_ENABLED(CONFIG_SPL_YMODEM_SUPPORT) &&
> > > >               spl_boot_device() == BOOT_DEVICE_UART))
> > > > --- a/drivers/core/lists.c
> > > > +++ b/drivers/core/lists.c
> > > > @@ -20,6 +20,10 @@
> > > >  #include <fdtdec.h>
> > > >  #include <linux/compiler.h>
> > > >  
> > > > +static int _errorcount;
> > > > +int errorlist[1024];
> > > > +int errorcount;
> > > > +
> > > >  struct driver *lists_driver_lookup_name(const char *name)
> > > >  {
> > > >         struct driver *drv =
> > > > @@ -120,8 +124,9 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
> > > >                 int ret;
> > > >  
> > > >                 ret = bind_drivers_pass(parent, pre_reloc_only);
> > > > -               if (!ret)
> > > > -                       break;
> > > > +               errorlist[_errorcount] = ret;
> > > > +               _errorcount++;
> > > > +               errorcount = _errorcount;
> > > >                 if (ret != -EAGAIN && !result)
> > > >                         result = ret;
> > > >         }
> > > > 
> > > > But errorcount is always 0 which is puzzling me since according to my think, lists_bind_drivers() is ran before secure_boot_verify_image().
> > > > 
> > > > Any idea on how to debug further ?
> > > 
> > > You should be able to enable DEBUG_UART and get output that way.  But
> > > it's likely something related to the space constraints of the HS chip
> > > rather than GP.
> > > 
> > 
> > Hello
> > 
> > Thanks for your suggestion, I successfully got futher with:
> > diff --git a/drivers/core/lists.c b/drivers/core/lists.c
> > index b23ee3030e..415ba814f1 100644
> > --- a/drivers/core/lists.c
> > +++ b/drivers/core/lists.c
> > @@ -111,6 +111,8 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
> >         int result = 0;
> >         int pass;
> >  
> > +       debug_uart_init();
> > +
> >         /*
> >          * 10 passes is 10 levels deep in the devicetree, which is plenty. If
> >          * OF_PLATDATA_PARENT is not enabled, then bind_drivers_pass() will
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index b4805a2e4e..7ab059b4ea 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -158,6 +158,7 @@ config TPL_DM_SERIAL
> >  
> >  config DEBUG_UART
> >         bool "Enable an early debug UART for debugging"
> > +       default y
> >         help
> >           The debug UART is intended for use very early in U-Boot to debug
> >           problems when an ICE or other debug mechanism is not available.
> > @@ -185,7 +186,7 @@ config DEBUG_UART
> >  choice
> >         prompt "Select which UART will provide the debug UART"
> >         depends on DEBUG_UART
> > -       default DEBUG_UART_NS16550
> > +       default DEBUG_UART_OMAP
> >  
> >  config DEBUG_UART_ALTERA_JTAGUART
> >         bool "Altera JTAG UART"
> > @@ -406,7 +407,7 @@ endchoice
> >  config DEBUG_UART_BASE
> >         hex "Base address of UART"
> >         depends on DEBUG_UART
> > -       default 0 if DEBUG_UART_SANDBOX
> > +       default 0x44e09000
> >         help
> >           This is the base address of your UART for memory-mapped UARTs.
> >  
> > @@ -416,7 +417,7 @@ config DEBUG_UART_BASE
> >  config DEBUG_UART_CLOCK
> >         int "UART input clock"
> >         depends on DEBUG_UART
> > -       default 0 if DEBUG_UART_SANDBOX
> > +       default 48000000
> >         help
> >           The UART input clock determines the speed of the internal UART
> >           circuitry. The baud rate is derived from this by dividing the input
> > @@ -428,7 +429,7 @@ config DEBUG_UART_CLOCK
> >  config DEBUG_UART_SHIFT
> >         int "UART register shift"
> >         depends on DEBUG_UART
> > -       default 0 if DEBUG_UART
> > +       default 2
> >         help
> >           Some UARTs (notably ns16550) support different register layouts
> >           where the registers are spaced either as bytes, words or some other
> > @@ -449,6 +450,7 @@ config DEBUG_UART_BOARD_INIT
> >  config DEBUG_UART_ANNOUNCE
> >         bool "Show a message when the debug UART starts up"
> >         depends on DEBUG_UART
> > +       default y
> >         help
> >           Enable this option to show a message when the debug UART is ready
> >           for use. You will see a message like "<debug_uart> " as soon as
> > 
> > I got:
> > <debug_uart>-
> > <debug_uart>
> > alloc space exhausted
> 
> Looks like you need to increase SPL_SYS_MALLOC_F_LEN

Thanks
I have increased it by step of 0x1000, the number of "alloc space exhausted" was reducing, but now I have none of them (with 0x6000 or 0x7000) but still no boot.