On Mon, May 06, 2024 at 07:45:07PM +0200, Rafael J. Wysocki wrote: > Hi, > > On Mon, Apr 29, 2024 at 4:55 PM Heikki Krogerus > <heikki.krogerus@xxxxxxxxxxxxxxx> wrote: > > > > Hi, > > > > There's a bug that is caused by an EmbeddedControl OpRegion which is > > declared inside the scope of a specific USB Type-C device (PNP0CA0): > > https://bugzilla.kernel.org/show_bug.cgi?id=218789 > > And in this bug you are essentially proposing to install the EC > OpRegion handler at the namespace root instead of the EC device. > > This sounds reasonable, although AFAICS this is a matter of modifying > the EC driver (before the EC OpRegion handler is installed by the EC > drvier, ACPICA has no way to handle EC address space accesses anyway). > > > It looks like that's not the only case where that OpRegion ID is used > > outside of the EC device scope. There is at least one driver in Linux > > Kernel (drivers/platform/x86/wmi.c) that already has a custom handler > > for the EmbeddedControl OpRegion, and based on a quick search, the > > problem "Region EmbeddedControl (ID=3) has no handler" has happened > > with some other devices too. > > AFAICS, installing the EC address space handler at the EC device > object itself is not based on any sound technical arguments, it's just > been always done this way in Linux. It's quite possible that the EC > address space handler should have been installed at the namespace root > from the outset. Okay, thank you for the explanation. So can we simply change it like this (I may have still misunderstood something)? diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 02255795b800..6b9dd27171ee 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1488,7 +1488,7 @@ 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_no_reg(ec->handle, + status = acpi_install_address_space_handler_no_reg(ACPI_ROOT_OBJECT, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, NULL, ec); @@ -1497,7 +1497,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; + ec->address_space_handler_holder = ACPI_ROOT_OBJECT; } if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) { thanks, -- heikki
