Florian Fainelli <f.faine...@gmail.com> writes: > On 07/25/2017 06:29 AM, Måns Rullgård wrote: >> Marc Gonzalez <marc_gonza...@sigmadesigns.com> writes: >> >>> On 25/07/2017 15:16, Måns Rullgård wrote: >>> >>>> What happened to the patch adding the proper combined function? >>> >>> It appears you're not CCed on v2. >>> >>> https://patchwork.kernel.org/patch/9859799/ >>> >>> Doug wrote: >>>> Yes, you understand correctly. The irq_mask_ack method is entirely >>>> optional and I assume that is why this issue went undetected for so >>>> long; however, it is slightly more efficient to combine the functions >>>> (even if the ack is unnecessary) which is why I chose to do so for my >>>> changes to the irqchip-brcmstb-l2 driver where I first discovered this >>>> issue. How much value the improved efficiency has is certainly >>>> debatable, but interrupt handling is one area where people might care >>>> about such a small difference. As the irqchip-tango driver maintainer >>>> you are welcome to decide whether or not the irq_mask_ack method makes >>>> sense to you. >>> >>> My preference goes to leaving the irq_mask_ack callback undefined, >>> and let the irqchip framework use irq_mask and irq_ack instead. >> >> Why would you prefer the less efficient way? >> > > Same question here, that does not really make sense to me. > > The whole point of this patch series is to have a set of efficient and > bugfree (or nearly) helper functions that drivers can rely on, are you > saying that somehow using irq_mask_and_ack is exposing a bug in the > tango irqchip driver and using the separate functions does not expose > this bug?
There is currently a bug in that the function used doesn't do what its name implies which can't be good. Using the separate mask and ack functions obviously works, but combining them saves a lock/unlock sequence. The correct combined function has already been written, so I see no reason not to use it. -- Måns Rullgård
