[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 12:02 PM, Eran Matityahu <eran.m at variscite.com> wrote:
> 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.
>
By the way, in my opinion, the iMX6 way (and this patch also), is the
preferred way,
since usually you'll only need one MMC device in SPL.

Regards,
Eran

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