[PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[Semen Protsenko]

It's possible that multiple MAX732X can be hooked up to the same
interrupt line with the processor. So add IRQF_SHARED in requesting irq.

Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com>
---
 drivers/gpio/gpio-max732x.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c
index 0fa4543..55b13d4 100644
--- a/drivers/gpio/gpio-max732x.c
+++ b/drivers/gpio/gpio-max732x.c
@@ -507,12 +507,10 @@ static int max732x_irq_setup(struct max732x_chip *chip,
 		chip->irq_features = has_irq;
 		mutex_init(&chip->irq_lock);
 
-		ret = devm_request_threaded_irq(&client->dev,
-					client->irq,
-					NULL,
-					max732x_irq_handler,
-					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					dev_name(&client->dev), chip);
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+				NULL, max732x_irq_handler, IRQF_ONESHOT |
+				IRQF_TRIGGER_FALLING | IRQF_SHARED,
+				dev_name(&client->dev), chip);
 		if (ret) {
 			dev_err(&client->dev, "failed to request irq %d\n",
 				client->irq);
-- 
1.7.9.5

Re: [PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[Vladimir Zapolskiy]

Hi Semen,

On 21.04.2015 16:19, Semen Protsenko wrote:
> It's possible that multiple MAX732X can be hooked up to the same
> interrupt line with the processor. So add IRQF_SHARED in requesting irq.
> 
> Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com>
> ---
>  drivers/gpio/gpio-max732x.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c
> index 0fa4543..55b13d4 100644
> --- a/drivers/gpio/gpio-max732x.c
> +++ b/drivers/gpio/gpio-max732x.c
> @@ -507,12 +507,10 @@ static int max732x_irq_setup(struct max732x_chip *chip,
>  		chip->irq_features = has_irq;
>  		mutex_init(&chip->irq_lock);
>  
> -		ret = devm_request_threaded_irq(&client->dev,
> -					client->irq,
> -					NULL,
> -					max732x_irq_handler,
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					dev_name(&client->dev), chip);
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +				NULL, max732x_irq_handler, IRQF_ONESHOT |
> +				IRQF_TRIGGER_FALLING | IRQF_SHARED,
> +				dev_name(&client->dev), chip);

is it still the case that for shared interrupts a hard IRQ handler is
mandatory to have?

Here I rely on
http://lists.kernelnewbies.org/pipermail/kernelnewbies/2011-March/001118.html

With best wishes,
Vladimir

Re: [PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[Sam Protsenko]

On Apr 22, 2015 at 10:53 PM, Vladimir Zapolskiy wrote:
> is it still the case that for shared interrupts a hard IRQ handler is
> mandatory to have?
>
> Here I rely on
> http://lists.kernelnewbies.org/pipermail/kernelnewbies/2011-March/001118.html
>
> With best wishes,
> Vladimir
>

Vladimir,

A hard IRQ handler is not mandatory to have for shared interrupts. It just can
make things a bit faster. In fact, there is a lot of drivers that implement the
same behavior as my patch proposes (requesting threaded interrupt with
IRQF_SHARED flag and default IRQ handler). Well, there *may* be some cases
when hard IRQ handler is mandatory, but it's definitely not this case.

Basically, what happens when interrupt arrives from the line that has multiple
devices on it -- is that all the interrupt handlers (for devices on that line)
are being executed. Then each handler reads interrupt register from associated
device and decides if interrupt was from it's device. In my case reading is
happening via I2C interface, which can take a while to finish, hence it's not
recommended to perform I2C operations in hard IRQ handlers.

The general approach for I2C drivers is to do request_threaded_irq() with
NULL specified for hard IRQ handler (so irq_default_primary_handler() will be
used, which just wakes bottom half handler). Driver should read interrupt
register (via I2C) in threaded IRQ handler, do corresponding actions and send
acknowledge to device, so device can release interrupt line. In my case
acknowledge is happening automatically on I2C read operation. Also it's
important to provide IRQF_ONESHOT flag when requesting threaded irq:
this way irq line will be disabled before running threaded handler.

Hope it answers your question.

Best regards,
  Sam Protsenko

Re: [PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[Linus Walleij]

On Tue, Apr 21, 2015 at 3:19 PM, Semen Protsenko
<semen.protsenko@globallogic.com> wrote:

> It's possible that multiple MAX732X can be hooked up to the same
> interrupt line with the processor. So add IRQF_SHARED in requesting irq.
>
> Signed-off-by: Semen Protsenko <semen.protsenko@globallogic.com>

Patch applied.

Since you're doing these patches I assume my conversion to
GPIOLIB_IRQCHIP in the last merge window just worked.

Yours,
Linus Walleij

Re: [PATCH] gpio: max732x: Add IRQF_SHARED to irq flags

[Sam Protsenko]

On Wed, May 6, 2015 at 4:07 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

> Since you're doing these patches I assume my conversion to
> GPIOLIB_IRQCHIP in the last merge window just worked.

Actually I've only verified those patches on k3.14, and tested that mainline
kernel builds successfully (with patches applied). For the moment I don't have
the board with MAX732X where I can't test the last kernel. But I'm finishing
building my own MAX7325 test board, and once it finished, I can verifyall mine
and yours patches on my BeagleBone Black (connected to my MAX7325 board).

But I'm pretty sure that all mentioned patches are gonna work just fine
(since my changes are really minimal and well-tested on k3.14, and things
affected by my patches weren't really changed since then).