Re: [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
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! -- Darren Hart VMware Open Source Technology Center
Re: [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
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
Re: [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
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.
--
Darren Hart
VMware Open Source Technology Center
Re: [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
> 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.
--
Best regards,
Michał Kępień
Re: [PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
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 ?
--
Darren Hart
VMware Open Source Technology Center
[PATCH 5/7] platform/x86: fujitsu-laptop: do not update ACPI device power status
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ń
---
drivers/platform/x86/fujitsu-laptop.c | 24
1 file changed, 4 insertions(+), 20 deletions(-)
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");
--
2.13.1
