[U-Boot] Converting to SPL_OF_CONTROL

[Alex Kiernan]

On Wed, Aug 1, 2018 at 11:41 AM Alex Kiernan <alex.kiernan at gmail.com> wrote:
>
> I've long been trying to convert our board (AM3352) to all DM, after
> knocking out lots of problems, I've run into one I can't seem to
> configure my way around, when I enable SPL_OF_CONTROL.
>
> We have MMC2 as our boot device, with no MMC1 (and hence not present
> in the DTB). Once we're into full U-Boot this is all okay, we get the
> board's MMC2 appearing as mmc0 and everything's fine.
>
> My problem's during SPL, where the boot device is MMC2, the code tries
> to lookup by devnum (which is 1 because it numbers from 0 at this
> point), which doesn't exist. I'd expected aliases to work, but we
> don't seem to lookup up by alias, only index, so I don't have any way
> that I can see of overriding it.
>
> If I make this change, then everything works (with the addition of an
> an alias for mmc1 = &mmc2):
>
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 0b2f059570..b7544ba183 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -131,7 +131,7 @@ static int spl_mmc_find_device(struct mmc **mmcp,
> u32 boot_device)
>         }
>
>  #if CONFIG_IS_ENABLED(DM_MMC)
> -       err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
> +       err = uclass_get_device_by_seq(UCLASS_MMC, mmc_dev, &dev);
>         if (!err)
>                 *mmcp = mmc_get_mmc_dev(dev);
>  #else
>
> But that feels like a pretty fundamental change to make across every
> board and I've no real idea if that would even be the right thing to
> do, or if I should be trying to fix this some other way.
>

Thanks to Johan for pointing me at the Rockchip code; with that I can
get the board to use the right MMC in SPL, but I still have the same
problem, I'm booting from MMC2, but because all the numbering is by
devnum, not sequence, I end up with a board_boot_order function that's
the equivalent of this:

void board_boot_order(u32 *spl_boot_list)
{
        u32 boot_device = spl_boot_device();

        if (boot_device == BOOT_DEVICE_MMC2)
                spl_boot_list[0] = BOOT_DEVICE_MMC1;
        else
                spl_boot_list[0] = boot_device;
}

Which seems fundamentally wrong...

I'm assuming that the change I was originally playing with is a non-starter:

--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -131,7 +131,7 @@ static int spl_mmc_find_device(struct mmc **mmcp,
u32 boot_device)
        }

#if CONFIG_IS_ENABLED(DM_MMC)
-       err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
+       err = uclass_get_device_by_seq(UCLASS_MMC, mmc_dev, &dev);
        if (!err)
                *mmcp = mmc_get_mmc_dev(dev);
#else

Obviously I could guard that change with yet another symbol, but that
feels like it's papering over the problem... anyone got a pointer as
to what the right way to fix this is?

--
Alex Kiernan