Re: Missing default handler for the EmbeddedControl OpRegion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux