Re: [PATCH] gpio: aggregator: Add interrupt support
Hi Kamel,
On Thu, Apr 13, 2023 at 9:48 AM wrote:
> Le 2021-10-04 14:44, Geert Uytterhoeven a écrit :
> What is the status for this patch, is there any remaining
> changes to be made ?
You mean commit a00128dfc8fc0cc8 ("gpio: aggregator: Add interrupt
support") in v5.17?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] gpio: aggregator: Add interrupt support
Hi Viresh,
On Tue, Oct 5, 2021 at 7:50 AM Viresh Kumar wrote:
> On 04-10-21, 14:44, Geert Uytterhoeven wrote:
> > Currently the GPIO Aggregator does not support interrupts. This means
> > that kernel drivers going from a GPIO to an IRQ using gpiod_to_irq(),
> > and userspace applications using line events do not work.
> >
> > Add interrupt support by providing a gpio_chip.to_irq() callback, which
> > just calls into the parent GPIO controller.
> >
> > Note that this does not implement full interrupt controller (irq_chip)
> > support, so using e.g. gpio-keys with "interrupts" instead of "gpios"
> > still does not work.
>
> Thanks for looking into this. I am not sure of the difference it makes
> with and without full irq-chip, but lemme explain the use case that we
> are concerned about with virtio.
>
> Eventually the interrupt should be visible to userspace, with
> something like libgpiod. Which can then send the information over
> virtio to the guest.
Exactly, that was what I had in mind, too.
> Will the interrupts be visible in userspace with your patch ?
Yes they are.
Before, gpiomon (test app from libgpiod) didn't work, now it does.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] gpio: aggregator: Add interrupt support
On 04-10-21, 14:44, Geert Uytterhoeven wrote: > Currently the GPIO Aggregator does not support interrupts. This means > that kernel drivers going from a GPIO to an IRQ using gpiod_to_irq(), > and userspace applications using line events do not work. > > Add interrupt support by providing a gpio_chip.to_irq() callback, which > just calls into the parent GPIO controller. > > Note that this does not implement full interrupt controller (irq_chip) > support, so using e.g. gpio-keys with "interrupts" instead of "gpios" > still does not work. Hi Geert, Thanks for looking into this. I am not sure of the difference it makes with and without full irq-chip, but lemme explain the use case that we are concerned about with virtio. Eventually the interrupt should be visible to userspace, with something like libgpiod. Which can then send the information over virtio to the guest. Will the interrupts be visible in userspace with your patch ? -- viresh ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] gpio: aggregator: Add interrupt support
Hi Andy,
On Mon, Oct 4, 2021 at 3:21 PM Andy Shevchenko
wrote:
> On Mon, Oct 4, 2021 at 3:45 PM Geert Uytterhoeven
> wrote:
> > Currently the GPIO Aggregator does not support interrupts. This means
> > that kernel drivers going from a GPIO to an IRQ using gpiod_to_irq(),
> > and userspace applications using line events do not work.
> >
> > Add interrupt support by providing a gpio_chip.to_irq() callback, which
> > just calls into the parent GPIO controller.
> >
> > Note that this does not implement full interrupt controller (irq_chip)
> > support, so using e.g. gpio-keys with "interrupts" instead of "gpios"
> > still does not work.
>
> ...
>
> > @@ -414,7 +421,8 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct
> > device *dev,
> > for (i = 0; i < ngpios; i++) {
> > struct gpio_chip *parent = gpiod_to_chip(descs[i]);
> >
> > - dev_dbg(dev, "%u => gpio-%d\n", i, desc_to_gpio(descs[i]));
> > + dev_dbg(dev, "%u => gpio %d irq %d\n", i,
> > + desc_to_gpio(descs[i]), gpiod_to_irq(descs[i]));
>
> This is an unconditional call that will allocate the IRQ descriptor
If DEBUG and CONFIG_DYNAMIC_DEBUG* are not enabled, it's a no-op
(protected by if (0) { ... }).
If CONFIG_DYNAMIC_DEBUG is enabled, the operation is a no-op if not
enabled dynamically (if (dynamic_checl) { ... }).
If DEBUG (CONFIG_DEBUG_GPIO) is enabled, the output is wanted.
(yes, I've just checked the preprocessor and assembler output ;-).
> even if we don't use it. Correct?
It calls .to_irq() of the parent GPIO controller, which is usually
just doing some offset addition. But that's driver-dependent.
> If so, I don't like this.
No worries, desc_to_gpio() and gpiod_to_irq() are only evaluated when
the debug output is wanted.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] gpio: aggregator: Add interrupt support
On Mon, Oct 4, 2021 at 3:45 PM Geert Uytterhoeven
wrote:
>
> Currently the GPIO Aggregator does not support interrupts. This means
> that kernel drivers going from a GPIO to an IRQ using gpiod_to_irq(),
> and userspace applications using line events do not work.
>
> Add interrupt support by providing a gpio_chip.to_irq() callback, which
> just calls into the parent GPIO controller.
>
> Note that this does not implement full interrupt controller (irq_chip)
> support, so using e.g. gpio-keys with "interrupts" instead of "gpios"
> still does not work.
...
> @@ -414,7 +421,8 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct
> device *dev,
> for (i = 0; i < ngpios; i++) {
> struct gpio_chip *parent = gpiod_to_chip(descs[i]);
>
> - dev_dbg(dev, "%u => gpio-%d\n", i, desc_to_gpio(descs[i]));
> + dev_dbg(dev, "%u => gpio %d irq %d\n", i,
> + desc_to_gpio(descs[i]), gpiod_to_irq(descs[i]));
This is an unconditional call that will allocate the IRQ descriptor
even if we don't use it. Correct?
If so, I don't like this.
--
With Best Regards,
Andy Shevchenko
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] gpio: aggregator: Add interrupt support
Currently the GPIO Aggregator does not support interrupts. This means
that kernel drivers going from a GPIO to an IRQ using gpiod_to_irq(),
and userspace applications using line events do not work.
Add interrupt support by providing a gpio_chip.to_irq() callback, which
just calls into the parent GPIO controller.
Note that this does not implement full interrupt controller (irq_chip)
support, so using e.g. gpio-keys with "interrupts" instead of "gpios"
still does not work.
Signed-off-by: Geert Uytterhoeven
---
I would prefer to avoid implementing irq_chip support, until there is a
real use case for this.
This has been tested with gpio-keys and gpiomon on the Koelsch
development board:
- gpio-keys, using a DT overlay[1]:
$ overlay add r8a7791-koelsch-keyboard-controlled-led
$ echo gpio-aggregator >
/sys/devices/platform/frobnicator/driver_override
$ echo frobnicator > /sys/bus/platform/drivers/gpio-aggregator/bind
$ gpioinfo frobnicator
gpiochip12 - 3 lines:
line 0: "light" "light" output active-high [used]
line 1: "on" "On" input active-low [used]
line 2:"off""Off" input active-low [used]
$ echo 255 > /sys/class/leds/light/brightness
$ echo 0 > /sys/class/leds/light/brightness
$ evtest /dev/input/event0
- gpiomon, using the GPIO sysfs API:
$ echo keyboard > /sys/bus/platform/drivers/gpio-keys/unbind
$ echo e6055800.gpio 2,6 >
/sys/bus/platform/drivers/gpio-aggregator/new_device
$ gpiomon gpiochip12 0 1
[1] "ARM: dts: koelsch: Add overlay for keyboard-controlled LED"
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/renesas-overlays=c78d817869e63a3485bb4ab98aeea6ce368a396e
---
drivers/gpio/gpio-aggregator.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 34e35b64dcdc0581..2ff867d2a3630d3b 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -374,6 +374,13 @@ static int gpio_fwd_set_config(struct gpio_chip *chip,
unsigned int offset,
return gpiod_set_config(fwd->descs[offset], config);
}
+static int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+ struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
+
+ return gpiod_to_irq(fwd->descs[offset]);
+}
+
/**
* gpiochip_fwd_create() - Create a new GPIO forwarder
* @dev: Parent device pointer
@@ -414,7 +421,8 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct
device *dev,
for (i = 0; i < ngpios; i++) {
struct gpio_chip *parent = gpiod_to_chip(descs[i]);
- dev_dbg(dev, "%u => gpio-%d\n", i, desc_to_gpio(descs[i]));
+ dev_dbg(dev, "%u => gpio %d irq %d\n", i,
+ desc_to_gpio(descs[i]), gpiod_to_irq(descs[i]));
if (gpiod_cansleep(descs[i]))
chip->can_sleep = true;
@@ -432,6 +440,7 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct
device *dev,
chip->get_multiple = gpio_fwd_get_multiple_locked;
chip->set = gpio_fwd_set;
chip->set_multiple = gpio_fwd_set_multiple_locked;
+ chip->to_irq = gpio_fwd_to_irq;
chip->base = -1;
chip->ngpio = ngpios;
fwd->descs = descs;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
