Skip to content

Commit

Permalink
ACPI: EC: fix ECDT probe ordering issues
Browse files Browse the repository at this point in the history
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>
---
Changes in v3:
- Rebase on top of v2 ACPICA patches which add 2 new functions for this
  instead of using a flags argument
  • Loading branch information
jwrdegoede committed Oct 15, 2022
1 parent 595c8f4 commit 1f2474c
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions drivers/acpi/ec.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ enum {
EC_FLAGS_QUERY_ENABLED, /* Query is enabled */
EC_FLAGS_EVENT_HANDLER_INSTALLED, /* Event handler installed */
EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */
EC_FLAGS_EC_REG_CALLED, /* OpReg ACPI _REG method called */
EC_FLAGS_QUERY_METHODS_INSTALLED, /* _Qxx handlers installed */
EC_FLAGS_STARTED, /* Driver is started */
EC_FLAGS_STOPPED, /* Driver is stopped */
Expand Down Expand Up @@ -1450,6 +1451,7 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec)
* ec_install_handlers - Install service callbacks and register query methods.
* @ec: Target EC.
* @device: ACPI device object corresponding to @ec.
* @call_reg: If _REG should be called to notify OpRegion availability
*
* Install a handler for the EC address space type unless it has been installed
* already. If @device is not NULL, also look for EC query methods in the
Expand All @@ -1462,18 +1464,19 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec)
* -EPROBE_DEFER if GPIO IRQ acquisition needs to be deferred,
* or 0 (success) otherwise.
*/
static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device)
static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device,
bool call_reg)
{
acpi_status status;

acpi_ec_start(ec, false);

if (!test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
acpi_ec_enter_noirq(ec);
status = acpi_install_address_space_handler(ec->handle,
ACPI_ADR_SPACE_EC,
&acpi_ec_space_handler,
NULL, ec);
status = acpi_install_address_space_handler_no_reg(ec->handle,
ACPI_ADR_SPACE_EC,
&acpi_ec_space_handler,
NULL, ec);
if (ACPI_FAILURE(status)) {
acpi_ec_stop(ec, false);
return -ENODEV;
Expand All @@ -1482,6 +1485,11 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device)
ec->address_space_handler_holder = ec->handle;
}

if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
acpi_execute_reg_methods(ec->handle, ACPI_ADR_SPACE_EC);
set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
}

if (!device)
return 0;

Expand Down Expand Up @@ -1568,11 +1576,11 @@ static void ec_remove_handlers(struct acpi_ec *ec)
}
}

static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device)
static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device, bool call_reg)
{
int ret;

ret = ec_install_handlers(ec, device);
ret = ec_install_handlers(ec, device, call_reg);
if (ret)
return ret;

Expand Down Expand Up @@ -1637,7 +1645,7 @@ static int acpi_ec_add(struct acpi_device *device)
}
}

ret = acpi_ec_setup(ec, device);
ret = acpi_ec_setup(ec, device, true);
if (ret)
goto err;

Expand Down Expand Up @@ -1757,7 +1765,7 @@ void __init acpi_ec_dsdt_probe(void)
* At this point, the GPE is not fully initialized, so do not to
* handle the events.
*/
ret = acpi_ec_setup(ec, NULL);
ret = acpi_ec_setup(ec, NULL, true);
if (ret) {
acpi_ec_free(ec);
return;
Expand Down Expand Up @@ -1941,7 +1949,7 @@ void __init acpi_ec_ecdt_probe(void)
* At this point, the namespace is not initialized, so do not find
* the namespace objects, or handle the events.
*/
ret = acpi_ec_setup(ec, NULL);
ret = acpi_ec_setup(ec, NULL, false);
if (ret) {
acpi_ec_free(ec);
goto out;
Expand Down

0 comments on commit 1f2474c

Please sign in to comment.