-ENOSYS suspend-powerdown regression

-ENOSYS suspend-powerdown regression

[Daniel Drake]

Hi,

The libertas_sdio driver has a suspend routine which returns -ENOSYS
when the card should effectively be removed and powered during
suspend, to be re-probed during resume.

This is broken in linus master. Everything appears somewhat normal
going down into suspend:

[   70.217948] libertas_sdio mmc1:0001:1: mmc1:0001:1: suspend: PM flags = 0x3
[   70.225588] uhci_hcd 0000:00:10.0: PCI INT A disabled
[   70.231140] viafb 0000:00:01.0: PCI INT A disabled
[   70.236233] libertas_sdio mmc1:0001:1: Suspend without wake params
-- powering down
[   70.244588] mmc1: card 0001 removed

but the "remove" routine of libertas_sdio is never called, meaning
that things get very confused during resume.

It was definitely working before (although I couldn't pinpoint exactly when).

Any ideas?

Thanks,
Daniel

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 30 May 2011 15:26, Daniel Drake <dsd@laptop.org> wrote:
> Hi,
>
> The libertas_sdio driver has a suspend routine which returns -ENOSYS
> when the card should effectively be removed and powered during
> suspend, to be re-probed during resume.
>
> This is broken in linus master. Everything appears somewhat normal
> going down into suspend:

Never mind - this still works in linus master. It just fails when I
enable runtime power management, another thing I'm working on. I'll
look closer.

Daniel

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 30 May 2011 15:26, Daniel Drake <dsd@laptop.org> wrote:
> Hi,
>
> The libertas_sdio driver has a suspend routine which returns -ENOSYS
> when the card should effectively be removed and powered during
> suspend, to be re-probed during resume.
>
> This is broken in linus master.

Actually, it only breaks when I enable runtime PM via the patch we are
discussing in the other thread.

Here is the call trace:

mmc_suspend_host
mmc_sdio_remove
sdio_remove_func
device_del
sdio_bus_remove

In sdio_bus_remove, we hit:

	/* Make sure card is powered before invoking ->remove() */
	if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) {
		ret = pm_runtime_get_sync(dev);
		if (ret < 0)
			goto out;
	}

pm_runtime_get_sync returns -11, therefore we skip the following
drv->remove call, causing this confusion. -11 is EAGAIN

Digging further, in the pm_runtime_get_sync() call we reach
rpm_resume() in drivers/base/power where we hit:

	else if (dev->power.disable_depth > 0)
		retval = -EAGAIN;

Not sure what this means. Any thoughts?

Thanks,
Daniel

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

On Sat, Jun 4, 2011 at 1:18 PM, Daniel Drake <dsd@laptop.org> wrote:
> Digging further, in the pm_runtime_get_sync() call we reach
> rpm_resume() in drivers/base/power where we hit:
>
>        else if (dev->power.disable_depth > 0)
>                retval = -EAGAIN;
>
> Not sure what this means. Any thoughts?

It means runtime PM is disabled for your device.

Which doesn't make sense, assuming you had MMC_CAP_POWER_OFF_CARD
configured (if you didn't, you wouldn't end up calling runtime PM api
at all).

If you post your entire diff, I might be able to spot what's wrong.

As a side note, if the additional CMD5 arg=0 fixes both sd8686's and
b43's issues, we might consider removing MMC_CAP_POWER_OFF_CARD
entirely. But let's not rush with that yet.

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On 4 June 2011 11:33, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> It means runtime PM is disabled for your device.
>
> Which doesn't make sense, assuming you had MMC_CAP_POWER_OFF_CARD
> configured (if you didn't, you wouldn't end up calling runtime PM api
> at all).
>
> If you post your entire diff, I might be able to spot what's wrong.

Its not really different from what I posted before. Just with some
added printks and a hack to enable POWER_OFF_CARD for sdhci. Attaching
it anyway.

Is it possible that there is confusion as to whether runtime PM is
enabled or disabled for the device because we are so late at this
point in device teardown?

Daniel

[-- Attachment #2: runtime-pm.txt --]
[-- Type: text/plain, Size: 6586 bytes --]

commit 2460eee717cbae4e683edeb17ffe448b0c32fd28
Author: Daniel Drake <dsd@laptop.org>
Date:   Tue May 31 12:51:39 2011 +0100

    runtime suspend WIP

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 0d4587b..ddb0ae5 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -452,7 +452,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
 	struct device *parent = NULL;
 	int retval = 0;
 
-	dev_dbg(dev, "%s flags 0x%x\n", __func__, rpmflags);
+	dev_warn(dev, "%s flags 0x%x\n", __func__, rpmflags);
 
  repeat:
 	if (dev->power.runtime_error)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68091dd..8a59072 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1664,6 +1664,7 @@ int mmc_power_save_host(struct mmc_host *host)
 	int ret = 0;
 
 	mmc_bus_get(host);
+printk("mmc_power_save_host %s\n", mmc_hostname(host));
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
 		mmc_bus_put(host);
@@ -1686,6 +1687,7 @@ int mmc_power_restore_host(struct mmc_host *host)
 	int ret;
 
 	mmc_bus_get(host);
+printk("mmc_power_restore_host %s\n", mmc_hostname(host));
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
 		mmc_bus_put(host);
@@ -1751,6 +1753,8 @@ int mmc_suspend_host(struct mmc_host *host)
 {
 	int err = 0;
 
+	printk("mmc_suspend_host %s\n", mmc_hostname(host));
+
 	if (host->caps & MMC_CAP_DISABLE)
 		cancel_delayed_work(&host->disable);
 	cancel_delayed_work(&host->detect);
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 4d0c15b..a241176 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -537,11 +537,14 @@ static void mmc_sdio_remove(struct mmc_host *host)
 {
 	int i;
 
+	printk("mmc_sdio_remove %s\n", mmc_hostname(host));
+
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
 	for (i = 0;i < host->card->sdio_funcs;i++) {
 		if (host->card->sdio_func[i]) {
+			printk("remove func %d\n", i);
 			sdio_remove_func(host->card->sdio_func[i]);
 			host->card->sdio_func[i] = NULL;
 		}
@@ -647,6 +650,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
+printk("mmc_sdio_resume %s\n", mmc_hostname(host));
+
 	/* Basic card reinitialization. */
 	mmc_claim_host(host);
 
@@ -691,15 +696,47 @@ static int mmc_sdio_resume(struct mmc_host *host)
 static int mmc_sdio_power_restore(struct mmc_host *host)
 {
 	int ret;
+	u32 ocr;
 
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
 	mmc_claim_host(host);
+
+	ret = mmc_send_io_op_cond(host, 0, &ocr);
+	if (ret)
+		goto out;
+
+	if (host->ocr_avail_sdio)
+		host->ocr_avail = host->ocr_avail_sdio;
+
+	/*
+	 * Sanity check the voltages that the card claims to
+	 * support.
+	 */
+	if (ocr & 0x7F) {
+		printk(KERN_WARNING "%s: card claims to support voltages "
+		       "below the defined range. These will be ignored.\n",
+		       mmc_hostname(host));
+		ocr &= ~0x7F;
+	}
+
+	host->ocr = mmc_select_voltage(host, ocr);
+
+	/*
+	 * Can we support the voltage(s) of the card(s)?
+	 */
+	if (!host->ocr) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	ret = mmc_sdio_init_card(host, host->ocr, host->card,
 				mmc_card_keep_power(host));
 	if (!ret && host->sdio_irqs)
 		mmc_signal_sdio_irq(host);
+
+out:
 	mmc_release_host(host);
 
 	return ret;
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index d29b9c3..fe332de 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -166,13 +166,17 @@ static int sdio_bus_remove(struct device *dev)
 	struct sdio_func *func = dev_to_sdio_func(dev);
 	int ret = 0;
 
+dev_warn(dev, "sdio_bus_remove\n");
+
 	/* Make sure card is powered before invoking ->remove() */
 	if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) {
 		ret = pm_runtime_get_sync(dev);
+printk("get_sync %d\n", ret);
 		if (ret < 0)
 			goto out;
 	}
 
+printk("CALL REMOVE\n");
 	drv->remove(func);
 
 	if (func->irq_handler) {
@@ -314,9 +318,11 @@ int sdio_add_func(struct sdio_func *func)
  */
 void sdio_remove_func(struct sdio_func *func)
 {
+	printk("sdio_remove_func\n");
 	if (!sdio_func_present(func))
 		return;
 
+	dev_warn(&func->dev, "do device_del\n");
 	device_del(&func->dev);
 	put_device(&func->dev);
 }
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 58d5436..ce3e2e2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2488,7 +2488,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23 | MMC_CAP_POWER_OFF_CARD;
 
 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_AUTO_CMD12;
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 224e985..d2b077b 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -946,7 +946,7 @@ static int if_sdio_probe(struct sdio_func *func,
 	unsigned int model;
 	struct if_sdio_packet *packet;
 	struct mmc_host *host = func->card->host;
-
+printk("if_sdio_probe %p\n", &func->dev);
 	lbs_deb_enter(LBS_DEB_SDIO);
 
 	for (i = 0;i < func->card->num_info;i++) {
@@ -1124,6 +1124,7 @@ static int if_sdio_probe(struct sdio_func *func,
 out:
 	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
 
+printk("if_sdio_probe done %d\n", ret);
 	return ret;
 
 err_activate_card:
@@ -1159,6 +1160,8 @@ static void if_sdio_remove(struct sdio_func *func)
 	struct if_sdio_card *card;
 	struct if_sdio_packet *packet;
 
+printk("if_sdio_remove\n");
+dump_stack();
 	lbs_deb_enter(LBS_DEB_SDIO);
 
 	card = sdio_get_drvdata(func);
@@ -1204,6 +1207,7 @@ static void if_sdio_remove(struct sdio_func *func)
 		kfree(card->firmware);
 	kfree(card);
 
+printk("if_sdio_remove done\n");
 	lbs_deb_leave(LBS_DEB_SDIO);
 }
 
@@ -1212,8 +1216,8 @@ static int if_sdio_suspend(struct device *dev)
 	struct sdio_func *func = dev_to_sdio_func(dev);
 	int ret;
 	struct if_sdio_card *card = sdio_get_drvdata(func);
-
 	mmc_pm_flag_t flags = sdio_get_host_pm_caps(func);
+printk("if_sdio_suspend %p\n", dev);
 
 	dev_info(dev, "%s: suspend: PM flags = 0x%x\n",
 		 sdio_func_id(func), flags);
@@ -1249,6 +1253,7 @@ static int if_sdio_resume(struct device *dev)
 	struct if_sdio_card *card = sdio_get_drvdata(func);
 	int ret;
 
+printk("if_sdio_resume %p\n", dev);
 	dev_info(dev, "%s: resume: we're back\n", sdio_func_id(func));
 
 	ret = lbs_resume(card->priv);

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

... going back to the call trace you provided ...

On Sat, Jun 4, 2011 at 1:18 PM, Daniel Drake <dsd@laptop.org> wrote:
> Here is the call trace:
>
> mmc_suspend_host
> mmc_sdio_remove
> sdio_remove_func
> device_del

This eventually disables runtime PM for your SDIO function.

We can stop checking the return value of pm_runtime_get_sync in
sdio_bus_remove, like pci_device_remove is doing, but I don't think
that's enough: I guess libertas' if_sdio_remove won't be so happy
dealing with a powered off card.

Can you try the following please (untested):

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 4d0c15b..8af3330 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -540,6 +540,13 @@ static void mmc_sdio_remove(struct mmc_host *host)
 	BUG_ON(!host);
 	BUG_ON(!host->card);

+	/*
+	 * if this card is managed by runtime pm, make sure it is powered on
+	 * before invoking its SDIO functions' ->remove() handler
+	 */
+	if (host->caps & MMC_CAP_POWER_OFF_CARD)
+		pm_runtime_get_sync(&host->card->dev);
+
 	for (i = 0;i < host->card->sdio_funcs;i++) {
 		if (host->card->sdio_func[i]) {
 			sdio_remove_func(host->card->sdio_func[i]);
@@ -547,6 +554,9 @@ static void mmc_sdio_remove(struct mmc_host *host)
 		}
 	}

+	if (host->caps & MMC_CAP_POWER_OFF_CARD)
+		pm_runtime_put_noidle(&host->card->dev);
+
 	mmc_remove_card(host->card);
 	host->card = NULL;
 }
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index d29b9c3..73dc3c2 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -167,11 +167,8 @@ static int sdio_bus_remove(struct device *dev)
 	int ret = 0;

 	/* Make sure card is powered before invoking ->remove() */
-	if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) {
-		ret = pm_runtime_get_sync(dev);
-		if (ret < 0)
-			goto out;
-	}
+	if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
+		pm_runtime_get_sync(dev);

 	drv->remove(func);

@@ -191,7 +188,6 @@ static int sdio_bus_remove(struct device *dev)
 	if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
 		pm_runtime_put_noidle(dev);

-out:
 	return ret;
 }

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> We can stop checking the return value of pm_runtime_get_sync in
> sdio_bus_remove, like pci_device_remove is doing, but I don't think
> that's enough: I guess libertas' if_sdio_remove won't be so happy
> dealing with a powered off card.
>
> Can you try the following please (untested):

Thanks, that seems to be working.

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

On Sun, Jun 5, 2011 at 3:31 PM, Daniel Drake <dsd@laptop.org> wrote:
> On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> We can stop checking the return value of pm_runtime_get_sync in
>> sdio_bus_remove, like pci_device_remove is doing, but I don't think
>> that's enough: I guess libertas' if_sdio_remove won't be so happy
>> dealing with a powered off card.
>>
>> Can you try the following please (untested):
>
> Thanks, that seems to be working.

Thanks.

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

Hi Daniel,

On Sun, Jun 5, 2011 at 3:31 PM, Daniel Drake <dsd@laptop.org> wrote:
> On 5 June 2011 00:08, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> We can stop checking the return value of pm_runtime_get_sync in
>> sdio_bus_remove, like pci_device_remove is doing, but I don't think
>> that's enough: I guess libertas' if_sdio_remove won't be so happy
>> dealing with a powered off card.
>>
>> Can you try the following please (untested):
>
> Thanks, that seems to be working.

That patch had two hunks.

The first one, in mmc_sdio_remove, made sure libertas' ->remove()
handler will be called powered on, even if the chip was powered off
beforehand.

The second one, in sdio_bus_remove, directly took care of the problem
you saw by ignoring rumtime PM's return value (which is a valid error
in your scenario).

Obviously the second hunk is necessary, but I'd like to know whether
the first one really is too or not. Can you please retest this without
that hunk (try to suspend/resume while the chip is powered off, and
again while it is powered on, but wol isn't used) ?

If the second hunk is sufficient (everything works as expected + no
scary error messages coming from libertas), I would like to avoid the
first one. I'm not sure at all there's any reason to power up the card
if it is being removed, and a quick glance shows libertas_sdio has
this 'user_rmmod' flag which should prevent scary error messages in
this exact scenario.

Thanks,
Ohad.

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Obviously the second hunk is necessary, but I'd like to know whether
> the first one really is too or not. Can you please retest this without
> that hunk (try to suspend/resume while the chip is powered off, and
> again while it is powered on, but wol isn't used) ?

Exactly which kernel should I run this test on?

Thanks,
Daniel

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

On Tue, Jun 28, 2011 at 11:59 PM, Daniel Drake <dsd@laptop.org> wrote:
> On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> Obviously the second hunk is necessary, but I'd like to know whether
>> the first one really is too or not. Can you please retest this without
>> that hunk (try to suspend/resume while the chip is powered off, and
>> again while it is powered on, but wol isn't used) ?
>
> Exactly which kernel should I run this test on?

Latest (isn't that what you've been working with all this time ?).

Thanks,
Ohad.

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 28 June 2011 22:47, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Tue, Jun 28, 2011 at 11:59 PM, Daniel Drake <dsd@laptop.org> wrote:
>> On 28 June 2011 06:55, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>>> Obviously the second hunk is necessary, but I'd like to know whether
>>> the first one really is too or not. Can you please retest this without
>>> that hunk (try to suspend/resume while the chip is powered off, and
>>> again while it is powered on, but wol isn't used) ?
>>
>> Exactly which kernel should I run this test on?
>
> Latest (isn't that what you've been working with all this time ?).

latest linux-mmc.git ? Or linux-next? Or linus?

I'll apply the patch in this thread, and my patch titled "mmc: sdio:
reset card during power_restore" - anything else?

Too many patches floating around, just trying to be sure of what I'm doing!

Thanks,
Daniel

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

On Wed, Jun 29, 2011 at 12:54 AM, Daniel Drake <dsd@laptop.org> wrote:
> Too many patches floating around, just trying to be sure of what I'm doing!

Ok, let's start from fresh :)

Let's take 3.0-rc5, it already includes our work:

297c7f2 mmc: sdio: fix runtime PM path during driver removal
c6e633a mmc: sdio: reset card during power_restore

Then apply these hunks:

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index d29b9c3..73dc3c2 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -167,11 +167,8 @@ static int sdio_bus_remove(struct device *dev)
       int ret = 0;

       /* Make sure card is powered before invoking ->remove() */
-       if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) {
-               ret = pm_runtime_get_sync(dev);
-               if (ret < 0)
-                       goto out;
-       }
+       if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
+               pm_runtime_get_sync(dev);

       drv->remove(func);

@@ -191,7 +188,6 @@ static int sdio_bus_remove(struct device *dev)
       if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD)
               pm_runtime_put_noidle(dev);

-out:
       return ret;
 }

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 58d5436..ce3e2e2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2488,7 +2488,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;

-	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
+	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23 |
MMC_CAP_POWER_OFF_CARD;

 	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
 		host->flags |= SDHCI_AUTO_CMD12;


I think this should be enough (I don't remember anything else pending).

Thanks,
Ohad.

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 28 June 2011 23:04, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Wed, Jun 29, 2011 at 12:54 AM, Daniel Drake <dsd@laptop.org> wrote:
>> Too many patches floating around, just trying to be sure of what I'm doing!
>
> Ok, let's start from fresh :)
>
> Let's take 3.0-rc5, it already includes our work:
>
> 297c7f2 mmc: sdio: fix runtime PM path during driver removal
> c6e633a mmc: sdio: reset card during power_restore
>
> Then apply these hunks:

Thanks, done. Sorry for the delay in getting to this.

Unfortunately something is still weird, this is possibly a case that
we missed before.

Even if I just do the simplistic test of booting, and then suspending
(without loading the libertas driver), things go wrong:
http://dev.laptop.org/~dsd/20110706/dmesg.txt

I added printk's in mmc_power_save_host and mmc_power_restore_host.
The strange thing is that it tries to resume mmc1 even though it is
powered down and there is no available driver, and also that it then
tries to power it down again (but it is already off).

Daniel

Re: -ENOSYS suspend-powerdown regression

[Daniel Drake]

On 6 July 2011 16:53, Daniel Drake <dsd@laptop.org> wrote:
> Thanks, done. Sorry for the delay in getting to this.
>
> Unfortunately something is still weird, this is possibly a case that
> we missed before.
>
> Even if I just do the simplistic test of booting, and then suspending
> (without loading the libertas driver), things go wrong:
> http://dev.laptop.org/~dsd/20110706/dmesg.txt
>
> I added printk's in mmc_power_save_host and mmc_power_restore_host.
> The strange thing is that it tries to resume mmc1 even though it is
> powered down and there is no available driver, and also that it then
> tries to power it down again (but it is already off).

I tested vanilla and realised that this bug exists there too - it is
not related to your patch. So lets ignore it for now so that we can
get unblocked on the runtime PM work.

Your latest patch works fine. Card got powered down during suspend,
and powered on,off,on during resume (as expected) and probed fine and
continued working :)

So I'd say we're good to go on this one and move onto the other pending stuff.

Thanks!
Daniel

Re: -ENOSYS suspend-powerdown regression

[Ohad Ben-Cohen]

On Fri, Jul 8, 2011 at 6:38 PM, Daniel Drake <dsd@laptop.org> wrote:
> I tested vanilla and realised that this bug exists there too

I'm aware of it - it's caused (indirectly) by another PM core change,
which since then was already changed again in 3.1.

Btw sorry for not being responsive - I've been mostly offline lately
due to family getting bigger ;)

Thanks for pushing the patches forward.