On Mon, Aug 9, 2021 at 9:30 AM Viresh Kumar viresh.kumar@linaro.org wrote:
On 06-08-21, 10:00, Arnd Bergmann wrote:
On Fri, Aug 6, 2021 at 9:44 AM Viresh Kumar via Stratos-dev stratos-dev@op-lists.linaro.org wrote:
On 05-08-21, 15:10, Arnd Bergmann wrote:
I hope this can still be simplified by working out better which state transitions are needed exactly. In particular, I would expect that we can get away with not sending a VIRTIO_GPIO_MSG_IRQ_TYPE for 'mask' state changes at all, but use that only for forcing 'enabled' state changes.
Something like this ?
static void virtio_gpio_irq_mask(struct irq_data *d) { /* Nothing to do here */ }
You'd have to do /something/ here I think, if only setting the flag that we don't want to deliver the next interrupt.
static void virtio_gpio_irq_unmask(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct virtio_gpio *vgpio = gpiochip_get_data(gc);
/* Queue the buffer unconditionally on unmask */ virtio_gpio_irq_prepare(vgpio, d->hwirq);}
And check the flag here to not requeue it if it's masked.
I am not sure I understand why this would be required. If the interrupt is getting disabled, then unmask will not get called here and so we won't requeue anything. Same will happen with threaded handlers where the interrupt gets unmasked at a later point of time.
Ah, right. There is already a flag that gets checked by the caller.
It does feel odd to have an empty 'irq_mask' callback though, so maybe there is still something missing, just not what I thought.
It's probably the result of calling handle_level_irq(), which as you said is closer to what we want, but is not exactly what we need for this protocol.
Arnd