| Message ID | 20211126095646.39998-1-josef@lusticky.cz |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | board: ti: am43xx: pass boot device information from SPL to U-Boot proper | expand |
Hi Josef On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky <josef@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@lusticky.cz> > Cc: Tom Rini <trini@konsulko.com> > Cc: Lokesh Vutla <lokeshvutla@ti.com> > Cc: Michael Trimarchi <michael@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) Michael > > #endif > diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c > index 1268a32503..191bb2a42d 100644 > --- a/arch/arm/mach-omap2/boot-common.c > +++ b/arch/arm/mach-omap2/boot-common.c > @@ -236,3 +236,12 @@ void arch_preboot_os(void) > ahci_reset((void __iomem *)DWC_AHSATA_BASE); > } > #endif > + > +#if CONFIG_IS_ENABLED(HANDOFF) > +int handoff_arch_save(struct spl_handoff *ho) > +{ > + ho->arch.omap_boot_device = spl_boot_device(); > + > + return 0; > +} > +#endif > diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c > index a71b588efc..8c5834fc25 100644 > --- a/board/ti/am43xx/board.c > +++ b/board/ti/am43xx/board.c > @@ -726,6 +726,44 @@ static int device_okay(const char *path) > int board_late_init(void) > { > struct udevice *dev; > + > +#if CONFIG_IS_ENABLED(HANDOFF) > + /* Read peripheral SPL was loaded from */ > + if (gd->spl_handoff) { > + switch (gd->spl_handoff->arch.omap_boot_device) { > + case BOOT_DEVICE_CPGMAC: > + env_set("boot_device", "cpgmac"); > + break; > + case BOOT_DEVICE_MMC1: > + env_set("boot_device", "mmc1"); > + break; > + case BOOT_DEVICE_MMC2: > + env_set("boot_device", "mmc2"); > + break; > + case BOOT_DEVICE_NAND: > + env_set("boot_device", "nand"); > + break; > + case BOOT_DEVICE_NOR: > + env_set("boot_device", "nor"); > + break; > + case BOOT_DEVICE_SPI: > + env_set("boot_device", "spi"); > + break; > + case BOOT_DEVICE_UART: > + env_set("boot_device", "uart"); > + break; > + case BOOT_DEVICE_USB: > + env_set("boot_device", "usb"); > + break; > + case BOOT_DEVICE_USBETH: > + env_set("boot_device", "usbeth"); > + break; > + default: > + env_set("boot_device", "unknown"); > + } > + } > +#endif > + > #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG > set_board_info_env(NULL); > > -- > 2.30.2 >
> > > Hi Josef > > On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky <josef@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@lusticky.cz> > > Cc: Tom Rini <trini@konsulko.com> > > Cc: Lokesh Vutla <lokeshvutla@ti.com> > > Cc: Michael Trimarchi <michael@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. I tried to make the same thing work on SUNXI and Atmel platforms. Atmel uses its at91bootstrap, so it's rather complicated and would require modification of both U-Boot and at91bootstrap. The SUNXI platform worked the exact same way, but only for SPI NOR and MMC boot_device. I've got Allwinner A13 to test and apart from SPI NOR and MMC, all SUNXI SoCs also support USB "FEL" boot. However I was not able to make my patch work for boot_device FEL detection, because on this particular platform it works a bit different: The SPL is transferred over USB and booted from FEL. After the clock setup, pinmux setup, etc. the SPL detects that it should continue booting from FEL and it returns back to FEL which loads U-Boot proper next. Unfortunately the return back to FEL happens before the HANDOFF is set. I patched the code to return to FEL after the HANDOFF, but I am almost certain that it would break other architectures. Here's the code change: diff --git a/common/spl/spl.c b/common/spl/spl.c index a0a608fd77..b1f872ad0b 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -708,12 +708,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2) spl_image.boot_device = BOOT_DEVICE_NONE; board_boot_order(spl_boot_list); - if (boot_from_devices(&spl_image, spl_boot_list, - ARRAY_SIZE(spl_boot_list))) { - puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n"); - hang(); - } - spl_perform_fixups(&spl_image); if (CONFIG_IS_ENABLED(HANDOFF)) { ret = write_spl_handoff(); @@ -728,6 +722,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2) ret); } + if (boot_from_devices(&spl_image, spl_boot_list, + ARRAY_SIZE(spl_boot_list))) { + puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n"); + hang(); + } + #ifdef CONFIG_CPU_V7M spl_image.entry_point |= 0x1; #endif Best regards Josef > Michael > > > > > #endif > > diff --git a/arch/arm/mach-omap2/boot-common.c > b/arch/arm/mach-omap2/boot-common.c > > index 1268a32503..191bb2a42d 100644 > > --- a/arch/arm/mach-omap2/boot-common.c > > +++ b/arch/arm/mach-omap2/boot-common.c > > @@ -236,3 +236,12 @@ void arch_preboot_os(void) > > ahci_reset((void __iomem *)DWC_AHSATA_BASE); > > } > > #endif > > + > > +#if CONFIG_IS_ENABLED(HANDOFF) > > +int handoff_arch_save(struct spl_handoff *ho) > > +{ > > + ho->arch.omap_boot_device = spl_boot_device(); > > + > > + return 0; > > +} > > +#endif > > diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c > > index a71b588efc..8c5834fc25 100644 > > --- a/board/ti/am43xx/board.c > > +++ b/board/ti/am43xx/board.c > > @@ -726,6 +726,44 @@ static int device_okay(const char *path) > > int board_late_init(void) > > { > > struct udevice *dev; > > + > > +#if CONFIG_IS_ENABLED(HANDOFF) > > + /* Read peripheral SPL was loaded from */ > > + if (gd->spl_handoff) { > > + switch (gd->spl_handoff->arch.omap_boot_device) { > > + case BOOT_DEVICE_CPGMAC: > > + env_set("boot_device", "cpgmac"); > > + break; > > + case BOOT_DEVICE_MMC1: > > + env_set("boot_device", "mmc1"); > > + break; > > + case BOOT_DEVICE_MMC2: > > + env_set("boot_device", "mmc2"); > > + break; > > + case BOOT_DEVICE_NAND: > > + env_set("boot_device", "nand"); > > + break; > > + case BOOT_DEVICE_NOR: > > + env_set("boot_device", "nor"); > > + break; > > + case BOOT_DEVICE_SPI: > > + env_set("boot_device", "spi"); > > + break; > > + case BOOT_DEVICE_UART: > > + env_set("boot_device", "uart"); > > + break; > > + case BOOT_DEVICE_USB: > > + env_set("boot_device", "usb"); > > + break; > > + case BOOT_DEVICE_USBETH: > > + env_set("boot_device", "usbeth"); > > + break; > > + default: > > + env_set("boot_device", "unknown"); > > + } > > + } > > +#endif > > + > > #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG > > set_board_info_env(NULL); > > > > -- > > 2.30.2 > > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com >
Hi, On Tue, 30 Nov 2021 at 00:26, Josef Luštický <josef@lusticky.cz> wrote: >> >> >> Hi Josef >> >> On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky <josef@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@lusticky.cz> >> > Cc: Tom Rini <trini@konsulko.com> >> > Cc: Lokesh Vutla <lokeshvutla@ti.com> >> > Cc: Michael Trimarchi <michael@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
> > > Hi, > > On Tue, 30 Nov 2021 at 00:26, Josef Luštický <josef@lusticky.cz> wrote: > >> > >> > >> Hi Josef > >> > >> On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky <josef@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@lusticky.cz> > >> > Cc: Tom Rini <trini@konsulko.com> > >> > Cc: Lokesh Vutla <lokeshvutla@ti.com> > >> > Cc: Michael Trimarchi <michael@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. > Thank you for your comments, Simon. I find the split of arch_spl_handoff as a good idea. I am also curious about how other platforms would work with such a way of passing boot_device information and other data. There are some more boards on my table, so I'll try to make a similar feature work on those boards to see if the approach can be used in general. But this will take some time... > [..] > > Regards, > Simon >
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 }; #endif diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index 1268a32503..191bb2a42d 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -236,3 +236,12 @@ void arch_preboot_os(void) ahci_reset((void __iomem *)DWC_AHSATA_BASE); } #endif + +#if CONFIG_IS_ENABLED(HANDOFF) +int handoff_arch_save(struct spl_handoff *ho) +{ + ho->arch.omap_boot_device = spl_boot_device(); + + return 0; +} +#endif diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c index a71b588efc..8c5834fc25 100644 --- a/board/ti/am43xx/board.c +++ b/board/ti/am43xx/board.c @@ -726,6 +726,44 @@ static int device_okay(const char *path) int board_late_init(void) { struct udevice *dev; + +#if CONFIG_IS_ENABLED(HANDOFF) + /* Read peripheral SPL was loaded from */ + if (gd->spl_handoff) { + switch (gd->spl_handoff->arch.omap_boot_device) { + case BOOT_DEVICE_CPGMAC: + env_set("boot_device", "cpgmac"); + break; + case BOOT_DEVICE_MMC1: + env_set("boot_device", "mmc1"); + break; + case BOOT_DEVICE_MMC2: + env_set("boot_device", "mmc2"); + break; + case BOOT_DEVICE_NAND: + env_set("boot_device", "nand"); + break; + case BOOT_DEVICE_NOR: + env_set("boot_device", "nor"); + break; + case BOOT_DEVICE_SPI: + env_set("boot_device", "spi"); + break; + case BOOT_DEVICE_UART: + env_set("boot_device", "uart"); + break; + case BOOT_DEVICE_USB: + env_set("boot_device", "usb"); + break; + case BOOT_DEVICE_USBETH: + env_set("boot_device", "usbeth"); + break; + default: + env_set("boot_device", "unknown"); + } + } +#endif + #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG set_board_info_env(NULL);
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@lusticky.cz> Cc: Tom Rini <trini@konsulko.com> Cc: Lokesh Vutla <lokeshvutla@ti.com> Cc: Michael Trimarchi <michael@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(+)