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

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

On 29-07-21, 13:39, Arnd Bergmann wrote:
> On Thu, Jul 29, 2021 at 12:59 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 28-07-21, 14:05, Arnd Bergmann wrote:
> > > I think the last sentence here is somewhat ambiguous, I'm not sure
> > > what it actually means to unmask a level triggered line if the driver has not
> > > queued an event request for that line.
> >
> > I think I now understand what you meant by this, I wasn't thinking of
> > fasteoi earlier :)
> >
> > What about following diff over this patch:
> >
> >  The device MUST mask the interrupt for a GPIO line, if the trigger type is set
> >  to \field{VIRTIO_GPIO_IRQ_TYPE_NONE} and also discard any latched interrupt
> > -status associated with the line. The device MUST unmask the interrupt for any
> > -other value of the trigger type.
> > +event associated with the line. The device MUST NOT notify the driver of an
> > +interrupt event if the trigger type is set to \field{VIRTIO_GPIO_IRQ_TYPE_NONE}
> > +for the GPIO line.
> 
> I actually thought that setting VIRTIO_GPIO_IRQ_TYPE_NONE would
> return the buffer to the driver, with status indicating that no event
> has happened.

Yeah, that is actually the case. What I meant here was that an
interrupt must not be reported by the device in this case, but yes the
buffer needs to be returned. Will clarify on that a bit here.

> > +For level trigger type, the device only reports an interrupt if the
> > +corresponding interrupt level was sensed when the buffers are received.
> 
> I think you mean the right thing, but this part is very misleading: we
> clearly want
> to be notified if the line was active when the buffer is received and also as
> soon as it becomes active afterwards, just not when it was active in the past.

Right.

> The rest looks good to me now, but it's a little hard to read in diff form.
> Are you able to post the driver soon? I think our understanding of how this
> should work has converged enough that I'd review that next.

I should be able to post that somewhere next week, making changes
according to spec changes.

-- 
viresh