* sdhci : reduce irq off latency
@ 2010-06-17 9:07 Matthieu CASTET
2010-06-18 0:16 ` Kyungmin Park
2010-08-28 2:23 ` Chris Ball
0 siblings, 2 replies; 6+ messages in thread
From: Matthieu CASTET @ 2010-06-17 9:07 UTC (permalink / raw)
To: linux-mmc
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
sdhci code got tasklets (sdhci_tasklet_card and sdhci_tasklet_finish),
that does :
{
spin_lock_irqsave
if (cond) {
sdhci_reset
sdhci_reset
}
spin_unlock_irqrestore
}
sdhci_reset {
...
while (read_reg) {
if (timeout == 0)
break;
timeout--;
mdelay(1);
}
...
}
The problem is that sdhci_reset [1] does busy pooling (with a
granularity of 1 ms) on a register up to a timeout of 100 ms.
With the current code, we got irq off during 2*1ms. With the attached
patch we reduce irq off to 30 us.
Note that worst case 100 ms irq off still exist.
Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
[-- Attachment #2: sdhci.diff --]
[-- Type: text/x-diff, Size: 2310 bytes --]
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..03d6bde 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -157,18 +157,17 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
host->clock = 0;
/* Wait max 100 ms */
- timeout = 100;
+ timeout = jiffies + msecs_to_jiffies(100);
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Reset 0x%x never completed.\n",
mmc_hostname(host->mmc), (int)mask);
sdhci_dumpregs(host);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
@@ -882,7 +881,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 = jiffies + msecs_to_jiffies(10);
mask = SDHCI_CMD_INHIBIT;
if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -894,7 +893,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
mask &= ~SDHCI_DATA_INHIBIT;
while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Controller never released "
"inhibit bit(s).\n", mmc_hostname(host->mmc));
sdhci_dumpregs(host);
@@ -902,8 +901,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
tasklet_schedule(&host->finish_tasklet);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
mod_timer(&host->timer, jiffies + 10 * HZ);
@@ -1007,17 +1005,16 @@ 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 = jiffies + msecs_to_jiffies(20)
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Internal clock never "
"stabilised.\n", mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
clk |= SDHCI_CLOCK_CARD_EN;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sdhci : reduce irq off latency
2010-06-17 9:07 sdhci : reduce irq off latency Matthieu CASTET
@ 2010-06-18 0:16 ` Kyungmin Park
2010-06-18 7:57 ` Matthieu CASTET
2010-08-28 2:23 ` Chris Ball
1 sibling, 1 reply; 6+ messages in thread
From: Kyungmin Park @ 2010-06-18 0:16 UTC (permalink / raw)
To: Matthieu CASTET; +Cc: linux-mmc
Hi,
Not good news. This patch make a hang at probe time.
it's tested against commit 7e27d6e778cd87b6f2415515d7127eba53fe5d02
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Fri Jun 11 19:14:04 2010 -0700
Linux 2.6.35-rc3
Thank you,
Kyungmin Park
On Thu, Jun 17, 2010 at 6:07 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> sdhci code got tasklets (sdhci_tasklet_card and sdhci_tasklet_finish),
> that does :
> {
> spin_lock_irqsave
>
> if (cond) {
> sdhci_reset
> sdhci_reset
> }
>
> spin_unlock_irqrestore
> }
>
> sdhci_reset {
> ...
> while (read_reg) {
> if (timeout == 0)
> break;
> timeout--;
> mdelay(1);
> }
> ...
> }
>
>
> The problem is that sdhci_reset [1] does busy pooling (with a granularity of
> 1 ms) on a register up to a timeout of 100 ms.
>
> With the current code, we got irq off during 2*1ms. With the attached patch
> we reduce irq off to 30 us.
>
> Note that worst case 100 ms irq off still exist.
>
> Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sdhci : reduce irq off latency
2010-06-18 0:16 ` Kyungmin Park
@ 2010-06-18 7:57 ` Matthieu CASTET
2010-06-18 8:26 ` Kyungmin Park
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu CASTET @ 2010-06-18 7:57 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]
Hi,
Thanks you for the test.
here a new version of the patch. It doesn't replace mdelay with jiffies
anymore. It changes mdelay to udelay (the granularity of 1 ms becomes 10
us).
I didn't mesure irq off latency with this patch.
Matthieu
Kyungmin Park a écrit :
> Hi,
>
> Not good news. This patch make a hang at probe time.
> it's tested against commit 7e27d6e778cd87b6f2415515d7127eba53fe5d02
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Fri Jun 11 19:14:04 2010 -0700
>
> Linux 2.6.35-rc3
>
> Thank you,
> Kyungmin Park
>
> On Thu, Jun 17, 2010 at 6:07 PM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
>> sdhci code got tasklets (sdhci_tasklet_card and sdhci_tasklet_finish),
>> that does :
>> {
>> spin_lock_irqsave
>>
>> if (cond) {
>> sdhci_reset
>> sdhci_reset
>> }
>>
>> spin_unlock_irqrestore
>> }
>>
>> sdhci_reset {
>> ...
>> while (read_reg) {
>> if (timeout == 0)
>> break;
>> timeout--;
>> mdelay(1);
>> }
>> ...
>> }
>>
>>
>> The problem is that sdhci_reset [1] does busy pooling (with a granularity of
>> 1 ms) on a register up to a timeout of 100 ms.
>>
>> With the current code, we got irq off during 2*1ms. With the attached patch
>> we reduce irq off to 30 us.
>>
>> Note that worst case 100 ms irq off still exist.
>>
>> Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
>>
>>
[-- Attachment #2: sdhci.diff --]
[-- Type: text/x-diff, Size: 1617 bytes --]
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..fa06c70 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -157,7 +157,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
host->clock = 0;
/* Wait max 100 ms */
- timeout = 100;
+ timeout = 100*100;
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
@@ -168,7 +168,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
return;
}
timeout--;
- mdelay(1);
+ udelay(10);
}
if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
@@ -882,7 +882,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 = 10*100;
mask = SDHCI_CMD_INHIBIT;
if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -903,7 +903,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);
@@ -1007,7 +1007,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 = 20*100;
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (timeout == 0) {
@@ -1017,7 +1017,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
return;
}
timeout--;
- mdelay(1);
+ udelay(10);
}
clk |= SDHCI_CLOCK_CARD_EN;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sdhci : reduce irq off latency
2010-06-18 7:57 ` Matthieu CASTET
@ 2010-06-18 8:26 ` Kyungmin Park
0 siblings, 0 replies; 6+ messages in thread
From: Kyungmin Park @ 2010-06-18 8:26 UTC (permalink / raw)
To: Matthieu CASTET; +Cc: linux-mmc@vger.kernel.org
The main problem is sdhci_send_command function. it loops at while
function infinitely.
I just modify it to use udelay at there. then it's working well.
also I reduce the granularity as 1 usec instead of 10 usec (as you suggested).
it's also works.
Thank you,
Kyungmin Park
On Fri, Jun 18, 2010 at 4:57 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Hi,
>
> Thanks you for the test.
>
> here a new version of the patch. It doesn't replace mdelay with jiffies
> anymore. It changes mdelay to udelay (the granularity of 1 ms becomes 10
> us).
> I didn't mesure irq off latency with this patch.
>
> Matthieu
>
>
> Kyungmin Park a écrit :
>>
>> Hi,
>>
>> Not good news. This patch make a hang at probe time.
>> it's tested against commit 7e27d6e778cd87b6f2415515d7127eba53fe5d02
>> Author: Linus Torvalds <torvalds@linux-foundation.org>
>> Date: Fri Jun 11 19:14:04 2010 -0700
>>
>> Linux 2.6.35-rc3
>>
>> Thank you,
>> Kyungmin Park
>>
>> On Thu, Jun 17, 2010 at 6:07 PM, Matthieu CASTET
>> <matthieu.castet@parrot.com> wrote:
>>>
>>> sdhci code got tasklets (sdhci_tasklet_card and sdhci_tasklet_finish),
>>> that does :
>>> {
>>> spin_lock_irqsave
>>>
>>> if (cond) {
>>> sdhci_reset
>>> sdhci_reset
>>> }
>>>
>>> spin_unlock_irqrestore
>>> }
>>>
>>> sdhci_reset {
>>> ...
>>> while (read_reg) {
>>> if (timeout == 0)
>>> break;
>>> timeout--;
>>> mdelay(1);
>>> }
>>> ...
>>> }
>>>
>>>
>>> The problem is that sdhci_reset [1] does busy pooling (with a granularity
>>> of
>>> 1 ms) on a register up to a timeout of 100 ms.
>>>
>>> With the current code, we got irq off during 2*1ms. With the attached
>>> patch
>>> we reduce irq off to 30 us.
>>>
>>> Note that worst case 100 ms irq off still exist.
>>>
>>> Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
>>>
>>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sdhci : reduce irq off latency
2010-06-17 9:07 sdhci : reduce irq off latency Matthieu CASTET
2010-06-18 0:16 ` Kyungmin Park
@ 2010-08-28 2:23 ` Chris Ball
2010-08-30 9:40 ` Matthieu CASTET
1 sibling, 1 reply; 6+ messages in thread
From: Chris Ball @ 2010-08-28 2:23 UTC (permalink / raw)
To: Matthieu CASTET; +Cc: Kyungmin Park, linux-mmc
Hi Matthieu,
This patch didn't make it upstream. Kyungmin offered a suggestion for
fixing it -- would you like to resend an updated patch?
(As a minor aside, I have a fairly irrational preference for
"time_after(jiffies, timeout)" over "time_is_before_jiffies(timeout)",
because the latter reads as the inverse of what it actually does to me.
They compile to the same code.)
Date: Thu, 17 Jun 2010 11:07:15 +0200
From: Matthieu CASTET <matthieu.castet@parrot.com>
sdhci code got tasklets (sdhci_tasklet_card and sdhci_tasklet_finish),
that does :
{
spin_lock_irqsave
if (cond) {
sdhci_reset
sdhci_reset
}
spin_unlock_irqrestore
}
sdhci_reset {
...
while (read_reg) {
if (timeout == 0)
break;
timeout--;
mdelay(1);
}
...
}
The problem is that sdhci_reset [1] does busy pooling (with a granularity
of 1 ms) on a register up to a timeout of 100 ms.
With the current code, we got irq off during 2*1ms. With the attached
patch we reduce irq off to 30 us.
Note that worst case 100 ms irq off still exist.
Signed-off-by: Matthieu CASTET <matthieu.castet@parrot.com>
Signed-off-by: Chris Ball <cjb@laptop.org>
---
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..03d6bde 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -157,18 +157,17 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
host->clock = 0;
/* Wait max 100 ms */
- timeout = 100;
+ timeout = jiffies + msecs_to_jiffies(100);
/* hw clears the bit when it's done */
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Reset 0x%x never completed.\n",
mmc_hostname(host->mmc), (int)mask);
sdhci_dumpregs(host);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
@@ -882,7 +881,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 = jiffies + msecs_to_jiffies(10);
mask = SDHCI_CMD_INHIBIT;
if ((cmd->data != NULL) || (cmd->flags & MMC_RSP_BUSY))
@@ -894,7 +893,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
mask &= ~SDHCI_DATA_INHIBIT;
while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Controller never released "
"inhibit bit(s).\n", mmc_hostname(host->mmc));
sdhci_dumpregs(host);
@@ -902,8 +901,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
tasklet_schedule(&host->finish_tasklet);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
mod_timer(&host->timer, jiffies + 10 * HZ);
@@ -1007,17 +1005,16 @@ 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 = jiffies + msecs_to_jiffies(20)
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
- if (timeout == 0) {
+ if (time_is_before_jiffies(timeout)) {
printk(KERN_ERR "%s: Internal clock never "
"stabilised.\n", mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return;
}
- timeout--;
- mdelay(1);
+ cpu_relax();
}
clk |= SDHCI_CLOCK_CARD_EN;
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sdhci : reduce irq off latency
2010-08-28 2:23 ` Chris Ball
@ 2010-08-30 9:40 ` Matthieu CASTET
0 siblings, 0 replies; 6+ messages in thread
From: Matthieu CASTET @ 2010-08-30 9:40 UTC (permalink / raw)
To: Chris Ball; +Cc: Kyungmin Park, linux-mmc@vger.kernel.org
Hi Chris,
Chris Ball a écrit :
> Hi Matthieu,
>
> This patch didn't make it upstream. Kyungmin offered a suggestion for
> fixing it -- would you like to resend an updated patch?
It seems http://marc.info/?l=linux-kernel&m=127911287108683&w=2
(http://marc.info/?a=127137055700010&r=1&w=2) is a better solution.
Matthieu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-30 9:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-17 9:07 sdhci : reduce irq off latency Matthieu CASTET
2010-06-18 0:16 ` Kyungmin Park
2010-06-18 7:57 ` Matthieu CASTET
2010-06-18 8:26 ` Kyungmin Park
2010-08-28 2:23 ` Chris Ball
2010-08-30 9:40 ` Matthieu CASTET
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).