Subject: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On 03.12.20 20:11, Enrico Weigelt, metux IT consult wrote:

Friends,

I've still got a problem w/ signal/irq handling:

The virtio-gpio device/host can raise a signal on line state change.
Kinda IRQ, but not actually running through real IRQs, instead by a
message running though queue. (hmm, kida MSI ? :o).

I've tried allocating an IRQ range and calling generic_handle_irq(),
but then I'm getting unhanled IRQ trap.

My hope was some gpio lib function for calling in when an line state
changes, that does all the magic (somebody listening on some gpio,
or gpio used as interrupt source), but the only thing I could find
was some helpers for gpio chips that have their own builtin
interrupt controller (VIRTIO_GPIO_EV_HOST_LEVEL).

Somehow feels that's not quite what I'm looking for.

Could anybody please give me more insights ?


--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287


2020-12-08 09:43:58

by Linus Walleij

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Sat, Dec 5, 2020 at 9:15 PM Enrico Weigelt, metux IT consult
<[email protected]> wrote:

> The virtio-gpio device/host can raise a signal on line state change.
> Kinda IRQ, but not actually running through real IRQs, instead by a
> message running though queue. (hmm, kida MSI ? :o).
>
> I've tried allocating an IRQ range and calling generic_handle_irq(),
> but then I'm getting unhanled IRQ trap.

This is Bartosz territory, but the gpio-mockup.c driver will insert
IRQs into the system, he went and added really core stuff
into kernel/irq to make this happen. Notice that in Kconfig
it does:

select IRQ_SIM

Then this is used:
include/linux/irq_sim.h

This is intended for simulating IRQs and both GPIO and IIO use it.
I think this inserts IRQs from debugfs and I have no idea how
flexible that is.

If it is suitable for what you want to do I don't know but it's
virtio so...

Yours,
Linus Walleij

Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On 08.12.20 10:38, Linus Walleij wrote:

Hi,

> This is Bartosz territory, but the gpio-mockup.c driver will insert
> IRQs into the system, he went and added really core stuff
> into kernel/irq to make this happen. Notice that in Kconfig
> it does:
>
> select IRQ_SIM
>
> Then this is used:
> include/linux/irq_sim.h
>
> This is intended for simulating IRQs and both GPIO and IIO use it.
> I think this inserts IRQs from debugfs and I have no idea how
> flexible that is.

Oh, thx.

It seems to implement a pseudo-irqchip driver. I've already though about
doing that, but didn't think its worth it, just for my driver alone.
I've implemented a few irq handling cb's directly the driver. But since
we already have it, I'll reconsider :)

BUT: this wasn't exactly my question :p

I've been looking for some more direct notification callback for gpio
consumers: here the consumer would register itself as a listener on
some gpio_desc and called back when something changes (with data what
exactly changed, eg. "gpio #3 input switched to high").

Seems we currently just have the indirect path via interrupts.

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2020-12-08 16:23:01

by Grygorii Strashko

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver



On 08/12/2020 16:04, Enrico Weigelt, metux IT consult wrote:
> On 08.12.20 10:38, Linus Walleij wrote:
>
> Hi,
>
>> This is Bartosz territory, but the gpio-mockup.c driver will insert
>> IRQs into the system, he went and added really core stuff
>> into kernel/irq to make this happen. Notice that in Kconfig
>> it does:
>>
>> select IRQ_SIM
>>
>> Then this is used:
>> include/linux/irq_sim.h
>>
>> This is intended for simulating IRQs and both GPIO and IIO use it.
>> I think this inserts IRQs from debugfs and I have no idea how
>> flexible that is.
>
> Oh, thx.
>
> It seems to implement a pseudo-irqchip driver. I've already though about
> doing that, but didn't think its worth it, just for my driver alone.
> I've implemented a few irq handling cb's directly the driver. But since
> we already have it, I'll reconsider :)
>
> BUT: this wasn't exactly my question :p
>
> I've been looking for some more direct notification callback for gpio
> consumers: here the consumer would register itself as a listener on
> some gpio_desc and called back when something changes (with data what
> exactly changed, eg. "gpio #3 input switched to high").

But this is exactly why there is GPIO IRQs in the first place,
which are used to notify consumers.

More over most consumers doesn't know where the IRQ came from - on one HW it can be gpio,
while on another HW - direct interrupt controller line.

--
Best regards,
grygorii

2020-12-09 08:56:27

by Linus Walleij

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult
<[email protected]> wrote:

> I've been looking for some more direct notification callback for gpio
> consumers: here the consumer would register itself as a listener on
> some gpio_desc and called back when something changes (with data what
> exactly changed, eg. "gpio #3 input switched to high").
>
> Seems we currently just have the indirect path via interrupts.

I don't know how indirect it is, it seems pretty direct to me. The subsystem
was designed in response to how the hardware in front of the developers
worked.

So far we have had:
- Cascaded interrupts
- Dedicated (hieararchical) interrupts
- Message Signalled Interrupts

And if you now bring something else to the show then it's not like the
subsystem was designed for some abstract quality such as
generic notification of events that occurred, all practical instances
have been around actual IRQs and that is why it is using
struct irq_chip.

What we need to understand is if your new usecase is an outlier
so it is simplest modeled by a "mock" irq_chip or we have to design
something new altogether like notifications on changes. I suspect
irq_chip would be best because all drivers using GPIOs for interrupts
are expecting interrupts, and it would be an enormous task to
change them all and really annoying to create a new mechanism
on the side.

Yours,
Linus Walleij

2020-12-09 12:56:37

by Linus Walleij

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Wed, Dec 9, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
> On Wed, Dec 9, 2020 at 9:51 AM Linus Walleij <[email protected]> wrote:
> > On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult <[email protected]> wrote:
>
> > What we need to understand is if your new usecase is an outlier
> > so it is simplest modeled by a "mock" irq_chip or we have to design
> > something new altogether like notifications on changes. I suspect
> > irq_chip would be best because all drivers using GPIOs for interrupts
> > are expecting interrupts, and it would be an enormous task to
> > change them all and really annoying to create a new mechanism
> > on the side.
>
> I would expect the platform abstraction to actually be close enough
> to a chained irqchip that it actually works: the notification should
> come in via vring_interrupt(), which is a normal interrupt handler
> that calls vq->vq.callback(), calling generic_handle_irq() (and
> possibly chained_irq_enter()/chained_irq_exit() around it) like the
> other gpio drivers do should just work here I think, and if it did
> not, then I would expect this to be just a bug in the driver rather
> than something missing in the gpio framework.

Performance/latency-wise that would also be strongly encouraged.

Tglx isn't super-happy about the chained interrupts at times, as they
can create really nasty bugs, but a pure IRQ in fastpath of some
kinde is preferable and intuitive either way.

Yours,
Linus Walleij

2020-12-09 20:46:57

by Grygorii Strashko

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver



On 09/12/2020 14:53, Linus Walleij wrote:
> On Wed, Dec 9, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
>> On Wed, Dec 9, 2020 at 9:51 AM Linus Walleij <[email protected]> wrote:
>>> On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult <[email protected]> wrote:
>>
>>> What we need to understand is if your new usecase is an outlier
>>> so it is simplest modeled by a "mock" irq_chip or we have to design
>>> something new altogether like notifications on changes. I suspect
>>> irq_chip would be best because all drivers using GPIOs for interrupts
>>> are expecting interrupts, and it would be an enormous task to
>>> change them all and really annoying to create a new mechanism
>>> on the side.
>>
>> I would expect the platform abstraction to actually be close enough
>> to a chained irqchip that it actually works: the notification should
>> come in via vring_interrupt(), which is a normal interrupt handler
>> that calls vq->vq.callback(), calling generic_handle_irq() (and
>> possibly chained_irq_enter()/chained_irq_exit() around it) like the
>> other gpio drivers do should just work here I think, and if it did
>> not, then I would expect this to be just a bug in the driver rather
>> than something missing in the gpio framework.
>
> Performance/latency-wise that would also be strongly encouraged.
>
> Tglx isn't super-happy about the chained interrupts at times, as they
> can create really nasty bugs, but a pure IRQ in fastpath of some
> kinde is preferable and intuitive either way.

In my opinion the problem here is that proposed patch somehow describes Front end, but
says nothing about Backend and overall design.

What is expected to be virtualized? whole GPIO chip? or set of GPIOs from different GPIO chips?
Most often nobody want to give Guest access to the whole GPIO chip, so, most probably, smth. similar to
GPIO Aggregator will be needed.

How is situation going to be resolved that GPIO framework and IRQ framework are independent, but intersect, and
GPIO controller drivers have no idea who and how requesting GPIO IRQ - threaded vs non-threaded vs oneshot.
So, some information need to be transferred to Back end - at minimum IRQ triggering type.

Overall, it might be better to start from pure gpio and leave GPIO IRQ aside.
--
Best regards,
grygorii

2020-12-09 23:07:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Wed, Dec 9, 2020 at 9:51 AM Linus Walleij <[email protected]> wrote:
> On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult <[email protected]> wrote:

> What we need to understand is if your new usecase is an outlier
> so it is simplest modeled by a "mock" irq_chip or we have to design
> something new altogether like notifications on changes. I suspect
> irq_chip would be best because all drivers using GPIOs for interrupts
> are expecting interrupts, and it would be an enormous task to
> change them all and really annoying to create a new mechanism
> on the side.

I would expect the platform abstraction to actually be close enough
to a chained irqchip that it actually works: the notification should
come in via vring_interrupt(), which is a normal interrupt handler
that calls vq->vq.callback(), calling generic_handle_irq() (and
possibly chained_irq_enter()/chained_irq_exit() around it) like the
other gpio drivers do should just work here I think, and if it did
not, then I would expect this to be just a bug in the driver rather
than something missing in the gpio framework.

Arnd

2020-12-09 23:53:37

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Wed, Dec 9, 2020 at 9:22 PM Grygorii Strashko
<[email protected]> wrote:
> On 09/12/2020 14:53, Linus Walleij wrote:
> > On Wed, Dec 9, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
> >> On Wed, Dec 9, 2020 at 9:51 AM Linus Walleij <[email protected]> wrote:
> >>> On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult <[email protected]> wrote:
> >>
> >>> What we need to understand is if your new usecase is an outlier
> >>> so it is simplest modeled by a "mock" irq_chip or we have to design
> >>> something new altogether like notifications on changes. I suspect
> >>> irq_chip would be best because all drivers using GPIOs for interrupts
> >>> are expecting interrupts, and it would be an enormous task to
> >>> change them all and really annoying to create a new mechanism
> >>> on the side.
> >>
> >> I would expect the platform abstraction to actually be close enough
> >> to a chained irqchip that it actually works: the notification should
> >> come in via vring_interrupt(), which is a normal interrupt handler
> >> that calls vq->vq.callback(), calling generic_handle_irq() (and
> >> possibly chained_irq_enter()/chained_irq_exit() around it) like the
> >> other gpio drivers do should just work here I think, and if it did
> >> not, then I would expect this to be just a bug in the driver rather
> >> than something missing in the gpio framework.
> >
> > Performance/latency-wise that would also be strongly encouraged.
> >
> > Tglx isn't super-happy about the chained interrupts at times, as they
> > can create really nasty bugs, but a pure IRQ in fastpath of some
> > kinde is preferable and intuitive either way.
>
> In my opinion the problem here is that proposed patch somehow describes Front end, but
> says nothing about Backend and overall design.
>
> What is expected to be virtualized? whole GPIO chip? or set of GPIOs from different GPIO chips?
> Most often nobody want to give Guest access to the whole GPIO chip, so, most probably, smth. similar to
> GPIO Aggregator will be needed.

I would argue that it does not matter, the virtual GPIO chip could really
be anything. Certain functions such as a gpio based keyboard require
interrupts, so it sounds useful to make them work.

Arnd

2020-12-10 14:46:30

by Grygorii Strashko

[permalink] [raw]
Subject: Re: Howto listen to/handle gpio state changes ? Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver



On 09/12/2020 22:38, Arnd Bergmann wrote:
> On Wed, Dec 9, 2020 at 9:22 PM Grygorii Strashko
> <[email protected]> wrote:
>> On 09/12/2020 14:53, Linus Walleij wrote:
>>> On Wed, Dec 9, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
>>>> On Wed, Dec 9, 2020 at 9:51 AM Linus Walleij <[email protected]> wrote:
>>>>> On Tue, Dec 8, 2020 at 3:07 PM Enrico Weigelt, metux IT consult <[email protected]> wrote:
>>>>
>>>>> What we need to understand is if your new usecase is an outlier
>>>>> so it is simplest modeled by a "mock" irq_chip or we have to design
>>>>> something new altogether like notifications on changes. I suspect
>>>>> irq_chip would be best because all drivers using GPIOs for interrupts
>>>>> are expecting interrupts, and it would be an enormous task to
>>>>> change them all and really annoying to create a new mechanism
>>>>> on the side.
>>>>
>>>> I would expect the platform abstraction to actually be close enough
>>>> to a chained irqchip that it actually works: the notification should
>>>> come in via vring_interrupt(), which is a normal interrupt handler
>>>> that calls vq->vq.callback(), calling generic_handle_irq() (and
>>>> possibly chained_irq_enter()/chained_irq_exit() around it) like the
>>>> other gpio drivers do should just work here I think, and if it did
>>>> not, then I would expect this to be just a bug in the driver rather
>>>> than something missing in the gpio framework.
>>>
>>> Performance/latency-wise that would also be strongly encouraged.
>>>
>>> Tglx isn't super-happy about the chained interrupts at times, as they
>>> can create really nasty bugs, but a pure IRQ in fastpath of some
>>> kinde is preferable and intuitive either way.
>>
>> In my opinion the problem here is that proposed patch somehow describes Front end, but
>> says nothing about Backend and overall design.
>>
>> What is expected to be virtualized? whole GPIO chip? or set of GPIOs from different GPIO chips?
>> Most often nobody want to give Guest access to the whole GPIO chip, so, most probably, smth. similar to
>> GPIO Aggregator will be needed.
>
> I would argue that it does not matter, the virtual GPIO chip could really
> be anything. Certain functions such as a gpio based keyboard require
> interrupts, so it sounds useful to make them work.

Agree, and my point was not to discard IRQ support, but solve problem step by step.
And existing Back end, in any form, may just help to understand virtio-gpio protocol spec and
identify missed pieces.

For example, should 'Configuration space' specify if IRQs are supported on not?

--
Best regards,
grygorii