[RFC 0/6] adding SDIO Runtime PM support

[RFC 0/6] adding SDIO Runtime PM support

[Ohad Ben-Cohen]

SDIO function drivers should be able to power down/up
their cards (e.g. a WLAN SDIO driver might want to
power down its card whenever wlan0 is down).

In the past few weeks we've been discussing how this
could be achieved, and I'd like to present a new approach,
which is based on runtime pm API.

The following few patches add runtime pm support for SDIO
devices, and demonstrate it with the wl1271_sdio driver.
Basic functionality is provided: The power state of the
device is now coupled with the state of the wlan0 interface.

The nice thing about this approach is how small it is,
as the runtime PM core is already taking care of everything
needed (usage counts, device hierarchy, established set of API,
synchronization, ...).

Having said that, I'd like to stress that these patches
are early submitted for RFC purposes only, as they
are still in a preliminary state and only lightly tested.

Please note that I will soon be leaving for the rest of August and
am going to have a very limited email access, so it might
take me some time to respond.

Appreciate your feedback,

Thank you,

Ohad Ben-Cohen (6):
  mmc: sdio: fully reconfigure oldcard on resume
  sdio: add power_restore support
  mmc: add general runtime PM support
  sdio: add general runtime PM support
  sdio: enable Runtime PM for SDIO cards
  wireless: wl1271_sdio: enable Runtime PM

 drivers/mmc/core/bus.c                    |   35 +++++++++++++++++++++++++++
 drivers/mmc/core/sdio.c                   |   24 +++++++++++++++---
 drivers/mmc/core/sdio_bus.c               |   10 ++++++++
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   37 ++++++++++++++++++++++++++++-
 4 files changed, 101 insertions(+), 5 deletions(-)

[RFC/PATCH 1/6] 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 |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index b9dee28..645f173 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -344,7 +344,6 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 			goto err;
 		}
 		card = oldcard;
-		return 0;
 	}
 
 	/*
@@ -487,9 +486,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_wide(host->card);
 	if (!err && host->sdio_irqs)
 		mmc_signal_sdio_irq(host);
 	mmc_release_host(host);
-- 
1.7.0.4

[RFC/PATCH 2/6] 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.

Note: this approach would also require changing
the power_restore/save bus handlers to support
a return value.

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

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 645f173..ddbb9d3 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -511,11 +511,27 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	return err;
 }
 
+static void mmc_sdio_power_restore(struct mmc_host *host)
+{
+	int err;
+
+	BUG_ON(!host);
+	BUG_ON(!host->card);
+
+	mmc_claim_host(host);
+	err = mmc_sdio_init_card(host, host->ocr, host->card,
+			(host->pm_flags & MMC_PM_KEEP_POWER));
+	if (!err && host->sdio_irqs)
+		mmc_signal_sdio_irq(host);
+	mmc_release_host(host);
+}
+
 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

[RFC/PATCH 3/6] mmc: add general runtime PM support

[Ohad Ben-Cohen]

Add Runtime PM handlers to mmc, which calls mmc_power_save_host
and mmc_power_restore_host in respond 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 |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 49d9dca..27902b4 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>
@@ -137,6 +138,39 @@ static int mmc_bus_resume(struct device *dev)
 	return ret;
 }
 
+static int mmc_runtime_suspend(struct device *dev)
+{
+	int status = 0;
+	struct mmc_card *card = dev_to_mmc_card(dev);
+
+	mmc_power_save_host(card->host);
+
+	return status;
+}
+
+static int mmc_runtime_resume(struct device *dev)
+{
+	int status = 0;
+	struct mmc_card *card = dev_to_mmc_card(dev);
+
+	mmc_power_restore_host(card->host);
+
+	return status;
+}
+
+static int mmc_runtime_idle(struct device *dev)
+{
+	struct mmc_card *card = dev_to_mmc_card(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,
+};
+
 static struct bus_type mmc_bus_type = {
 	.name		= "mmc",
 	.dev_attrs	= mmc_dev_attrs,
@@ -146,6 +180,7 @@ static struct bus_type mmc_bus_type = {
 	.remove		= mmc_bus_remove,
 	.suspend	= mmc_bus_suspend,
 	.resume		= mmc_bus_resume,
+	.pm		= &mmc_bus_pm_ops,
 };
 
 int mmc_register_bus(void)
-- 
1.7.0.4

RE: [RFC/PATCH 3/6] mmc: add general runtime PM support

[Gao, Yunpeng]

>>+static int mmc_runtime_suspend(struct device *dev)
>+{
>+	int status = 0;
>+	struct mmc_card *card = dev_to_mmc_card(dev);
>+
>+	mmc_power_save_host(card->host);
>+
>+	return status;
>+}

It seems the power_save callback is not implemented (Null pointer) yet for both mmc and sdio. Does it mean that the power_save callback will be implemented in future or just don't need at all? Thanks.

>+static int mmc_runtime_resume(struct device *dev)
>+{
>+	int status = 0;
>+	struct mmc_card *card = dev_to_mmc_card(dev);
>+
>+	mmc_power_restore_host(card->host);
>+
>+	return status;
>+}
>+
>+static int mmc_runtime_idle(struct device *dev)
>+{
>+	struct mmc_card *card = dev_to_mmc_card(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,
>+};
>+
> static struct bus_type mmc_bus_type = {
> 	.name		= "mmc",
> 	.dev_attrs	= mmc_dev_attrs,
>@@ -146,6 +180,7 @@ static struct bus_type mmc_bus_type = {
> 	.remove		= mmc_bus_remove,
> 	.suspend	= mmc_bus_suspend,
> 	.resume		= mmc_bus_resume,
>+	.pm		= &mmc_bus_pm_ops,
> };
>
I think this is a good example to implement runtime PM support for mmc driver. But seems it doesn't provide a runtime PM example on host controller driver side. To my understanding, once the functions (mmc card or sdio devices) and mmc/sdio bus enter into rumtime suspend, it should then make the host controller enter into runtime suspend, too. Any ideas on how to achieve this? Thanks.

Best Regards,
Yunpeng Gao

Re: [RFC/PATCH 3/6] mmc: add general runtime PM support

[Ohad Ben-Cohen]

Hi Yunpeng,

Sorry for the late reply (vacation..).

On Thu, Aug 26, 2010 at 12:42 PM, Gao, Yunpeng <yunpeng.gao@intel.com> wrote:
> It seems the power_save callback is not implemented (Null pointer) yet for both mmc and sdio. Does it mean that the power_save callback will be implemented in future or just don't need at all? Thanks.

Yes, currently seems it is not needed.

> But seems it doesn't provide a runtime PM example on host controller driver side. To my understanding, once the functions (mmc card or sdio devices) and mmc/sdio bus enter into rumtime suspend, it should then make the host controller enter into runtime suspend, too. Any ideas on how to achieve this? Thanks.

This should already be taken care of by the PM core:

When a device is suspended (e.g. our sdio/mmc card), an idle request
is sent by the PM core to its parent device (in our case, that would
be the host controller).
Now, if the host controller driver has enabled runtime PM, its idle
handler would be called so it could act upon it.
Similarly, when the card is resumed, the PM core will first invoke its
host controller parent's resume handler.

For an (initial) reference example of enabling runtime PM in a host
controller driver (omap_hsmmc), you can take a look at Kevin Hilman's
pm-wip/mmc branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

Btw I have an updated version of this patchset - I am about to start
testing it (will make sure your scenario works too) and then soon
after post it here for review.

Thanks,
Ohad.

>
> Best Regards,
> Yunpeng Gao

[RFC/PATCH 4/6] sdio: add general runtime PM support

[Ohad Ben-Cohen]

Add generic Runtime PM handlers to sdio.

When a runtime pm event kicks in, those handlers call the
runtime pm handlers of the relevant SDIO function drivers.

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 |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4a890dc..2e417f9 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,14 @@ static int sdio_bus_remove(struct device *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
+	)
+};
+
 static struct bus_type sdio_bus_type = {
 	.name		= "sdio",
 	.dev_attrs	= sdio_dev_attrs,
@@ -161,6 +170,7 @@ static struct bus_type sdio_bus_type = {
 	.uevent		= sdio_bus_uevent,
 	.probe		= sdio_bus_probe,
 	.remove		= sdio_bus_remove,
+	.pm		= &sdio_bus_pm_ops,
 };
 
 int sdio_register_bus(void)
-- 
1.7.0.4

[RFC/PATCH 5/6] sdio: enable Runtime PM for SDIO cards

[Ohad Ben-Cohen]

Before adding SDIO functions, indicate to the runtime pm core
the card is active, and enable runtime pm.

The power will be released automatically if the card
will be idle.

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

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index ddbb9d3..498fdfb 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>
@@ -610,6 +611,9 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	if (err)
 		goto remove_added;
 
+	pm_runtime_set_active(&host->card->dev);
+	pm_runtime_enable(&host->card->dev);
+
 	/*
 	 * ...then the SDIO functions.
 	 */
-- 
1.7.0.4

[RFC/PATCH 6/6] wireless: 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>
---
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   37 ++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index bfb18b6..2dede64 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -33,6 +33,7 @@
 #include <linux/gpio.h>
 #include <linux/platform_device.h>
 #include <linux/completion.h>
+#include <linux/pm_runtime.h>
 
 #include "wl1271.h"
 #include "wl12xx_80211.h"
@@ -156,20 +157,23 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,
 static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
 {
 	struct sdio_func *func = wl_to_func(wl);
+	int err;
 
 	/* Let the SDIO stack handle wlan_enable control, so we
 	 * keep host claimed while wlan is in use to keep wl1271
 	 * alive.
 	 */
 	if (enable) {
+		err = pm_runtime_get_sync(&func->dev);
 		sdio_claim_host(func);
 		sdio_enable_func(func);
 	} else {
 		sdio_disable_func(func);
 		sdio_release_host(func);
+		err = pm_runtime_put_sync(&func->dev);
 	}
 
-	return 0;
+	return err;
 }
 
 static struct wl1271_if_operations sdio_ops = {
@@ -318,10 +322,23 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 
 	sdio_set_drvdata(func, wl);
 
+	/* indicate to Runtime PM core that our device is active */
+	ret = pm_runtime_set_active(&func->dev);
+	if (ret)
+		goto unreg_hw;
+
+	/* enable Runtime PM.
+	 * When probe will return, runtime pm will immediately release power
+	 * for us since we call pm_runtime_get() only when the user brings up
+	 * the WLAN interface */
+	pm_runtime_enable(&func->dev);
+
 	wl1271_notice("initialized");
 
 	return 0;
 
+unreg_hw:
+	wl1271_unregister_hw(wl);
 out_irq:
 	free_irq(wl->irq, wl);
 put_data:
@@ -346,11 +363,29 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	wl1271_free_hw(wl);
 }
 
+static int wl1271_sdio_runtime_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int wl1271_sdio_runtime_resume(struct device *dev)
+{
+	return 0;
+}
+
+static const struct dev_pm_ops wl1271_sdio_pm_ops = {
+	.runtime_suspend = wl1271_sdio_runtime_suspend,
+	.runtime_resume = wl1271_sdio_runtime_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