[PATCH v2 2/4] mx6cuboxi: customize board_boot_order to access eMMC

[Walter Lozano <walter.lozano@collabora.com>]

On 16/3/20 13:28, Baruch Siach wrote:
> Hi Walter,
>
> On Thu, Mar 12, 2020 at 01:52:13PM -0300, Walter Lozano wrote:
>> Hi Baruch,
>>
>> Thanks for sharing.
>>
>> On 12/3/20 02:02, Baruch Siach wrote:
>>> Hi Walter,
>>>
>>> On Wed, Mar 11 2020, Walter Lozano wrote:
>>>> In SPL legacy code only one MMC device is created, based on BOOT_CFG
>>>> register, which can be either SD or eMMC. In this context
>>>> board_boot_order return always MMC1 when configure to boot from
>>>> SD/eMMC. After switching to DM both SD and eMMC devices are created
>>>> based on the information available on DT, but as board_boot_order
>>>> only returns MMC1 is not possible to boot from eMMC.
>>>>
>>>> This patch customizes board_boot_order taking into account BOOT_CFG
>>>> register to point to correct MMC1 / MMC2 device. Additionally, handle
>>>> IO mux for the desired boot device.
>>>>
>>>> Signed-off-by: Walter Lozano <walter.lozano@collabora.com>
>>>> ---
>>>>    board/solidrun/mx6cuboxi/mx6cuboxi.c | 49 ++++++++++++++++++++++++++++
>>>>    1 file changed, 49 insertions(+)
>>>>
>>>> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>> index 6a96f9ecdb..9bf3645f72 100644
>>>> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>> @@ -435,6 +435,7 @@ int board_early_init_f(void)
>>>>    #ifdef CONFIG_CMD_SATA
>>>>    	setup_sata();
>>>>    #endif
>>>> +
>>> This hunk should not be part of this commit.
>> Thanks for pointing to this silly hunk. I will prepare a V3.
>>
>>> Looks good to me, otherwise.
>>>
>>> I can't test at the moment. Have you tested boot from both SD card and eMMC?
>> Most of the work was done booting from SD. In order to test booting from
>> eMMC, as I have some specific eFUSE configs, I tweaked board_boot_order to
>> force booting from eMMC.
> But that does not cover SPL boot from eMMC, right?

Basically I think this approach should cover the necessary steps. To be 
more clear about my tweak

1- BootROM loads SPL from SD

2- SPL is tweaked to load U-Boot from eMMC, and in this way test its 
support on SPL

> Anyway I tested your patches here on real hardware with unfused SOM and
> SD/eMMC boot select jumpers.

Thank you much for taking the time to test these patches in you board. I 
really appreciate your help

> Tested-by: Baruch Siach <baruch@tkos.co.il>
Thanks. I'll add the tag to the v3.
>> Regarding booting from SD/eMMC, I also wonder if having a list of boot
>> devices would be useful, like
>>
>> 	switch (boot_mode >> IMX6_BMODE_SHIFT) {
>> 	case IMX6_BMODE_SD:
>> 	case IMX6_BMODE_ESD:
>> 	case IMX6_BMODE_MMC:
>> 	case IMX6_BMODE_EMMC:
>> 		SETUP_IOMUX_PADS(usdhc2_pads);
>> 		SETUP_IOMUX_PADS(usdhc3_pads);
>>
>> 		/*
>> 		 * Upon reading BOOT_CFG register the following map is done:
>> 		 * Bit 11 and 12 of BOOT_CFG register can determine the current
>> 		 * mmc port
>> 		 * 0x1                  SD2
>> 		 * 0x2                  SD3
>> 		 */
>>
>> 		reg &= 0x3; /* Only care about bottom 2 bits */
>> 		switch (reg) {
>> 		case 1:
>> 			spl_boot_list[0] = BOOT_DEVICE_MMC1;
>> 			spl_boot_list[1] = BOOT_DEVICE_MMC2;
>> 			break;
>> 		case 2:
>> 			spl_boot_list[0] = BOOT_DEVICE_MMC2;
>> 			spl_boot_list[1] = BOOT_DEVICE_MMC1;
>> 			break;
>> 		}
>> 		break;
>>
>> What do you think?
> This might cause surprising results when the U-Boot image on the SPL boot
> device is damaged for some reason. This kind of fallback device (other than
> USB boot) should not be enabled by default, in my opinion.

Thanks for sharing your opinion. I agree with you, but I was still 
wondering what were your thoughts about this.

Regards,

Walter

> baruch
>
>>> In the future, please keep i.MX maintainers (Stefano, Fabio) in Cc.
>>
>> Thanks for the advice. You are totally right.
>>
>> Regards,
>>
>> Walter
>>
>>> Thanks,
>>> baruch
>>>
>>>>    	return 0;
>>>>    }
>>>> @@ -624,6 +625,54 @@ int board_fit_config_name_match(const char *name)
>>>>    	return strcmp(name, tmp_name);
>>>>    }
>>>> +void board_boot_order(u32 *spl_boot_list)
>>>> +{
>>>> +	struct src *psrc = (struct src *)SRC_BASE_ADDR;
>>>> +	unsigned int reg = readl(&psrc->sbmr1) >> 11;
>>>> +	u32 boot_mode = imx6_src_get_boot_mode() & IMX6_BMODE_MASK;
>>>> +	unsigned int bmode = readl(&src_base->sbmr2);
>>>> +
>>>> +	/* If bmode is serial or USB phy is active, return serial */
>>>> +	if (((bmode >> 24) & 0x03) == 0x01 || is_usbotg_phy_active()) {
>>>> +		spl_boot_list[0] = BOOT_DEVICE_BOARD;
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	switch (boot_mode >> IMX6_BMODE_SHIFT) {
>>>> +	case IMX6_BMODE_SD:
>>>> +	case IMX6_BMODE_ESD:
>>>> +	case IMX6_BMODE_MMC:
>>>> +	case IMX6_BMODE_EMMC:
>>>> +		/*
>>>> +		 * Upon reading BOOT_CFG register the following map is done:
>>>> +		 * Bit 11 and 12 of BOOT_CFG register can determine the current
>>>> +		 * mmc port
>>>> +		 * 0x1                  SD2
>>>> +		 * 0x2                  SD3
>>>> +		 */
>>>> +
>>>> +		reg &= 0x3; /* Only care about bottom 2 bits */
>>>> +		switch (reg) {
>>>> +		case 1:
>>>> +			SETUP_IOMUX_PADS(usdhc2_pads);
>>>> +			spl_boot_list[0] = BOOT_DEVICE_MMC1;
>>>> +			break;
>>>> +		case 2:
>>>> +			SETUP_IOMUX_PADS(usdhc3_pads);
>>>> +			spl_boot_list[0] = BOOT_DEVICE_MMC2;
>>>> +			break;
>>>> +		}
>>>> +		break;
>>>> +	default:
>>>> +		/* By default use USB downloader */
>>>> +		spl_boot_list[0] = BOOT_DEVICE_BOARD;
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	/* As a last resort, use serial downloader */
>>>> +	spl_boot_list[1] = BOOT_DEVICE_BOARD;
>>>> +}
>>>> +
>>>>    #ifdef CONFIG_SPL_BUILD
>>>>    #include <asm/arch/mx6-ddr.h>
>>>>    static const struct mx6dq_iomux_ddr_regs mx6q_ddr_ioregs = {