Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support

[Masahiro Yamada]

Hi Ulf,

2016-12-13 16:52 GMT+09:00 Ulf Hansson <ulf.hansson@xxxxxxxxxx>:
> [...]
>
>>>> +
>>>> +Optional properties:
>>>> +For eMMC configuration, supported speed modes are not indicated by the SDHCI
>>>> +Capabilities Register.  Instead, the following properties should be specified
>>>> +if supported.  See mmc.txt for details.
>>>> +- mmc-ddr-1_8v
>>>> +- mmc-ddr-1_2v
>>>> +- mmc-hs200-1_8v
>>>> +- mmc-hs200-1_2v
>>>> +- mmc-hs400-1_8v
>>>> +- mmc-hs400-1_2v
>>>
>>> There's now a property to override SDHCI capabilities register. Maybe
>>> you should use that instead? I'll defer to Ulf.
>>>
>>
>> I did not know this new property.
>>
>> So, now we have two ways to specify MMC speed mode capabilities
>> by only touching DT.
>
> Let me clarify a bit.
>
> The point with the new bindings is to be able to override *broken*
> SDHCI caps bits. So, not only related to the speed modes.

Right.

So my approach ([1] Add MMC mode flags directly)
basically sounds OK.



>>
>> [1] Add MMC mode flags directly, like I did.
>> [2] Use "sdhci-caps-mask" and "sdhci-caps"
>>
>>
>> The problem for [2] is that eMMC capabilities
>> do not perfectly correspond to the SDHCI capabilities register.
>>
>>
>>>> +- mmc-hs400-1_8v
>>>> +- mmc-hs400-1_2v
>>
>> If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
>> we can use the bit63 of caps for specifying HS400.
>>
>> But, this is not defined in the SDHCI standard.
>> #define  SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */
>>
>>
>>
>>>> +- mmc-ddr-1_8v
>>
>> For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR
>> from MMC_CAP_UHS_DDR50  (bit34 of caps)
>>
>> This is not supported in the current code, but
>> if this is a good idea, I can send a patch.
>>
>>
>>>> +- mmc-ddr-1_2v
>>
>> This does not have the corresponding bit, but
>> 1.2V is not commonly used, so this is not a fatal problem.
>>
>>
>>
>> What I can do at most now, is to delete the
>> Optional properties section entirely
>> so users can choose [1] or [2] as they like.
>>
>
> I think a better approach is to use the new sdhci-caps* bindings to
> mask those caps that can't be trusted. And then use the generic mmc
> bindings for speed modes instead.
>
> That should be a safer approach, right!?

Right.

But, if I know the caps register bits 63-32 are all zero,
I need not explicitly specify sdhci-caps-mask, right?



-- 
Best Regards
Masahiro Yamada