mmc: sdio: reset card during power_restore

Commit Message

Ohad Ben Cohen June 9, 2011, 5:27 p.m. UTC

On Thu, Jun 9, 2011 at 7:44 PM, Daniel Drake <[e-mail]> wrote:
> Note that during the insmod/rmmod calls, runtime PM did not touch the
> power state - the card remained powered ever since the first insmod.
> Not sure if this is in-line with your expectations.

It definitely isn't. And that may very well be our culprit here.

Let's focus now on just this one sequence:

- boot
- cat /sys/.../ios (power should be off)
- insmod
- cat /sys/.../ios (power should be on)
- rmmod
- cat /sys/.../ios (power should be off again)

You will see the problem now.

And I think I know why - there was a driver core change that
indirectly caused this.

Please try to revert e1866b33b1e89f077b7132daae3dfd9a594e9a1a "PM /
Runtime: Rework runtime PM handling during driver removal" and tell me
if it helped.

If it did, un-revert that change, and try this one patch, which adopts
SDIO runtime PM to that change:


Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Daniel Drake June 9, 2011, 5:56 p.m. UTC | #1

On 9 June 2011 18:27, Ohad Ben-Cohen <[e-mail]> wrote:
> Let's focus now on just this one sequence:
>
> - boot
> - cat /sys/.../ios (power should be off)
> - insmod
> - cat /sys/.../ios (power should be on)
> - rmmod
> - cat /sys/.../ios (power should be off again)
>
> You will see the problem now.

As I think you expected, everything went fine until the last "cat"
where it showed that the power is still on (no change from the
previous cat).

> And I think I know why - there was a driver core change that
> indirectly caused this.
>
> Please try to revert e1866b33b1e89f077b7132daae3dfd9a594e9a1a "PM /
> Runtime: Rework runtime PM handling during driver removal" and tell me
> if it helped.

It does. Now your test case works, power gets turned off on rmmod.

> If it did, un-revert that change, and try this one patch, which adopts
> SDIO runtime PM to that change:
>
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index d29b9c3..d2565df 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -189,7 +189,7 @@ static int sdio_bus_remove(struct device *dev)
>
>        /* Then undo the runtime PM settings in sdio_bus_probe() */
>        if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
> -               pm_runtime_put_noidle(dev);
> +               pm_runtime_put_sync(dev);
>
>  out:
>        return ret;

Done, the rmmod issue remains fixed - the card gets powered down during rmmod.

rmmod/insmod repeatedly works as expected. It gets powered down on
rmmod, and powered up on insmod. The power-up works fine and ios
values look fine at all steps.

Progress! :)



Next up is the problem where on this setup, if I suspend after loading
the module, and the module asks the card to shut down during suspend,
it fails to get brought up during resume.
Note that I have now rolled in a patch you made in another thread to
correctly call into the driver's remove routine during suspend when
powering down the card.
I also added a printk message to detect when the CMD5 reset command failed.

New patch: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug5.patch

Test case:

- boot
- mount debugfs
- insmod
- suspend
- resume
- cat /sys/.../ios

Output from resume onwards:

[   60.210326] Restarting tasks ... done.
[   60.346261] cfg80211: World regulatory domain updated:
[   60.368829] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   60.394947] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   60.425084] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   60.433390] usb 1-2: USB disconnect, device number 2
[   60.475817] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   60.587866] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   60.611597] cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   60.684817] cfg80211: Calling CRDA to update world regulatory domain
[   60.736397] cfg80211: World regulatory domain updated:
[   60.784745] cfg80211:     (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp)
[   60.801158] usb 1-2: new high speed USB device number 3 using ehci_hcd
[   60.815608] cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   60.841586] cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   60.883868] cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz),
(300 mBi, 2000 mBm)
[   60.893420] mmc1: new SDIO card at address 0001
[   60.913405] mmc_power_save_host mmc1
[   60.917148] mmc_power_restore_host mmc1
[   60.931707] cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz),
(300 mBi, 2000 mBm)
[   60.983389] usb 1-2: New USB device found, idVendor=090c, idProduct=1000
[   61.011508] CMD5 reset failed, err=-110
[   61.015475] libertas_sdio: probe of mmc1:0001:1 failed with error -16

Full log: http://dev.laptop.org/~dsd/20110609/sd-pwr-debug5.txt

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index d29b9c3..d2565df 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -189,7 +189,7 @@  static int sdio_bus_remove(struct device *dev)

        /* Then undo the runtime PM settings in sdio_bus_probe() */
        if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
-               pm_runtime_put_noidle(dev);
+               pm_runtime_put_sync(dev);

 out:
        return ret;