All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Drake <drake@endlessm.com>
To: rjw@rjwysocki.net, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, lv.zheng@intel.com,
	chiu@endlessm.com, linux@endlessm.com
Subject: [PATCH] ACPI / EC: handle ECDT EC and DSDT EC simultaneously
Date: Thu, 20 Apr 2017 14:12:19 -0600	[thread overview]
Message-ID: <20170420201219.21568-1-drake@endlessm.com> (raw)

The Asus laptops X550VXK/FX502VD/FX502VE have a BIOS bug where the
ECDT (correctly) states that EC events trigger GPE 0x23, but the
DSDT _GPE method (incorrectly) says GPE 0x33.

A cursory glance of the code suggests that this should work regardless
(because it looks like Linux would treat the ECDT and DSDT as two separate
ECs supported simultaneously), but in practice it is not working since the
ECDT is ultimately ignored in favour of the DSDT EC. The sequence of
events is:

1. acpi_ec_ecdt_probe() is called in early boot, and calls
   acpi_config_boot_ec(is_ecdt=true) for the ECDT EC.

acpi_config_boot_ec() sets boot_ec to this new EC, and
boot_ec_is_ecdt = true.

2. Later, acpi_ec_dsdt_probe() is called and creates a new ec to represent
   the DSDT EC. Then:
    /*
     * When the DSDT EC is available, always re-configure boot EC to
     * have _REG evaluated. _REG can only be evaluated after the
     * namespace initialization.
     * At this point, the GPE is not fully initialized, so do not to
     * handle the events.
     */
    ret = acpi_config_boot_ec(ec, ec->handle, false, false);

So the DSDT EC is passed to acpi_config_boot_ec() which does:
    /* Unset old boot EC */
    if (boot_ec != ec)
        acpi_ec_free(boot_ec);

acpi_ec_free() sets boot_ec to NULL.
Further down in acpi_config_boot_ec() we reach:

    /* Set new boot EC */
    if (!boot_ec) {
        boot_ec = ec;
        boot_ec_is_ecdt = is_ecdt;
    }

So now boot_ec points to the DSDT EC and boot_ec_is_ecdt is false.

3. Later, ecpi_ec_ecdt_start() is called and this looks like it would
   enable GPEs on our ECDT, but it bails out because of:

    if (!boot_ec_is_ecdt)
        return -ENODEV;


The comment I pasted above from acpi_ec_dsdt_probe() says that it is going
to reconfigure the boot EC, but it instead calls acpi_config_boot_ec() on
the newly probed DSDT EC. It seems like the code is not following the
comment here.

Adjusting that code to work with boot_ec adjusts the sequence of events so
that both boot EC and DSDT are treated independently and as a result, we
get EC interrupts firing correctly.

This fixes media keys on the mentioned laptop models.
Thanks to Chris Chiu for diagnosis.

Signed-off-by: Daniel Drake <drake@endlessm.com>
---
 drivers/acpi/ec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index c24235d8fb52..78395395e3d9 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1691,7 +1691,7 @@ int __init acpi_ec_dsdt_probe(void)
 	 * At this point, the GPE is not fully initialized, so do not to
 	 * handle the events.
 	 */
-	ret = acpi_config_boot_ec(ec, ec->handle, false, false);
+	ret = acpi_config_boot_ec(boot_ec, boot_ec->handle, false, false);
 error:
 	if (ret)
 		acpi_ec_free(ec);
-- 
2.11.0


             reply	other threads:[~2017-04-20 20:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-20 20:12 Daniel Drake [this message]
2017-04-20 20:59 ` [PATCH] ACPI / EC: handle ECDT EC and DSDT EC simultaneously Rafael J. Wysocki
2017-04-24  4:43 ` Zheng, Lv
2017-04-26 13:11   ` Daniel Drake
2017-04-27  3:18     ` Zheng, Lv
2017-04-28  0:33       ` Rafael J. Wysocki
2017-04-28  0:44         ` Daniel Drake
2017-04-28  6:33           ` Zheng, Lv
2017-04-28 12:52             ` Daniel Drake
2017-05-03 20:06               ` Daniel Drake
2017-05-04  5:05                 ` Zheng, Lv
2017-05-04  4:52               ` Zheng, Lv

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170420201219.21568-1-drake@endlessm.com \
    --to=drake@endlessm.com \
    --cc=chiu@endlessm.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux@endlessm.com \
    --cc=lv.zheng@intel.com \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.