[PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Zhao Yakui]

Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC 
From: Zhao Yakui <yakui.zhao@intel.com>

On some broken BIOS the ACPI object in EC _REG method can't be found in
ACPI namespace, which causes that the AE_NOT_FOUND status code is returned by
the EC _REG object. In such case the EC device can't be initialized correctly, 
which causes that battery/AC adapter can't work normally. As the EC address
space handler is not removed and the memory pointed by its input argument is
already free, sometimes the kernel will also be panic when EC internal register
is still accessed. But the windows can work well on such broken BIOS.

Maybe it will be reasonable that OS ignores the AE_NOT_FOUND error 
returned by the EC _REG object and continues to initialize EC device 
for some broken BIOS. Of course the warning message will be printed.
For example: the ACPI object in EC _REG method can't be found and status error
code is AE_NOT_FOUND. 

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

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>

---
 drivers/acpi/ec.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/acpi/ec.c
===================================================================
--- linux-2.6.orig/drivers/acpi/ec.c
+++ linux-2.6/drivers/acpi/ec.c
@@ -835,8 +835,19 @@ static int ec_install_handlers(struct ac
 						    &acpi_ec_space_handler,
 						    NULL, ec);
 	if (ACPI_FAILURE(status)) {
-		acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler);
-		return -ENODEV;
+		if (status == AE_NOT_FOUND) {
+			/*
+			 * Maybe OS fails in evaluating the _REG object.
+			 * The AE_NOT_FOUND error will be ignored and OS
+			 * continue to initialize EC.
+			 */
+			printk(KERN_ERR "Fail in evaluating _REG object."
+				" It is broken BIOS.\n");
+		} else {
+			acpi_remove_gpe_handler(NULL, ec->gpe,
+				&acpi_ec_gpe_handler);
+			return -ENODEV;
+		}
 	}
 
 	ec->handlers_installed = 1;

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Andi Kleen]

On Mon, Jul 28, 2008 at 11:25:50AM +0800, Zhao Yakui wrote:
> Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC 
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> On some broken BIOS the ACPI object in EC _REG method can't be found in
> ACPI namespace, which causes that the AE_NOT_FOUND status code is returned by
> the EC _REG object. In such case the EC device can't be initialized correctly, 

Thanks. Queued for test. How urgent is this? Do we need to consider it 
for .27?

-Andi

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Zhao Yakui]

On Mon, 2008-07-28 at 16:59 +0200, Andi Kleen wrote:
> On Mon, Jul 28, 2008 at 11:25:50AM +0800, Zhao Yakui wrote:
> > Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC 
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > On some broken BIOS the ACPI object in EC _REG method can't be found in
> > ACPI namespace, which causes that the AE_NOT_FOUND status code is returned by
> > the EC _REG object. In such case the EC device can't be initialized correctly, 
> 
> Thanks. Queued for test. How urgent is this? Do we need to consider it 
> for .27?
Not very urgent. It is a workaround patch for some broken BIOS. Of course it is also OK for .27.

Windows can work well on the broken BIOS(the error is ignored by
windows). Maybe we can also ignore such error and continue to initialize
EC.

Thanks.
> -Andi

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Joshua C.]

On Mon, Jul 28, 2008 at 14:59:26AM +0800, Andi Kleen wrote:
> >Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC
> >From: Zhao Yakui <yakui.zhao@intel.com>
>Thanks. Queued for test. How urgent is this? Do we need to consider it
>for .27?
>
>-Andi

It will be great if you can put this in .27.

--joshua

RE: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Moore, Robert]

Looking at this patch, I see that it solves a very specific case, namely
the AE_NOT_FOUND exception. However, this is just one of many exceptions
that we may want to ignore. In fact, I wonder if in the general case we
should just ignore all exceptions from _REG methods, other than printing
some kind of message.

Bob


>-----Original Message-----
>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>owner@vger.kernel.org] On Behalf Of Joshua C.
>Sent: Wednesday, July 30, 2008 7:03 PM
>To: linux-acpi@vger.kernel.org
>Subject: Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method
and
>continue to initialize EC
>
>On Mon, Jul 28, 2008 at 14:59:26AM +0800, Andi Kleen wrote:
>> >Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and
continue
>to initialize EC
>> >From: Zhao Yakui <yakui.zhao@intel.com>
>>Thanks. Queued for test. How urgent is this? Do we need to consider it
>>for .27?
>>
>>-Andi
>
>It will be great if you can put this in .27.
>
>--joshua
>--
>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: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Kleen, Andi]

 

>Looking at this patch, I see that it solves a very specific 
>case, namely the AE_NOT_FOUND exception. However, this is just 
>one of many exceptions that we may want to ignore. In fact, I 
>wonder if in the general case we should just ignore all 
>exceptions from _REG methods, other than printing some kind of message.

You mean the exceptions should be silently ignored?  I think ignoring
exceptions on more methods is probably a good idea.

But at least some kind of warning would be probably good, wouldn't it?

-Andi

RE: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Moore, Robert]

>>just ignore all
>>exceptions from _REG methods, other than printing some kind of
message.

Yes, I mean that some kind of warning would be appropriate.

There may be many places where we don't want to simply give up because
some method failed.

>-----Original Message-----
>From: Kleen, Andi
>Sent: Wednesday, July 30, 2008 7:20 PM
>To: Moore, Robert; 'Joshua C.'; 'linux-acpi@vger.kernel.org'
>Cc: Lin, Ming M
>Subject: RE: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method
and
>continue to initialize EC
>
>
>
>>Looking at this patch, I see that it solves a very specific
>>case, namely the AE_NOT_FOUND exception. However, this is just
>>one of many exceptions that we may want to ignore. In fact, I
>>wonder if in the general case we should just ignore all
>>exceptions from _REG methods, other than printing some kind of
message.
>
>You mean the exceptions should be silently ignored?  I think ignoring
>exceptions on more methods is probably a good idea.
>
>But at least some kind of warning would be probably good, wouldn't it?
>
>-Andi

RE: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Zhao Yakui]

On Wed, 2008-07-30 at 19:11 -0700, Moore, Robert wrote:
> Looking at this patch, I see that it solves a very specific case, namely
> the AE_NOT_FOUND exception. However, this is just one of many exceptions
> that we may want to ignore. In fact, I wonder if in the general case we
> should just ignore all exceptions from _REG methods, other than printing
> some kind of message.
Yes.  This patch only solves the specific case. For example: in the bug
8953, 10237.

But if all the exceptions from the _REG methods are ignored, it will be
too aggressive. Maybe some potential problems will be hidden. We had
better wait for that more people complain the similar bugs.

Thanks.

> 
> Bob
> 
> 
> >-----Original Message-----
> >From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> >owner@vger.kernel.org] On Behalf Of Joshua C.
> >Sent: Wednesday, July 30, 2008 7:03 PM
> >To: linux-acpi@vger.kernel.org
> >Subject: Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method
> and
> >continue to initialize EC
> >
> >On Mon, Jul 28, 2008 at 14:59:26AM +0800, Andi Kleen wrote:
> >> >Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and
> continue
> >to initialize EC
> >> >From: Zhao Yakui <yakui.zhao@intel.com>
> >>Thanks. Queued for test. How urgent is this? Do we need to consider it
> >>for .27?
> >>
> >>-Andi
> >
> >It will be great if you can put this in .27.
> >
> >--joshua
> >--
> >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
> --
> 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: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Joshua C.]

2008/7/31 Zhao Yakui <yakui.zhao@intel.com>:
> Yes.  This patch only solves the specific case. For example: in the bug
> 8953, 10237.
>
> But if all the exceptions from the _REG methods are ignored, it will be
> too aggressive. Maybe some potential problems will be hidden. We had
> better wait for that more people complain the similar bugs.
>
> Thanks.
>
 I agree with that. Maybe not all exceptions should be ignored.

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Alexey Starikovskiy]

NAK.

As already mentioned, there is no point to put this workaround in ec.c.
Instead, it should be placed into acpi_register_operation_region, or may 
be even
higher (like a flag to ignore errors during execution of the method).

Regards,
Alex.

Zhao Yakui wrote:
> On Wed, 2008-07-30 at 19:11 -0700, Moore, Robert wrote:
>   
>> Looking at this patch, I see that it solves a very specific case, namely
>> the AE_NOT_FOUND exception. However, this is just one of many exceptions
>> that we may want to ignore. In fact, I wonder if in the general case we
>> should just ignore all exceptions from _REG methods, other than printing
>> some kind of message.
>>     
> Yes.  This patch only solves the specific case. For example: in the bug
> 8953, 10237.
>
> But if all the exceptions from the _REG methods are ignored, it will be
> too aggressive. Maybe some potential problems will be hidden. We had
> better wait for that more people complain the similar bugs.
>
> Thanks.
>
>   
>> Bob
>>
>>
>>     
>>> -----Original Message-----
>>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>>> owner@vger.kernel.org] On Behalf Of Joshua C.
>>> Sent: Wednesday, July 30, 2008 7:03 PM
>>> To: linux-acpi@vger.kernel.org
>>> Subject: Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method
>>>       
>> and
>>     
>>> continue to initialize EC
>>>
>>> On Mon, Jul 28, 2008 at 14:59:26AM +0800, Andi Kleen wrote:
>>>       
>>>>> Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and
>>>>>           
>> continue
>>     
>>> to initialize EC
>>>       
>>>>> From: Zhao Yakui <yakui.zhao@intel.com>
>>>>>           
>>>> Thanks. Queued for test. How urgent is this? Do we need to consider it
>>>> for .27?
>>>>
>>>> -Andi
>>>>         
>>> It will be great if you can put this in .27.
>>>
>>> --joshua
>>> --
>>> 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
>>>       
>> --
>> 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
>>     
>
> --
> 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: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Kleen, Andi]

 

>As already mentioned, there is no point to put this workaround in ec.c.
>Instead, it should be placed into 
>acpi_register_operation_region, or may 
>be even
>higher (like a flag to ignore errors during execution of the method).

Ok I will remove the patch again.  But we'll need an replacement patch
that does all this in a general way.

-Andi

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Zhao Yakui]

On Thu, 2008-07-31 at 23:13 +0400, Alexey Starikovskiy wrote:
> NAK.
> 
> As already mentioned, there is no point to put this workaround in ec.c.
> Instead, it should be placed into acpi_register_operation_region, or may 
> be even
> higher (like a flag to ignore errors during execution of the method).
Maybe what your said is right. The generic method about ignoring the
error during the execution of the method is very good. But it will
become more complicated. Maybe it is too aggressive and will have other
side effect that we can't know. 

As the acpi_install_address_space_handler is realized in ACPICA and
called by other drivers, it is not appropriate to ignore the error in
the generic ACPICA function. At the same time this error is related with
EC device, maybe it is more appropriate that EC driver determines
whether OS continues to initialize EC device.

IMO this workaround patch can fix the issue that is related with the EC
_REG method(For example: bug 8953, 10237). At the same time this will
have no effect on the other systems. 

best regards.
   Yakui

> Regards,
> Alex.
> 
> Zhao Yakui wrote:
> > On Wed, 2008-07-30 at 19:11 -0700, Moore, Robert wrote:
> >   
> >> Looking at this patch, I see that it solves a very specific case, namely
> >> the AE_NOT_FOUND exception. However, this is just one of many exceptions
> >> that we may want to ignore. In fact, I wonder if in the general case we
> >> should just ignore all exceptions from _REG methods, other than printing
> >> some kind of message.
> >>     
> > Yes.  This patch only solves the specific case. For example: in the bug
> > 8953, 10237.
> >
> > But if all the exceptions from the _REG methods are ignored, it will be
> > too aggressive. Maybe some potential problems will be hidden. We had
> > better wait for that more people complain the similar bugs.
> >
> > Thanks.
> >
> >   
> >> Bob
> >>
> >>
> >>     
> >>> -----Original Message-----
> >>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> >>> owner@vger.kernel.org] On Behalf Of Joshua C.
> >>> Sent: Wednesday, July 30, 2008 7:03 PM
> >>> To: linux-acpi@vger.kernel.org
> >>> Subject: Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method
> >>>       
> >> and
> >>     
> >>> continue to initialize EC
> >>>
> >>> On Mon, Jul 28, 2008 at 14:59:26AM +0800, Andi Kleen wrote:
> >>>       
> >>>>> Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and
> >>>>>           
> >> continue
> >>     
> >>> to initialize EC
> >>>       
> >>>>> From: Zhao Yakui <yakui.zhao@intel.com>
> >>>>>           
> >>>> Thanks. Queued for test. How urgent is this? Do we need to consider it
> >>>> for .27?
> >>>>
> >>>> -Andi
> >>>>         
> >>> It will be great if you can put this in .27.
> >>>
> >>> --joshua
> >>> --
> >>> 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
> >>>       
> >> --
> >> 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
> >>     
> >
> > --
> > 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: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Joshua C.]

2008/8/1 Zhao Yakui <yakui.zhao@intel.com>:
> On Thu, 2008-07-31 at 23:13 +0400, Alexey Starikovskiy wrote:
>> NAK.
>>
>> As already mentioned, there is no point to put this workaround in ec.c.
>> Instead, it should be placed into acpi_register_operation_region, or may
>> be even
>> higher (like a flag to ignore errors during execution of the method).
> Maybe what your said is right. The generic method about ignoring the
> error during the execution of the method is very good. But it will
> become more complicated. Maybe it is too aggressive and will have other
> side effect that we can't know.
>
> As the acpi_install_address_space_handler is realized in ACPICA and
> called by other drivers, it is not appropriate to ignore the error in
> the generic ACPICA function. At the same time this error is related with
> EC device, maybe it is more appropriate that EC driver determines
> whether OS continues to initialize EC device.
>
> IMO this workaround patch can fix the issue that is related with the EC
> _REG method(For example: bug 8953, 10237). At the same time this will
> have no effect on the other systems.
>
> best regards.
>   Yakui

I  also support these arguments. The supposed method will complicate
the whole process and take more time to be realised. For now this
patch has effekt only on relatively small number of machines that are
related to bugs 8953 and 10237. Otherwise, it won't affect other
systems. So I don't see any obstacles to putting this in the
mainstream.

--joshua

[PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Zhao Yakui]

Subject:ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC 
From: Zhao Yakui <yakui.zhao@intel.com>

On some broken BIOS the ACPI object in EC _REG method can't be found in
ACPI namespace, which causes that the status code of AE_NOT_FOUND is returned by
the EC _REG object. In such case the EC device can't be initialized correctly, 
which causes that battery/AC adapter can't work normally. As the EC address
space handler is not removed and the memory pointed by its input argument is
already free, sometimes the kernel will also be panic when EC internal register
is still accessed. But the windows can work well on such broken BIOS.

Maybe it will be reasonable that OS ignores the AE_NOT_FOUND error 
returned by the EC _REG object and continues to initialize EC device 
on some broken BIOS. 
For example: the ACPI object in EC _REG method can't be found and status error
code is AE_NOT_FOUND. 

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

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>

---
 drivers/acpi/ec.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/acpi/ec.c
===================================================================
--- linux-2.6.orig/drivers/acpi/ec.c
+++ linux-2.6/drivers/acpi/ec.c
@@ -862,8 +862,19 @@ static int ec_install_handlers(struct ac
 						    &acpi_ec_space_handler,
 						    NULL, ec);
 	if (ACPI_FAILURE(status)) {
-		acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler);
-		return -ENODEV;
+		if (status == AE_NOT_FOUND) {
+			/*
+			 * Maybe OS fails in evaluating the _REG object.
+			 * The AE_NOT_FOUND error will be ignored and OS
+			 * continue to initialize EC.
+			 */
+			printk(KERN_ERR "Fail in evaluating the _REG object"
+				" of EC device. Broken bios is suspected.\n");
+		} else {
+			acpi_remove_gpe_handler(NULL, ec->gpe,
+				&acpi_ec_gpe_handler);
+			return -ENODEV;
+		}
 	}
 
 	ec->handlers_installed = 1;

Re: [PATCH] ACPI: Ignore AE_NOT_FOUND error of EC _REG method and continue to initialize EC

[Len Brown]

I'm inclined to include this b/c it fixes the problems at hand
and we don't have a better solution at the moment.

When we have a more general solution, we can always revert this.

thanks,
-Len