Hi Thomas,
On 08/04/2016 12:05 PM, Thomas Gleixner wrote:
> On Tue, 2 Aug 2016, Sebastian Frias wrote:
>
>> Use IRQ_TYPE_SENSE_MASK on irq_domain_xlate_onetwocell()
>>
>> According to the xlate() callback definition, the 'out_type' parameter
>> needs to be the "linux irq type". A mask for such bits exists,
>> IRQ_TYPE_SENSE_MASK, and as a matter of fact it is used in
>> irq_domain_xlate_twocell(), use it for irq_domain_xlate_onetwocell() as
>> well.
>>
>> Fixes: 16b2e6e2f31d ("irq_domain: Create common xlate functions that device
>> drivers can use")
>
> You seem to be obsessed by adding "Fixes" tags and 'Fix' to the $subject, but
> your changelog just babbles about making the code the same as in
> irq_domain_xlate_twocell() and avoids carefully to explain what is broken,
> what consequences that has and what the fix is.
I'm sorry but I think your comment above is unfair, so you could have omitted
it entirely.
>
> Now if you look at the call site which uses the xlate functions then you'll
> notice that we have a sanity check there already:
>
> if (irq_domain_translate(domain, fwspec, &hwirq, &type))
> return 0;
> /*
> * WARN if the irqchip returns a type with bits
> * outside the sense mask set and clear these bits.
> */
> if (WARN_ON(type & ~IRQ_TYPE_SENSE_MASK))
> type &= IRQ_TYPE_SENSE_MASK;
>
(for what is worth, my patch was based on a recent 4.7 that does not has the
code you mention above)
> So we better remove the masking in the xlate functions completely and let all
> the offending device trees trip over the warning so they get eventually fixed.
That's fine by me.
If you want I can submit a patch that does what you want (if so, should I use
"fixes" or not?), let me know.
Best regards,
Sebastian
