Support running driver's probe for a device powered off
Subsystems (7)
Sakari Ailus
maintainerv2 cover Support running driver's probe for a device powered off
Hi all,
These patches enable calling (and finishing) a driver's probe function without powering on the respective device on busses where the practice is to power on the device for probe. While it generally is a driver's job to check the that the device is there, there are cases where it might be undesirable. (In this case it stems from a combination of hardware design and user expectations; see below.) The downside with this change is that if there is something wrong with the device, it will only be found at the time the device is used. In this case (the camera sensors + EEPROM in a sensor) I don't see any tangible harm from that though.
An indication both from the driver and the firmware is required to allow the device's power state to remain off during probe (see the first patch).
The use case is such that there is a privacy LED next to an integrated user-facing laptop camera, and this LED is there to signal the user that the camera is recording a video or capturing images. That LED also happens to be wired to one of the power supplies of the camera, so whenever you power on the camera, the LED will be lit, whether images are captured from the camera --- or not. There's no way to implement this differently without additional software control (allowing of which is itself a hardware design decision) on most CSI-2-connected camera sensors as they simply have no pin to signal the camera streaming state.
This is also what happens during driver probe: the camera will be powered on by the I²C subsystem calling dev_pm_domain_attach() and the device is already powered on when the driver's own probe function is called. To the user this visible during the boot process as a blink of the privacy LED, suggesting that the camera is recording without the user having used an application to do that. From the end user's point of view the behaviour is not expected and for someone unfamiliar with internal workings of a computer surely seems quite suspicious --- even if images are not being actually captured.
since v1:
- Rename probe_powered_off struct device field as probe_low_power and reflect the similar naming to the patches overall.
- Work with CONFIG_PM disabled, too.
Rajmohan Mani (1):
media: i2c: imx319: Support probe while the device is off
Sakari Ailus (4):
ACPI: Enable driver and firmware hints to control power at probe time
ACPI: Add a convenience function to tell a device is suspended in
probe
ov5670: Support probe whilst the device is in a low power state
at24: Support probing while off
drivers/acpi/device_pm.c | 50 ++++++++++++++++++++++++++++++++++++-- drivers/media/i2c/imx319.c | 23 +++++++++++------- drivers/media/i2c/ov5670.c | 23 +++++++++++------- drivers/misc/eeprom/at24.c | 31 +++++++++++++++-------- include/linux/acpi.h | 5 ++++ include/linux/device.h | 7 ++++++ 6 files changed, 109 insertions(+), 30 deletions(-)
Signature · 1 line
2.20.1
Sakari Ailus
maintainerPATCH v2 1/5 ACPI: Enable driver and firmware hints to control power at probe time
Allow drivers and firmware tell ACPI that there's no need to power on a device for probe. This requires both a hint from the firmware as well as an indication from a driver to leave the device off.
--- drivers/acpi/device_pm.c | 15 +++++++++++++-- include/linux/device.h | 7 +++++++ 2 files changed, 20 insertions(+), 2 deletions(-)
drivers/acpi/device_pm.c +13 -2
@@ -1276,7 +1276,12 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off)if (adev && dev->pm_domain == &acpi_general_pm_domain) {dev_pm_domain_set (dev,NULL );acpi_remove_pm_notifier (adev); -if (power_off) { +if (power_off +#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && +device_property_present (dev,"probe-low-power" )) +#endif + ) {/* * If the device's PM QoS resume latency limit or flags * have been exposed to user space, they have to be
@@ -1324,7 +1329,13 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)acpi_add_pm_notifier (adev, dev, acpi_pm_notify_work_func);dev_pm_domain_set (dev, &acpi_general_pm_domain); -if (power_on) { + +if (power_on +#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && +device_property_present (dev,"probe-low-power" )) +#endif + ) {acpi_dev_pm_full_power (adev);acpi_device_wakeup_disable (adev); }
include/linux/device.h +7 -0
@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table.
@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;enum probe_type probe_type;const struct of_device_id *of_match_table;
Signature · 1 line
2.20.1
Greg Kroah-Hartman
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Quoted 61 lines · Sakari Ailus
Allow drivers and firmware tell ACPI that there's no need to power on a device for probe. This requires both a hint from the firmware as well as an indication from a driver to leave the device off.
Signed-off-by Sakari Ailus <[email protected]>--- drivers/acpi/device_pm.c | 15 +++++++++++++-- include/linux/device.h | 7 +++++++ 2 files changed, 20 insertions(+), 2 deletions(-)drivers/acpi/device_pm.c +13
-2@@ -1276,7 +1276,12 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off)if (adev && dev->pm_domain == &acpi_general_pm_domain) {dev_pm_domain_set (dev,NULL );acpi_remove_pm_notifier (adev); -if (power_off) { +if (power_off +#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && +device_property_present (dev,"probe-low-power" )) +#endif + ) {/* * If the device's PM QoS resume latency limit or flags * have been exposed to user space, they have to be @@ -1324,7 +1329,13 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)acpi_add_pm_notifier (adev, dev, acpi_pm_notify_work_func);dev_pm_domain_set (dev, &acpi_general_pm_domain); -if (power_on) { + +if (power_on +#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && +device_property_present (dev,"probe-low-power" )) +#endif + ) {acpi_dev_pm_full_power (adev);acpi_device_wakeup_disable (adev); }include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table.
include/linux/device.h struct device_driver { · line 294
Quoted 4 lines
include/linux/device.h +1
-0@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;
Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
thanks,
greg k-h
Sakari Ailus
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 20 lines
Quoted 20 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. @@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */
Quoted 5 lines
include/linux/device.h struct device_driver { · line 294
Quoted 2 lines
include/linux/device.h +1
-0bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Signature · 4 lines
Regards, Sakari Ailus [email protected]
Greg Kroah-Hartman
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Quoted 26 lines · Sakari Ailus
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 20 lines
Quoted 20 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. @@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */
Quoted 8 lines
Quoted 5 lines
include/linux/device.h struct device_driver { · line 294
Quoted 2 lines
include/linux/device.h +1
-0bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Why is this even needed to be a bus/device attribute at all? You are checking the firmware property in the probe function, just do the logic there as you are, what needs to be saved to the bus's logic?
thanks,
greg k-h
Sakari Ailus
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Quoted 33 lines · Greg Kroah-Hartman
Quoted 32 lines · Sakari Ailus
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 25 lines
Quoted 18 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. include/linux/device.h struct device_driver { · line 294
Quoted 4 lines
include/linux/device.h +1
-0@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
Quoted 6 lines
Quoted 2 lines
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Why is this even needed to be a bus/device attribute at all? You are checking the firmware property in the probe function, just do the logic there as you are, what needs to be saved to the bus's logic?
By the time the driver gets hold of the device, or gets to control its power state, the I²C framework has already called dev_pm_domain_attach() to power on the device. The I²C drivers on ACPI based systems expect the device to be powered on for probe.
Signature · 2 lines
Sakari Ailus [email protected]
Rafael J. Wysocki
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
In reply to Greg Kroah-HartmanQuoted 34 lines · Greg Kroah-Hartman
Quoted 32 lines · Sakari Ailus
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 25 lines
Quoted 18 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. include/linux/device.h struct device_driver { · line 294
Quoted 4 lines
include/linux/device.h +1
-0@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
Quoted 6 lines
Quoted 2 lines
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Why is this even needed to be a bus/device attribute at all? You are checking the firmware property in the probe function, just do the logic there as you are, what needs to be saved to the bus's logic?
The situation today is that all devices are put into D0 by the ACPI layer before driver probing since drivers generally expect devices to be in D0 when their probe routines run. If the driver is prepared to cope with devices in low-power states, though, powering them up before probing for a driver may not be necessary, but still the core (or generally the code invoking the driver probe) needs to know that the driver really is prepared for that. Hence the driver flag AFAICS.
Now, in theory there may be some platform requirements regarding the power states of devices during driver probe, although admittedly it is not entirely clear to me why that would be the case) and hence the property. I would think that if the driver could cope with devices in low-power states during probe, the platform wouldn't need to worry about that.
Sakari Ailus
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Hi Rafael,
Quoted 51 lines · Rafael J. Wysocki
Quoted 34 lines · Greg Kroah-Hartman
Quoted 32 lines · Sakari Ailus
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 25 lines
Quoted 18 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. include/linux/device.h struct device_driver { · line 294
Quoted 4 lines
include/linux/device.h +1
-0@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
Quoted 6 lines
Quoted 2 lines
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Why is this even needed to be a bus/device attribute at all? You are checking the firmware property in the probe function, just do the logic there as you are, what needs to be saved to the bus's logic?
The situation today is that all devices are put into D0 by the ACPI layer before driver probing since drivers generally expect devices to be in D0 when their probe routines run. If the driver is prepared to cope with devices in low-power states, though, powering them up before probing for a driver may not be necessary, but still the core (or generally the code invoking the driver probe) needs to know that the driver really is prepared for that. Hence the driver flag AFAICS.
Quoted 3 lines
Now, in theory there may be some platform requirements regarding the power states of devices during driver probe, although admittedly it is not entirely clear to me why that would be the case) and hence the
Please see the cover page of the set (also here):
<URL:https://lkml.org/lkml/2019/8/26/175>
Quoted 3 lines
property. I would think that if the driver could cope with devices in low-power states during probe, the platform wouldn't need to worry about that.
I understand this as driver deciding whether it'd power on the device during probe.
That way there's no way to judge whether the device is accessible, and probe would succeed without an error, which then manifests itself on the first access of the device. Such as on the at24 EEPROM driver, the error would take place on first read of the contents, not in probe.
Somebody might consider that as a driver bug.
Signature · 4 lines
Kind regards, Sakari Ailus [email protected]
Rafael J. Wysocki
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Quoted 67 lines · Sakari Ailus
Hi Rafael,
Quoted 51 lines · Rafael J. Wysocki
Quoted 34 lines · Greg Kroah-Hartman
Quoted 32 lines · Sakari Ailus
Hi Greg,
On Mon, Aug 26, 2019 at 10:43:43AM +0200, Greg Kroah-Hartman wrote:
...
Quoted 25 lines
Quoted 18 lines
include/linux/device.h +6
-0@@ -248,6 +248,12 @@ enum probe_type {* @owner: The module owner. * @mod_name: Used for built-in modules. * @suppress_bind_attrs: Disables bind/unbind via sysfs. +* @probe_low_power: The driver supports its probe function being called while +* the device is in a low power state, independently of the +* expected behaviour on combination of a given bus and +* firmware interface etc. The driver is responsible for +* powering the device on using runtime PM in such case. +* This configuration has no effect if CONFIG_PM is disabled. * @probe_type: Type of the probe (synchronous or asynchronous) to use. * @of_match_table: The open firmware table. * @acpi_match_table: The ACPI match table. include/linux/device.h struct device_driver { · line 294
Quoted 4 lines
include/linux/device.h +1
-0@@ -285,6 +291,7 @@ struct device_driver {const char *mod_name;/* used for built-in modules */ bool suppress_bind_attrs;/* disables bind/unbind via sysfs */ +bool probe_low_power;Ick, no, this should be a bus-specific thing to handle such messed up hardware. Why polute this in the driver core?
Quoted 6 lines
Quoted 2 lines
The alternative could be to make it I²C specific indeed; the vast majority of camera sensors are I²C devices these days.
Why is this even needed to be a bus/device attribute at all? You are checking the firmware property in the probe function, just do the logic there as you are, what needs to be saved to the bus's logic?
The situation today is that all devices are put into D0 by the ACPI layer before driver probing since drivers generally expect devices to be in D0 when their probe routines run. If the driver is prepared to cope with devices in low-power states, though, powering them up before probing for a driver may not be necessary, but still the core (or generally the code invoking the driver probe) needs to know that the driver really is prepared for that. Hence the driver flag AFAICS.
Quoted 3 lines
Now, in theory there may be some platform requirements regarding the power states of devices during driver probe, although admittedly it is not entirely clear to me why that would be the case) and hence the
Please see the cover page of the set (also here):
<URL:https://lkml.org/lkml/2019/8/26/175>
Quoted 3 lines
property. I would think that if the driver could cope with devices in low-power states during probe, the platform wouldn't need to worry about that.
Quoted 6 lines
I understand this as driver deciding whether it'd power on the device during probe.
That way there's no way to judge whether the device is accessible, and probe would succeed without an error, which then manifests itself on the first access of the device.
OK, so the property really represents the platform preference in that respect and the presence of it by no means guarantees that there won't be any problems on the first device access.
Quoted 4 lines
Such as on the at24 EEPROM driver, the error
would take place on first read of the contents, not in probe.Somebody might consider that as a driver bug.
Well, I guess you can argue that the safer thing, which therefore should be the default, is to power up the device before probing and to check whether or not it is accessible at that time. However, in some cases it may be desirable to avoid powering up the device at that point, whatever the reason, and the property provides a hint about that.
Fair enough to me, but honestly I'm not sure about the example in the cover letter. :-)
Greg Kroah-Hartman
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
In reply to Sakari AilusQuoted 15 lines · Sakari Ailus
Allow drivers and firmware tell ACPI that there's no need to power on a device for probe. This requires both a hint from the firmware as well as an indication from a driver to leave the device off.
Signed-off-by Sakari Ailus <[email protected]>--- drivers/acpi/device_pm.c | 15 +++++++++++++-- include/linux/device.h | 7 +++++++ 2 files changed, 20 insertions(+), 2 deletions(-)drivers/acpi/device_pm.c +0
-0
drivers/acpi/device_pm.c static void acpi_dev_pm_detach(struct device *dev, bool power_off) · line 1283
Quoted 9 lines
drivers/acpi/device_pm.c +5
-1@@ -1276,7 +1276,12 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off) if (adev && dev->pm_domain == &acpi_general_pm_domain) { dev_pm_domain_set(dev, NULL); acpi_remove_pm_notifier(adev); -if (power_off) { + if (power_off +#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && + device_property_present(dev, "probe-low-power")) +#endif
As proof of the "only a bus-specific thing", why is probe_low_power even needed? Why not just always trigger off of this crazy device_property? That makes the driver changes less.
Also, is this #ifdef really needed?
thanks,
greg k-h
Sakari Ailus
maintainerRe: [PATCH v2 1/5] ACPI: Enable driver and firmware hints to control power at probe time
Hi Greg,
Thanks for the comments.
Quoted 21 lines · Greg Kroah-Hartman
Quoted 20 lines · Sakari Ailus
Allow drivers and firmware tell ACPI that there's no need to power on a device for probe. This requires both a hint from the firmware as well as an indication from a driver to leave the device off.
Signed-off-by Sakari Ailus <[email protected]>--- drivers/acpi/device_pm.c | 15 +++++++++++++-- include/linux/device.h | 7 +++++++ 2 files changed, 20 insertions(+), 2 deletions(-)drivers/acpi/device_pm.c +1
-1@@ -1276,7 +1276,12 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off)if (adev && dev->pm_domain == &acpi_general_pm_domain) {dev_pm_domain_set (dev,NULL );acpi_remove_pm_notifier (adev); -if (power_off) { + if (power_off
Quoted 8 lines
drivers/acpi/device_pm.c static void acpi_dev_pm_detach(struct device *dev, bool power_off) · line 1283
Quoted 4 lines
drivers/acpi/device_pm.c +4
-0+#ifdef CONFIG_PM + && !(dev->driver->probe_low_power && + device_property_present(dev, "probe-low-power")) +#endif As proof of the "only a bus-specific thing", why is
probe_low_powereven needed? Why not just always trigger off of this crazydevice_property? That makes the driver changes less.
That's an option, too, but firmware having this property for a device the driver of which doesn't expect it will fail to power on the device for probe. This leaves some room for unexpected failures that admittedly are easy to fix, but could be harder to debug.
Quoted 2 lines
Also, is this #ifdef really needed?
I thought it was but it seems if CONFIG_PM is disabled, dev_pm_domain_attach() has a nop implementation. So I agree it is not.
Signature · 4 lines
Regards, Sakari Ailus [email protected]
Sakari Ailus
maintainerPATCH v2 2/5 ACPI: Add a convenience function to tell a device is suspended in probe
In reply to Sakari AilusAdd a convenience function to tell whether a device is suspended for probe or remove, for busses where the custom is that drivers don't need to resume devices in probe, or suspend them in their remove handlers.
Returns false on non-ACPI systems.
--- drivers/acpi/device_pm.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/acpi.h | 5 +++++ 2 files changed, 40 insertions(+)
drivers/acpi/device_pm.c +35 -0
@@ -1344,4 +1344,39 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on) return 1; }EXPORT_SYMBOL_GPL (acpi_dev_pm_attach); + +/** +* acpi_dev_low_power_state_probe - Tell if a device is in a low power state +* during probe +* @dev: The device +* +* Tell whether a given device is in a low power state during the driver's probe +* or remove operation. +* +* Drivers of devices on certain busses such as I²C can generally assume (on +* ACPI based systems) that the devices they control are powered on without +* driver having to do anything about it. Using struct +* device_driver.probe_low_power and "probe-low-power" property, this can be +* negated and the driver has full control of the device power management. +* Always returns false on non-ACPI based systems. True is returned on ACPI +* based systems iff the device is in a low power state during probe or remove. +*/ +bool acpi_dev_low_power_state_probe (struct device *dev) +{ +int power_state; +int ret; + +if (!is_acpi_device_node (dev_fwnode (dev))) +return false; + + ret =acpi_device_get_power (ACPI_COMPANION (dev), &power_state); +if (ret) { +dev_warn (dev,"Cannot obtain power state (%d)\n" , ret); +return false; + } + +return power_state !=ACPI_STATE_D0 ; +} +EXPORT_SYMBOL_GPL (acpi_dev_low_power_state_probe); +#endif /* CONFIG_PM */
include/linux/acpi.h +5 -0
@@ -912,6 +912,7 @@ int acpi_dev_resume(struct device *dev);int acpi_subsys_runtime_suspend (struct device *dev);int acpi_subsys_runtime_resume (struct device *dev);int acpi_dev_pm_attach (struct device *dev,bool power_on); +bool acpi_dev_low_power_state_probe (struct device *dev);#else static inline int acpi_dev_runtime_suspend (struct device *dev) {return 0 ; }static inline int acpi_dev_runtime_resume (struct device *dev) {return 0 ; }
@@ -921,6 +922,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on) {return 0 ; } +static inline bool acpi_dev_low_power_state_probe (struct device *dev) +{ +return false; +}#endif #if defined(CONFIG_ACPI ) && defined(CONFIG_PM_SLEEP )
Signature · 1 line
2.20.1
Sakari Ailus
maintainerPATCH v2 3/5 ov5670: Support probe whilst the device is in a low power state
In reply to Sakari AilusTell ACPI device PM code that the driver supports the device being in a low power state when the driver's probe function is entered.
--- drivers/media/i2c/ov5670.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/media/i2c/ov5670.c +14 -9
@@ -2444,6 +2444,7 @@ static int ov5670_probe(struct i2c_client *client)struct ov5670 *ov5670;const char *err_msg;u32 input_clk =0 ; +bool low_power;int ret;device_property_read_u32 (&client->dev,"clock-frequency" , &input_clk);
@@ -2460,11 +2461,14 @@ static int ov5670_probe(struct i2c_client *client)/* Initialize subdev */ v4l2_i2c_subdev_init (&ov5670->sd, client, &ov5670_subdev_ops); -/* Check module identity */ - ret =ov5670_identify_module (ov5670); -if (ret) { - err_msg ="ov5670_identify_module() error" ; - goto error_print; + low_power =acpi_dev_low_power_state_probe (&client->dev); +if (!low_power) { +/* Check module identity */ + ret =ov5670_identify_module (ov5670); +if (ret) { + err_msg ="ov5670_identify_module() error" ; + goto error_print; + } }mutex_init (&ov5670->mutex);
@@ -2501,10 +2505,10 @@ static int ov5670_probe(struct i2c_client *client) ov5670->streaming = false;/* -* Device is already turned on by i2c-core with ACPI domain PM. -* Enable runtime PM and turn off the device. +* Don't set the device's state to active if it's in a low power state. */ -pm_runtime_set_active (&client->dev); +if (!low_power) +pm_runtime_set_active (&client->dev);pm_runtime_enable (&client->dev);pm_runtime_idle (&client->dev);
@@ -2546,7 +2550,7 @@ static const struct dev_pm_ops ov5670_pm_ops = {#ifdef CONFIG_ACPI static const struct acpi_device_id ov5670_acpi_ids[] = { - {"INT3479" }, + {"INT3479" }, {/* sentinel */ } };
@@ -2556,6 +2560,7 @@ MODULE_DEVICE_TABLE(acpi, ov5670_acpi_ids);static struct i2c_driver ov5670_i2c_driver = { .driver = { .name ="ov5670" , + .probe_low_power = true, .pm = &ov5670_pm_ops, .acpi_match_table =ACPI_PTR (ov5670_acpi_ids), },
Signature · 1 line
2.20.1
Sakari Ailus
maintainerPATCH v2 4/5 media: i2c: imx319: Support probe while the device is off
In reply to Sakari AilusFrom: Rajmohan Mani <[email protected]>
Tell ACPI device PM code that the driver supports the device being powered off when the driver's probe function is entered.
--- drivers/media/i2c/imx319.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/media/i2c/imx319.c +14 -9
@@ -2424,6 +2424,7 @@ static struct imx319_hwcfg *imx319_get_hwcfg(struct device *dev)static int imx319_probe (struct i2c_client *client) {struct imx319 *imx319; +bool low_power;int ret;u32 i;
@@ -2436,11 +2437,14 @@ static int imx319_probe(struct i2c_client *client)/* Initialize subdev */ v4l2_i2c_subdev_init (&imx319->sd, client, &imx319_subdev_ops); -/* Check module identity */ - ret =imx319_identify_module (imx319); -if (ret) { -dev_err (&client->dev,"failed to find sensor: %d" , ret); - goto error_probe; + low_power =acpi_dev_low_power_state_probe (&client->dev); +if (!low_power) { +/* Check module identity */ + ret =imx319_identify_module (imx319); +if (ret) { +dev_err (&client->dev,"failed to find sensor: %d" , ret); + goto error_probe; + } } imx319->hwcfg =imx319_get_hwcfg (&client->dev);
@@ -2493,10 +2497,10 @@ static int imx319_probe(struct i2c_client *client) goto error_media_entity;/* -* Device is already turned on by i2c-core with ACPI domain PM. -* Enable runtime PM and turn off the device. +* Don't set the device's state to active if it's in a low power state. */ -pm_runtime_set_active (&client->dev); +if (!low_power) +pm_runtime_set_active (&client->dev);pm_runtime_enable (&client->dev);pm_runtime_idle (&client->dev);
@@ -2536,7 +2540,7 @@ static const struct dev_pm_ops imx319_pm_ops = { };static const struct acpi_device_id imx319_acpi_ids[] = { - {"SONY319A" }, + {"SONY319A" , }, {/* sentinel */ } };MODULE_DEVICE_TABLE (acpi, imx319_acpi_ids);
@@ -2544,6 +2548,7 @@ MODULE_DEVICE_TABLE(acpi, imx319_acpi_ids);static struct i2c_driver imx319_i2c_driver = { .driver = { .name ="imx319" , + .probe_low_power = true, .pm = &imx319_pm_ops, .acpi_match_table =ACPI_PTR (imx319_acpi_ids), },
Signature · 1 line
2.20.1
Sakari Ailus
maintainerPATCH v2 5/5 at24: Support probing while off
In reply to Sakari AilusIn certain use cases (where the chip is part of a camera module, and the camera module is wired together with a camera privacy LED), powering on the device during probe is undesirable. Add support for the at24 to execute probe while being powered off. For this to happen, a hint in form of a device property is required from the firmware.
--- drivers/misc/eeprom/at24.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
drivers/misc/eeprom/at24.c +21 -10
@@ -565,6 +565,7 @@ static int at24_probe(struct i2c_client *client)bool i2c_fn_i2c, i2c_fn_block;unsigned int i, num_addresses;struct at24_data *at24; +bool low_power;struct regmap *regmap;bool writable;u8 test_byte;
@@ -702,19 +703,24 @@ static int at24_probe(struct i2c_client *client)i2c_set_clientdata (client, at24); -/* enable runtime pm */ -pm_runtime_set_active (dev); + low_power =acpi_dev_low_power_state_probe (&client->dev); +if (!low_power) +pm_runtime_set_active (dev); +pm_runtime_enable (dev);/* -* Perform a one-byte test read to verify that the -* chip is functional. +* Perform a one-byte test read to verify that the chip is functional, +* unless powering on the device is to be avoided during probe (i.e. +* it's powered off right now). */ - err =at24_read (at24,0 , &test_byte,1 ); -pm_runtime_idle (dev); -if (err) { -pm_runtime_disable (dev); -return -ENODEV ; +if (!low_power) { + err =at24_read (at24,0 , &test_byte,1 ); +pm_runtime_idle (dev); +if (err) { +pm_runtime_disable (dev); +return -ENODEV ; + } } dev_info(dev, "%u byte %s EEPROM, %s, %u bytes/write\n",
@@ -726,8 +732,12 @@ static int at24_probe(struct i2c_client *client)static int at24_remove (struct i2c_client *client) { +bool low_power; +pm_runtime_disable (&client->dev); -pm_runtime_set_suspended (&client->dev); + low_power =acpi_dev_low_power_state_probe (&client->dev); +if (!low_power) +pm_runtime_set_suspended (&client->dev);return 0 ; }
@@ -735,6 +745,7 @@ static int at24_remove(struct i2c_client *client)static struct i2c_driver at24_driver = { .driver = { .name ="at24" , + .probe_low_power = true, .of_match_table = at24_of_match, .acpi_match_table =ACPI_PTR (at24_acpi_ids), },
Signature · 1 line
2.20.1