linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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 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 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 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).