On Mon, Jan 7, 2019 at 12:51 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> ca On Sat, Jan 5, 2019 at 2:17 PM Zhang Rui <rui.zhang@xxxxxxxxx> wrote:
> >
> > From 6f2fac0ffcd7fd61036baa0798ab171496cff50f Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@xxxxxxxxx>
> > Date: Fri, 4 Jan 2019 14:35:52 +0800
> > Subject: [PATCH] ACPI/ACPICA: Run EC _REG explicitly for ECDT
> >
> > commit d737f333b211 ("ACPI: probe ECDT before loading AML tables
> > regardless of module-level code flag") probes ECDT before loading
> > the AML tables.
> >
> > This is the right thing to do according to the ACPI Spec, but
> > unfortunately, it breaks the current kernel EC/ECDT support, and makes
> > many devices, including battery, lid, etc, fails to work on a variety
> > of platforms.
> >
> > This is because:
> > 1. Probing ECDT requires installing EC address space handler
> > 2. EC _REG can not be evaluated at the time when probing ECDT because AML
> > tables have not been loaded yet.
> > 3. Many devices fail to work because EC _REG is not evaluated.
> >
> > To fix this, a solution is proposed in this patch to evaluate EC _REG
> > explicitly in ACPICA, if ECDT has been probed.
>
> It would be good to give some more details here as the patch itself
> appears to be rather convoluted.
>
> Also, the description above doesn't actually explain why the problem
> is there, as it doesn't explain why probing the ECDT early causes the
> EC _REG to be not evaluated.
>
> It looks like the failure is due to the change of the ordering between
> acpi_load_tables() and acpi_ec_ecdt_probe() in the
> acpi_gbl_group_module_level_code case which causes the EC to be probed
> before instantiating the namespace and _REG obviously cannot be
> evaluated then.
So what happens is:
acpi_ec_ecdt_probe()
acpi_ec_ecdt_probe()
acpi_config_boot_ec(ec, ACPI_ROOT_OBJECT, false, true)
...
ec->handle = ACPI_ROOT_OBJECT;
...
acpi_ec_setup(ec, false)
ec_install_handlers(ec, false)
acpi_install_address_space_handler(ACPI_ROOT_OBJECT,
ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, NULL, ec)
...
set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)
and now it returns, because handle_events is "false".
Next time acpi_ec_setup() is called from acpi_ec_add() that can be
invoked in two ways, either through
acpi_bus_register_driver(&acpi_ec_driver) which runs it for the DSDT
EC (if found), or through acpi_ec_ecdt_start() and
acpi_bus_register_early_device().
There are two cases in there, the acpi_config_boot_ec() one and the
direct invocation of acpi_ec_setup() when acpi_is_boot_ec(ec) returns
"false". The former is the failing case AFAICS, because
acpi_ec_ecdt_probe() (if successful) has set boot_ec as well as
ec->command_addr and ec->data_addr.
So acpi_config_boot_ec() runs again and since boot_ec->handle has not
been updated, it doesn't call ec_remove_handlers(), and because the
EC_FLAGS_EC_HANDLER_INSTALLED is set, the address space handler
installed previously is retained and _REG is not evaluated. Since it
wasn't evaluated before (as it was not present then), it is never
evaluated and the failure occurs.
So it looks like clearing ec-> handle before invoking
acpi_config_boot_ec() in the is_ecdt case in acpi_ec_add() can help,
can't it?
