Messages in this thread | | | From | Bjorn Helgaas <> | Date | Fri, 4 May 2012 13:50:55 -0600 | Subject | Re: [RFC v2 2/5] PM, Add sysfs file power_off to control device power off policy |
| |
On Fri, May 4, 2012 at 2:13 AM, Huang Ying <ying.huang@intel.com> wrote: > From: Lan Tianyu <tianyu.lan@intel.com> > > Some devices can be powered off to save more power via some platform > mechanism, e.g., ACPI. But that may not work as expected for some > device or platform. So, this patch adds a sysfs file named power_off > under <device>/power directory to provide a mechanism for user to control > whether to allow the device to be power off. > > power_off => "enabled" means allowing the device to be powered off if > possible. > > power_off => "disabled" means the device must be power on anytime. > > Also add flag power_off_user to struct dev_pm_info to record users' > choice. The bus layer can use this field to determine whether to > power off the device.
My first thought was that writing to "power_off" would actually turn the power off, which isn't true. Maybe something like "poweroff_allowed" would work.
I think there's only one use of this new field, in pci_pm_runtime_suspend(). Maybe you could pull out that hunk from patch 5, combine it with this one, and move it to after patch 5?
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > Signed-off-by: Huang Ying <ying.huang@intel.com> > --- > drivers/base/power/sysfs.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/pm.h | 1 + > 2 files changed, 34 insertions(+) > > --- a/drivers/base/power/sysfs.c > +++ b/drivers/base/power/sysfs.c > @@ -243,6 +243,38 @@ static ssize_t pm_qos_latency_store(stru > > static DEVICE_ATTR(pm_qos_resume_latency_us, 0644, > pm_qos_latency_show, pm_qos_latency_store); > + > +static ssize_t power_off_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", > + dev->power.power_off_user ? enabled : disabled); > +} > + > +static ssize_t power_off_store(struct device * dev, > + struct device_attribute *attr, > + const char * buf, size_t n) > +{ > + char *cp; > + int len = n; > + unsigned int power_off_user; > + > + cp = memchr(buf, '\n', n); > + if (cp) > + len = cp - buf; > + > + if (len == sizeof enabled - 1 && strncmp(buf, enabled, len) == 0) > + dev->power.power_off_user = true; > + else if (len == sizeof disabled - 1 && strncmp(buf, disabled, len) == 0) > + dev->power.power_off_user = false; > + else > + return -EINVAL; > + > + pm_runtime_resume(dev); > + return n; > + > +} > +static DEVICE_ATTR(power_off, 0644, power_off_show, power_off_store); > #endif /* CONFIG_PM_RUNTIME */ > > #ifdef CONFIG_PM_SLEEP > @@ -508,6 +540,7 @@ static struct attribute *runtime_attrs[] > &dev_attr_runtime_suspended_time.attr, > &dev_attr_runtime_active_time.attr, > &dev_attr_autosuspend_delay_ms.attr, > + &dev_attr_power_off.attr, > #endif /* CONFIG_PM_RUNTIME */ > NULL, > }; > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -537,6 +537,7 @@ struct dev_pm_info { > unsigned int use_autosuspend:1; > unsigned int timer_autosuspends:1; > unsigned int power_must_be_on:1; > + unsigned int power_off_user:1;
This name definitely doesn't suggest anything useful I think "poweroff_allowed" or similar would make a lot more sense when reading the code.
> enum rpm_request request; > enum rpm_status runtime_status; > int runtime_error; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |