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

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

On 26-07-21, 14:13, Arnd Bergmann wrote:
> On Mon, Jul 26, 2021 at 1:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 26-07-21, 13:10, Arnd Bergmann wrote:
> >
> > > I would still hope that we can combine the irq_type, mask and request
> >
> > request ? you wanted to say "unmask" ?
> 
> No, I meant VIRTIO_GPIO_MSG_IRQ_TYPE,

Ah, I was confused since this request type isn't there anymore.

> VIRTIO_GPIO_MSG_IRQ_{UN,}MASK and virtio_gpio_irq_request messages.
> 
> All three can change the state between 'can interrupt' and 'can not interrupt',
> so you only get notified if all three are in the active state (irq type != none,
> mask disabled, request queued).
> 
> > > steps into either one or two steps. I think the minimalistic approach would
> > > be to expand the message to include both the "mask" and "set-type"
> > > operations, and this should work, as long as we define sensible semantics.
> >
> > I think we can easily merge unmask and set-type into a single message,
> > since that will fit within the request structure used for most of the
> > requests on the requestq. i.e. a single byte of data.
> >
> > mask() doesn't require irq-type, so it can live separately.
> 
> The question is whether it adds any value though. As far as I can tell,
> it is completely redundant, duplicating either what is implied by
> irq type==none, or what is implied by the request not being queued.

Right, we shouldn't have redundant stuff in here.

Though, binding of all interrupt operations to the eventq can be trick
IMO.

- How will the driver mask an interrupt for which the buffer is
  already sent over the eventq? There is no API available to recall a
  buffer. So we would need an API over the requestq for that.

- Latching of edge interrupt between the buffer is returned by the
  device and requeued by the driver. How will the device know if the
  user has simply requeued the eventq buffer (where the latched value
  is meaningful) or it has done something like free_irq() followed by
  request_irq() in the kernel (where the latched value must be
  discarded).

-- 
viresh