Skip to contentSkip to conversation

irqdomain: fix irq_domain_xlate_onetwocell()

PATCH In review Sebastian Frias 2 participants 4 messages Aug 2, 2016 – Aug 4, 2016

Subsystems (2)
IRQ IRQ domains (IRQ number mapping library)
lore
Share & tools
lore

Sebastian Frias

author
#0 lore raw

PATCH 1/2 irqdomain: fix irq_domain_xlate_onetwocell()

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")

Signed-off-by Sebastian Frias <[email protected]>
---
 kernel/irq/irqdomain.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
kernel/irq/irqdomain.c +4 -1
index 8798b6c..1bdd3fe 100644
@@ -824,7 +824,10 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
 	if (WARN_ON(intsize < 1))
 		return -EINVAL;
 	*out_hwirq = intspec[0];
-	*out_type = (intsize > 1) ? intspec[1] : IRQ_TYPE_NONE;
+	if (intsize > 1)
+		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+	else
+		*out_type = IRQ_TYPE_NONE;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(irq_domain_xlate_onetwocell);
Signature · 1 line
1.7.11.2
#1 lore raw
Quoted 10 lines · Sebastian Frias

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.

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;

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.

Thanks,

tglx

author
#2 lore raw

Hi Thomas,

Quoted 10 lines · Thomas Gleixner
Quoted 8 lines · Sebastian Frias

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.

Quoted 7 lines
Quoted 2 lines
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.

Quoted 4 lines

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:

Quoted 9 lines

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)

Quoted 2 lines

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

#3 lore raw
Quoted 3 lines · Sebastian Frias

On 08/04/2016 12:05 PM, Thomas Gleixner wrote:
(for what is worth, my patch was based on a recent 4.7 that does not has the code you mention above)

In future, please always check against mainline or the head of the relevant maintainer tree.

Quoted 6 lines
Quoted 2 lines

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.

If it is broken then it wants a fixes tag plus an explanation WHY it is broken, what consequences the breakage has etc.

If it's just a regular code improvement then not.

Decide yourself.

Thanks,

tglx