* [PATCH] gpio: max732x: Use irqchip template
@ 2020-07-17 14:19 Linus Walleij
2020-07-19 16:34 ` Sam Protsenko
0 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2020-07-17 14:19 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij, Sam Protsenko
This makes the driver use the irqchip template to assign
properties to the gpio_irq_chip instead of using the
explicit calls to gpiochip_irqchip_add_nested() and
gpiochip_set_nested_irqchip(). The irqchip is instead
added while adding the gpiochip.
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-max732x.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c
index 63472f308857..347415344a20 100644
--- a/drivers/gpio/gpio-max732x.c
+++ b/drivers/gpio/gpio-max732x.c
@@ -503,6 +503,8 @@ static int max732x_irq_setup(struct max732x_chip *chip,
if (((pdata && pdata->irq_base) || client->irq)
&& has_irq != INT_NONE) {
+ struct gpio_irq_chip *girq;
+
if (pdata)
irq_base = pdata->irq_base;
chip->irq_features = has_irq;
@@ -517,19 +519,17 @@ static int max732x_irq_setup(struct max732x_chip *chip,
client->irq);
return ret;
}
- ret = gpiochip_irqchip_add_nested(&chip->gpio_chip,
- &max732x_irq_chip,
- irq_base,
- handle_simple_irq,
- IRQ_TYPE_NONE);
- if (ret) {
- dev_err(&client->dev,
- "could not connect irqchip to gpiochip\n");
- return ret;
- }
- gpiochip_set_nested_irqchip(&chip->gpio_chip,
- &max732x_irq_chip,
- client->irq);
+
+ girq = &chip->gpio_chip.irq;
+ girq->chip = &max732x_irq_chip;
+ /* This will let us handle the parent IRQ in the driver */
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ girq->threaded = true;
+ girq->first = irq_base; /* FIXME: get rid of this */
}
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: max732x: Use irqchip template
2020-07-17 14:19 [PATCH] gpio: max732x: Use irqchip template Linus Walleij
@ 2020-07-19 16:34 ` Sam Protsenko
2020-07-21 9:54 ` Linus Walleij
0 siblings, 1 reply; 5+ messages in thread
From: Sam Protsenko @ 2020-07-19 16:34 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, Bartosz Golaszewski
Hi Linus,
On Fri, 17 Jul 2020 at 17:19, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This makes the driver use the irqchip template to assign
> properties to the gpio_irq_chip instead of using the
> explicit calls to gpiochip_irqchip_add_nested() and
> gpiochip_set_nested_irqchip(). The irqchip is instead
> added while adding the gpiochip.
>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
I can test it on my MAX7325 board with BeagleBone Black in a day or
two. Do you want me to verify your patch on top of linux-mainline or
linux-next? Also, is there any specific stuff you want me to look at,
or making sure there are no regressions w.r.t. IRQ from the chip is
enough?
Thanks!
> drivers/gpio/gpio-max732x.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpio-max732x.c b/drivers/gpio/gpio-max732x.c
> index 63472f308857..347415344a20 100644
> --- a/drivers/gpio/gpio-max732x.c
> +++ b/drivers/gpio/gpio-max732x.c
> @@ -503,6 +503,8 @@ static int max732x_irq_setup(struct max732x_chip *chip,
>
> if (((pdata && pdata->irq_base) || client->irq)
> && has_irq != INT_NONE) {
> + struct gpio_irq_chip *girq;
> +
> if (pdata)
> irq_base = pdata->irq_base;
> chip->irq_features = has_irq;
> @@ -517,19 +519,17 @@ static int max732x_irq_setup(struct max732x_chip *chip,
> client->irq);
> return ret;
> }
> - ret = gpiochip_irqchip_add_nested(&chip->gpio_chip,
> - &max732x_irq_chip,
> - irq_base,
> - handle_simple_irq,
> - IRQ_TYPE_NONE);
> - if (ret) {
> - dev_err(&client->dev,
> - "could not connect irqchip to gpiochip\n");
> - return ret;
> - }
> - gpiochip_set_nested_irqchip(&chip->gpio_chip,
> - &max732x_irq_chip,
> - client->irq);
> +
> + girq = &chip->gpio_chip.irq;
> + girq->chip = &max732x_irq_chip;
> + /* This will let us handle the parent IRQ in the driver */
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> + girq->threaded = true;
> + girq->first = irq_base; /* FIXME: get rid of this */
> }
>
> return 0;
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: max732x: Use irqchip template
2020-07-19 16:34 ` Sam Protsenko
@ 2020-07-21 9:54 ` Linus Walleij
2020-07-24 10:50 ` Sam Protsenko
0 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2020-07-21 9:54 UTC (permalink / raw)
To: Sam Protsenko; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
On Sun, Jul 19, 2020 at 6:34 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> On Fri, 17 Jul 2020 at 17:19, Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > This makes the driver use the irqchip template to assign
> > properties to the gpio_irq_chip instead of using the
> > explicit calls to gpiochip_irqchip_add_nested() and
> > gpiochip_set_nested_irqchip(). The irqchip is instead
> > added while adding the gpiochip.
> >
> > Cc: Sam Protsenko <semen.protsenko@linaro.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> I can test it on my MAX7325 board with BeagleBone Black in a day or
> two.
Thanks! No hurry.
> Do you want me to verify your patch on top of linux-mainline or
> linux-next?
Either should work.
> Also, is there any specific stuff you want me to look at,
> or making sure there are no regressions w.r.t. IRQ from the chip is
> enough?
Just that really, that the IRQs happen as before.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: max732x: Use irqchip template
2020-07-21 9:54 ` Linus Walleij
@ 2020-07-24 10:50 ` Sam Protsenko
2020-07-24 19:47 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Sam Protsenko @ 2020-07-24 10:50 UTC (permalink / raw)
To: Linus Walleij; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
Hi Linus,
On Tue, 21 Jul 2020 at 12:54, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Jul 19, 2020 at 6:34 PM Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
> > On Fri, 17 Jul 2020 at 17:19, Linus Walleij <linus.walleij@linaro.org> wrote:
> > >
> > > This makes the driver use the irqchip template to assign
> > > properties to the gpio_irq_chip instead of using the
> > > explicit calls to gpiochip_irqchip_add_nested() and
> > > gpiochip_set_nested_irqchip(). The irqchip is instead
> > > added while adding the gpiochip.
> > >
> > > Cc: Sam Protsenko <semen.protsenko@linaro.org>
> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> > I can test it on my MAX7325 board with BeagleBone Black in a day or
> > two.
>
> Thanks! No hurry.
>
> > Do you want me to verify your patch on top of linux-mainline or
> > linux-next?
>
> Either should work.
>
> > Also, is there any specific stuff you want me to look at,
> > or making sure there are no regressions w.r.t. IRQ from the chip is
> > enough?
>
> Just that really, that the IRQs happen as before.
>
Just tested it on my MAX7325 board [1], by adding gpio-keys and
gpio-leds to BBB dts [2]. Alas, the patch seems to be breaking IRQs.
Before the patch, I can see gpio-buttons appear in /proc/interrupts
and dmesg is clear of errors. After applying the patch, no gpio-keys
appear in /proc/interrupts and dmesg is reporting errors like this:
irq: no irq domain found for max7325@68 !
This is probably because the patch is setting gpio_chip structure
fields after devm_gpiochip_add_data() was executed. Next hacky change
fixes it:
<<<<<<<<<<<<<<<<<< cut here >>>>>>>>>>>>>>>>>
@@ -695,11 +695,11 @@ static int max732x_probe(struct i2c_client *client,
return ret;
}
- ret = devm_gpiochip_add_data(&client->dev, &chip->gpio_chip, chip);
+ ret = max732x_irq_setup(chip, id);
if (ret)
return ret;
- ret = max732x_irq_setup(chip, id);
+ ret = devm_gpiochip_add_data(&client->dev, &chip->gpio_chip, chip);
if (ret)
return ret;
<<<<<<<<<<<<<<<<<< cut here >>>>>>>>>>>>>>>>>
I didn't check if it's a correct fix, so the diff above is only to
check the idea. Anyway, if you send v2 I can retest it, as my setup is
ready.
Thanks!
[1] https://github.com/joe-skb7/max7325-pcb
[2] https://github.com/joe-skb7/linux-mainline-bbb-max732x/commits/max732_bbb_test
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: max732x: Use irqchip template
2020-07-24 10:50 ` Sam Protsenko
@ 2020-07-24 19:47 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2020-07-24 19:47 UTC (permalink / raw)
To: Sam Protsenko
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski
On Fri, Jul 24, 2020 at 1:50 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
> On Tue, 21 Jul 2020 at 12:54, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Sun, Jul 19, 2020 at 6:34 PM Sam Protsenko
> > <semen.protsenko@linaro.org> wrote:
...
> > Just that really, that the IRQs happen as before.
> Just tested it on my MAX7325 board [1], by adding gpio-keys and
> gpio-leds to BBB dts [2]. Alas, the patch seems to be breaking IRQs.
> Before the patch, I can see gpio-buttons appear in /proc/interrupts
> and dmesg is clear of errors. After applying the patch, no gpio-keys
> appear in /proc/interrupts and dmesg is reporting errors like this:
>
> irq: no irq domain found for max7325@68 !
>
> This is probably because the patch is setting gpio_chip structure
> fields after devm_gpiochip_add_data() was executed.
...
> I didn't check if it's a correct fix, so the diff above is only to
> check the idea.
I checked pca953x and there the same trick has been applied. In my
setup that driver works. I think it's the right way to go.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-24 19:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 14:19 [PATCH] gpio: max732x: Use irqchip template Linus Walleij
2020-07-19 16:34 ` Sam Protsenko
2020-07-21 9:54 ` Linus Walleij
2020-07-24 10:50 ` Sam Protsenko
2020-07-24 19:47 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).