| Message ID | 200902031505.58697.jpihet@mvista.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
On Tue, 3 Feb 2009 15:05:58 +0100 Jean Pihet <jpihet@mvista.com> wrote: > + while (OMAP_HSMMC_READ(host->base, > + SYSCTL) & SRD) > + ; Is a __raw_readl() sufficient to prevent the cpu from burning up here, or should we add cpu_relax()? 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.. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 5 Feb 2009, Andrew Morton wrote: > On Tue, 3 Feb 2009 15:05:58 +0100 > Jean Pihet <jpihet@mvista.com> wrote: > > > + while (OMAP_HSMMC_READ(host->base, > > + SYSCTL) & SRD) > > + ; > > Is a __raw_readl() sufficient to prevent the cpu from burning up here, > or should we add cpu_relax()? The __raw_readl() should be sufficient. The MMC controller is located on the L4 CORE interconnect, so the round trip latency for the read from MMC is at least 90 ns, while the CPU cycle time is only about 1 to 2 ns. > 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.. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 05 February 2009 21:32:03 Paul Walmsley wrote: > On Thu, 5 Feb 2009, Andrew Morton wrote: > > On Tue, 3 Feb 2009 15:05:58 +0100 > > > > Jean Pihet <jpihet@mvista.com> wrote: > > > + while (OMAP_HSMMC_READ(host->base, > > > + SYSCTL) & SRD) > > > + ; > > > > Is a __raw_readl() sufficient to prevent the cpu from burning up here, > > or should we add cpu_relax()? > > The __raw_readl() should be sufficient. The MMC controller is located on > the L4 CORE interconnect, so the round trip latency for the read from MMC > is at least 90 ns, while the CPU cycle time is only about 1 to 2 ns. Ok. > > > 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. > > - Paul Regards, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 6 Feb 2009 14:22:32 +0100 Jean Pihet <jpihet@mvista.com> wrote: > On Thursday 05 February 2009 21:32:03 Paul Walmsley wrote: > > 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) :) Related, who is the maintainer of this driver? Tony? I'd like to have someone who checks patches before I queue them up. Rgds
On Thursday 05 February 2009, Paul Walmsley wrote: > > > > +Â Â Â while (OMAP_HSMMC_READ(host->base, > > > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â SYSCTL) & SRD) > > > + Â Â Â Â Â Â Â Â Â Â ; > > > > Is a __raw_readl() sufficient to prevent the cpu from burning up here, > > or should we add cpu_relax()? > > The __raw_readl() should be sufficient. Â The MMC controller is located on > the L4 CORE interconnect, so the round trip latency for the read from MMC > is at least 90 ns, while the CPU cycle time is only about 1 to 2 ns. It's still good policy to have a cpu_relax() in such loops. Not that it'll do much on most ARMs, but empty statements are in general worth avoiding. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pierre Ossman wrote: > Related, who is the maintainer of this driver? Tony? I'd like > to have someone who checks patches before I queue them up. I can be. Tony asked me a month ago to set up git tree for OMAP HSMMC and put the workaround for the HSMMC multi-block DMA read corruption there, but hate to admit things have been hanging on todo list. There are some other fixes waiting. Omap_mmc_set_ios() doesn't handle power off case correctly and resume from suspend can hang if the SDBP bit fails to set. Cheers Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Jarkko Lavinen <jarkko.lavinen@nokia.com> [090209 10:06]: > Pierre Ossman wrote: > > Related, who is the maintainer of this driver? Tony? I'd like > > to have someone who checks patches before I queue them up. > > I can be. Tony asked me a month ago to set up git tree for OMAP > HSMMC and put the workaround for the HSMMC multi-block DMA read > corruption there, but hate to admit things have been hanging on > todo list. Yeah it would be great if Jarkkko can queue up all the omap MMC patches from now on. > There are some other fixes waiting. Omap_mmc_set_ios() doesn't > handle power off case correctly and resume from suspend can hang > if the SDBP bit fails to set. And then all your PM patches for the next merge window.. Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 6 Feb 2009, Jean Pihet wrote: > 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. Just as an aside, from the standpoint of minimizing CPU usage, there seems little point in using udelay() here, since it is implemented as a CPU busy-wait. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From d143f6b2e705aa4e9d2b032097fd1c82f8163262 Mon Sep 17 00:00:00 2001 From: Jean Pihet <jpihet@mvista.com> Date: Thu, 8 Jan 2009 12:35:21 +0100 Subject: [PATCH] OMAP: MMC: recover from transfer failures Timeouts during a command that has a data phase can result in the next command issued after the command that failed not being processed, i.e. no interrupt ever occurs to indicate the command has completed. This failure can result in a deadlock. This patch resets the data state machine to clear the error in case of a command timeout. Tested on OMAP3430 chip and intensive MMC/SD device removal while transferring data. Signed-off-by: Andy Lowe <alowe@mvista.com> Signed-off-by: Jean Pihet <jpihet@mvista.com> Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com> --- drivers/mmc/host/omap_hsmmc.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index d5c1e9d..97150c0 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -464,8 +464,15 @@ static irqreturn_t mmc_omap_irq(int irq, void *dev_id) } end_cmd = 1; } - if (host->data) + 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) + ; + } } if ((status & DATA_TIMEOUT) || (status & DATA_CRC)) { -- 1.5.4.4.21.gc4a6c