From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
Len Brown <lenb@kernel.org>,
Robert Moore <robert.moore@intel.com>,
linux-acpi@vger.kernel.org, acpica-devel@lists.linux.dev
Subject: Re: Missing default handler for the EmbeddedControl OpRegion
Date: Wed, 8 May 2024 15:20:59 +0300 [thread overview]
Message-ID: <ZjtuKzqpfKFbRaUi@kuha.fi.intel.com> (raw)
In-Reply-To: <CAJZ5v0iAMG251n6yg-h6KCUywK2vSg4sX3FTnOuxcLaEvUwFPQ@mail.gmail.com>
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@linux.intel.com> 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
prev parent reply other threads:[~2024-05-08 12:21 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 14:54 Missing default handler for the EmbeddedControl OpRegion Heikki Krogerus
2024-05-05 17:03 ` Armin Wolf
2024-05-06 17:45 ` Rafael J. Wysocki
2024-05-08 12:20 ` Heikki Krogerus [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZjtuKzqpfKFbRaUi@kuha.fi.intel.com \
--to=heikki.krogerus@linux.intel.com \
--cc=acpica-devel@lists.linux.dev \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=rafael.j.wysocki@intel.com \
--cc=rafael@kernel.org \
--cc=robert.moore@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).