diff mbox

[27/30] ACPI: EC: Add a quirk for CLEVO M720T/M730T laptop

Message ID 67bfa9b60bd689601554526d144b21d529f78a09.1349761836.git.len.brown@intel.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Len Brown Oct. 9, 2012, 5:54 a.m. UTC
From: Feng Tang <feng.tang@intel.com>

By enlarging the GPE storm threshold back to 20, that laptop's
EC works fine with interrupt mode instead of polling mode.

https://bugzilla.kernel.org/show_bug.cgi?id=45151

Reported-and-Tested-by: Francesco <trentini@dei.unipd.it>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
cc: stable@vger.kernel.org
---
 drivers/acpi/ec.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Jonathan Nieder Oct. 9, 2012, 6:16 a.m. UTC | #1
Len Brown wrote:

> By enlarging the GPE storm threshold back to 20, that laptop's
> EC works fine with interrupt mode instead of polling mode.

What would go wrong if the threshold were just increased to 20 on all
models?

Thanks,
Jonathan
--
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
Feng Tang Oct. 9, 2012, 6:22 a.m. UTC | #2
Hi Jonathan,

On Tue, 9 Oct 2012 14:16:33 +0800
Jonathan Nieder <jrnieder@gmail.com> wrote:

> Len Brown wrote:
> 
> > By enlarging the GPE storm threshold back to 20, that laptop's
> > EC works fine with interrupt mode instead of polling mode.
> 
> What would go wrong if the threshold were just increased to 20 on all
> models?

Then some other platform will stop to work.
	https://bugzilla.kernel.org/show_bug.cgi?id=11892 

The 26/30 and 27/30 patches are bound together to fix one bug. Some
quote from description from the 26th patch:

ACPI_EC_STORM_THRESHOLD was initially 20 when it's created, and
was changed to 8 in 2.6.28 commit 06cf7d3c7 "ACPI: EC: lower interrupt storm
threshold" to fix kernel bug 11892 by forcing the laptop in that bug to
work in polling mode.

Hope this answers your question.

Thanks,
Feng
--
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
Jonathan Nieder Oct. 9, 2012, 6:59 a.m. UTC | #3
Feng Tang wrote:
> Jonathan Nieder <jrnieder@gmail.com> wrote:

>> What would go wrong if the threshold were just increased to 20 on all
>> models?
>
> Then some other platform will stop to work.
>	https://bugzilla.kernel.org/show_bug.cgi?id=11892 
>
> The 26/30 and 27/30 patches are bound together to fix one bug. Some
> quote from description from the 26th patch:
>
> ACPI_EC_STORM_THRESHOLD was initially 20 when it's created, and
> was changed to 8 in 2.6.28 commit 06cf7d3c7 "ACPI: EC: lower interrupt storm
> threshold" to fix kernel bug 11892 by forcing the laptop in that bug to
> work in polling mode.
>
> Hope this answers your question.

Thanks much.  Yes, that clarifies.

The magic numbers are not too thrilling.  If the polling mode just
doesn't work on the Clevo M720, why isn't the appropriate storm
threshold 999999 or infinity rather than 20?  Do we know why the
polling mode doesn't work?

Jonathan
--
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
Feng Tang Oct. 9, 2012, 7:19 a.m. UTC | #4
On Mon, 8 Oct 2012 23:59:07 -0700
Jonathan Nieder <jrnieder@gmail.com> wrote:

> Feng Tang wrote:
> > Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> >> What would go wrong if the threshold were just increased to 20 on all
> >> models?
> >
> > Then some other platform will stop to work.
> >	https://bugzilla.kernel.org/show_bug.cgi?id=11892 
> >
> > The 26/30 and 27/30 patches are bound together to fix one bug. Some
> > quote from description from the 26th patch:
> >
> > ACPI_EC_STORM_THRESHOLD was initially 20 when it's created, and
> > was changed to 8 in 2.6.28 commit 06cf7d3c7 "ACPI: EC: lower interrupt storm
> > threshold" to fix kernel bug 11892 by forcing the laptop in that bug to
> > work in polling mode.
> >
> > Hope this answers your question.
> 
> Thanks much.  Yes, that clarifies.
> 
> The magic numbers are not too thrilling.  If the polling mode just
> doesn't work on the Clevo M720, why isn't the appropriate storm
> threshold 999999 or infinity rather than 20?  Do we know why the
> polling mode doesn't work?

I don't know why it doesn't work, if you check the https://bugzilla.kernel.org/show_bug.cgi?id=45151 
you'll see the debugging model is
   test result --> patch --> 1-2 weeks + result --> patch
    --> 1-2 weeks + result ...
over and over, which makes it difficult to root cause it but
provide a workaround.

And frankly speaking, I'm not sure if I can figure it out 100%
even if I had that HW at hand. As per my understanding, EC is
very tricky, as OS, ACPI FW, EC FW, BIOS will all access it
without a global lock (in most cases), which makes it hard
to work properly without race condition. Not mentioning its
hardware may be broken.

Thanks,
Feng 
--
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
diff mbox

Patch

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 615264c..a51df96 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -930,6 +930,17 @@  static int ec_flag_msi(const struct dmi_system_id *id)
 	return 0;
 }
 
+/*
+ * Clevo M720 notebook actually works ok with IRQ mode, if we lifted
+ * the GPE storm threshold back to 20
+ */
+static int ec_enlarge_storm_threshold(const struct dmi_system_id *id)
+{
+	pr_debug("Setting the EC GPE storm threshold to 20\n");
+	ec_storm_threshold  = 20;
+	return 0;
+}
+
 static struct dmi_system_id __initdata ec_dmi_table[] = {
 	{
 	ec_skip_dsdt_scan, "Compal JFL92", {
@@ -961,10 +972,13 @@  static struct dmi_system_id __initdata ec_dmi_table[] = {
 	{
 	ec_validate_ecdt, "ASUS hardware", {
 	DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK Computer Inc.") }, NULL},
+	{
+	ec_enlarge_storm_threshold, "CLEVO hardware", {
+	DMI_MATCH(DMI_SYS_VENDOR, "CLEVO Co."),
+	DMI_MATCH(DMI_PRODUCT_NAME, "M720T/M730T"),}, NULL},
 	{},
 };
 
-
 int __init acpi_ec_ecdt_probe(void)
 {
 	acpi_status status;