ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk
authorHans de Goede <hdegoede@redhat.com>
Mon, 20 Jun 2022 09:25:44 +0000 (11:25 +0200)
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>
Wed, 29 Jun 2022 17:40:46 +0000 (19:40 +0200)
commitf7090e0ef360d674f08a22fab90e4e209fb1f658
tree4602f888cd547ce7b50ca1758180600e590a1fb9
parent0dd6db359e5f206cbf1dd1fd40dd211588cd2725
ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk

It seems that these quirks are no longer necessary since
commit 69b957c26b32 ("ACPI: EC: Fix possible issues related to EC
initialization order"), which has fixed this in a generic manner.

There are 3 commits adding DMI entries with this quirk (adding multiple
DMI entries per commit). 2/3 commits are from before the generic fix.

Which leaves commit 6306f0431914 ("ACPI: EC: Make more Asus laptops
use ECDT _GPE"), which was committed way after the generic fix.
But this was just due to slow upstreaming of it. This commit stems
from Endless from 15 Aug 2017 (committed upstream 20 May 2021):
https://github.com/endlessm/linux/pull/288

The current code should work fine without this:

 1. The EC_FLAGS_IGNORE_DSDT_GPE flag is only checked in ec_parse_device(),
    like this:

if (boot_ec && boot_ec_is_ecdt && EC_FLAGS_IGNORE_DSDT_GPE) {
ec->gpe = boot_ec->gpe;
} else {
/* parse GPE */
}

 2. ec_parse_device() is only called from acpi_ec_add() and
    acpi_ec_dsdt_probe()

 3. acpi_ec_dsdt_probe() starts with:

if (boot_ec)
return;

    so it only calls ec_parse_device() when boot_ec == NULL, meaning that
    the quirk never triggers for this call. So only the call in
    acpi_ec_add() matters.

 4. acpi_ec_add() does the following after the ec_parse_device() call:

if (boot_ec && ec->command_addr == boot_ec->command_addr &&
    ec->data_addr == boot_ec->data_addr &&
    !EC_FLAGS_TRUST_DSDT_GPE) {
/*
 * Trust PNP0C09 namespace location rather than
 * ECDT ID. But trust ECDT GPE rather than _GPE
 * because of ASUS quirks, so do not change
 * boot_ec->gpe to ec->gpe.
 */
boot_ec->handle = ec->handle;
acpi_handle_debug(ec->handle, "duplicated.\n");
acpi_ec_free(ec);
ec = boot_ec;
}

The quirk only matters if boot_ec != NULL and EC_FLAGS_TRUST_DSDT_GPE
is never set at the same time as EC_FLAGS_IGNORE_DSDT_GPE.

That means that if the addresses match we always enter this if block and
then only the ec->handle part of the data stored in ec by ec_parse_device()
is used and the rest is thrown away, after which ec is made to point
to boot_ec, at which point ec->gpe == boot_ec->gpe, so the same result
as with the quirk set, independent of the value of the quirk.

Also note the comment in this block which indicates that the gpe result
from ec_parse_device() is deliberately not taken to deal with buggy
Asus laptops and all DMI quirks setting EC_FLAGS_IGNORE_DSDT_GPE are for
Asus laptops.

Based on the above I believe that unless on some quirked laptops
the ECDT and DSDT EC addresses do not match we can drop the quirk.

I've checked dmesg output to ensure the ECDT and DSDT EC addresses match
for quirked models using https://linux-hardware.org hw-probe reports.

I've been able to confirm that the addresses match for the following
models this way: GL702VMK, X505BA, X505BP, X550VXK, X580VD.
Whereas for the following models I could find any dmesg output:
FX502VD, FX502VE, X542BA, X542BP.

Note the models without dmesg all were submitted in patches with a batch
of models and other models from the same batch checkout ok.

This, combined with that all the code adding the quirks was written before
the generic fix makes me believe that it is safe to remove this quirk now.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Daniel Drake <drake@endlessos.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
drivers/acpi/ec.c