Merge additional ACPI EC driver fixes for 6.2-rc1:
- Fix EC address space handler unregistration (Hans de Goede).
- Defer the evaluation of _REG for ECDT described ECs till the matching
EC device in the DSDT gets parsed and acpi_ec_add() gets called for
it (Hans de Goede).
* acpi-ec:
ACPI: EC: Fix ECDT probe ordering issues
ACPI: EC: Fix EC address space handler unregistration
Make ACPI power management changes, ACPI processor driver updates, ACPI
EC driver quirk and ACPI backlight driver updates for 6.2-rc1:
- Print full name paths of ACPI power resources objects during
enumeration (Kane Chen).
- Eliminate a compiler warning regarding a missing function prototype
in the ACPI power management code (Sudeep Holla).
- Fix and clean up the ACPI processor driver (Rafael Wysocki, Li Zhong,
Colin Ian King, Sudeep Holla).
- Add quirk for the HP Pavilion Gaming 15-cx0041ur to the ACPI EC
driver (Mia Kanashi).
- Add some mew ACPI backlight handling quirks and update some existing
ones (Hans de Goede).
- Make the ACPI backlight driver prefer the native backlight control
over vendor backlight control when possible (Hans de Goede).
* acpi-pm:
ACPI: PM: Silence missing prototype warning
ACPI: PM: Print full name path while adding power resource
* acpi-processor:
ACPI: processor: perflib: Adjust acpi_processor_notify_smm() return value
ACPI: processor: perflib: Rearrange acpi_processor_notify_smm()
ACPI: processor: perflib: Rearrange unregistration routine
ACPI: processor: perflib: Drop redundant parentheses
ACPI: processor: perflib: Adjust white space
ACPI: processor: idle: Drop unnecessary statements and parens
ACPI: processor: Silence missing prototype warnings
ACPI: processor_idle: Silence missing prototype warnings
ACPI: processor: throttling: remove variable count
ACPI: processor: idle: Check acpi_fetch_acpi_dev() return value
* acpi-ec:
ACPI: EC: Add quirk for the HP Pavilion Gaming 15-cx0041ur
* acpi-video:
ACPI: video: Prefer native over vendor
ACPI: video: Simplify __acpi_video_get_backlight_type()
ACPI: video: Add force_native quirk for Sony Vaio VPCY11S1E
ACPI: video: Add force_vendor quirk for Sony Vaio PCG-FRV35
ACPI: video: Change Sony Vaio VPCEH3U1E quirk to force_native
ACPI: video: Change GIGABYTE GB-BXBT-2807 quirk to force_none
ACPI: video: Add a few bugtracker links to DMI quirks
ACPI-2.0 says that the EC OpRegion handler must be available immediately
(like the standard default OpRegion handlers):
Quoting from the ACPI spec version 6.3: "6.5.4 _REG (Region) ...
2. OSPM must make Embedded Controller operation regions, accessed via
the Embedded Controllers described in ECDT, available before executing
any control method. These operation regions may become inaccessible
after OSPM runs _REG(EmbeddedControl, 0)."
So acpi_bus_init() calls acpi_ec_ecdt_probe(), which calls
acpi_install_address_space_handler() to install the EC's OpRegion
handler, early on.
This not only installs the OpRegion handler, but also calls the EC's
_REG method. The _REG method call is a problem because it may rely on
initialization done by the _INI methods of one of the PCI / _SB root devs,
see for example: https://bugzilla.kernel.org/show_bug.cgi?id=214899 .
Generally speaking _REG methods are executed when the ACPI-device they
are part of has a driver bound to it. Where as _INI methods must be
executed at table load time (according to the spec). The problem here
is that the early acpi_install_address_space_handler() call causes
the _REG handler to run too early.
To allow fixing this the ACPICA code now allows to split the OpRegion
handler installation and the executing of _REG into 2 separate steps.
This commit uses this ACPICA functionality to fix the EC probe ordering
by delaying the executing of _REG for ECDT described ECs till the matching
EC device in the DSDT gets parsed and acpi_ec_add() for it gets called.
This moves the calling of _REG for the EC on devices with an ECDT to
the same point in time where it is called on devices without an ECDT table.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214899
Reported-and-tested-by: Johannes Penßel <johannespenssel@posteo.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
When an ECDT table is present the EC address space handler gets registered
on the root node. So to unregister it properly the unregister call also
must be done on the root node.
Store the ACPI handle used for the acpi_install_address_space_handler()
call and use te same handle for the acpi_remove_address_space_handler()
call.
Reported-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
For bus-based driver, device removal is implemented as:
1 device_remove()->
2 bus->remove()->
3 driver->remove()
Driver core needs no inform from callee(bus driver) about the
result of remove callback. In that case, commit fc7a6209d5
("bus: Make remove callback return void") forces bus_type::remove
be void-returned.
Now we have the situation that both 1 & 2 of calling chain are
void-returned, so it does not make much sense for 3(driver->remove)
to return non-void to its caller.
So the basic idea behind this change is making remove() callback of
any bus-based driver to be void-returned.
This change, for itself, is for device drivers based on acpi-bus.
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Lee Jones <lee@kernel.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Dawei Li <set_pte_at@outlook.com>
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> # for drivers/platform/surface/*
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Added GPE quirk entry for the HP Pavilion Gaming 15-cx0041ur.
There is a quirk entry for the 15-cx0xxx laptops, but this one has
different DMI_PRODUCT_NAME.
Notably backlight keys and other ACPI events now function correctly.
Signed-off-by: Mia Kanashi <chad@redpilled.dev>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Return the acpi_ec_write() return value directly instead of storing it
in another redundant variable.
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn>
[ rjw: Subject and changelog edits ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
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 <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
EC_FLAGS_TRUST_DSDT_GPE only does anything when the:
if (boot_ec && ec->command_addr == boot_ec->command_addr &&
ec->data_addr == boot_ec->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->gpe to ec->gpe, when the flag is set,
similar to how this is already one done for boot_ec->handle.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It seems that these quirks are no longer necessary since
commit 69b957c26b ("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 6306f04319 ("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>
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 <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Rearange acpi_ec_event_handler() so as to avoid releasing ec->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 <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
Commit 4a9af6cac0 ("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: 4a9af6cac0 ("ACPI: EC: Rework flushing of EC work while suspended to idle")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
The EC driver uses a relatively simple state machine for the event
work handling, but it is not really straightforward to figure out.
The states are as follows:
"Ready": The event handling work can be submitted.
In this state, the EC_FLAGS_QUERY_PENDING flag is clear.
"In progress": The event handling work is pending or is being
processed. It cannot be submitted again.
In ths state, the EC_FLAGS_QUERY_PENDING flag is set and both the
events_to_process count is nonzero and the EC_FLAGS_QUERY_GUARDING
flag is clear.
"Complete": The event handling work has been completed, but it still
cannot be submitted again.
In ths state, the EC_FLAGS_QUERY_PENDING flag is set and the
events_to_process count is zero or the EC_FLAGS_QUERY_GUARDING
flag is set.
The state changes from "Ready" to "In progress" when new event is
detected by advance_transaction() and acpi_ec_submit_event() is
called by it.
Next, the state can change from "In progress" directly to "Ready" in
the following situations:
* ec_event_clearing is ACPI_EC_EVT_TIMING_STATUS and the state of
an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_POLL.
* ec_event_clearing is ACPI_EC_EVT_TIMING_QUERY and the state of
an ACPI_EC_COMMAND_QUERY transaction becomes
ACPI_EC_COMMAND_COMPLETE.
* ec_event_clearing is either ACPI_EC_EVT_TIMING_STATUS or
ACPI_EC_EVT_TIMING_QUERY and there are no more events to
process (ie. ec->events_to_process becomes 0).
If ec_event_clearing is ACPI_EC_EVT_TIMING_EVENT, however, the
state must change from "In progress" to "Complete" before it
can change to "Ready". The changes from "In progress" to
"Complete" in that case occur in the following situations:
* The state of an ACPI_EC_COMMAND_QUERY transaction becomes
ACPI_EC_COMMAND_COMPLETE.
* There are no more events to process (ie. ec->events_to_process
becomes 0).
Finally, the state changes from "Complete" to "Ready" when
advance_transaction() is invoked when the state is "Complete" and
the state of the current transaction is not ACPI_EC_COMMAND_POLL.
To make this state machine visible in the code, add a new
event_state field to struct acpi_ec and modify the code to use
it istead the EC_FLAGS_QUERY_PENDING and EC_FLAGS_QUERY_GUARDING
flags.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Notice that it is not necessary to queue up the event work again
if the while () loop in acpi_ec_event_handler() is still running
which is the case if nr_pending_queries is greater than 0 at the
beginning of acpi_ec_submit_event() and modify the code to avoid
doing that.
While at it, rename nr_pending_queries in struct acpi_ec to
events_to_process which actually matches the role of that field
and change its data type to unsigned int which is sufficient.
No expected functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Rename acpi_ec_submit_query() to acpi_ec_submit_event(),
acpi_ec_query() to acpi_ec_submit_query(), and
acpi_ec_complete_query() to acpi_ec_close_event() to make
the names reflect what the functions do.
No expected functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because acpi_ec_event_handler() is a work function, it always
runs in process context with interrupts enabled, so it can use
spin_lock_irq() and spin_unlock_irq() for the locking.
Make it do so and adjust white space around those calls.
No expected functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is not necessary to check ec->nr_pending_queries against 0 in the
while () loop in acpi_ec_event_handler(), because that loop terminates
when ec->nr_pending_queries is 0 and the code depending on that can be
run after the loop has ended.
Modify the code accordingly and while at it rewrite the comment
regarding that code to make it clearer.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because acpi_ec_event_handler() is the only caller of
acpi_ec_check_event() and the separation of these two functions
makes it harder to follow the code flow, fold the latter into the
former (and simplify that code while at it).
No expected functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Notice that the second argument to acpi_ec_query() is redundant,
because in the only case when it is not NULL, the value passed
through it is only checked against 0 and it can only be 0 when
acpi_ec_query() returns an error code, but its return value
is checked along with the value passed through its second
argument.
Accordingly, modify acpi_ec_query() to take only one argument
and while at it, change its handling of the case when
acpi_ec_transaction() returns an error so as to return that
error value to the caller right away.
No expected functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Calling acpi_dispatch_gpe() from acpi_ec_dispatch_gpe() is generally
problematic, because it may cause the spurious interrupt handling in
advance_transaction() to trigger in theory.
However, instead of calling acpi_dispatch_gpe() to dispatch the EC
GPE, acpi_ec_dispatch_gpe() can call advance_transaction() directly
on first_ec and it can pass 'false' as its second argument to indicate
calling it from process context.
Moreover, if advance_transaction() is modified to return a bool value
indicating whether or not the EC work needs to be flushed, it can be
used to avoid unnecessary EC work flushing in acpi_ec_dispatch_gpe(),
so change the code accordingly.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The flushing of pending work in the EC driver uses drain_workqueue()
to flush the event handling work that can requeue itself via
advance_transaction(), but this is problematic, because that
work may also be requeued from the query workqueue.
Namely, if an EC transaction is carried out during the execution of
a query handler, it involves calling advance_transaction() which
may queue up the event handling work again. This causes the kernel
to complain about attempts to add a work item to the EC event
workqueue while it is being drained and worst-case it may cause a
valid event to be skipped.
To avoid this problem, introduce two new counters, events_in_progress
and queries_in_progress, incremented when a work item is queued on
the event workqueue or the query workqueue, respectively, and
decremented at the end of the corresponding work function, and make
acpi_ec_dispatch_gpe() the workqueues in a loop until the both of
these counters are zero (or system wakeup is pending) instead of
calling acpi_ec_flush_work().
At the same time, change __acpi_ec_flush_work() to call
flush_workqueue() instead of drain_workqueue() to flush the event
workqueue.
While at it, use the observation that the work item queued in
acpi_ec_query() cannot be pending at that time, because it is used
only once, to simplify the code in there.
Additionally, clean up a comment in acpi_ec_query() and adjust white
space in acpi_ec_event_processor().
Fixes: f0ac20c3f6 ("ACPI: EC: Fix flushing of pending work")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Remove the initialization of two static variables to false which is
pointless.
Signed-off-by: wangzhitong <wangzhitong@uniontech.com>
[ rjw: Subject and changelog ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
EC interrupts constantly wake up system from s2idle, so set
ec_no_wakeup by default to keep the system in s2idle and reduce
energy consumption.
Signed-off-by: Binbin Zhou <zhoubinbin@uniontech.com>
[ rjw: Changelog and subject edits ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
* acpi-ec:
ACPI: EC: trust DSDT GPE for certain HP laptop
ACPI: EC: Make more Asus laptops use ECDT _GPE
* acpi-apei:
ACPI: APEI: fix synchronous external aborts in user-mode
ACPI: APEI: Don't warn if ACPI is disabled
* acpi-soc:
ACPI: LPSS: Use kstrtol() instead of simple_strtol()
* acpi-misc:
ACPI: NVS: fix doc warnings in nvs.c
ACPI: NUMA: fix typo in a comment
ACPI: OSL: Use DEFINE_RES_IO_NAMED() to simplify code
ACPI: bus: Call kobject_put() in acpi_init() error path
ACPI: bus: Remove unneeded assignment
ACPI: configfs: Replace ACPI_INFO() with pr_debug()
ACPI: ipmi: Remove address space handler in error path
ACPI: event: Remove redundant initialization of local variable
ACPI: sbshc: Fix fall-through warning for Clang
On HP Pavilion Gaming Laptop 15-cx0xxx, the ECDT EC and DSDT EC share
the same port addresses but different GPEs. And the DSDT GPE is the
right one to use.
The current code duplicates DSDT EC with ECDT EC if the port addresses
are the same, and uses ECDT GPE as a result, which breaks this machine.
Introduce a new quirk for the HP laptop to trust the DSDT GPE,
and avoid duplicating even if the port addresses are the same.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=209989
Reported-and-tested-by: Shao Fu, Chen <leo881003@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The acpi_walk_dep_device_list() function is not as generic as its
name implies, serving only to decrement the dependency count for each
dependent device of the input.
Extend it to accept a callback which can be applied to all the
dependencies in acpi_dep_list.
Replace all existing calls to the function with calls to a wrapper,
passing a callback that applies the same dependency reduction.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Maximilian Luz <luzmaximilian@gmail.com> # for platform/surface parts
Signed-off-by: Daniel Scally <djrscally@gmail.com>
[ rjw: Changelog edits ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
More ASUS laptops have the _GPE define in the DSDT table with a
different value than the _GPE number in the ECDT.
This is causing media keys not working on ASUS X505BA/BP, X542BA/BP
Add model info to the quirks list.
Signed-off-by: Chris Chiu <chris.chiu@canonical.com>
Signed-off-by: Jian-Hong Pan <jhp@endlessos.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Eliminate comparisons from the status flags checks in
advance_transaction() (especially from the one that is only correct,
because the value of the flag checked in there is 1) and rearrange
the code for more clarity while at it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Introduce acpi_ec_spurious_interrupt() for recording spurious
interrupts and use it for error handling in advance_transaction(),
drop the (now redundant) original error handling block from there
along with a frew goto statements that are not necessary any more.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Notice that the value of t in advance_transaction() does not change
after its initialization and:
- Initialize t upfront (and rearrange the definitions of local
variables while at it).
- Check it against NULL in a block executed when it is NULL.
- Skip error handling for t == NULL, because a valid pointer value
of t is required for the error handling.
- Drop the (now redundant) check of t against NULL from the error
handling block and reduce the indentation level in there.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Rename acpi_ec_is_gpe_raised() into acpi_ec_gpe_status_set(),
update its callers accordingly and drop the ternary operator
(which isn't really necessary in there) from it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Fold acpi_ec_clear_gpe() which is only used in one place into its
caller and clean up comments related to that function while at it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
advance_transaction() is using in_interrupt() to distinguish between
an invocation from the interrupt handler and an invocation from
another part of the stack.
This looks misleading because chains like
acpi_update_all_gpes() -> acpi_ev_gpe_detect() ->
acpi_ev_detect_gpe() -> acpi_ec_gpe_handler()
should probably also behave as if they were called from an interrupt
handler.
Replace in_interrupt() usage with a function parameter.
Set this parameter to `true' if invoked from an interrupt handler
(acpi_ec_gpe_handler() and acpi_ec_irq_handler()) and `false'
otherwise.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
[ rjw: Subject edits ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It turns out that in some cases there are EC events to flush in
acpi_ec_dispatch_gpe() even though the ec_no_wakeup kernel parameter
is set and the EC GPE is disabled while sleeping, so drop the
ec_no_wakeup check that prevents those events from being processed
from acpi_ec_dispatch_gpe().
Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Commit 607b9df630 ("ACPI: EC: PM: Avoid flushing EC work when EC
GPE is inactive") has been reported to cause some power button wakeup
events to be missed on some systems, so modify acpi_ec_dispatch_gpe()
to call acpi_ec_flush_work() unconditionally to effectively reverse
the changes made by that commit.
Also note that the problem which prompted commit 607b9df630 is not
reproducible any more on the affected machine.
Fixes: 607b9df630 ("ACPI: EC: PM: Avoid flushing EC work when EC GPE is inactive")
Reported-by: Raymond Tan <raymond.tan@intel.com>
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
When I cat acpi module parameter
'/sys/module/acpi/parameters/ec_event_clearing', it displays as follows.
It is better to add a newline for easy reading.
[root@hulk-202 ~]# cat /sys/module/acpi/parameters/ec_event_clearing
query[root@hulk-202 ~]#
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
- Update the ACPICA code in the kernel to upstream revision
20200430:
* Move acpi_gbl_next_cmd_num definition (Erik Kaneda).
* Ignore AE_ALREADY_EXISTS status in the disassembler when parsing
create operators (Erik Kaneda).
* Add status checks to the dispatcher (Erik Kaneda).
* Fix required parameters for _NIG and _NIH (Erik Kaneda).
* Make acpi_protocol_lengths static (Yue Haibing).
- Fix ACPI table reference counting errors in several places, mostly
in error code paths (Hanjun Guo).
- Extend the Generic Event Device (GED) driver to support _Exx and
_Lxx handler methods (Ard Biesheuvel).
- Add new acpi_evaluate_reg() helper and modify the ACPI PCI hotplug
code to use it (Hans de Goede).
- Add new DPTF battery participant driver and make the DPFT power
participant driver create more sysfs device attributes (Srinivas
Pandruvada).
- Improve the handling of memory failures in APEI (James Morse).
- Add new blacklist entry for Acer TravelMate 5735Z to the backlight
driver (Paul Menzel).
- Add i2c address for thermal control to the PMIC driver (Mauro
Carvalho Chehab).
- Allow the ACPI processor idle driver to work on platforms with
only one ACPI C-state present (Zhang Rui).
- Fix kobject reference count leaks in error code paths in two
places (Qiushi Wu).
- Delete unused proc filename macros and make some symbols static
(Pascal Terjan, Zheng Zengkai, Zou Wei).
-----BEGIN PGP SIGNATURE-----
iQJGBAABCAAwFiEE4fcc61cGeeHD/fCwgsRv/nhiVHEFAl7VHb8SHHJqd0Byand5
c29ja2kubmV0AAoJEILEb/54YlRxVboQAIjYda2RhQANIlIvoEa+Qd2/FBd3HXgU
Mv0LZ6y1xxxEZYeKne7zja1hzt5WetuZ1hZHGfg8YkXyrLqZGxfCIFbbhSA90BGG
PGzFerGmOBNzB3I9SN6iQY7vSqoFHvQEV1PVh24d+aHWZqj2lnaRRq+GT54qbRLX
/U3Hy5glFl8A/DCBP4cpoEjDr4IJHY68DathkDK2Ep2ybXV6B401uuqx8Su/OBd/
MQmJTYI1UK/RYBXfdzS9TIZahnkxBbU1cnLFy08Ve2mawl5YsHPEbvm77a0yX2M6
sOAerpgyzYNivAuOLpNIwhUZjpOY66nQuKAQaEl2cfRUkqt4nbmq7yDoH3d2MJLC
/Ccz955rV2YyD1DtyV+PyT+HB+/EVwH/+UCZ+gsSbdHvOiwdFU6VaTc2eI1qq8K9
4m5eEZFrAMPlvTzj/xVxr2Hfw1lbm23J5B5n7sM5HzYbT6MUWRQpvfV4zM3jTGz0
rQd8JmcHVvZk/MV1mGrYHrN5TnGTLWpbS4Yv1lAQa6FP0N0NxzVud7KRfLKnCnJ1
vh5yzW2fCYmVulJpuqxJDfXSqNV7n40CFrIewSp6nJRQXnWpImqHwwiA8fl51+hC
fBL72Ey08EHGFnnNQqbebvNglsodRWJddBy43ppnMHtuLBA/2GVKYf2GihPbpEBq
NHtX+Rd3vlWW
=xH3i
-----END PGP SIGNATURE-----
Merge tag 'acpi-5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
Pull ACPI updates from Rafael Wysocki:
"These update the ACPICA code in the kernel to upstream revision
20200430, fix several reference counting errors related to ACPI
tables, add _Exx / _Lxx support to the GED driver, add a new
acpi_evaluate_reg() helper, add new DPTF battery participant driver
and extend the DPFT power participant driver, improve the handling of
memory failures in the APEI code, add a blacklist entry to the
backlight driver, update the PMIC driver and the processor idle
driver, fix two kobject reference count leaks, and make a few janitory
changes.
Specifics:
- Update the ACPICA code in the kernel to upstream revision 20200430:
- Move acpi_gbl_next_cmd_num definition (Erik Kaneda).
- Ignore AE_ALREADY_EXISTS status in the disassembler when parsing
create operators (Erik Kaneda).
- Add status checks to the dispatcher (Erik Kaneda).
- Fix required parameters for _NIG and _NIH (Erik Kaneda).
- Make acpi_protocol_lengths static (Yue Haibing).
- Fix ACPI table reference counting errors in several places, mostly
in error code paths (Hanjun Guo).
- Extend the Generic Event Device (GED) driver to support _Exx and
_Lxx handler methods (Ard Biesheuvel).
- Add new acpi_evaluate_reg() helper and modify the ACPI PCI hotplug
code to use it (Hans de Goede).
- Add new DPTF battery participant driver and make the DPFT power
participant driver create more sysfs device attributes (Srinivas
Pandruvada).
- Improve the handling of memory failures in APEI (James Morse).
- Add new blacklist entry for Acer TravelMate 5735Z to the backlight
driver (Paul Menzel).
- Add i2c address for thermal control to the PMIC driver (Mauro
Carvalho Chehab).
- Allow the ACPI processor idle driver to work on platforms with only
one ACPI C-state present (Zhang Rui).
- Fix kobject reference count leaks in error code paths in two places
(Qiushi Wu).
- Delete unused proc filename macros and make some symbols static
(Pascal Terjan, Zheng Zengkai, Zou Wei)"
* tag 'acpi-5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm: (32 commits)
ACPI: CPPC: Fix reference count leak in acpi_cppc_processor_probe()
ACPI: sysfs: Fix reference count leak in acpi_sysfs_add_hotplug_profile()
ACPI: GED: use correct trigger type field in _Exx / _Lxx handling
ACPI: DPTF: Add battery participant driver
ACPI: DPTF: Additional sysfs attributes for power participant driver
ACPI: video: Use native backlight on Acer TravelMate 5735Z
arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
ACPI: APEI: Kick the memory_failure() queue for synchronous errors
mm/memory-failure: Add memory_failure_queue_kick()
ACPI / PMIC: Add i2c address for thermal control
ACPI: GED: add support for _Exx / _Lxx handler methods
ACPI: Delete unused proc filename macros
ACPI: hotplug: PCI: Use the new acpi_evaluate_reg() helper
ACPI: utils: Add acpi_evaluate_reg() helper
ACPI: debug: Make two functions static
ACPI: sleep: Put the FACS table after using it
ACPI: scan: Put SPCR and STAO table after using it
ACPI: EC: Put the ACPI table after using it
ACPI: APEI: Put the HEST table for error path
ACPI: APEI: Put the error record serialization table for error path
...
* acpi-processor:
ACPI: processor: idle: Allow probing on platforms with one ACPI C-state
* acpi-cppc:
ACPI: CPPC: Fix reference count leak in acpi_cppc_processor_probe()
ACPI: CPPC: Make some symbols static
* acpi-dbg:
ACPI: debug: Make two functions static
* acpi-misc:
ACPI: GED: use correct trigger type field in _Exx / _Lxx handling
ACPI: GED: add support for _Exx / _Lxx handler methods
ACPI: Delete unused proc filename macros
* acpi-pci:
ACPI: hotplug: PCI: Use the new acpi_evaluate_reg() helper
ACPI: utils: Add acpi_evaluate_reg() helper
* acpica:
ACPICA: Update version to 20200430
ACPICA: Fix required parameters for _NIG and _NIH
ACPICA: Dispatcher: add status checks
ACPICA: Disassembler: ignore AE_ALREADY_EXISTS status when parsing create operators
ACPICA: Move acpi_gbl_next_cmd_num definition to acglobal.h
ACPICA: Make acpi_protocol_lengths static
* acpi-tables:
ACPI: sleep: Put the FACS table after using it
ACPI: scan: Put SPCR and STAO table after using it
ACPI: EC: Put the ACPI table after using it
ACPI: APEI: Put the HEST table for error path
ACPI: APEI: Put the error record serialization table for error path
ACPI: APEI: Put the error injection table for error path and module exit
ACPI: APEI: Put the boot error record table after parsing
ACPI: watchdog: Put the watchdog action table after parsing
ACPI: LPIT: Put the low power idle table after using it
Flushing the EC work while suspended to idle when the EC GPE status
is not set causes some EC wakeup events (notably power button and
lid ones) to be missed after a series of spurious wakeups on the Dell
XPS13 9360 in my office.
If that happens, the machine cannot be woken up from suspend-to-idle
by the power button or lid status change and it needs to be woken up
in some other way (eg. by a key press).
Flushing the EC work only after successful dispatching the EC GPE,
which means that its status has been set, avoids the issue, so change
the code in question accordingly.
Fixes: 7b301750f7 ("ACPI: EC: PM: Avoid premature returns from acpi_s2idle_wake()")
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Chris Chiu <chiu@endlessm.com>
Those were used to create files in /proc/acpi long ago
and were missed when that code was deleted.
Signed-off-by: Pascal Terjan <pterjan@google.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If the EC GPE status is not set after checking all of the other GPEs,
acpi_s2idle_wake() returns 'false', to indicate that the SCI event
that has just triggered is not a system wakeup one, but it does that
without canceling the pending wakeup and re-arming the SCI for system
wakeup which is a mistake, because it may cause s2idle_loop() to busy
spin until the next valid wakeup event. [If that happens, the first
spurious wakeup is still pending after acpi_s2idle_wake() has
returned, so s2idle_enter() does nothing, acpi_s2idle_wake()
is called again and it sees that the SCI has triggered, but no GPEs
are active, so 'false' is returned again, and so on.]
Fix that by moving all of the GPE checking logic from
acpi_s2idle_wake() to acpi_ec_dispatch_gpe() and making the
latter return 'true' only if a non-EC GPE has triggered and
'false' otherwise, which will cause acpi_s2idle_wake() to
cancel the pending SCI wakeup and re-arm the SCI for system
wakeup regardless of the EC GPE status.
This also addresses a lockup observed on an Elitegroup EF20EA laptop
after attempting to wake it up from suspend-to-idle by a key press.
Fixes: d5406284ff ("ACPI: PM: s2idle: Refine active GPEs check")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207603
Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
Fixes: fdde0ff859 ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system")
Link: https://lore.kernel.org/linux-acpi/CAB4CAwdqo7=MvyG_PE+PGVfeA17AHF5i5JucgaKqqMX6mjArbQ@mail.gmail.com/
Reported-by: Chris Chiu <chiu@endlessm.com>
Tested-by: Chris Chiu <chiu@endlessm.com>
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The embedded controller boot resources table needs to be
released after using it.
Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
[ rjw: avoid adding a label in acpi_ec_ecdt_start() ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The fast path check in acpi_ec_add() is not incorrect, because in
fact acpi_device_hid(device) can be equal to ACPI_ECDT_HID only if
boot_ec is not NULL, but it may confuse static checkers, so change
it to explicitly check boot_ec upfront and use the slow path if
that pointer is NULL.
Link: https://lore.kernel.org/linux-acpi/20200406144217.GA68494@mwanda/
Fixes: 3d9b8dd832 ("ACPI: EC: Use fast path in acpi_ec_add() for DSDT boot EC")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
