Re: [RFC 1/2] sdhci: Add device tree property sd-broken-highspeed

From: Adrian Hunter
Date: Thu Oct 06 2016 - 02:18:34 EST

On 06/10/16 04:34, Shawn Lin wrote:
> On 2016/10/6 5:22, Julia Cartwright wrote:
>> On Wed, Oct 05, 2016 at 03:03:44PM -0500, Rob Herring wrote:
>>> On Wed, Oct 5, 2016 at 1:33 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>> On 23 September 2016 at 22:01, Zach Brown <zach.brown@xxxxxx> wrote:
>>>>> Certain board configurations can make highspeed malfunction due to
>>>>> timing issues. In these cases a way is needed to force the controller
>>>>> and card into standard speed even if they otherwise appear to be capable
>>>>> of highspeed.
>>>>> The sd-broken-highspeed property will let the sdhci driver know that
>>>>> highspeed will not work.
>>>>> Signed-off-by: Zach Brown <zach.brown@xxxxxx>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mmc/mmc.txt | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt
>>>>> b/Documentation/devicetree/bindings/mmc/mmc.txt
>>>>> index 8a37782..59332ea 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>>>>> @@ -52,6 +52,8 @@ Optional properties:
>>>>> - no-sdio: controller is limited to send sdio cmd during initialization
>>>>> - no-sd: controller is limited to send sd cmd during initialization
>>>>> - no-mmc: controller is limited to send mmc cmd during initialization
>>>>> +- sd-broken-highspeed: Highspeed is broken, even if the controller and
>>>>> card
>>>>> + themselves claim they support highspeed.
>>>> Regarding a broken card, that is managed via the card quirks and not in DT.
>>>> If this is about a controller limitation, we already have the option
>>>> to describe what it supports, so we don't need an option to tell what
>>>> it *not* supports.
>>>> For example "cap-sd-highspeed" tells whether the controller supports
>>>> SD high-speed, please use that instead.
>>> If a controller has a capability register and it lies (perhaps the
>>> board has limitations that the SoC does not), then you may need to
>>> disable a feature.
>> That's precisely the case here. This is a board-level problem, not a
>> card or controller problem. As Zach mentioned in the cover letter, the
>> trace length between controller and card on some of our boards is too
>> long to meet high-speed timings, even though both card and controller
>> advertise it.
> IIRC, I saw the same problem while using sdhci to bring up a
> industrial board for vehicle. The trace length is so long that
> I have to limit the max-frequency to make it works properly when
> running at hishspeed.
> So could you try to limit the max-frequency in your DT to see
> if it could work for you? I guess it should work as once reducing
> the frequency, the timing per cycle will be large enough to meet
> the spec. At least that helped me solve my problem.
> For further consideration, I deployed a mechanism called "tuning for
> non UHS or non-hs200/400" for my donwstream tree at that time. The basic
> concept is to ask devices to send ext_csd(or send status for SD case)
> *repeatedly*. Some hosts, i.e. sdhci-of-arasan or dw_mmc-rockchip, can
> manually set rx delay via some clock unit registers to capture the
> working sample window and select the middle point of the longest good
> phase region. The same for retune. But it is a little complicated, and
> could only be applicable to the hosts who could adjust the rx delay
> manually when claiming the caps of MMC_CAPS_CAN_TUNING_FOR_HS, whatever
> the name is...
> I don't know if it is worth to add this, and I don't know if it's
> a legit way. Anyway, I just share my thought(experience) for you and
> linux-mmc to think more about how to deal with the case you meet rather
> than sacrificing the performence by removing highspeed or reducing
> frequency..

As Shawn points out, using max-frequency is a possibility. Did you consider

There are 2 high speed caps:

Yet patch in 2, the effect of sd-broken-highspeed is to suppress highspeed
for both SD and MMC. Should it then be called just broken-highspeed?