Re: [PATCH 2/4] ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk

From: Hans de Goede <hdegoede@redhat.com>
To: Daniel Drake <drake@endlessos.org>
Cc: "Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>,
	Robert Moore <robert.moore@intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Zhang Rui <rui.zhang@intel.com>,
	Andy Shevchenko <andy@kernel.org>,
	Kai-Heng Feng <kai.heng.feng@canonical.com>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	devel@acpica.org, Lv Zheng <lv.zheng@intel.com>,
	Chris Chiu <chris.chiu@canonical.com>,
	Jian-Hong Pan <jhp@endlessos.org>,
	Carlo Caione <carlo@caione.org>
Subject: Re: [PATCH 2/4] ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk
Date: Mon, 20 Jun 2022 16:33:17 +0200	[thread overview]
Message-ID: <b2ad4b62-89bc-48c5-ebc3-c9d8f86aa902@redhat.com> (raw)
In-Reply-To: <CAD8Lp45ismm5yG2ajGH_h=BKLh6hs8yC7gy3jq1Kn1pst0AFKg@mail.gmail.com>

Hi,

On 6/20/22 12:47, Daniel Drake wrote:
> Hi Hans,
> 
> Thanks for looking at this.
> 
> On Mon, Jun 20, 2022 at 5:26 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> 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:
> 
> Your explanation of the code flow seems clear and logical, but I have
> not checked the details. This is a bit of a tricky issue as you have
> probably seen from history, we went in a couple of wrong directions
> before we spotted the real cause.
> 
> The one thing I don't see clearly in your explanation (which I may
> have read too quickly) is how the generic fix 69b957c26b32 is
> responsible for making this a "no-op" code flow now.

It is a no-op now because after that commit the acpi_ec struct
which gets allocated when parsing the ECDT now gets re-used
when parsing the DSDT if the EC's cmd + data addresses match.

When we enter the if for re-using that boot_ec acpi_ec struct then
only boot_ec->handle is re-used; and we keep boot_ec->gpe to the
value set when parsing the ECDT.

The quirk does:

ec->gpe is boot_ec->gpe, but since we throw ec away now
(after taking ec->handle) and reuse boot_ec->gpe we will end
up using boot_ec->gpe just as the quirk caused us to do before
we started re-using the boot_ec acpi_ec struct.

Regards,

Hans