Re: [PATCH V5 2/2] virtio-gpio: Add support for interrupts

[Viresh Kumar <viresh.kumar@linaro.org>]

On 20-07-21, 09:17, Arnd Bergmann wrote:
> I don't think you can have a message for more than one line here,
> that would mess up the synchronization as then you have to
> keep double accounting of which gpios are already pending
> on each side.

Yes.

> The most sensible way I see this can be done is to have a message
> per line, and have it as a token that is either on the device or the
> driver side at any point. When the driver has sent the token over
> to the device, the irq is armed, and when the device replies, it is
> implicitly masked.

I like the idea in general but at this point I am not very sure how it
will work. Looking at kernel (and qemu) side implementations, what
really transfers between device and driver is a series of buffers
(marked as in or out sgs). Now these are either device writable or
driver. I am not sure if the device is required to write anything here
to the buffer, it can simply return the buffer as is and so we can
make it driver writable only, i.e. only 1 sg element required.

> Ideally this could even replace the VIRTIO_GPIO_MSG_IRQ_TYPE,
> VIRTIO_GPIO_MSG_IRQ_MASK

Maybe yes.

> and VIRTIO_GPIO_MSG_IRQ_UNMASK

I think we would still require some kind of unmasking here as the
driver would have already sent the buffer to the device and wants the
ownership back, otherwise it may hit an interrupt later on.

Although I would like to keep the three routines separate, just to be
robust here. For example what will we do if the user does this (yes it
won't do it, but I am trying to handle the corner cases well):

1. irq-type-edge-falling
2. irq-unmask
3. irq-type-edge-both

Since we would have already sent the buffer at (2), we can't do (3)
without getting the buffer back.

Tying everything with availability of the buffer itself makes it kind
of messy.

> messages with a single message type on the interrupt virtqueue:
> 
> struct virtio_gpio_irq_event {
>       __le16 line;
>       __u8 type;
>       __u8 status;
> }
> 
> When the driver wants an event on a gpio line, it enqueues this
> message to the virtqueue, and then the device either adds the
> corresponding file descriptor to its poll table, or replies immediately
> with one of these status words:
> 
> - request not supported
> - level interrupt is still active (line remains at requested level)
> - edge interrupt has happened since last reply

-- 
viresh