driver-core: platform: automatically mark wakeup devices
PATCH 1/1 driver-core: platform: automatically mark wakeup devices
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/base/platform.c +8 -1
@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup); ret = drv->probe (dev); -if (ret) +if (ret) { +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true); + } }if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER ) {
@@ -540,6 +545,8 @@ static int platform_drv_remove(struct device *_dev)if (drv->remove) ret = drv->remove (dev); + +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true);return ret;
Signature · 5 lines
2.6.0.rc2.230.g3dd15c0 -- Dmitry
Greg Kroah-Hartman
maintainerQuoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
How much boilerplate code can be removed? Do you have an example patch of this removal for any drivers if we move this logic into the driver core?
thanks,
greg k-h
Quoted 9 lines · Greg Kroah-Hartman
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
How much boilerplate code can be removed? Do you have an example patch of this removal for any drivers if we move this logic into the driver core?
Admittedly not a lot, a few lines. There is a couple of lines for checking the property and calling device_init_wakeup() and also sometimes clearing wakeup flag is the only thing that is left in remove() method after converting to devm*. I am more interested in standardizing on the property name and having wakeup flag cleared on removal or probe failure, similarly how we do it for driver data in device structure.
I do not have good patches in input at the moment as even though we are using "wakeup-source" now lots if the drivers did not start with it and so we have compatibility parsing still that we want to keep around. I want the new drivers to use only this property though.
FWIW I2C bus code implements automatic parsing of this property as well and I wonder if we want to do the same for SPI.
Thanks.
Signature · 1 line
Dmitry
Quoted 19 lines · Dmitry Torokhov
Quoted 9 lines · Greg Kroah-Hartman
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
How much boilerplate code can be removed? Do you have an example patch of this removal for any drivers if we move this logic into the driver core?
Admittedly not a lot, a few lines. There is a couple of lines for checking the property and calling
device_init_wakeup()and also sometimes clearing wakeup flag is the only thing that is left inremove()method after converting to devm*. I am more interested in standardizing on the property name and having wakeup flag cleared on removal or probe failure, similarly how we do it for driver data in device structure.
Quoted 5 lines
I do not have good patches in input at the moment as even though we are using "wakeup-source" now lots if the drivers did not start with it and so we have compatibility parsing still that we want to keep around. I want the new drivers to use only this property though.
True. We can move the standard property to driver core while legacy compatible match can be retained in the individual drivers. The list[1] is not too bad though,
and more over it remain static from now on.
--
Regards,
Sudeep
[1] Documentation/devicetree/bindings/power/wakeup-source.txt
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
Quoted 10 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0
drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);
But I'm wondering if this should be device_set_wakeup_capable(dev, true) rather?
device_init_wakeup() additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.
Quoted 19 lines
drivers/base/platform.c +5
-1ret = drv->probe (dev); -if (ret) +if (ret) { +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true); + } }if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER ) {@@ -540,6 +545,8 @@ static int platform_drv_remove(struct device *_dev)if (drv->remove) ret = drv->remove (dev); + +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true);return ret;
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Quoted 11 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
Just curious to know what you mean when you say this property can also be used with ACPI. Do you mean we could use "wakeup-source" DSD ?
If so, won't that go against rule for DSD (i.e we *should not* bypass the existing mechanisms defined by the ACPI, e.g. _SxW in this case)
--
Regards,
Sudeep
Quoted 8 lines · Sudeep Holla
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 4 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
Just curious to know what you mean when you say this property can also be used with ACPI. Do you mean we could use "wakeup-source" DSD ?
Yes.
Quoted 2 lines
If so, won't that go against rule for DSD (i.e we *should not* bypass the existing mechanisms defined by the ACPI, e.g.
_SxWin this case)
Not necessarily.
What if the device doesn't use ACPI PM and still can wake up the system?
Thanks,
Rafael
Quoted 17 lines · Rafael J. Wysocki
Quoted 8 lines · Sudeep Holla
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 4 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
Just curious to know what you mean when you say this property can also be used with ACPI. Do you mean we could use "wakeup-source" DSD ?
Quoted 9 lines
Yes.
Quoted 2 lines
If so, won't that go against rule for DSD (i.e we *should not* bypass the existing mechanisms defined by the ACPI, e.g.
_SxWin this case)Not necessarily.
What if the device doesn't use ACPI PM and still can wake up the system?
May be I don't understand the exact configuration you are referring, but if you don't use ACPI PM, how will that system enter sleep states ? IIUC you are referring suspend-to-idle case only here which doesn't require any ACPI _Sx methods, which make sense.
But still I don't see a strong reason to provide an alternate method to specify the same information. Firmware responsible for ACPI tables have to specify this DSD in question, instead it can use the existing mechanism in ACPI.
Let me know if I am missing some information about the systems you are referring ?
Signature · 2 lines
Regards, Sudeep
Rafael J. Wysocki
maintainerQuoted 37 lines · Sudeep Holla
Quoted 34 lines · Rafael J. Wysocki
Quoted 15 lines · Sudeep Holla
Quoted 12 lines · Rafael J. Wysocki
Quoted 8 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static
platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this
property and reduce amount of boilerplate code in the drivers.Quoted 7 lines
Quoted 4 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
Just curious to know what you mean when you say this property can also be used with ACPI. Do you mean we could use "wakeup-source" DSD ?
Yes.
Quoted 2 lines
If so, won't that go against rule for DSD (i.e we *should not* bypass the existing mechanisms defined by the ACPI, e.g.
_SxWin this case)Not necessarily.
Quoted 7 lines
Quoted 2 lines
What if the device doesn't use ACPI PM and still can wake up the system?
May be I don't understand the exact configuration you are referring, but if you don't use ACPI PM, how will that system enter sleep states ? IIUC you are referring suspend-to-idle case only here which doesn't require any ACPI
_Sxmethods, which make sense.
That plus PM on HW-reduced ACPI platforms where GPIOs can be used for system wakeup in principle (although admittedly I have no experience with such platforms, so this is just a theory).
Quoted 4 lines
But still I don't see a strong reason to provide an alternate method to specify the same information. Firmware responsible for ACPI tables have to specify this DSD in question, instead it can use the existing mechanism in ACPI.
We actually don't use _SxW to determine whether or not devices are wakeup-capable. We look at _PRW and we set "wakeup-capable" if it is present and the contents is valid.
Quoted 2 lines
Let me know if I am missing some information about the systems you are referring ?
That said I'm actually quite unsure about the property suggested by Dmitry, but let me reply to him on that. :-)
Thanks,
Rafael
Quoted 10 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.
Quoted 20 lines
Quoted 11 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?
Quoted 2 lines
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.
Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources? This is how most of the drivers I am involved in behave. All of them use device_init_wakeup() and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).
Quoted 12 lines
Quoted 12 lines
drivers/base/platform.c +3
-1ret = drv->probe (dev); -if (ret) +if (ret) { +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true); + } }if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER ) {@@ -540,6 +545,8 @@ static int platform_drv_remove(struct device *_dev)
Quoted 9 lines
drivers/base/platform.c static int platform_drv_remove(struct device *_dev) · line 552
Quoted 8 lines
drivers/base/platform.c +2
-0if (drv->remove) ret = drv->remove (dev); + +device_init_wakeup (_dev, false);dev_pm_domain_detach (_dev, true);return ret;
Thanks.
Signature · 1 line
Dmitry
Rafael J. Wysocki
maintainerQuoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 3 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
That is not really going to happen any time soon AFAICS.
Quoted 6 lines
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.
First, they shouldn't need to do the last part on ACPI systems (where they should be belong to the ACPI PM domain), or can look up the relevant information in the devices' ACPI companions.
Second, I'm quite unsure about the "wakeup-capable" property, because I'm not sure what it is really supposed to mean.
For the suspend-to-idle case all devices that can generate interrupts can potentially wake up the system if set up for that during suspend.
For deeper system sleep modes (in which power is removed from interrupt controllers) there needs to be a special way to set up the device for system wakeup that should be described by the firmware. Without that, saying that the device is "wakeup-capable" alone is pretty much meaningless.
Quoted 19 lines
Quoted 19 lines
Quoted 10 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?
Quoted 5 lines
Quoted 2 lines
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources?
Because user space may not want to use them for system wakeup?
Quoted 5 lines
This is how most of the drivers I am involved in behave. All of them use
device_init_wakeup()and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).
Plus there is the sysfs attribute that needs to be taken into account (so user space can disable wakeup devices it doesn't want to be used).
But it is fine if device_init_wakeup() is used by a driver from the start (which only means that its default value for the above-mentioned sysfs attribute is "enabled"), but using that in the core may change the default for some drivers and lead to problems.
Thanks,
Rafael
Quoted 27 lines · Rafael J. Wysocki
Quoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 3 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
That is not really going to happen any time soon AFAICS.
Quoted 6 lines
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.
Quoted 3 lines
First, they shouldn't need to do the last part on ACPI systems (where they should be belong to the ACPI PM domain), or can look up the relevant information in the devices' ACPI companions.
That is the point. We do not want to look into ACPI components, or OF, or platform data. We want to have unified way of setting the device as wakeup capable and enabled, preferably by the bus or device core.
Quoted 3 lines
Second, I'm quite unsure about the "wakeup-capable" property, because I'm not sure what it is really supposed to mean.
On OF system it means that platform integrator decided that this device is supposed to be waking up the particular system and the platform and device driver should perform whatever steps are necessary to wake up the system. The same behavior we also have with drivers supporting legacy platform data (usually expressed as pdata->wakeup). ACPI is an outlier here.
Quoted 3 lines
For the suspend-to-idle case all devices that can generate interrupts can potentially wake up the system if set up for that during suspend.
You need to tell the driver to not put the device in "too deep" sleep mode though so the interrupts can still be generated.
Quoted 6 lines
For deeper system sleep modes (in which power is removed from interrupt controllers) there needs to be a special way to set up the device for system wakeup that should be described by the firmware. Without that, saying that the device is "wakeup-capable" alone is pretty much meaningless.
Right, and most of the drivers have the following in their PM code paths:
if (device_wakeup_enabled(dev)) {
...
enable driver's "light" sleep
...
enable_irq_wake();
} else { ...
enable deep sleep
...
}
or something similar. The point is that at this time we do not check for ACPI, or DT, or whatever but rely on single call to device_may_wakeup().
Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 11 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources?
Because user space may not want to use them for system wakeup?
Then userspace will disable it. The point is that platform integrator intended for the device to be wakeup source and so it should be enabled by default for such devices.
Quoted 9 lines
Quoted 6 lines
This is how most of the drivers I am involved in behave. All of them use
device_init_wakeup()and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).Plus there is the sysfs attribute that needs to be taken into account (so user space can disable wakeup devices it doesn't want to be used).
It is already taken into account as drivers are supposed to be using device_may_wakeup() to check if wakeup is enabled at time of power state transition.
Quoted 4 lines
But it is fine if
device_init_wakeup()is used by a driver from the start (which only means that its default value for the above-mentioned sysfs attribute is "enabled"), but using that in the core may change the default for some drivers and lead to problems.
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Thanks.
Signature · 1 line
Dmitry
Rafael J. Wysocki
maintainerQuoted 28 lines · Dmitry Torokhov
Quoted 27 lines · Rafael J. Wysocki
Quoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 3 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
That is not really going to happen any time soon AFAICS.
Quoted 6 lines
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.
Quoted 7 lines
Quoted 3 lines
First, they shouldn't need to do the last part on ACPI systems (where they should be belong to the ACPI PM domain), or can look up the relevant information in the devices' ACPI companions.
That is the point. We do not want to look into ACPI components, or OF, or platform data. We want to have unified way of setting the device as wakeup capable and enabled, preferably by the bus or device core.
On a properly set up ACPI system, the ACPI layer should take care of this.
The driver only has to look at device_may_wakeup() and do some device-specific setup (if necessary) for system wakeup during system suspend if that returns 'true'.
IMO, on a non-ACPI system it should work exactly the same way. That is, the platform/board code should take care of whatever needs to be done except for the device-specific setup.
Quoted 7 lines
Quoted 2 lines
Second, I'm quite unsure about the "wakeup-capable" property, because I'm not sure what it is really supposed to mean.
On OF system it means that platform integrator decided that this device is supposed to be waking up the particular system and the platform and device driver should perform whatever steps are necessary to wake up the system.
This really sounds like "I wish the device woke up the system and you go figure out how to make that happen". Not very useful to me, to be honest.
Let me repeat what I said below: Without specifying how to set up the device for system wakeup, using the "wakeup-capable" property is meaningless. However, if it is specified how to do that, the property is simply redundant, because the wakeup capability obviously follows from the fact that there is a way to set up the device for system wakeup.
Quoted 3 lines
The same behavior we also have with drivers
supporting legacy platform data (usually expressed aspdata->wakeup). ACPI is an outlier here.
Which doesn't mean that it is wrong and everybody else is right.
Quoted 5 lines
Quoted 2 lines
For the suspend-to-idle case all devices that can generate interrupts can potentially wake up the system if set up for that during suspend.
You need to tell the driver to not put the device in "too deep" sleep mode though so the interrupts can still be generated.
Checking device_may_wakeup() should help here, though.
Quoted 15 lines
Quoted 6 lines
For deeper system sleep modes (in which power is removed from interrupt controllers) there needs to be a special way to set up the device for system wakeup that should be described by the firmware. Without that, saying that the device is "wakeup-capable" alone is pretty much meaningless.
Right, and most of the drivers have the following in their PM code paths:
if (device_wakeup_enabled(dev)) { ... enable driver's "light" sleep ... enable_irq_wake(); } else {
Quoted 4 lines
...
enable deep sleep
...
}
OK, so this appears to indicate that the "wakeup-capable" property really is supposed to mean "the in-band interrupt of this device can wake up the system if set up properly", which is far more precise than the interpretation of it mentioned so far. Is that the case?
Quoted 4 lines
or something similar. The point is that at this time we do not check for ACPI, or DT, or whatever but rely on single call to
device_may_wakeup().
Well, device_may_wakeup() need not imply that you're supposed to set up the in-band interrupt of the device for system wakeup, really.
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Quoted 10 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources?
Because user space may not want to use them for system wakeup?
Then userspace will disable it. The point is that platform integrator intended for the device to be wakeup source and so it should be enabled by default for such devices.
Whatever the system integrator intended need not matter to the driver core and can you actually guarantee that "wakeup-capable" will always be used correctly by all platform firmware?
And what about devices that generate spurious wakeups that will be magically enabled by this change?
Quoted 13 lines
Quoted 8 lines
Quoted 5 lines
This is how most of the drivers I am involved in behave. All of them use
device_init_wakeup()and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).Plus there is the sysfs attribute that needs to be taken into account (so user space can disable wakeup devices it doesn't want to be used).
It is already taken into account as drivers are supposed to be using
device_may_wakeup()to check if wakeup is enabled at time of power state transition.
Quoted 7 lines
Quoted 4 lines
But it is fine if
device_init_wakeup()is used by a driver from the start (which only means that its default value for the above-mentioned sysfs attribute is "enabled"), but using that in the core may change the default for some drivers and lead to problems.I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Thanks,
Rafael
Quoted 34 lines · Rafael J. Wysocki
Quoted 32 lines · Dmitry Torokhov
Quoted 30 lines · Rafael J. Wysocki
Quoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 3 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
That is not really going to happen any time soon AFAICS.
Quoted 6 lines
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.First, they shouldn't need to do the last part on ACPI systems (where they should be belong to the ACPI PM domain), or can look up the relevant information in the devices' ACPI companions.
Quoted 5 lines
Quoted 3 lines
That is the point. We do not want to look into ACPI components, or OF, or platform data. We want to have unified way of setting the device as wakeup capable and enabled, preferably by the bus or device core.
On a properly set up ACPI system, the ACPI layer should take care of this.
You mean marking device as wakeup-capable? Good, then this patch will likely be a noop on ACPI systems. However can you please point me how we do that, for example, for I2C devices instantiated via ACPI?
Quoted 4 lines
The driver only has to look at
device_may_wakeup()and do some device-specific setup (if necessary) for system wakeup during system suspend if that returns 'true'.
Right.
Quoted 4 lines
IMO, on a non-ACPI system it should work exactly the same way. That is, the platform/board code should take care of whatever needs to be done except for the device-specific setup.
Yes and no. Like with PM domains, we now have generic property API (that is implemented by platform code in platform-specific way) so we can just query necessary properties in needed places, such as various buses code.
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Second, I'm quite unsure about the "wakeup-capable" property, because I'm not sure what it is really supposed to mean.
Quoted 8 lines
Quoted 4 lines
On OF system it means that platform integrator decided that this device is supposed to be waking up the particular system and the platform and device driver should perform whatever steps are necessary to wake up the system.
This really sounds like "I wish the device woke up the system and you go figure out how to make that happen". Not very useful to me, to be honest.
No, not really. It is like "I want this device to wake up system and driver is supposed to know how to configure this device for wakeup". Right now drivers parse this attribute themselves, I'd like to centralize it.
Quoted 7 lines
Let me repeat what I said below: Without specifying how to set up the device for system wakeup, using the "wakeup-capable" property is meaningless. However, if it is specified how to do that, the property is simply redundant, because the wakeup capability obviously follows from the fact that there is a way to set up the device for system wakeup.
It is not. You need to tell the driver that on given board:
1. Wakeup by given device can be enabled (hardware is wired properly)
2. Wakeup by given device should be enabled
On both OF and static boards there usually no separate annotations for the above, they all rolled into single "wakeup-source" property.
Quoted 8 lines
Quoted 4 lines
The same behavior we also have with drivers
supporting legacy platform data (usually expressed aspdata->wakeup). ACPI is an outlier here.Which doesn't mean that it is wrong and everybody else is right.
From the driver's point of view it is wrong in the sense that it makes
them implement ACPI-specific behavior and code paths.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
For the suspend-to-idle case all devices that can generate interrupts can potentially wake up the system if set up for that during suspend.
You need to tell the driver to not put the device in "too deep" sleep mode though so the interrupts can still be generated.
Checking
device_may_wakeup()should help here, though.
And that's what everyone does. But somebody *has to set* this flag.
Quoted 21 lines
Quoted 16 lines
Quoted 7 lines
For deeper system sleep modes (in which power is removed from interrupt controllers) there needs to be a special way to set up the device for system wakeup that should be described by the firmware. Without that, saying that the device is "wakeup-capable" alone is pretty much meaningless.
Right, and most of the drivers have the following in their PM code paths:
if (device_wakeup_enabled(dev)) { ... enable driver's "light" sleep ... enable_irq_wake(); } else {Quoted 4 lines
...
enable deep sleep
...
}
Quoted 4 lines
OK, so this appears to indicate that the "wakeup-capable" property really is supposed to mean "the in-band interrupt of this device can wake up the system if set up properly", which is far more precise than the interpretation of it mentioned so far. Is that the case?
No, not really. It does not say anything about in band interrupt or anything else. It depends on the driver.
Quoted 8 lines
Quoted 5 lines
or something similar. The point is that at this time we do not check for ACPI, or DT, or whatever but rely on single call to
device_may_wakeup().Well,
device_may_wakeup()need not imply that you're supposed to set up the in-band interrupt of the device for system wakeup, really.
I never said it did imply that.
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Quoted 25 lines
Quoted 20 lines
Quoted 11 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?Quoted 2 lines
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources?
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Because user space may not want to use them for system wakeup?
Then userspace will disable it. The point is that platform integrator intended for the device to be wakeup source and so it should be enabled by default for such devices.
Whatever the system integrator intended need not matter to the driver core
Huh? Should we ignore ACPI-provided interrupt and address assignments? Because they are set by platform integrators.
Quoted 2 lines
and can you actually guarantee that "wakeup-capable" will always
be used correctly by all platform firmware?
Can you guarantee that all ACPI code ever produced is perfect and doe snot do stupid things?
Quoted 3 lines
And what about devices that generate spurious wakeups that will be magically enabled by this change?
Maybe that could happen on ACPI systems, but see below.
Quoted 19 lines
Quoted 19 lines
Quoted 9 lines
Quoted 6 lines
This is how most of the drivers I am involved in behave. All of them use
device_init_wakeup()and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).Plus there is the sysfs attribute that needs to be taken into account (so user space can disable wakeup devices it doesn't want to be used).
It is already taken into account as drivers are supposed to be using
device_may_wakeup()to check if wakeup is enabled at time of power state transition.Quoted 4 lines
But it is fine if
device_init_wakeup()is used by a driver from the start (which only means that its default value for the above-mentioned sysfs attribute is "enabled"), but using that in the core may change the default for some drivers and lead to problems.
Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
Thanks.
Signature · 1 line
Dmitry
Rafael J. Wysocki
maintainerQuoted 39 lines · Dmitry Torokhov
Quoted 38 lines · Rafael J. Wysocki
Quoted 35 lines · Dmitry Torokhov
Quoted 27 lines · Rafael J. Wysocki
Quoted 8 lines · Dmitry Torokhov
Quoted 7 lines · Rafael J. Wysocki
Quoted 5 lines · Dmitry Torokhov
When probing platform drivers let's check if corresponding devices have "wakeup-source" property defined (either in device tree, ACPI, or static platform properties) and automatically enable such devices as wakeup sources for the system. This will help us standardize on the name for this property and reduce amount of boilerplate code in the drivers.
Quoted 7 lines
Quoted 3 lines
ACPI has other ways of telling the OS that the device is wakeup-capable, but I guess the property in question can be used too (as long as it is consistent with the other methods).
I was thinking that down the road ACPI can wire its internal wakeup knowledge into generic device property. Unfortunately at the moment most drivers go like this:
That is not really going to happen any time soon AFAICS.
Quoted 6 lines
- if it is device tree platform: good, we'll take the data from device tree property and set up driver behavior accordingly;
- if it is legacy board setup: OK, take for driver-specific platform data;
- ACPI... hmm... dunno... enable wakeup and if it is not correct the system will just not wake up, not the best but not terribly wrong either.Quoted 3 lines
First, they shouldn't need to do the last part on ACPI systems (where they should be belong to the ACPI PM domain), or can look up the relevant information in the devices' ACPI companions.
That is the point. We do not want to look into ACPI components, or OF, or platform data. We want to have unified way of setting the device as wakeup capable and enabled, preferably by the bus or device core.
Quoted 5 lines
Quoted 1 line
On a properly set up ACPI system, the ACPI layer should take care of this.
You mean marking device as wakeup-capable? Good, then this patch will likely be a noop on ACPI systems. However can you please point me how we do that, for example, for I2C devices instantiated via ACPI?
First, acpi_bind_one() associates the device with its ACPI companion (this happens during its registration via the ->platform_notify callback). In the last step it looks at the ACPI companion's flags and calls device_set_wakeup_capable(dev, true) if wakeup support is indicated.
Next, i2c_device_probe() -> dev_pm_domain_attach() -> acpi_dev_pm_attach() adds the device to acpi_general_pm_domain which has appropriate PM callbacks that take care of setting up wakeup among other things.
Caveat: this generally won't work on HW-reduced ACPI systems where there are no GPEs.
Quoted 8 lines
Quoted 4 lines
The driver only has to look at
device_may_wakeup()and do some device-specific setup (if necessary) for system wakeup during system suspend if that returns 'true'.Right.
Quoted 1 line
Quoted 7 lines
Quoted 3 lines
IMO, on a non-ACPI system it should work exactly the same way. That is, the platform/board code should take care of whatever needs to be done except for the device-specific setup.
Yes and no. Like with PM domains, we now have generic property API (that is implemented by platform code in platform-specific way) so we can just query necessary properties in needed places, such as various buses code.
Well, say platform A has a separate dedicated wakeup IRQ line for each device and platform B uses in-band interrupts for system wakeup with all devices.
What you're saying seems to mean that the exact interpretation of "wakeup-capable" would depend on the platform in question.
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
Second, I'm quite unsure about the "wakeup-capable" property, because I'm not sure what it is really supposed to mean.
On OF system it means that platform integrator decided that this device is supposed to be waking up the particular system and the platform and device driver should perform whatever steps are necessary to wake up the system.
Quoted 6 lines
Quoted 3 lines
This really sounds like "I wish the device woke up the system and you go figure out how to make that happen". Not very useful to me, to be honest.
No, not really. It is like "I want this device to wake up system and driver is supposed to know how to configure this device for wakeup".
How is the driver supposed to know that in practice? Is every driver supposed to contain some platform knowledge about every platform it is intended to work with?
Quoted 1 line
Right now drivers parse this attribute themselves, I'd like to centralize it.
Oh I'm all for unification of things, but in a way I can understand. :-)
Device wakeup setup necessarily consists of two parts, device-specific and platform-specific or bus-specific (for buses whose protocols define that). On ACPI platforms (full HW ACPI at least) the last bit is taken care of by ACPI. On non-ACPI platforms there are two ways this may be addressed. First, there may be a wakeup setup method described by platform firmware or equivalent, like "in-band device interrupt should be used for system wakeup". Second, there may be platform-specific code in the kernel that will know how to carry out the thing.
In the second case "wakeup-capable" may be interpreted as "the kernel has some code which knows how to set up that thing and it should work for this device". In the first case it won't be sufficient, though, so I'm guessing that we're talking about the second case?
Quoted 10 lines
Quoted 6 lines
Let me repeat what I said below: Without specifying how to set up the device for system wakeup, using the "wakeup-capable" property is meaningless. However, if it is specified how to do that, the property is simply redundant, because the wakeup capability obviously follows from the fact that there is a way to set up the device for system wakeup.
It is not. You need to tell the driver that on given board:
1. Wakeup by given device can be enabled (hardware is wired properly)
The information that the hardware is wired up properly is something that's good to know, but insufficient if you need to know *how* to make the wakeup actually happen. However, if you are told "this is how to make the wakeup happen", then you know that the hardware is wired up properly too. Which is exactly my point.
Quoted 1 line
2. Wakeup by given device should be enabled
I'm reading this as "If you're unsure what the default should be, system wakeup enabled is the right choice". Fair enough.
Quoted 11 lines
On both OF and static boards there usually no separate annotations for the above, they all rolled into single "wakeup-source" property.
Quoted 5 lines
Quoted 3 lines
The same behavior we also have with drivers
supporting legacy platform data (usually expressed aspdata->wakeup). ACPI is an outlier here.Which doesn't mean that it is wrong and everybody else is right.
From the driver's point of view it is wrong in the sense that it makes them implement ACPI-specific behavior and code paths.
Whereas they don't have to that. All of it is supposed to be transparent.
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
For the suspend-to-idle case all devices that can generate interrupts can potentially wake up the system if set up for that during suspend.
You need to tell the driver to not put the device in "too deep" sleep mode though so the interrupts can still be generated.
Checking
device_may_wakeup()should help here, though.And that's what everyone does. But somebody *has to set* this flag.
On what basis?
If the kernel has some platform-specific code that knows how to carry out platform-specific parts of wakuep preparation, the property should really be parsed by that code and not by the core, because the core a priori doesn't know whether or not that code is present and really working.
If the kernel doesn't have any platform-specific code like that, the property also should specify how the platform-specific parts of wakeup preparation are supposed to be carried out.
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines
Quoted 5 lines
For deeper system sleep modes (in which power is removed from interrupt controllers) there needs to be a special way to set up the device for system wakeup that should be described by the firmware. Without that, saying that the device is "wakeup-capable" alone is pretty much meaningless.
Right, and most of the drivers have the following in their PM code paths:
if (device_wakeup_enabled(dev)) { ... enable driver's "light" sleep ... enable_irq_wake(); } else {...
enable deep sleep
...
}
Quoted 7 lines
Quoted 4 lines
OK, so this appears to indicate that the "wakeup-capable" property really is supposed to mean "the in-band interrupt of this device can wake up the system if set up properly", which is far more precise than the interpretation of it mentioned so far. Is that the case?
No, not really. It does not say anything about in band interrupt or anything else. It depends on the driver.
How so? The driver should be able to run on multiple platforms with different ways to set up system wakeup.
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
or something similar. The point is that at this time we do not check for ACPI, or DT, or whatever but rely on single call to
device_may_wakeup().Well,
device_may_wakeup()need not imply that you're supposed to set up the in-band interrupt of the device for system wakeup, really.I never said it did imply that.
The piece of pseudo code above indicates that. Or that there is a dedicated wakeup interrupt for the device which the driver knows about in some magical way.
Quoted 31 lines
Quoted 31 lines
Quoted 30 lines
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 10 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/base/platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/base/platform.c +0
-0drivers/base/platform.c static int platform_drv_probe(struct device *_dev) · line 519
Quoted 6 lines
drivers/base/platform.c +3
-0@@ -514,9 +514,14 @@ static int platform_drv_probe(struct device *_dev) ret =dev_pm_domain_attach (_dev, true);if (ret != -EPROBE_DEFER && drv->probe) { +bool wakeup =device_property_read_bool (_dev,"wakeup-source" ); + +device_init_wakeup (_dev, wakeup);But I'm wondering if this should be
device_set_wakeup_capable(dev, true)rather?Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
device_init_wakeup()additionally sets the default in sysfs to "do wakeup" which in principle may unblock spurious wakeups on some systems.Why would we not want enable wakeup for devices where system (platform) tells us that they are wakeup sources?
Because user space may not want to use them for system wakeup?
Then userspace will disable it. The point is that platform integrator intended for the device to be wakeup source and so it should be enabled by default for such devices.
Quoted 5 lines
Quoted 2 lines
Whatever the system integrator intended need not matter to the driver core
Huh? Should we ignore ACPI-provided interrupt and address assignments? Because they are set by platform integrators.
They are configuration data, not information on what defaults to use for certain features.
Also we sometimes ignore them too if we have better ways to figure things out.
Quoted 5 lines
Quoted 2 lines
and can you actually guarantee that "wakeup-capable" will always
be used correctly by all platform firmware?Can you guarantee that all ACPI code ever produced is perfect and doe snot do stupid things?
Obviously not and I really only want it to be used where there are no other means to gather the necessary information, exactly for this reason.
Quoted 4 lines
Quoted 2 lines
And what about devices that generate spurious wakeups that will be magically enabled by this change?
Maybe that could happen on ACPI systems, but see below.
Oh it does happen there.
Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
Quoted 8 lines
Quoted 5 lines
This is how most of the drivers I am involved in behave. All of them use
device_init_wakeup()and then we adjust the behavior depending on runtime state, i.e. do not wake up from touchpad when lid is closed (which is useful if lid is flexible and may interact with touchpad if slammed down hard enough).Plus there is the sysfs attribute that needs to be taken into account (so user space can disable wakeup devices it doesn't want to be used).
It is already taken into account as drivers are supposed to be using
device_may_wakeup()to check if wakeup is enabled at time of power state transition.Quoted 4 lines
But it is fine if
device_init_wakeup()is used by a driver from the start (which only means that its default value for the above-mentioned sysfs attribute is "enabled"), but using that in the core may change the default for some drivers and lead to problems.Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Quoted 7 lines
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
The defaults to use should generally depend on two things IMO: On what the firmware tells us to use (given the argument that the firmware people probably have a good reason to tell us what they are telling us) and on what we were doing for this class of devices in the past. Ignoring the last bit generally leads to regressions in the cases when the firmware people are wrong after all, so I'm always cautious about this. And this rule applies to DT as well as to ACPI, although the DT people usually won't admit it. :-)
Thanks,
Rafael
Rafael J. Wysocki
maintainerQuoted 2 lines · Rafael J. Wysocki
On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov
<[email protected]> wrote:
[cut]
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Quoted 7 lines
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
Quoted 8 lines
The defaults to use should generally depend on two things IMO: On what the firmware tells us to use (given the argument that the firmware people probably have a good reason to tell us what they are telling us) and on what we were doing for this class of devices in the past. Ignoring the last bit generally leads to regressions in the cases when the firmware people are wrong after all, so I'm always cautious about this. And this rule applies to DT as well as to ACPI, although the DT people usually won't admit it. :-)
OK, I realize that the example I gave was not really a good one, because the wakeup part was added to i8042 to support wakeup from suspend-to-idle which is really special and the implementation is based on what the driver was doing previously.
But this also makes me think that the problem is really more complicated than it may seem to be, so what about starting over and looking at the wakeup problem in general?
To that end, there are two categories of wakeup support in devices. The first one is support for something called "remote wakeup" in USB and which means that the device is able to generate wakeup signals when it (the device) is suspended and the rest of the system can generally be considered as working. I'll use the "remote wakeup" term in general for the lack of a better one.
Remote wakeup support is used in runtime PM and suspend-to-idle (which essentially uses the same kind of hardware mechanics, but in a different way).
Note, though, that "device is suspended" need not map 1:1 onto a particular hardware state. What it really means is that either it has been suspended using the runtime PM framework, or all devices have gone through the device suspend sequence during suspend-to-idle. The hardware state the device ends up in depends on the driver/bus type/PM domain combination and is generally expected to be low-power.
It is easy in PCI or USB and on ACPI systems where low-power states of devices are well defined and the connection between them and the ability to generate wakeup signals is known. It is not so easy in the other cases, though. My personal opinion is that if wakeup support is required, the device should be put into the lowest-power state from which it can generate wakeup signals. That very well may be the full-power state of it if that's the only suitable one.
If that point of view is adopted, then any device that (a) can take input and (b) uses interrupts can do remote wakeup. We don't really have a good way to express that in the driver core.
The second category of wakeup support is for platform-assisted low-power states of the whole system where there's a big switch (or a bunch of them) allowing multiple things to be powered off in one go from the OS perspective.
I'll write about that in the next message, as I need to take care of some urgent stuff now.
Thanks,
Rafael
Rafael J. Wysocki
maintainerQuoted 32 lines · Rafael J. Wysocki
Quoted 2 lines · Rafael J. Wysocki
On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov
<[email protected]> wrote:[cut]
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Quoted 7 lines
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
Quoted 8 lines
The defaults to use should generally depend on two things IMO: On what the firmware tells us to use (given the argument that the firmware people probably have a good reason to tell us what they are telling us) and on what we were doing for this class of devices in the past. Ignoring the last bit generally leads to regressions in the cases when the firmware people are wrong after all, so I'm always cautious about this. And this rule applies to DT as well as to ACPI, although the DT people usually won't admit it. :-)
Quoted 4 lines
OK, I realize that the example I gave was not really a good one, because the wakeup part was added to i8042 to support wakeup from suspend-to-idle which is really special and the implementation is based on what the driver was doing previously.
Which is not to say that there are no good (or at least better) examples.
USB HID devices pretty much universally support remote wakeup officially, but enough of them cause problems to happen while using it (spurious wakeups etc) that we don't enable them for system wakeup by default. Plus some of them are cordless mice and such and you really need to be careful to avoid waking up a sleeping system by accident with them (when enabled).
Ethernet NICs support wakeup signaling, but at least some of them are not enabled for system wakeup by default. Moreover, there are multiple ways to trigger the wakeup there that the user needs to choose from in the first place.
Some wireless adapters support WoW, but enabling them to wake up the system from sleep by default would be asking for troubles.
All in all, combining the information that the device can signal wakeup with the requirement to enable it to wake up the system from sleep states by default doesn't look like a good idea to me.
Quoted 34 lines
But this also makes me think that the problem is really more complicated than it may seem to be, so what about starting over and looking at the wakeup problem in general?
To that end, there are two categories of wakeup support in devices. The first one is support for something called "remote wakeup" in USB and which means that the device is able to generate wakeup signals when it (the device) is suspended and the rest of the system can generally be considered as working. I'll use the "remote wakeup" term in general for the lack of a better one.
Remote wakeup support is used in runtime PM and suspend-to-idle (which essentially uses the same kind of hardware mechanics, but in a different way).
Note, though, that "device is suspended" need not map 1:1 onto a particular hardware state. What it really means is that either it has been suspended using the runtime PM framework, or all devices have gone through the device suspend sequence during suspend-to-idle. The hardware state the device ends up in depends on the driver/bus type/PM domain combination and is generally expected to be low-power.
It is easy in PCI or USB and on ACPI systems where low-power states of devices are well defined and the connection between them and the ability to generate wakeup signals is known. It is not so easy in the other cases, though. My personal opinion is that if wakeup support is required, the device should be put into the lowest-power state from which it can generate wakeup signals. That very well may be the full-power state of it if that's the only suitable one.
If that point of view is adopted, then any device that (a) can take input and (b) uses interrupts can do remote wakeup. We don't really have a good way to express that in the driver core.
Quoted 7 lines
The second category of wakeup support is for platform-assisted low-power states of the whole system where there's a big switch (or a bunch of them) allowing multiple things to be powered off in one go from the OS perspective.
I'll write about that in the next message, as I need to take care of some urgent stuff now.
Continuing.
If the device in question belongs to the part of the system powered off by the big switch, it has to be provided with some special "wakeup" power source so it can generate signals. There needs to be a physical line (or bus or similar) that will be activated when wakeup is signaled by the device. The activation of it needs to be noticed by something and turned into a signal that eventually will wake up the CPU (or make it start executing a power-up sequence or equivalent). All of that generally requires specific setup.
The device has to be left in a state in which it can use the "wakeup" power source in the first place. The "wakeup" power source for it has to be turned on. The signal "line" needs to be prepared for activation by the device's wakeup signals. The part of the system responsible for waking up the CPU when that "line" is active has to be prepared too. All this means that generally it is not enough to say "things are wired up properly" for the right stuff to happen.
Traditionally, we set the can_wakeup bit if (a) there is an interface we can use to set up those things (either by accessing some registers available to us or by invoking the platform firmware to do it) and (b) the device is hooked up to that interface in a known way. IOW, that bit is inferred from what we can find out about the device's configuration rather that just taken from the firmware for granted.
Now, this generally may not be the right approach.
Maybe we just don't need the can_wakeup bit at all. Maybe we should just create the "wakeup" sysfs attribute for all devices, let user space set it the way it wants and handle it on the best effort basis. That is, it will wake up from the sleep states it can wake up from, but it may not wake up from any of them if there's no support. Granted, we really can't guarantee that it will work anyway (even if the firmware exposes the interface to us, it may not be correctly implemented etc and there's no way to verify that upfront). And we may allow the same attribute to be used for disabling remote wakeup for runtime PM if someone wants to.
In that case, though, we really won't need the firmware to tell us whether or not the device is "wakeup-capable". We will always treat it this way and if it turns out that something is missing, wakeup will just not work.
Thanks,
Rafael
Quoted 52 lines · Rafael J. Wysocki
Quoted 32 lines · Rafael J. Wysocki
Quoted 2 lines · Rafael J. Wysocki
On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov
<[email protected]> wrote:[cut]
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Quoted 7 lines
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
Quoted 8 lines
The defaults to use should generally depend on two things IMO: On what the firmware tells us to use (given the argument that the firmware people probably have a good reason to tell us what they are telling us) and on what we were doing for this class of devices in the past. Ignoring the last bit generally leads to regressions in the cases when the firmware people are wrong after all, so I'm always cautious about this. And this rule applies to DT as well as to ACPI, although the DT people usually won't admit it. :-)
Quoted 4 lines
OK, I realize that the example I gave was not really a good one, because the wakeup part was added to i8042 to support wakeup from suspend-to-idle which is really special and the implementation is based on what the driver was doing previously.
Which is not to say that there are no good (or at least better) examples.
USB HID devices pretty much universally support remote wakeup officially, but enough of them cause problems to happen while using it (spurious wakeups etc) that we don't enable them for system wakeup by default. Plus some of them are cordless mice and such and you really need to be careful to avoid waking up a sleeping system by accident with them (when enabled).
Ethernet NICs support wakeup signaling, but at least some of them are not enabled for system wakeup by default. Moreover, there are multiple ways to trigger the wakeup there that the user needs to choose from in the first place.
Quoted 6 lines
Some wireless adapters support WoW, but enabling them to wake up the system from sleep by default would be asking for troubles.
All in all, combining the information that the device can signal wakeup with the requirement to enable it to wake up the system from sleep states by default doesn't look like a good idea to me.
You are making a convincing argument here for having an option to allow specifying that device is just wakeup capable, but not enabled by default. For ACPI you have your own ways of defining whether the device is wakeup capable (and you also have some rules to mark certain devices as wakeup enabled by default - button GPEs for example), so that leaves DT and static board files where historically we enabled wakeup for platform/I2C/SPI devices that board believes are wakeup-capable.
I wonder if I should do that property parsing in genpd code, similarly to what you are doing in ACPI power domain. What does not help is that we have different notions of power domain at once - one where device is under control of ACPI and another that denotes grouping of devices together.
Quoted 54 lines
Quoted 35 lines
But this also makes me think that the problem is really more complicated than it may seem to be, so what about starting over and looking at the wakeup problem in general?
To that end, there are two categories of wakeup support in devices. The first one is support for something called "remote wakeup" in USB and which means that the device is able to generate wakeup signals when it (the device) is suspended and the rest of the system can generally be considered as working. I'll use the "remote wakeup" term in general for the lack of a better one.
Remote wakeup support is used in runtime PM and suspend-to-idle (which essentially uses the same kind of hardware mechanics, but in a different way).
Note, though, that "device is suspended" need not map 1:1 onto a particular hardware state. What it really means is that either it has been suspended using the runtime PM framework, or all devices have gone through the device suspend sequence during suspend-to-idle. The hardware state the device ends up in depends on the driver/bus type/PM domain combination and is generally expected to be low-power.
It is easy in PCI or USB and on ACPI systems where low-power states of devices are well defined and the connection between them and the ability to generate wakeup signals is known. It is not so easy in the other cases, though. My personal opinion is that if wakeup support is required, the device should be put into the lowest-power state from which it can generate wakeup signals. That very well may be the full-power state of it if that's the only suitable one.
If that point of view is adopted, then any device that (a) can take input and (b) uses interrupts can do remote wakeup. We don't really have a good way to express that in the driver core.
Quoted 7 lines
The second category of wakeup support is for platform-assisted low-power states of the whole system where there's a big switch (or a bunch of them) allowing multiple things to be powered off in one go from the OS perspective.
I'll write about that in the next message, as I need to take care of some urgent stuff now.
Continuing.
If the device in question belongs to the part of the system powered off by the big switch, it has to be provided with some special "wakeup" power source so it can generate signals. There needs to be a physical line (or bus or similar) that will be activated when wakeup is signaled by the device. The activation of it needs to be noticed by something and turned into a signal that eventually will wake up the CPU (or make it start executing a power-up sequence or equivalent). All of that generally requires specific setup.
Quoted 7 lines
The device has to be left in a state in which it can use the "wakeup" power source in the first place. The "wakeup" power source for it has to be turned on. The signal "line" needs to be prepared for activation by the device's wakeup signals. The part of the system responsible for waking up the CPU when that "line" is active has to be prepared too. All this means that generally it is not enough to say "things are wired up properly" for the right stuff to happen.
There are usually 2 components to this, one from device/driver perspective and another from system perspective. From the device perspective there is not difference between "remote wakeup" and "big switch" wakeup. In both cases the driver should put the device into lowest power state that still allows it to react to external inputs. Putting the device into that power state may also affect other devices it is connected to, for example it may request to power down some regulators and/or stop some clocks if they are not needed in that low power mode. When there is activity the device sends signal (could be dedicated interrupt pin or in-bound interrupt pin - this is device specific) and how they are wired is board-specific.
Quoted 1 line
From the system perspective it provides notion of power domains as in
grouping (so that we know when we can turn off certain supplies and what parts of the system should stay powered up even when system transitions into lower power state), performs PMU set up (the bit that actually sets up the system to process wake sources and it is PMU specific and board-specific and is usually expressed via various DT properties, both on PMU node and individual device nodes) and it does know how wakeup signals are set up. In the past most of the drivers assumed that in-band IRQ is the one that should wake up the system, now in I2C bus we added OF "wakeup-interrupts" that can be used to set up a dedicated interrupt as wakeup irq, otherwise we set client's in-band irq (client->irq) as wakeup source (provided that device is annotated as "wakeup-source"). For platform devices this does not quite work as we do not know how many interrupts they are using.
I think we are talking basically about the same thing, just in different terms. I do not expect that by simply marking device as "wakeup-source" it will magically wake up my system, but there is normally support for wakeup support in both driver and platform code that actually does the right thing when requested via this property. The property _has_ to be documented in binding for the driver, which ensures driver-side support is present, and making sure that system part (power domain, PMIC, PMU) are set up appropriately is system integrators task, no different from the task of providing appropriate BIOS/ACPI tables for system integrators creating X86 boxes.
Quoted 7 lines
Traditionally, we set the
can_wakeupbit if (a) there is an interface we can use to set up those things (either by accessing some registers available to us or by invoking the platform firmware to do it) and (b) the device is hooked up to that interface in a known way. IOW, that bit is inferred from what we can find out about the device's configuration rather that just taken from the firmware for granted.
It _really_ depends on the device. Some of them are non-discoverable and thus you need to trust firmware. For example for I2C device you need to know interrupt number, you can't normally query device for it.
Quoted 14 lines
Now, this generally may not be the right approach.
Maybe we just don't need the
can_wakeupbit at all. Maybe we should just create the "wakeup" sysfs attribute for all devices, let user space set it the way it wants and handle it on the best effort basis. That is, it will wake up from the sleep states it can wake up from, but it may not wake up from any of them if there's no support. Granted, we really can't guarantee that it will work anyway (even if the firmware exposes the interface to us, it may not be correctly implemented etc and there's no way to verify that upfront). And we may allow the same attribute to be used for disabling remote wakeup for runtime PM if someone wants to.
Quoted 4 lines
In that case, though, we really won't need the firmware to tell us whether or not the device is "wakeup-capable". We will always treat it this way and if it turns out that something is missing, wakeup will just not work.
This is a non-starter. Quite often there is difference in power consumption between deep sleep/powered off state and state in which device can signal activity so that the system (or subsystem) can wake up. So if firmware knows that device is not wired to wake up the system we better communicate it to kernel/userspace instead of them having to cross their fingers and hope for the best.
Thanks.
Signature · 1 line
Dmitry
Rafael J. Wysocki
maintainerSorry about the delay, I'm traveling ATM.
Quoted 68 lines · Dmitry Torokhov
Quoted 52 lines · Rafael J. Wysocki
Quoted 32 lines · Rafael J. Wysocki
Quoted 2 lines · Rafael J. Wysocki
On Wed, Jan 20, 2016 at 1:45 AM, Dmitry Torokhov
<[email protected]> wrote:[cut]
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
I am not aware of any drivers denoting itself as being wakeup sources and not enabling wakeup. Do you have examples?
Yes, I do.
i8042 is one of them, exactly to avoid enabling those devices to do system wakeup on systems where they were not enabled to do that in the past.
Quoted 7 lines
Ah, yes, you added it to i8042. Well, maybe that is an argument for not automatically enabling wakeup on ACPI systems because that was the default behavior for them. OF/board platforms have different default behavior. I guess if we do not do anything besides what the patch is doing, then ACPI will not have the property I propose, so they will be capable but not enabled, and OF/boards will have the property and will be capable and enabled.
Quoted 8 lines
The defaults to use should generally depend on two things IMO: On what the firmware tells us to use (given the argument that the firmware people probably have a good reason to tell us what they are telling us) and on what we were doing for this class of devices in the past. Ignoring the last bit generally leads to regressions in the cases when the firmware people are wrong after all, so I'm always cautious about this. And this rule applies to DT as well as to ACPI, although the DT people usually won't admit it. :-)
Quoted 4 lines
OK, I realize that the example I gave was not really a good one, because the wakeup part was added to i8042 to support wakeup from suspend-to-idle which is really special and the implementation is based on what the driver was doing previously.
Which is not to say that there are no good (or at least better) examples.
USB HID devices pretty much universally support remote wakeup officially, but enough of them cause problems to happen while using it (spurious wakeups etc) that we don't enable them for system wakeup by default. Plus some of them are cordless mice and such and you really need to be careful to avoid waking up a sleeping system by accident with them (when enabled).
Ethernet NICs support wakeup signaling, but at least some of them are not enabled for system wakeup by default. Moreover, there are multiple ways to trigger the wakeup there that the user needs to choose from in the first place.
Quoted 6 lines
Some wireless adapters support WoW, but enabling them to wake up the system from sleep by default would be asking for troubles.
All in all, combining the information that the device can signal wakeup with the requirement to enable it to wake up the system from sleep states by default doesn't look like a good idea to me.
You are making a convincing argument here for having an option to allow specifying that device is just wakeup capable, but not enabled by default. For ACPI you have your own ways of defining whether the device is wakeup capable (and you also have some rules to mark certain devices as wakeup enabled by default - button GPEs for example), so that leaves DT and static board files where historically we enabled wakeup for platform/I2C/SPI devices that board believes are wakeup-capable.
Quoted 2 lines
I wonder if I should do that property parsing in genpd code, similarly to what you are doing in ACPI power domain.
That would be more suitable IMO.
Quoted 4 lines
What does not help is that
we have different notions of power domain at once - one where device is under control of ACPI and another that denotes grouping of devices together.
That's why a "PM domain" really means "a group of devices with analogous PM handling" rather than "a group of devices with the same clock or VR" etc.
Quoted 62 lines
Quoted 54 lines
Quoted 35 lines
But this also makes me think that the problem is really more complicated than it may seem to be, so what about starting over and looking at the wakeup problem in general?
To that end, there are two categories of wakeup support in devices. The first one is support for something called "remote wakeup" in USB and which means that the device is able to generate wakeup signals when it (the device) is suspended and the rest of the system can generally be considered as working. I'll use the "remote wakeup" term in general for the lack of a better one.
Remote wakeup support is used in runtime PM and suspend-to-idle (which essentially uses the same kind of hardware mechanics, but in a different way).
Note, though, that "device is suspended" need not map 1:1 onto a particular hardware state. What it really means is that either it has been suspended using the runtime PM framework, or all devices have gone through the device suspend sequence during suspend-to-idle. The hardware state the device ends up in depends on the driver/bus type/PM domain combination and is generally expected to be low-power.
It is easy in PCI or USB and on ACPI systems where low-power states of devices are well defined and the connection between them and the ability to generate wakeup signals is known. It is not so easy in the other cases, though. My personal opinion is that if wakeup support is required, the device should be put into the lowest-power state from which it can generate wakeup signals. That very well may be the full-power state of it if that's the only suitable one.
If that point of view is adopted, then any device that (a) can take input and (b) uses interrupts can do remote wakeup. We don't really have a good way to express that in the driver core.
Quoted 7 lines
The second category of wakeup support is for platform-assisted low-power states of the whole system where there's a big switch (or a bunch of them) allowing multiple things to be powered off in one go from the OS perspective.
I'll write about that in the next message, as I need to take care of some urgent stuff now.
Continuing.
If the device in question belongs to the part of the system powered off by the big switch, it has to be provided with some special "wakeup" power source so it can generate signals. There needs to be a physical line (or bus or similar) that will be activated when wakeup is signaled by the device. The activation of it needs to be noticed by something and turned into a signal that eventually will wake up the CPU (or make it start executing a power-up sequence or equivalent). All of that generally requires specific setup.
Quoted 7 lines
The device has to be left in a state in which it can use the "wakeup" power source in the first place. The "wakeup" power source for it has to be turned on. The signal "line" needs to be prepared for activation by the device's wakeup signals. The part of the system responsible for waking up the CPU when that "line" is active has to be prepared too. All this means that generally it is not enough to say "things are wired up properly" for the right stuff to happen.
Quoted 5 lines
There are usually 2 components to this, one from device/driver perspective and another from system perspective. From the device perspective there is not difference between "remote wakeup" and "big switch" wakeup. In both cases the driver should put the device into lowest power state that still allows it to react to external inputs.
Moreover, in cases like ACPI and PCI devices are not put into low-power states by drivers even. Rather, bus types and/or PM domains do that then, so there really is no difference from the perspective of what the driver is expected to do.
Quoted 7 lines
Putting the device into that power state may also affect other devices it is connected to, for example it may request to power down some regulators and/or stop some clocks if they are not needed in that low power mode. When there is activity the device sends signal (could be dedicated interrupt pin or in-bound interrupt pin - this is device specific) and how they are wired is board-specific.
Quoted 14 lines
From the system perspective it provides notion of power domains as in grouping (so that we know when we can turn off certain supplies and what parts of the system should stay powered up even when system transitions into lower power state), performs PMU set up (the bit that actually sets up the system to process wake sources and it is PMU specific and board-specific and is usually expressed via various DT properties, both on PMU node and individual device nodes) and it does know how wakeup signals are set up. In the past most of the drivers assumed that in-band IRQ is the one that should wake up the system, now in I2C bus we added OF "wakeup-interrupts" that can be used to set up a dedicated interrupt as wakeup irq, otherwise we set client's in-band irq (
client->irq) as wakeup source (provided that device is annotated as "wakeup-source"). For platform devices this does not quite work as we do not know how many interrupts they are using.
Right.
Quoted 10 lines
I think we are talking basically about the same thing, just in different terms. I do not expect that by simply marking device as "wakeup-source" it will magically wake up my system, but there is normally support for wakeup support in both driver and platform code that actually does the right thing when requested via this property. The property _has_ to be documented in binding for the driver, which ensures driver-side support is present, and making sure that system part (power domain, PMIC, PMU) are set up appropriately is system integrators task, no different from the task of providing appropriate BIOS/ACPI tables for system integrators creating X86 boxes.
OK, so that answers my question about what "wakeup-source" is supposed to mean. :-)
The case in question seems to be when the same driver works with platforms that may or may not wire up the device for wakeup and that attribute tells it what the case is on the given platform/board.
Quoted 31 lines
Quoted 7 lines
Traditionally, we set the
can_wakeupbit if (a) there is an interface we can use to set up those things (either by accessing some registers available to us or by invoking the platform firmware to do it) and (b) the device is hooked up to that interface in a known way. IOW, that bit is inferred from what we can find out about the device's configuration rather that just taken from the firmware for granted.It _really_ depends on the device. Some of them are non-discoverable and thus you need to trust firmware. For example for I2C device you need to know interrupt number, you can't normally query device for it.
Quoted 14 lines
Now, this generally may not be the right approach.
Maybe we just don't need the
can_wakeupbit at all. Maybe we should just create the "wakeup" sysfs attribute for all devices, let user space set it the way it wants and handle it on the best effort basis. That is, it will wake up from the sleep states it can wake up from, but it may not wake up from any of them if there's no support. Granted, we really can't guarantee that it will work anyway (even if the firmware exposes the interface to us, it may not be correctly implemented etc and there's no way to verify that upfront). And we may allow the same attribute to be used for disabling remote wakeup for runtime PM if someone wants to.Quoted 4 lines
In that case, though, we really won't need the firmware to tell us whether or not the device is "wakeup-capable". We will always treat it this way and if it turns out that something is missing, wakeup will just not work.
Quoted 6 lines
This is a non-starter. Quite often there is difference in power consumption between deep sleep/powered off state and state in which device can signal activity so that the system (or subsystem) can wake up. So if firmware knows that device is not wired to wake up the system we better communicate it to kernel/userspace instead of them having to cross their fingers and hope for the best.
OK, I see.
Thanks,
Rafael