[U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get mmc handler

[Kever Yang]

Hi Peng,

On 2019/7/30 上午9:30, Peng Fan wrote:
>> Subject: Re: [U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get
>> mmc handler
>>
>> +Peng
>>
>> Hi Kever,
>>
>> On 25/07/19 3:10 PM, Kever Yang wrote:
>>> Like cmd/mmc.c, the spl_mmc.c are using block driver interface like
>>> blk_dread() to access mmc devices, we need to get the mmc device
>>> handler by block driver interface so that we can always get correct
>>> device number, eg. the alias may change the device number which make
>>> the device number different in UCLASS_MMC list and UCLASS_BLK list.
> It is unclear to me.
> spl_mmc_get_device_index will get the mmc_dev number,
> Then uclass_get_device will use it to get the mmc device.
>
> You mean this not work on your platform?


The code as-is now works on my platform.
It's true that spl_mmc_get_device_index will get the mmc_dev number ,
but the BOOT_DEVICE_MMC1 and BOOT_DEVICE_MMC2 does not have enough 
information
about this is a SD card or a eMMC device.
So we want to enable the feature of alias in dts to identify eMMC and SD 
card, which already
used in U-Boot proper, and this is implemented in mmc_bind() and change 
the devnum
for blk_create_devicef(), which may lead to the devnum in UCLASS_MMC 
list and in UCLASS_BLK
list are different. In this case, if you get the devnum from UCLASS_MMC, 
you will get the wrong
device, and the right one should be from UCLASS_BLK.
I should send the patch for enable mmc alias in SPL at the same time, 
which may help you
understand.


Thanks,
- Kever
>
> And mmc device and blk device are connected with the uclass_platdata of
> the mmc device, so from mmc, you could always get the correct blk device.
>
> Please share more info about your issue/fix.
>
> Regards,
> Peng.
>
>
>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>>> ---
>>>
>>>   common/spl/spl_mmc.c | 9 ---------
>>>   1 file changed, 9 deletions(-)
>>>
>>> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index
>>> b3619889f7..3a93e20f04 100644
>>> --- a/common/spl/spl_mmc.c
>>> +++ b/common/spl/spl_mmc.c
>>> @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32
>>> boot_device)
>>>
>>>   static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)  {
>>> -#if CONFIG_IS_ENABLED(DM_MMC)
>>> -	struct udevice *dev;
>>> -#endif
>>>   	int err, mmc_dev;
>>>
>>>   	mmc_dev = spl_mmc_get_device_index(boot_device);
>>> @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc
>> **mmcp, u32 boot_device)
>>>   		return err;
>>>   	}
>>>
>>> -#if CONFIG_IS_ENABLED(DM_MMC)
>>> -	err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
>>> -	if (!err)
>>> -		*mmcp = mmc_get_mmc_dev(dev);
>>> -#else
>>>   	*mmcp = find_mmc_device(mmc_dev);
>>>   	err = *mmcp ? 0 : -ENODEV;
>>> -#endif
>> I am not against the change but trying to understand what is the problem you
>> faced. mmc_initialize() would have initialized the devices based on the seq
>> number that is provided in DT. So in your case shouldn't uclass_get_device
>> give the right device or something else triggered this patch?
>>
>> Also we are facing a different problem with mmc_initialize(). Very early in
>> boot there is no access to any peripheral other than boot peripheral(Need
>> system co processor to enable peripherals). There are 2 MMC controllers in
>> our devices.
>> So, SD boot is failing while loading system firmware as mmc_initialize is trying
>> to probe both the controllers. In SPL, shouldn't we just probe the needed
>> controller instead of calling mmc_initialize?
>>
>> Thanks and regards,
>> Lokesh