On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
> ---
> 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.
>
> *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
> polarity properties, we have to fix the meaning of the "normal" and "inverted"
> --
> 2.7.4
>
Kind regards
Uffe
On Wed, Oct 5, 2016 at 1:33 PM, Ulf Hansson <[email protected]> wrote:
> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
>> ---
>> 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.
Rob
On Wed, Oct 05, 2016 at 03:03:44PM -0500, Rob Herring wrote:
> On Wed, Oct 5, 2016 at 1:33 PM, Ulf Hansson <[email protected]> wrote:
> > On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
> >> ---
> >> 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.
Thanks,
Julia
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 <[email protected]> wrote:
>>> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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..
>
> Thanks,
> Julia
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Best Regards
Shawn Lin
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 <[email protected]> wrote:
>>>> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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
that?
There are 2 high speed caps:
cap-sd-highspeed
cap-mmc-highspeed
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?
On 5 October 2016 at 22:03, Rob Herring <[email protected]> wrote:
> On Wed, Oct 5, 2016 at 1:33 PM, Ulf Hansson <[email protected]> wrote:
>> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
I understand, although the SDHCI capabilities register is broken for
most SDHCI variants. In principle, we would more or less have to add a
*-broken binding for each bit in that register. I don't like that!
Maybe a better option is to add a "sdhci-cap-broken" or perhaps
"sdhci-cap-speed-modes-broken", which tells the driver to not rely on
the capabilities register and instead find out what *is* supported by
looking at the other mmc generic DT bindings.
What do you think of that?
Kind regards
Uffe
On Thu, Oct 06, 2016 at 09:13:24AM +0300, Adrian Hunter wrote:
> 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 <[email protected]> wrote:
> >>>> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
> >>>>> ---
> >>>>> 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
> that?
>
I hadn't. So I looked into it and, as I stated the issue using max-frequency
would work, but I forgot to add that, because we pass the signal through an
fpga it has to be standard speed. If the card and controller are not in
standard speed then it won't pass hold time. Specifically we need the data to
change on the falling edge of the clock.
I'll add this justification to the commit message.
> There are 2 high speed caps:
> cap-sd-highspeed
> cap-mmc-highspeed
>
> 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?
>
I agree, broken-highspeed makes more sense. Will change in the next version.
On Thu, Oct 06, 2016 at 10:03:56AM +0200, Ulf Hansson wrote:
> On 5 October 2016 at 22:03, Rob Herring <[email protected]> wrote:
> > On Wed, Oct 5, 2016 at 1:33 PM, Ulf Hansson <[email protected]> wrote:
> >> On 23 September 2016 at 22:01, Zach Brown <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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.
>
> I understand, although the SDHCI capabilities register is broken for
> most SDHCI variants. In principle, we would more or less have to add a
> *-broken binding for each bit in that register. I don't like that!
>
> Maybe a better option is to add a "sdhci-cap-broken" or perhaps
> "sdhci-cap-speed-modes-broken", which tells the driver to not rely on
> the capabilities register and instead find out what *is* supported by
> looking at the other mmc generic DT bindings.
>
> What do you think of that?
>
> Kind regards
> Uffe
"sdhci-cap-broken" seems too aggressive. There might only be one capability
that the register incorrectly advertises.
"sdhci-cap-speed-modes-broken" makes more sense and I will re-create this patch
set using that idea.