[PATCH 0/2] Check ACPI device poweroff capability

[PATCH 0/2] Check ACPI device poweroff capability

[Aaron Lu]

This patchset introduces a way to check if ACPI has provided a way
to put the devicei into D3 cold power state.

This patchset is based on Rafael's patchset:
ACPI / PM: ACPI power management callback routines for subsystems
http://marc.info/?l=linux-acpi&m=135150188412673&w=2

Aaron Lu (2):
  acpi: introduce swset flag for power_state
  acpi: add function to check if poweroff is possible

 drivers/acpi/scan.c     | 9 ++++++++-
 include/acpi/acpi_bus.h | 6 ++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

-- 
1.7.12.4

[PATCH 1/2] acpi: introduce swset flag for power_state

[Aaron Lu]

Currently we have valid flag to represent if this ACPI device power
state is valid. A device power state is valid does not necessarily mean
we, as OSPM, has a mean to put the device into that power state, e.g. D3
cold is always a valid power state for any ACPI device, but if there is
no _PS3 or _PRx for this device, we can't really put that device into
D3 cold power state. The same is true for D0 power state.

So here comes the swset flag, which is only set if the device has
provided us the required means to put it into that power state, e.g. if
we have _PS3 or _PRx, we can put the device into D3 cold state and thus,
D3 cold power state's swset flag will be set in this case.

This flag is mainly used to check if firmware has provided us a way to
put the device into D3 cold state, as the valid flag for D3 cold is
always true.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/acpi/scan.c     | 9 ++++++++-
 include/acpi/acpi_bus.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 95ff1e8..60d271a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1038,8 +1038,10 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 		 * D3hot is only valid if _PR3 present.
 		 */
 		if (ps->resources.count ||
-		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT))
+		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
 			ps->flags.valid = 1;
+			ps->flags.swset = 1;
+		}
 
 		ps->power = -1;	/* Unknown - driver assigned */
 		ps->latency = -1;	/* Unknown - driver assigned */
@@ -1055,6 +1057,11 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set)
 		device->power.states[ACPI_STATE_D3_COLD].flags.explicit_set = 1;
 
+	/* Presence of _PS3 or _PRx means we can put the device into D3 cold */
+	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set ||
+			device->power.flags.power_resources)
+		device->power.states[ACPI_STATE_D3_COLD].flags.swset = 1;
+
 	acpi_bus_init_power(device);
 
 	return 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 2242c10..7d20617 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -200,6 +200,7 @@ struct acpi_device_power_flags {
 struct acpi_device_power_state {
 	struct {
 		u8 valid:1;
+		u8 swset:1;
 		u8 explicit_set:1;	/* _PSx present? */
 		u8 reserved:6;
 	} flags;
-- 
1.7.12.4

[PATCH 2/2] acpi: add function to check if poweroff is possible

[Aaron Lu]

>From ACPI's perspective, a device is powered off if we put it into D3
cold power state. This wrapper inline function can be used to check if
firmware has provided us a way to power off the device during runtime.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 include/acpi/acpi_bus.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7d20617..932977a 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -496,6 +496,11 @@ static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
 	return adev->wakeup.flags.valid;
 }
 
+static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
+{
+	return adev->power.states[ACPI_STATE_D3_COLD].flags.swset;
+}
+
 #else	/* CONFIG_ACPI */
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }
-- 
1.7.12.4

Re: [PATCH 1/2] acpi: introduce swset flag for power_state

[Rafael J. Wysocki]

On Friday, November 09, 2012 09:54:55 AM Aaron Lu wrote:
> Currently we have valid flag to represent if this ACPI device power
> state is valid. A device power state is valid does not necessarily mean
> we, as OSPM, has a mean to put the device into that power state, e.g. D3
> cold is always a valid power state for any ACPI device, but if there is
> no _PS3 or _PRx for this device, we can't really put that device into
> D3 cold power state. The same is true for D0 power state.
> 
> So here comes the swset flag, which is only set if the device has
> provided us the required means to put it into that power state, e.g. if
> we have _PS3 or _PRx, we can put the device into D3 cold state and thus,
> D3 cold power state's swset flag will be set in this case.
> 
> This flag is mainly used to check if firmware has provided us a way to
> put the device into D3 cold state, as the valid flag for D3 cold is
> always true.

I would prefer the new flag to be called os_accessible.  The meaning of it
would be more clear this way in my opinion.

Please also fold patch [2/2] into [1/2].

Apart from this it looks OK.

Thanks,
Rafael


> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>  drivers/acpi/scan.c     | 9 ++++++++-
>  include/acpi/acpi_bus.h | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 95ff1e8..60d271a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1038,8 +1038,10 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
>  		 * D3hot is only valid if _PR3 present.
>  		 */
>  		if (ps->resources.count ||
> -		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT))
> +		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
>  			ps->flags.valid = 1;
> +			ps->flags.swset = 1;
> +		}
>  
>  		ps->power = -1;	/* Unknown - driver assigned */
>  		ps->latency = -1;	/* Unknown - driver assigned */
> @@ -1055,6 +1057,11 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
>  	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set)
>  		device->power.states[ACPI_STATE_D3_COLD].flags.explicit_set = 1;
>  
> +	/* Presence of _PS3 or _PRx means we can put the device into D3 cold */
> +	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set ||
> +			device->power.flags.power_resources)
> +		device->power.states[ACPI_STATE_D3_COLD].flags.swset = 1;
> +
>  	acpi_bus_init_power(device);
>  
>  	return 0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 2242c10..7d20617 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -200,6 +200,7 @@ struct acpi_device_power_flags {
>  struct acpi_device_power_state {
>  	struct {
>  		u8 valid:1;
> +		u8 swset:1;
>  		u8 explicit_set:1;	/* _PSx present? */
>  		u8 reserved:6;
>  	} flags;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v2] acpi: introduce os_accessible flag for power_state

[Aaron Lu]

Currently we have valid flag to represent if this ACPI device power
state is valid. A device power state is valid does not necessarily
mean we, as OSPM, has a mean to put the device into that power state,
e.g. D3 cold is always a valid power state for any ACPI device, but if
there is no _PS3 or _PRx for this device, we can't really put that
device into D3 cold power state. The same is true for D0 power state.

So here comes the os_accessible flag, which is only set if the device
has provided us the required means to put it into that power state,
e.g. if we have _PS3 or _PRx, we can put the device into D3 cold state
and thus, D3 cold power state's os_accessible flag will be set in this
case.

And a new wrapper inline function is added to be used to check if
firmware has provided us a way to power off the device during runtime.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
v2:
Rename swset to os_accessible as suggested by Rafael Wysocki.
Fold patch 2 into patch 1 as suggested by Rafael Wysocki.

 drivers/acpi/scan.c     | 9 ++++++++-
 include/acpi/acpi_bus.h | 6 ++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3c646e6..f250c5b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1062,8 +1062,10 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 		 * D3hot is only valid if _PR3 present.
 		 */
 		if (ps->resources.count ||
-		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT))
+		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
 			ps->flags.valid = 1;
+			ps->flags.os_accessible = 1;
+		}
 
 		ps->power = -1;	/* Unknown - driver assigned */
 		ps->latency = -1;	/* Unknown - driver assigned */
@@ -1079,6 +1081,11 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set)
 		device->power.states[ACPI_STATE_D3_COLD].flags.explicit_set = 1;
 
+	/* Presence of _PS3 or _PRx means we can put the device into D3 cold */
+	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set ||
+			device->power.flags.power_resources)
+		device->power.states[ACPI_STATE_D3_COLD].flags.os_accessible = 1;
+
 	acpi_bus_init_power(device);
 
 	return 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ce77cd7..644d941 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -201,6 +201,7 @@ struct acpi_device_power_flags {
 struct acpi_device_power_state {
 	struct {
 		u8 valid:1;
+		u8 os_accessible:1;
 		u8 explicit_set:1;	/* _PSx present? */
 		u8 reserved:6;
 	} flags;
@@ -500,6 +501,11 @@ static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
 	return adev->wakeup.flags.valid;
 }
 
+static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
+{
+	return adev->power.states[ACPI_STATE_D3_COLD].flags.os_accessible;
+}
+
 #else	/* CONFIG_ACPI */
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }
-- 
1.7.12.4

Re: [PATCH v2] acpi: introduce os_accessible flag for power_state

[Rafael J. Wysocki]

On Wednesday, November 21, 2012 08:24:20 PM Aaron Lu wrote:
> Currently we have valid flag to represent if this ACPI device power
> state is valid. A device power state is valid does not necessarily
> mean we, as OSPM, has a mean to put the device into that power state,
> e.g. D3 cold is always a valid power state for any ACPI device, but if
> there is no _PS3 or _PRx for this device, we can't really put that
> device into D3 cold power state. The same is true for D0 power state.
> 
> So here comes the os_accessible flag, which is only set if the device
> has provided us the required means to put it into that power state,
> e.g. if we have _PS3 or _PRx, we can put the device into D3 cold state
> and thus, D3 cold power state's os_accessible flag will be set in this
> case.
> 
> And a new wrapper inline function is added to be used to check if
> firmware has provided us a way to power off the device during runtime.

Applied to linux-pm.git/linux-next as v3.8 material.

Thanks,
Rafael


> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
> v2:
> Rename swset to os_accessible as suggested by Rafael Wysocki.
> Fold patch 2 into patch 1 as suggested by Rafael Wysocki.
> 
>  drivers/acpi/scan.c     | 9 ++++++++-
>  include/acpi/acpi_bus.h | 6 ++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 3c646e6..f250c5b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1062,8 +1062,10 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
>  		 * D3hot is only valid if _PR3 present.
>  		 */
>  		if (ps->resources.count ||
> -		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT))
> +		    (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
>  			ps->flags.valid = 1;
> +			ps->flags.os_accessible = 1;
> +		}
>  
>  		ps->power = -1;	/* Unknown - driver assigned */
>  		ps->latency = -1;	/* Unknown - driver assigned */
> @@ -1079,6 +1081,11 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
>  	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set)
>  		device->power.states[ACPI_STATE_D3_COLD].flags.explicit_set = 1;
>  
> +	/* Presence of _PS3 or _PRx means we can put the device into D3 cold */
> +	if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set ||
> +			device->power.flags.power_resources)
> +		device->power.states[ACPI_STATE_D3_COLD].flags.os_accessible = 1;
> +
>  	acpi_bus_init_power(device);
>  
>  	return 0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index ce77cd7..644d941 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -201,6 +201,7 @@ struct acpi_device_power_flags {
>  struct acpi_device_power_state {
>  	struct {
>  		u8 valid:1;
> +		u8 os_accessible:1;
>  		u8 explicit_set:1;	/* _PSx present? */
>  		u8 reserved:6;
>  	} flags;
> @@ -500,6 +501,11 @@ static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
>  	return adev->wakeup.flags.valid;
>  }
>  
> +static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
> +{
> +	return adev->power.states[ACPI_STATE_D3_COLD].flags.os_accessible;
> +}
> +
>  #else	/* CONFIG_ACPI */
>  
>  static inline int register_acpi_bus_type(void *bus) { return 0; }
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.