commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

Just bisected it.

I also tried linux-acpi-next/test, and no change.

The sympthoms are that EC does't sent any GPEs, and therefore battery
insert/removal events don't show up.

It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
With regression the line is shown like this:


/sys/firmware/acpi/interrupts/gpe1C:   1	enabled

Without regression it is

/sys/firmware/acpi/interrupts/gpe1C:   22889	enabled

and steadily increasing.

After suspend/resume, regression disappears.

Best regards,
	Maxim Levitsky

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Len Brown]

On Fri, 11 Jun 2010, Maxim Levitsky wrote:

> Just bisected it.
> 
> I also tried linux-acpi-next/test, and no change.
> 
> The sympthoms are that EC does't sent any GPEs, and therefore battery
> insert/removal events don't show up.
> 
> It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> With regression the line is shown like this:
> 
> 
> /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> 
> Without regression it is
> 
> /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> 
> and steadily increasing.
> 
> After suspend/resume, regression disappears.
> 
> Best regards,
> 	Maxim Levitsky
> 

Woops, as commit 0f849d2cc6863c7874889ea60a871fb71399dd3f
(ACPICA: Minimize the differences between linux GPE code and ACPICA code 
base) shipped in 2.6.35-rc1, this is a regression vs 2.6.34.

thanks,
-Len Brown, Intel Open Source Technology Center

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Rafael J. Wysocki]

On Friday, June 11, 2010, Maxim Levitsky wrote:
> Just bisected it.
> 
> I also tried linux-acpi-next/test, and no change.
> 
> The sympthoms are that EC does't sent any GPEs, and therefore battery
> insert/removal events don't show up.
> 
> It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> With regression the line is shown like this:
> 
> 
> /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> 
> Without regression it is
> 
> /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> 
> and steadily increasing.
> 
> After suspend/resume, regression disappears.

Hmm.

Can you please apply the following patches:

https://patchwork.kernel.org/patch/104903/
https://patchwork.kernel.org/patch/104912/
https://patchwork.kernel.org/patch/104909/
https://patchwork.kernel.org/patch/104911/
https://patchwork.kernel.org/patch/104910/

on top of current -git and see if the problem is still there?

Rafael

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Rafael J. Wysocki]

On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> On Friday, June 11, 2010, Maxim Levitsky wrote:
> > Just bisected it.
> > 
> > I also tried linux-acpi-next/test, and no change.
> > 
> > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > insert/removal events don't show up.
> > 
> > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > With regression the line is shown like this:
> > 
> > 
> > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > 
> > Without regression it is
> > 
> > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > 
> > and steadily increasing.
> > 
> > After suspend/resume, regression disappears.
> 
> Hmm.
> 
> Can you please apply the following patches:
> 
> https://patchwork.kernel.org/patch/104903/
> https://patchwork.kernel.org/patch/104912/
> https://patchwork.kernel.org/patch/104909/
> https://patchwork.kernel.org/patch/104911/
> https://patchwork.kernel.org/patch/104910/
> 
> on top of current -git and see if the problem is still there?

Also, regardless of whether or not this helps, please try to revert only the
changes made by the "guilty" commit in drivers/acpi/acpica/evxface.c and see
if that helps (this revert will conflict with the patches above, so you'll need
to unapply them before).

Thanks,
Rafael

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

On Fri, 2010-06-11 at 21:23 +0200, Rafael J. Wysocki wrote: 
> On Friday, June 11, 2010, Maxim Levitsky wrote:
> > Just bisected it.
> > 
> > I also tried linux-acpi-next/test, and no change.
> > 
> > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > insert/removal events don't show up.
> > 
> > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > With regression the line is shown like this:
> > 
> > 
> > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > 
> > Without regression it is
> > 
> > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > 
> > and steadily increasing.
> > 
> > After suspend/resume, regression disappears.
> 
> Hmm.
> 
> Can you please apply the following patches:
> 
> https://patchwork.kernel.org/patch/104903/
> https://patchwork.kernel.org/patch/104912/
> https://patchwork.kernel.org/patch/104909/
> https://patchwork.kernel.org/patch/104911/
> https://patchwork.kernel.org/patch/104910/
> 
> on top of current -git and see if the problem is still there?
> 
> Rafael

These patch don't apply nether on top of linus tree nor on top of
acpi-test tree (which includes these?)

(Patches weren't damaged I know how to get them from patchwork)

Best regards,
Maxim Levitsky 

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Rafael J. Wysocki]

On Friday, June 11, 2010, Maxim Levitsky wrote:
> On Fri, 2010-06-11 at 21:23 +0200, Rafael J. Wysocki wrote: 
> > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > Just bisected it.
> > > 
> > > I also tried linux-acpi-next/test, and no change.
> > > 
> > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > insert/removal events don't show up.
> > > 
> > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > With regression the line is shown like this:
> > > 
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > 
> > > Without regression it is
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > 
> > > and steadily increasing.
> > > 
> > > After suspend/resume, regression disappears.
> > 
> > Hmm.
> > 
> > Can you please apply the following patches:
> > 
> > https://patchwork.kernel.org/patch/104903/
> > https://patchwork.kernel.org/patch/104912/
> > https://patchwork.kernel.org/patch/104909/
> > https://patchwork.kernel.org/patch/104911/
> > https://patchwork.kernel.org/patch/104910/
> > 
> > on top of current -git and see if the problem is still there?
> > 
> > Rafael
> 
> These patch don't apply nether on top of linus tree nor on top of
> acpi-test tree (which includes these?)

Yes, acpi-test contains them, so you can test that tree.  I'm not sure why
they don't apply on top of the Linus' tree, though.

Rafael

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

On Fri, 2010-06-11 at 22:47 +0300, Maxim Levitsky wrote: 
> On Fri, 2010-06-11 at 21:23 +0200, Rafael J. Wysocki wrote: 
> > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > Just bisected it.
> > > 
> > > I also tried linux-acpi-next/test, and no change.
> > > 
> > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > insert/removal events don't show up.
> > > 
> > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > With regression the line is shown like this:
> > > 
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > 
> > > Without regression it is
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > 
> > > and steadily increasing.
> > > 
> > > After suspend/resume, regression disappears.
> > 
> > Hmm.
> > 
> > Can you please apply the following patches:
> > 
> > https://patchwork.kernel.org/patch/104903/
> > https://patchwork.kernel.org/patch/104912/
> > https://patchwork.kernel.org/patch/104909/
> > https://patchwork.kernel.org/patch/104911/
> > https://patchwork.kernel.org/patch/104910/
> > 
> > on top of current -git and see if the problem is still there?
> > 
> > Rafael
> 
> These patch don't apply nether on top of linus tree nor on top of
> acpi-test tree (which includes these?)

Drat, I was in .patches folder, and this is why these didn't apply... 
> 
> (Patches weren't damaged I know how to get them from patchwork)
> 
> Best regards,
> Maxim Levitsky 
> 

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

On Fri, 2010-06-11 at 21:50 +0200, Rafael J. Wysocki wrote: 
> On Friday, June 11, 2010, Maxim Levitsky wrote:
> > On Fri, 2010-06-11 at 21:23 +0200, Rafael J. Wysocki wrote: 
> > > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > > Just bisected it.
> > > > 
> > > > I also tried linux-acpi-next/test, and no change.
> > > > 
> > > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > > insert/removal events don't show up.
> > > > 
> > > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > > With regression the line is shown like this:
> > > > 
> > > > 
> > > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > > 
> > > > Without regression it is
> > > > 
> > > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > > 
> > > > and steadily increasing.
> > > > 
> > > > After suspend/resume, regression disappears.
> > > 
> > > Hmm.
> > > 
> > > Can you please apply the following patches:
> > > 
> > > https://patchwork.kernel.org/patch/104903/
> > > https://patchwork.kernel.org/patch/104912/
> > > https://patchwork.kernel.org/patch/104909/
> > > https://patchwork.kernel.org/patch/104911/
> > > https://patchwork.kernel.org/patch/104910/
> > > 
> > > on top of current -git and see if the problem is still there?
> > > 
> > > Rafael
> > 
> > These patch don't apply nether on top of linus tree nor on top of
> > acpi-test tree (which includes these?)
> 
> Yes, acpi-test contains them, so you can test that tree.  I'm not sure why
> they don't apply on top of the Linus' tree, though.
I was using acpi-next all the time, so these patches don't help.
I partially reverted the guilty commit, and compile the kernel.


Best regards,
Maxim Levitsky

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

On Fri, 2010-06-11 at 21:43 +0200, Rafael J. Wysocki wrote: 
> On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > Just bisected it.
> > > 
> > > I also tried linux-acpi-next/test, and no change.
> > > 
> > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > insert/removal events don't show up.
> > > 
> > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > With regression the line is shown like this:
> > > 
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > 
> > > Without regression it is
> > > 
> > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > 
> > > and steadily increasing.
> > > 
> > > After suspend/resume, regression disappears.
> > 
> > Hmm.
> > 
> > Can you please apply the following patches:
> > 
> > https://patchwork.kernel.org/patch/104903/
> > https://patchwork.kernel.org/patch/104912/
> > https://patchwork.kernel.org/patch/104909/
> > https://patchwork.kernel.org/patch/104911/
> > https://patchwork.kernel.org/patch/104910/
> > 
> > on top of current -git and see if the problem is still there?
> 
> Also, regardless of whether or not this helps, please try to revert only the
> changes made by the "guilty" commit in drivers/acpi/acpica/evxface.c and see
> if that helps (this revert will conflict with the patches above, so you'll need
> to unapply them before).
> 
> 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

Yes, reverting changes in 'drivers/acpi/acpica/evxface.c' do restore
correct behavior.

Best regards,
Maxim Levitsky

Re: commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Rafael J. Wysocki]

On Friday, June 11, 2010, Maxim Levitsky wrote:
> On Fri, 2010-06-11 at 21:43 +0200, Rafael J. Wysocki wrote: 
> > On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> > > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > > Just bisected it.
> > > > 
> > > > I also tried linux-acpi-next/test, and no change.
> > > > 
> > > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > > insert/removal events don't show up.
> > > > 
> > > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > > With regression the line is shown like this:
> > > > 
> > > > 
> > > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > > 
> > > > Without regression it is
> > > > 
> > > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > > 
> > > > and steadily increasing.
> > > > 
> > > > After suspend/resume, regression disappears.
> > > 
> > > Hmm.
> > > 
> > > Can you please apply the following patches:
> > > 
> > > https://patchwork.kernel.org/patch/104903/
> > > https://patchwork.kernel.org/patch/104912/
> > > https://patchwork.kernel.org/patch/104909/
> > > https://patchwork.kernel.org/patch/104911/
> > > https://patchwork.kernel.org/patch/104910/
> > > 
> > > on top of current -git and see if the problem is still there?
> > 
> > Also, regardless of whether or not this helps, please try to revert only the
> > changes made by the "guilty" commit in drivers/acpi/acpica/evxface.c and see
> > if that helps (this revert will conflict with the patches above, so you'll need
> > to unapply them before).
> > 
> > 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
> 
> Yes, reverting changes in 'drivers/acpi/acpica/evxface.c' do restore
> correct behavior.

Good.  I think we'll need to revert them for .35, then.

I'll prepare a patch and send it to Len.

Rafael

Re: [linux-pm] commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Rafael J. Wysocki]

On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> On Friday, June 11, 2010, Maxim Levitsky wrote:
> > On Fri, 2010-06-11 at 21:43 +0200, Rafael J. Wysocki wrote: 
> > > On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> > > > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > > > Just bisected it.
> > > > > 
> > > > > I also tried linux-acpi-next/test, and no change.
> > > > > 
> > > > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > > > insert/removal events don't show up.
> > > > > 
> > > > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > > > With regression the line is shown like this:
> > > > > 
> > > > > 
> > > > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > > > 
> > > > > Without regression it is
> > > > > 
> > > > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > > > 
> > > > > and steadily increasing.
> > > > > 
> > > > > After suspend/resume, regression disappears.
> > > > 
> > > > Hmm.
> > > > 
> > > > Can you please apply the following patches:
> > > > 
> > > > https://patchwork.kernel.org/patch/104903/
> > > > https://patchwork.kernel.org/patch/104912/
> > > > https://patchwork.kernel.org/patch/104909/
> > > > https://patchwork.kernel.org/patch/104911/
> > > > https://patchwork.kernel.org/patch/104910/
> > > > 
> > > > on top of current -git and see if the problem is still there?
> > > 
> > > Also, regardless of whether or not this helps, please try to revert only the
> > > changes made by the "guilty" commit in drivers/acpi/acpica/evxface.c and see
> > > if that helps (this revert will conflict with the patches above, so you'll need
> > > to unapply them before).
> > > 
> > > 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
> > 
> > Yes, reverting changes in 'drivers/acpi/acpica/evxface.c' do restore
> > correct behavior.
> 
> Good.  I think we'll need to revert them for .35, then.
> 
> I'll prepare a patch and send it to Len.

Can you check if the patch below fixes the issue?

Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: ACPI / ACPICA: Do not attempt to disable GPE when installing handler

Commit 0f849d2cc6863c7874889ea60a871fb71399dd3f (ACPICA: Minimize
the differences between linux GPE code and ACPICA code base)
introduced a change attempting to disable a GPE before installing
a handler for it in acpi_install_gpe_handler() which was incorrect.
First, the GPE disabled by it is never enabled again (except during
resume) which leads to battery insert/remove events not being
reported on the Maxim Levitsky's machine.  Second, the disabled
GPE is still reported as enabled by the sysfs interface that only
checks its enable register's enable_for_run mask.

Revert this change for now, because it causes more damage to happen
than the bug it was supposed to fix.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reported-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/acpi/acpica/evxface.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

Index: linux-2.6/drivers/acpi/acpica/evxface.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evxface.c
+++ linux-2.6/drivers/acpi/acpica/evxface.c
@@ -719,13 +720,6 @@ acpi_install_gpe_handler(acpi_handle gpe
 	handler->context = context;
 	handler->method_node = gpe_event_info->dispatch.method_node;
 
-	/* Disable the GPE before installing the handler */
-
-	status = acpi_ev_disable_gpe(gpe_event_info);
-	if (ACPI_FAILURE (status)) {
-		goto unlock_and_exit;
-	}
-
 	/* Install the handler */
 
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);

Re: [linux-pm] commit 'ACPICA: Minimize the differences between linux GPE code and ACPICA code base' breaks EC GPE on my system

[Maxim Levitsky]

On Fri, 2010-06-11 at 22:59 +0200, Rafael J. Wysocki wrote: 
> On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > On Fri, 2010-06-11 at 21:43 +0200, Rafael J. Wysocki wrote: 
> > > > On Friday, June 11, 2010, Rafael J. Wysocki wrote:
> > > > > On Friday, June 11, 2010, Maxim Levitsky wrote:
> > > > > > Just bisected it.
> > > > > > 
> > > > > > I also tried linux-acpi-next/test, and no change.
> > > > > > 
> > > > > > The sympthoms are that EC does't sent any GPEs, and therefore battery
> > > > > > insert/removal events don't show up.
> > > > > > 
> > > > > > It can be see by doing 'grep  . /sys/firmware/acpi/interrupts/*'
> > > > > > With regression the line is shown like this:
> > > > > > 
> > > > > > 
> > > > > > /sys/firmware/acpi/interrupts/gpe1C:   1	enabled
> > > > > > 
> > > > > > Without regression it is
> > > > > > 
> > > > > > /sys/firmware/acpi/interrupts/gpe1C:   22889	enabled
> > > > > > 
> > > > > > and steadily increasing.
> > > > > > 
> > > > > > After suspend/resume, regression disappears.
> > > > > 
> > > > > Hmm.
> > > > > 
> > > > > Can you please apply the following patches:
> > > > > 
> > > > > https://patchwork.kernel.org/patch/104903/
> > > > > https://patchwork.kernel.org/patch/104912/
> > > > > https://patchwork.kernel.org/patch/104909/
> > > > > https://patchwork.kernel.org/patch/104911/
> > > > > https://patchwork.kernel.org/patch/104910/
> > > > > 
> > > > > on top of current -git and see if the problem is still there?
> > > > 
> > > > Also, regardless of whether or not this helps, please try to revert only the
> > > > changes made by the "guilty" commit in drivers/acpi/acpica/evxface.c and see
> > > > if that helps (this revert will conflict with the patches above, so you'll need
> > > > to unapply them before).
> > > > 
> > > > 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
> > > 
> > > Yes, reverting changes in 'drivers/acpi/acpica/evxface.c' do restore
> > > correct behavior.
> > 
> > Good.  I think we'll need to revert them for .35, then.
> > 
> > I'll prepare a patch and send it to Len.
> 
> Can you check if the patch below fixes the issue?
Yes it does. Thanks!

Best regards,
Maxim Levitsky