1. Patchew
  2. linux
  3. [PATCH 0/4] irqchip/gic: Use GIC_* DT binding definitions
  4. View patch

[PATCH 3/4] irqchip/gic: Use GIC_* DT binding definitions

Geert Uytterhoeven posted 4 patches 1 month ago
[PATCH 3/4] irqchip/gic: Use GIC_* DT binding definitions
Posted by Geert Uytterhoeven 1 month ago 
Replace magic numbers by symbolic DT binding definitions.  This improves
readability, and makes it easier to find where the various GIC
interrupts types are handled.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-gic-v3.c | 14 ++++++++------
 drivers/irqchip/irq-gic.c    |  6 ++++--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d75163e71bf22473..6ac103cb40097acc 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -35,6 +35,8 @@
 #include <asm/smp_plat.h>
 #include <asm/virt.h>
 
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "irq-gic-common.h"
 
 static u8 dist_prio_irq __ro_after_init = GICV3_PRIO_IRQ;
@@ -1602,25 +1604,25 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 			return -EINVAL;
 
 		switch (fwspec->param[0]) {
-		case 0:			/* SPI */
+		case GIC_SPI:
 			if (fwspec->param[1] > 987)
 				pr_warn_once("SPI %u out of range (use ESPI?)\n",
 					     fwspec->param[1]);
 			*hwirq = fwspec->param[1] + 32;
 			break;
-		case 1:			/* PPI */
+		case GIC_PPI:
 			if (fwspec->param[1] > 16)
 				pr_warn_once("PPI %u out of range (use EPPI?)\n",
 					     fwspec->param[1]);
 			*hwirq = fwspec->param[1] + 16;
 			break;
-		case 2:			/* ESPI */
+		case GIC_ESPI:
 			if (fwspec->param[1] > 1023)
 				pr_warn_once("ESPI %u out of range\n",
 					     fwspec->param[1]);
 			*hwirq = fwspec->param[1] + ESPI_BASE_INTID;
 			break;
-		case 3:			/* EPPI */
+		case GIC_EPPI:
 			if (fwspec->param[1] > 63)
 				pr_warn_once("EPPI %u out of range\n",
 					     fwspec->param[1]);
@@ -1738,8 +1740,8 @@ static int gic_irq_get_fwspec_info(struct irq_fwspec *fwspec, struct irq_fwspec_
 		struct fwnode_handle *fw;
 
 		switch (fwspec->param[0]) {
-		case 1:			/* PPI */
-		case 3:			/* EPPI */
+		case GIC_PPI:
+		case GIC_EPPI:
 			break;
 		default:
 			return 0;
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ec70c84e9f91dd7b..a2225ca1efeb700d 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -47,6 +47,8 @@
 #include <asm/smp_plat.h>
 #include <asm/virt.h>
 
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "irq-gic-common.h"
 
 #ifdef CONFIG_ARM64
@@ -1094,10 +1096,10 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 			return -EINVAL;
 
 		switch (fwspec->param[0]) {
-		case 0:			/* SPI */
+		case GIC_SPI:
 			*hwirq = fwspec->param[1] + 32;
 			break;
-		case 1:			/* PPI */
+		case GIC_PPI:
 			*hwirq = fwspec->param[1] + 16;
 			break;
 		default:
-- 
2.43.0
Re: [PATCH 3/4] irqchip/gic: Use GIC_* DT binding definitions
Posted by Marc Zyngier 1 month ago 
On Wed, 04 Mar 2026 17:21:58 +0000,
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> Replace magic numbers by symbolic DT binding definitions.  This improves
> readability, and makes it easier to find where the various GIC
> interrupts types are handled.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/irqchip/irq-gic-v3.c | 14 ++++++++------
>  drivers/irqchip/irq-gic.c    |  6 ++++--
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index d75163e71bf22473..6ac103cb40097acc 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -35,6 +35,8 @@
>  #include <asm/smp_plat.h>
>  #include <asm/virt.h>
>  
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
>  #include "irq-gic-common.h"
>  
>  static u8 dist_prio_irq __ro_after_init = GICV3_PRIO_IRQ;
> @@ -1602,25 +1604,25 @@ static int gic_irq_domain_translate(struct irq_domain *d,
>  			return -EINVAL;
>  
>  		switch (fwspec->param[0]) {
> -		case 0:			/* SPI */
> +		case GIC_SPI:

I'd rather not do that. I use *numeric* values on purpose, because
that's what the DT *binding* describes, and I have no control over
what lives in that include file (it gets changed without me being even
Cc'd).

So I want to stick to the binding, and not to the interpretation of
it. If you want symbolic values to be used, describe them in the
binding, have a tool to generate the values from the binding, and use
that everywhere.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH 3/4] irqchip/gic: Use GIC_* DT binding definitions
Posted by Geert Uytterhoeven 1 month ago 
Hi Marc,

On Thu, 5 Mar 2026 at 11:13, Marc Zyngier <maz@kernel.org> wrote:
> On Wed, 04 Mar 2026 17:21:58 +0000,
> Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> > Replace magic numbers by symbolic DT binding definitions.  This improves
> > readability, and makes it easier to find where the various GIC
> > interrupts types are handled.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -35,6 +35,8 @@
> >  #include <asm/smp_plat.h>
> >  #include <asm/virt.h>
> >
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> >  #include "irq-gic-common.h"
> >
> >  static u8 dist_prio_irq __ro_after_init = GICV3_PRIO_IRQ;
> > @@ -1602,25 +1604,25 @@ static int gic_irq_domain_translate(struct irq_domain *d,
> >                       return -EINVAL;
> >
> >               switch (fwspec->param[0]) {
> > -             case 0:                 /* SPI */
> > +             case GIC_SPI:
>
> I'd rather not do that. I use *numeric* values on purpose, because
> that's what the DT *binding* describes, and I have no control over
> what lives in that include file (it gets changed without me being even
> Cc'd).
>
> So I want to stick to the binding, and not to the interpretation of
> it. If you want symbolic values to be used, describe them in the
> binding, have a tool to generate the values from the binding, and use
> that everywhere.

This sounds more like a philosophical debate, so I'd like to defer
to the DT maintainers...

About you not being notified: that can be fixed easily ;-)

--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2110,6 +2110,7 @@ F:        arch/arm64/include/asm/arch_gicv3.h
 F:     drivers/irqchip/irq-gic*.[ch]
 F:     include/linux/irqchip/arm-gic*.h
 F:     include/linux/irqchip/arm-vgic-info.h
+F:     include/dt-bindings/interrupt-controller/arm-gic.h

 ARM GENERIC INTERRUPT CONTROLLER V5 DRIVERS
 M:     Lorenzo Pieralisi <lpieralisi@kernel.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 3/4] irqchip/gic: Use GIC_* DT binding definitions
Posted by Marc Zyngier 1 month ago 
On Thu, 05 Mar 2026 10:24:23 +0000,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Marc,
> 
> On Thu, 5 Mar 2026 at 11:13, Marc Zyngier <maz@kernel.org> wrote:
> > On Wed, 04 Mar 2026 17:21:58 +0000,
> > Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> > > Replace magic numbers by symbolic DT binding definitions.  This improves
> > > readability, and makes it easier to find where the various GIC
> > > interrupts types are handled.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > --- a/drivers/irqchip/irq-gic-v3.c
> > > +++ b/drivers/irqchip/irq-gic-v3.c
> > > @@ -35,6 +35,8 @@
> > >  #include <asm/smp_plat.h>
> > >  #include <asm/virt.h>
> > >
> > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +
> > >  #include "irq-gic-common.h"
> > >
> > >  static u8 dist_prio_irq __ro_after_init = GICV3_PRIO_IRQ;
> > > @@ -1602,25 +1604,25 @@ static int gic_irq_domain_translate(struct irq_domain *d,
> > >                       return -EINVAL;
> > >
> > >               switch (fwspec->param[0]) {
> > > -             case 0:                 /* SPI */
> > > +             case GIC_SPI:
> >
> > I'd rather not do that. I use *numeric* values on purpose, because
> > that's what the DT *binding* describes, and I have no control over
> > what lives in that include file (it gets changed without me being even
> > Cc'd).
> >
> > So I want to stick to the binding, and not to the interpretation of
> > it. If you want symbolic values to be used, describe them in the
> > binding, have a tool to generate the values from the binding, and use
> > that everywhere.
> 
> This sounds more like a philosophical debate, so I'd like to defer
> to the DT maintainers...

That's not philosophical.

That's a pragmatic approach to having a common source of information,
and a unique reference. Carrying an extra copy that can be
independently changed is a source of errors, which I've been trying to
reduce in other parts of the kernel (system register description and
encoding, for example).

> 
> About you not being notified: that can be fixed easily ;-)
> 
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2110,6 +2110,7 @@ F:        arch/arm64/include/asm/arch_gicv3.h
>  F:     drivers/irqchip/irq-gic*.[ch]
>  F:     include/linux/irqchip/arm-gic*.h
>  F:     include/linux/irqchip/arm-vgic-info.h
> +F:     include/dt-bindings/interrupt-controller/arm-gic.h

I'm actively trying to *remove* myself from the kernel, not to grab
more stuff.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.