Patchwork ACPI / EC: Restart transaction even when the IBF flag set

login
register
mail settings
Submitter Alex Hung
Date June 4, 2013, 9:46 a.m.
Message ID <1370339209-17140-2-git-send-email-alex.hung@canonical.com>
Download mbox | patch
Permalink /patch/249090/
State New
Headers show

Comments

Alex Hung - June 4, 2013, 9:46 a.m.
From: Lan Tianyu <tianyu.lan@intel.com>

The EC driver works abnormally with IBF flag always set.
IBF means "The host has written a byte of data to the command
or data port, but the embedded controller has not yet read it".
If IBF is set in the EC status and not cleared, this will cause
all subsequent EC requests to fail with a timeout error.

Change the EC driver so that it doesn't refuse to restart a
transaction if IBF is set in the status.  Also increase the
number of transaction restarts to 5, as it turns out that 2
is not sufficient in some cases.

This bug happens on several different machines (Asus V1S,
Dell Latitude E6530, Samsung R719, Acer Aspire 5930G,
Sony Vaio SR19VN and others).

[rjw: Changelog]
References: https://bugzilla.kernel.org/show_bug.cgi?id=14733
References: https://bugzilla.kernel.org/show_bug.cgi?id=15560
References: https://bugzilla.kernel.org/show_bug.cgi?id=15946
References: https://bugzilla.kernel.org/show_bug.cgi?id=42945
References: https://bugzilla.kernel.org/show_bug.cgi?id=48221
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Cc: All <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
(cherry picked from commit 28fe5c825f8e15744d04c7c1b8df197950923ecd)

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/acpi/ec.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Luis Henriques - June 5, 2013, 2:57 p.m.
Alex Hung <alex.hung@canonical.com> writes:

> From: Lan Tianyu <tianyu.lan@intel.com>
>
> The EC driver works abnormally with IBF flag always set.
> IBF means "The host has written a byte of data to the command
> or data port, but the embedded controller has not yet read it".
> If IBF is set in the EC status and not cleared, this will cause
> all subsequent EC requests to fail with a timeout error.
>
> Change the EC driver so that it doesn't refuse to restart a
> transaction if IBF is set in the status.  Also increase the
> number of transaction restarts to 5, as it turns out that 2
> is not sufficient in some cases.
>
> This bug happens on several different machines (Asus V1S,
> Dell Latitude E6530, Samsung R719, Acer Aspire 5930G,
> Sony Vaio SR19VN and others).
>
> [rjw: Changelog]
> References: https://bugzilla.kernel.org/show_bug.cgi?id=14733
> References: https://bugzilla.kernel.org/show_bug.cgi?id=15560
> References: https://bugzilla.kernel.org/show_bug.cgi?id=15946
> References: https://bugzilla.kernel.org/show_bug.cgi?id=42945
> References: https://bugzilla.kernel.org/show_bug.cgi?id=48221
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> Cc: All <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> (cherry picked from commit 28fe5c825f8e15744d04c7c1b8df197950923ecd)

Besides the fact that this commit doesn't contain the buglink (its
only in the cover email), it is already present in both Q and R
kernels.

Cheers,

Patch

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 354007d..a563e11 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -223,7 +223,7 @@  static int ec_check_sci_sync(struct acpi_ec *ec, u8 state)
 static int ec_poll(struct acpi_ec *ec)
 {
 	unsigned long flags;
-	int repeat = 2; /* number of command restarts */
+	int repeat = 5; /* number of command restarts */
 	while (repeat--) {
 		unsigned long delay = jiffies +
 			msecs_to_jiffies(ec_delay);
@@ -241,8 +241,6 @@  static int ec_poll(struct acpi_ec *ec)
 			}
 			advance_transaction(ec, acpi_ec_read_status(ec));
 		} while (time_before(jiffies, delay));
-		if (acpi_ec_read_status(ec) & ACPI_EC_FLAG_IBF)
-			break;
 		pr_debug(PREFIX "controller reset, restart transaction\n");
 		spin_lock_irqsave(&ec->lock, flags);
 		start_transaction(ec);