switching off autosuspend through sysfs
Subsystems (5)
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 7 lines · Greg KH
Quoted 6 lines · David Brownell
Quoted 6 lines
Quoted 5 lines
Review
Quoted 3 lines
static struct attribute *dev_attrs[] = {
(quoted diff) +2
-0+ /* power management attributes */ + &dev_attr_autosuspend.attr,Belongs in
/sys/devices/.../power/... then, right?
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
No, I thought we want to drop that power/ directory.
Dropping that directory hasn't AFAIK ever been discussed. If it were to be dropped, where would the per-device wakeup flags live?
I don't know, it just really annoys me to see that power directory there with no use for it for a lot of devices :)
Would it help to add a flag somewhere in struct device (or struct dev_pm_info) for indicating that the device is not cognizant of PM? For instance, all those USB endpoint pseudo-devices we create -- it's a waste of time to try doing power management on them and it generates a bunch of useless and distracting warning messages in the system log.
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
I have no
objection to it going into the USB device directory, as that is the proper place for it.It'd be OK for a USB-specific mechanism, but there was at one point some discussion about this mechanism being more generally appropriate.
Hm, I missed that. This seems to be a very usb-specific thing at the moment.
It is USB-specific because I wanted to start out in a well-defined environment where the mechanisms could get tested and shaken out. When the problems are gone, it will make sense to think about moving the whole autosuspend infrastructure into the PM core.
Quoted 3 lines
And an annoying one too for some of my devices...powering off
the bus for USB devices with no drivers might start causing devices that use usbfs to do wierd things. Still trying to work it all out though...
If it does then we need to know and find a way to deal with it.
Alan Stern
Quoted 15 lines · Alan Stern
Quoted 13 lines · Greg KH
Quoted 11 lines · David Brownell
Quoted 7 lines
Quoted 5 lines
Review
Quoted 3 lines
static struct attribute *dev_attrs[] = {
(quoted diff) +2
-0+ /* power management attributes */ + &dev_attr_autosuspend.attr,Belongs in
/sys/devices/.../power/... then, right?No, I thought we want to drop that power/ directory.
Dropping that directory hasn't AFAIK ever been discussed. If it were to be dropped, where would the per-device wakeup flags live?
Quoted 8 lines
Quoted 2 lines
I don't know, it just really annoys me to see that power directory there with no use for it for a lot of devices :)
Would it help to add a flag somewhere in struct device (or struct
dev_pm_info) for indicating that the device is not cognizant of PM? For instance, all those USB endpoint pseudo-devices we create -- it's a waste of time to try doing power management on them and it generates a bunch of useless and distracting warning messages in the system log.
Yes. In fact we should just make it a "has pm" type flag, as the majority of devices do not.
So, what kind of devices do support these files? I can think of:
PCI
USB
and that's it right now. Do platform devices really use those files?
thanks,
greg k-h
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Quoted 25 lines · Greg KH
Quoted 15 lines · Alan Stern
Quoted 13 lines · Greg KH
Quoted 11 lines · David Brownell
Quoted 7 lines
Quoted 5 lines
Review
Quoted 3 lines
static struct attribute *dev_attrs[] = {
(quoted diff) +2
-0+ /* power management attributes */ + &dev_attr_autosuspend.attr,Belongs in
/sys/devices/.../power/... then, right?No, I thought we want to drop that power/ directory.
Dropping that directory hasn't AFAIK ever been discussed. If it were to be dropped, where would the per-device wakeup flags live?
Quoted 8 lines
Quoted 2 lines
I don't know, it just really annoys me to see that power directory there with no use for it for a lot of devices :)
Would it help to add a flag somewhere in struct device (or struct
dev_pm_info) for indicating that the device is not cognizant of PM? For instance, all those USB endpoint pseudo-devices we create -- it's a waste of time to try doing power management on them and it generates a bunch of useless and distracting warning messages in the system log.
Quoted 7 lines
Yes. In fact we should just make it a "has pm" type flag, as the majority of devices do not.
So, what kind of devices do support these files? I can think of:
PCI
USB
and that's it right now. Do platform devices really use those files?
Something as simple as this patch perhaps?
thanks,
greg k-h
--- drivers/base/power/sysfs.c | 7 +++++-- drivers/pci/probe.c | 1 + include/linux/device.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-)
drivers/base/power/sysfs.c +5 -2
@@ -156,10 +156,13 @@ static struct attribute_group pm_attr_grint dpm_sysfs_add (struct device * dev) { -return sysfs_create_group (&dev->kobj, &pm_attr_group); +if (dev->supports_power) +return sysfs_create_group (&dev->kobj, &pm_attr_group); +return 0 ; }void dpm_sysfs_remove (struct device * dev) { -sysfs_remove_group (&dev->kobj, &pm_attr_group); +if (dev->supports_power) +sysfs_remove_group (&dev->kobj, &pm_attr_group); }
drivers/pci/probe.c +1 -0
@@ -893,6 +893,7 @@ pci_scan_device(struct pci_bus *bus, int dev->sysdata = bus->sysdata; dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; + dev->dev.supports_power =1 ; dev->devfn = devfn; dev->hdr_type = hdr_type &0x7f ; dev->multifunction = !!(hdr_type &0x80 );
include/linux/device.h +1 -0
@@ -365,6 +365,7 @@ struct device {char bus_id[BUS_ID_SIZE ];/* position on parent bus */ struct device_type *type; unsigned is_registered:1; + unsigned supports_power:1;struct device_attribute uevent_attr;struct device_attribute *devt_attr;
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Quoted 9 lines · Greg KH
Quoted 9 lines
Quoted 5 lines
Would it help to add a flag somewhere in struct device (or struct
dev_pm_info) for indicating that the device is not cognizant of PM? For instance, all those USB endpoint pseudo-devices we create -- it's a waste of time to try doing power management on them and it generates a bunch of useless and distracting warning messages in the system log.Yes. In fact we should just make it a "has pm" type flag, as the majority of devices do not.
Quoted 6 lines
Quoted 4 lines
So, what kind of devices do support these files? I can think of:
PCI
USB
and that's it right now. Do platform devices really use those files?Something as simple as this patch perhaps?
I had something bigger in mind. More like the patch below (completely untested). Plus of course code to turn off the new flag for the appropriate devices.
Alan Stern
drivers/base/power/sysfs.c +5 -2
@@ -152,10 +152,13 @@ static struct attribute_group pm_attr_grint dpm_sysfs_add (struct device * dev) { -return sysfs_create_group (&dev->kobj, &pm_attr_group); +if (dev->supports_pm) +return sysfs_create_group (&dev->kobj, &pm_attr_group); +return 0 ; }void dpm_sysfs_remove (struct device * dev) { -sysfs_remove_group (&dev->kobj, &pm_attr_group); +if (dev->supports_pm) +sysfs_remove_group (&dev->kobj, &pm_attr_group); }
drivers/pci/probe.c +1 -0
@@ -893,6 +893,7 @@ pci_scan_device(struct pci_bus *bus, int dev->sysdata = bus->sysdata; dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; + dev->dev.supports_pm =1 ; dev->devfn = devfn; dev->hdr_type = hdr_type &0x7f ; dev->multifunction = !!(hdr_type &0x80 );
include/linux/device.h +1 -0
@@ -367,6 +367,7 @@ struct device {char bus_id[BUS_ID_SIZE ];/* position on parent bus */ struct device_type *type; unsigned is_registered:1; + unsigned supports_pm:1;struct device_attribute uevent_attr;struct device_attribute *devt_attr;
drivers/base/power/resume.c +6 -0
@@ -24,6 +24,9 @@ int resume_device(struct device * dev) {int error =0 ; +if (!dev->supports_pm) +return error; +TRACE_DEVICE (dev);TRACE_RESUME (0 );down (&dev->sem);
@@ -52,6 +55,9 @@ static int resume_device_early(struct de {int error =0 ; +if (!dev->supports_pm) +return error; +TRACE_DEVICE (dev);TRACE_RESUME (0 );if (dev->bus && dev->bus->resume_early) {
drivers/base/power/suspend.c +5 -1
@@ -50,6 +50,9 @@ int suspend_device(struct device * dev, {int error =0 ; +if (!dev->supports_pm) +return error; +down (&dev->sem);if (dev->power.power_state.event) { dev_dbg(dev, "PM: suspend %d-->%d\n",
@@ -103,7 +106,8 @@ static int suspend_device_late(struct de {int error =0 ; -if (dev->bus && dev->bus->suspend_late && !dev->power.power_state.event) { +if (dev->supports_pm && dev->bus && dev->bus->suspend_late && + !dev->power.power_state.event) { dev_dbg(dev, "LATE %s%s\n", suspend_verb(state.event), ((state.event == PM_EVENT_SUSPEND)
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I don't know, it just really annoys me to see that power directory there with no use for it for a lot of devices :)
Would it help to add a flag somewhere in struct device (or struct
dev_pm_info) for indicating that the device is not cognizant of PM?
If it would, the semantics would need to be clarified. Is the problem that they can't suspend/resume at all? Or is the problem instead that they (or their drivers) don't know how to do that without any loss of functionality ... without a complete reset or whatever?
Most devices can -- if poked and prodded appropriately, and if no BIOS or uploaded firmware interferes badly enough -- make it through some kind of system suspend state. Whether that's the one the user has chosen ... different issue! And whether the driver knows how to do that ... also a different issue!
But the specific problems we're seeing right now with the USB autosuspend stuff relate to _runtime_ suspend. I believe some of them are just bugs in the stack; where better coddling of the devices would let them make it through unscathed. But some of them, likewise, probably _do_ have real problems with real runtime suspend states (that don't map to "off").
Quoted 10 lines
Quoted 4 lines
For
instance, all those USB endpoint pseudo-devices we create -- it's a waste of time to try doing power management on them and it generates a bunch of useless and distracting warning messages in the system log.Yes. In fact we should just make it a "has pm" type flag, as the majority of devices do not.
So, what kind of devices do support these files? I can think of:
PCI
Actually, PCI still doesn't because of strangeness in the init sequencing on at least PPC ... I can forward the patch that makes X86 init the wakeup flag from the PM attributes.
There is however also the issue that on x86, ACPI (in my own experience) just basically doesn't work as an intermediary for most wakeup events. And after all these years, /proc/acpi/wakeup STILL doesn't hook up to the relevant devices in any useful way. Drivers have no idea if their devices are expected to support wakeup events at all. (That is, assuming ACPI supported it ...)
Quoted 2 lines
USB
and that's it right now. Do platform devices really use those files?
Depends on the platform. AT91 is pretty good about it; most of the platform devices there support wakeup from at least one of the two
power states. Some of the OMAP devices do, and PXA isn't far from
having at least the IRQ wake mechanisms working. I've been working on getting a lot of RTCs to act as wakeup event sources, since that's one of the few device types that issues wakeup events on most systems.
There's a chicken/egg problem here too ... how to get to a critical mass of drivers (and platforms) that handle wakeup events properly.
... When most platforms won't even go to _sleep_ properly.
It's actually been a lot *easier* in my experience to get that to work on an embedded Linux platform than on an x86 PC. That's because despite the usual glitches in chips and specs, the vendors are mostly accustomed to giving that sort of information out to folk implementing
a variety of operating systems. Whereas on PCs, they basically only
give it out to BIOS vendors, who then generate buggy firmware and buggy ACPI bytecode to deliver to otherwise happy penguins; naturally that sort of third or fourth hand stuff works poorly.
- Dave
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Quoted 6 lines · David Brownell
Quoted 2 lines
So, what kind of devices do support these files? I can think of:
PCIActually, PCI still doesn't because of strangeness in the init sequencing on at least PPC ... I can forward the patch that makes X86 init the wakeup flag from the PM attributes.
And here it is. Last refreshed against 2.6.20-rc5; it still works like a charm on x86. (As it did since before that PPC patch...) I have no idea whether this still makes trouble on PPC.
- Dave
================= CUT HERE
This patch teaches "pci_dev" about the driver model wakeup support:
- It marks devices as supporting wakeup when the PME# capability is listed in its PCI PM capability.
- pci_enable_wake() refuses to enable wake if that's been disabled (e.g. through sysfs).
NOTE that PowerPC does PCI probing a bit differently ... which evidently causes this to fail on that platform (and maybe others).
One issue is that the driver model "init + add == register" pattern isn't used inside PCI ... and the PCI probe change that made PowerPC happier worsened the problem by making "add" do some "init" too. Maybe PCI should match the driver model a lot more closely and grow a "pci_dev_init()".
drivers/pci/probe.c +15 -1
@@ -654,6 +654,7 @@ static void pci_read_irq(struct pci_devstatic int pci_setup_device (struct pci_dev * dev) {u32 class; +u16 pm;sprintf (pci_name (dev),"%04x:%02x:%02x.%d" ,pci_domain_nr (dev->bus), dev->bus->number,PCI_SLOT (dev->devfn),PCI_FUNC (dev->devfn));
@@ -682,6 +683,19 @@ static int pci_setup_device(struct pci_dpci_read_config_word (dev,PCI_SUBSYSTEM_VENDOR_ID , &dev->subsystem_vendor);pci_read_config_word (dev,PCI_SUBSYSTEM_ID , &dev->subsystem_device); +/* PCI PM capable devices may be able to issue PME# (wakeup) */ + pm =pci_find_capability (dev,PCI_CAP_ID_PM ); +if (pm) { +pci_read_config_word (dev, pm +PCI_PM_PMC , &pm); +if (pm &PCI_PM_CAP_PME_MASK ) +device_init_wakeup (&dev->dev,1 ); + +/* REVISIT: if (pm & PCI_PM_CAP_PME_D3cold) then +* pci pm spec 1.2, section 3.2.4 says we should +* init PCI_PM_CTRL_PME_{STATUS,ENABLE} ... +*/ + } +/* * Do the ugly legacy mode stuff here rather than broken chip * quirk code. Legacy mode ATA controllers have fixed
@@ -848,6 +862,7 @@ pci_scan_device(struct pci_bus *bus, int dev->bus = bus; dev->sysdata = bus->sysdata; +device_initialize (&dev->dev); dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; dev->devfn = devfn;
@@ -871,7 +886,6 @@ pci_scan_device(struct pci_bus *bus, intvoid __devinit pci_device_add (struct pci_dev *dev,struct pci_bus *bus) { -device_initialize (&dev->dev); dev->dev.release = pci_release_dev;pci_dev_get (dev);
drivers/pci/pci.c +4 -0
@@ -817,6 +817,10 @@ int pci_enable_wake(struct pci_dev *dev,if (!pm)return enable ? -EIO :0 ; +/* don't enable unless policy set through driver core allows it */ +if (!device_may_wakeup (&dev->dev) && enable) +return -EROFS ; +/* Check device's ability to generate PME# */ pci_read_config_word (dev,pm+PCI_PM_PMC ,&value);
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Quoted 11 lines · David Brownell
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
I don't know, it just really annoys me to see that power directory there with no use for it for a lot of devices :)
Would it help to add a flag somewhere in struct device (or struct
dev_pm_info) for indicating that the device is not cognizant of PM?If it would, the semantics would need to be clarified. Is the problem that they can't suspend/resume at all? Or is the problem instead that they (or their drivers) don't know how to do that without any loss of functionality ... without a complete reset or whatever?
The semantics I intended was that the flag should indicate the driver was completely ignorant of PM. It has no suspend or resume methods and no way of enabling or disabling remote wakeup. As is true, for example, of those USB endpoint pseudo-devices.
Maybe this means the flag belongs in the driver's structure instead of the device's. But the PM core tries to avoid looking at drivers...
Quoted 11 lines
Most devices can -- if poked and prodded appropriately, and if no BIOS or uploaded firmware interferes badly enough -- make it through some kind of system suspend state. Whether that's the one the user has chosen ... different issue! And whether the driver knows how to do that ... also a different issue!
But the specific problems we're seeing right now with the USB autosuspend stuff relate to _runtime_ suspend. I believe some of them are just bugs in the stack; where better coddling of the devices would let them make it through unscathed. But some of them, likewise, probably _do_ have real problems with real runtime suspend states (that don't map to "off").
That would be a separate issue. For now there's a (large?) set of devices whose drivers don't know Power Management from a hole in the wall.
Alan Stern
Am Mittwoch, 24. Januar 2007 17:51 schrieb Alan Stern:
Quoted 7 lines
Quoted 2 lines
Hm, I missed that. This seems to be a very usb-specific thing at the moment.
It is USB-specific because I wanted to start out in a well-defined environment where the mechanisms could get tested and shaken out. When the problems are gone, it will make sense to think about moving the whole autosuspend infrastructure into the PM core.
USB has some advantages. There's only one level of suspend, which is supposed to be mandatory. In addition, it's important for PM because USB always means DMA.
Regards
Oliver
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel