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

Peng Fan
2019-08-09T00:58:06Z

> Subject: Re: [U-Boot] [PATCH 1/3] spl: imx6: Let spl_boot_device return
> USDHC1 or USDHC2
>
> On Wed, Aug 7, 2019 at 6:44 PM Ricardo Salveti <rsalveti@rsalveti.net>
> wrote:
> >
> > Hi Adam,
> >
> > On Thu, May 23, 2019 at 4:11 PM Adam Ford <aford173@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@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.
>
> >
> > Looking at mx6cuboxi.c in particular, fsl_esdhc_initialize will only
> > be called once as it already knows which MMC device to initialize
> > based on the BOOT_CFG register, causing the device mmc devnum to be 1
> > only. The issue shows up when booting SPL as find_mmc_device will look
> > for a matching dev_num index, which gets automatically increased when
> > fsl_esdhc_initialize gets called (and which is only called once in my
> > case, for MMC2).
> >
> > This is not an issue with imx6q_logic as at
> > 8f4691e31a18254d225524a4b018b8cbcddc70b1 you removed
> > fsl_esdhc_initialize, but all the other boards using MMC2 and doing
> > only one single call will have the same problem.
> >
> > Not sure if there is an easy fix here, but converting everything to
> > dynamic mmc initialization will required quite a bit of effort.

The short term workaround would be create multiple mmc in SPL stage.

For long term, it would be better to use DM.

Regards,
Peng.

>
> A bunch of boards have already started migrating to SPL_OF_CONTROL and
> DM_MMC. The imx6q_logic board was just a matter of changing some
> config options and fixing and/or creating a -u-boot.dtsi file to
> enable the various drivers in SPL, then removing the legacy code.
>
> Stefano, what would your preference be?
>
> adam
> >
> > Thanks,
> > --
> > Ricardo Salveti de Araujo