From: Jean Pihet <jpihet@mvista.com>
To: Adrian Hunter <ext-adrian.hunter@nokia.com>
Cc: Pierre Ossman <drzeus-mmc@drzeus.cx>,
"tony@atomide.com" <tony@atomide.com>,
Paul Walmsley <paul@pwsan.com>,
Andrew Morton <akpm@linux-foundation.org>,
"linux-arm-kernel@lists.arm.linux.org.uk"
<linux-arm-kernel@lists.arm.linux.org.uk>,
"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
"Lavinen Jarkko (Nokia-D/Helsinki)" <jarkko.lavinen@nokia.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] OMAP: MMC: replace infinite loops with timeouts (was Re: [PATCH] OMAP: MMC: recover from transfer failures - Resend)
Date: Wed, 11 Feb 2009 10:41:16 +0100 [thread overview]
Message-ID: <200902111041.17095.jpihet@mvista.com> (raw)
In-Reply-To: <49905296.2060308@nokia.com>
[-- Attachment #1: Type: text/plain, Size: 6479 bytes --]
Hi,
Here is the new version of the patch, which now uses cpu_relax in an inline
function. The new function can be used from an irq handler as well.
Regards,
Jean
From 9d1993dcf4d217837437a1cd9f6e9c1b81533549 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: Change while(); loops with finite version
Replace the infinite 'while() ;' loops
with a finite loop version.
Signed-off-by: Jean Pihet <jpihet@mvista.com>
---
drivers/mmc/host/omap_hsmmc.c | 54 ++++++++++++++++++++++------------------
1 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 1ac6918..aabf28d 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -375,6 +375,32 @@ static void mmc_omap_report_irq(struct mmc_omap_host
*host, u32 status)
}
#endif /* CONFIG_MMC_DEBUG */
+/*
+ * MMC controller internal state machines reset
+ *
+ * Used to reset command or data internal state machines, using respectively
+ * SRC or SRD bit of SYSCTL register
+ * Can be called from interrupt context
+ */
+static inline void mmc_omap_reset_controller_fsm(struct mmc_omap_host *host,
+ unsigned long bit)
+{
+ unsigned long i = 0;
+ unsigned long limit = (loops_per_jiffy *
+ msecs_to_jiffies(MMC_TIMEOUT_MS));
+
+ OMAP_HSMMC_WRITE(host->base, SYSCTL,
+ OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
+
+ while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) &&
+ (i++ < limit))
+ cpu_relax();
+
+ if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on controller reset in %s\n",
+ __func__);
+}
/*
* MMC controller IRQ handler
@@ -403,13 +429,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
(status & CMD_CRC)) {
if (host->cmd) {
if (status & CMD_TIMEOUT) {
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) |
SRC);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRC)
- ;
-
+ mmc_omap_reset_controller_fsm(host,
SRC);
host->cmd->error = -ETIMEDOUT;
} else {
host->cmd->error = -EILSEQ;
@@ -418,12 +438,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
}
if (host->data) {
mmc_dma_cleanup(host);
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ mmc_omap_reset_controller_fsm(host, SRD);
}
}
if ((status & DATA_TIMEOUT) ||
@@ -433,12 +448,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
mmc_dma_cleanup(host);
else
host->data->error = -EILSEQ;
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ mmc_omap_reset_controller_fsm(host, SRD);
end_trans = 1;
}
}
@@ -529,11 +539,7 @@ static void mmc_omap_detect(struct work_struct *work)
if (host->carddetect) {
mmc_detect_change(host->mmc, (HZ * 200) / 1000);
} else {
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
- ;
-
+ mmc_omap_reset_controller_fsm(host, SRD);
mmc_detect_change(host->mmc, (HZ * 50) / 1000);
}
}
--
1.6.0.1.42.gf8c9c
On Monday 09 February 2009 16:58:14 Adrian Hunter wrote:
> Jean Pihet wrote:
> > 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?
>
> What about making use of a function like this:
>
> static void reset_host_controller(struct mmc_omap_host *host, unsigned bit)
> {
> unsigned long i, limit = loops_per_jiffy;
>
> OMAP_HSMMC_WRITE(host->base, SYSCTL,
> OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
> for (i = 0; i < limit; i++) {
> if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit))
> return;
> cpu_relax();
> }
> dev_err(mmc_dev(host->mmc), "%s: timeout waiting on software reset\n",
> mmc_hostname(host->mmc));
> }
>
> And then just put:
>
> reset_host_controller(host, SRD);
>
> and
>
> reset_host_controller(host, SRC);
>
> in the right places.
[-- Attachment #2: hsmmc_finite_timeout.patch --]
[-- Type: text/x-diff, Size: 3379 bytes --]
From 9d1993dcf4d217837437a1cd9f6e9c1b81533549 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: Change while(); loops with finite version
Replace the infinite 'while() ;' loops
with a finite loop version.
Signed-off-by: Jean Pihet <jpihet@mvista.com>
---
drivers/mmc/host/omap_hsmmc.c | 54 ++++++++++++++++++++++------------------
1 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 1ac6918..aabf28d 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -375,6 +375,32 @@ static void mmc_omap_report_irq(struct mmc_omap_host *host, u32 status)
}
#endif /* CONFIG_MMC_DEBUG */
+/*
+ * MMC controller internal state machines reset
+ *
+ * Used to reset command or data internal state machines, using respectively
+ * SRC or SRD bit of SYSCTL register
+ * Can be called from interrupt context
+ */
+static inline void mmc_omap_reset_controller_fsm(struct mmc_omap_host *host,
+ unsigned long bit)
+{
+ unsigned long i = 0;
+ unsigned long limit = (loops_per_jiffy *
+ msecs_to_jiffies(MMC_TIMEOUT_MS));
+
+ OMAP_HSMMC_WRITE(host->base, SYSCTL,
+ OMAP_HSMMC_READ(host->base, SYSCTL) | bit);
+
+ while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) &&
+ (i++ < limit))
+ cpu_relax();
+
+ if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit)
+ dev_err(mmc_dev(host->mmc),
+ "Timeout waiting on controller reset in %s\n",
+ __func__);
+}
/*
* MMC controller IRQ handler
@@ -403,13 +429,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
(status & CMD_CRC)) {
if (host->cmd) {
if (status & CMD_TIMEOUT) {
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) | SRC);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRC)
- ;
-
+ mmc_omap_reset_controller_fsm(host, SRC);
host->cmd->error = -ETIMEDOUT;
} else {
host->cmd->error = -EILSEQ;
@@ -418,12 +438,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
}
if (host->data) {
mmc_dma_cleanup(host);
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ mmc_omap_reset_controller_fsm(host, SRD);
}
}
if ((status & DATA_TIMEOUT) ||
@@ -433,12 +448,7 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
mmc_dma_cleanup(host);
else
host->data->error = -EILSEQ;
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base,
- SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base,
- SYSCTL) & SRD)
- ;
+ mmc_omap_reset_controller_fsm(host, SRD);
end_trans = 1;
}
}
@@ -529,11 +539,7 @@ static void mmc_omap_detect(struct work_struct *work)
if (host->carddetect) {
mmc_detect_change(host->mmc, (HZ * 200) / 1000);
} else {
- OMAP_HSMMC_WRITE(host->base, SYSCTL,
- OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
- while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
- ;
-
+ mmc_omap_reset_controller_fsm(host, SRD);
mmc_detect_change(host->mmc, (HZ * 50) / 1000);
}
}
--
1.6.0.1.42.gf8c9c
next prev parent reply other threads:[~2009-02-11 9:41 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-07 21:45 [PATCH 0/5] Omap MMC init updates and new controller for 2.6.29 merge window Tony Lindgren
2008-12-07 21:46 ` [PATCH 1/5] omap mmc: Remove broken MMC init code Tony Lindgren
2008-12-07 21:47 ` [PATCH 2/5] omap mmc: Add better MMC low-level init Tony Lindgren
2009-01-10 22:49 ` Ladislav Michl
2009-01-10 23:00 ` Ladislav Michl
2009-01-13 13:43 ` Tony Lindgren
2009-03-23 17:47 ` Ladislav Michl
2009-03-24 20:04 ` [APPLIED] " Tony Lindgren
2009-01-10 23:03 ` Ladislav Michl
2008-12-07 21:49 ` [PATCH 3/5] omap mmc: Add low-level initialization for hsmmc controller Tony Lindgren
2008-12-07 21:50 ` [PATCH 4/5] omap mmc: force MMC module reset on boot Tony Lindgren
2008-12-07 21:51 ` [PATCH 5/5] omap mmc: Add new omap hsmmc controller for 2430 and 34xx Tony Lindgren
2008-12-21 16:17 ` Pierre Ossman
2008-12-29 16:55 ` Tony Lindgren
2008-12-30 8:36 ` Tony Lindgren
2008-12-31 17:59 ` Pierre Ossman
2009-01-07 10:18 ` Tony Lindgren
2009-01-07 10:28 ` [PATCH] OMAP: MMC: recover from transfer failures (was: Re: [PATCH 5/5] omap mmc: Add new omap hsmmc controller for 2430 and 34xx) Jean Pihet
2009-01-07 15:40 ` Tony Lindgren
2009-01-08 9:02 ` [PATCH] OMAP: MMC: recover from transfer failures Adrian Hunter
2009-01-08 11:49 ` Jean Pihet
2009-01-08 12:17 ` Adrian Hunter
2009-02-02 8:46 ` Jean Pihet
2009-02-02 19:05 ` Tony Lindgren
2009-02-03 14:05 ` [PATCH] OMAP: MMC: recover from transfer failures - Resend Jean Pihet
2009-02-05 20:10 ` Andrew Morton
2009-02-05 20:32 ` Paul Walmsley
2009-02-06 13:22 ` Jean Pihet
2009-02-06 13:53 ` Pierre Ossman
2009-02-06 15:53 ` [PATCH] OMAP: MMC: replace infinite loops with timeouts (was Re: [PATCH] OMAP: MMC: recover from transfer failures - Resend) Jean Pihet
2009-02-09 15:58 ` Adrian Hunter
2009-02-11 9:41 ` Jean Pihet [this message]
2009-02-09 17:58 ` [PATCH] OMAP: MMC: recover from transfer failures - Resend Jarkko Lavinen
2009-02-09 18:46 ` Tony Lindgren
2009-02-10 0:09 ` Paul Walmsley
2009-02-08 20:27 ` David Brownell
2008-12-15 22:26 ` git pull request for omap mmc init changes (Re: [PATCH 0/5] Omap MMC init updates and new controller for 2.6.29 merge window) Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200902111041.17095.jpihet@mvista.com \
--to=jpihet@mvista.com \
--cc=akpm@linux-foundation.org \
--cc=drzeus-mmc@drzeus.cx \
--cc=ext-adrian.hunter@nokia.com \
--cc=jarkko.lavinen@nokia.com \
--cc=linux-arm-kernel@lists.arm.linux.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=paul@pwsan.com \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox