diff mbox

[RFC,4/4] sdio: pm: set device's power state after driver runtime suspended it

Message ID 1350011561-21039-5-git-send-email-aaron.lu@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aaron Lu Oct. 12, 2012, 3:12 a.m. UTC
In sdio bus level runtime callback function, after call the driver's
runtime suspend callback, we will check if the device supports a
platform level power management, and if so, a proper power state is
chosen by the corresponding platform callback and then set.

Platform level runtime wakeup is also set, if device is enabled for
runtime wakeup by its driver, it will be armed the ability to generate
a wakeup event by the platform.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/mmc/core/sdio_bus.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

Comments

Rafael Wysocki Oct. 18, 2012, 11:39 p.m. UTC | #1
On Friday 12 of October 2012 11:12:41 Aaron Lu wrote:
> In sdio bus level runtime callback function, after call the driver's
> runtime suspend callback, we will check if the device supports a
> platform level power management, and if so, a proper power state is
> chosen by the corresponding platform callback and then set.
> 
> Platform level runtime wakeup is also set, if device is enabled for
> runtime wakeup by its driver, it will be armed the ability to generate
> a wakeup event by the platform.
> 
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>  drivers/mmc/core/sdio_bus.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index aaec9e2..d83dea8 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -23,6 +23,7 @@
>  
>  #include "sdio_cis.h"
>  #include "sdio_bus.h"
> +#include "sdio.h"
>  #include "sdio_acpi.h"
>  
>  /* show configuration fields */
> @@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
>  }
>  
>  #ifdef CONFIG_PM
> +
> +static int sdio_bus_runtime_suspend(struct device *dev)
> +{
> +	int ret;
> +	sdio_power_t state;
> +
> +	ret = pm_generic_runtime_suspend(dev);
> +	if (ret)
> +		goto out;
> +
> +	if (!platform_sdio_power_manageable(dev))
> +		goto out;
> +
> +	platform_sdio_run_wake(dev, true);
> +
> +	state = platform_sdio_choose_power_state(dev);
> +	if (state == SDIO_POWER_ERROR) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	ret = platform_sdio_set_power_state(dev, state);
> +
> +out:
> +	return ret;
> +}
> +
> +static int sdio_bus_runtime_resume(struct device *dev)
> +{
> +	int ret;
> +
> +	if (platform_sdio_power_manageable(dev)) {
> +		platform_sdio_run_wake(dev, false);
> +		ret = platform_sdio_set_power_state(dev, SDIO_D0);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	ret = pm_generic_runtime_resume(dev);
> +
> +out:
> +	return ret;
> +}
> +

Most likely we will need to make analogous changes for other bus types that
don't support power management natively, like platform, SPI, I2C etc.  In all
of them the _runtime_suspend() and _runtime_resume() routine will look
almost exactly the same except for the platform_sdio_ prefix.

For this reason, I think it would be better to simply define two functions
acpi_pm_runtime_suspend() and acpi_pm_runtime_resume() that will do all of
the ACPI-specific operations related to runtime suspend/resume.  Then, we
will be able to use these functions for all of the bus types in question
in the same way (we may also need to add analogous functions for system
suspend/resume handling).

If we do that, then the sdio_platform_pm_ops structure from the previous
patch won't be necessary any more.

Thanks,
Rafael
diff mbox

Patch

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index aaec9e2..d83dea8 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -23,6 +23,7 @@ 
 
 #include "sdio_cis.h"
 #include "sdio_bus.h"
+#include "sdio.h"
 #include "sdio_acpi.h"
 
 /* show configuration fields */
@@ -194,10 +195,54 @@  static int sdio_bus_remove(struct device *dev)
 }
 
 #ifdef CONFIG_PM
+
+static int sdio_bus_runtime_suspend(struct device *dev)
+{
+	int ret;
+	sdio_power_t state;
+
+	ret = pm_generic_runtime_suspend(dev);
+	if (ret)
+		goto out;
+
+	if (!platform_sdio_power_manageable(dev))
+		goto out;
+
+	platform_sdio_run_wake(dev, true);
+
+	state = platform_sdio_choose_power_state(dev);
+	if (state == SDIO_POWER_ERROR) {
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = platform_sdio_set_power_state(dev, state);
+
+out:
+	return ret;
+}
+
+static int sdio_bus_runtime_resume(struct device *dev)
+{
+	int ret;
+
+	if (platform_sdio_power_manageable(dev)) {
+		platform_sdio_run_wake(dev, false);
+		ret = platform_sdio_set_power_state(dev, SDIO_D0);
+		if (ret)
+			goto out;
+	}
+
+	ret = pm_generic_runtime_resume(dev);
+
+out:
+	return ret;
+}
+
 static const struct dev_pm_ops sdio_bus_pm_ops = {
 	SET_RUNTIME_PM_OPS(
-		pm_generic_runtime_suspend,
-		pm_generic_runtime_resume,
+		sdio_bus_runtime_suspend,
+		sdio_bus_runtime_resume,
 		pm_generic_runtime_idle
 	)
 };