diff mbox

mmc: add a short delay in mmc_power_off

Message ID 20110907092209.A52409D401C@zog.reactivated.net (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Drake Sept. 7, 2011, 9:22 a.m. UTC
Stress-testing the runtime power management of libertas_sdio
through a rmmod/insmod loop revealed that it is quite easy to
cause an ETIMEDOUT failure in mmc_sdio_power_restore() leading to:
   libertas_sdio: probe of mmc1:0001:1 failed with error -16

Experimentation shows that a very short delay (100us) is needed in
the power down path before the card can be successfully booted again.
We know that this setup is lacking poweroff clamps on the card's power
lines, but as only a short delay is needed, apply this unconditionally.
Also bump up to 1ms sleep for extra legroom.

Signed-off-by: Daniel Drake <dsd@laptop.org>
---
 drivers/mmc/core/core.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

Comments

Daniel Drake Sept. 7, 2011, 7:03 p.m. UTC | #1
On Wed, Sep 7, 2011 at 12:44 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> No, I wouldn't write a driver for this...
>
> I suggest first to understand exactly what's the bug, and that really
> involves using a scope here.

I'll see what can be done, but no promises.

> Adding random sleeps without understanding what exactly do they solve
> generally feels wrong...

Well, if you remove the existing sleeps from mmc_power_up do things
keep working for you?

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen Sept. 7, 2011, 7:12 p.m. UTC | #2
On Wed, Sep 7, 2011 at 10:03 PM, Daniel Drake <dsd@laptop.org> wrote:
>> Adding random sleeps without understanding what exactly do they solve
>> generally feels wrong...
>
> Well, if you remove the existing sleeps from mmc_power_up do things
> keep working for you?

I wouldn't call them random - they're pretty much explained.

OTOH, the delay you're suggesting to add isn't: we don't know what is
it solving exactly and why is it needed.

IMHO mainline kernel code should be well explained, otherwise it'd
become impossible to maintain.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ohad Ben Cohen Sept. 7, 2011, 7:37 p.m. UTC | #3
On Wed, Sep 7, 2011 at 10:03 PM, Daniel Drake <dsd@laptop.org> wrote:
> On Wed, Sep 7, 2011 at 12:44 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> No, I wouldn't write a driver for this...
>>
>> I suggest first to understand exactly what's the bug, and that really
>> involves using a scope here.
>
> I'll see what can be done, but no promises.

It would be nice if you can at least ask your hardware guys about it.

The delay is benign and no one would notice it but if you can at least
get some explanation of what's going on it'd really help, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 557856b..7289e99 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1214,6 +1214,13 @@  static void mmc_power_off(struct mmc_host *host)
 	host->ios.timing = MMC_TIMING_LEGACY;
 	mmc_set_ios(host);
 
+	/*
+	 * Some configurations, such as the 802.11 SDIO card in the OLPC
+	 * XO-1.5, require a short delay after poweroff before the card
+	 * can be successfully turned on again.
+	 */
+	mmc_delay(1);
+
 	mmc_host_clk_release(host);
 }