[PATCH 0/2] gpio / ACPI: Two minor cleanups related to ACPI_HANDLE()

[PATCH 0/2] gpio / ACPI: Two minor cleanups related to ACPI_HANDLE()

[Rafael J. Wysocki]

Hi,

As per the subject, avoid evaluating ACPI_HANDLE() if we can as that one is
rather costly.

Kind regards,
Rafael

[PATCH 1/2] gpio / ACPI: Avoid unnecessary checks in __gpiod_get_index()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If dev is NULL in __gpiod_get_index() and both ACPI and OF are
enabled, it will be checked twice before the code decides to give
up with DT/ACPI lookup, so avoid that.

Also use the observation that ACPI_COMPANION() is much more efficient
than ACPI_HANDLE(), because the latter uses the former and carries out
a check and a pointer dereference on top of it, so replace the
ACPI_HANDLE() check with an ACPI_COMPANION() one which does not
require the additional IS_ENABLED(CONFIG_ACPI) check too.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/gpio/gpiolib.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/gpio/gpiolib.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.c
+++ linux-pm/drivers/gpio/gpiolib.c
@@ -1865,13 +1865,15 @@ struct gpio_desc *__must_check __gpiod_g
 
 	dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
 
-	/* Using device tree? */
-	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node) {
-		dev_dbg(dev, "using device tree for GPIO lookup\n");
-		desc = of_find_gpio(dev, con_id, idx, &lookupflags);
-	} else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev)) {
-		dev_dbg(dev, "using ACPI for GPIO lookup\n");
-		desc = acpi_find_gpio(dev, con_id, idx, &lookupflags);
+	if (dev) {
+		/* Using device tree? */
+		if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+			dev_dbg(dev, "using device tree for GPIO lookup\n");
+			desc = of_find_gpio(dev, con_id, idx, &lookupflags);
+		} else if (ACPI_COMPANION(dev)) {
+			dev_dbg(dev, "using ACPI for GPIO lookup\n");
+			desc = acpi_find_gpio(dev, con_id, idx, &lookupflags);
+		}
 	}
 
 	/*

Re: [PATCH 1/2] gpio / ACPI: Avoid unnecessary checks in __gpiod_get_index()

[Hanjun Guo]

On 2015/3/11 6:08, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If dev is NULL in __gpiod_get_index() and both ACPI and OF are
> enabled, it will be checked twice before the code decides to give
> up with DT/ACPI lookup, so avoid that.
>
> Also use the observation that ACPI_COMPANION() is much more efficient
> than ACPI_HANDLE(), because the latter uses the former and carries out
> a check and a pointer dereference on top of it, so replace the
> ACPI_HANDLE() check with an ACPI_COMPANION() one which does not
> require the additional IS_ENABLED(CONFIG_ACPI) check too.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Quite straight forward to me, for both two patches,

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

> ---
>  drivers/gpio/gpiolib.c |   16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> Index: linux-pm/drivers/gpio/gpiolib.c
> ===================================================================
> --- linux-pm.orig/drivers/gpio/gpiolib.c
> +++ linux-pm/drivers/gpio/gpiolib.c
> @@ -1865,13 +1865,15 @@ struct gpio_desc *__must_check __gpiod_g
>  
>  	dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
>  
> -	/* Using device tree? */
> -	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node) {
> -		dev_dbg(dev, "using device tree for GPIO lookup\n");
> -		desc = of_find_gpio(dev, con_id, idx, &lookupflags);
> -	} else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev)) {
> -		dev_dbg(dev, "using ACPI for GPIO lookup\n");
> -		desc = acpi_find_gpio(dev, con_id, idx, &lookupflags);
> +	if (dev) {
> +		/* Using device tree? */
> +		if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +			dev_dbg(dev, "using device tree for GPIO lookup\n");
> +			desc = of_find_gpio(dev, con_id, idx, &lookupflags);
> +		} else if (ACPI_COMPANION(dev)) {
> +			dev_dbg(dev, "using ACPI for GPIO lookup\n");
> +			desc = acpi_find_gpio(dev, con_id, idx, &lookupflags);
> +		}
>  	}
>  
>  	/*
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> .
>

Re: [PATCH 1/2] gpio / ACPI: Avoid unnecessary checks in __gpiod_get_index()

[Mika Westerberg]

On Tue, Mar 10, 2015 at 11:08:57PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If dev is NULL in __gpiod_get_index() and both ACPI and OF are
> enabled, it will be checked twice before the code decides to give
> up with DT/ACPI lookup, so avoid that.
> 
> Also use the observation that ACPI_COMPANION() is much more efficient
> than ACPI_HANDLE(), because the latter uses the former and carries out
> a check and a pointer dereference on top of it, so replace the
> ACPI_HANDLE() check with an ACPI_COMPANION() one which does not
> require the additional IS_ENABLED(CONFIG_ACPI) check too.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH 1/2] gpio / ACPI: Avoid unnecessary checks in __gpiod_get_index()

[Linus Walleij]

On Tue, Mar 10, 2015 at 11:08 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If dev is NULL in __gpiod_get_index() and both ACPI and OF are
> enabled, it will be checked twice before the code decides to give
> up with DT/ACPI lookup, so avoid that.
>
> Also use the observation that ACPI_COMPANION() is much more efficient
> than ACPI_HANDLE(), because the latter uses the former and carries out
> a check and a pointer dereference on top of it, so replace the
> ACPI_HANDLE() check with an ACPI_COMPANION() one which does not
> require the additional IS_ENABLED(CONFIG_ACPI) check too.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Patch applied with review and ACK tags.

Yours,
Linus Walleij

[PATCH 2/2] gpio / ACPI: Use local variable instead of ACPI_HANDLE()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In acpi_gpiochip_request_interrupts() the handle local
variable already contains the value that we want to pass
to acpi_walk_resources(), so it is better to use that
variable instead of evaluating ACPI_HANDLE() once more
for the same device.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/gpio/gpiolib-acpi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/gpio/gpiolib-acpi.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib-acpi.c
+++ linux-pm/drivers/gpio/gpiolib-acpi.c
@@ -300,7 +300,7 @@ void acpi_gpiochip_request_interrupts(st
 		return;
 
 	INIT_LIST_HEAD(&acpi_gpio->events);
-	acpi_walk_resources(ACPI_HANDLE(chip->dev), "_AEI",
+	acpi_walk_resources(handle, "_AEI",
 			    acpi_gpiochip_request_interrupt, acpi_gpio);
 }
 

Re: [PATCH 2/2] gpio / ACPI: Use local variable instead of ACPI_HANDLE()

[Mika Westerberg]

On Tue, Mar 10, 2015 at 11:10:01PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In acpi_gpiochip_request_interrupts() the handle local
> variable already contains the value that we want to pass
> to acpi_walk_resources(), so it is better to use that
> variable instead of evaluating ACPI_HANDLE() once more
> for the same device.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH 2/2] gpio / ACPI: Use local variable instead of ACPI_HANDLE()

[Linus Walleij]

On Tue, Mar 10, 2015 at 11:10 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> In acpi_gpiochip_request_interrupts() the handle local
> variable already contains the value that we want to pass
> to acpi_walk_resources(), so it is better to use that
> variable instead of evaluating ACPI_HANDLE() once more
> for the same device.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Patch applied with Mika's ACK.

Yours,
Linus Walleij

Re: [PATCH 0/2] gpio / ACPI: Two minor cleanups related to ACPI_HANDLE()

[Rafael J. Wysocki]

On Tuesday, March 10, 2015 11:07:36 PM Rafael J. Wysocki wrote:
> Hi,
> 
> As per the subject, avoid evaluating ACPI_HANDLE() if we can as that one is
> rather costly.

Linus, Alexandre, any objections to this series?

If not, would there be any problem if I took these to my tree?  I'll probably
have material on top of them.

Rafael

Re: [PATCH 0/2] gpio / ACPI: Two minor cleanups related to ACPI_HANDLE()

[Alexandre Courbot]

On Wed, Mar 18, 2015 at 10:37 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, March 10, 2015 11:07:36 PM Rafael J. Wysocki wrote:
>> Hi,
>>
>> As per the subject, avoid evaluating ACPI_HANDLE() if we can as that one is
>> rather costly.
>
> Linus, Alexandre, any objections to this series?
>
> If not, would there be any problem if I took these to my tree?  I'll probably
> have material on top of them.

Sorry for the delay. Looks like Linus just took the patches in his
tree, I hope that's ok?

Re: [PATCH 0/2] gpio / ACPI: Two minor cleanups related to ACPI_HANDLE()

[Rafael J. Wysocki]

On Wednesday, March 18, 2015 10:47:30 AM Alexandre Courbot wrote:
> On Wed, Mar 18, 2015 at 10:37 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, March 10, 2015 11:07:36 PM Rafael J. Wysocki wrote:
> >> Hi,
> >>
> >> As per the subject, avoid evaluating ACPI_HANDLE() if we can as that one is
> >> rather costly.
> >
> > Linus, Alexandre, any objections to this series?
> >
> > If not, would there be any problem if I took these to my tree?  I'll probably
> > have material on top of them.
> 
> Sorry for the delay. Looks like Linus just took the patches in his
> tree, I hope that's ok?

Yes, that's fine, thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.