Re: Re: [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context.

[majianpeng <majianpeng@gmail.com>]

>On Wednesday 21 August 2013 12:22 AM, Hein Tibosch wrote:
>> Hi,
>>
>> [ added some people from TI ]
>>
>> On 8/7/2013 6:05 PM, majianpeng wrote:
>>> V2:
>>> 	clean up code.
>>> V1:
>>> 	www.mail-archive.com/linux-omap@vger.../msg93239.html‎
>>>
>>>
>>> We found a problem when we removed a working sd card that the irqaction
>>> of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work.
>>> In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1
>>> transition.But avoiding endless waiting, it used loops_per_jiffy as the timer.
>>
>> Tried on a OMAP4460:
>> This can easily be replicated: just withdraw an SD-card and the kernel
>> will get blocked during more than 3 seconds.
>> Calling OMAP_HSMMC_READ() in this loop makes it last so long.
>>
>> The function waits for a 0=>1, followed by a 1=>0 transition.
>> The value of 1 always comes, but in most cases the code is just too
>> slow to detect it. The first loop will only stop when (i == limit)
>>
>>> The code is:
>>>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
>>>>                 && (i++ < limit))
>>>>                         cpu_relax();
>>> But generanly loops_per_jiffy as a timer,it should like:
>>>>   while(i++ < limit)
>>>> cpu_relax();
>>> I found for the long time case, the while-opeation stoped because 'i == limit'.
>>> Because added some code, so the duration became too longer than
>>> MMC_TIMEOU_US(20ms).
>>>
>>> The software can't monitor the transition of hardware for thi case.
>>>
>>> Becasue those codes in ISR context, it can't use timer_before/after.
>>> I divived the time into 1ms and used udelay(1) to instead.
>>> It will cause do additional udelay(1).But from my test,it looks good.
>>>
>>> Reported-by: Yuzheng Ma <mayuzheng@kedacom.com>
>>> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com>
>>> Reviewed-by: Hein Tibosch <hein_tibosch@yahoo.es>
>>> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
>>> ---
>>>   drivers/mmc/host/omap_hsmmc.c | 25 +++++++++++--------------
>>>   1 file changed, 11 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>>> index 1865321..bbda5ed 100644
>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>> @@ -973,9 +973,8 @@ static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host,
>>>   static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
>>>   						   unsigned long bit)
>>>   {
>>> -	unsigned long i = 0;
>>> -	unsigned long limit = (loops_per_jiffy *
>>> -				msecs_to_jiffies(MMC_TIMEOUT_MS));
>>> +	/*change to 1ms,so we can use udelay(1)*/
>>> +	unsigned long limit = MMC_TIMEOUT_MS * 1000;
>>>
>>>   	OMAP_HSMMC_WRITE(host->base, SYSCTL,
>>>   			 OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
>>
>> Checked here: the SRC-bit indeed becomes high.
>> After the test 'features & HSMMC_HAS_UPDATED_RESET', the bit has
>> become low again already.
>> Changing to order of statements worked for me, but for Jianpeng Ma
>> this didn't work (timings are 'on the edge').
>
>I think 1->0 transition is missed sometimes and waiting until timeout,
>Jianpeng Ma, which SoC are you testing ?
>
Hi, 
	my sos is DM8107.
Thanks!
Jianpeng Ma!