[PATCH] board: ti: am43xx: pass boot device information from SPL to U-Boot proper

[Simon Glass]

Hi,

On Tue, 30 Nov 2021 at 00:26, Josef Luštický <josef at lusticky.cz> wrote:
>>
>>
>> Hi Josef
>>
>> On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky <josef at lusticky.cz> wrote:
>> >
>> > TI AM43xx SoC supports various boot devices (peripherals).
>> > There is already handoff mechanism prepared to allow passing
>> > the information which boot device was used to load the SPL.
>> >
>> > Use the handoff mechanism to pass this information to U-Boot proper
>> > and set the "boot_device" environment variable in board_late_init.
>> >
>> > Signed-off-by: Josef Lusticky <josef at lusticky.cz>
>> > Cc: Tom Rini <trini at konsulko.com>
>> > Cc: Lokesh Vutla <lokeshvutla at ti.com>
>> > Cc: Michael Trimarchi <michael at amarulasolutions.com>
>> > ---
>> >
>> > I use the boot_device variable later in U-Boot scripting - e.g. to avoid running
>> > bootcmd when the SPL was loaded from UART but run it when loaded from MMC.
>> > Only AM43xx is supported by this patch, but for other TI SoCs
>> > the procedure should be the same:
>> > - figure out supported boot devices from arch/arm/include/asm/arch-am33xx/spl.h
>> > or arch/arm/include/asm/arch-omapX/spl.h
>> > - implement setting the boot_device env variable in board_late_init()
>> >
>> > You'll need to enable the following in the config:
>> > CONFIG_BLOBLIST=y (required by CONFIG_HANDOFF)
>> > CONFIG_HANDOFF=y
>> > CONFIG_BLOBLIST_ADDR=0x87000000 (i set this based on other values defined by
>> > the DEFAULT_LINUX_BOOT_ENV macro in include/configs/ti_armv7_common.h, you
>> > may want to use a different address)
>> >
>> >  arch/arm/include/asm/handoff.h    |  3 +++
>> >  arch/arm/mach-omap2/boot-common.c |  9 ++++++++
>> >  board/ti/am43xx/board.c           | 38 +++++++++++++++++++++++++++++++
>> >  3 files changed, 50 insertions(+)
>> >
>> > diff --git a/arch/arm/include/asm/handoff.h b/arch/arm/include/asm/handoff.h
>> > index 0790d2ab1e..1b7aa432a2 100644
>> > --- a/arch/arm/include/asm/handoff.h
>> > +++ b/arch/arm/include/asm/handoff.h
>> > @@ -16,6 +16,9 @@
>> >   */
>> >  struct arch_spl_handoff {
>> >         ulong usable_ram_top;
>> > +#ifdef CONFIG_ARCH_OMAP2PLUS
>> > +       u32 omap_boot_device;
>> > +#endif
>> >  };
>>
>> Simon is working on a more structured way to pass arguments in
>> multi-stage boot. I forget to read all the patches.
>> Anyway adding a specific handoff parameter for one architecture makes
>> no such sense. I will remind you that this
>> was already implemented in the past using dts injection (something
>> that I don't  like)
>>
> Hi Michael,
> thank you for your response.
>
> I agree that such support for a single architecture makes no such sense.

Note that the more structured approach is in fact using a bloblist,
and SPL handoff can just be one of the blobs. So this patch is not
actually in conflict with that, and will 'just work' when standard
flow is in there.

As to the arch-specific nature of this, I would rather handle it by
either avoiding the #ifdef (so the field is unused on other baords) or
adding a new level of include for arch-specific handoff info, so that
arch_spl_handoff includes mach_spl_handoff, for example.

But other than that, and the comments below, it seems OK to me.

[..]

Regards,
Simon