[PATCH] OMAP: MMC: replace infinite loops with timeouts (was Re: [PATCH] OMAP: MMC: recover from transfer failures - Resend)

Jean Pihet
2009-02-06T15:53:48Z

[-- Attachment #1: Type: text/plain, Size: 7307 bytes --]

Hi,

> > > On Thu, 5 Feb 2009, Andrew Morton wrote:
> > > > An infinite loop which assumes the hardware is perfect is always a
> > > > worry. But I see the driver already does that, so we're no worse
> > > > off..
> >
> > Do you want a finite loop with udelay in it? I located 4 places were this
> > could be used. If so I can generate a new patch for that.
>
> Even if Andrew doesn't, I'd sure like it. (the finite bit at least) :)
Ok here is a patch that replaces the infinite loops with a timeout version.
This patch applies on top of the previous one I sent ('[PATCH] OMAP: MMC:
recover from transfer failures - Resend'). Is that OK?

> Related, who is the maintainer of this driver? Tony? I'd like to have
> someone who checks patches before I queue them up.
>
>
> Rgds

From 5ee867d09efe22a903ac7373a05e9e047bad6544 Mon Sep 17 00:00:00 2001
From: Jean Pihet <jpihet@mvista.com>
Date: Fri, 6 Feb 2009 16:42:51 +0100
Subject: [PATCH] OMAP: MMC: replace infinite loops with timeouts

Replace the 'while() ;' with a timeout

Signed-off-by: Jean Pihet <jpihet@mvista.com>
---
drivers/mmc/host/omap_hsmmc.c | 56
+++++++++++++++++++++++++++++++++--------
1 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 1ac6918..7ddc77e 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -102,6 +102,8 @@
#define OMAP_MMC_DATADIR_READ 1
#define OMAP_MMC_DATADIR_WRITE 2
#define MMC_TIMEOUT_MS 20
+#define MMC_TIMEOUT_WAIT 100 /* Active wait duration, in usec */
+#define MMC_TIMEOUT_WAIT_LOOPS ((MMC_TIMEOUT_MS * 1000) / MMC_TIMEOUT_WAIT)
#define OMAP_MMC_MASTER_CLOCK 96000000
#define DRIVER_NAME "mmci-omap-hs"

@@ -384,6 +386,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
struct mmc_omap_host *host = dev_id;
struct mmc_data *data;
int end_cmd = 0, end_trans = 0, status;
+ unsigned long timeout_counter;

if (host->cmd == NULL && host->data == NULL) {
OMAP_HSMMC_WRITE(host->base, STAT,
@@ -406,9 +409,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) |
SRC);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRC)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRC)
+ && (timeout_counter++ <
+
MMC_TIMEOUT_WAIT_LOOPS))
+
udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRC)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on
SRC\n");

host->cmd->error = -ETIMEDOUT;
} else {
@@ -421,9 +432,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ && (timeout_counter++ <
+
MMC_TIMEOUT_WAIT_LOOPS))
+ udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on SRD\n");
}
}
if ((status & DATA_TIMEOUT) ||
@@ -436,9 +455,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ && (timeout_counter++ <
+
MMC_TIMEOUT_WAIT_LOOPS))
+ udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on SRD\n");
end_trans = 1;
}
}
@@ -524,6 +551,7 @@ static void mmc_omap_detect(struct work_struct *work)
{
struct mmc_omap_host *host = container_of(work, struct mmc_omap_host,
mmc_carddetect_work);
+ unsigned long timeout;

sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
if (host->carddetect) {
@@ -531,8 +559,14 @@ static void mmc_omap_detect(struct work_struct *work)
} else {
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
- ;
+ /* Wait till the SRD bit is reset */
+ timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
+ while ((OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
+ && time_before(jiffies, timeout))
+ msleep(1);
+
+ if (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc), "Timeout waiting on
SRD\n");

mmc_detect_change(host->mmc, (HZ * 50) / 1000);
}
--
1.6.0.1.42.gf8c9c

[-- Attachment #2: OMAP-MMC-replace-infinite-loops-with-timeouts.patch --]
[-- Type: text/x-diff, Size: 4137 bytes --]

From 5ee867d09efe22a903ac7373a05e9e047bad6544 Mon Sep 17 00:00:00 2001
From: Jean Pihet <jpihet@mvista.com>
Date: Fri, 6 Feb 2009 16:42:51 +0100
Subject: [PATCH] OMAP: MMC: replace infinite loops with timeouts

Replace the 'while() ;' with a timeout

Signed-off-by: Jean Pihet <jpihet@mvista.com>
---
drivers/mmc/host/omap_hsmmc.c | 56 +++++++++++++++++++++++++++++++++--------
1 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 1ac6918..7ddc77e 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -102,6 +102,8 @@
#define OMAP_MMC_DATADIR_READ 1
#define OMAP_MMC_DATADIR_WRITE 2
#define MMC_TIMEOUT_MS 20
+#define MMC_TIMEOUT_WAIT 100 /* Active wait duration, in usec */
+#define MMC_TIMEOUT_WAIT_LOOPS ((MMC_TIMEOUT_MS * 1000) / MMC_TIMEOUT_WAIT)
#define OMAP_MMC_MASTER_CLOCK 96000000
#define DRIVER_NAME "mmci-omap-hs"

@@ -384,6 +386,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
struct mmc_omap_host *host = dev_id;
struct mmc_data *data;
int end_cmd = 0, end_trans = 0, status;
+ unsigned long timeout_counter;

if (host->cmd == NULL && host->data == NULL) {
OMAP_HSMMC_WRITE(host->base, STAT,
@@ -406,9 +409,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) | SRC);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRC)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRC)
+ && (timeout_counter++ <
+ MMC_TIMEOUT_WAIT_LOOPS))
+ udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRC)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on SRC\n");

host->cmd->error = -ETIMEDOUT;
} else {
@@ -421,9 +432,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ && (timeout_counter++ <
+ MMC_TIMEOUT_WAIT_LOOPS))
+ udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on SRD\n");
}
}
if ((status & DATA_TIMEOUT) ||
@@ -436,9 +455,17 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base,
SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ timeout_counter = 0;
+ while ((OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ && (timeout_counter++ <
+ MMC_TIMEOUT_WAIT_LOOPS))
+ udelay(MMC_TIMEOUT_WAIT);
+
+ if (OMAP_HSMMC_READ(host->base,
+ SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on SRD\n");
end_trans = 1;
}
}
@@ -524,6 +551,7 @@ static void mmc_omap_detect(struct work_struct *work)
{
struct mmc_omap_host *host = container_of(work, struct mmc_omap_host,
mmc_carddetect_work);
+ unsigned long timeout;

sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
if (host->carddetect) {
@@ -531,8 +559,14 @@ static void mmc_omap_detect(struct work_struct *work)
} else {
OMAP_HSMMC_WRITE(host->base, SYSCTL,
OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
- ;
+ /* Wait till the SRD bit is reset */
+ timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
+ while ((OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
+ && time_before(jiffies, timeout))
+ msleep(1);
+
+ if (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
+ dev_err(mmc_dev(host->mmc), "Timeout waiting on SRD\n");

mmc_detect_change(host->mmc, (HZ * 50) / 1000);
}
--
1.6.0.1.42.gf8c9c