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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
Tested-by: Yuzheng Ma <mayuzheng@xxxxxxxxxxx>
Reviewed-by: Hein Tibosch <hein_tibosch@xxxxxxxx>
Signed-off-by: Jianpeng Ma <majianpeng@xxxxxxxxx>
---
  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 ?


This reset function is also called while mounting a card and then 'SRC'
will be high long enough (more than 100 loops). It is after withdrawing
the card that the reset is performed extremely fast.

@@ -984,17 +983,15 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host,
  	 * OMAP4 ES2 and greater has an updated reset logic.
  	 * 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 = 0;
-
-	while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) &&
-		(i++ < limit))
-		cpu_relax();
-
+	if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET)
+		while (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)
+			&& limit--)
+			udelay(1);

In this way, the loop will only last about 'MMC_TIMEOUT_MS' ms
and my WDT doesn't get triggered :-)

+	limit = MMC_TIMEOUT_MS * 1000;
+	while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && limit--)
+		udelay(1);
+
  	if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit)
  		dev_err(mmc_dev(host->mmc),
  			"Timeout waiting on controller reset in %s\n",

Anybody an opinion on this?

Thanks, Hein


--
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




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux