[U-Boot] [PATCH v3 04/11] mx6: add boot device support for SPL

[Igor Grinberg]

Hi Tim,

On 05/29/14 07:11, Tim Harvey wrote:
> On Wed, May 28, 2014 at 9:38 AM, Nikita Kiryanov <nikita at compulab.co.il> wrote:
>> Hi Tim,
>> Sorry for the late reply.
> 
> no worries - your the only review of this revision thus far ;)
> 
>>
>>
>> On 08/05/14 08:16, Tim Harvey wrote:
>>>
>>> Add a common spl.c file to support boot device functions needed for SPL
>>> such as detecting the boot device.
>>>
>>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>>> ---
>>> v3:
>>>   - added Freescale copyright
>>>
>>> v2:
>>>   - re-base on top of Masahiro Yamada's consolidation patch:
>>>     http://patchwork.ozlabs.org/patch/341817/
>>>   - moved spl_boot_device() and get_boot_mode() into own common imx spl.c
>>> file
>>>   - use mem-mapped structure for smbr reg access
>>> ---
>>
>>
>> [..snip..]
>>
>>
>>> +               }
>>> +               break;
>>> +       /* SD/eSD: 8.5.3, Table 8-15  */
>>> +       case 0x4:
>>> +       case 0x5:
>>> +               return BOOT_DEVICE_MMC1;
>>> +       /* MMC/eMMC: 8.5.3 */
>>> +       case 0x6:
>>> +       case 0x7:
>>> +               return BOOT_DEVICE_MMC1;
>>> +       /* NAND Flash: 8.5.2 */
>>> +       case 0x8 ... 0xf:
>>> +               return BOOT_DEVICE_NAND;
>>> +       }
>>> +       return BOOT_DEVICE_NONE;
>>> +}
>>> +#endif
>>> +
>>> +#if defined(CONFIG_SPL_MMC_SUPPORT)
>>> +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT)
>>> */
>>> +u32 spl_boot_mode(void)
>>> +{
>>> +       switch (spl_boot_device()) {
>>> +       /* for MMC return either RAW or FAT mode */
>>> +       case BOOT_DEVICE_MMC1:
>>> +       case BOOT_DEVICE_MMC2:
>>
>>
>> spl_boot_device() returns only BOOT_DEVICE_MMC1. Was the MMC/eMMC case
>> supposed to return a BOOT_DEVICE_MMC2?
> 
> Its hard to say - I think the use of BOOT_DEVICE_* is very
> implementation specific. I don't quite understand why there is a
> BOOT_DEVICE_MMC{1,2,2_2} defined in arch/arm/include/asm/spl.h - the
> BOOT_DEVICE_MMC2_2 is certainly quite odd.

AFAIK, this is needed to support eMMC boot partitions which are physical
and not logical, but they are still within the same device.

> In uboot/common/spl/spl.c
> they are all treated the same and spl_mmc_load_image() always assumes
> the 'first' mmc device anyway.
> 
> In the IMX6 case I'm not sure an SD/uSD card would be treated any
> different than an MMC card so it would come down to which specific
> device is flagged as the boot device, of which there are 4 sdhc's. So
> eventually, one might need to add more BOOT_DEVICE_MMC enums if that
> is to be supported. For now, I'm keeping it simple and there is an
> issue with IMX6 spl_mmc_load_image() anyway which I plan on addressing
> later.
> 
> Tim
> 
>>
>> Aside from this nitpick,
>> Acked-by: Nikita Kiryanov <nikita at compulab.co.il>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

-- 
Regards,
Igor.