[PATCH 0/3] ACPI / EC: Add quirk modes for boot EC support

[PATCH 0/3] ACPI / EC: Add quirk modes for boot EC support

[Lv Zheng]

It's reported that Asus laptop X550VXK/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 prepares required code for adding quirks for such platforms,
however it doesn't finally add the quirks due to lack of platform
information.

Link: https://www.spinics.net/lists/linux-acpi/msg73763.html
      https://bugzilla.kernel.org/show_bug.cgi?id=195651

Lv Zheng (3):
  ACPI / EC: Enhance boot EC sanity check
  ACPI / EC: Add support to skip boot stage DSDT probe
  ACPI / EC: Add EC setting preference support

 drivers/acpi/ec.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 1/3] ACPI / EC: Enhance boot EC sanity check

[Lv Zheng]

It's reported that some buggy BIOS tables can contain 2 DSDT ECs and one of
them is invalid. As we shouldn't evaluate _STA from acpi_ec_dsdt_probe()
due to the unknown Windows enumeration order, 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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index c24235d..a920db6 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1362,6 +1362,14 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
 				     ec_parse_io_ports, ec);
 	if (ACPI_FAILURE(status))
 		return status;
+	/*
+	 * It's better to evaluate _STA to determine if the device is
+	 * valid. But that could potentially trigger issues related to
+	 * the unknown orders of _INI/_STA evaluations.
+	 * However we can abort due to invalid _CRS information here.
+	 */
+	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

[PATCH 2/3] ACPI / EC: Add support to skip boot stage DSDT probe

[Lv Zheng]

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 is wrong as 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.

But after we reverted back to the expected behavior, someone reported
regressions (see link #2 for reference). That forced us to restore the
wrong behavior back.

Now we've been reported 3rd functional breakage, the only way to fix it is
to evaluate _STA (see link #3 for newer requirement). And the safest way
of evaluating _STA is to do that during Linux ACPI device enumeration. As
the link #2 reported issue is not fatal, we could still skip boot stage
DSDT probe. But in order not to trigger regressions, this patch just
introduces a boot parameter instead of unconditionally changing the
upstreamed behavior.

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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index a920db6..a7e74ae 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -151,6 +151,10 @@ static bool ec_freeze_events __read_mostly = true;
 module_param(ec_freeze_events, bool, 0644);
 MODULE_PARM_DESC(ec_freeze_events, "Disabling event handling during suspend/resume");
 
+static bool ec_no_dsdt_boot_ec __read_mostly = false;
+module_param(ec_no_dsdt_boot_ec, bool, 0644);
+MODULE_PARM_DESC(ec_no_dsdt_boot_ec, "Do not probe DSDT EC as boot EC");
+
 struct acpi_ec_query_handler {
 	struct list_head node;
 	acpi_ec_query_func func;
@@ -1679,6 +1683,9 @@ int __init acpi_ec_dsdt_probe(void)
 	struct acpi_ec *ec;
 	int ret;
 
+	if (ec_no_dsdt_boot_ec)
+		return -ENODEV;
+
 	ec = acpi_ec_alloc();
 	if (!ec)
 		return -ENOMEM;
-- 
2.7.4

[PATCH 3/3] ACPI / EC: Add EC setting preference support

[Lv Zheng]

Due to unknown reasons, Windows may prefer ECDT EC GPE rather than DSDT
EC GPE setting as some ASUS BIOSen report wrong GPE in DSDT EC but correct
one in ECDT EC. Without knowing the root cause, this patch simply allows
users to specify preferred EC settings (ID and GPE) via command line and
provides default value to always prefere DSDT settings. Users can work
the ASUS BIOS problem around with "ec_use_boot_ec_gpe=Y".

Link: https://bugzilla.kernel.org/show_bug.cgi?id=195651
Tested-by: Daniel Drake <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index a7e74ae..b4ae29f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -155,6 +155,14 @@ static bool ec_no_dsdt_boot_ec __read_mostly = false;
 module_param(ec_no_dsdt_boot_ec, bool, 0644);
 MODULE_PARM_DESC(ec_no_dsdt_boot_ec, "Do not probe DSDT EC as boot EC");
 
+static bool ec_use_boot_ec_id __read_mostly = false;
+module_param(ec_use_boot_ec_id, bool, 0644);
+MODULE_PARM_DESC(ec_use_boot_ec_id, "Override EC_ID using boot EC setting");
+
+static bool ec_use_boot_ec_gpe __read_mostly = false;
+module_param(ec_use_boot_ec_gpe, bool, 0644);
+MODULE_PARM_DESC(ec_use_boot_ec_gpe, "Override GPE_BIT using boot EC setting");
+
 struct acpi_ec_query_handler {
 	struct list_head node;
 	acpi_ec_query_func func;
@@ -1574,14 +1582,28 @@ static bool acpi_is_boot_ec(struct acpi_ec *ec)
 {
 	if (!boot_ec)
 		return false;
-	if (ec->handle == boot_ec->handle &&
-	    ec->gpe == boot_ec->gpe &&
-	    ec->command_addr == boot_ec->command_addr &&
+	if (ec->command_addr == boot_ec->command_addr &&
 	    ec->data_addr == boot_ec->data_addr)
 		return true;
 	return false;
 }
 
+static void acpi_ec_config_preference(struct acpi_ec *ec)
+{
+	if (ec->handle != boot_ec->handle) {
+		if (ec_use_boot_ec_id)
+			ec->handle = boot_ec->handle;
+		else
+			boot_ec->handle = ec->handle;
+	}
+	if (ec->gpe != boot_ec->gpe) {
+		if (ec_use_boot_ec_gpe)
+			ec->gpe = boot_ec->gpe;
+		else
+			boot_ec->gpe = ec->gpe;
+	}
+}
+
 static int acpi_ec_add(struct acpi_device *device)
 {
 	struct acpi_ec *ec = NULL;
@@ -1600,8 +1622,8 @@ static int acpi_ec_add(struct acpi_device *device)
 	}
 
 	if (acpi_is_boot_ec(ec)) {
-		boot_ec_is_ecdt = false;
-		acpi_handle_debug(ec->handle, "duplicated.\n");
+		acpi_handle_debug(ec->handle, "duplicated late.\n");
+		acpi_ec_config_preference(ec);
 		acpi_ec_free(ec);
 		ec = boot_ec;
 		ret = acpi_config_boot_ec(ec, ec->handle, true, false);
@@ -1699,6 +1721,10 @@ int __init acpi_ec_dsdt_probe(void)
 		ret = -ENODEV;
 		goto error;
 	}
+	if (acpi_is_boot_ec(ec)) {
+		acpi_handle_debug(ec->handle, "duplicated early.\n");
+		acpi_ec_config_preference(ec);
+	}
 	/*
 	 * When the DSDT EC is available, always re-configure boot EC to
 	 * have _REG evaluated. _REG can only be evaluated after the
-- 
2.7.4

[PATCH v2 0/3] ACPI / EC: Add quirk modes for boot EC support

[Lv Zheng]

It's reported that Asus laptop X580VD/X550VXK/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.

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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] ACPI / EC: Enhance boot EC sanity check

[Lv Zheng]

It's reported that some buggy BIOS tables can contain 2 DSDT ECs and one of
them is invalid. As we shouldn't evaluate _STA from acpi_ec_dsdt_probe()
due to the unknown Windows enumeration order, 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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index c24235d..a920db6 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1362,6 +1362,14 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
 				     ec_parse_io_ports, ec);
 	if (ACPI_FAILURE(status))
 		return status;
+	/*
+	 * It's better to evaluate _STA to determine if the device is
+	 * valid. But that could potentially trigger issues related to
+	 * the unknown orders of _INI/_STA evaluations.
+	 * However we can abort due to invalid _CRS information here.
+	 */
+	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

[PATCH v2 2/3] ACPI / EC: Add support to skip boot stage DSDT probe

[Lv Zheng]

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 is wrong as 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.

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 methds access EC operation region earlier than
Linux expects. Without knowing the exact device enumeration order on
Windows, we in fact cannot conclude anything but can just follow the
regression rule to revert to old wrong behavior to probe DSDT EC at the old
position.

Now we've been reported 3rd functional breakage (link #3). The safest way
of solving it includes evaluating _STA. Due to the reason above, it is not
such safe to evaluate _STA in acpi_ec_dsdt_probe(). In order to handle both
issues (link #2 and link #3), we could just skip boot stage DSDT probe when
ECDT exists.

Note this change doesn't solve the reported problem, it can only be
resolved by a GPE setting quirk, and without this commit but with only the
GPE setting quirk, the reported problem can be solved. However, this commit
can improve our code quality by making unexpected behavior less effective.

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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index a920db6..e232a1c 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1679,6 +1679,14 @@ int __init acpi_ec_dsdt_probe(void)
 	struct acpi_ec *ec;
 	int ret;
 
+	/*
+	 * If a platform has ECDT, there is no need to proceed as the
+	 * following unsafe probe is not a part of ACPI device enumeration,
+	 * and hence _STA is not executed.
+	 */
+	if (boot_ec)
+		return -ENODEV;
+
 	ec = acpi_ec_alloc();
 	if (!ec)
 		return -ENOMEM;
-- 
2.7.4

[PATCH v2 3/3] ACPI / EC: Fix media keys not working problem on some Asus laptops

[Lv Zheng]

From: Chris Chiu <chiu@endlessm.com>

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 use a DMI quirk
to prevent calling _GPE when doing ec_parse_device() and keep the ECDT
GPE number setting for the EC device.

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 <drake@endlessm.com>
Signed-off-by: Chris Chiu <chiu@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e232a1c..b6d28ef 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
@@ -1371,12 +1372,21 @@ 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 && EC_FLAGS_IGNORE_DSDT_GPE) {
+		/*
+		 * First boot_ec is always ECDT EC.
+		 * Always inherit the GPE number setting from the first
+		 * boot_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);
@@ -1769,11 +1779,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
+ * Link: 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

[PATCH v3 0/4] ACPI / EC: Add quirk modes for boot EC support

[Lv Zheng]

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

[PATCH v3 1/4] ACPI / EC: Enhance boot EC sanity check

[Lv Zheng]

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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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

[PATCH v3 2/4] ACPI / EC: Add support to skip boot stage DSDT probe

[Lv Zheng]

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 <drake@endlessm.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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

[PATCH v3 3/4] ACPI / EC: Fix media keys not working problem on some Asus laptops

[Lv Zheng]

From: Chris Chiu <chiu@endlessm.com>

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 <drake@endlessm.com>
Signed-off-by: Chris Chiu <chiu@endlessm.com>
Signed-off-by: Carlo Caione <carlo@caione.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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

[PATCH v3 4/4] ACPI / EC: Add quirk for GL720VMK

[Lv Zheng]

From: Carlo Caione <carlo@caione.org>

ASUS GL720VMK is also affected by the EC GPE preference issue.

Signed-off-by: Carlo Caione <carlo@caione.org>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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