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

Message ID 3133c2fe-427e-f742-9647-d1f45eeaed4a@redhat.com
State New
Headers show

Commit Message

Hans de Goede March 13, 2017, 5:56 p.m.
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

Comments

Andy Shevchenko March 13, 2017, 7:19 p.m. | #1
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).
Hans de Goede March 13, 2017, 7:23 p.m. | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko March 13, 2017, 7:31 p.m. | #3
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 March 13, 2017, 7:38 p.m. | #4
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!
Hans de Goede March 13, 2017, 10:02 p.m. | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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