It's reported that Asus laptop X580VD/X550VXK/GL720VMK/FX502VD/FX502VE have
a BIOS bug where the ECDT correctly states that EC events trigger GPE 0x23,
but the DSDT _GPE method incorrectly returns GPE 0x33.
This patchset fixes this issue.
Link: https://www.spinics.net/lists/linux-acpi/msg73763.html
https://bugzilla.kernel.org/show_bug.cgi?id=195651
v2: Stops doing craps related to EC_ID (it's already too complicated), and
reduces one unnecessary boot parameter. For the final bug fix, prefers
the fix from endlessm.com developers.
v3: Removes improper statments related to _STA as acpi_get_devices() has
executed _STA before invoking ec_parse_device() adds detailed
explanation for why acpi_ec_dsdt_probe() is a wrong approach.
Carlo Caione helps to add a new quirk for GL720VMK.
Carlo Caione (1):
ACPI / EC: Add quirk for GL720VMK
Chris Chiu (1):
ACPI / EC: Fix media keys not working problem on some Asus laptops
Lv Zheng (2):
ACPI / EC: Enhance boot EC sanity check
ACPI / EC: Add support to skip boot stage DSDT probe
drivers/acpi/ec.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 71 insertions(+), 6 deletions(-)
--
2.7.4
It's reported that some buggy BIOS tables can contain 2 DSDT ECs, one of
them is invalid but acpi_ec_dsdt_probe() fails to pick the valid one.
This patch simply enhances sanity checks in ec_parse_device() as a
workaround to skip probing wrong namespace ECs.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=195651
Tested-by: Daniel Drake <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/ec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index c24235d..c1f480b 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1362,6 +1362,8 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
ec_parse_io_ports, ec);
if (ACPI_FAILURE(status))
return status;
+ if (ec->data_addr == 0 || ec->command_addr == 0)
+ return AE_OK;
/* Get GPE bit assignment (EC events). */
/* TODO: Add support for _GPE returning a package */
--
2.7.4
From: Chris Chiu <[email protected]>
Some Asus laptops (verified on X550VXK/FX502VD/FX502VE) get no
interrupts when pressing media keys thus the corresponding functions
are not invoked. It's due to the _GPE defines in DSDT for EC returns
differnt value compared to the GPE Number in ECDT. Confirmed with Asus
that the vale in ECDT is the correct one. This commit uses DMI quirks
to prevent calling _GPE when doing ec_parse_device() and keep the ECDT
GPE number setting for the EC device.
With previous commit, it is ensured that if there is an ECDT, it can always
be kept as boot_ec, this patch thus can implement the quirk on top of the
determined ECDT boot_ec.
Link: https://phabricator.endlessm.com/T16033
https://phabricator.endlessm.com/T16722
Link: https://bugzilla.kernel.org/show_bug.cgi?id=195651
Tested-by: Daniel Drake <[email protected]>
Signed-off-by: Chris Chiu <[email protected]>
Signed-off-by: Carlo Caione <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/ec.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 44b973e..2189048 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -190,6 +190,7 @@ static struct workqueue_struct *ec_query_wq;
static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */
static int EC_FLAGS_CORRECT_ECDT; /* Needs ECDT port address correction */
+static int EC_FLAGS_IGNORE_DSDT_GPE; /* Needs ECDT GPE as correction setting */
/* --------------------------------------------------------------------------
* Logging/Debugging
@@ -1365,12 +1366,20 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
if (ec->data_addr == 0 || ec->command_addr == 0)
return AE_OK;
- /* Get GPE bit assignment (EC events). */
- /* TODO: Add support for _GPE returning a package */
- status = acpi_evaluate_integer(handle, "_GPE", NULL, &tmp);
- if (ACPI_FAILURE(status))
- return status;
- ec->gpe = tmp;
+ if (boot_ec && boot_ec_is_ecdt && EC_FLAGS_IGNORE_DSDT_GPE) {
+ /*
+ * Always inherit the GPE number setting from the ECDT
+ * EC.
+ */
+ ec->gpe = boot_ec->gpe;
+ } else {
+ /* Get GPE bit assignment (EC events). */
+ /* TODO: Add support for _GPE returning a package */
+ status = acpi_evaluate_integer(handle, "_GPE", NULL, &tmp);
+ if (ACPI_FAILURE(status))
+ return status;
+ ec->gpe = tmp;
+ }
/* Use the global lock for all EC transactions? */
tmp = 0;
acpi_evaluate_integer(handle, "_GLK", NULL, &tmp);
@@ -1777,11 +1786,39 @@ static int ec_correct_ecdt(const struct dmi_system_id *id)
return 0;
}
+/*
+ * Some DSDTs contain wrong GPE setting.
+ * Asus FX502VD/VE, X550VXK, X580VD
+ * https://bugzilla.kernel.org/show_bug.cgi?id=195651
+ */
+static int ec_honor_ecdt_gpe(const struct dmi_system_id *id)
+{
+ pr_debug("Detected system needing ignore DSDT GPE setting.\n");
+ EC_FLAGS_IGNORE_DSDT_GPE = 1;
+ return 0;
+}
+
static struct dmi_system_id ec_dmi_table[] __initdata = {
{
ec_correct_ecdt, "MSI MS-171F", {
DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star"),
DMI_MATCH(DMI_PRODUCT_NAME, "MS-171F"),}, NULL},
+ {
+ ec_honor_ecdt_gpe, "ASUS FX502VD", {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "FX502VD"),}, NULL},
+ {
+ ec_honor_ecdt_gpe, "ASUS FX502VE", {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "FX502VE"),}, NULL},
+ {
+ ec_honor_ecdt_gpe, "ASUS X550VXK", {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X550VXK"),}, NULL},
+ {
+ ec_honor_ecdt_gpe, "ASUS X580VD", {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X580VD"),}, NULL},
{},
};
--
2.7.4
From: Carlo Caione <[email protected]>
ASUS GL720VMK is also affected by the EC GPE preference issue.
Signed-off-by: Carlo Caione <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/ec.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 2189048..ab04f0c 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1788,7 +1788,7 @@ static int ec_correct_ecdt(const struct dmi_system_id *id)
/*
* Some DSDTs contain wrong GPE setting.
- * Asus FX502VD/VE, X550VXK, X580VD
+ * Asus FX502VD/VE, GL702VMK, X550VXK, X580VD
* https://bugzilla.kernel.org/show_bug.cgi?id=195651
*/
static int ec_honor_ecdt_gpe(const struct dmi_system_id *id)
@@ -1812,6 +1812,10 @@ static struct dmi_system_id ec_dmi_table[] __initdata = {
DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
DMI_MATCH(DMI_PRODUCT_NAME, "FX502VE"),}, NULL},
{
+ ec_honor_ecdt_gpe, "ASUS GL702VMK", {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "GL702VMK"),}, NULL},
+ {
ec_honor_ecdt_gpe, "ASUS X550VXK", {
DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
DMI_MATCH(DMI_PRODUCT_NAME, "X550VXK"),}, NULL},
--
2.7.4
We prepared _INI/_STA methods for \_SB, \_SB.PCI0, \_SB.LID0 and \_SB.EC,
_HID(PNP0C09)/_CRS/_GPE for \_SB.EC to poke Windows behavior with qemu, we
got the following execution sequence:
\_SB._INI
\_SB.PCI0._STA
\_SB.LID0._STA
\_SB.EC._STA
\_SB.PCI0._INI
\_SB.LID0._INI
\_SB.EC._INI
There is no extra DSDT EC device enumeration process occurring before the
main ACPI device enumeration process. That means acpi_ec_dsdt_probe() is
not a Windows compliant approach, it's just a linux workaround.
Tracking back, we can see a long time ago, Linux EC driver won't probe DSDT
EC during boot. It was added by the following commit (see link #1 for bug
report):
Commit: c5279dee26c0e8d7c4200993bfc4b540d2469598
Subject: ACPI: EC: Add some basic check for ECDT data
This commit simply drivers Linux EC driver to a wrong direction.
Why we shouldn't enumerate DSDT EC before the main ACPI device enumeration?
The only way to know if the DSDT EC is valid is to evaluate its _STA
control method, but it's not proper to evaluate this control method that
early and out of the ACPI enumeration process because _STA may contain
other devices' initialization code and such devices may not have been
initialized before OSPM starts to enumerate them via the main ACPI device
enumeration.
But after we reverted back to the expected behavior, someone reported a
regression (see link #2 for reference). On that platform, there is no ECDT,
but the platform control methods access EC operation region earlier than
Linux expects, causing some ACPI method execution errors. According to the
regression rule, we just revert back to old behavior to still probe DSDT EC
as boot EC.
Now we've been reported 3rd functional breakage (link #3). In order to
handle both issues (link #2 and link #3), we could just skip boot stage
DSDT probe when ECDT exists so that a later quirk can always use correct
ECDT GPE setting.
Link: http://bugzilla.kernel.org/show_bug.cgi?id=11880 [#1]
Link: http://bugzilla.kernel.org/show_bug.cgi?id=119261 [#2]
Link: http://bugzilla.kernel.org/show_bug.cgi?id=195651 [#3]
Tested-by: Daniel Drake <[email protected]>
Signed-off-by: Lv Zheng <[email protected]>
---
drivers/acpi/ec.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index c1f480b..44b973e 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1667,12 +1667,34 @@ static const struct acpi_device_id ec_device_ids[] = {
{"", 0},
};
+/*
+ * This function is not Windows compliant as Windows never enumerates the
+ * namespace EC before the main ACPI device enumeration process. This
+ * function is kept for historical reason and will be deprecated in the
+ * future.
+ *
+ * It's added due to a non-root-caused bug fix:
+ * http://bugzilla.kernel.org/show_bug.cgi?id=11880
+ * But removing it now unfortunately triggers user noticable regression:
+ * http://bugzilla.kernel.org/show_bug.cgi?id=119261
+ */
int __init acpi_ec_dsdt_probe(void)
{
acpi_status status;
struct acpi_ec *ec;
int ret;
+ /*
+ * If a platform has ECDT, there is no need to proceed as the
+ * following probe is not a part of the ACPI device enumeration,
+ * executing _STA is not safe, and thus this probe may risk of
+ * picking up an invalid EC device.
+ * So we should catch any possible chance to avoid applying this
+ * quirk.
+ */
+ if (boot_ec)
+ return -ENODEV;
+
ec = acpi_ec_alloc();
if (!ec)
return -ENOMEM;
--
2.7.4