* [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
@ 2011-10-27 4:18 Shawn.Dong
2011-10-27 5:09 ` Barry Song
2011-10-27 5:15 ` Wolfram Sang
0 siblings, 2 replies; 6+ messages in thread
From: Shawn.Dong @ 2011-10-27 4:18 UTC (permalink / raw)
To: linux-mmc
sdhci_set_clock or sdhci_reset or sdhci_send_command may be used in
critical region which is protected by spin_lock_irqsave. Thus, these
functions will delay the responsing of the kernel interrupts.
So in this case, using a mdelay will cause unnecessary latency. Our
hardware, in most case will not cause 1ms to finish its job. Using
udelay instead can reduce it.
By default, sdhci_do_set_ios will need about 1ms to finish in my platform.
After using udelay instead, sdhci_do_set_ios only need a few microseconds.
Signed-off-by: Shawn.Dong <Shawn.dong.cx@gmail.com>
---
drivers/mmc/host/sdhci.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6d8eea3..9314cd1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -195,7 +195,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
host->clock = 0;
/* Wait max 100 ms */
- timeout = 100;
+ timeout = 10000;
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
@@ -206,7 +206,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
return;
}
timeout--;
- mdelay(1);
+ udelay(10);
}
if (host->ops->platform_reset_exit)
@@ -959,7 +959,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
WARN_ON(host->cmd);
/* Wait max 10 ms */
- timeout = 10;
+ timeout = 1000;
mask = SDHCI_CMD_INHIBIT;
if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -980,7 +980,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
return;
}
timeout--;
- mdelay(1);
+ udelay(10);
}
mod_timer(&host->timer, jiffies + 10 * HZ);
@@ -1140,7 +1140,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
/* Wait max 20 ms */
- timeout = 20;
+ timeout = 2000;
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (timeout == 0) {
@@ -1150,7 +1150,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
return;
}
timeout--;
- mdelay(1);
+ udelay(10);
}
clk |= SDHCI_CLOCK_CARD_EN;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
2011-10-27 4:18 [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases Shawn.Dong
@ 2011-10-27 5:09 ` Barry Song
2011-10-27 5:15 ` Wolfram Sang
1 sibling, 0 replies; 6+ messages in thread
From: Barry Song @ 2011-10-27 5:09 UTC (permalink / raw)
To: Shawn.Dong; +Cc: linux-mmc
2011/10/27 Shawn.Dong <shawn.dong.cx@gmail.com>:
> sdhci_set_clock or sdhci_reset or sdhci_send_command may be used in
> critical region which is protected by spin_lock_irqsave. Thus, these
> functions will delay the responsing of the kernel interrupts.
>
> So in this case, using a mdelay will cause unnecessary latency. Our
> hardware, in most case will not cause 1ms to finish its job. Using
> udelay instead can reduce it.
>
> By default, sdhci_do_set_ios will need about 1ms to finish in my platform.
> After using udelay instead, sdhci_do_set_ios only need a few microseconds.
>
> Signed-off-by: Shawn.Dong <Shawn.dong.cx@gmail.com>
ACK, see my patch as well:
http://www.spinics.net/lists/linux-mmc/msg10257.html
> ---
> drivers/mmc/host/sdhci.c | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 6d8eea3..9314cd1 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -195,7 +195,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
> host->clock = 0;
>
> /* Wait max 100 ms */
> - timeout = 100;
> + timeout = 10000;
>
> /* hw clears the bit when it's done */
> while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
> @@ -206,7 +206,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
> return;
> }
> timeout--;
> - mdelay(1);
> + udelay(10);
> }
>
> if (host->ops->platform_reset_exit)
> @@ -959,7 +959,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
> WARN_ON(host->cmd);
>
> /* Wait max 10 ms */
> - timeout = 10;
> + timeout = 1000;
>
> mask = SDHCI_CMD_INHIBIT;
> if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
> @@ -980,7 +980,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
> return;
> }
> timeout--;
> - mdelay(1);
> + udelay(10);
> }
>
> mod_timer(&host->timer, jiffies + 10 * HZ);
> @@ -1140,7 +1140,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
> /* Wait max 20 ms */
> - timeout = 20;
> + timeout = 2000;
> while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> & SDHCI_CLOCK_INT_STABLE)) {
> if (timeout == 0) {
> @@ -1150,7 +1150,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> return;
> }
> timeout--;
> - mdelay(1);
> + udelay(10);
> }
>
> clk |= SDHCI_CLOCK_CARD_EN;
> --
> 1.7.3.4
-barry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
2011-10-27 4:18 [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases Shawn.Dong
2011-10-27 5:09 ` Barry Song
@ 2011-10-27 5:15 ` Wolfram Sang
2011-10-27 5:09 ` Shawn.Dong
1 sibling, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2011-10-27 5:15 UTC (permalink / raw)
To: Shawn.Dong; +Cc: linux-mmc
[-- Attachment #1: Type: text/plain, Size: 773 bytes --]
Hi,
On Thu, Oct 27, 2011 at 12:18:53PM +0800, Shawn.Dong wrote:
> sdhci_set_clock or sdhci_reset or sdhci_send_command may be used in
> critical region which is protected by spin_lock_irqsave. Thus, these
> functions will delay the responsing of the kernel interrupts.
Yes, so this needs to be improved, not the delay values.
> So in this case, using a mdelay will cause unnecessary latency. Our
> hardware, in most case will not cause 1ms to finish its job. Using
> udelay instead can reduce it.
Could you guarantee this for all other SDHCI hardware out there as well?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
2011-10-27 5:15 ` Wolfram Sang
@ 2011-10-27 5:09 ` Shawn.Dong
2011-10-27 6:02 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Shawn.Dong @ 2011-10-27 5:09 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc
On Thu, Oct 27, 2011 at 07:15:32AM +0200, Wolfram Sang wrote:
>Hi,
>
>On Thu, Oct 27, 2011 at 12:18:53PM +0800, Shawn.Dong wrote:
>> sdhci_set_clock or sdhci_reset or sdhci_send_command may be used in
>> critical region which is protected by spin_lock_irqsave. Thus, these
>> functions will delay the responsing of the kernel interrupts.
>
>Yes, so this needs to be improved, not the delay values.
>
>> So in this case, using a mdelay will cause unnecessary latency. Our
>> hardware, in most case will not cause 1ms to finish its job. Using
>> udelay instead can reduce it.
>
>Could you guarantee this for all other SDHCI hardware out there as well?
Even there are some SDHCI hardwares cannot be stable in microseconds, I
think this is also OK since they just need to wait for a few more loops. The
total waiting time is the same as before.
For the SDHCI hardware which need less time to be stable, this patch can improve
performance for them.
Thanks
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
2011-10-27 5:09 ` Shawn.Dong
@ 2011-10-27 6:02 ` Wolfram Sang
2011-11-04 8:24 ` Shawn.Dong
0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2011-10-27 6:02 UTC (permalink / raw)
To: Shawn.Dong; +Cc: linux-mmc
[-- Attachment #1: Type: text/plain, Size: 507 bytes --]
> Even there are some SDHCI hardwares cannot be stable in microseconds, I
> think this is also OK since they just need to wait for a few more loops. The
> total waiting time is the same as before.
Well, I still think this is curing the symptoms not the cause. But will talk
with Chris about it since we are both in Prague at the moment...
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases
2011-10-27 6:02 ` Wolfram Sang
@ 2011-11-04 8:24 ` Shawn.Dong
0 siblings, 0 replies; 6+ messages in thread
From: Shawn.Dong @ 2011-11-04 8:24 UTC (permalink / raw)
To: cjb, w.sang; +Cc: linux-mmc
On Thu, Oct 27, 2011 at 08:02:13AM +0200, Wolfram Sang wrote:
>
>> Even there are some SDHCI hardwares cannot be stable in microseconds, I
>> think this is also OK since they just need to wait for a few more loops. The
>> total waiting time is the same as before.
>
>Well, I still think this is curing the symptoms not the cause. But will talk
>with Chris about it since we are both in Prague at the moment...
Hi Wolfram & Chris,
This issue was found by using ftrace to track the irqsoff latency. Each time
to call mmc_power_off, the latency is about 1ms on my platform. And this 1ms
delay is causing by the mdelay(1) in _set_ios.
You know, after changing some register like CLOCK_CONTROL or RESET, driver
will start to check them every 1ms since we are using mdelay(1). For some
devices, it needn't software to wait for 1ms, but only some milliseconds. So
I think change to use udelay() is much better for those devices.
How do you think? Can you give some of your advice? Thank you.
Best Regards
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-04 8:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-27 4:18 [PATCH]mmc: sdhci: use udelay instead of mdelay for some cases Shawn.Dong
2011-10-27 5:09 ` Barry Song
2011-10-27 5:15 ` Wolfram Sang
2011-10-27 5:09 ` Shawn.Dong
2011-10-27 6:02 ` Wolfram Sang
2011-11-04 8:24 ` Shawn.Dong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.