gpio/at91: free GPIO after configuring as input

PATCH New Alexander Stein 4 participants 5 messages Apr 11, 2014 – Apr 12, 2014

Subsystems (4)

HID Arm/Microchip (AT91) SoC support Pin control Pin controller - Microchip AT91

lore

Share & tools

lore

Alexander Stein

author

2014-04-11 14:24 UTC #0 lore raw

PATCH 1/1 gpio/at91: free GPIO after configuring as input

If the GPIO stays requested a device driver can't request it again. e.g. Without this patch the ads7846 driver returns the following error:
ads7846 spi32766.3: failed to request/setup pendown GPIO15: -16
ads7846: probe of spi32766.3 failed with error -16

/sys/kernel/debug/gpio shows this:
GPIOs 0-31, platform/fffff200.gpio, fffff200.gpio:
[/ahb/apb/pinctrl at fffff200/gpio at fffff200] GPIOfffff200.gpio15: [gpio] set

Signed-off-by Alexander Stein <[email protected]>

---
I'm aware that it makes sense this GPIO is/stays requested, but either the pinctl or device driver have to be adjusted as both can't request this GPIO. I think the latter shouldn't change.

 drivers/pinctrl/pinctrl-at91.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/pinctrl/pinctrl-at91.c +2 -0

index d990e33..63176f2 100644
@@ -1493,6 +1493,8 @@ static int at91_gpio_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
+	gpio_free(pin);
+
 	return 0;
 }

Signature · 1 line

1.9.2

Jean-Christophe PLAGNIOL-VILLARD

2014-04-11 15:30 UTC #1 lore raw

PATCH 1/1 gpio/at91: free GPIO after configuring as input

Quoted 24 lines · Alexander Stein

If the GPIO stays requested a device driver can't request it again. e.g. Without this patch the ads7846 driver returns the following error:
ads7846 spi32766.3: failed to request/setup pendown GPIO15: -16
ads7846: probe of spi32766.3 failed with error -16
/sys/kernel/debug/gpio shows this:
GPIOs 0-31, platform/fffff200.gpio, fffff200.gpio:
[/ahb/apb/pinctrl at fffff200/gpio at fffff200] GPIOfffff200.gpio15: [gpio] set
Signed-off-by Alexander Stein <[email protected]>
---
I'm aware that it makes sense this GPIO is/stays requested, but either the pinctl or device driver have to be adjusted as both can't request this GPIO. I think the latter shouldn't change.
drivers/pinctrl/pinctrl-at91.c | 2 ++
1 file changed, 2 insertions(+)
drivers/pinctrl/pinctrl-at91.c +0 -0
index d990e33..63176f2 100644

drivers/pinctrl/pinctrl-at91.c static int at91_gpio_irq_domain_xlate(struct irq_domain *d, · line 1497

Quoted 5 lines

drivers/pinctrl/pinctrl-at91.c +2 -0

@@ -1493,6 +1493,8 @@ static int at91_gpio_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
+	gpio_free(pin);
+

NACK it the whole key point the gpio use as a IRQ so the irq generic code request it

Quoted 6 lines

drivers/pinctrl/pinctrl-at91.c +0 -0
 	return 0;
 }
 
Signature · 1 line
1.9.2

Alexander Stein

author

2014-04-11 15:45 UTC #2 lore raw

PATCH 1/1 gpio/at91: free GPIO after configuring as input

I'm CC'ing some input guys if this can't be solved in the pinctrl/irq side.

On Friday 11 April 2014, 23:30:33 wrote Jean-Christophe PLAGNIOL-VILLARD:

Quoted 38 lines · Alexander Stein

Quoted 25 lines
If the GPIO stays requested a device driver can't request it again. e.g. Without this patch the ads7846 driver returns the following error:
ads7846 spi32766.3: failed to request/setup pendown GPIO15: -16
ads7846: probe of spi32766.3 failed with error -16
/sys/kernel/debug/gpio shows this:
GPIOs 0-31, platform/fffff200.gpio, fffff200.gpio:
[/ahb/apb/pinctrl at fffff200/gpio at fffff200] GPIOfffff200.gpio15: [gpio] set
Signed-off-by Alexander Stein <[email protected]>
---
I'm aware that it makes sense this GPIO is/stays requested, but either the pinctl or device driver have to be adjusted as both can't request this GPIO. I think the latter shouldn't change.
drivers/pinctrl/pinctrl-at91.c | 2 ++
1 file changed, 2 insertions(+)
drivers/pinctrl/pinctrl-at91.c +0 -0
index d990e33..63176f2 100644
drivers/pinctrl/pinctrl-at91.c static int at91_gpio_irq_domain_xlate(struct irq_domain *d, · line 1497
Quoted 5 lines
drivers/pinctrl/pinctrl-at91.c +2 -0
@@ -1493,6 +1493,8 @@ static int at91_gpio_irq_domain_xlate(struct irq_domain *d,
 	if (ret)
 		return ret;
 
+	gpio_free(pin);
+
NACK it the whole key point the gpio use as a IRQ so the irq generic code request it
Quoted 3 lines
drivers/pinctrl/pinctrl-at91.c +0 -0
 	return 0;
 }
 

Javier Martinez Canillas

2014-04-11 17:28 UTC #3 lore raw

PATCH 1/1 gpio/at91: free GPIO after configuring as input

Quoted 41 lines · Alexander Stein

I'm CC'ing some input guys if this can't be solved in the pinctrl/irq side.
On Friday 11 April 2014, 23:30:33 wrote Jean-Christophe PLAGNIOL-VILLARD:
Quoted 38 lines · Alexander Stein
Quoted 25 lines
If the GPIO stays requested a device driver can't request it again. e.g. Without this patch the ads7846 driver returns the following error:
ads7846 spi32766.3: failed to request/setup pendown GPIO15: -16
ads7846: probe of spi32766.3 failed with error -16
/sys/kernel/debug/gpio shows this:
GPIOs 0-31, platform/fffff200.gpio, fffff200.gpio:
[/ahb/apb/pinctrl at fffff200/gpio at fffff200] GPIOfffff200.gpio15: [gpio] set
Signed-off-by Alexander Stein <[email protected]>
---
I'm aware that it makes sense this GPIO is/stays requested, but either the pinctl or device driver have to be adjusted as both can't request this GPIO. I think the latter shouldn't change.
drivers/pinctrl/pinctrl-at91.c | 2 ++
1 file changed, 2 insertions(+)
drivers/pinctrl/pinctrl-at91.c +0 -0
index d990e33..63176f2 100644
drivers/pinctrl/pinctrl-at91.c static int at91_gpio_irq_domain_xlate(struct irq_domain *d, · line 1497
Quoted 5 lines
drivers/pinctrl/pinctrl-at91.c +2 -0
@@ -1493,6 +1493,8 @@ static int at91_gpio_irq_domain_xlate(struct irq_domain *d,
    if (ret)
            return ret;
 
+   gpio_free(pin);
+
NACK it the whole key point the gpio use as a IRQ so the irq generic code request it
Quoted 3 lines
drivers/pinctrl/pinctrl-at91.c +0 -0
    return 0;
 }
 

The problem is that the GPIO and IRQ subsystems are somehow related but completely independent. A GPIO pin used as an IRQ line is completely orthogonal to requesting a GPIO pin.

Is true that here is a common pattern in the kernel that is:

gpio_request(gpio,...);
gpio_direction_input()
request[_threaded]_irq(gpio_to_irq(gpio), ...);

But one should not assume that requesting a GPIO to be used as an IRQ line implies requesting the GPIO first. It is completely legal to only request the IRQ without requesting the GPIO before.

Of course at a hardware level the pin has to be configured as input if needed by the chip controller and also a following call to gpio_request() as output should not be supported.

After a long discussion the agreement is that the driver should be able to setup as a hw level if another driver wants to use a GPIO pin as an IRQ line but not requesting the GPIO. So, I think that is wrong to call gpio_request() and gpio_direction_input() on a irq_domain_ops .xlate() function handler since by doing that a subsequent call to gpio_request() is failing like you are reporting.

The GPIO subsystem has a new gpio_lock_as_irq() helper function to mark a GPIO pin as already used as a IRQ line and only allowing to request a GPIO as input if is already marked as an IRQ.

Please take a look to commit 2f56e0a ("gpio/omap: use gpiolib API to mark a GPIO used as an IRQ") for an example on how to use this helper function.

Also, there is a new GPIOLIB_IRQCHIP infrastructure in gpiolib that are a set of helper functions to associate an IRQ chip to a GPIO controller. This should cover the most common use case and most probably this driver and can be converted to use it.

Please refer to commits:

1425052 ("gpio: add IRQ chip helpers in gpiolib") e0bc34a ("pinctrl: nomadik: convert driver to use gpiolib irqchip")

for an example on how to use it.

Thanks a lot and best regards,
Javier

Alexandre Belloni

2014-04-12 22:48 UTC #4 lore raw

PATCH 1/1 gpio/at91: free GPIO after configuring as input

Hi,

Quoted 81 lines · Javier Martinez Canillas

Quoted 41 lines · Alexander Stein
I'm CC'ing some input guys if this can't be solved in the pinctrl/irq side.
On Friday 11 April 2014, 23:30:33 wrote Jean-Christophe PLAGNIOL-VILLARD:
Quoted 38 lines · Alexander Stein
Quoted 25 lines
If the GPIO stays requested a device driver can't request it again. e.g. Without this patch the ads7846 driver returns the following error:
ads7846 spi32766.3: failed to request/setup pendown GPIO15: -16
ads7846: probe of spi32766.3 failed with error -16
/sys/kernel/debug/gpio shows this:
GPIOs 0-31, platform/fffff200.gpio, fffff200.gpio:
[/ahb/apb/pinctrl at fffff200/gpio at fffff200] GPIOfffff200.gpio15: [gpio] set
Signed-off-by Alexander Stein <[email protected]>
---
I'm aware that it makes sense this GPIO is/stays requested, but either the pinctl or device driver have to be adjusted as both can't request this GPIO. I think the latter shouldn't change.
drivers/pinctrl/pinctrl-at91.c | 2 ++
1 file changed, 2 insertions(+)
drivers/pinctrl/pinctrl-at91.c +0 -0
index d990e33..63176f2 100644
drivers/pinctrl/pinctrl-at91.c static int at91_gpio_irq_domain_xlate(struct irq_domain *d, · line 1497
Quoted 5 lines
drivers/pinctrl/pinctrl-at91.c +2 -0
@@ -1493,6 +1493,8 @@ static int at91_gpio_irq_domain_xlate(struct irq_domain *d,
    if (ret)
            return ret;
 
+   gpio_free(pin);
+
NACK it the whole key point the gpio use as a IRQ so the irq generic code request it
Quoted 3 lines
drivers/pinctrl/pinctrl-at91.c +0 -0
    return 0;
 }
 
The problem is that the GPIO and IRQ subsystems are somehow related but completely independent. A GPIO pin used as an IRQ line is completely orthogonal to requesting a GPIO pin.
Is true that here is a common pattern in the kernel that is:
gpio_request(gpio,...);
gpio_direction_input()
request[_threaded]_irq(gpio_to_irq(gpio), ...);
But one should not assume that requesting a GPIO to be used as an IRQ line implies requesting the GPIO first. It is completely legal to only request the IRQ without requesting the GPIO before.
Of course at a hardware level the pin has to be configured as input if needed by the chip controller and also a following call to gpio_request() as output should not be supported.
After a long discussion the agreement is that the driver should be able to setup as a hw level if another driver wants to use a GPIO pin as an IRQ line but not requesting the GPIO. So, I think that is wrong to call gpio_request() and gpio_direction_input() on a irq_domain_ops .xlate() function handler since by doing that a subsequent call to gpio_request() is failing like you are reporting.
The GPIO subsystem has a new gpio_lock_as_irq() helper function to mark a GPIO pin as already used as a IRQ line and only allowing to request a GPIO as input if is already marked as an IRQ.
Please take a look to commit 2f56e0a ("gpio/omap: use gpiolib API to mark a GPIO used as an IRQ") for an example on how to use this helper function.
Also, there is a new GPIOLIB_IRQCHIP infrastructure in gpiolib that are a set of helper functions to associate an IRQ chip to a GPIO controller. This should cover the most common use case and most probably this driver and can be converted to use it.

Quoted 7 lines

Please refer to commits:
1425052 ("gpio: add IRQ chip helpers in gpiolib") e0bc34a ("pinctrl: nomadik: convert driver to use gpiolib irqchip")
for an example on how to use it.

You probably want to have a look at:
https://lkml.org/lkml/2014/3/3/110

Signature · 3 lines

Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com