[PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Hans de Goede]

Hi All,

While trying to get a driver working for a device where I want to
only use the irq in the acpi resources with index 1, and we do not
have a driver for the gpiochip with the irq with index 0 yet,
I hit this problem that acpi_dev_gpio_irq_get will bail with
-EPROBE_DEFER because it cannot get a gpio_desc for index 0,
this really should not be a problem when asking for index 1
and this series fixes this.

Note these patches apply on top of the gpiolib acpi work
Andy Shevchenko has been doing and as such may not apply
cleanly on top of current master / next.

Regards,

Hans

[PATCH 1/2] gpio: acpi: acpi_gpio_resource_lookup: return info even if there is no desc

[Hans de Goede]

Make acpi_gpio_resource_lookup fill acpi_gpio_info even if there is
no gpio_desc for the gpio and add a found flag to lookup which is
used to indicate that valid info was found.

This is a preparation patch for making acpi_dev_gpio_irq_get actually
skip interrupts which do not match the requested index, rather then
bailing if it cannot retreive a gpio_desc for them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpio/gpiolib-acpi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 3fe63d8..86fabdd 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -475,6 +475,7 @@ struct acpi_gpio_lookup {
 	int index;
 	int pin_index;
 	bool active_low;
+	bool found;
 	struct acpi_device *adev;
 	struct gpio_desc *desc;
 	int n;
@@ -513,6 +514,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 		} else {
 			lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
 		}
+		lookup->found = true;
 	}
 
 	return 1;
@@ -534,15 +536,13 @@ static int acpi_gpio_resource_lookup(struct acpi_gpio_lookup *lookup,
 
 	acpi_dev_free_resource_list(&res_list);
 
-	if (!lookup->desc)
-		return -ENOENT;
-
 	if (info) {
 		*info = lookup->info;
 		if (lookup->active_low)
 			info->polarity = lookup->active_low;
 	}
-	return 0;
+
+	return lookup->desc ? 0 : -ENOENT;
 }
 
 static int acpi_gpio_property_lookup(struct fwnode_handle *fwnode,
-- 
2.9.3

[PATCH 2/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Hans de Goede]

When acpi_dev_gpio_irq_get gets called with an index of say 2, it
should not care if the gpioint matching index 0 or 1 has a valid
descriptor.

Before this commit acpi_dev_gpio_irq_get would exit on the first gpioint
with an invalid descriptor it would encounter, this commit modifies it to
only care if the gpioint with the requested index has a valid descriptor.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpio/gpiolib-acpi.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 86fabdd..11bd1a6 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -746,21 +746,27 @@ struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
 int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
 	unsigned int irq_flags;
-	int idx, i;
+	int idx, i, irq, ret;
 
 	for (i = 0, idx = 0; idx <= index; i++) {
 		struct acpi_gpio_info info;
-		struct gpio_desc *desc;
+		struct acpi_gpio_lookup lookup;
 
-		desc = acpi_get_gpiod_by_index(adev, NULL, i, &info);
-		if (IS_ERR(desc))
-			return PTR_ERR(desc);
+		memset(&lookup, 0, sizeof(lookup));
+		lookup.index = index;
+		lookup.adev = adev;
+		ret = acpi_gpio_resource_lookup(&lookup, &info);
+		if (!lookup.found)
+			return ret;
 
 		if (info.gpioint && idx++ == index) {
-			int irq = gpiod_to_irq(desc);
+			struct gpio_desc *desc = lookup.desc;
 			char label[32];
-			int ret;
 
+			if (!desc)
+				return ret;
+
+			irq = gpiod_to_irq(desc);
 			if (irq < 0)
 				return irq;
 
-- 
2.9.3

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Andy Shevchenko]

On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:
> Hi All,
> 
> While trying to get a driver working for a device where I want to
> only use the irq in the acpi resources with index 1, and we do not
> have a driver for the gpiochip with the irq with index 0 yet,
> I hit this problem that acpi_dev_gpio_irq_get will bail with
> -EPROBE_DEFER because it cannot get a gpio_desc for index 0,
> this really should not be a problem when asking for index 1
> and this series fixes this.
> 
> Note these patches apply on top of the gpiolib acpi work
> Andy Shevchenko has been doing and as such may not apply
> cleanly on top of current master / next.

I will incorporate slightly different version of this (due to changes
happened) into my branch. Please verify if everything is okay.

P.S. I'm going to push an updated branch within couple of hours.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

Hi,

On 13-03-17 18:23, Andy Shevchenko wrote:
> On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:
>> Hi All,
>>
>> While trying to get a driver working for a device where I want to
>> only use the irq in the acpi resources with index 1, and we do not
>> have a driver for the gpiochip with the irq with index 0 yet,
>> I hit this problem that acpi_dev_gpio_irq_get will bail with
>> -EPROBE_DEFER because it cannot get a gpio_desc for index 0,
>> this really should not be a problem when asking for index 1
>> and this series fixes this.
>>
>> Note these patches apply on top of the gpiolib acpi work
>> Andy Shevchenko has been doing and as such may not apply
>> cleanly on top of current master / next.
>
> I will incorporate slightly different version of this (due to changes
> happened) into my branch. Please verify if everything is okay.

I just noticed that my patch was botched up a bit, it is
taking a more complex approach then needed and it contained a bug
(it did lookup->index = index, which should be lookup->index = i).

I've just finished writing a v2 (sorry). I've attached the v2 here,
note this is against an unmodified v4.11-rc2 rather then your branch
as I needed a clean base to debug some problems. But it should be
easy to adapt to your branch I think.

Note this can go upstream either way (through your branch or
directly since it is based on a clean v4.11-rc2 now) but it is
probably easier to take it upstream through your branch to
avoid conflicts.

Regards,

Hans

[-- Attachment #2: 0001-gpio-acpi-acpi_dev_gpio_irq_get-ignore-EPROBE_DEFER-.patch --]
[-- Type: text/x-patch, Size: 1821 bytes --]

>From 93c2c9334a24b845c5e816299d61b92f93601b07 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 13 Mar 2017 18:43:30 +0100
Subject: [PATCH v2] gpio: acpi: acpi_dev_gpio_irq_get: ignore -EPROBE_DEFER
 for unselected gpioints

When acpi_dev_gpio_irq_get gets called with an index of say 2, it should
not care if acpi_get_gpiod for index 0 or 1 returns -EPROBE_DEFER.

This allows drivers which request a gpioint with index > 0 to function
if there is no gpiochip driver (loaded) for gpioints with a lower index.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Completely new patch replacing my previous somewhat broken attempt
---
 drivers/gpio/gpiolib-acpi.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 9b37a36..fc6f34a 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -651,7 +651,7 @@ struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
  */
 int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
-	int idx, i;
+	int idx, i, irq;
 	unsigned int irq_flags;
 	int ret = -ENOENT;
 
@@ -660,13 +660,18 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 		struct gpio_desc *desc;
 
 		desc = acpi_get_gpiod_by_index(adev, NULL, i, &info);
-		if (IS_ERR(desc)) {
+		/* Ignore -EPROBE_DEFER, it only matters if idx matches */
+		if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) {
 			ret = PTR_ERR(desc);
 			break;
 		}
 		if (info.gpioint && idx++ == index) {
-			int irq = gpiod_to_irq(desc);
+			if (IS_ERR(desc)) {
+				ret = PTR_ERR(desc);
+				break;
+			}
 
+			irq = gpiod_to_irq(desc);
 			if (irq < 0)
 				return irq;
 
-- 
2.9.3

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Andy Shevchenko]

On Mon, 2017-03-13 at 18:56 +0100, Hans de Goede wrote:
> On 13-03-17 18:23, Andy Shevchenko wrote:
> > On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:

> > I will incorporate slightly different version of this (due to
> > changes
> > happened) into my branch. Please verify if everything is okay.
> 
> I just noticed that my patch was botched up a bit, it is
> taking a more complex approach then needed and it contained a bug
> (it did lookup->index = index, which should be lookup->index = i).
> 
> I've just finished writing a v2 (sorry).

NP.

Does it mean this one replaces both from v2? (to me looks like)

>  I've attached the v2 here,
> note this is against an unmodified v4.11-rc2 rather then your branch
> as I needed a clean base to debug some problems. But it should be
> easy to adapt to your branch I think.
> 
> Note this can go upstream either way (through your branch or
> directly since it is based on a clean v4.11-rc2 now) but it is
> probably easier to take it upstream through your branch to
> avoid conflicts.

I will rebase my series on your patch, but I'm going to slightly modify
it anyway (basically squash one of my patches).

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Hans de Goede]

Hi,

On 13-03-17 20:19, Andy Shevchenko wrote:
> On Mon, 2017-03-13 at 18:56 +0100, Hans de Goede wrote:
>> On 13-03-17 18:23, Andy Shevchenko wrote:
>>> On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:
>
>>> I will incorporate slightly different version of this (due to
>>> changes
>>> happened) into my branch. Please verify if everything is okay.
>>
>> I just noticed that my patch was botched up a bit, it is
>> taking a more complex approach then needed and it contained a bug
>> (it did lookup->index = index, which should be lookup->index = i).
>>
>> I've just finished writing a v2 (sorry).
>
> NP.
>
> Does it mean this one replaces both from v2? (to me looks like)

Yes it replaces both.

>>  I've attached the v2 here,
>> note this is against an unmodified v4.11-rc2 rather then your branch
>> as I needed a clean base to debug some problems. But it should be
>> easy to adapt to your branch I think.
>>
>> Note this can go upstream either way (through your branch or
>> directly since it is based on a clean v4.11-rc2 now) but it is
>> probably easier to take it upstream through your branch to
>> avoid conflicts.
>
> I will rebase my series on your patch, but I'm going to slightly modify
> it anyway (basically squash one of my patches).

Ok, sounds good. If you're going to use my patch as a base,
shall I cherry-pick the version with your changes squashed and
submit that upstream stand-alone ?

Regards,

Hans

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Andy Shevchenko]

On Mon, 2017-03-13 at 20:23 +0100, Hans de Goede wrote:
> Hi,
> 
> On 13-03-17 20:19, Andy Shevchenko wrote:
> > On Mon, 2017-03-13 at 18:56 +0100, Hans de Goede wrote:
> > > On 13-03-17 18:23, Andy Shevchenko wrote:
> > > > On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:

> > I will rebase my series on your patch, but I'm going to slightly
> > modify
> > it anyway (basically squash one of my patches).
> 
> Ok, sounds good. If you're going to use my patch as a base,
> shall I cherry-pick the version with your changes squashed and
> submit that upstream stand-alone ?

If you want to, I have no objections.
Lemme submit the branch first.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Andy Shevchenko]

On Mon, 2017-03-13 at 21:31 +0200, Andy Shevchenko wrote:
> On Mon, 2017-03-13 at 20:23 +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 13-03-17 20:19, Andy Shevchenko wrote:
> > > On Mon, 2017-03-13 at 18:56 +0100, Hans de Goede wrote:
> > > > On 13-03-17 18:23, Andy Shevchenko wrote:
> > > > > On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:
> > > I will rebase my series on your patch, but I'm going to slightly
> > > modify
> > > it anyway (basically squash one of my patches).
> > 
> > Ok, sounds good. If you're going to use my patch as a base,
> > shall I cherry-pick the version with your changes squashed and
> > submit that upstream stand-alone ?
> 
> If you want to, I have no objections.
> Lemme submit the branch first.

Done.

Please, test it!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 0/2] gpio: acpi: acpi_dev_gpio_irq_get: ignore the status of unselected irqs

[Hans de Goede]

Hi,

On 13-03-17 20:38, Andy Shevchenko wrote:
> On Mon, 2017-03-13 at 21:31 +0200, Andy Shevchenko wrote:
>> On Mon, 2017-03-13 at 20:23 +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 13-03-17 20:19, Andy Shevchenko wrote:
>>>> On Mon, 2017-03-13 at 18:56 +0100, Hans de Goede wrote:
>>>>> On 13-03-17 18:23, Andy Shevchenko wrote:
>>>>>> On Fri, 2017-03-10 at 21:58 +0100, Hans de Goede wrote:
>>>> I will rebase my series on your patch, but I'm going to slightly
>>>> modify
>>>> it anyway (basically squash one of my patches).
>>>
>>> Ok, sounds good. If you're going to use my patch as a base,
>>> shall I cherry-pick the version with your changes squashed and
>>> submit that upstream stand-alone ?
>>
>> If you want to, I have no objections.
>> Lemme submit the branch first.

Ok.

> Done.
>
> Please, test it!

I've just completed tested your version of the patch (but not
your entire branch, sorry -ENOTIME) on 2 machines, so I'm
going to post it upstream now.

Regards,

Hans

Re: [PATCH 1/2] gpio: acpi: acpi_gpio_resource_lookup: return info even if there is no desc

[Linus Walleij]

On Fri, Mar 10, 2017 at 9:58 PM, Hans de Goede <hdegoede@redhat.com> wrote:

> Make acpi_gpio_resource_lookup fill acpi_gpio_info even if there is
> no gpio_desc for the gpio and add a found flag to lookup which is
> used to indicate that valid info was found.
>
> This is a preparation patch for making acpi_dev_gpio_irq_get actually
> skip interrupts which do not match the requested index, rather then
> bailing if it cannot retreive a gpio_desc for them.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I'm totally dependent on Mika/Rafael to review these ACPI patches.

Mika?

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: acpi: acpi_gpio_resource_lookup: return info even if there is no desc

[Hans de Goede]

Hi,

On 15-03-17 11:17, Linus Walleij wrote:
> On Fri, Mar 10, 2017 at 9:58 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Make acpi_gpio_resource_lookup fill acpi_gpio_info even if there is
>> no gpio_desc for the gpio and add a found flag to lookup which is
>> used to indicate that valid info was found.
>>
>> This is a preparation patch for making acpi_dev_gpio_irq_get actually
>> skip interrupts which do not match the requested index, rather then
>> bailing if it cannot retreive a gpio_desc for them.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I'm totally dependent on Mika/Rafael to review these ACPI patches.

This series has been superseded by: "gpio: acpi: Ignore -EPROBE_DEFER for unselected gpioints"
which is a single simpler patch fixing the issue in a better way.

Regards,

Hans