* Userspace interface breakage in power/state @ 2006-01-16 0:25 Matthew Garrett 2006-01-16 19:20 ` Pavel Machek 0 siblings, 1 reply; 7+ messages in thread From: Matthew Garrett @ 2006-01-16 0:25 UTC (permalink / raw) To: linux-kernel, linux-pm [-- Attachment #1: Type: text/plain, Size: 405 bytes --] In older kernels, power/state for PCI devices took a PCI power state as an argument and so "3" was an entirely sensible thing to echo into it. In current kernels, it hits a BUG() in pci_choose_state and things blow up. While I realise that the former interface was broken and wrong, would it be possible to move to a new one without breaking existing code? -- Matthew Garrett | mjg59@srcf.ucam.org [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Userspace interface breakage in power/state 2006-01-16 0:25 Userspace interface breakage in power/state Matthew Garrett @ 2006-01-16 19:20 ` Pavel Machek 2006-01-16 19:25 ` Matthew Garrett 0 siblings, 1 reply; 7+ messages in thread From: Pavel Machek @ 2006-01-16 19:20 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 541 bytes --] On Po 16-01-06 00:25:17, Matthew Garrett wrote: > In older kernels, power/state for PCI devices took a PCI power state as > an argument and so "3" was an entirely sensible thing to echo into it. > In current kernels, it hits a BUG() in pci_choose_state and things blow > up. > > While I realise that the former interface was broken and wrong, would it > be possible to move to a new one without breaking existing code? I had patch to fix this breakage, but it was rejected. What depends on that code? Pavel -- Thanks, Sharp! [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Userspace interface breakage in power/state 2006-01-16 19:20 ` Pavel Machek @ 2006-01-16 19:25 ` Matthew Garrett 2006-01-16 20:05 ` Pavel Machek 0 siblings, 1 reply; 7+ messages in thread From: Matthew Garrett @ 2006-01-16 19:25 UTC (permalink / raw) To: Pavel Machek; +Cc: linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 227 bytes --] On Mon, Jan 16, 2006 at 08:20:36PM +0100, Pavel Machek wrote: > I had patch to fix this breakage, but it was rejected. What depends on > that code? Ubuntu's wireless disable script. -- Matthew Garrett | mjg59@srcf.ucam.org [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Userspace interface breakage in power/state 2006-01-16 19:25 ` Matthew Garrett @ 2006-01-16 20:05 ` Pavel Machek 2006-01-16 20:22 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Pavel Machek @ 2006-01-16 20:05 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 453 bytes --] On Po 16-01-06 19:25:50, Matthew Garrett wrote: > On Mon, Jan 16, 2006 at 08:20:36PM +0100, Pavel Machek wrote: > > > I had patch to fix this breakage, but it was rejected. What depends on > > that code? > > Ubuntu's wireless disable script. Can you search linux-pm archives? Search for "3" (that's '"', '3', '"'). It should be somewhere. I no longer have a copy. (Probably could get it from my working tree git...) Pavel -- Thanks, Sharp! [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Userspace interface breakage in power/state 2006-01-16 20:05 ` Pavel Machek @ 2006-01-16 20:22 ` Rafael J. Wysocki 2006-01-16 20:24 ` Pavel Machek 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2006-01-16 20:22 UTC (permalink / raw) To: linux-pm; +Cc: Matthew Garrett, linux-kernel [-- Attachment #1: Type: text/plain, Size: 3720 bytes --] Hi, On Monday, 16 January 2006 21:05, Pavel Machek wrote: > On Po 16-01-06 19:25:50, Matthew Garrett wrote: > > On Mon, Jan 16, 2006 at 08:20:36PM +0100, Pavel Machek wrote: > > > > > I had patch to fix this breakage, but it was rejected. What depends on > > > that code? > > > > Ubuntu's wireless disable script. > > Can you search linux-pm archives? Search for "3" (that's '"', '3', > '"'). It should be somewhere. I no longer have a copy. (Probably could > get it from my working tree git...) I think the appended message contains the latest version. Greetings, Rafael ---------- Forwarded Message ---------- Subject: Re: [linux-pm] [patch] pm: fix runtime powermanagement's /sys interface Date: Friday, 6 January 2006 00:08 From: Pavel Machek <pavel@ucw.cz> To: Dominik Brodowski <linux@dominikbrodowski.net>, Patrick Mochel <mochel@digitalimplant.org>, Andrew Morton <akpm@osdl.org>, Linux-pm mailing list <linux-pm@lists.osdl.org>, kernel list <linux-kernel@vger.kernel.org> On Čt 05-01-06 23:27:05, Dominik Brodowski wrote: > On Thu, Jan 05, 2006 at 11:23:38PM +0100, Pavel Machek wrote: > > > In addition, your patch breaks pcmcia / pcmciautils which already uses > > > numbers (which I already had to change from "3" to "2" before...). > > > > pcmcia actually uses this? Ouch. Do you just read the power file, or > > do you write to it, too? > > Reading and writing. Replacement for "cardctl suspend" and "cardctl resume". > > > static int pccardctl_power_one(unsigned long socket_no, unsigned int device, > unsigned int power) > { > int ret; > char file[SYSFS_PATH_MAX]; > struct sysfs_attribute *attr; > > snprintf(file, SYSFS_PATH_MAX, > "/sys/bus/pcmcia/devices/%lu.%u/power/state", > socket_no, device); > > attr = sysfs_open_attribute(file); > if (!attr) > return -ENODEV; > > ret = sysfs_write_attribute(attr, power ? "2" : "0", 1); > > sysfs_close_attribute(attr); > > return (ret); > } > > > NB: it will break one day, one way or another, when gregkh makes the > /sys/class -> /sys/devices conversion. However, I'd want to try not to break > the new pcmciautils userspace too often... Ok, so lets at least add value-checking to .../power file, and prevent userspace see changes to PM_EVENT_SUSPEND value. 2 and 0 are now "arbitrary cookies". I'd like to use "on" and "off", but pcmcia apparently depends on "2" and "0", so... Any objections? Signed-off-by: Pavel Machek <pavel@suse.cz> Pavel diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c --- a/drivers/base/power/sysfs.c +++ b/drivers/base/power/sysfs.c @@ -27,22 +27,25 @@ static ssize_t state_show(struct device * dev, struct device_attribute *attr, char * buf) { - return sprintf(buf, "%u\n", dev->power.power_state.event); + if (dev->power.power_state.event) + return sprintf(buf, "2\n"); + else + return sprintf(buf, "0\n"); } static ssize_t state_store(struct device * dev, struct device_attribute *attr, const char * buf, size_t n) { pm_message_t state; - char * rest; - int error = 0; + int error = -EINVAL; - state.event = simple_strtoul(buf, &rest, 10); - if (*rest) - return -EINVAL; - if (state.event) - error = dpm_runtime_suspend(dev, state); - else + state.event = PM_EVENT_SUSPEND; + if ((n == 1) && !strncmp(buf, "2", 1)) { dpm_runtime_resume(dev); + error = 0; + } + if ((n == 1) && !strncmp(buf, "0", 1)) + error = dpm_runtime_suspend(dev, state); + return error ? error : n; } [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Userspace interface breakage in power/state 2006-01-16 20:22 ` Rafael J. Wysocki @ 2006-01-16 20:24 ` Pavel Machek 0 siblings, 0 replies; 7+ messages in thread From: Pavel Machek @ 2006-01-16 20:24 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Matthew Garrett, linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2173 bytes --] On Po 16-01-06 21:22:10, Rafael J. Wysocki wrote: > Hi, > > On Monday, 16 January 2006 21:05, Pavel Machek wrote: > > On Po 16-01-06 19:25:50, Matthew Garrett wrote: > > > On Mon, Jan 16, 2006 at 08:20:36PM +0100, Pavel Machek wrote: > > > > > > > I had patch to fix this breakage, but it was rejected. What depends on > > > > that code? > > > > > > Ubuntu's wireless disable script. > > > > Can you search linux-pm archives? Search for "3" (that's '"', '3', > > '"'). It should be somewhere. I no longer have a copy. (Probably could > > get it from my working tree git...) > > I think the appended message contains the latest version. Thanks, thats it. Matthew, you probably want to replace "2" with "3" below. Pavel > Ok, so lets at least add value-checking to .../power file, and prevent > userspace see changes to PM_EVENT_SUSPEND value. 2 and 0 are now > "arbitrary cookies". I'd like to use "on" and "off", but pcmcia > apparently depends on "2" and "0", so... > > Any objections? > > Signed-off-by: Pavel Machek <pavel@suse.cz> > Pavel > > diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c > --- a/drivers/base/power/sysfs.c > +++ b/drivers/base/power/sysfs.c > @@ -27,22 +27,25 @@ > > static ssize_t state_show(struct device * dev, struct device_attribute *attr, char * buf) > { > - return sprintf(buf, "%u\n", dev->power.power_state.event); > + if (dev->power.power_state.event) > + return sprintf(buf, "2\n"); > + else > + return sprintf(buf, "0\n"); > } > > static ssize_t state_store(struct device * dev, struct device_attribute *attr, const char * buf, size_t n) > { > pm_message_t state; > - char * rest; > - int error = 0; > + int error = -EINVAL; > > - state.event = simple_strtoul(buf, &rest, 10); > - if (*rest) > - return -EINVAL; > - if (state.event) > - error = dpm_runtime_suspend(dev, state); > - else > + state.event = PM_EVENT_SUSPEND; > + if ((n == 1) && !strncmp(buf, "2", 1)) { > dpm_runtime_resume(dev); > + error = 0; > + } > + if ((n == 1) && !strncmp(buf, "0", 1)) > + error = dpm_runtime_suspend(dev, state); > + > return error ? error : n; > } > -- Thanks, Sharp! [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Userspace interface breakage in power/state @ 2006-01-16 0:27 Matthew Garrett 0 siblings, 0 replies; 7+ messages in thread From: Matthew Garrett @ 2006-01-16 0:27 UTC (permalink / raw) To: linux-kernel, linux-pm [-- Attachment #1: Type: text/plain, Size: 494 bytes --] (Resent, without the wrong address for lkml) In older kernels, power/state for PCI devices took a PCI power state as an argument and so "3" was an entirely sensible thing to echo into it. In current kernels, it hits a BUG() in pci_choose_state and things blow up. While I realise that the former interface was broken and wrong, would it be possible to move to a new one without breaking existing code? -- Matthew Garrett | mjg59@srcf.ucam.org -- Matthew Garrett | mjg59@srcf.ucam.org [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-01-16 20:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-01-16 0:25 Userspace interface breakage in power/state Matthew Garrett 2006-01-16 19:20 ` Pavel Machek 2006-01-16 19:25 ` Matthew Garrett 2006-01-16 20:05 ` Pavel Machek 2006-01-16 20:22 ` Rafael J. Wysocki 2006-01-16 20:24 ` Pavel Machek -- strict thread matches above, loose matches on Subject: below -- 2006-01-16 0:27 Matthew Garrett
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox