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

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

[Adam Ford]

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;
 	/* NAND Flash: 8.5.2, Table 8-10 */
 	case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
 		return BOOT_DEVICE_NAND;
-- 
2.17.1

[U-Boot] [PATCH 2/3] ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL

[Adam Ford]

With the spl code correctly returning either MMC1 or MMC2,
this board can not boot either from internal eMMC (MMC1) or
the uSD card on the baseboard (MMC2) using the device tree.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/configs/imx6q_logic_defconfig b/configs/imx6q_logic_defconfig
index 0bb2fc6bbf..07cdeb821b 100644
--- a/configs/imx6q_logic_defconfig
+++ b/configs/imx6q_logic_defconfig
@@ -4,6 +4,7 @@ CONFIG_SYS_TEXT_BASE=0x17800000
 CONFIG_SPL_GPIO_SUPPORT=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_TARGET_MX6LOGICPD=y
 CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
@@ -54,12 +55,16 @@ CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDIDS_DEFAULT="nand0=gpmi-nand"
 CONFIG_MTDPARTS_DEFAULT="mtdparts=gpmi-nand:2m(spl),2m(uboot),1m(env),16m(kernel),1m(dtb),-(fs)"
 CONFIG_CMD_UBI=y
+CONFIG_SPL_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="imx6q-logicpd"
 CONFIG_ENV_IS_IN_FAT=y
 CONFIG_ENV_IS_IN_NAND=y
 CONFIG_ENV_FAT_INTERFACE="mmc"
 CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
 CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
+CONFIG_SPL_DM=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_PCF8575_GPIO=y
 CONFIG_LED=y
 CONFIG_LED_GPIO=y
@@ -82,6 +87,7 @@ CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_MXC_UART=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
+# CONFIG_SPL_DM_USB is not set
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_MANUFACTURER="FSL"
 CONFIG_USB_GADGET_VENDOR_NUM=0x0525
-- 
2.17.1

[U-Boot] [PATCH 2/3] ARM: imx6q_logic: Enable SPL_DM with SPL_OF_CONTROL

[sbabic at denx.de]

> With the spl code correctly returning either MMC1 or MMC2,
> this board can not boot either from internal eMMC (MMC1) or
> the uSD card on the baseboard (MMC2) using the device tree.
> Signed-off-by: Adam Ford <aford173@gmail.com>
> diff --git a/configs/imx6q_logic_defconfig b/configs/imx6q_logic_defconfig
> index 0bb2fc6bbf..07cdeb821b 100644
> --- a/configs/imx6q_logic_defconfig
> +++ b/configs/imx6q_logic_defconfig
> @@ -4,6 +4,7 @@ CONFIG_SYS_TEXT_BASE=0x17800000
>  CONFIG_SPL_GPIO_SUPPORT=y
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> +CONFIG_SYS_MALLOC_F_LEN=0x2000
>  CONFIG_TARGET_MX6LOGICPD=y
>  CONFIG_SPL_MMC_SUPPORT=y
>  CONFIG_SPL_SERIAL_SUPPORT=y
> @@ -54,12 +55,16 @@ CONFIG_CMD_MTDPARTS=y
>  CONFIG_MTDIDS_DEFAULT="nand0=gpmi-nand"
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=gpmi-nand:2m(spl),2m(uboot),1m(env),16m(kernel),1m(dtb),-(fs)"
>  CONFIG_CMD_UBI=y
> +CONFIG_SPL_OF_CONTROL=y
>  CONFIG_DEFAULT_DEVICE_TREE="imx6q-logicpd"
>  CONFIG_ENV_IS_IN_FAT=y
>  CONFIG_ENV_IS_IN_NAND=y
>  CONFIG_ENV_FAT_INTERFACE="mmc"
>  CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
>  CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
> +CONFIG_SPL_DM=y
> +CONFIG_SPL_DM_SEQ_ALIAS=y
> +CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_PCF8575_GPIO=y
>  CONFIG_LED=y
>  CONFIG_LED_GPIO=y
> @@ -82,6 +87,7 @@ CONFIG_DM_REGULATOR_GPIO=y
>  CONFIG_MXC_UART=y
>  CONFIG_USB=y
>  CONFIG_DM_USB=y
> +# CONFIG_SPL_DM_USB is not set
>  CONFIG_USB_GADGET=y
>  CONFIG_USB_GADGET_MANUFACTURER="FSL"
>  CONFIG_USB_GADGET_VENDOR_NUM=0x0525

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 3/3] ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init

[Adam Ford]

Since the board uses SPL_OF_CONTROL now, we don't need to
explicitly initialize the MMC driver, but we still need to
pinmux the corresponding pins.  This patch removes the
initialization code and leave just the muxing behind.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/board/logicpd/imx6/imx6logic.c b/board/logicpd/imx6/imx6logic.c
index b17a3b1d39..ab1c7f266e 100644
--- a/board/logicpd/imx6/imx6logic.c
+++ b/board/logicpd/imx6/imx6logic.c
@@ -222,25 +222,15 @@ int board_mmc_init(bd_t *bis)
 	switch (reg) {
 	case 0:
 		SETUP_IOMUX_PADS(usdhc1_pads);
-		usdhc_cfg[0].esdhc_base = USDHC1_BASE_ADDR;
-		usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK);
-		gd->arch.sdhc_clk = usdhc_cfg[0].sdhc_clk;
 		break;
 	case 1:
 		SETUP_IOMUX_PADS(usdhc2_pads);
-		usdhc_cfg[1].esdhc_base = USDHC2_BASE_ADDR;
-		usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
-		gd->arch.sdhc_clk = usdhc_cfg[1].sdhc_clk;
 		break;
 	}
 
-	return fsl_esdhc_initialize(bis, &usdhc_cfg[reg]);
+	return 0;
 }
 
-int board_mmc_getcd(struct mmc *mmc)
-{
-	return 1;
-}
 #endif
 
 static void ccgr_init(void)
-- 
2.17.1

[U-Boot] [PATCH 3/3] ARM: imx6q_logic: With SPL_OF_CONTROL enabled, remove MMC init

[sbabic at denx.de]

> Since the board uses SPL_OF_CONTROL now, we don't need to
> explicitly initialize the MMC driver, but we still need to
> pinmux the corresponding pins.  This patch removes the
> initialization code and leave just the muxing behind.
> Signed-off-by: Adam Ford <aford173@gmail.com>
> diff --git a/board/logicpd/imx6/imx6logic.c b/board/logicpd/imx6/imx6logic.c
> index b17a3b1d39..ab1c7f266e 100644
> --- a/board/logicpd/imx6/imx6logic.c
> +++ b/board/logicpd/imx6/imx6logic.c
> @@ -222,25 +222,15 @@ int board_mmc_init(bd_t *bis)
>  	switch (reg) {
>  	case 0:
>  		SETUP_IOMUX_PADS(usdhc1_pads);
> -		usdhc_cfg[0].esdhc_base = USDHC1_BASE_ADDR;
> -		usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK);
> -		gd->arch.sdhc_clk = usdhc_cfg[0].sdhc_clk;
>  		break;
>  	case 1:
>  		SETUP_IOMUX_PADS(usdhc2_pads);
> -		usdhc_cfg[1].esdhc_base = USDHC2_BASE_ADDR;
> -		usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
> -		gd->arch.sdhc_clk = usdhc_cfg[1].sdhc_clk;
>  		break;
>  	}
>  
> -	return fsl_esdhc_initialize(bis, &usdhc_cfg[reg]);
> +	return 0;
>  }
>  
> -int board_mmc_getcd(struct mmc *mmc)
> -{
> -	return 1;
> -}
>  #endif
>  
>  static void ccgr_init(void)

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

[Adam Ford]

On Thu, May 23, 2019 at 2: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>
>

Stefano,
Since the release was just cut, is there any way this can be applied?

adam
> 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;
>         /* NAND Flash: 8.5.2, Table 8-10 */
>         case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
>                 return BOOT_DEVICE_NAND;
> --
> 2.17.1
>

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

[Stefano Babic]

On 15/07/19 15:58, Adam Ford wrote:
> On Thu, May 23, 2019 at 2: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>
>>
> 
> Stefano,
> Since the release was just cut, is there any way this can be applied?

Yes - I am just busy with some projects, I had just to find a slot to
apply imx's patches.

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

[sbabic at denx.de]

> 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;
>  	/* NAND Flash: 8.5.2, Table 8-10 */
>  	case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
>  		return BOOT_DEVICE_NAND;

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

[Ricardo Salveti]

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.

Looks like this change breaks devices that are not using device
tree/dynamic mmc initialization, as the MMC index will not necessarily
be correct.

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.

Thanks,
-- 
Ricardo Salveti de Araujo

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

[Adam Ford]

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.

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

[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@gmail.com> wrote:
> 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.

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

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

[Peng Fan]

> 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