Regression: ACPI AC driver doesn't work on Toshiba Portege R500

Regression: ACPI AC driver doesn't work on Toshiba Portege R500

[Rafael J. Wysocki]

Hi,

With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
status of the AC adapter is not updated when the adapter is unplugged and
plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
same value.  Interestingly enough, though, if the box is suspended to RAM and
resumed, the status of the AC adapter is correctly updated, but the value read
at that time remains in /sys/class/power_supply/ADP1/online until the next
suspend/resume cycle regardless of what's going on with the AC adapter.

2.6.27.7 works correctly on this box so the recent EC patches don't seem to
cause this regression to happen.

Any other ideas?

Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> Hi,
> 
> With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> status of the AC adapter is not updated when the adapter is unplugged and
> plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> same value.  Interestingly enough, though, if the box is suspended to RAM and
> resumed, the status of the AC adapter is correctly updated, but the value read
> at that time remains in /sys/class/power_supply/ADP1/online until the next
> suspend/resume cycle regardless of what's going on with the AC adapter.
> 
> 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> cause this regression to happen.
> 
> Any other ideas?

The problem was introduced by the following commit:

commit faee816b1502385dc9bc5abf2960d1cc645844d1
Author: Zhang Rui <rui.zhang@intel.com>
Date:   Fri Sep 12 11:12:25 2008 +0800

    ACPI: don't enable control method power button as wakeup device when Fixed Power button is used

    don't enable control method power button as wakeup device
    when Fixed Power button is used.

    http://bugzilla.kernel.org/show_bug.cgi?id=10503

    Tested-by: walken@zoy.org <walken@zoy.org>
    Signed-off-by: Zhang Rui <rui.zhang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

Reverting this commit on top of the current mainline makes the kernel behave
correctly again.

Thanks,
Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Monday, 24 of November 2008, Rafael J. Wysocki wrote:
> On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > status of the AC adapter is not updated when the adapter is unplugged and
> > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > resumed, the status of the AC adapter is correctly updated, but the value read
> > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > suspend/resume cycle regardless of what's going on with the AC adapter.
> > 
> > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > cause this regression to happen.
> > 
> > Any other ideas?
> 
> The problem was introduced by the following commit:
> 
> commit faee816b1502385dc9bc5abf2960d1cc645844d1
> Author: Zhang Rui <rui.zhang@intel.com>
> Date:   Fri Sep 12 11:12:25 2008 +0800
> 
>     ACPI: don't enable control method power button as wakeup device when Fixed Power button is used
> 
>     don't enable control method power button as wakeup device
>     when Fixed Power button is used.
> 
>     http://bugzilla.kernel.org/show_bug.cgi?id=10503
> 
>     Tested-by: walken@zoy.org <walken@zoy.org>
>     Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>     Signed-off-by: Len Brown <len.brown@intel.com>
> 
> Reverting this commit on top of the current mainline makes the kernel behave
> correctly again.

Alas, after reverting the patch the box hangs on resume from suspend to RAM
when it is woken up by opening the lid.

I suspect that the GPE which is normally used for signalling the AC adapter
events is the same one associated with the fake power button and it is not
enabled if that button is not regarded as a wake-up device.  However, it causes
problems to happen during resume if the fake power button _is_ regarded as
a wake-up device.

The DSDT for the box is at:
http://www.sisk.pl/kernel/debug/ACPI/R500/DSDT.dsl

Thanks,
Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Zhao Yakui]

On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > status of the AC adapter is not updated when the adapter is unplugged and
> > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > resumed, the status of the AC adapter is correctly updated, but the value read
> > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > suspend/resume cycle regardless of what's going on with the AC adapter.
> > 
> > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > cause this regression to happen.
> > 
> > Any other ideas?
Hi, Rafael
    Will you please open a new bug at
http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
output of acpidump, dmesg, lspci -vxxx?
    If no ACPI event is reported when AC adapter is unplugged and
plugged, the /sys/class interface can't display the correct AC status.
Now Rui is working on this issue and the patch is already finished. But
it is not sent to Lenb.

    From the problem description it seems that the problem is related
with the AC driver. But from the git-bisect it seems that the problem is
related with the button driver(Fix power button device). 

    Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.

Thanks.
> 
> The problem was introduced by the following commit:
> 
> commit faee816b1502385dc9bc5abf2960d1cc645844d1
> Author: Zhang Rui <rui.zhang@intel.com>
> Date:   Fri Sep 12 11:12:25 2008 +0800
> 
>     ACPI: don't enable control method power button as wakeup device when Fixed Power button is used
> 
>     don't enable control method power button as wakeup device
>     when Fixed Power button is used.
> 
>     http://bugzilla.kernel.org/show_bug.cgi?id=10503
> 
>     Tested-by: walken@zoy.org <walken@zoy.org>
>     Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>     Signed-off-by: Len Brown <len.brown@intel.com>
> 
> Reverting this commit on top of the current mainline makes the kernel behave
> correctly again.
> 
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Monday, 24 of November 2008, Zhao Yakui wrote:
> On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > status of the AC adapter is not updated when the adapter is unplugged and
> > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > 
> > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > cause this regression to happen.
> > > 
> > > Any other ideas?
> Hi, Rafael
>     Will you please open a new bug at
> http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> output of acpidump, dmesg, lspci -vxxx?
>     If no ACPI event is reported when AC adapter is unplugged and
> plugged, the /sys/class interface can't display the correct AC status.
> Now Rui is working on this issue and the patch is already finished. But
> it is not sent to Lenb.

Can you give me a link to the patch, please?
 
>     From the problem description it seems that the problem is related
> with the AC driver. But from the git-bisect it seems that the problem is
> related with the button driver(Fix power button device). 
> 
>     Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.

Created http://bugzilla.kernel.org/show_bug.cgi?id=12091 with the information
attached as requested.

Thanks,
Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Zhang Rui]

[-- Attachment #1: Type: text/plain, Size: 3088 bytes --]

On Mon, 2008-11-24 at 09:21 +0800, Rafael J. Wysocki wrote:
> On Monday, 24 of November 2008, Zhao Yakui wrote:
> > On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > > Hi,
> > > > 
> > > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > > status of the AC adapter is not updated when the adapter is unplugged and
> > > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > > 
> > > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > > cause this regression to happen.
> > > > 
> > > > Any other ideas?
> > Hi, Rafael
> >     Will you please open a new bug at
> > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> > output of acpidump, dmesg, lspci -vxxx?
> >     If no ACPI event is reported when AC adapter is unplugged and
> > plugged, the /sys/class interface can't display the correct AC status.
> > Now Rui is working on this issue and the patch is already finished. But
> > it is not sent to Lenb.
> 
> Can you give me a link to the patch, please?
From: Zhang Rui <rui.zhang@intel.com>
Subject: power_supply: update ac status before acquiring it.

Update AC status in sysfs I/F.
http://bugzilla.kernel.org/show_bug.cgi?id=12035

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/ac.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/ac.c
===================================================================
--- linux-2.6.orig/drivers/acpi/ac.c
+++ linux-2.6/drivers/acpi/ac.c
@@ -87,6 +87,7 @@ struct acpi_ac {
 	unsigned long long state;
 };
 
+static int acpi_ac_get_state(struct acpi_ac *ac);
 #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger);
 
 #ifdef CONFIG_ACPI_PROCFS_POWER
@@ -103,15 +104,18 @@ static int get_ac_property(struct power_
 			   enum power_supply_property psp,
 			   union power_supply_propval *val)
 {
+	int result;
 	struct acpi_ac *ac = to_acpi_ac(psy);
+
 	switch (psp) {
 	case POWER_SUPPLY_PROP_ONLINE:
+		result = acpi_ac_get_state(ac);
 		val->intval = ac->state;
 		break;
 	default:
 		return -EINVAL;
 	}
-	return 0;
+	return result;
 }
 
 static enum power_supply_property ac_props[] = {

>  
> >     From the problem description it seems that the problem is related
> > with the AC driver. But from the git-bisect it seems that the problem is
> > related with the button driver(Fix power button device). 
> > 
> >     Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.
> 
> Created http://bugzilla.kernel.org/show_bug.cgi?id=12091 with the information
> attached as requested.
> 
I'll take this bug. :)

thanks,
rui

[-- Attachment #2: patch-12035-power_supply-update-AC-status --]
[-- Type: message/rfc822, Size: 1287 bytes --]

From: Zhang Rui <rui.zhang@intel.com>
Subject: power_supply: update ac status before acquiring it.
Date: Mon, 24 Nov 2008 09:31:07 +0800
Message-ID: <1227490267.15014.290.camel@rzhang-dt>

Update AC status in sysfs I/F.
http://bugzilla.kernel.org/show_bug.cgi?id=12035

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/ac.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/ac.c
===================================================================
--- linux-2.6.orig/drivers/acpi/ac.c
+++ linux-2.6/drivers/acpi/ac.c
@@ -87,6 +87,7 @@ struct acpi_ac {
 	unsigned long long state;
 };
 
+static int acpi_ac_get_state(struct acpi_ac *ac);
 #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger);
 
 #ifdef CONFIG_ACPI_PROCFS_POWER
@@ -103,15 +104,18 @@ static int get_ac_property(struct power_
 			   enum power_supply_property psp,
 			   union power_supply_propval *val)
 {
+	int result;
 	struct acpi_ac *ac = to_acpi_ac(psy);
+
 	switch (psp) {
 	case POWER_SUPPLY_PROP_ONLINE:
+		result = acpi_ac_get_state(ac);
 		val->intval = ac->state;
 		break;
 	default:
 		return -EINVAL;
 	}
-	return 0;
+	return result;
 }
 
 static enum power_supply_property ac_props[] = {

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Monday, 24 of November 2008, Rafael J. Wysocki wrote:
> On Monday, 24 of November 2008, Rafael J. Wysocki wrote:
> > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > status of the AC adapter is not updated when the adapter is unplugged and
> > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > 
> > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > cause this regression to happen.
> > > 
> > > Any other ideas?
> > 
> > The problem was introduced by the following commit:
> > 
> > commit faee816b1502385dc9bc5abf2960d1cc645844d1
> > Author: Zhang Rui <rui.zhang@intel.com>
> > Date:   Fri Sep 12 11:12:25 2008 +0800
> > 
> >     ACPI: don't enable control method power button as wakeup device when Fixed Power button is used
> > 
> >     don't enable control method power button as wakeup device
> >     when Fixed Power button is used.
> > 
> >     http://bugzilla.kernel.org/show_bug.cgi?id=10503
> > 
> >     Tested-by: walken@zoy.org <walken@zoy.org>
> >     Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> >     Signed-off-by: Len Brown <len.brown@intel.com>
> > 
> > Reverting this commit on top of the current mainline makes the kernel behave
> > correctly again.
> 
> Alas, after reverting the patch the box hangs on resume from suspend to RAM
> when it is woken up by opening the lid.

This was actually wrong.  There is a separate resume regression on this box,
caused most probably by DRM, so in fact the reverting of the commit above is
fine.

Thanks,
Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Monday, 24 of November 2008, Zhang Rui wrote:
> On Mon, 2008-11-24 at 09:21 +0800, Rafael J. Wysocki wrote:
> > On Monday, 24 of November 2008, Zhao Yakui wrote:
> > > On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > > > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > > > Hi,
> > > > > 
> > > > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > > > status of the AC adapter is not updated when the adapter is unplugged and
> > > > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > > > 
> > > > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > > > cause this regression to happen.
> > > > > 
> > > > > Any other ideas?
> > > Hi, Rafael
> > >     Will you please open a new bug at
> > > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> > > output of acpidump, dmesg, lspci -vxxx?
> > >     If no ACPI event is reported when AC adapter is unplugged and
> > > plugged, the /sys/class interface can't display the correct AC status.
> > > Now Rui is working on this issue and the patch is already finished. But
> > > it is not sent to Lenb.
> > 
> > Can you give me a link to the patch, please?

Thanks.

Still, it doesn't seem to be related to the problem at hand.

> From: Zhang Rui <rui.zhang@intel.com>
> Subject: power_supply: update ac status before acquiring it.
> 
> Update AC status in sysfs I/F.
> http://bugzilla.kernel.org/show_bug.cgi?id=12035
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/ac.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/acpi/ac.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/ac.c
> +++ linux-2.6/drivers/acpi/ac.c
> @@ -87,6 +87,7 @@ struct acpi_ac {
>  	unsigned long long state;
>  };
>  
> +static int acpi_ac_get_state(struct acpi_ac *ac);
>  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger);
>  
>  #ifdef CONFIG_ACPI_PROCFS_POWER
> @@ -103,15 +104,18 @@ static int get_ac_property(struct power_
>  			   enum power_supply_property psp,
>  			   union power_supply_propval *val)
>  {
> +	int result;
>  	struct acpi_ac *ac = to_acpi_ac(psy);
> +
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_ONLINE:
> +		result = acpi_ac_get_state(ac);
>  		val->intval = ac->state;
>  		break;
>  	default:
>  		return -EINVAL;
>  	}
> -	return 0;
> +	return result;
>  }
>  
>  static enum power_supply_property ac_props[] = {
> 
> >  
> > >     From the problem description it seems that the problem is related
> > > with the AC driver. But from the git-bisect it seems that the problem is
> > > related with the button driver(Fix power button device). 
> > > 
> > >     Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.
> > 
> > Created http://bugzilla.kernel.org/show_bug.cgi?id=12091 with the information
> > attached as requested.
> > 
> I'll take this bug. :)

Thanks again.

Do I understand correctly that you intend to revert commit
faee816b1502385dc9bc5abf2960d1cc645844d1 and provide an alternative patch
for walken?

Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Zhang Rui]

On Mon, 2008-11-24 at 21:30 +0800, Rafael J. Wysocki wrote:
> On Monday, 24 of November 2008, Zhang Rui wrote:
> > On Mon, 2008-11-24 at 09:21 +0800, Rafael J. Wysocki wrote:
> > > On Monday, 24 of November 2008, Zhao Yakui wrote:
> > > > On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > > > > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > > > > status of the AC adapter is not updated when the adapter is unplugged and
> > > > > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > > > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > > > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > > > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > > > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > > > > 
> > > > > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > > > > cause this regression to happen.
> > > > > > 
> > > > > > Any other ideas?
> > > > Hi, Rafael
> > > >     Will you please open a new bug at
> > > > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> > > > output of acpidump, dmesg, lspci -vxxx?
> > > >     If no ACPI event is reported when AC adapter is unplugged and
> > > > plugged, the /sys/class interface can't display the correct AC status.
> > > > Now Rui is working on this issue and the patch is already finished. But
> > > > it is not sent to Lenb.
> > > 
> > > Can you give me a link to the patch, please?
> 
> Thanks.
> 
> Still, it doesn't seem to be related to the problem at hand.
> 
right, it's another problem.

> > From: Zhang Rui <rui.zhang@intel.com>
> > Subject: power_supply: update ac status before acquiring it.
> > 
> > Update AC status in sysfs I/F.
> > http://bugzilla.kernel.org/show_bug.cgi?id=12035
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/ac.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6/drivers/acpi/ac.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/ac.c
> > +++ linux-2.6/drivers/acpi/ac.c
> > @@ -87,6 +87,7 @@ struct acpi_ac {
> >  	unsigned long long state;
> >  };
> >  
> > +static int acpi_ac_get_state(struct acpi_ac *ac);
> >  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger);
> >  
> >  #ifdef CONFIG_ACPI_PROCFS_POWER
> > @@ -103,15 +104,18 @@ static int get_ac_property(struct power_
> >  			   enum power_supply_property psp,
> >  			   union power_supply_propval *val)
> >  {
> > +	int result;
> >  	struct acpi_ac *ac = to_acpi_ac(psy);
> > +
> >  	switch (psp) {
> >  	case POWER_SUPPLY_PROP_ONLINE:
> > +		result = acpi_ac_get_state(ac);
> >  		val->intval = ac->state;
> >  		break;
> >  	default:
> >  		return -EINVAL;
> >  	}
> > -	return 0;
> > +	return result;
> >  }
> >  
> >  static enum power_supply_property ac_props[] = {
> > 
> > >  
> > > >     From the problem description it seems that the problem is related
> > > > with the AC driver. But from the git-bisect it seems that the problem is
> > > > related with the button driver(Fix power button device). 
> > > > 
> > > >     Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.
> > > 
> > > Created http://bugzilla.kernel.org/show_bug.cgi?id=12091 with the information
> > > attached as requested.
> > > 
> > I'll take this bug. :)
> 
> Thanks again.
> 
> Do I understand correctly that you intend to revert commit
> faee816b1502385dc9bc5abf2960d1cc645844d1 and provide an alternative patch
> for walken?
> 
First, I think it's safe to revert
commit faee816b1502385dc9bc5abf2960d1cc645844d1
because bug #10503 can not be reproduced in the upstream kernel as a
result of another commit  49db139955d3392c6c4facf987905d0a9afed581

Second, if we can make sure all the pci devices won't issue any PMEs
when the /sys/devices/.../power/wakeup files are "disabled", then I'd
glad to generate another patch which always enables the PCI root bridge
(PNP0A03) GPE, both at runtime and sleep state, just like what we do for
the buttons.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Zhao Yakui]

On Mon, 2008-11-24 at 21:30 +0800, Rafael J. Wysocki wrote: 
> On Monday, 24 of November 2008, Zhang Rui wrote:
> > On Mon, 2008-11-24 at 09:21 +0800, Rafael J. Wysocki wrote:
> > > On Monday, 24 of November 2008, Zhao Yakui wrote:
> > > > On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > > > > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > > > > status of the AC adapter is not updated when the adapter is unplugged and
> > > > > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > > > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > > > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > > > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > > > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > > > > 
> > > > > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > > > > cause this regression to happen.
> > > > > > 
> > > > > > Any other ideas?
> > > > Hi, Rafael
> > > >     Will you please open a new bug at
> > > > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> > > > output of acpidump, dmesg, lspci -vxxx?
> > > >     If no ACPI event is reported when AC adapter is unplugged and
> > > > plugged, the /sys/class interface can't display the correct AC status.
> > > > Now Rui is working on this issue and the patch is already finished. But
> > > > it is not sent to Lenb.
> > > 
> > > Can you give me a link to the patch, please?
> 
> Thanks.
> 
> Still, it doesn't seem to be related to the problem at hand.
On the R500 box there exist two issues related with AC driver
    a. no ACPI event is reported by AC adapter. The root cause is that
the GPE is shared by several devices(Fake Power button, LID device,HS86,
HS87, ADP1). If the GPE is registered as run_wake type by the fake power
button, the AC adapter can be reported correctly. If the GPE is
registered as wake type, the AC adapter can't be reported correctly.
    In the commit the GPE obtained from the _PRW packge of Power button
device won't be registered as the Fix power button is used instead of
generic power button.
    b. the /sys/class/power_supply/ADP1/online can't report the correct
status of AC adapter.But the /proc/acpi/ interface can report the
correct status even when there is no ACPI event. This can be fixed by
the patch from Rui. 

Thanks.
> 
> > From: Zhang Rui <rui.zhang@intel.com>
> > Subject: power_supply: update ac status before acquiring it.
> > 
> > Update AC status in sysfs I/F.
> > http://bugzilla.kernel.org/show_bug.cgi?id=12035
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/ac.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6/drivers/acpi/ac.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/ac.c
> > +++ linux-2.6/drivers/acpi/ac.c
> > @@ -87,6 +87,7 @@ struct acpi_ac {
> >  	unsigned long long state;
> >  };
> >  
> > +static int acpi_ac_get_state(struct acpi_ac *ac);
> >  #define to_acpi_ac(x) container_of(x, struct acpi_ac, charger);
> >  
> >  #ifdef CONFIG_ACPI_PROCFS_POWER
> > @@ -103,15 +104,18 @@ static int get_ac_property(struct power_
> >  			   enum power_supply_property psp,
> >  			   union power_supply_propval *val)
> >  {
> > +	int result;
> >  	struct acpi_ac *ac = to_acpi_ac(psy);
> > +
> >  	switch (psp) {
> >  	case POWER_SUPPLY_PROP_ONLINE:
> > +		result = acpi_ac_get_state(ac);
> >  		val->intval = ac->state;
> >  		break;
> >  	default:
> >  		return -EINVAL;
> >  	}
> > -	return 0;
> > +	return result;
> >  }
> >  
> >  static enum power_supply_property ac_props[] = {
> > 
> > >  
> > > >     From the problem description it seems that the problem is related
> > > > with the AC driver. But from the git-bisect it seems that the problem is
> > > > related with the button driver(Fix power button device). 
> > > > 
> > > >     Any-way, please attach the output of dmesg, acpidump, lspci -vxxx.
> > > 
> > > Created http://bugzilla.kernel.org/show_bug.cgi?id=12091 with the information
> > > attached as requested.
> > > 
> > I'll take this bug. :)
> 
> Thanks again.
> 
> Do I understand correctly that you intend to revert commit
> faee816b1502385dc9bc5abf2960d1cc645844d1 and provide an alternative patch
> for walken?
> 
> Rafael

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Tuesday, 25 of November 2008, Zhang Rui wrote:
> On Mon, 2008-11-24 at 21:30 +0800, Rafael J. Wysocki wrote:
> > On Monday, 24 of November 2008, Zhang Rui wrote:
[--snip--]
> > 
> > Do I understand correctly that you intend to revert commit
> > faee816b1502385dc9bc5abf2960d1cc645844d1 and provide an alternative patch
> > for walken?
> > 
> First, I think it's safe to revert
> commit faee816b1502385dc9bc5abf2960d1cc645844d1
> because bug #10503 can not be reproduced in the upstream kernel as a
> result of another commit  49db139955d3392c6c4facf987905d0a9afed581

OK, I'll send the revert of faee816b1502385dc9bc5abf2960d1cc645844d1 to Len
later today.

> Second, if we can make sure all the pci devices won't issue any PMEs
> when the /sys/devices/.../power/wakeup files are "disabled",

It is supposed to work like this.  If it doesn't, then there is a bug that has
to be fixed.

> then I'd glad to generate another patch which always enables the PCI root
> bridge (PNP0A03) GPE, both at runtime and sleep state, just like what we do
> for the buttons.

Please go for it.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression: ACPI AC driver doesn't work on Toshiba Portege R500 (bisected)

[Rafael J. Wysocki]

On Tuesday, 25 of November 2008, Zhao Yakui wrote:
> On Mon, 2008-11-24 at 21:30 +0800, Rafael J. Wysocki wrote: 
> > On Monday, 24 of November 2008, Zhang Rui wrote:
> > > On Mon, 2008-11-24 at 09:21 +0800, Rafael J. Wysocki wrote:
> > > > On Monday, 24 of November 2008, Zhao Yakui wrote:
> > > > > On Mon, 2008-11-24 at 07:37 +0800, Rafael J. Wysocki wrote:
> > > > > > On Sunday, 23 of November 2008, Rafael J. Wysocki wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > With current mainline (2.6.28-rc6-git1 as of today) on Toshiba Portege R500 the
> > > > > > > status of the AC adapter is not updated when the adapter is unplugged and
> > > > > > > plugged in.  Evidently, /sys/class/power_supply/ADP1/online always contains the
> > > > > > > same value.  Interestingly enough, though, if the box is suspended to RAM and
> > > > > > > resumed, the status of the AC adapter is correctly updated, but the value read
> > > > > > > at that time remains in /sys/class/power_supply/ADP1/online until the next
> > > > > > > suspend/resume cycle regardless of what's going on with the AC adapter.
> > > > > > > 
> > > > > > > 2.6.27.7 works correctly on this box so the recent EC patches don't seem to
> > > > > > > cause this regression to happen.
> > > > > > > 
> > > > > > > Any other ideas?
> > > > > Hi, Rafael
> > > > >     Will you please open a new bug at
> > > > > http://bugzilla.kernel.org/enter_bug.cgi?product=ACPI and attach the
> > > > > output of acpidump, dmesg, lspci -vxxx?
> > > > >     If no ACPI event is reported when AC adapter is unplugged and
> > > > > plugged, the /sys/class interface can't display the correct AC status.
> > > > > Now Rui is working on this issue and the patch is already finished. But
> > > > > it is not sent to Lenb.
> > > > 
> > > > Can you give me a link to the patch, please?
> > 
> > Thanks.
> > 
> > Still, it doesn't seem to be related to the problem at hand.
> On the R500 box there exist two issues related with AC driver
>     a. no ACPI event is reported by AC adapter. The root cause is that
> the GPE is shared by several devices(Fake Power button, LID device,HS86,
> HS87, ADP1). If the GPE is registered as run_wake type by the fake power
> button, the AC adapter can be reported correctly. If the GPE is
> registered as wake type, the AC adapter can't be reported correctly.
>     In the commit the GPE obtained from the _PRW packge of Power button
> device won't be registered as the Fix power button is used instead of
> generic power button.
>     b. the /sys/class/power_supply/ADP1/online can't report the correct
> status of AC adapter.But the /proc/acpi/ interface can report the
> correct status even when there is no ACPI event. This can be fixed by
> the patch from Rui. 

I see, thanks for the explanation.

The /proc/acpi interface is not compiled in here, so obviously I didn't notice
the issue with it.

Thanks,
Rafael

[PATCH] ACPI: Revert commit faee816b150238 due to regression

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: ACPI: Revert commit faee816b150238 due to regression

Commit faee816b1502385dc9bc5abf2960d1cc645844d1
"ACPI: don't enable control method power button as wakeup device
when Fixed Power button is used" caused the regression tracked
as http://bugzilla.kernel.org/show_bug.cgi?id=12091 (there are no
AC adapter events on Toshiba Portege R500) and the problem this
commit was intended to fix can be fixed differently.

For this reason, the author of the commit, Zhang Rui, has requested
that it be reverted.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/scan.c |   10 ----------
 1 file changed, 10 deletions(-)

Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -751,16 +751,6 @@ static int acpi_bus_get_wakeup_device_fl
 	if (!acpi_match_device_ids(device, button_device_ids))
 		device->wakeup.flags.run_wake = 1;
 
-	/*
-	 * Don't set Power button GPE as run_wake
-	 * if Fixed Power button is used
-	 */
-	if (!strcmp(device->pnp.hardware_id, "PNP0C0C") &&
-		!(acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON)) {
-		device->wakeup.flags.run_wake = 0;
-		device->wakeup.flags.valid = 0;
-	}
-
       end:
 	if (ACPI_FAILURE(status))
 		device->flags.wake_capable = 0;