Re: [PATCH v2 1/2] spl: spl_nor: add BOOT_DEVICE_NOR2 as alternative SPL_LOAD_IMAGE_METHOD

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Fri, Jan 27, 2023 at 05:56:48PM +0100, Mario Kicherer wrote:
> Hello Tom,
> 
> On 2023-01-26 20:17, Tom Rini wrote:
> > This breaks a lot of platforms, as it only covers a few of the cases
> > where BOOT_DEVICE_NOR is listed. I would also really like to see how
> > this ends up being used in the board specific case as I do wonder if we
> > can't solve this some other way that won't have impact so many other
> > platforms.  Thanks!
> 
> Hm, I did a grep through the source code and that were the only places
> where the new value is used as part of an enum. BOOT_DEVICE_NOR is used
> in more places but they also #define this themselves - if I did not
> miss something.

Yes, all of the platforms that define the value (since it roughly means
"ROM set this value in something we can check") instead of enum'ing it
still compile that file and now fail to build.

> Furthermore, BOOT_DEVICE_NOR2 will not be used by default. A board has
> to define its own board_boot_order() function like this to use the new
> BOOT_DEVICE_NOR2 value:
> 
> void board_boot_order(u32 *spl_boot_list)
> {
> 	spl_boot_list[0] = BOOT_DEVICE_NOR;
> 	spl_boot_list[1] = BOOT_DEVICE_NOR2;
> }
> 
> If they do not explicitly add BOOT_DEVICE_NOR2, they should not be
> affected by this patch, as far as I understood the code. I tried to
> model this similar to the BOOT_DEVICE_MMC1 and _MMC2 values.
> 
> Then, they can also define an own spl_nor_get_uboot_base() like the
> following that should return an address where U-Boot tries to load
> a valid image:
> 
> unsigned long spl_nor_get_uboot_base(struct spl_boot_device *bootdev)
> {
>         if (bootdev->boot_device == BOOT_DEVICE_NOR) {
>                 /* first address that is tried */
>                 return UBOOT_PARTITION_1_IN_NOR;
>         else if (bootdev->boot_device == BOOT_DEVICE_NOR2) {
>                 /*
>                  * if loading of the first image failed for whatever
>                  * reason, try this address as well:
>                  */
>                 return UBOOT_PARTITION_2_IN_NOR;
>         }
> }
> 
> With this patch, it is possible to provide a fallback U-Boot image like
> above or to use an A/B slot scheme in case a bootloader update could be
> necessary in the field in the future.

Ah, OK. Keep in mind that MMC1/MMC2 are for different physical MMC
devices on a given platform. I think this is more like the case where
you should be able to override spl_nor_get_uboot_base at the board level
to say if you're loading A or B?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]