[U-Boot] [PATCH 1/2] imx7: spl: Use SPL boot device MMC1 for all of the SOCs MMC/SD boot devices

[Eran Matityahu]

On Thu, Jan 4, 2018 at 11:14 AM, Stefano Babic <sbabic at denx.de> wrote:
> Hi Eran,
>
> On 03/01/2018 14:58, Eran Matityahu wrote:
>> Hi Uri.
>>
>>> Hello Eran,
>>>
>>> On 01/03/2018 12:53 PM, Eran Matityahu wrote:
>>>>
>>>> Use only one SPL MMC device, similarly to the iMX6 code
>>>
>>>
>>> What is the reason for not using MMC2?
>>
>> The reason is so that you won't have to initialize more than one MMC
>> device in SPL.
>> Also, to be consistent with the iMX6 SPL code.
>>
>>>
>>>>
>>>> Signed-off-by: Eran Matityahu <eran.m at variscite.com>
>>>> ---
>>>>   arch/arm/mach-imx/spl.c | 3 +--
>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
>>>> index d0d1b73aa6..6b5bd8990c 100644
>>>> --- a/arch/arm/mach-imx/spl.c
>>>> +++ b/arch/arm/mach-imx/spl.c
>>>> @@ -106,10 +106,9 @@ u32 spl_boot_device(void)
>>>>         switch (boot_device_spl) {
>>>>         case SD1_BOOT:
>>>>         case MMC1_BOOT:
>>>> -               return BOOT_DEVICE_MMC1;
>>>>         case SD2_BOOT:
>>>>         case MMC2_BOOT:
>>>> -               return BOOT_DEVICE_MMC2;
>>>> +               return BOOT_DEVICE_MMC1;
>>>>         case SPI_NOR_BOOT:
>>>>                 return BOOT_DEVICE_SPI;
>>>>         default:
>
> The reason to have spl_boot_device() is not to initialize more as one
> MMC device, but to find which storage contains the next image to be
> started (u-boot.img). This is generally (but not in all projects) the
> same storage from where the BootROM has loaded SPL.
>
> According to this, this patch seems wrong. If SPL / u-boot.img are
> stored on MMC2 (and maybe MMC2 is the only MMC device for the board),
> your patch breaks booting.
>
> If you have special case, you can write a board_boot_order() in your
> board code to overwrite the behavior.
>
> Best regards,
> Stefano Babic

The iMX6 spl_boot_device() doesn't even check which MMC device the
BootROM has loaded SPL from. It just returns BOOT_DEVICE_MMC1
in case the boot device was any MMC/SD device, and leaves it to the
board code to detect the exact device and init the appropriate device
with the next image (u-boot,img), accordingly.
My suggestion is to do the same here.

In my iMX7 board, I can boot from MMC1 (SD card) and MMC3 (eMMC),
but let's say it's MMC2 in sake of this explanation.
Without this patch, in order to boot from MMC2 (with both SPL and u-boot.img
on MMC2), I have to initialize both MMC1 and MMC2 devices because SPL
loops on all devices until it finds a match, and it halts if the first
device is not
initialized.

With this patch I can use get_boot_device() inside board_mmc_init() and
only initialize the MMC device I want to load the next image from (usually
the same device).

I know I can approach it differently and change the spl_boot_list[0] device to
BOOT_DEVICE_MMC1 in board_boot_order(), but I figured the behaviour
should be the same for iMX6 and iMX7.
If you think the correct way is to return BOOT_DEVICE_MMC2, then we
should probably also add a BOOT_DEVICE_MMC3 definition, and also change
the iMX6 code to do the same.

>
> --
> =====================================================================
> 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
> =====================================================================