Re: [PATCH 02/13] mmc: host: Add facility to support re-tuning

[Adrian Hunter <adrian.hunter@intel.com>]

On 14/01/15 12:13, Ulf Hansson wrote:
> On 14 January 2015 at 10:57, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 14/01/15 11:47, Ulf Hansson wrote:
>>> On 13 January 2015 at 17:02, Arend van Spriel <arend@broadcom.com> wrote:
>>>> On 01/13/15 16:41, Ulf Hansson wrote:
>>>>>
>>>>> On 13 January 2015 at 16:11, Arend van Spriel<arend@broadcom.com>  wrote:
>>>>>>
>>>>>> On 01/13/15 15:56, Ulf Hansson wrote:
>>>>>>>
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>>> Thank you for looking at the patches.
>>>>>>>>>>
>>>>>>>>>> I am not sure I know what you mean. sdhci already has a re-tuning
>>>>>>>>>> timer, so
>>>>>>>>>> this is just moving it into core, where it won't be used by other
>>>>>>>>>> drivers
>>>>>>>>>> unless they enable it.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I am kind of questioning the re-tuning timer in sdhci. What is it good
>>>>>>>>> for?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It is part of the SD Host Controller Standard Specification. The timer
>>>>>>>> ensures that re-tuning is done before temperature changes could affect
>>>>>>>> the
>>>>>>>> "sampling point". It is needed for re-tuning mode 1 for UHS-I modes
>>>>>>>> like
>>>>>>>> SDR104.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Does the spec say what value the timer should have?
>>>>>>
>>>>>>
>>>>>>
>>>>>> It is read from the Capabilities register in the SD host controller, ie.
>>>>>> in
>>>>>> field "Timer Count for Re-Tuning" (see below).
>>>>>>
>>>>>> Regards,
>>>>>> Arend
>>>>>>
>>>>>> Timer Count for Re-Tuning
>>>>>> This field indicates an initial value of the Re-Tuning Timer for
>>>>>> Re-Tuning
>>>>>> Mode 1 to 3. Setting to 0 disables Re-Tuning Timer.
>>>>>> 0h      Re-Tuning Timer disabled
>>>>>> 1h      1 seconds
>>>>>> 2h      2 seconds
>>>>>> 3h      4 seconds
>>>>>> 4h      8 seconds
>>>>>> .....   ......................
>>>>>> n       2(n-1) seconds
>>>>>> .....   ......................
>>>>>> Bh      1024 seconds
>>>>>> Eh - Ch Reserved
>>>>>> Fh      Get information from other source
>>>>>
>>>>>
>>>>> Thanks for sharing this information, but unfortunate I don't
>>>>> understand much from it.
>>>>>
>>>>> Is the host driver intended to read/poll this register to find a good
>>>>> value?
>>>>
>>>>
>>>> You can download the spec (and others) here [1]. sdhci currently implements
>>>> retuning mode 1, which is decribed in the spec:
>>>>
>>>> Re-Tuning Timer Control Example for Re-Tuning Mode 1
>>>> The initial value of re-tuning timer is provided by Timer Count for
>>>> Re-Tuning field in this register. The timer starts counting by loading the
>>>> initial value. When the timer expires, the Host Driver marks an expiration
>>>> flag. On receiving a command request, the Host driver checks the expiration
>>>> flag. If the expiration flag is set, then the Host Driver should perform the
>>>> re-tuning procedure before issuing a command. If the expiration flag is not
>>>> set, then the Host Driver issues a command without performing the re-tuning
>>>> procedure. Every time the re-tuning procedure is performed, the timer loads
>>>> the new initial value and the expiration flag is cleared.
>>>>
>>>> So the host controller could indeed update this register for subsequent
>>>> retuning.
>>>
>>> Arend, thanks for the link and information. So, I decided to go for a
>>> look in there.
>>>
>>> >From the same section you quoted above:
>>> ------
>>> (1) Re-Tuning Mode 1
>>> The host controller does not have any internal logic to detect when
>>> the re-tuning needs to be performed. In this case, the Host Driver
>>> should maintain all re-tuning timings by using a Re-Tuning Timer. To
>>> enable inserting the re-tuning procedure during data transfers, the
>>> data length per read/write command shall be limited up to 4MB.
>>> ------
>>>
>>> That means, we can't get _any_ help from the controller HW (in mode 1)
>>> to find a good value for the timer.
>>
>> In fact the timer value *is* defined in the Capabilities Register (Offset
>> 040h) bits 43-40 Timer Count for Re-Tuning
>>
>> It has been supported since 2011, see:
>>
>>         commit cf2b5eea1ea0ff9b3184bc6771bcb93a9fdcd1d9
>>         "mmc: sdhci: add support for retuning mode 1"
>>
> 
> The value from the register is also just randomly selected, only
> difference is that it's the HW that has randomly set it.

Presumably the value is chosen based on the maximum rate of temperature
change and the corresponding effect that has on the signal.

> 
> Even if the above commit was merged, I don't think it was the correct
> way of dealing with re-tuning.
> 
> First of all, re-tuning this is a mmc protocol specific thing should
> be managed from the mmc core, like the approach you have taken in your
> $subject patchset. Second I question whether the timer is useful at
> all.

The SD Host Controller Specification does not document another way to do
mode 1 re-tuning. The timer is it. Otherwise re-tuning is never done.

In the patches I sent, the driver must call mmc_retune_needed() to set
host->need_retune = 1 otherwise mmc_retune() does nothing.

I would like to extend the model to include transparently re-tuning and
re-trying when there is a CRC error, but that is a separate issue, not
documented in the spec but recommended by others.