On Mon, Aug 9, 2021 at 12:47 PM Viresh Kumar via Stratos-dev stratos-dev@op-lists.linaro.org wrote:
On 09-08-21, 09:55, Arnd Bergmann wrote:
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.
Okay, I have tried to take care of locking as well now and used local flags only to make sure I can depend on them to get the locking working properly. Lets see what's broken in this now :)
I don't see anything wrong with this version, but let's see what Marc thinks. I expect that he can still poke some holes in it, or at least find some simplifications.
I was slightly surprised at the relation between the disabled and masked states, where 'disable' always implies 'mask' and 'enable' always implies 'unmask', but I don't actually know how those two are actually defined in the irqchip code in Linux, so I assume you did this correctly.
Arnd
On 09-08-21, 13:19, Arnd Bergmann wrote:
I don't see anything wrong with this version,
Great.
but let's see what Marc thinks. I expect that he can still poke some holes in it, or at least find some simplifications.
Right, though I may resend the patches properly first to make it less confusing.
I was slightly surprised at the relation between the disabled and masked states, where 'disable' always implies 'mask' and 'enable' always implies 'unmask', but I don't actually know how those two are actually defined in the irqchip code in Linux, so I assume you did this correctly.
I did have a look at the irq-core, but didn't go in real depth. I rather saw how stuff happens at the driver's end.
- On setup-irq, the core only calls enable() and not unmask(). - On interrupt, the core calls mask(), followed by unmask() (which can be delayed for threaded irqs). - On disable_irq(), the core only calls disable(). - On enable_irq(), the core only calls enable().
And so I ended up at this version :)
stratos-dev@op-lists.linaro.org
-
Arnd Bergmann -
Viresh Kumar