Re: [PATCH] acpi: Avoid dropping rapid hotkey events (or other GPEs) on Asus EeePC

[Alan Jenkins <alan-jenkins@tuffmail.co.uk>]

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

[Dupe apology: CC'd to stable@kernel.org, with the right address this time]

Andrew Morton wrote:
> On Wed, 13 Aug 2008 11:21:10 +0100 Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>
>   
>> Andrew Morton wrote:
>>     
>>> Did this get fixed yet?
>>>
>>> I have an patch in -mm which I just restored (I had to tempdrop it
>>> because the acpi tree was busted for some time).  But it seems to be
>>> old.
>>>
>>> http://bugzilla.kernel.org/show_bug.cgi?id=10919 is marked "resolved"
>>> but the reporter (Maximilian) seems to think otherwise.  2.6.26.x is,
>>> afaik, still unfixed, as is 2.6.27-rc.
>>>   
>>>       
>> That's correct.  I think this specific patch should go in 2.6.27 and
>> 2.6.26-stable.  No objections have been raised so far.
>>
>> I still need this patch to make my brightness and volume control keys
>> usable in 2.6.27-rc3.  (They auto-repeat fast enough to trigger the
>> bug).  This is true even after applying the latest patches from bug
>> 10919 (#25 + #27).
>>
>>     
>
> Confusing.  Please send the patch which you think we should apply.
>
>   
>> I think the 10919 fix makes it harder to reproduce, but it definitely
>> still happens.  I guess this is because the polling-driven EC
>> transactions add 1ms delays between each byte.  The slower timings leave
>> a window where the buggy behaviour of my EC can make a difference.  (It
>> has been seen to clear the "pending event" bit after a single event is
>> read, despite having more events pending).
>>
>> There are more serious consequences of this bug.  After a while it can
>> confuse the EC enough to cause lockups or reboots during boot, or after
>> pressing a single hotkey.  This bad state is preserved over reboots,
>> even into known good kernels.  Fortunately the badness clears when power
>> is removed for a long enough period.  For a while I was worried that
>> something had physically burnt out.
>>     
>
> Oh gad.  And there's no workaround?
>
Sorry, that was confusing.

The patch in currently in -mm _is_ the workaround for this damage.  It
was not initially obvious just how important it was :-).  I've
re-attached it as requested.

10919, "laggy hotkeys" is just what it says; ACPI EC events are slower
because of polling.  It appears to be a more cosmetic issue which is
orthogonal to the _dropping_ of events.

Thanks
Alan


[-- Attachment #2: Attached Message --]
[-- Type: message/rfc822, Size: 2636 bytes --]

From: Alan Jenkins <aj504@student.cs.york.ac.uk>
To: astarikovskiy@suse.de
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] acpi: Avoid dropping rapid hotkey events (or other GPEs) on Asus EeePC
Date: Tue, 15 Jul 2008 23:25:07 +0100
Message-ID: <487D23C3.5070301@student.cs.york.ac.uk>

It looks like this EC clears the SMI_EVT bit after every query, even if there
are more events pending.  The workaround is to repeatedly query the EC until
it reports that no events remain.

This fixes a regression in 2.6.26 (from 2.6.25.3).  Initially reported as
"Asus Eee PC hotkeys stop working if pressed quickly" in bugzilla
<http://bugzilla.kernel.org/show_bug.cgi?id=11089>.

The regression was caused by a recently added check for interrupt storms.
The Eee PC triggers this check and switches to polling.  When multiple events
arrive between polling intervals, only one is fetched from the EC.  This causes
erroneous behaviour; ultimately events stop being delivered altogether when the
EC buffer overflows.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

---
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 5622aee..2b4c5a2 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -459,14 +459,10 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit)
 
 EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler);
 
-static void acpi_ec_gpe_query(void *ec_cxt)
+static void acpi_ec_gpe_run_handler(struct acpi_ec *ec, u8 value)
 {
-	struct acpi_ec *ec = ec_cxt;
-	u8 value = 0;
 	struct acpi_ec_query_handler *handler, copy;
 
-	if (!ec || acpi_ec_query(ec, &value))
-		return;
 	mutex_lock(&ec->lock);
 	list_for_each_entry(handler, &ec->list, node) {
 		if (value == handler->query_bit) {
@@ -484,6 +480,18 @@ static void acpi_ec_gpe_query(void *ec_cxt)
 	mutex_unlock(&ec->lock);
 }
 
+static void acpi_ec_gpe_query(void *ec_cxt)
+{
+	struct acpi_ec *ec = ec_cxt;
+	u8 value = 0;
+
+	if (!ec)
+		return;
+
+	while (!acpi_ec_query(ec, &value))
+		acpi_ec_gpe_run_handler(ec, value);
+}
+
 static u32 acpi_ec_gpe_handler(void *data)
 {
 	acpi_status status = AE_OK;