Re: [PATCH] sdhci: do not program timer when tuning_count is zero

[Aaron Lu <aaron.lu@intel.com>]

On 11/08/2013 05:10 PM, Arend van Spriel wrote:
> On 11/08/2013 07:50 AM, Aaron Lu wrote:
>> On 11/07/2013 06:59 PM, Arend van Spriel wrote:
>>> When the host->tuning_count is zero it means that the
>>
>> If the tuning_count is zero, then the retuning timer shouldn't be
>> started in the first place and not possible to run code there. Or is the
>> tuning_count dynamically changed?
> 
> Actually, the sdhci_execute_tuning() must run once to do the initial 
> tuning procedure. This is mandatory for SDR104. However, *re*tuning is 
> not and a zero tuning_count disables it.

So the host in question doesn't need do retuning while in SDR104 mode?
It seems your host's retuning mode is mode_1.

> 
> The function is executed initially. The 'if' statement above the patched 
> 'else' statement is actually responsible for programming the retuning 
> timer for the first time. However, it requires tuning_count to be 
> non-zero. The 'else' statement is actually for reloading the retuning 
> timer, which is not the case. Adding the non-zero check assures the 
> retuning timer is never started.

OK, I see.
The SDHCI_USING_RETUNING_TIMER flag is supposed to mean if the host is
currently using retuning timer to do retuning, it could also be used to
decide if retuning timer needs be re-programmed.

Anyway, a host in retuning mode 1 does not have tuning_count set seems a
little odd to me...

Thanks,
Aaron

> 
> I guess the fact that this needs explaining indicates that the commit 
> message should be updated. I will send a V2 for this.
> 
> Regards,
> Arend
> 
>> Thanks,
>> Aaron
>>
>>> retuning is disabled. Doing a mod_timer() with a zero
>>> tuning_count does something else.
>>>
>>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>>> ---
>>>   drivers/mmc/host/sdhci.c |    3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 7a7fb4f..9803e7a 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2007,7 +2007,8 @@ out:
>>>   	} else {
>>>   		host->flags &= ~SDHCI_NEEDS_RETUNING;
>>>   		/* Reload the new initial value for timer */
>>> -		if (host->tuning_mode == SDHCI_TUNING_MODE_1)
>>> +		if (host->tuning_count &&
>>> +		    host->tuning_mode == SDHCI_TUNING_MODE_1)
>>>   			mod_timer(&host->tuning_timer, jiffies +
>>>   				host->tuning_count * HZ);
>>>   	}
>>>
>>
>>
> 
>