<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux.git/drivers/acpi/ec.c, branch v6.0</title>
<subtitle>Linux kernel source tree</subtitle>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/'/>
<entry>
<title>ACPI: EC: Drop unused ident initializers from dmi_system_id tables</title>
<updated>2022-06-29T17:40:46+00:00</updated>
<author>
<name>Hans de Goede</name>
<email>hdegoede@redhat.com</email>
</author>
<published>2022-06-20T09:25:46+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=3e6573c5d275cc7c109f824c6710354fa9a878b1'/>
<id>3e6573c5d275cc7c109f824c6710354fa9a878b1</id>
<content type='text'>
Drop the unused const string ident initializers from
the dmi_system_id tables to make the object size a bit smaller.

While at it also use proper named struct-member initializers for
the ec_dmi_table[].

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Drop the unused const string ident initializers from
the dmi_system_id tables to make the object size a bit smaller.

While at it also use proper named struct-member initializers for
the ec_dmi_table[].

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Re-use boot_ec when possible even when EC_FLAGS_TRUST_DSDT_GPE is set</title>
<updated>2022-06-29T17:40:46+00:00</updated>
<author>
<name>Hans de Goede</name>
<email>hdegoede@redhat.com</email>
</author>
<published>2022-06-20T09:25:45+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=81df5f91974347b9d95de06953b839101fec4a5e'/>
<id>81df5f91974347b9d95de06953b839101fec4a5e</id>
<content type='text'>
EC_FLAGS_TRUST_DSDT_GPE only does anything when the:

	if (boot_ec &amp;&amp; ec-&gt;command_addr == boot_ec-&gt;command_addr &amp;&amp;
	    ec-&gt;data_addr == boot_ec-&gt;data_addr)

conditions are all true. Normally acpi_ec_add() would re-use the boot_ec
struct acpi_ec in this case. But when the EC_FLAGS_TRUST_DSDT_GPE flag was
set the code would continue with a newly allocated (second) struct acpi_ec.

There is no reason to use a second struct acpi_ec if all the above checks
match. Instead just change boot_ec-&gt;gpe to ec-&gt;gpe, when the flag is set,
similar to how this is already one done for boot_ec-&gt;handle.

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
EC_FLAGS_TRUST_DSDT_GPE only does anything when the:

	if (boot_ec &amp;&amp; ec-&gt;command_addr == boot_ec-&gt;command_addr &amp;&amp;
	    ec-&gt;data_addr == boot_ec-&gt;data_addr)

conditions are all true. Normally acpi_ec_add() would re-use the boot_ec
struct acpi_ec in this case. But when the EC_FLAGS_TRUST_DSDT_GPE flag was
set the code would continue with a newly allocated (second) struct acpi_ec.

There is no reason to use a second struct acpi_ec if all the above checks
match. Instead just change boot_ec-&gt;gpe to ec-&gt;gpe, when the flag is set,
similar to how this is already one done for boot_ec-&gt;handle.

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Drop the EC_FLAGS_IGNORE_DSDT_GPE quirk</title>
<updated>2022-06-29T17:40:46+00:00</updated>
<author>
<name>Hans de Goede</name>
<email>hdegoede@redhat.com</email>
</author>
<published>2022-06-20T09:25:44+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=f7090e0ef360d674f08a22fab90e4e209fb1f658'/>
<id>f7090e0ef360d674f08a22fab90e4e209fb1f658</id>
<content type='text'>
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 &amp;&amp; boot_ec_is_ecdt &amp;&amp; EC_FLAGS_IGNORE_DSDT_GPE) {
		ec-&gt;gpe = boot_ec-&gt;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 &amp;&amp; ec-&gt;command_addr == boot_ec-&gt;command_addr &amp;&amp;
	    ec-&gt;data_addr == boot_ec-&gt;data_addr &amp;&amp;
	    !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-&gt;gpe to ec-&gt;gpe.
		 */
		boot_ec-&gt;handle = ec-&gt;handle;
		acpi_handle_debug(ec-&gt;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-&gt;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-&gt;gpe == boot_ec-&gt;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 &lt;hdegoede@redhat.com&gt;
Reviewed-by: Daniel Drake &lt;drake@endlessos.org&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &amp;&amp; boot_ec_is_ecdt &amp;&amp; EC_FLAGS_IGNORE_DSDT_GPE) {
		ec-&gt;gpe = boot_ec-&gt;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 &amp;&amp; ec-&gt;command_addr == boot_ec-&gt;command_addr &amp;&amp;
	    ec-&gt;data_addr == boot_ec-&gt;data_addr &amp;&amp;
	    !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-&gt;gpe to ec-&gt;gpe.
		 */
		boot_ec-&gt;handle = ec-&gt;handle;
		acpi_handle_debug(ec-&gt;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-&gt;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-&gt;gpe == boot_ec-&gt;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 &lt;hdegoede@redhat.com&gt;
Reviewed-by: Daniel Drake &lt;drake@endlessos.org&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Remove duplicate ThinkPad X1 Carbon 6th entry from DMI quirks</title>
<updated>2022-06-29T17:40:45+00:00</updated>
<author>
<name>Hans de Goede</name>
<email>hdegoede@redhat.com</email>
</author>
<published>2022-06-20T09:25:43+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=0dd6db359e5f206cbf1dd1fd40dd211588cd2725'/>
<id>0dd6db359e5f206cbf1dd1fd40dd211588cd2725</id>
<content type='text'>
Somehow the "ThinkPad X1 Carbon 6th" entry ended up twice in the
struct dmi_system_id acpi_ec_no_wakeup[] array. Remove one of
the entries.

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Somehow the "ThinkPad X1 Carbon 6th" entry ended up twice in the
struct dmi_system_id acpi_ec_no_wakeup[] array. Remove one of
the entries.

Signed-off-by: Hans de Goede &lt;hdegoede@redhat.com&gt;
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Rearrange code in acpi_ec_submit_event()</title>
<updated>2022-02-14T19:14:40+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2022-02-04T17:43:14+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=13a62d0e13308d62426c38223a3b6f78878f2173'/>
<id>13a62d0e13308d62426c38223a3b6f78878f2173</id>
<content type='text'>
Rearange acpi_ec_event_handler() so as to avoid releasing ec-&gt;lock
and acquiring it again right away in the case when ec_event_clearing
is not ACPI_EC_EVT_TIMING_EVENT.

This also reduces the number of checks done by acpi_ec_event_handler()
in that case.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Rearange acpi_ec_event_handler() so as to avoid releasing ec-&gt;lock
and acquiring it again right away in the case when ec_event_clearing
is not ACPI_EC_EVT_TIMING_EVENT.

This also reduces the number of checks done by acpi_ec_event_handler()
in that case.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Reduce indentation level in acpi_ec_submit_event()</title>
<updated>2022-02-14T19:14:40+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2022-02-04T17:40:55+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=54b86141d71db2a16743cfa4a3417180d7feb24a'/>
<id>54b86141d71db2a16743cfa4a3417180d7feb24a</id>
<content type='text'>
The indentation level in acpi_ec_submit_event() can be reduced, so
do that and while at it fix a typo in the comment affected by that
change.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The indentation level in acpi_ec_submit_event() can be reduced, so
do that and while at it fix a typo in the comment affected by that
change.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Do not return result from advance_transaction()</title>
<updated>2022-02-14T19:14:40+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2022-02-04T17:40:20+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=9aa60f3cbafb0facc15a6114df7616a1bf23a88d'/>
<id>9aa60f3cbafb0facc15a6114df7616a1bf23a88d</id>
<content type='text'>
Notice that the if the event state is EC_EVENT_READY, the event
handling work cannot be pending, so it is not necessary to check
the return value of queue_work() in acpi_ec_submit_event().
Moreover, whether or not there is any EC work pending at the
moment can always be checked by looking at the events_in_progress
and queries_in_progress counters, so acpi_ec_submit_event() and
consequently advance_transaction() need not return results.

Accordingly, make acpi_ec_dispatch_gpe() always use the counters
mentioned above (for first_ec) to check if there is any pending EC
work to flush and turn both acpi_ec_submit_event() and
advance_transaction() into void functions (again, because they were
void functions in the past).

While at it, add a clarifying comment about the acpi_ec_mask_events()
call in advance_transaction().

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Notice that the if the event state is EC_EVENT_READY, the event
handling work cannot be pending, so it is not necessary to check
the return value of queue_work() in acpi_ec_submit_event().
Moreover, whether or not there is any EC work pending at the
moment can always be checked by looking at the events_in_progress
and queries_in_progress counters, so acpi_ec_submit_event() and
consequently advance_transaction() need not return results.

Accordingly, make acpi_ec_dispatch_gpe() always use the counters
mentioned above (for first_ec) to check if there is any pending EC
work to flush and turn both acpi_ec_submit_event() and
advance_transaction() into void functions (again, because they were
void functions in the past).

While at it, add a clarifying comment about the acpi_ec_mask_events()
call in advance_transaction().

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC / PM: Print additional debug message in acpi_ec_dispatch_gpe()</title>
<updated>2022-02-14T18:41:11+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2022-02-01T19:18:56+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=977dc3082285206e7b1fcbc4496671194cfb7980'/>
<id>977dc3082285206e7b1fcbc4496671194cfb7980</id>
<content type='text'>
Make acpi_ec_dispatch_gpe() print an additional debug message after
seeing the EC GPE status bit set to help diagnose wakeup-related
issues.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Make acpi_ec_dispatch_gpe() print an additional debug message after
seeing the EC GPE status bit set to help diagnose wakeup-related
issues.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: PM: s2idle: Cancel wakeup before dispatching EC GPE</title>
<updated>2022-02-07T20:02:31+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2022-02-04T17:31:02+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=dc0075ba7f387fe4c48a8c674b11ab6f374a6acc'/>
<id>dc0075ba7f387fe4c48a8c674b11ab6f374a6acc</id>
<content type='text'>
Commit 4a9af6cac050 ("ACPI: EC: Rework flushing of EC work while
suspended to idle") made acpi_ec_dispatch_gpe() check
pm_wakeup_pending(), but that is before canceling the SCI wakeup,
so pm_wakeup_pending() is always true.  This causes the loop in
acpi_ec_dispatch_gpe() to always terminate after one iteration which
may not be correct.

Address this issue by canceling the SCI wakeup earlier, from
acpi_ec_dispatch_gpe() itself.

Fixes: 4a9af6cac050 ("ACPI: EC: Rework flushing of EC work while suspended to idle")
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Commit 4a9af6cac050 ("ACPI: EC: Rework flushing of EC work while
suspended to idle") made acpi_ec_dispatch_gpe() check
pm_wakeup_pending(), but that is before canceling the SCI wakeup,
so pm_wakeup_pending() is always true.  This causes the loop in
acpi_ec_dispatch_gpe() to always terminate after one iteration which
may not be correct.

Address this issue by canceling the SCI wakeup earlier, from
acpi_ec_dispatch_gpe() itself.

Fixes: 4a9af6cac050 ("ACPI: EC: Rework flushing of EC work while suspended to idle")
Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ACPI: EC: Relocate acpi_ec_create_query() and drop acpi_ec_delete_query()</title>
<updated>2021-12-01T19:17:32+00:00</updated>
<author>
<name>Rafael J. Wysocki</name>
<email>rafael.j.wysocki@intel.com</email>
</author>
<published>2021-11-23T18:46:09+00:00</published>
<link rel='alternate' type='text/html' href='https://git.tavy.me/linux.git/commit/?id=befd9b5b0c621af33a363596c65a8fc0176e2795'/>
<id>befd9b5b0c621af33a363596c65a8fc0176e2795</id>
<content type='text'>
Move acpi_ec_create_query() after acpi_ec_event_processor(), drop the
no longer needed forward declaration of the latter, and eliminate
acpi_ec_delete_query() which isn't really necessary.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Move acpi_ec_create_query() after acpi_ec_event_processor(), drop the
no longer needed forward declaration of the latter, and eliminate
acpi_ec_delete_query() which isn't really necessary.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki &lt;rafael.j.wysocki@intel.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
