[PATCH v1 0/8] SDIO Runtime PM Support

[PATCH v1 0/8] SDIO Runtime PM Support

[Ohad Ben-Cohen]

This patchset introduces runtime PM support to SDIO cards.

What have been achieved:

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). Last patch in this series demonstrates this with the
   wl1271 driver which couples the card power with the state of the wlan
   interface (interface up -> power is up, interface down -> power is down)

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

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

6. 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

Patches were tested on ZOOM3, 2.6.36-rc3.

Dependencies (in case you want to test this):
- The SDIO/MMC part depends on http://www.spinics.net/lists/linux-pm/msg21663.html (patch was already accepted by PM maintainer)
- The WLAN part depends on http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34148.html

Maturity level is much better than the previous RFC patches, and also testing that have been conducted are more intense. We still plan to stress this even further, but it's certainly ready for wide review.

Next thing I plan on doing is testing this with SDIO suspend/resume (with and without MMC_PM_KEEP_POWER). I'll update as soon as I have results (setup issues and a business trip will keep me away for several days).

Thanks,

Ohad Ben-Cohen (8):
  mmc: sdio: fully reconfigure oldcard on resume
  mmc: let power save/restore ops return a value
  sdio: add power_restore support
  mmc: add general Runtime PM support
  sdio: add general runtime PM support
  sdio: enable Runtime PM for SDIO cards
  sdio: enable Runtime PM for SDIO functions
  wireless: wl1271_sdio: enable Runtime PM

 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                   |   42 +++++++++++++++++++-----
 drivers/mmc/core/sdio_bus.c               |   50 +++++++++++++++++++++++++++-
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   14 +++++++-
 include/linux/mmc/host.h                  |    4 +-
 9 files changed, 161 insertions(+), 26 deletions(-)

[PATCH v1 0/8] SDIO Runtime PM Support

[Ohad Ben-Cohen]

This patchset introduces runtime PM support to SDIO cards.

What have been achieved:

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). Last patch in this series demonstrates this with the
   wl1271 driver which couples the card power with the state of the wlan
   interface (interface up -> power is up, interface down -> power is down)

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

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

6. 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

Patches were tested on ZOOM3, 2.6.36-rc3.

Dependencies (in case you want to test this):
- The SDIO/MMC part depends on http://www.spinics.net/lists/linux-pm/msg21663.html (patch was already accepted by PM maintainer)
- The WLAN part depends on http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34148.html

Maturity level is much better than the previous RFC patches, and also testing that have been conducted are more intense. We still plan to stress this even further, but it's certainly ready for wide review.

Next thing I plan on doing is testing this with SDIO suspend/resume (with and without MMC_PM_KEEP_POWER). I'll update as soon as I have results (setup issues and a business trip will keep me away for several days).

Thanks,

Ohad Ben-Cohen (8):
  mmc: sdio: fully reconfigure oldcard on resume
  mmc: let power save/restore ops return a value
  sdio: add power_restore support
  mmc: add general Runtime PM support
  sdio: add general runtime PM support
  sdio: enable Runtime PM for SDIO cards
  sdio: enable Runtime PM for SDIO functions
  wireless: wl1271_sdio: enable Runtime PM

 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                   |   42 +++++++++++++++++++-----
 drivers/mmc/core/sdio_bus.c               |   50 +++++++++++++++++++++++++++-
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   14 +++++++-
 include/linux/mmc/host.h                  |    4 +-
 9 files changed, 161 insertions(+), 26 deletions(-)

[PATCH v1 4/8] mmc: add general Runtime PM support

[Ohad Ben-Cohen]

Add runtime PM handlers to mmc, 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 7cd9749..a302e57 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>
@@ -142,6 +143,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 = dev_to_mmc_card(dev);
+
+	return mmc_power_save_host(card->host);
+}
+
+static int mmc_runtime_resume(struct device *dev)
+{
+	struct mmc_card *card = dev_to_mmc_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,
@@ -151,6 +187,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 v1 4/8] mmc: add general Runtime PM support

[Ohad Ben-Cohen]

Add runtime PM handlers to mmc, 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 7cd9749..a302e57 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>
@@ -142,6 +143,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 = dev_to_mmc_card(dev);
+
+	return mmc_power_save_host(card->host);
+}
+
+static int mmc_runtime_resume(struct device *dev)
+{
+	struct mmc_card *card = dev_to_mmc_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,
@@ -151,6 +187,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 v1 5/8] sdio: add general runtime PM support

[Ohad Ben-Cohen]

Add generic runtime PM handlers to sdio.

Those handlers invoke the relevant SDIO function drivers' handlers, if
such exist. If not they just return success.

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 v1 5/8] sdio: add general runtime PM support

[Ohad Ben-Cohen]

Add generic runtime PM handlers to sdio.

Those handlers invoke the relevant SDIO function drivers' handlers, if
such exist. If not they just return success.

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 v1 7/8] 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 status,
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 on (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 |   30 ++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index a5a76a8..c4022da 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -732,6 +732,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..d55a592 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -126,15 +126,34 @@ 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)
+		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 0;
 
-	return drv->probe(func, id);
+disable_runtimepm:
+	pm_runtime_put_noidle(dev);
+out:
+	return ret;
 }
 
 static int sdio_bus_remove(struct device *dev)
@@ -142,6 +161,8 @@ 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);
 
+	pm_runtime_get_sync(dev);
+
 	drv->remove(func);
 
 	if (func->irq_handler) {
@@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
 		sdio_release_host(func);
 	}
 
+	pm_runtime_put_noidle(dev);
+
+	/* Undo the runtime PM settings in sdio_bus_probe() */
+	pm_runtime_put_noidle(dev);
+
 	return 0;
 }
 
-- 
1.7.0.4

[PATCH v1 7/8] 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 status,
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 on (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 |   30 ++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index a5a76a8..c4022da 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -732,6 +732,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..d55a592 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -126,15 +126,34 @@ 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)
+		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 0;
 
-	return drv->probe(func, id);
+disable_runtimepm:
+	pm_runtime_put_noidle(dev);
+out:
+	return ret;
 }
 
 static int sdio_bus_remove(struct device *dev)
@@ -142,6 +161,8 @@ 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);
 
+	pm_runtime_get_sync(dev);
+
 	drv->remove(func);
 
 	if (func->irq_handler) {
@@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
 		sdio_release_host(func);
 	}
 
+	pm_runtime_put_noidle(dev);
+
+	/* Undo the runtime PM settings in sdio_bus_probe() */
+	pm_runtime_put_noidle(dev);
+
 	return 0;
 }
 
-- 
1.7.0.4

Re: [PATCH v1 7/8] sdio: enable Runtime PM for SDIO functions

[Chris Ball]

Hi Ohad,

On Tue, Sep 07, 2010 at 02:29:08PM +0300, Ohad Ben-Cohen wrote:
> @@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
>  		sdio_release_host(func);
>  	}
>  
> +	pm_runtime_put_noidle(dev);
> +
> +	/* Undo the runtime PM settings in sdio_bus_probe() */
> +	pm_runtime_put_noidle(dev);
> +
>  	return 0;
>  }
>  

Nit:  This makes sense in context, but stands out as a possible paste
accident on first sight.  :)  I think it's worth adding a comment to
the first call; something like: 

> +     /* 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);
> +     

Thanks,

-- 
Chris Ball   <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>   <http://printf.net/>
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 7/8] sdio: enable Runtime PM for SDIO functions

[Chris Ball]

Hi Ohad,

On Tue, Sep 07, 2010 at 02:29:08PM +0300, Ohad Ben-Cohen wrote:
> @@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
>  		sdio_release_host(func);
>  	}
>  
> +	pm_runtime_put_noidle(dev);
> +
> +	/* Undo the runtime PM settings in sdio_bus_probe() */
> +	pm_runtime_put_noidle(dev);
> +
>  	return 0;
>  }
>  

Nit:  This makes sense in context, but stands out as a possible paste
accident on first sight.  :)  I think it's worth adding a comment to
the first call; something like: 

> +     /* 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);
> +     

Thanks,

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

Re: [PATCH v1 7/8] sdio: enable Runtime PM for SDIO functions

[Ohad Ben-Cohen]

Hi Chris,

On Tue, Sep 14, 2010 at 9:27 PM, Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Sep 07, 2010 at 02:29:08PM +0300, Ohad Ben-Cohen wrote:
> > @@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
> >               sdio_release_host(func);
> >       }
> >
> > +     pm_runtime_put_noidle(dev);
> > +
> > +     /* Undo the runtime PM settings in sdio_bus_probe() */
> > +     pm_runtime_put_noidle(dev);
> > +
> >       return 0;
> >  }
> >
>
> Nit:  This makes sense in context, but stands out as a possible paste
> accident on first sight.  :)  I think it's worth adding a comment to
> the first call

Will do, thanks for the review!

Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 7/8] sdio: enable Runtime PM for SDIO functions

[Ohad Ben-Cohen]

Hi Chris,

On Tue, Sep 14, 2010 at 9:27 PM, Chris Ball <cjb@laptop.org> wrote:
> On Tue, Sep 07, 2010 at 02:29:08PM +0300, Ohad Ben-Cohen wrote:
> > @@ -152,6 +173,11 @@ static int sdio_bus_remove(struct device *dev)
> >               sdio_release_host(func);
> >       }
> >
> > +     pm_runtime_put_noidle(dev);
> > +
> > +     /* Undo the runtime PM settings in sdio_bus_probe() */
> > +     pm_runtime_put_noidle(dev);
> > +
> >       return 0;
> >  }
> >
>
> Nit:  This makes sense in context, but stands out as a possible paste
> accident on first sight.  :)  I think it's worth adding a comment to
> the first call

Will do, thanks for the review!

Ohad.

[PATCH v1 1/8] 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
---
 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 bd2755e..8a34945 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -457,7 +457,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) {
@@ -615,14 +614,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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/8] 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 bd2755e..8a34945 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -457,7 +457,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) {
@@ -615,14 +614,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 v1 2/8] 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
---
 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 5db49b1..73575c4 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1538,37 +1538,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 9d9eef5..fff58fa 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 6909a54..1f69f5a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -623,12 +623,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 0f52410..32c58b4 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 1575b52..0935841 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -236,8 +236,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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/8] 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 5db49b1..73575c4 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1538,37 +1538,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 9d9eef5..fff58fa 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 6909a54..1f69f5a 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -623,12 +623,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 0f52410..32c58b4 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 1575b52..0935841 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -236,8 +236,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 v1 3/8] 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
---
 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 8a34945..c118cc2 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -639,11 +639,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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 3/8] 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 8a34945..c118cc2 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -639,11 +639,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 v1 6/8] 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
---
 drivers/mmc/core/sdio.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index c118cc2..a5a76a8 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>
@@ -709,6 +710,15 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	card = host->card;
 
 	/*
+	 * Enable Runtime PM for this card
+	 */
+	err = pm_runtime_set_active(&card->dev);
+	if (err)
+		goto remove;
+
+	pm_runtime_enable(&card->dev);
+
+	/*
 	 * The number of functions on the card is encoded inside
 	 * the ocr.
 	 */
-- 
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index c118cc2..a5a76a8 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>
@@ -709,6 +710,15 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	card = host->card;
 
 	/*
+	 * Enable Runtime PM for this card
+	 */
+	err = pm_runtime_set_active(&card->dev);
+	if (err)
+		goto remove;
+
+	pm_runtime_enable(&card->dev);
+
+	/*
 	 * The number of functions on the card is encoded inside
 	 * the ocr.
 	 */
-- 
1.7.0.4

[PATCH v1 8/8] 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
---
 drivers/net/wireless/wl12xx/wl1271_sdio.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 824b9e8..235191e 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"
@@ -153,20 +154,27 @@ 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);
+		if (err)
+			goto out;
+
 		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;
+out:
+	return err;
 }
 
 static struct wl1271_if_operations sdio_ops = {
@@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 
 	sdio_set_drvdata(func, wl);
 
+	pm_runtime_put_noidle(&func->dev);
+
 	wl1271_notice("initialized");
 
 	return 0;
@@ -255,6 +265,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 
 	wl1271_unregister_hw(wl);
 	wl1271_free_hw(wl);
+
+	pm_runtime_get_noresume(&func->dev);
 }
 
 static struct sdio_driver wl1271_sdio_driver = {
-- 
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 8/8] 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 |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 824b9e8..235191e 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"
@@ -153,20 +154,27 @@ 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);
+		if (err)
+			goto out;
+
 		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;
+out:
+	return err;
 }
 
 static struct wl1271_if_operations sdio_ops = {
@@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 
 	sdio_set_drvdata(func, wl);
 
+	pm_runtime_put_noidle(&func->dev);
+
 	wl1271_notice("initialized");
 
 	return 0;
@@ -255,6 +265,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 
 	wl1271_unregister_hw(wl);
 	wl1271_free_hw(wl);
+
+	pm_runtime_get_noresume(&func->dev);
 }
 
 static struct sdio_driver wl1271_sdio_driver = {
-- 
1.7.0.4

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[David Vrabel]

Ohad Ben-Cohen wrote:
> 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).
[...]
> @@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>  
>  	sdio_set_drvdata(func, wl);
>  
> +	pm_runtime_put_noidle(&func->dev);

I find this a little odd and confusing as this is releasing the
reference taken by mmc core from inside the function driver.

Instead, I would suggest changing all existing SDIO function drivers to
call pm_runtime_get() in probe() and pm_runtime_put() in remove().  I
think this makes the runtime PM usage more obvious from a function
driver point of view.

> +
>  	wl1271_notice("initialized");
>  
>  	return 0;
> @@ -255,6 +265,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
>  
>  	wl1271_unregister_hw(wl);
>  	wl1271_free_hw(wl);
> +
> +	pm_runtime_get_noresume(&func->dev);
>  }
>  
>  static struct sdio_driver wl1271_sdio_driver = {

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[David Vrabel]

Ohad Ben-Cohen wrote:
> 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).
[...]
> @@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>  
>  	sdio_set_drvdata(func, wl);
>  
> +	pm_runtime_put_noidle(&func->dev);

I find this a little odd and confusing as this is releasing the
reference taken by mmc core from inside the function driver.

Instead, I would suggest changing all existing SDIO function drivers to
call pm_runtime_get() in probe() and pm_runtime_put() in remove().  I
think this makes the runtime PM usage more obvious from a function
driver point of view.

> +
>  	wl1271_notice("initialized");
>  
>  	return 0;
> @@ -255,6 +265,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
>  
>  	wl1271_unregister_hw(wl);
>  	wl1271_free_hw(wl);
> +
> +	pm_runtime_get_noresume(&func->dev);
>  }
>  
>  static struct sdio_driver wl1271_sdio_driver = {

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[Ohad Ben-Cohen]

On Tue, Sep 7, 2010 at 4:02 PM, David Vrabel <david.vrabel@csr.com> wrote:
> Ohad Ben-Cohen wrote:
>> 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).
> [...]
>> @@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>>
>>       sdio_set_drvdata(func, wl);
>>
>> +     pm_runtime_put_noidle(&func->dev);
>
> I find this a little odd and confusing as this is releasing the
> reference taken by mmc core from inside the function driver.

This is done in order to still support function drivers that are not
aware of runtime pm (same approach was taken in the PCI implementation
of runtime pm).

> Instead, I would suggest changing all existing SDIO function drivers to
> call pm_runtime_get() in probe() and pm_runtime_put() in remove().

If we are willing to require runtime pm awareness from all SDIO
function drivers then we can do this (obviously we will break all
out-of-tree drivers by doing so).

> think this makes the runtime PM usage more obvious from a function
> driver point of view.
>
>> +
>>       wl1271_notice("initialized");
>>
>>       return 0;
>> @@ -255,6 +265,8 @@ static void __devexit wl1271_remove(struct sdio_func *func)
>>
>>       wl1271_unregister_hw(wl);
>>       wl1271_free_hw(wl);
>> +
>> +     pm_runtime_get_noresume(&func->dev);
>>  }
>>
>>  static struct sdio_driver wl1271_sdio_driver = {
>
> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
>

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[David Vrabel]

On 07/09/2010 14:15, Ohad Ben-Cohen wrote:
> On Tue, Sep 7, 2010 at 4:02 PM, David Vrabel <david.vrabel@csr.com> wrote:
>> Ohad Ben-Cohen wrote:
>>> 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).
>> [...]
>>> @@ -233,6 +241,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
>>>
>>>       sdio_set_drvdata(func, wl);
>>>
>>> +     pm_runtime_put_noidle(&func->dev);
>>
>> I find this a little odd and confusing as this is releasing the
>> reference taken by mmc core from inside the function driver.
> 
> This is done in order to still support function drivers that are not
> aware of runtime pm (same approach was taken in the PCI implementation
> of runtime pm).

Best to do the same as PCI then.

David

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[Luciano Coelho]

On Tue, 2010-09-07 at 13:29 +0200, ext Ohad Ben-Cohen wrote:
> 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-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
> ---

Looks fine to me, after considering David's comment and since it's done
in the same way for PCI.

Acked-by: Luciano Coelho <luciano.coelho-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>


-- 
Cheers,
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 8/8] wireless: wl1271_sdio: enable Runtime PM

[Luciano Coelho]

On Tue, 2010-09-07 at 13:29 +0200, ext Ohad Ben-Cohen wrote:
> 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>
> ---

Looks fine to me, after considering David's comment and since it's done
in the same way for PCI.

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


-- 
Cheers,
Luca.

Re: [PATCH v1 0/8] SDIO Runtime PM Support

[Luciano Coelho]

On Tue, 2010-09-07 at 13:29 +0200, ext Ohad Ben-Cohen wrote:
> This patchset introduces runtime PM support to SDIO cards.

I guess this whole patchset should be pulled via the mmc tree, since
there are more mmc patches, if that's fine with Andrew.

I haven't checked the mmc patches, since I'm not very familiar with that
area, but the wl1271 driver change is ack from me.

-- 
Cheers,
Luca.