[PATCH v2 00/11] SDIO Runtime PM Support

[PATCH v2 00/11] SDIO Runtime PM Support

[Ohad Ben-Cohen]

Introduce SDIO runtime PM support:

1. Power to SDIO cards is kept low until one of its functions is bound
   (i.e. a matching driver is successfully probed)

2. If the matching driver supports runtime PM, power to the card is
   dropped soon after probe() returns. It is then up to the driver
   to request power to its function, using runtime PM API (the get/put
   variants). This is demonstrated with the wl1271 driver, in which
   the power of the card power is coupled with the state of the wlan
   interface (interface up -> power is up, interface down -> power is down)

3. If a matching driver does not support runtime PM, power to the card
   is kept high during the whole lifetime of the driver

4. When the driver is removed, power to the card is immediately dropped

5. If there are multiple drivers for the same card (several SDIO functions),
   power will be pulled high before the first driver probes, and dropped
   down after the last driver is removed. In between, power will be
   maintained accrording to the accumulated usage count of the complete
   drivers group

6. SDIO suspend/resume semantics are unchanged. In addition, when the system
   comes out of suspend, it is guaranteed that the power state of the
   SDIO card will reflect its runtime PM usage count.

7. What was NOT changed:
   - Interface: drivers can still assume that the card is powered
     when probe/remove/suspend/resume are called
   - Existing behavior: drivers that do not support runtime PM
     are unchanged

Changes since v1:
- Interaction with system suspend/resume
- Better commentary

Dependencies:
- SDIO patches are against mmc-next, and have a runtime dependency on
  commit "PM / Runtime: Lenient generic runtime pm callbacks"
  (patch is in linux-next now)
- WLAN patches depend on recent wl1271 activity, and they are here
  just to demonstrate the usage of the SDIO patchset (will be
  resubmitted separately)

The full patchset, together with all its dependencies, is also available at:
git://wizery.com/pub/mmc.git runtime-pm-v2

Tests:
Besides the usual functional tests, the patchset was stress tested with
an overnight execution of (thousands of suspend-to-rams interacting with
the different possible runtime PM power states of the device/driver):

#!/bin/sh
mount -t debugfs none /sys/kernel/debug
echo core > /sys/power/pm_test

validate_module_up()
{
	lsmod | grep wl1271_sdio
	if [ "$?" -ne 0 ]; then echo "Module is not up"; exit; fi
}

validate_module_down()
{
	lsmod | grep wl1271_sdio
	if [ "$?" -eq 0 ]; then echo "Module is not down"; exit; fi
}

validate_card_is_powered()
{
	cat /sys/kernel/debug/mmc2/ios | grep "power mode" | grep "on"
	if [ "$?" -ne 0 ]; then echo "Power is not on"; exit; fi

	cat /sys/kernel/debug/mmc2/ios | grep "clock" | grep "25000000"
	if [ "$?" -ne 0 ]; then echo "Clock failure"; exit; fi

	cat /sys/kernel/debug/mmc2/ios | grep "vdd" | grep "1.65 - 1.95"
	if [ "$?" -ne 0 ]; then echo "Voltage failure"; exit; fi

	cat /sys/kernel/debug/mmc2/ios | grep "bus width" | grep "4 bits"
	if [ "$?" -ne 0 ]; then echo "Bus width failure"; exit; fi
}

validate_card_is_suspended()
{
	cat /sys/kernel/debug/mmc2/ios | grep "power mode" | grep "off"
	if [ "$?" -ne 0 ]; then echo "power is not off"; exit; fi

	cat /sys/kernel/debug/mmc2/ios | grep "vdd" | grep "invalid"
	if [ "$?" -ne 0 ]; then echo "Voltage not down ?"; exit; fi
}

while [ 1 ]
do
	echo "beginning iteration $i"

	validate_module_down
	validate_card_is_suspended

	echo mem > /sys/power/state
	validate_card_is_suspended

	insmod wl1271_sdio.ko
	validate_module_up
	validate_card_is_suspended

	echo mem > /sys/power/state
	validate_card_is_suspended

	ifconfig wlan0 up
	validate_card_is_powered

	echo mem > /sys/power/state
	validate_card_is_powered

	ifconfig wlan0 down
	validate_card_is_suspended

	echo mem > /sys/power/state
	validate_card_is_suspended

	rmmod wl1271_sdio

	let "i+=1"
done

Note: the ios values tested for are obviously specific to the
wl1271 device.

Ohad Ben-Cohen (11):
  mmc: sdio: fully reconfigure oldcard on resume
  mmc: propagate power save/restore ops return value
  sdio: add power_restore support
  mmc: add runtime PM handlers
  sdio: use the generic runtime PM handlers
  sdio: enable runtime PM for SDIO cards
  sdio: enable runtime PM for SDIO functions
  sdio: ensure mmc_sdio_detect is powered
  sdio: support suspend/resume while runtime suspended
  wl1271: sdio: enable runtime PM
  wl1271: sdio: add suspend/resume support

 drivers/mmc/core/bus.c                    |   37 +++++++++++++
 drivers/mmc/core/core.c                   |   20 +++++--
 drivers/mmc/core/core.h                   |    4 +-
 drivers/mmc/core/mmc.c                    |    8 ++-
 drivers/mmc/core/sd.c                     |    8 ++-
 drivers/mmc/core/sdio.c                   |   54 +++++++++++++++---
 drivers/mmc/core/sdio_bus.c               |   85 ++++++++++++++++++++++++++++-
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   43 +++++++++++++--
 include/linux/mmc/host.h                  |    4 +-
 9 files changed, 232 insertions(+), 31 deletions(-)

[PATCH v2 01/11] mmc: sdio: fully reconfigure oldcard on resume

[Ohad Ben-Cohen]

On resume, let mmc_sdio_init_card go all the way, instead
of skipping the reconfiguration of the card's speed and width.

This is needed to ensure cards wake up with their clock
reconfigured (otherwise it's kept low).

This patch also removes the explicit bus width reconfiguration
on resume, since now this is part of mmc_sdio_init_card.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index f332c52..3be1571 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -456,7 +456,6 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 			return -ENOENT;
 
 		card = oldcard;
-		return 0;
 	}
 
 	if (card->type == MMC_TYPE_SD_COMBO) {
@@ -614,14 +613,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	mmc_claim_host(host);
 	err = mmc_sdio_init_card(host, host->ocr, host->card,
 				 (host->pm_flags & MMC_PM_KEEP_POWER));
-	if (!err) {
-		/* We may have switched to 1-bit mode during suspend. */
-		err = sdio_enable_4bit_bus(host->card);
-		if (err > 0) {
-			mmc_set_bus_width(host, MMC_BUS_WIDTH_4);
-			err = 0;
-		}
-	}
 	if (!err && host->sdio_irqs)
 		mmc_signal_sdio_irq(host);
 	mmc_release_host(host);
-- 
1.7.0.4

[PATCH v2 02/11] mmc: propagate power save/restore ops return value

[Ohad Ben-Cohen]

Allow power save/restore and their relevant mmc_bus_ops handlers
exit with a return value.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/core.c  |   20 ++++++++++++++------
 drivers/mmc/core/core.h  |    4 ++--
 drivers/mmc/core/mmc.c   |    8 ++++++--
 drivers/mmc/core/sd.c    |    8 ++++++--
 include/linux/mmc/host.h |    4 ++--
 5 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5def192..c94565d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1581,37 +1581,45 @@ void mmc_stop_host(struct mmc_host *host)
 	mmc_power_off(host);
 }
 
-void mmc_power_save_host(struct mmc_host *host)
+int mmc_power_save_host(struct mmc_host *host)
 {
+	int ret = 0;
+
 	mmc_bus_get(host);
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
 		mmc_bus_put(host);
-		return;
+		return -EINVAL;
 	}
 
 	if (host->bus_ops->power_save)
-		host->bus_ops->power_save(host);
+		ret = host->bus_ops->power_save(host);
 
 	mmc_bus_put(host);
 
 	mmc_power_off(host);
+
+	return ret;
 }
 EXPORT_SYMBOL(mmc_power_save_host);
 
-void mmc_power_restore_host(struct mmc_host *host)
+int mmc_power_restore_host(struct mmc_host *host)
 {
+	int ret;
+
 	mmc_bus_get(host);
 
 	if (!host->bus_ops || host->bus_dead || !host->bus_ops->power_restore) {
 		mmc_bus_put(host);
-		return;
+		return -EINVAL;
 	}
 
 	mmc_power_up(host);
-	host->bus_ops->power_restore(host);
+	ret = host->bus_ops->power_restore(host);
 
 	mmc_bus_put(host);
+
+	return ret;
 }
 EXPORT_SYMBOL(mmc_power_restore_host);
 
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 13240d1..90e0ac7 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -22,8 +22,8 @@ struct mmc_bus_ops {
 	void (*detect)(struct mmc_host *);
 	int (*suspend)(struct mmc_host *);
 	int (*resume)(struct mmc_host *);
-	void (*power_save)(struct mmc_host *);
-	void (*power_restore)(struct mmc_host *);
+	int (*power_save)(struct mmc_host *);
+	int (*power_restore)(struct mmc_host *);
 };
 
 void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 3ea58ce..31a8bbe 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -657,12 +657,16 @@ static int mmc_resume(struct mmc_host *host)
 	return err;
 }
 
-static void mmc_power_restore(struct mmc_host *host)
+static int mmc_power_restore(struct mmc_host *host)
 {
+	int ret;
+
 	host->card->state &= ~MMC_STATE_HIGHSPEED;
 	mmc_claim_host(host);
-	mmc_init_card(host, host->ocr, host->card);
+	ret = mmc_init_card(host, host->ocr, host->card);
 	mmc_release_host(host);
+
+	return ret;
 }
 
 static int mmc_sleep(struct mmc_host *host)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index bc745e1..49da4df 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -722,12 +722,16 @@ static int mmc_sd_resume(struct mmc_host *host)
 	return err;
 }
 
-static void mmc_sd_power_restore(struct mmc_host *host)
+static int mmc_sd_power_restore(struct mmc_host *host)
 {
+	int ret;
+
 	host->card->state &= ~MMC_STATE_HIGHSPEED;
 	mmc_claim_host(host);
-	mmc_sd_init_card(host, host->ocr, host->card);
+	ret = mmc_sd_init_card(host, host->ocr, host->card);
 	mmc_release_host(host);
+
+	return ret;
 }
 
 static const struct mmc_bus_ops mmc_sd_ops = {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c4fb1c5..c3ffad8 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -249,8 +249,8 @@ static inline void *mmc_priv(struct mmc_host *host)
 extern int mmc_suspend_host(struct mmc_host *);
 extern int mmc_resume_host(struct mmc_host *);
 
-extern void mmc_power_save_host(struct mmc_host *host);
-extern void mmc_power_restore_host(struct mmc_host *host);
+extern int mmc_power_save_host(struct mmc_host *host);
+extern int mmc_power_restore_host(struct mmc_host *host);
 
 extern void mmc_detect_change(struct mmc_host *, unsigned long delay);
 extern void mmc_request_done(struct mmc_host *, struct mmc_request *);
-- 
1.7.0.4

[PATCH v2 03/11] sdio: add power_restore support

[Ohad Ben-Cohen]

Add a power_restore handler to the SDIO bus ops,
in order to support waking up SDIO cards that
were powered off by runtime pm.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 3be1571..f17e0e0 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -638,11 +638,29 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	return err;
 }
 
+static int mmc_sdio_power_restore(struct mmc_host *host)
+{
+	int ret;
+
+	BUG_ON(!host);
+	BUG_ON(!host->card);
+
+	mmc_claim_host(host);
+	ret = mmc_sdio_init_card(host, host->ocr, host->card,
+			(host->pm_flags & MMC_PM_KEEP_POWER));
+	if (!ret && host->sdio_irqs)
+		mmc_signal_sdio_irq(host);
+	mmc_release_host(host);
+
+	return ret;
+}
+
 static const struct mmc_bus_ops mmc_sdio_ops = {
 	.remove = mmc_sdio_remove,
 	.detect = mmc_sdio_detect,
 	.suspend = mmc_sdio_suspend,
 	.resume = mmc_sdio_resume,
+	.power_restore = mmc_sdio_power_restore,
 };
 
 
-- 
1.7.0.4

[PATCH v2 04/11] mmc: add runtime PM handlers

[Ohad Ben-Cohen]

Add MMC runtime PM handlers, which call mmc_power_save_host
and mmc_power_restore_host in response to runtime_suspend and
runtime_resume events.

Runtime PM is still disabled by default, so this patch alone
has no immediate effect.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/bus.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index da3c01b..af8dc6a 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
@@ -141,6 +142,41 @@ static int mmc_bus_resume(struct device *dev)
 	return ret;
 }
 
+#ifdef CONFIG_PM_RUNTIME
+
+static int mmc_runtime_suspend(struct device *dev)
+{
+	struct mmc_card *card = mmc_dev_to_card(dev);
+
+	return mmc_power_save_host(card->host);
+}
+
+static int mmc_runtime_resume(struct device *dev)
+{
+	struct mmc_card *card = mmc_dev_to_card(dev);
+
+	return mmc_power_restore_host(card->host);
+}
+
+static int mmc_runtime_idle(struct device *dev)
+{
+	return pm_runtime_suspend(dev);
+}
+
+static const struct dev_pm_ops mmc_bus_pm_ops = {
+	.runtime_suspend	= mmc_runtime_suspend,
+	.runtime_resume		= mmc_runtime_resume,
+	.runtime_idle		= mmc_runtime_idle,
+};
+
+#define MMC_PM_OPS_PTR	(&mmc_bus_pm_ops)
+
+#else /* !CONFIG_PM_RUNTIME */
+
+#define MMC_PM_OPS_PTR	NULL
+
+#endif /* !CONFIG_PM_RUNTIME */
+
 static struct bus_type mmc_bus_type = {
 	.name		= "mmc",
 	.dev_attrs	= mmc_dev_attrs,
@@ -150,6 +186,7 @@ static struct bus_type mmc_bus_type = {
 	.remove		= mmc_bus_remove,
 	.suspend	= mmc_bus_suspend,
 	.resume		= mmc_bus_resume,
+	.pm		= MMC_PM_OPS_PTR,
 };
 
 int mmc_register_bus(void)
-- 
1.7.0.4

[PATCH v2 05/11] sdio: use the generic runtime PM handlers

[Ohad Ben-Cohen]

Assign the generic runtime PM handlers for SDIO.

These handlers invoke the relevant SDIO function drivers'
handlers, if exist, otherwise they just return success
(so SDIO drivers don't have to define any runtime PM handlers
unless they need to).

Runtime PM is still disabled by default, so this patch alone
has no immediate effect.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio_bus.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4a890dc..256a968 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/sdio_func.h>
@@ -154,6 +155,24 @@ static int sdio_bus_remove(struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_RUNTIME
+
+static const struct dev_pm_ops sdio_bus_pm_ops = {
+	SET_RUNTIME_PM_OPS(
+		pm_generic_runtime_suspend,
+		pm_generic_runtime_resume,
+		pm_generic_runtime_idle
+	)
+};
+
+#define SDIO_PM_OPS_PTR	(&sdio_bus_pm_ops)
+
+#else /* !CONFIG_PM_RUNTIME */
+
+#define SDIO_PM_OPS_PTR	NULL
+
+#endif /* !CONFIG_PM_RUNTIME */
+
 static struct bus_type sdio_bus_type = {
 	.name		= "sdio",
 	.dev_attrs	= sdio_dev_attrs,
@@ -161,6 +180,7 @@ static struct bus_type sdio_bus_type = {
 	.uevent		= sdio_bus_uevent,
 	.probe		= sdio_bus_probe,
 	.remove		= sdio_bus_remove,
+	.pm		= SDIO_PM_OPS_PTR,
 };
 
 int sdio_register_bus(void)
-- 
1.7.0.4

[PATCH v2 06/11] sdio: enable runtime PM for SDIO cards

[Ohad Ben-Cohen]

Enable runtime PM for new SDIO cards.

As soon as the card will be added to the device tree, runtime PM core
will release its power, since it doesn't have any users yet.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index f17e0e0..0dbd6fe 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/err.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
@@ -708,6 +709,18 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	card = host->card;
 
 	/*
+	 * Let runtime PM core know our card is active
+	 */
+	err = pm_runtime_set_active(&card->dev);
+	if (err)
+		goto remove;
+
+	/*
+	 * Enable runtime PM for this card
+	 */
+	pm_runtime_enable(&card->dev);
+
+	/*
 	 * The number of functions on the card is encoded inside
 	 * the ocr.
 	 */
-- 
1.7.0.4

[PATCH v2 07/11] sdio: enable runtime PM for SDIO functions

[Ohad Ben-Cohen]

Enable runtime PM for SDIO functions.

SDIO functions are initialized with a disabled runtime PM state,
and are set active (and their usage count is incremented)
only before potential drivers are probed.

SDIO function drivers that support runtime PM should call
pm_runtime_put_noidle() in their probe routine, and
pm_runtime_get_noresume() in their remove routine (very
similarly to PCI drivers).

In case a matching driver does not support runtime PM, power will
always be kept high (since the usage count is positive).

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio.c     |    5 +++++
 drivers/mmc/core/sdio_bus.c |   38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 0dbd6fe..85561dd 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -734,6 +734,11 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 		err = sdio_init_func(host->card, i + 1);
 		if (err)
 			goto remove;
+
+		/*
+		 * Enable Runtime PM for this func
+		 */
+		pm_runtime_enable(&card->sdio_func[i]->dev);
 	}
 
 	mmc_release_host(host);
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 256a968..3637483 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -126,21 +126,46 @@ static int sdio_bus_probe(struct device *dev)
 	if (!id)
 		return -ENODEV;
 
+	/* Unbound SDIO functions are always suspended.
+	 * During probe, the function is set active and the usage count
+	 * is incremented.  If the driver supports runtime PM,
+	 * it should call pm_runtime_put_noidle() in its probe routine and
+	 * pm_runtime_get_noresume() in its remove routine.
+	 */
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		goto out;
+
 	/* Set the default block size so the driver is sure it's something
 	 * sensible. */
 	sdio_claim_host(func);
 	ret = sdio_set_block_size(func, 0);
 	sdio_release_host(func);
 	if (ret)
-		return ret;
+		goto disable_runtimepm;
+
+	ret = drv->probe(func, id);
+	if (ret)
+		goto disable_runtimepm;
 
-	return drv->probe(func, id);
+	return 0;
+
+disable_runtimepm:
+	pm_runtime_put_noidle(dev);
+out:
+	return ret;
 }
 
 static int sdio_bus_remove(struct device *dev)
 {
 	struct sdio_driver *drv = to_sdio_driver(dev->driver);
 	struct sdio_func *func = dev_to_sdio_func(dev);
+	int ret;
+
+	/* Make sure card is powered before invoking ->remove() */
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		goto out;
 
 	drv->remove(func);
 
@@ -152,7 +177,14 @@ static int sdio_bus_remove(struct device *dev)
 		sdio_release_host(func);
 	}
 
-	return 0;
+	/* First, undo the increment made directly above */
+	pm_runtime_put_noidle(dev);
+
+	/* Then undo the runtime PM settings in sdio_bus_probe() */
+	pm_runtime_put_noidle(dev);
+
+out:
+	return ret;
 }
 
 #ifdef CONFIG_PM_RUNTIME
-- 
1.7.0.4

[PATCH v2 08/11] sdio: ensure mmc_sdio_detect is powered

[Ohad Ben-Cohen]

To prevent an erroneous removal of the card, make sure
the device is powered when it is mmc_sdio_detect()ed.

This is required since mmc_sdio_detect may be invoked
while the device is runtime suspended (e.g., MMC core
is rescanning when system comes out of suspend).

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 85561dd..c3ad105 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -546,6 +546,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
 	BUG_ON(!host);
 	BUG_ON(!host->card);
 
+	/* Make sure card is powered before detecting it */
+	err = pm_runtime_get_sync(&host->card->dev);
+	if (err < 0)
+		goto out;
+
 	mmc_claim_host(host);
 
 	/*
@@ -555,6 +560,7 @@ static void mmc_sdio_detect(struct mmc_host *host)
 
 	mmc_release_host(host);
 
+out:
 	if (err) {
 		mmc_sdio_remove(host);
 
@@ -562,6 +568,9 @@ static void mmc_sdio_detect(struct mmc_host *host)
 		mmc_detach_bus(host);
 		mmc_release_host(host);
 	}
+
+	/* Tell PM core that we're done */
+	pm_runtime_put(&host->card->dev);
 }
 
 /*
-- 
1.7.0.4

[PATCH v2 09/11] sdio: support suspend/resume while runtime suspended

[Ohad Ben-Cohen]

Bring SDIO devices back to full power before their suspend
handler is invoked.

Doing so ensures that SDIO suspend/resume semantics are
maintained (drivers still get to decide whether their
card should be removed or kept during system suspend,
and at what power state), and that SDIO suspend/resume
execution paths are unchanged.

This is achieved by resuming a runtime-suspended SDIO device
in its ->prepare() PM callback (similary to the PCI subsystem).

Since the PM core always increments the run-time usage
counter before calling the ->prepare() callback and decrements
it after calling the ->complete() callback, it is guaranteed
that when the system will come out of suspend, our device's
power state will reflect its runtime PM usage counter.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/mmc/core/sdio_bus.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 3637483..2716c7a 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -189,12 +189,41 @@ out:
 
 #ifdef CONFIG_PM_RUNTIME
 
+static int sdio_bus_pm_prepare(struct device *dev)
+{
+	/*
+	 * Resume an SDIO device which was suspended at run time at this
+	 * point, in order to allow standard SDIO suspend/resume paths
+	 * to keep working as usual.
+	 *
+	 * Ultimately, the SDIO driver itself will decide (in its
+	 * suspend handler, or lack thereof) whether the card should be
+	 * removed or kept, and if kept, at what power state.
+	 *
+	 * At this point, PM core have increased our use count, so it's
+	 * safe to directly resume the device. After system is resumed
+	 * again, PM core will drop back its runtime PM use count, and if
+	 * needed device will be suspended again.
+	 *
+	 * The end result is guaranteed to be a power state that is
+	 * coherent with the device's runtime PM use count.
+	 *
+	 * The return value of pm_runtime_resume is deliberately unchecked
+	 * since there is little point in failing system suspend if a
+	 * device can't be resumed.
+	 */
+	pm_runtime_resume(dev);
+
+	return 0;
+}
+
 static const struct dev_pm_ops sdio_bus_pm_ops = {
 	SET_RUNTIME_PM_OPS(
 		pm_generic_runtime_suspend,
 		pm_generic_runtime_resume,
 		pm_generic_runtime_idle
 	)
+	.prepare = sdio_bus_pm_prepare,
 };
 
 #define SDIO_PM_OPS_PTR	(&sdio_bus_pm_ops)
-- 
1.7.0.4

[PATCH v2 10/11] wl1271: sdio: enable runtime PM

[Ohad Ben-Cohen]

Enable runtime PM for the wl1271 SDIO device.

We request power whenever the WLAN interface is brought up,
and release it after the WLAN interface is taken down.

As a result, power is released immediately after probe returns,
since at that point power has not been explicitly requested yet
(i.e. the WLAN interface is still down).

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Acked-by: Luciano Coelho <luciano.coelho@nokia.com>
---
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 4c250d7..f7bef32 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -30,6 +30,7 @@
 #include <linux/mmc/card.h>
 #include <linux/gpio.h>
 #include <linux/wl12xx.h>
+#include <linux/pm_runtime.h>
 
 #include "wl1271.h"
 #include "wl12xx_80211.h"
@@ -160,12 +161,19 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 static int wl1271_sdio_power_on(struct wl1271 *wl)
 {
 	struct sdio_func *func = wl_to_func(wl);
+	int ret;
+
+	/* Power up the card */
+	ret = pm_runtime_get_sync(&func->dev);
+	if (ret < 0)
+		goto out;
 
 	sdio_claim_host(func);
 	sdio_enable_func(func);
 	sdio_release_host(func);
 
-	return 0;
+out:
+	return ret;
 }
 
 static int wl1271_sdio_power_off(struct wl1271 *wl)
@@ -176,15 +184,12 @@ static int wl1271_sdio_power_off(struct wl1271 *wl)
 	sdio_disable_func(func);
 	sdio_release_host(func);
 
-	return 0;
+	/* Power down the card */
+	return pm_runtime_put_sync(&func->dev);
 }
 
 static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
 {
-	/* Let the SDIO stack handle wlan_enable control, so we
-	 * keep host claimed while wlan is in use to keep wl1271
-	 * alive.
-	 */
 	if (enable)
 		return wl1271_sdio_power_on(wl);
 	else
@@ -256,6 +261,9 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 
 	sdio_set_drvdata(func, wl);
 
+	/* Tell PM core that we don't need the card to be powered now */
+	pm_runtime_put_noidle(&func->dev);
+
 	wl1271_notice("initialized");
 
 	return 0;
@@ -274,6 +282,9 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 {
 	struct wl1271 *wl = sdio_get_drvdata(func);
 
+	/* Undo decrement done above in wl1271_probe */
+	pm_runtime_get_noresume(&func->dev);
+
 	wl1271_unregister_hw(wl);
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
-- 
1.7.0.4

[PATCH v2 11/11] wl1271: sdio: add suspend/resume support

[Ohad Ben-Cohen]

Add required suspend/resume support to prevent the SDIO
core from removing our card completely during system suspend.

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index f7bef32..784ef34 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -290,11 +290,31 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	wl1271_free_hw(wl);
 }
 
+static int wl1271_suspend(struct device *dev)
+{
+	/* Tell MMC/SDIO core it's OK to power down the card
+	 * (if it isn't already), but not to remove it completely */
+	return 0;
+}
+
+static int wl1271_resume(struct device *dev)
+{
+	return 0;
+}
+
+static const struct dev_pm_ops wl1271_sdio_pm_ops = {
+	.suspend	= wl1271_suspend,
+	.resume		= wl1271_resume,
+};
+
 static struct sdio_driver wl1271_sdio_driver = {
 	.name		= "wl1271_sdio",
 	.id_table	= wl1271_devices,
 	.probe		= wl1271_probe,
 	.remove		= __devexit_p(wl1271_remove),
+	.drv = {
+		.pm = &wl1271_sdio_pm_ops,
+	},
 };
 
 static int __init wl1271_init(void)
-- 
1.7.0.4

Re: [PATCH v2 00/11] SDIO Runtime PM Support

[Luciano Coelho]

On Sat, 2010-10-02 at 13:54 +0200, ext Ohad Ben-Cohen wrote:
> Introduce SDIO runtime PM support:
> 
> 1. Power to SDIO cards is kept low until one of its functions is bound
>    (i.e. a matching driver is successfully probed)
> 
> 2. If the matching driver supports runtime PM, power to the card is
>    dropped soon after probe() returns. It is then up to the driver
>    to request power to its function, using runtime PM API (the get/put
>    variants). This is demonstrated with the wl1271 driver, in which
>    the power of the card power is coupled with the state of the wlan
>    interface (interface up -> power is up, interface down -> power is down)
> 
> 3. If a matching driver does not support runtime PM, power to the card
>    is kept high during the whole lifetime of the driver
> 
> 4. When the driver is removed, power to the card is immediately dropped
> 
> 5. If there are multiple drivers for the same card (several SDIO functions),
>    power will be pulled high before the first driver probes, and dropped
>    down after the last driver is removed. In between, power will be
>    maintained accrording to the accumulated usage count of the complete
>    drivers group
> 
> 6. SDIO suspend/resume semantics are unchanged. In addition, when the system
>    comes out of suspend, it is guaranteed that the power state of the
>    SDIO card will reflect its runtime PM usage count.
> 
> 7. What was NOT changed:
>    - Interface: drivers can still assume that the card is powered
>      when probe/remove/suspend/resume are called
>    - Existing behavior: drivers that do not support runtime PM
>      are unchanged
> 
> Changes since v1:
> - Interaction with system suspend/resume
> - Better commentary
> 
> Dependencies:
> - SDIO patches are against mmc-next, and have a runtime dependency on
>   commit "PM / Runtime: Lenient generic runtime pm callbacks"
>   (patch is in linux-next now)
> - WLAN patches depend on recent wl1271 activity, and they are here
>   just to demonstrate the usage of the SDIO patchset (will be
>   resubmitted separately)
> 
> The full patchset, together with all its dependencies, is also available at:
> git://wizery.com/pub/mmc.git runtime-pm-v2

I have tested this on my beagle board with the wl1271 patches that Ohad
provided and it seems to be working fine.  I've also created a small
script to stress test the implementation a little bit and didn't see any
problems.

Tested-by: Luciano Coelho <luciano.coelho@nokia.com>

This is quite an important fix in the wl1271 point-of-view.  Chris, do
you think there is any chance that this could make it to 2.6.37?


-- 
Cheers,
Luca.

Re: [PATCH v2 00/11] SDIO Runtime PM Support

[Chris Ball]

Hi Luciano, Ohad,

On Thu, Oct 07, 2010 at 03:32:16PM +0300, Luciano Coelho wrote:
> I have tested this on my beagle board with the wl1271 patches that Ohad
> provided and it seems to be working fine.  I've also created a small
> script to stress test the implementation a little bit and didn't see any
> problems.
> 
> Tested-by: Luciano Coelho <luciano.coelho@nokia.com>
> 
> This is quite an important fix in the wl1271 point-of-view.  Chris, do
> you think there is any chance that this could make it to 2.6.37?

Yes, can do -- the patchset looks good, and I've just pushed patches
1-9 (the MMC patches) to mmc-next.  Please go ahead and submit the
two wl1271 patches through wireless-testing whenever you're ready.

Thanks!

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2 00/11] SDIO Runtime PM Support

[Ohad Ben-Cohen]

On Thu, Oct 7, 2010 at 8:01 PM, Chris Ball <cjb@laptop.org> wrote:
> Yes, can do -- the patchset looks good, and I've just pushed patches
> 1-9 (the MMC patches) to mmc-next.

Thanks, Chris.

>  Please go ahead and submit the
> two wl1271 patches through wireless-testing whenever you're ready.
>
> Thanks!
>
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
>

Re: [PATCH v2 00/11] SDIO Runtime PM Support

[Luciano Coelho]

On Thu, 2010-10-07 at 20:01 +0200, ext Chris Ball wrote:
> Hi Luciano, Ohad,
> 
> On Thu, Oct 07, 2010 at 03:32:16PM +0300, Luciano Coelho wrote:
> > I have tested this on my beagle board with the wl1271 patches that Ohad
> > provided and it seems to be working fine.  I've also created a small
> > script to stress test the implementation a little bit and didn't see any
> > problems.
> > 
> > Tested-by: Luciano Coelho <luciano.coelho@nokia.com>
> > 
> > This is quite an important fix in the wl1271 point-of-view.  Chris, do
> > you think there is any chance that this could make it to 2.6.37?
> 
> Yes, can do -- the patchset looks good, and I've just pushed patches
> 1-9 (the MMC patches) to mmc-next.  Please go ahead and submit the
> two wl1271 patches through wireless-testing whenever you're ready.

Thanks a lot, Chris! This will easy our development work quite much,
since we don't have to be applying separate patches to be able to enable
and disable the chip at runtime.

I'll apply the wl1271 patches via the wl12xx tree.

-- 
Cheers,
Luca.