* [PATCH 0/2] ACPI: EC: Simplify boot EC setup
@ 2019-01-22 11:55 Rafael J. Wysocki
2019-01-22 11:56 ` [PATCH 1/2] ACPI: EC: Untangle " Rafael J. Wysocki
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2019-01-22 11:55 UTC (permalink / raw)
To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML
Hi All,
The setup of the boot EC is unnecessarily tangled now, so untangle it to
make the code flow in there easier to follow.
The only intentional functional impact of this series should be changes
in messages printed to the kernel log.
The patches are on top of https://patchwork.kernel.org/patch/10773757/
Thanks,
Rafael
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] ACPI: EC: Untangle boot EC setup 2019-01-22 11:55 [PATCH 0/2] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki @ 2019-01-22 11:56 ` Rafael J. Wysocki 2019-01-22 11:57 ` [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-01-22 11:56 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The checks in acpi_config_boot_ec() are mostly redundant in all of the cases when it is called and it is better to do them directly in its callers anyway, so do that and get rid of it. First, note that acpi_ec_ecdt_probe() is called when boot_ec is not set, so it doesn't neeed to take the other possibility into account. Accordingly, it only needs to set the handle field in the ec object to ACPI_ROOT_OBJECT, call acpi_ec_setup() and (if that is successful) set boot_ec to ec and boot_ec_is_ecdt to 'true'. Make it do so directly, without calling acpi_config_boot_ec(), and avoid the pointless checks in the latter. Second, acpi_ec_dsdt_probe() returns early if boot_ec is set, so at the point when it calls acpi_config_boot_ec() (passing ec->handle as the handle argument to it), boot_ec is always unset. Thus calling acpi_config_boot_ec() then is not really useful. It is sufficient to call acpi_ec_setup() directly and (if that is successful) set boot_ec, so make acpi_ec_dsdt_probe() do that. Finally, acpi_ec_add() calls acpi_config_boot_ec() when it finds that the device object passed to it represents a "boot" EC, but in that case the ec pointer passed to acpi_config_boot_ec() is guaranteed to be equal to boot_ec (and ec->handle is passed as the handle argument to it), so acpi_config_boot_ec() really only calls acpi_ec_setup() and prints a message. Avoid the pointless checks in acpi_config_boot_ec() by calling acpi_ec_setup() directly and print the message separately. With the above changes in place, there are no users of acpi_config_boot_ec(), so drop it. No intentional functional impact except for changed messages. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 69 ++++++++++++++++-------------------------------------- 1 file changed, 21 insertions(+), 48 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1539,49 +1539,6 @@ static int acpi_ec_setup(struct acpi_ec return ret; } -static int acpi_config_boot_ec(struct acpi_ec *ec, acpi_handle handle, - bool handle_events, bool is_ecdt) -{ - int ret; - - /* - * Changing the ACPI handle results in a re-configuration of the - * boot EC. And if it happens after the namespace initialization, - * it causes _REG evaluations. - */ - if (boot_ec && boot_ec->handle != handle) - ec_remove_handlers(boot_ec); - - /* Unset old boot EC */ - if (boot_ec != ec) - acpi_ec_free(boot_ec); - - /* - * ECDT device creation is split into acpi_ec_ecdt_probe() and - * acpi_ec_ecdt_start(). This function takes care of completing the - * ECDT parsing logic as the handle update should be performed - * between the installation/uninstallation of the handlers. - */ - if (ec->handle != handle) - ec->handle = handle; - - ret = acpi_ec_setup(ec, handle_events); - if (ret) - return ret; - - /* Set new boot EC */ - if (!boot_ec) { - boot_ec = ec; - boot_ec_is_ecdt = is_ecdt; - } - - acpi_handle_info(boot_ec->handle, - "Used as boot %s EC to handle transactions%s\n", - is_ecdt ? "ECDT" : "DSDT", - handle_events ? " and events" : ""); - return ret; -} - static bool acpi_ec_ecdt_get_handle(acpi_handle *phandle) { struct acpi_table_ecdt *ecdt_ptr; @@ -1649,12 +1606,17 @@ static int acpi_ec_add(struct acpi_devic acpi_ec_free(ec); ec = boot_ec; } - ret = acpi_config_boot_ec(ec, ec->handle, true, is_ecdt); - } else - ret = acpi_ec_setup(ec, true); + } + + ret = acpi_ec_setup(ec, true); if (ret) goto err_query; + if (ec == boot_ec) + acpi_handle_info(ec->handle, + "Boot %s EC used to handle transactions and events\n", + is_ecdt ? "ECDT" : "DSDT"); + device->driver_data = ec; ret = !!request_region(ec->data_addr, 1, "EC data"); @@ -1766,9 +1728,14 @@ void __init acpi_ec_dsdt_probe(void) * At this point, the GPE is not fully initialized, so do not to * handle the events. */ - ret = acpi_config_boot_ec(ec, ec->handle, false, false); + ret = acpi_ec_setup(ec, false); if (ret) acpi_ec_free(ec); + + boot_ec = ec; + + acpi_handle_info(ec->handle, + "Boot DSDT EC used to handle transactions\n"); } /* @@ -1905,14 +1872,20 @@ void __init acpi_ec_ecdt_probe(void) ec->data_addr = ecdt_ptr->data.address; } ec->gpe = ecdt_ptr->gpe; + ec->handle = ACPI_ROOT_OBJECT; /* * At this point, the namespace is not initialized, so do not find * the namespace objects, or handle the events. */ - ret = acpi_config_boot_ec(ec, ACPI_ROOT_OBJECT, false, true); + ret = acpi_ec_setup(ec, false); if (ret) acpi_ec_free(ec); + + boot_ec = ec; + boot_ec_is_ecdt = true; + + pr_info("Boot ECDT EC used to handle transactions\n"); } #ifdef CONFIG_PM_SLEEP ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() 2019-01-22 11:55 [PATCH 0/2] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-01-22 11:56 ` [PATCH 1/2] ACPI: EC: Untangle " Rafael J. Wysocki @ 2019-01-22 11:57 ` Rafael J. Wysocki 2019-01-22 12:03 ` Rafael J. Wysocki 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2 siblings, 1 reply; 12+ messages in thread From: Rafael J. Wysocki @ 2019-01-22 11:57 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Consolidate boot EC checks in acpi_ec_add(), put the acpi_is_boot_ec() checks directly into it and drop the latter. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1558,16 +1558,6 @@ static bool acpi_ec_ecdt_get_handle(acpi return true; } -static bool acpi_is_boot_ec(struct acpi_ec *ec) -{ - if (!boot_ec) - return false; - if (ec->command_addr == boot_ec->command_addr && - ec->data_addr == boot_ec->data_addr) - return true; - return false; -} - static int acpi_ec_add(struct acpi_device *device) { struct acpi_ec *ec = NULL; @@ -1579,22 +1569,22 @@ static int acpi_ec_add(struct acpi_devic strcpy(acpi_device_class(device), ACPI_EC_CLASS); if (!strcmp(acpi_device_hid(device), ACPI_ECDT_HID)) { - is_ecdt = true; ec = boot_ec; + boot_ec_is_ecdt = true; + is_ecdt = true; } else { ec = acpi_ec_alloc(); if (!ec) return -ENOMEM; + status = ec_parse_device(device->handle, 0, ec, NULL); if (status != AE_CTRL_TERMINATE) { ret = -EINVAL; goto err_alloc; } - } - if (acpi_is_boot_ec(ec)) { - boot_ec_is_ecdt = is_ecdt; - if (!is_ecdt) { + if (boot_ec && ec->command_addr == boot_ec->command_addr && + ec->data_addr == boot_ec->data_addr) { /* * Trust PNP0C09 namespace location rather than * ECDT ID. But trust ECDT GPE rather than _GPE ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() 2019-01-22 11:57 ` [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki @ 2019-01-22 12:03 ` Rafael J. Wysocki 0 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-01-22 12:03 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML On Tue, Jan 22, 2019 at 1:01 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Consolidate boot EC checks in acpi_ec_add(), put the acpi_is_boot_ec() > checks directly into it and drop the latter. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> This is the second patch in the series, the numbering in the subject is incorrect. Sorry about that. > --- > drivers/acpi/ec.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > > Index: linux-pm/drivers/acpi/ec.c > =================================================================== > --- linux-pm.orig/drivers/acpi/ec.c > +++ linux-pm/drivers/acpi/ec.c > @@ -1558,16 +1558,6 @@ static bool acpi_ec_ecdt_get_handle(acpi > return true; > } > > -static bool acpi_is_boot_ec(struct acpi_ec *ec) > -{ > - if (!boot_ec) > - return false; > - if (ec->command_addr == boot_ec->command_addr && > - ec->data_addr == boot_ec->data_addr) > - return true; > - return false; > -} > - > static int acpi_ec_add(struct acpi_device *device) > { > struct acpi_ec *ec = NULL; > @@ -1579,22 +1569,22 @@ static int acpi_ec_add(struct acpi_devic > strcpy(acpi_device_class(device), ACPI_EC_CLASS); > > if (!strcmp(acpi_device_hid(device), ACPI_ECDT_HID)) { > - is_ecdt = true; > ec = boot_ec; > + boot_ec_is_ecdt = true; > + is_ecdt = true; > } else { > ec = acpi_ec_alloc(); > if (!ec) > return -ENOMEM; > + > status = ec_parse_device(device->handle, 0, ec, NULL); > if (status != AE_CTRL_TERMINATE) { > ret = -EINVAL; > goto err_alloc; > } > - } > > - if (acpi_is_boot_ec(ec)) { > - boot_ec_is_ecdt = is_ecdt; > - if (!is_ecdt) { > + if (boot_ec && ec->command_addr == boot_ec->command_addr && > + ec->data_addr == boot_ec->data_addr) { > /* > * Trust PNP0C09 namespace location rather than > * ECDT ID. But trust ECDT GPE rather than _GPE > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup 2019-01-22 11:55 [PATCH 0/2] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-01-22 11:56 ` [PATCH 1/2] ACPI: EC: Untangle " Rafael J. Wysocki 2019-01-22 11:57 ` [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki @ 2019-02-01 9:56 ` Rafael J. Wysocki 2019-02-01 9:57 ` [PATCH v2 1/5] ACPI: EC: Declare boot_ec as static Rafael J. Wysocki ` (4 more replies) 2 siblings, 5 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 9:56 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML Hi All, On Tuesday, January 22, 2019 12:55:18 PM CET Rafael J. Wysocki wrote: > Hi All, > > The setup of the boot EC is unnecessarily tangled now, so untangle it to > make the code flow in there easier to follow. > > The only intentional functional impact of this series should be changes > in messages printed to the kernel log. > > The patches are on top of https://patchwork.kernel.org/patch/10773757/ This v2 is here, because the first patch in the previous version contained too bugs that were overlooked, because it attempted to do too much in one go. So this time use smaller patches and add one to make boot_ec static (which it should be). Thanks, Rafael ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/5] ACPI: EC: Declare boot_ec as static 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki @ 2019-02-01 9:57 ` Rafael J. Wysocki 2019-02-01 9:58 ` [PATCH v2 2/5] ACPI: EC: Make acpi_ec_ecdt_probe() more straightforward Rafael J. Wysocki ` (3 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 9:57 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The boot_ec variable is not used outside of the file it is defined in, so declare it as static. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -186,8 +186,10 @@ static void advance_transaction(struct a static void acpi_ec_event_handler(struct work_struct *work); static void acpi_ec_event_processor(struct work_struct *work); -struct acpi_ec *boot_ec, *first_ec; +struct acpi_ec *first_ec; EXPORT_SYMBOL(first_ec); + +static struct acpi_ec *boot_ec; static bool boot_ec_is_ecdt = false; static struct workqueue_struct *ec_query_wq; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] ACPI: EC: Make acpi_ec_ecdt_probe() more straightforward 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-02-01 9:57 ` [PATCH v2 1/5] ACPI: EC: Declare boot_ec as static Rafael J. Wysocki @ 2019-02-01 9:58 ` Rafael J. Wysocki 2019-02-01 9:59 ` [PATCH v2 3/5] ACPI: EC: Make acpi_ec_dsdt_probe() " Rafael J. Wysocki ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 9:58 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Since acpi_ec_ecdt_probe() is called when boot_ec is not set, it doesn't neeed to take the other possibility into account. Accordingly, it only needs to set the handle field in the ec object to ACPI_ROOT_OBJECT, call acpi_ec_setup() and (if that is successful) set boot_ec to ec and boot_ec_is_ecdt to 'true'. Make it do so directly, without calling acpi_config_boot_ec(), and avoid some pointless checks in the latter. No intentional functional impact except for a changed message. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1907,14 +1907,22 @@ void __init acpi_ec_ecdt_probe(void) ec->data_addr = ecdt_ptr->data.address; } ec->gpe = ecdt_ptr->gpe; + ec->handle = ACPI_ROOT_OBJECT; /* * At this point, the namespace is not initialized, so do not find * the namespace objects, or handle the events. */ - ret = acpi_config_boot_ec(ec, ACPI_ROOT_OBJECT, false, true); - if (ret) + ret = acpi_ec_setup(ec, false); + if (ret) { acpi_ec_free(ec); + return; + } + + boot_ec = ec; + boot_ec_is_ecdt = true; + + pr_info("Boot ECDT EC used to handle transactions\n"); } #ifdef CONFIG_PM_SLEEP ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] ACPI: EC: Make acpi_ec_dsdt_probe() more straightforward 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-02-01 9:57 ` [PATCH v2 1/5] ACPI: EC: Declare boot_ec as static Rafael J. Wysocki 2019-02-01 9:58 ` [PATCH v2 2/5] ACPI: EC: Make acpi_ec_ecdt_probe() more straightforward Rafael J. Wysocki @ 2019-02-01 9:59 ` Rafael J. Wysocki 2019-02-01 10:01 ` [PATCH v2 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() Rafael J. Wysocki 2019-02-01 10:03 ` [PATCH v2 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki 4 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 9:59 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Since acpi_ec_dsdt_probe() returns early if boot_ec is set, it is always unset when that function calls acpi_config_boot_ec() (passing ec->handle as the handle argument to it). Thus it is not really useful to call acpi_config_boot_ec() at that point. It is sufficient to call acpi_ec_setup() directly and (if that is successful) set boot_ec, so make acpi_ec_dsdt_probe() do that and avoid some pointless checks in acpi_config_boot_ec(). No intentional functional impact except for a changed message. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1768,9 +1768,16 @@ void __init acpi_ec_dsdt_probe(void) * At this point, the GPE is not fully initialized, so do not to * handle the events. */ - ret = acpi_config_boot_ec(ec, ec->handle, false, false); - if (ret) + ret = acpi_ec_setup(ec, false); + if (ret) { acpi_ec_free(ec); + return; + } + + boot_ec = ec; + + acpi_handle_info(ec->handle, + "Boot DSDT EC used to handle transactions\n"); } /* ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki ` (2 preceding siblings ...) 2019-02-01 9:59 ` [PATCH v2 3/5] ACPI: EC: Make acpi_ec_dsdt_probe() " Rafael J. Wysocki @ 2019-02-01 10:01 ` Rafael J. Wysocki 2019-02-01 11:47 ` [PATCH v3 " Rafael J. Wysocki 2019-02-01 10:03 ` [PATCH v2 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki 4 siblings, 1 reply; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 10:01 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Notice that acpi_ec_add() calls acpi_config_boot_ec() when it finds that the device object passed to it represents a "boot" EC, but in that case the ec pointer passed to acpi_config_boot_ec() is guaranteed to be equal to boot_ec and ec->handle is passed as the handle argument to it, so acpi_config_boot_ec() really only calls acpi_ec_setup() and prints a message. Avoid the pointless checks in acpi_config_boot_ec() by calling acpi_ec_setup() directly and print the message separately. With the above changes in place, there are no users of acpi_config_boot_ec(), so drop it. No intentional functional impact except for a changed message. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/ec.c | 54 ++++++++---------------------------------------------- 1 file changed, 8 insertions(+), 46 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1541,49 +1541,6 @@ static int acpi_ec_setup(struct acpi_ec return ret; } -static int acpi_config_boot_ec(struct acpi_ec *ec, acpi_handle handle, - bool handle_events, bool is_ecdt) -{ - int ret; - - /* - * Changing the ACPI handle results in a re-configuration of the - * boot EC. And if it happens after the namespace initialization, - * it causes _REG evaluations. - */ - if (boot_ec && boot_ec->handle != handle) - ec_remove_handlers(boot_ec); - - /* Unset old boot EC */ - if (boot_ec != ec) - acpi_ec_free(boot_ec); - - /* - * ECDT device creation is split into acpi_ec_ecdt_probe() and - * acpi_ec_ecdt_start(). This function takes care of completing the - * ECDT parsing logic as the handle update should be performed - * between the installation/uninstallation of the handlers. - */ - if (ec->handle != handle) - ec->handle = handle; - - ret = acpi_ec_setup(ec, handle_events); - if (ret) - return ret; - - /* Set new boot EC */ - if (!boot_ec) { - boot_ec = ec; - boot_ec_is_ecdt = is_ecdt; - } - - acpi_handle_info(boot_ec->handle, - "Used as boot %s EC to handle transactions%s\n", - is_ecdt ? "ECDT" : "DSDT", - handle_events ? " and events" : ""); - return ret; -} - static bool acpi_ec_ecdt_get_handle(acpi_handle *phandle) { struct acpi_table_ecdt *ecdt_ptr; @@ -1651,12 +1608,17 @@ static int acpi_ec_add(struct acpi_devic acpi_ec_free(ec); ec = boot_ec; } - ret = acpi_config_boot_ec(ec, ec->handle, true, is_ecdt); - } else - ret = acpi_ec_setup(ec, true); + } + + ret = acpi_ec_setup(ec, true); if (ret) goto err_query; + if (boot_ec) + acpi_handle_info(boot_ec->handle, + "Boot %s EC used to handle transactions and events\n", + is_ecdt ? "ECDT" : "DSDT"); + device->driver_data = ec; ret = !!request_region(ec->data_addr, 1, "EC data"); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() 2019-02-01 10:01 ` [PATCH v2 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() Rafael J. Wysocki @ 2019-02-01 11:47 ` Rafael J. Wysocki 0 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 11:47 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux ACPI, Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Notice that acpi_ec_add() calls acpi_config_boot_ec() when it finds that the device object passed to it represents a "boot" EC, but in that case the ec pointer passed to acpi_config_boot_ec() is guaranteed to be equal to boot_ec and ec->handle is passed as the handle argument to it, so acpi_config_boot_ec() really only calls acpi_ec_setup() and prints a message. Avoid the pointless checks in acpi_config_boot_ec() by calling acpi_ec_setup() directly and print the message separately. With the above changes in place, there are no users of acpi_config_boot_ec(), so drop it. No intentional functional impact except for a changed message. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- v2 -> v3: * In acpi_ec_add() compare ec to boot_ec before printing the message instead of just checking if boot_ec is not NULL. --- drivers/acpi/ec.c | 54 ++++++++---------------------------------------------- 1 file changed, 8 insertions(+), 46 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1541,49 +1541,6 @@ static int acpi_ec_setup(struct acpi_ec return ret; } -static int acpi_config_boot_ec(struct acpi_ec *ec, acpi_handle handle, - bool handle_events, bool is_ecdt) -{ - int ret; - - /* - * Changing the ACPI handle results in a re-configuration of the - * boot EC. And if it happens after the namespace initialization, - * it causes _REG evaluations. - */ - if (boot_ec && boot_ec->handle != handle) - ec_remove_handlers(boot_ec); - - /* Unset old boot EC */ - if (boot_ec != ec) - acpi_ec_free(boot_ec); - - /* - * ECDT device creation is split into acpi_ec_ecdt_probe() and - * acpi_ec_ecdt_start(). This function takes care of completing the - * ECDT parsing logic as the handle update should be performed - * between the installation/uninstallation of the handlers. - */ - if (ec->handle != handle) - ec->handle = handle; - - ret = acpi_ec_setup(ec, handle_events); - if (ret) - return ret; - - /* Set new boot EC */ - if (!boot_ec) { - boot_ec = ec; - boot_ec_is_ecdt = is_ecdt; - } - - acpi_handle_info(boot_ec->handle, - "Used as boot %s EC to handle transactions%s\n", - is_ecdt ? "ECDT" : "DSDT", - handle_events ? " and events" : ""); - return ret; -} - static bool acpi_ec_ecdt_get_handle(acpi_handle *phandle) { struct acpi_table_ecdt *ecdt_ptr; @@ -1651,12 +1608,17 @@ static int acpi_ec_add(struct acpi_devic acpi_ec_free(ec); ec = boot_ec; } - ret = acpi_config_boot_ec(ec, ec->handle, true, is_ecdt); - } else - ret = acpi_ec_setup(ec, true); + } + + ret = acpi_ec_setup(ec, true); if (ret) goto err_query; + if (ec == boot_ec) + acpi_handle_info(boot_ec->handle, + "Boot %s EC used to handle transactions and events\n", + is_ecdt ? "ECDT" : "DSDT"); + device->driver_data = ec; ret = !!request_region(ec->data_addr, 1, "EC data"); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki ` (3 preceding siblings ...) 2019-02-01 10:01 ` [PATCH v2 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() Rafael J. Wysocki @ 2019-02-01 10:03 ` Rafael J. Wysocki 2019-02-01 11:50 ` [PATCH v3 " Rafael J. Wysocki 4 siblings, 1 reply; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 10:03 UTC (permalink / raw) To: Linux ACPI; +Cc: Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Consolidate boot EC checks in acpi_ec_add(), put the acpi_is_boot_ec() checks directly into it and drop the latter. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- This patch hans't changed since the previous posting. --- drivers/acpi/ec.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1560,16 +1560,6 @@ static bool acpi_ec_ecdt_get_handle(acpi return true; } -static bool acpi_is_boot_ec(struct acpi_ec *ec) -{ - if (!boot_ec) - return false; - if (ec->command_addr == boot_ec->command_addr && - ec->data_addr == boot_ec->data_addr) - return true; - return false; -} - static int acpi_ec_add(struct acpi_device *device) { struct acpi_ec *ec = NULL; @@ -1581,22 +1571,22 @@ static int acpi_ec_add(struct acpi_devic strcpy(acpi_device_class(device), ACPI_EC_CLASS); if (!strcmp(acpi_device_hid(device), ACPI_ECDT_HID)) { - is_ecdt = true; ec = boot_ec; + boot_ec_is_ecdt = true; + is_ecdt = true; } else { ec = acpi_ec_alloc(); if (!ec) return -ENOMEM; + status = ec_parse_device(device->handle, 0, ec, NULL); if (status != AE_CTRL_TERMINATE) { ret = -EINVAL; goto err_alloc; } - } - if (acpi_is_boot_ec(ec)) { - boot_ec_is_ecdt = is_ecdt; - if (!is_ecdt) { + if (boot_ec && ec->command_addr == boot_ec->command_addr && + ec->data_addr == boot_ec->data_addr) { /* * Trust PNP0C09 namespace location rather than * ECDT ID. But trust ECDT GPE rather than _GPE ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() 2019-02-01 10:03 ` [PATCH v2 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki @ 2019-02-01 11:50 ` Rafael J. Wysocki 0 siblings, 0 replies; 12+ messages in thread From: Rafael J. Wysocki @ 2019-02-01 11:50 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux ACPI, Erik Schmauss, Zhang Rui, LKML From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Subject: [PATCH] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Consolidate boot EC checks in acpi_ec_add(), put the acpi_is_boot_ec() checks directly into it and drop the latter. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- v2 -> v3: * Drop the is_ecdt local var from acpi_ec_add() and update boot_ec_is_ecdt directly (adding a missing boot_ec_is_ecdt update). * Add dep_update local var to acpi_ec_add() to indicate when dependent devices shouls be re-probed. --- drivers/acpi/ec.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) Index: linux-pm/drivers/acpi/ec.c =================================================================== --- linux-pm.orig/drivers/acpi/ec.c +++ linux-pm/drivers/acpi/ec.c @@ -1560,43 +1560,34 @@ static bool acpi_ec_ecdt_get_handle(acpi return true; } -static bool acpi_is_boot_ec(struct acpi_ec *ec) -{ - if (!boot_ec) - return false; - if (ec->command_addr == boot_ec->command_addr && - ec->data_addr == boot_ec->data_addr) - return true; - return false; -} - static int acpi_ec_add(struct acpi_device *device) { struct acpi_ec *ec = NULL; - int ret; - bool is_ecdt = false; + bool dep_update = true; acpi_status status; + int ret; strcpy(acpi_device_name(device), ACPI_EC_DEVICE_NAME); strcpy(acpi_device_class(device), ACPI_EC_CLASS); if (!strcmp(acpi_device_hid(device), ACPI_ECDT_HID)) { - is_ecdt = true; + boot_ec_is_ecdt = true; ec = boot_ec; + dep_update = false; } else { ec = acpi_ec_alloc(); if (!ec) return -ENOMEM; + status = ec_parse_device(device->handle, 0, ec, NULL); if (status != AE_CTRL_TERMINATE) { ret = -EINVAL; goto err_alloc; } - } - if (acpi_is_boot_ec(ec)) { - boot_ec_is_ecdt = is_ecdt; - if (!is_ecdt) { + if (boot_ec && ec->command_addr == boot_ec->command_addr && + ec->data_addr == boot_ec->data_addr) { + boot_ec_is_ecdt = false; /* * Trust PNP0C09 namespace location rather than * ECDT ID. But trust ECDT GPE rather than _GPE @@ -1617,7 +1608,7 @@ static int acpi_ec_add(struct acpi_devic if (ec == boot_ec) acpi_handle_info(boot_ec->handle, "Boot %s EC used to handle transactions and events\n", - is_ecdt ? "ECDT" : "DSDT"); + boot_ec_is_ecdt ? "ECDT" : "DSDT"); device->driver_data = ec; @@ -1626,7 +1617,7 @@ static int acpi_ec_add(struct acpi_devic ret = !!request_region(ec->command_addr, 1, "EC cmd"); WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr); - if (!is_ecdt) { + if (dep_update) { /* Reprobe devices depending on the EC */ acpi_walk_dep_device_list(ec->handle); } ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-01 11:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-22 11:55 [PATCH 0/2] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-01-22 11:56 ` [PATCH 1/2] ACPI: EC: Untangle " Rafael J. Wysocki 2019-01-22 11:57 ` [PATCH 1/2] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki 2019-01-22 12:03 ` Rafael J. Wysocki 2019-02-01 9:56 ` [PATCH v2 0/5] ACPI: EC: Simplify boot EC setup Rafael J. Wysocki 2019-02-01 9:57 ` [PATCH v2 1/5] ACPI: EC: Declare boot_ec as static Rafael J. Wysocki 2019-02-01 9:58 ` [PATCH v2 2/5] ACPI: EC: Make acpi_ec_ecdt_probe() more straightforward Rafael J. Wysocki 2019-02-01 9:59 ` [PATCH v2 3/5] ACPI: EC: Make acpi_ec_dsdt_probe() " Rafael J. Wysocki 2019-02-01 10:01 ` [PATCH v2 4/5] ACPI: EC: Eliminate acpi_config_boot_ec() Rafael J. Wysocki 2019-02-01 11:47 ` [PATCH v3 " Rafael J. Wysocki 2019-02-01 10:03 ` [PATCH v2 5/5] ACPI: EC: Simplify boot EC checks in acpi_ec_add() Rafael J. Wysocki 2019-02-01 11:50 ` [PATCH v3 " Rafael J. Wysocki
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 NNTP newsgroup(s).