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

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

On 14/01/15 14:59, Ulf Hansson wrote:
> [...]
> 
>>>
>>> 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.
>>
> 
> That perfect and in line from what I heard as recommendations from
> memory vendors as well.

How would that work for SDIO? How do you know it is OK to retry SDIO operations?

> 
> Now, can we stop arguing about the timer and try without it?
> 
> If we do see a need for a more frequent re-tuning to happen, due to
> that we get lots of CRC errors to recover from, then I think we should
> look into using runtime PM instead of the timer. And that's because I
> want to minimize the impact on performance.

The minimum timer value is 1 second. The maximum is 1024 seconds. The ASUS
T100TA had a timer value of 128 seconds. The timer is not a performance issue.

There is a performance question with runtime PM because that happens far
more frequently (typical auto-suspend delay is 50ms) and we re-tune after
that. In fact I generalized that a bit in patch 13.

	[PATCH 13/13] mmc: sdhci: Change to new way of doing re-tuning

	Make use of mmc core support for re-tuning instead
	of doing it all in the sdhci driver.

	This patch also changes to flag the need for re-tuning
	always after runtime suspend when tuning has been used
	at initialization. Previously it was only done if
	the re-tuning timer was in use.

One option to reduce the impact of the latency would be to increase the
auto-suspend delay.

I have cc'ed the author of "mmc: sdhci: add support for retuning mode 1"