From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 29 Jul 2021 17:17:28 +0530 From: Viresh Kumar Subject: Re: [PATCH V7 2/2] virtio-gpio: Add support for interrupts Message-ID: <20210729114728.kjdegdbyd67makjo@vireshk-i7> References: <0dac5ab26a5fcc94ec530dbf83247788922123d7.1627469463.git.viresh.kumar@linaro.org> <20210729105957.zem2tbhixoxuxry6@vireshk-i7> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline To: Arnd Bergmann Cc: Jason Wang , "Michael S. Tsirkin" , Cornelia Huck , Linus Walleij , Bartosz Golaszewski , Vincent Guittot , Jean-Philippe Brucker , Bill Mills , Alex =?utf-8?Q?Benn=C3=A9e?= , "Enrico Weigelt, metux IT consult" , virtio-dev@lists.oasis-open.org, Geert Uytterhoeven , Stratos Mailing List , Thomas Gleixner , Marc Zyngier List-ID: On 29-07-21, 13:39, Arnd Bergmann wrote: > On Thu, Jul 29, 2021 at 12:59 PM Viresh Kumar 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