| Message ID | 20210317151928.41544-1-andriy.shevchenko@linux.intel.com |
|---|---|
| Headers | show |
| Series | gpio: sch: Interrupt support | expand |
On Wed, Mar 17, 2021 at 05:19:26PM +0200, Andy Shevchenko wrote: > The series adds event support to the Intel GPIO SCH driver. The hardware > routes all events through GPE0 GPIO event. > > I validated this on Intel Minnowboard (v1). > > If somebody has different hardware with the same GPIO controller, I would > appreciate additional testing. I've applied this to my review and testing queue, thanks! > Changes in v5: > - added missed IRQ acknowledge callback (hence kernel Oops) > - rewrite patch 2 completely from SCI to GPE hook > > Changes in v4 (https://lore.kernel.org/linux-gpio/20210316162613.87710-1-andriy.shevchenko@linux.intel.com/T/#u): > - turned to GPIO core infrastructure of IRQ chip instantiation (Linus) > - converted IRQ callbacks to use better APIs > - use handle_bad_irq() as default handler and now I know why, see > eb441337c714 ("gpio: pca953x: Set IRQ type when handle Intel Galileo Gen 2") > for the real example what happens if it's preset to something meaningful > - fixed remove stage (we have to remove SCI handler, which wasn't done in v3) > > Changes in v3 (https://lore.kernel.org/linux-gpio/cover.1574277614.git.jan.kiszka@siemens.com/T/#u): > - split-up of the irq enabling patch as requested by Andy > > Andy Shevchenko (1): > gpio: sch: Hook into ACPI GPE handler to catch GPIO edge events > > Jan Kiszka (1): > gpio: sch: Add edge event support > > drivers/gpio/gpio-sch.c | 196 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 188 insertions(+), 8 deletions(-) > > -- > 2.30.2 > -- With Best Regards, Andy Shevchenko
On Wed, Mar 17, 2021 at 4:19 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Add the required infrastructure to enable and report edge events > of the pins to the GPIO core. The actual hook-up of the event interrupt > will happen separately. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> I can't believe it that nobody added irq support to this driver for 10 years given how widely deployed it is! (Good work.) Don't you need to add select GPIOLIB_IRQCHIP to Kconfig? So the gpio_chip contains the .irq member you're using. > + sch->irqchip.name = "sch_gpio"; > + sch->irqchip.irq_ack = sch_irq_ack; > + sch->irqchip.irq_mask = sch_irq_mask; > + sch->irqchip.irq_unmask = sch_irq_unmask; > + sch->irqchip.irq_set_type = sch_irq_type; > + > + sch->chip.irq.chip = &sch->irqchip; > + sch->chip.irq.num_parents = 0; > + sch->chip.irq.parents = NULL; > + sch->chip.irq.parent_handler = NULL; > + sch->chip.irq.default_type = IRQ_TYPE_NONE; > + sch->chip.irq.handler = handle_bad_irq; I always add a local variable like: struct gpio_irq_chip *girq; And assign with the arrow, so as to make it easier to read: girq->parent_handler = NULL etc. +/- the above: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, Mar 25, 2021 at 09:13:51AM +0100, Linus Walleij wrote: > On Wed, Mar 17, 2021 at 4:19 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > Add the required infrastructure to enable and report edge events > > of the pins to the GPIO core. The actual hook-up of the event interrupt > > will happen separately. > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Co-developed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > I can't believe it that nobody added irq support to this driver for 10 > years given how widely deployed it is! (Good work.) > > Don't you need to add > > select GPIOLIB_IRQCHIP > > to Kconfig? So the gpio_chip contains the .irq member you're using. Seems legit, thanks! > > + sch->irqchip.name = "sch_gpio"; > > + sch->irqchip.irq_ack = sch_irq_ack; > > + sch->irqchip.irq_mask = sch_irq_mask; > > + sch->irqchip.irq_unmask = sch_irq_unmask; > > + sch->irqchip.irq_set_type = sch_irq_type; > > + > > + sch->chip.irq.chip = &sch->irqchip; > > + sch->chip.irq.num_parents = 0; > > + sch->chip.irq.parents = NULL; > > + sch->chip.irq.parent_handler = NULL; > > + sch->chip.irq.default_type = IRQ_TYPE_NONE; > > + sch->chip.irq.handler = handle_bad_irq; > > I always add a local variable like: > > struct gpio_irq_chip *girq; > > And assign with the arrow, so as to make it easier to read: > > girq->parent_handler = NULL > > etc. OK! > +/- the above: > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Thanks! -- With Best Regards, Andy Shevchenko
The series adds event support to the Intel GPIO SCH driver. The hardware routes all events through GPE0 GPIO event. I validated this on Intel Minnowboard (v1). If somebody has different hardware with the same GPIO controller, I would appreciate additional testing. Changes in v5: - added missed IRQ acknowledge callback (hence kernel Oops) - rewrite patch 2 completely from SCI to GPE hook Changes in v4 (https://lore.kernel.org/linux-gpio/20210316162613.87710-1-andriy.shevchenko@linux.intel.com/T/#u): - turned to GPIO core infrastructure of IRQ chip instantiation (Linus) - converted IRQ callbacks to use better APIs - use handle_bad_irq() as default handler and now I know why, see eb441337c714 ("gpio: pca953x: Set IRQ type when handle Intel Galileo Gen 2") for the real example what happens if it's preset to something meaningful - fixed remove stage (we have to remove SCI handler, which wasn't done in v3) Changes in v3 (https://lore.kernel.org/linux-gpio/cover.1574277614.git.jan.kiszka@siemens.com/T/#u): - split-up of the irq enabling patch as requested by Andy Andy Shevchenko (1): gpio: sch: Hook into ACPI GPE handler to catch GPIO edge events Jan Kiszka (1): gpio: sch: Add edge event support drivers/gpio/gpio-sch.c | 196 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 188 insertions(+), 8 deletions(-)