[U-Boot] [PATCH 1/3] spl: imx6: Let spl_boot_device return USDHC1 or USDHC2

[Ricardo Salveti]

On Thu, Aug 8, 2019 at 10:13 AM Adam Ford <aford173 at gmail.com> wrote:
> On Wed, Aug 7, 2019 at 6:44 PM Ricardo Salveti <rsalveti at rsalveti.net> wrote:
> > Hi Adam,
> >
> > On Thu, May 23, 2019 at 4:11 PM Adam Ford <aford173 at gmail.com> wrote:
> > >
> > > Currently, when the spl_boot_device checks the boot device, it
> > > will only return MMC1 when it's either sd or eMMC regardless
> > > of whether or not it's MMC1 or MMC2.  This is a problem when
> > > booting from MMC2 if MMC isn't being manually configured like in
> > > the DM_SPL case with SPL_OF_CONTROL.
> > >
> > > This patch will check the register and return either MMC1 or MMC2.
> > >
> > > Signed-off-by: Adam Ford <aford173 at gmail.com>
> > >
> > > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> > > index 9f1e0f6a72..1f230aca33 100644
> > > --- a/arch/arm/mach-imx/spl.c
> > > +++ b/arch/arm/mach-imx/spl.c
> > > @@ -24,6 +24,7 @@ u32 spl_boot_device(void)
> > >  {
> > >         unsigned int bmode = readl(&src_base->sbmr2);
> > >         u32 reg = imx6_src_get_boot_mode();
> > > +       u32 mmc_index = ((reg >> 11) & 0x03);
> > >
> > >         /*
> > >          * Check for BMODE if serial downloader is enabled
> > > @@ -84,11 +85,12 @@ u32 spl_boot_device(void)
> > >         /* SD/eSD: 8.5.3, Table 8-15  */
> > >         case IMX6_BMODE_SD:
> > >         case IMX6_BMODE_ESD:
> > > -               return BOOT_DEVICE_MMC1;
> > > -       /* MMC/eMMC: 8.5.3 */
> > >         case IMX6_BMODE_MMC:
> > >         case IMX6_BMODE_EMMC:
> > > -               return BOOT_DEVICE_MMC1;
> > > +               if (mmc_index == 1)
> > > +                       return BOOT_DEVICE_MMC2;
> > > +               else
> > > +                       return BOOT_DEVICE_MMC1;
> >
> > I just got to test v2019.10-rc1, which includes this change, and I'm
> > unable to boot SPL on my Hummingboard 2, which uses USDHC-2 for
> > sdcard.
>
> I wondered if it would break a board.  It's why I originally sent it
> as an RFC.  In my mind, it seems like we've created a system that only
> supports one MMC, then we created a work-around to correctly identify
> the MMC because the original implementation didn't.  I will admit, my
> patch only checks for MMC1 or MMC2, but I don't have hardware to test
> MMC3.
> >
> > Looks like this change breaks devices that are not using device
> > tree/dynamic mmc initialization, as the MMC index will not necessarily
> > be correct.
>
> How hard would it be to use the device tree/dynamic initialization for
> your board?  It seems to be the trend, and at least in some cases,
> they've made it required and boards that don't comply get removed.  It
> seems like we're prolonging.  I'd be open for an #ifdef hook around
> it, but I am not sure how that would fly with the maintainers.  My
> goal is to remove as much board-specific code as possible and move it
> to the shared code to reduce the overhead and code size. Checking for
> SPL_OF_CONTROL && DM_MMC might be potential work-around to the
> work-around.

I agree that moving to use device tree/dynamic initialization is the
way to go, but in this case we should probably have an ifdef as you
suggested, so we can avoid breaking the boot process while everything
gets migrated to dt.

Cheers,
-- 
Ricardo Salveti de Araujo