Skip to content

Commit ab4620f

Browse files
jwrdegoederafaeljw
authored andcommitted
ACPI: EC: Fix ECDT probe ordering issues
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 <[email protected]> Signed-off-by: Hans de Goede <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]>
1 parent a507207 commit ab4620f

File tree

1 file changed

+18
-10
lines changed

1 file changed

+18
-10
lines changed

drivers/acpi/ec.c

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ enum {
9494
EC_FLAGS_QUERY_ENABLED, /* Query is enabled */
9595
EC_FLAGS_EVENT_HANDLER_INSTALLED, /* Event handler installed */
9696
EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */
97+
EC_FLAGS_EC_REG_CALLED, /* OpReg ACPI _REG method called */
9798
EC_FLAGS_QUERY_METHODS_INSTALLED, /* _Qxx handlers installed */
9899
EC_FLAGS_STARTED, /* Driver is started */
99100
EC_FLAGS_STOPPED, /* Driver is stopped */
@@ -1446,6 +1447,7 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec)
14461447
* ec_install_handlers - Install service callbacks and register query methods.
14471448
* @ec: Target EC.
14481449
* @device: ACPI device object corresponding to @ec.
1450+
* @call_reg: If _REG should be called to notify OpRegion availability
14491451
*
14501452
* Install a handler for the EC address space type unless it has been installed
14511453
* already. If @device is not NULL, also look for EC query methods in the
@@ -1458,18 +1460,19 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec)
14581460
* -EPROBE_DEFER if GPIO IRQ acquisition needs to be deferred,
14591461
* or 0 (success) otherwise.
14601462
*/
1461-
static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device)
1463+
static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device,
1464+
bool call_reg)
14621465
{
14631466
acpi_status status;
14641467

14651468
acpi_ec_start(ec, false);
14661469

14671470
if (!test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
14681471
acpi_ec_enter_noirq(ec);
1469-
status = acpi_install_address_space_handler(ec->handle,
1470-
ACPI_ADR_SPACE_EC,
1471-
&acpi_ec_space_handler,
1472-
NULL, ec);
1472+
status = acpi_install_address_space_handler_no_reg(ec->handle,
1473+
ACPI_ADR_SPACE_EC,
1474+
&acpi_ec_space_handler,
1475+
NULL, ec);
14731476
if (ACPI_FAILURE(status)) {
14741477
acpi_ec_stop(ec, false);
14751478
return -ENODEV;
@@ -1478,6 +1481,11 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device)
14781481
ec->address_space_handler_holder = ec->handle;
14791482
}
14801483

1484+
if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
1485+
acpi_execute_reg_methods(ec->handle, ACPI_ADR_SPACE_EC);
1486+
set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
1487+
}
1488+
14811489
if (!device)
14821490
return 0;
14831491

@@ -1564,11 +1572,11 @@ static void ec_remove_handlers(struct acpi_ec *ec)
15641572
}
15651573
}
15661574

1567-
static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device)
1575+
static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device, bool call_reg)
15681576
{
15691577
int ret;
15701578

1571-
ret = ec_install_handlers(ec, device);
1579+
ret = ec_install_handlers(ec, device, call_reg);
15721580
if (ret)
15731581
return ret;
15741582

@@ -1633,7 +1641,7 @@ static int acpi_ec_add(struct acpi_device *device)
16331641
}
16341642
}
16351643

1636-
ret = acpi_ec_setup(ec, device);
1644+
ret = acpi_ec_setup(ec, device, true);
16371645
if (ret)
16381646
goto err;
16391647

@@ -1753,7 +1761,7 @@ void __init acpi_ec_dsdt_probe(void)
17531761
* At this point, the GPE is not fully initialized, so do not to
17541762
* handle the events.
17551763
*/
1756-
ret = acpi_ec_setup(ec, NULL);
1764+
ret = acpi_ec_setup(ec, NULL, true);
17571765
if (ret) {
17581766
acpi_ec_free(ec);
17591767
return;
@@ -1947,7 +1955,7 @@ void __init acpi_ec_ecdt_probe(void)
19471955
* At this point, the namespace is not initialized, so do not find
19481956
* the namespace objects, or handle the events.
19491957
*/
1950-
ret = acpi_ec_setup(ec, NULL);
1958+
ret = acpi_ec_setup(ec, NULL, false);
19511959
if (ret) {
19521960
acpi_ec_free(ec);
19531961
goto out;

0 commit comments

Comments
 (0)