On 11/08/2013 07:55 PM, Arend van Spriel wrote:
> On 11/08/2013 12:49 PM, Aaron Lu wrote:
>> 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.
>
> True. I can go for that approach.
>
>> Anyway, a host in retuning mode 1 does not have tuning_count set seems a
>> little odd to me...
>
> Looking at sdhci.h it actually seems sdhci code is missing support for
> the other modes:
>
> unsigned int tuning_count; /* Timer count for re-tuning */
> unsigned int tuning_mode; /* Re-tuning mode supported by host */
> #define SDHCI_TUNING_MODE_1 0
> struct timer_list tuning_timer; /* Timer for tuning */
>
> At least the other modes are not defined here.
Right, only retuning mode 1 is implemented currently.
Thanks,
Aaron
>
> Regards,
> Arend
>
>> 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@xxxxxxxxxxxx>
>>>>> ---
>>>>> 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);
>>>>> }
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
