| Message ID | 20170616044058.30443-6-kernel@kempniu.pl (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Fri, Jun 16, 2017 at 06:40:56AM +0200, Michał Kępień wrote: > Calling acpi_bus_update_power() for ACPI devices FUJ02B1 and FUJ02E3 is > pointless as they are not power manageable (neither _PS0 nor _PR0 is > defined for any of them), which causes their power state to be inherited > from their parent devices. Given the ACPI paths of these two devices > (\_SB.PCI0.LPCB.FJEX, \_SB.FEXT), their parent devices are also not > power manageable. These parent devices will thus have their power state > initialized to ACPI_STATE_D0, which in turn causes the power state for > both FUJ02B1 and FUJ02E3 to always be ACPI_STATE_D0 ("on"). > How confident are we that all implementations of these two ACPI devices lack _PS0 and _PR0 ?
> On Fri, Jun 16, 2017 at 06:40:56AM +0200, Michał Kępień wrote: > > Calling acpi_bus_update_power() for ACPI devices FUJ02B1 and FUJ02E3 is > > pointless as they are not power manageable (neither _PS0 nor _PR0 is > > defined for any of them), which causes their power state to be inherited > > from their parent devices. Given the ACPI paths of these two devices > > (\_SB.PCI0.LPCB.FJEX, \_SB.FEXT), their parent devices are also not > > power manageable. These parent devices will thus have their power state > > initialized to ACPI_STATE_D0, which in turn causes the power state for > > both FUJ02B1 and FUJ02E3 to always be ACPI_STATE_D0 ("on"). > > > > How confident are we that all implementations of these two ACPI devices lack > _PS0 and _PR0 ? I looked at DSDT dumps of four different Fujitsu laptops released in the past ten years or so for which at least one of these two ACPI devices is present and found no traces of either of these methods being defined for them. I do not think we have a way of ensuring that the above holds true for every other model out there, but I will point out that fujitsu-laptop is the only user of acpi_bus_update_power() outside of drivers/acpi.
On Thu, Jun 22, 2017 at 11:02:35PM +0200, Michał Kępień wrote: > > On Fri, Jun 16, 2017 at 06:40:56AM +0200, Michał Kępień wrote: > > > Calling acpi_bus_update_power() for ACPI devices FUJ02B1 and FUJ02E3 is > > > pointless as they are not power manageable (neither _PS0 nor _PR0 is > > > defined for any of them), which causes their power state to be inherited > > > from their parent devices. Given the ACPI paths of these two devices > > > (\_SB.PCI0.LPCB.FJEX, \_SB.FEXT), their parent devices are also not > > > power manageable. These parent devices will thus have their power state > > > initialized to ACPI_STATE_D0, which in turn causes the power state for > > > both FUJ02B1 and FUJ02E3 to always be ACPI_STATE_D0 ("on"). > > > > > > > How confident are we that all implementations of these two ACPI devices lack > > _PS0 and _PR0 ? > > I looked at DSDT dumps of four different Fujitsu laptops released in the > past ten years or so for which at least one of these two ACPI devices is > present and found no traces of either of these methods being defined for > them. I do not think we have a way of ensuring that the above holds > true for every other model out there, but I will point out that > fujitsu-laptop is the only user of acpi_bus_update_power() outside of > drivers/acpi. OK, thanks. Queueing to testing.
On Thu, Jun 22, 2017 at 04:58:43PM -0700, Darren Hart wrote: > On Thu, Jun 22, 2017 at 11:02:35PM +0200, Micha?? K??pie?? wrote: > > > On Fri, Jun 16, 2017 at 06:40:56AM +0200, Micha?? K??pie?? wrote: > > > > Calling acpi_bus_update_power() for ACPI devices FUJ02B1 and FUJ02E3 is > > > > pointless as they are not power manageable (neither _PS0 nor _PR0 is > > > > defined for any of them), which causes their power state to be inherited > > > > from their parent devices. Given the ACPI paths of these two devices > > > > (\_SB.PCI0.LPCB.FJEX, \_SB.FEXT), their parent devices are also not > > > > power manageable. These parent devices will thus have their power state > > > > initialized to ACPI_STATE_D0, which in turn causes the power state for > > > > both FUJ02B1 and FUJ02E3 to always be ACPI_STATE_D0 ("on"). > > > > > > > > > > How confident are we that all implementations of these two ACPI devices lack > > > _PS0 and _PR0 ? > > > > I looked at DSDT dumps of four different Fujitsu laptops released in the > > past ten years or so for which at least one of these two ACPI devices is > > present and found no traces of either of these methods being defined for > > them. I do not think we have a way of ensuring that the above holds > > true for every other model out there, but I will point out that > > fujitsu-laptop is the only user of acpi_bus_update_power() outside of > > drivers/acpi. > > OK, thanks. Queueing to testing. Thanks. In case it was missed, I supplied my reviewed-by message and sign-off in an earlier post. Regards jonathan
On Fri, Jun 23, 2017 at 09:46:59AM +0930, Jonathan Woithe wrote: > Thanks. In case it was missed, I supplied my reviewed-by message and > sign-off in an earlier post. Yup, got it - thanks!
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index 5c0dc2126313..b9f3ede4d567 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -400,7 +400,6 @@ static int fujitsu_backlight_register(struct acpi_device *device) static int acpi_fujitsu_bl_add(struct acpi_device *device) { struct fujitsu_bl *priv; - int state = 0; int error; if (acpi_video_get_backlight_type() != acpi_backlight_vendor) @@ -419,15 +418,8 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device) if (error) return error; - error = acpi_bus_update_power(device->handle, &state); - if (error) { - pr_err("Error reading power state\n"); - return error; - } - - pr_info("ACPI: %s [%s] (%s)\n", - acpi_device_name(device), acpi_device_bid(device), - !device->power.state ? "on" : "off"); + pr_info("ACPI: %s [%s]\n", + acpi_device_name(device), acpi_device_bid(device)); if (acpi_has_method(device->handle, METHOD_NAME__INI)) { vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n"); @@ -788,7 +780,6 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) static int acpi_fujitsu_laptop_add(struct acpi_device *device) { struct fujitsu_laptop *priv; - int state = 0; int error; int i; @@ -807,15 +798,8 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) if (error) return error; - error = acpi_bus_update_power(device->handle, &state); - if (error) { - pr_err("Error reading power state\n"); - return error; - } - - pr_info("ACPI: %s [%s] (%s)\n", - acpi_device_name(device), acpi_device_bid(device), - !device->power.state ? "on" : "off"); + pr_info("ACPI: %s [%s]\n", + acpi_device_name(device), acpi_device_bid(device)); if (acpi_has_method(device->handle, METHOD_NAME__INI)) { vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
Calling acpi_bus_update_power() for ACPI devices FUJ02B1 and FUJ02E3 is pointless as they are not power manageable (neither _PS0 nor _PR0 is defined for any of them), which causes their power state to be inherited from their parent devices. Given the ACPI paths of these two devices (\_SB.PCI0.LPCB.FJEX, \_SB.FEXT), their parent devices are also not power manageable. These parent devices will thus have their power state initialized to ACPI_STATE_D0, which in turn causes the power state for both FUJ02B1 and FUJ02E3 to always be ACPI_STATE_D0 ("on"). Remove relevant acpi_bus_update_power() calls along with parts of debug messages that they were supposed to have an effect on. Signed-off-by: Michał Kępień <kernel@kempniu.pl> --- drivers/platform/x86/fujitsu-laptop.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)