From a5072078dbfaa9d70130805766dfa34bbb7bf2a7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 8 Dec 2022 15:23:34 +0100 Subject: ACPI: EC: Fix EC address space handler unregistration 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 Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 4 +++- drivers/acpi/internal.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 9751b84c1b22..5a21e4d58322 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1475,6 +1475,7 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) return -ENODEV; } set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags); + ec->address_space_handler_holder = ec->handle; } if (!device) @@ -1526,7 +1527,8 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) static void ec_remove_handlers(struct acpi_ec *ec) { if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) { - if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle, + if (ACPI_FAILURE(acpi_remove_address_space_handler( + ec->address_space_handler_holder, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler))) pr_err("failed to remove space handler\n"); clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags); diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 219c02df9a08..ec584442fb29 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -173,6 +173,7 @@ enum acpi_ec_event_state { struct acpi_ec { acpi_handle handle; + acpi_handle address_space_handler_holder; int gpe; int irq; unsigned long command_addr; -- cgit v1.2.3-70-g09d2 From ab4620f58d38206687b9f99d9d2cc1d5a2640985 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 8 Dec 2022 15:23:35 +0100 Subject: ACPI: EC: Fix ECDT probe ordering issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 5a21e4d58322..73ac2f222897 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -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 */ @@ -1446,6 +1447,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 @@ -1458,7 +1460,8 @@ 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; @@ -1466,10 +1469,10 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) 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; @@ -1478,6 +1481,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; @@ -1564,11 +1572,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; @@ -1633,7 +1641,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; @@ -1753,7 +1761,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; @@ -1947,7 +1955,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; -- cgit v1.2.3-70-g09d2