irqchip/gic: Use GIC_SPI symbolic constant

Use GIC_SPI explicitly instead of an implicit 0.

---
FWIW, 'make C=2' flagged a few lines:

$ make C=2 drivers/irqchip/irq-gic.o
  CHECK   drivers/irqchip/irq-gic.c
drivers/irqchip/irq-gic.c:1079:44: warning: incorrect type in assignment (different address spaces)
drivers/irqchip/irq-gic.c:1079:44:    expected void [noderef] <asn:3>*[noderef] <asn:2>*percpu_base
drivers/irqchip/irq-gic.c:1079:44:    got void [noderef] <asn:2>*[noderef] <asn:3>*<noident>
drivers/irqchip/irq-gic.c:1080:43: warning: incorrect type in assignment (different address spaces)
drivers/irqchip/irq-gic.c:1080:43:    expected void [noderef] <asn:3>*[noderef] <asn:2>*percpu_base
drivers/irqchip/irq-gic.c:1080:43:    got void [noderef] <asn:2>*[noderef] <asn:3>*<noident>
drivers/irqchip/irq-gic.c:1091:26: warning: incorrect type in initializer (different address spaces)
drivers/irqchip/irq-gic.c:1091:26:    expected void const [noderef] <asn:3>*__vpp_verify
drivers/irqchip/irq-gic.c:1091:26:    got void [noderef] <asn:3>*[noderef] <asn:2>*<noident>
drivers/irqchip/irq-gic.c:1091:71: warning: incorrect type in assignment (different address spaces)
drivers/irqchip/irq-gic.c:1091:71:    expected void [noderef] <asn:3>*<noident>
drivers/irqchip/irq-gic.c:1091:71:    got void [noderef] <asn:2>*
drivers/irqchip/irq-gic.c:1093:26: warning: incorrect type in initializer (different address spaces)
drivers/irqchip/irq-gic.c:1093:26:    expected void const [noderef] <asn:3>*__vpp_verify
drivers/irqchip/irq-gic.c:1093:26:    got void [noderef] <asn:3>*[noderef] <asn:2>*<noident>
drivers/irqchip/irq-gic.c:1093:70: warning: incorrect type in assignment (different address spaces)
drivers/irqchip/irq-gic.c:1093:70:    expected void [noderef] <asn:3>*<noident>
drivers/irqchip/irq-gic.c:1093:70:    got void [noderef] <asn:2>*
drivers/irqchip/irq-gic.c:1167:43: warning: incorrect type in argument 1 (different address spaces)
drivers/irqchip/irq-gic.c:1167:43:    expected void [noderef] <asn:3>*__pdata
drivers/irqchip/irq-gic.c:1167:43:    got void [noderef] <asn:3>*[noderef] <asn:2>*percpu_base
drivers/irqchip/irq-gic.c:1168:42: warning: incorrect type in argument 1 (different address spaces)
drivers/irqchip/irq-gic.c:1168:42:    expected void [noderef] <asn:3>*__pdata
drivers/irqchip/irq-gic.c:1168:42:    got void [noderef] <asn:3>*[noderef] <asn:2>*percpu_base
---
 drivers/irqchip/irq-gic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 1b1df4f770bd..ae414f5223b6 100644
@@ -42,6 +42,8 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/arm-gic.h>
 
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include <asm/cputype.h>
 #include <asm/irq.h>
 #include <asm/exception.h>

@@ -990,7 +992,7 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 		 * For SPIs, we need to add 16 more to get the GIC irq
 		 * ID number
 		 */
-		if (!fwspec->param[0])
+		if (fwspec->param[0] == GIC_SPI)
 			*hwirq += 16;
 
 		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;

2.8.2

What bug is this fixing? What benefit does this bring?

How is that related to this patch?

	M.

Jazz is not dead. It just smells funny.

The patch aims to replace an (implicit) literal constant with the corresponding symbolic macro. IMO, it makes the intent somewhat clearer, and, more importantly, grepping for said symbol now returns the respective file/line. (It took me a while to find the line.)

Are you saying that changing the code at this point is not worth the trouble?

I tested the patch with 'make C=2' and reported the output FWIW. Are you saying I should have written a separate message, or not bothered altogether?

Regards.

You're assuming that this GIC_SPI macro has anything to do with the GIC driver. It doesn't. That's just a convenience macro for people writing DT, and definitely not something I'd ever want to rely on in the Linux driver. The binding defines the raw value, and not this macro.

So to sum it up, thank you, but no thank out.

An even better outcome would be analysing the issue and coming up with a patch if there is something to fix. On its own, dumping these warnings in the middle of something unrelated is a best at waste of bandwidth.

	M.

Jazz is not dead. It just smells funny.

AFAIU, you're referring to Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt "The 1st cell is the interrupt type;
0 for SPI interrupts, 1 for PPI interrupts."

Doesn't it make sense to have symbolic constants for the 0 and 1 above? In linux/irqchip/arm-gic.h ?

IIUC, headers in include/dt-bindings are just convenience macros, and are not to be included from driver code?

Regards.

Hello,

FWIW, a few drivers are already using GIC_SPI:

irq-mvebu-gicp.c:       fwspec.param[0] = GIC_SPI;
irq-mvebu-odmi.c:       fwspec.param[0] = GIC_SPI;
irq-tegra.c:    if (fwspec->param[0] != GIC_SPI)
irq-vf610-mscm-ir.c:            parent_fwspec.param[0] = GIC_SPI;

For pretty much exactly the situation being patched by Mason.

Best regards,

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com