| Message ID | 1783611.vYsnfNDZr6@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make it possible to retrieve the current power state of a device with > ACPI power management from user space via sysfs by adding a new > attribute power_state to the sysfs directory associated with the > struct acpi_device object representing the device's ACPI node. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > drivers/acpi/scan.c | 29 +++++++++++++++++++- > 2 files changed, 49 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -178,6 +178,23 @@ err_out: > } > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > +static ssize_t power_state_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct acpi_device *adev = to_acpi_device(dev); > + int state; > + int ret; > + > + ret = acpi_device_get_power(adev, &state); > + if (ret) > + return ret; > + > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > + acpi_power_state_string(adev->power.state)); > +} You are showing 2 different things here in a single sysfs file, which is really frowned apon. Any chance to split this up into two different sysfs files instead? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 21, 2013 at 11:27:32PM +0100, Rafael J. Wysocki wrote: > On Monday, January 21, 2013 12:53:05 PM Greg Kroah-Hartman wrote: > > On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > Make it possible to retrieve the current power state of a device with > > > ACPI power management from user space via sysfs by adding a new > > > attribute power_state to the sysfs directory associated with the > > > struct acpi_device object representing the device's ACPI node. > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > --- > > > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > > > drivers/acpi/scan.c | 29 +++++++++++++++++++- > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > Index: linux-pm/drivers/acpi/scan.c > > > =================================================================== > > > --- linux-pm.orig/drivers/acpi/scan.c > > > +++ linux-pm/drivers/acpi/scan.c > > > @@ -178,6 +178,23 @@ err_out: > > > } > > > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > > > > > +static ssize_t power_state_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + struct acpi_device *adev = to_acpi_device(dev); > > > + int state; > > > + int ret; > > > + > > > + ret = acpi_device_get_power(adev, &state); > > > + if (ret) > > > + return ret; > > > + > > > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > > > + acpi_power_state_string(adev->power.state)); > > > +} > > > > You are showing 2 different things here in a single sysfs file, which is > > really frowned apon. Any chance to split this up into two different > > sysfs files instead? > > Well, I can, but I'm not sure how to call the other one. "sw_power_state" > perhaps? I don't know, as I'm not quite sure what it is supposed to represent :) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, January 21, 2013 12:53:05 PM Greg Kroah-Hartman wrote: > On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Make it possible to retrieve the current power state of a device with > > ACPI power management from user space via sysfs by adding a new > > attribute power_state to the sysfs directory associated with the > > struct acpi_device object representing the device's ACPI node. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > > drivers/acpi/scan.c | 29 +++++++++++++++++++- > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > Index: linux-pm/drivers/acpi/scan.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -178,6 +178,23 @@ err_out: > > } > > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > > > +static ssize_t power_state_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct acpi_device *adev = to_acpi_device(dev); > > + int state; > > + int ret; > > + > > + ret = acpi_device_get_power(adev, &state); > > + if (ret) > > + return ret; > > + > > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > > + acpi_power_state_string(adev->power.state)); > > +} > > You are showing 2 different things here in a single sysfs file, which is > really frowned apon. Any chance to split this up into two different > sysfs files instead? Well, I can, but I'm not sure how to call the other one. "sw_power_state" perhaps? Rafael
On Monday, January 21, 2013 02:26:47 PM Greg Kroah-Hartman wrote: > On Mon, Jan 21, 2013 at 11:27:32PM +0100, Rafael J. Wysocki wrote: > > On Monday, January 21, 2013 12:53:05 PM Greg Kroah-Hartman wrote: > > > On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > Make it possible to retrieve the current power state of a device with > > > > ACPI power management from user space via sysfs by adding a new > > > > attribute power_state to the sysfs directory associated with the > > > > struct acpi_device object representing the device's ACPI node. > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > --- > > > > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > > > > drivers/acpi/scan.c | 29 +++++++++++++++++++- > > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > Index: linux-pm/drivers/acpi/scan.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/acpi/scan.c > > > > +++ linux-pm/drivers/acpi/scan.c > > > > @@ -178,6 +178,23 @@ err_out: > > > > } > > > > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > > > > > > > +static ssize_t power_state_show(struct device *dev, > > > > + struct device_attribute *attr, char *buf) > > > > +{ > > > > + struct acpi_device *adev = to_acpi_device(dev); > > > > + int state; > > > > + int ret; > > > > + > > > > + ret = acpi_device_get_power(adev, &state); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > > > > + acpi_power_state_string(adev->power.state)); > > > > +} > > > > > > You are showing 2 different things here in a single sysfs file, which is > > > really frowned apon. Any chance to split this up into two different > > > sysfs files instead? > > > > Well, I can, but I'm not sure how to call the other one. "sw_power_state" > > perhaps? > > I don't know, as I'm not quite sure what it is supposed to represent :) The first one is power state as read using _PSC or inferred from power resources on/off configuration. That's easy. Now, if power resources are shared between two or more devices, it is possible that one device will be in, say, D3hot from the software point of view, but its real ("physical") power state will be different, because the other devices still keep the shared resource "on". In that case, if the "software" power state of a device is lower-power (higher-number) than its real power state, we know that that particular device doesn't prevent the shared resource from being turned off. This is good to know, I think. :-) Thanks, Rafael
On Mon, Jan 21, 2013 at 11:59:12PM +0100, Rafael J. Wysocki wrote: > On Monday, January 21, 2013 02:26:47 PM Greg Kroah-Hartman wrote: > > On Mon, Jan 21, 2013 at 11:27:32PM +0100, Rafael J. Wysocki wrote: > > > On Monday, January 21, 2013 12:53:05 PM Greg Kroah-Hartman wrote: > > > > On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > > > Make it possible to retrieve the current power state of a device with > > > > > ACPI power management from user space via sysfs by adding a new > > > > > attribute power_state to the sysfs directory associated with the > > > > > struct acpi_device object representing the device's ACPI node. > > > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > --- > > > > > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > > > > > drivers/acpi/scan.c | 29 +++++++++++++++++++- > > > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > > > Index: linux-pm/drivers/acpi/scan.c > > > > > =================================================================== > > > > > --- linux-pm.orig/drivers/acpi/scan.c > > > > > +++ linux-pm/drivers/acpi/scan.c > > > > > @@ -178,6 +178,23 @@ err_out: > > > > > } > > > > > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > > > > > > > > > +static ssize_t power_state_show(struct device *dev, > > > > > + struct device_attribute *attr, char *buf) > > > > > +{ > > > > > + struct acpi_device *adev = to_acpi_device(dev); > > > > > + int state; > > > > > + int ret; > > > > > + > > > > > + ret = acpi_device_get_power(adev, &state); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > > > > > + acpi_power_state_string(adev->power.state)); > > > > > +} > > > > > > > > You are showing 2 different things here in a single sysfs file, which is > > > > really frowned apon. Any chance to split this up into two different > > > > sysfs files instead? > > > > > > Well, I can, but I'm not sure how to call the other one. "sw_power_state" > > > perhaps? > > > > I don't know, as I'm not quite sure what it is supposed to represent :) > > The first one is power state as read using _PSC or inferred from power > resources on/off configuration. That's easy. > > Now, if power resources are shared between two or more devices, it is possible > that one device will be in, say, D3hot from the software point of view, but its > real ("physical") power state will be different, because the other devices > still keep the shared resource "on". In that case, if the "software" power > state of a device is lower-power (higher-number) than its real power state, > we know that that particular device doesn't prevent the shared resource from > being turned off. This is good to know, I think. :-) I agree that it's good to know, just what to call it. I think you just named it with "real_power_state", right? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, January 21, 2013 03:08:04 PM Greg Kroah-Hartman wrote: > On Mon, Jan 21, 2013 at 11:59:12PM +0100, Rafael J. Wysocki wrote: > > On Monday, January 21, 2013 02:26:47 PM Greg Kroah-Hartman wrote: > > > On Mon, Jan 21, 2013 at 11:27:32PM +0100, Rafael J. Wysocki wrote: > > > > On Monday, January 21, 2013 12:53:05 PM Greg Kroah-Hartman wrote: > > > > > On Mon, Jan 21, 2013 at 02:04:32PM +0100, Rafael J. Wysocki wrote: > > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > > > > > Make it possible to retrieve the current power state of a device with > > > > > > ACPI power management from user space via sysfs by adding a new > > > > > > attribute power_state to the sysfs directory associated with the > > > > > > struct acpi_device object representing the device's ACPI node. > > > > > > > > > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > --- > > > > > > Documentation/ABI/testing/sysfs-devices-power_state | 21 ++++++++++++++ > > > > > > drivers/acpi/scan.c | 29 +++++++++++++++++++- > > > > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > > > > > Index: linux-pm/drivers/acpi/scan.c > > > > > > =================================================================== > > > > > > --- linux-pm.orig/drivers/acpi/scan.c > > > > > > +++ linux-pm/drivers/acpi/scan.c > > > > > > @@ -178,6 +178,23 @@ err_out: > > > > > > } > > > > > > EXPORT_SYMBOL(acpi_bus_hot_remove_device); > > > > > > > > > > > > +static ssize_t power_state_show(struct device *dev, > > > > > > + struct device_attribute *attr, char *buf) > > > > > > +{ > > > > > > + struct acpi_device *adev = to_acpi_device(dev); > > > > > > + int state; > > > > > > + int ret; > > > > > > + > > > > > > + ret = acpi_device_get_power(adev, &state); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), > > > > > > + acpi_power_state_string(adev->power.state)); > > > > > > +} > > > > > > > > > > You are showing 2 different things here in a single sysfs file, which is > > > > > really frowned apon. Any chance to split this up into two different > > > > > sysfs files instead? > > > > > > > > Well, I can, but I'm not sure how to call the other one. "sw_power_state" > > > > perhaps? > > > > > > I don't know, as I'm not quite sure what it is supposed to represent :) > > > > The first one is power state as read using _PSC or inferred from power > > resources on/off configuration. That's easy. > > > > Now, if power resources are shared between two or more devices, it is possible > > that one device will be in, say, D3hot from the software point of view, but its > > real ("physical") power state will be different, because the other devices > > still keep the shared resource "on". In that case, if the "software" power > > state of a device is lower-power (higher-number) than its real power state, > > we know that that particular device doesn't prevent the shared resource from > > being turned off. This is good to know, I think. :-) > > I agree that it's good to know, just what to call it. I think you just > named it with "real_power_state", right? Yes, I think I'll just call them "power_state" and "real_power_state". That should be clear enough. Thanks, Rafael
Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -178,6 +178,23 @@ err_out: } EXPORT_SYMBOL(acpi_bus_hot_remove_device); +static ssize_t power_state_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct acpi_device *adev = to_acpi_device(dev); + int state; + int ret; + + ret = acpi_device_get_power(adev, &state); + if (ret) + return ret; + + return sprintf(buf, "%s %s\n", acpi_power_state_string(state), + acpi_power_state_string(adev->power.state)); +} + +static DEVICE_ATTR(power_state, 0444, power_state_show, NULL); + static ssize_t acpi_eject_store(struct device *d, struct device_attribute *attr, const char *buf, size_t count) @@ -369,8 +386,15 @@ static int acpi_device_setup_files(struc * hot-removal function from userland. */ status = acpi_get_handle(dev->handle, "_EJ0", &temp); - if (ACPI_SUCCESS(status)) + if (ACPI_SUCCESS(status)) { result = device_create_file(&dev->dev, &dev_attr_eject); + if (result) + goto end; + } + + if (dev->flags.power_manageable) + result = device_create_file(&dev->dev, &dev_attr_power_state); + end: return result; } @@ -380,6 +404,9 @@ static void acpi_device_remove_files(str acpi_status status; acpi_handle temp; + if (dev->flags.power_manageable) + device_remove_file(&dev->dev, &dev_attr_power_state); + /* * If device has _STR, remove 'description' file */ Index: linux-pm/Documentation/ABI/testing/sysfs-devices-power_state =================================================================== --- /dev/null +++ linux-pm/Documentation/ABI/testing/sysfs-devices-power_state @@ -0,0 +1,21 @@ +What: /sys/devices/.../power_state +Date: January 2013 +Contact: Rafael J. Wysocki <rafael.j.wysocki@intel.com> +Description: + The /sys/devices/.../power_state attribute is only present for + device objects representing ACPI device nodes that provide power + management methods. + + If present, it contains a pair of strings representing the + current ACPI power state of the given device node and the ACPI + power state the device node would be in if it did not share + power resources with other device nodes, respectively. If the + given device node does not share power resources with other + device nodes or it does not use power resource objects for power + management, the strings are always the same. + + For each of the strings the possible values are "D0", "D1", + "D2", "D3hot", and "D3cold" which reflect the power state names + defined by the ACPI specification (4.0 and above). + + This attribute is read-only.