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

[Hein Tibosch <hein_tibosch@yahoo.es>]

Hi Jianpeng Ma,

On 8/1/2013 10:18 AM, majianpeng wrote:
> 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.It used loops_per_jiffy as the timer.
> The code is:
>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
>>                && (i++ < limit))
>>                        cpu_relax();
> But the loops_per_jiffy is:
>>  while(i++ < limit)
>> 	cpu_relax();
> It add some codes so the time became long.
> 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>
> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 1865321..96daca1 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
>  	unsigned long limit = (loops_per_jiffy *
>  				msecs_to_jiffies(MMC_TIMEOUT_MS));
>  
> +	/*Divided time into us for unit 1,we can use udelay(1)*/
> +	i = limit / (MMC_TIMEOUT_MS * 1000);

'limit' is a number of loops, which you now divide by 20,000?

To get uS, you could just change:

-	unsigned long limit = (loops_per_jiffy *
-				msecs_to_jiffies(MMC_TIMEOUT_MS));
+	unsigned long limit = 1000 * MMC_TIMEOUT_MS;

and make this amount of loops using udelay().

>  	OMAP_HSMMC_WRITE(host->base, SYSCTL,
>  			 OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
>  
> @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
>  	 * Monitor a 0->1 transition first
>  	 */
>  	if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) {
> -		while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
> -					&& (i++ < limit))
> -			cpu_relax();

I still don't see why any of these loops could last 3.6 seconds?
Yes the __raw_readl() will add some time, but so much?
I'd like to see which value you get for 'limit' on your machine
Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated
on time?

> +		while (i--) {
> +			if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
> +				break;
> +			udelay(1);

In earlier threads, the use of udelay was disliked because it's a waste
of cpu cycles. The desired bit in SYSCTL will change, while udelay()
is still making many useless loops.

> +		}
>  	}
> -	i = 0;
>  
> -	while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) &&
> -		(i++ < limit))
> -		cpu_relax();
> +	i = limit / (MMC_TIMEOUT_MS * 1000);
> +	while (i--) {
> +		if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
> +			break;
> +		udealy(1);
> +	}
>  
>  	if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit)
>  		dev_err(mmc_dev(host->mmc),
Hein