[PATCH 0/6] irqchip/al-fic: shared parent IRQ, FIC v2/v3 and affinity

[PATCH 0/6] irqchip/al-fic: shared parent IRQ, FIC v2/v3 and affinity

[Eliav Farber]

This series extends the Amazon Annapurna Labs Fabric Interrupt Controller
(FIC) driver to support newer hardware revisions and configurations found
on later Annapurna Labs SoCs, plus a couple of smaller fixes.

FIC v2 and later add two interrupt groups alongside the existing legacy
group - an "error" group and a "fatal" group - each described by its own
device tree node and matched by a distinct compatible string. On some
platforms several FIC instances also share a single parent GIC SPI, which
the driver did not previously support.

The series is organised as follows:

  1. Use the full node name in log messages and raise the successful-init
     message to pr_info, so each FIC instance is identifiable at boot.
  2. Request the parent interrupt as shared (IRQF_SHARED) instead of a
     chained handler, so multiple FIC instances can share one GIC SPI.
  3. Extend the binding with the amazon,al-fic-error and amazon,al-fic-fatal
     compatibles for the new groups.
  4. Add FIC v2 support: version detection, the error/fatal groups, and a
     mask_cache workaround for a v2 erratum where the error and fatal mask
     registers always read back as 0.
  5. Add FIC v3 support. v3 shares v2's group layout but fixes the mask
     read-back erratum, so the v2 workaround is deliberately not applied.
  6. Add an irq_set_affinity callback so drivers that call
     irq_set_affinity() (which fails with -EINVAL when the irqchip has no
     such callback) can probe when routed through a FIC instance.

Eliav Farber (6):
  irqchip/al-fic: use full node name and raise init log level
  irqchip/al-fic: switch to shared parent interrupt
  dt-bindings: interrupt-controller: amazon,al-fic: add error/fatal
    groups
  irqchip/al-fic: add support for FIC v2
  irqchip/al-fic: add support for FIC v3
  irqchip/al-fic: add irq_set_affinity callback

 .../interrupt-controller/amazon,al-fic.yaml   |   5 +-
 drivers/irqchip/irq-al-fic.c                  | 176 +++++++++++++++---
 2 files changed, 150 insertions(+), 31 deletions(-)


base-commit: 6828157ab3e96cd1c957b78d995c5e508a2eb2c2
-- 
2.47.3

[PATCH 1/6] irqchip/al-fic: use full node name and raise init log level

[Eliav Farber]

Use node->full_name instead of node->name in the driver's log messages
so that they identify the specific FIC instance, e.g.:

  interrupt-controller@fd803000 initialized successfully in Legacy mode (parent-irq=3)

Raise the successful-initialization message from pr_debug to pr_info, as
this single boot-time printout is useful when dynamic debug is not
enabled at this stage of boot. While here, fix a small typo in the
al_fic_wire_init() comment.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 drivers/irqchip/irq-al-fic.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index d10ac9b63c99..efa7f2b62d00 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -180,7 +180,7 @@ static int al_fic_register(struct device_node *node,
  * @name: name of the fic
  * @parent_irq: interrupt of parent
  *
- * This API will configure the fic hardware to to work in wire mode.
+ * This API will configure the fic hardware to work in wire mode.
  * In wire mode, fic hardware is generating a wire ("wired") interrupt.
  * Interrupt can be generated based on positive edge or level - configuration is
  * to be determined based on connected hardware to this fic.
@@ -216,8 +216,8 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 		goto err_free;
 	}
 
-	pr_debug("%s initialized successfully in Legacy mode (parent-irq=%u)\n",
-		 fic->name, parent_irq);
+	pr_info("%s initialized successfully in Legacy mode (parent-irq=%u)\n",
+		fic->name, parent_irq);
 
 	return fic;
 
@@ -236,30 +236,30 @@ static int __init al_fic_init_dt(struct device_node *node,
 
 	if (!parent) {
 		pr_err("%s: unsupported - device require a parent\n",
-		       node->name);
+		       node->full_name);
 		return -EINVAL;
 	}
 
 	base = of_iomap(node, 0);
 	if (!base) {
-		pr_err("%s: fail to map memory\n", node->name);
+		pr_err("%s: fail to map memory\n", node->full_name);
 		return -ENOMEM;
 	}
 
 	parent_irq = irq_of_parse_and_map(node, 0);
 	if (!parent_irq) {
-		pr_err("%s: fail to map irq\n", node->name);
+		pr_err("%s: fail to map irq\n", node->full_name);
 		ret = -EINVAL;
 		goto err_unmap;
 	}
 
 	fic = al_fic_wire_init(node,
 			       base,
-			       node->name,
+			       node->full_name,
 			       parent_irq);
 	if (IS_ERR(fic)) {
 		pr_err("%s: fail to initialize irqchip (%lu)\n",
-		       node->name,
+		       node->full_name,
 		       PTR_ERR(fic));
 		ret = PTR_ERR(fic);
 		goto err_irq_dispose;
-- 
2.47.3

[PATCH 2/6] irqchip/al-fic: switch to shared parent interrupt

[Eliav Farber]

Until now the driver requested its parent interrupt using the chained
IRQ API (irq_set_chained_handler_and_data()), which only works when each
parent interrupt is wired to a single FIC instance.

On some platforms several FIC instances share the same parent GIC SPI.
To support that, request the parent interrupt as a shared interrupt
(IRQF_SHARED) instead of installing a chained handler. The handler now
has the standard irqreturn_t prototype and reports whether it handled any
child interrupt, so the shared-IRQ core can dispatch to the correct
instance.

generic_handle_domain_irq() is retained for dispatch; its return value is
used to determine whether a pending child was actually handled so the
handler can return IRQ_HANDLED/IRQ_NONE correctly.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 drivers/irqchip/irq-al-fic.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index efa7f2b62d00..9cf3eedac97f 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -4,9 +4,9 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
-#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -95,24 +95,24 @@ static int al_fic_irq_set_type(struct irq_data *data, unsigned int flow_type)
 	return 0;
 }
 
-static void al_fic_irq_handler(struct irq_desc *desc)
+static irqreturn_t al_fic_irq_handler(int irq, void *data)
 {
-	struct al_fic *fic = irq_desc_get_handler_data(desc);
+	struct al_fic *fic = data;
 	struct irq_domain *domain = fic->domain;
-	struct irq_chip *irqchip = irq_desc_get_chip(desc);
 	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+	irqreturn_t ret = IRQ_NONE;
 	unsigned long pending;
 	u32 hwirq;
 
-	chained_irq_enter(irqchip, desc);
-
 	pending = readl_relaxed(fic->base + AL_FIC_CAUSE);
 	pending &= ~gc->mask_cache;
 
-	for_each_set_bit(hwirq, &pending, NR_FIC_IRQS)
-		generic_handle_domain_irq(domain, hwirq);
+	for_each_set_bit(hwirq, &pending, NR_FIC_IRQS) {
+		if (!generic_handle_domain_irq(domain, hwirq))
+			ret = IRQ_HANDLED;
+	}
 
-	chained_irq_exit(irqchip, desc);
+	return ret;
 }
 
 static int al_fic_irq_retrigger(struct irq_data *data)
@@ -162,11 +162,17 @@ static int al_fic_register(struct device_node *node,
 	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 	gc->private = fic;
 
-	irq_set_chained_handler_and_data(fic->parent_irq,
-					 al_fic_irq_handler,
-					 fic);
+	ret = request_irq(fic->parent_irq, al_fic_irq_handler,
+			  IRQF_NO_THREAD | IRQF_SHARED, fic->name, fic);
+	if (ret) {
+		pr_err("fail to request irq (%d)\n", ret);
+		goto err_free_generic_chip;
+	}
+
 	return 0;
 
+err_free_generic_chip:
+	irq_free_generic_chip(gc);
 err_domain_remove:
 	irq_domain_remove(fic->domain);
 
-- 
2.47.3

[PATCH 3/6] dt-bindings: interrupt-controller: amazon,al-fic: add error/fatal groups

[Eliav Farber]

FIC v2 and later expose two additional interrupt groups alongside the
legacy group - an error group and a fatal group - each described by its
own node and matched by a distinct compatible string. Turn the single
compatible into an enum covering all three:

  amazon,al-fic         - legacy group
  amazon,al-fic-error   - error group
  amazon,al-fic-fatal   - fatal group

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 .../bindings/interrupt-controller/amazon,al-fic.yaml         | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
index 26bc05dee0bc..f43039901bd3 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
@@ -11,7 +11,10 @@ maintainers:
 
 properties:
   compatible:
-    const: amazon,al-fic
+    enum:
+      - amazon,al-fic
+      - amazon,al-fic-error
+      - amazon,al-fic-fatal
 
   reg:
     maxItems: 1
-- 
2.47.3

Re: [PATCH 3/6] dt-bindings: interrupt-controller: amazon,al-fic: add error/fatal groups

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Thu, Sep 24, 2026 at 06:23:08AM +0000, Eliav Farber wrote:
> FIC v2 and later expose two additional interrupt groups alongside the
> legacy group - an error group and a fatal group - each described by its
> own node and matched by a distinct compatible string. Turn the single
> compatible into an enum covering all three:

Without any explanation relating to hardware, I find this very hard to
understand. Nodes and compatible strings are devicetree concepts, that
portion of the commit message should explain hardware detail.

What this sounds like from your commit message is that you have a new
revision of this block, and instead of adding an al-fic-v2 compatible,
you're using two new compatibles to describe the new features and using
the old compatible to describe the common featureset. Without a dts, I
cannot say for sure.

pw-bot: changes-requested

> 
>   amazon,al-fic         - legacy group
>   amazon,al-fic-error   - error group
>   amazon,al-fic-fatal   - fatal group
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
>  .../bindings/interrupt-controller/amazon,al-fic.yaml         | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
> index 26bc05dee0bc..f43039901bd3 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
> @@ -11,7 +11,10 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: amazon,al-fic
> +    enum:
> +      - amazon,al-fic
> +      - amazon,al-fic-error
> +      - amazon,al-fic-fatal
>  
>    reg:
>      maxItems: 1
> -- 
> 2.47.3
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH 3/6] dt-bindings: interrupt-controller: amazon, al-fic: add error/fatal groups

[Farber, Eliav]

On Thu, Sep 24, 2026 at 06:13:52PM +0100, Conor Dooley wrote:
> Without any explanation relating to hardware, I find this very hard to
> understand. Nodes and compatible strings are devicetree concepts, that
> portion of the commit message should explain hardware detail.

Agreed, the message failed to describe the hardware. Here it is, and v2
will carry it.

The block that this driver calls a FIC is the generic Annapurna
interrupt controller. It is built from groups of up to 32 triggers. The
number of groups differs from one controller to another. Each group has
its own 0x40 register block.

Note the granularity, because it decides the shape of this binding: a
node here describes ONE GROUP, not a whole controller. reg points at
that group's 0x40 block, and the 32 hwirqs of the domain are that
group's 32 triggers. A controller with several groups appears as several
nodes, and controllers cascade when a tree needs more triggers than one
controller has.

The group registers:

  0x00  cause
  0x10  mask           gates the INFO output
  0x28  group control  revision in bits 29-28
  0x2c  error mask     gates the ERROR output
  0x34  fatal mask     gates the FATAL output

A group has one cause register. The mask registers decide which output a
set cause bit drives. The classic revision has neither the error nor the
fatal mask, and neither output; the revision that added both reports 1 in
the control register, and every group reports it in its own.

The SoC carries INFO, ERROR and FATAL as three separate aggregation
trees, and a unit is required to keep them separate: mapping one event
to more than one severity is forbidden by the interrupt methodology, so
a group belongs to exactly one tree. A group in the error tree has the
controller's ERROR output wired towards its parent and its INFO output
unused. The error tree terminates in one dedicated GIC SPI, the fatal
tree in another. The two trees differ in trigger type as well, error
being level high and fatal edge rising.

> What this sounds like from your commit message is that you have a new
> revision of this block, and instead of adding an al-fic-v2 compatible,
> you're using two new compatibles to describe the new features and using
> the old compatible to describe the common featureset.

Not quite, on both halves.

There is a new revision, and no string encodes it, because of the
granularity above. The node describes a group, and the group reports the
revision itself, in its own control register. An amazon,al-fic-v2 would
put a controller-level version number into a node that describes one
group of that controller, and would duplicate a register that same group
already exposes. 4/6 reads the register instead. v3 reports 2 there, and
differs from v2 only by an erratum in the two new mask registers, which
is why 5/6 is four lines.

The strings name which output of the controller is the one connected, and
so which of the three mask registers the driver must program for this
group. Nothing in the block reports that. Cause, control, revision and
the 32-trigger domain are identical in all three cases, and the choice is
fixed when the SoC is wired.

> Without a dts, I cannot say for sure.

Fair, and there is nothing in tree to look at. The driver and binding
landed in 2019 without a devicetree. Here is the topology, which v2 will
add to the binding as an example:

    /* group A and group B of one controller in the error tree. The
     * controller has a single ERROR output, so both groups reach the
     * parent through the same line.
     */
    err_fic_a: interrupt-controller@fd8a8500 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8500 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
    };

    err_fic_b: interrupt-controller@fd8a8540 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8540 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
    };

    /* a peripheral's own error group, cascaded into bit 6 of group A */
    interrupt-controller@fd8a8580 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8580 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupt-parent = <&err_fic_a>;
        interrupts = <6 IRQ_TYPE_LEVEL_HIGH>;
    };

The first two nodes are what patch 2/6 is for. The groups of one
controller share that controller's outputs, so a real devicetree has
nodes on one parent line, and a chained handler can only be installed
once per parent. Cascading adds more of the same, since an aggregating
group collects many peripherals onto the line above it. I will say that
in 2/6 instead of "on some platforms".

Thanks,
Eliav

Re: [PATCH 3/6] dt-bindings: interrupt-controller: amazon, al-fic: add error/fatal groups

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7095 bytes --]

On Fri, Sep 25, 2026 at 10:24:38AM +0000, Farber, Eliav wrote:
> On Thu, Sep 24, 2026 at 06:13:52PM +0100, Conor Dooley wrote:
> > Without any explanation relating to hardware, I find this very hard to
> > understand. Nodes and compatible strings are devicetree concepts, that
> > portion of the commit message should explain hardware detail.
> 
> Agreed, the message failed to describe the hardware. Here it is, and v2
> will carry it.

Cool. I won't pretend to understand all of what is written here, but it
does assuage my concern that you were coming up with compatibles for
the 0x2c and 0x34 registers. I'm not entirely sure that a compatible
change is the right way to communicate which aspect of the device is
being used - typically things like this get dedicated properties because
AFAICT from the below the hardware is the same, just the output path
differs, something like this incomplete diff:
diff --git a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
index 26bc05dee0bc..f1f7b555c48d 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
@@ -11,7 +11,11 @@ maintainers:
 
 properties:
   compatible:
-    const: amazon,al-fic
+    oneOf:
+      - const: amazon,al-fic
+      - items:
+          - amazon,al-fic-v2
+          - amazon,al-fic
 
   reg:
     maxItems: 1
@@ -24,6 +28,11 @@ properties:
   interrupts:
     maxItems: 1
 
+  amazon,something:
+    ref: ...string
+    enum: [info, error, fatal]
+    default: info
+
 required:
   - compatible
   - reg
@@ -31,6 +40,16 @@ required:
   - '#interrupt-cells'
   - interrupts
 
+if:
+  properties:
+    compatible:
+      not:
+        contains:
+          const: amazon,al-fic-v2
+then:
+  properties:
+    amazon,something: false
+
 additionalProperties: false
 
 examples:

The fallback does worry me a little though, since it'd operate the
instances intended to be fatal or error as info, which would probably
cause problems? Unless each has different output ports, and it's not
controlling a mux to a single port, and the worst outcome then would be
that nothing would ever be reported.

Conor.

> 
> The block that this driver calls a FIC is the generic Annapurna
> interrupt controller. It is built from groups of up to 32 triggers. The
> number of groups differs from one controller to another. Each group has
> its own 0x40 register block.
> 
> Note the granularity, because it decides the shape of this binding: a
> node here describes ONE GROUP, not a whole controller. reg points at
> that group's 0x40 block, and the 32 hwirqs of the domain are that
> group's 32 triggers. A controller with several groups appears as several
> nodes, and controllers cascade when a tree needs more triggers than one
> controller has.
> 
> The group registers:
> 
>   0x00  cause
>   0x10  mask           gates the INFO output
>   0x28  group control  revision in bits 29-28
>   0x2c  error mask     gates the ERROR output
>   0x34  fatal mask     gates the FATAL output
> 
> A group has one cause register. The mask registers decide which output a
> set cause bit drives. The classic revision has neither the error nor the
> fatal mask, and neither output; the revision that added both reports 1 in
> the control register, and every group reports it in its own.
> 
> The SoC carries INFO, ERROR and FATAL as three separate aggregation
> trees, and a unit is required to keep them separate: mapping one event
> to more than one severity is forbidden by the interrupt methodology, so
> a group belongs to exactly one tree. A group in the error tree has the
> controller's ERROR output wired towards its parent and its INFO output
> unused. The error tree terminates in one dedicated GIC SPI, the fatal
> tree in another. The two trees differ in trigger type as well, error
> being level high and fatal edge rising.
> 
> > What this sounds like from your commit message is that you have a new
> > revision of this block, and instead of adding an al-fic-v2 compatible,
> > you're using two new compatibles to describe the new features and using
> > the old compatible to describe the common featureset.
> 
> Not quite, on both halves.
> 
> There is a new revision, and no string encodes it, because of the
> granularity above. The node describes a group, and the group reports the
> revision itself, in its own control register. An amazon,al-fic-v2 would
> put a controller-level version number into a node that describes one
> group of that controller, and would duplicate a register that same group
> already exposes. 4/6 reads the register instead. v3 reports 2 there, and
> differs from v2 only by an erratum in the two new mask registers, which
> is why 5/6 is four lines.
> 
> The strings name which output of the controller is the one connected, and
> so which of the three mask registers the driver must program for this
> group. Nothing in the block reports that. Cause, control, revision and
> the 32-trigger domain are identical in all three cases, and the choice is
> fixed when the SoC is wired.
> 
> > Without a dts, I cannot say for sure.
> 
> Fair, and there is nothing in tree to look at. The driver and binding
> landed in 2019 without a devicetree. Here is the topology, which v2 will
> add to the binding as an example:
> 
>     /* group A and group B of one controller in the error tree. The
>      * controller has a single ERROR output, so both groups reach the
>      * parent through the same line.
>      */
>     err_fic_a: interrupt-controller@fd8a8500 {
>         compatible = "amazon,al-fic-error";
>         reg = <0xfd8a8500 0x40>;
>         interrupt-controller;
>         #interrupt-cells = <2>;
>         interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>     };
> 
>     err_fic_b: interrupt-controller@fd8a8540 {
>         compatible = "amazon,al-fic-error";
>         reg = <0xfd8a8540 0x40>;
>         interrupt-controller;
>         #interrupt-cells = <2>;
>         interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>     };
> 
>     /* a peripheral's own error group, cascaded into bit 6 of group A */
>     interrupt-controller@fd8a8580 {
>         compatible = "amazon,al-fic-error";
>         reg = <0xfd8a8580 0x40>;
>         interrupt-controller;
>         #interrupt-cells = <2>;
>         interrupt-parent = <&err_fic_a>;
>         interrupts = <6 IRQ_TYPE_LEVEL_HIGH>;
>     };
> 
> The first two nodes are what patch 2/6 is for. The groups of one
> controller share that controller's outputs, so a real devicetree has
> nodes on one parent line, and a chained handler can only be installed
> once per parent. Cascading adds more of the same, since an aggregating
> group collects many peripherals onto the line above it. I will say that
> in 2/6 instead of "on some platforms".
> 
> Thanks,
> Eliav

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH 3/6] dt-bindings: interrupt-controller: amazon, al-fic: add error/fatal groups

[Farber, Eliav]

On Fri, Sep 25, 2026 at 06:13:38PM +0100, Conor Dooley wrote:
> On Fri, Sep 25, 2026 at 10:24:38AM +0000, Farber, Eliav wrote:
> > Agreed, the message failed to describe the hardware. Here it is, and v2
> > will carry it.
>
> Cool. I won't pretend to understand all of what is written here, but it
> does assuage my concern that you were coming up with compatibles for
> the 0x2c and 0x34 registers. I'm not entirely sure that a compatible
> change is the right way to communicate which aspect of the device is
> being used - typically things like this get dedicated properties because
> AFAICT from the below the hardware is the same, just the output path
> differs, something like this incomplete diff:

Agreed, a property is the right fit. v2 does that.

One compatible (amazon,al-fic) and a new optional property,
amazon,al-fic-mask, enum [info, error, fatal], default info. No
amazon,al-fic-v2 fallback: a node describes one group, and the group
reports its revision in its own control register, so the driver reads it
rather than encoding it in a string. The default keeps existing
amazon,al-fic nodes behaving as they do today.

> The fallback does worry me a little though, since it'd operate the
> instances intended to be fatal or error as info, which would probably
> cause problems? Unless each has different output ports, and it's not
> controlling a mux to a single port, and the worst outcome then would be
> that nothing would ever be reported.

Right on the second half: they are different output ports, not a mux. A
group drives exactly one of three separate outputs (info, error, fatal),
each with its own mask register and its own aggregation tree to the GIC.
The property picks which mask register the driver programs; it does not
retarget a shared port.

That is also why there is no unsafe fallback. On the classic revision the
error and fatal masks do not exist, so asking for error or fatal there is
rejected at probe against the control register rather than silently
falling back to info.

The commit message now describes the hardware and the group granularity,
and the binding gains an example.

Thanks for the review.

Eliav

[PATCH 4/6] irqchip/al-fic: add support for FIC v2

[Eliav Farber]

FIC v2 hardware adds two additional interrupt groups on top of the
legacy group: an "error" group and a "fatal" group, each with its own
mask register (AL_FIC_ERROR_MASK, AL_FIC_FATAL_MASK). A single FIC
instance exposes one of these groups, selected by the compatible string.

Detect the hardware version from the CONTROL register version id field
(bits 28-29) and, on v2, mask out the error and fatal groups during
initialization.

The driver now distinguishes the three groups via enum al_fic_id, wired
up through three IRQCHIP_DECLARE entries:

  amazon,al-fic         - legacy group (existing, unchanged behaviour)
  amazon,al-fic-error   - error group  (v2+)
  amazon,al-fic-fatal   - fatal group  (v2+)

Requesting an error/fatal group on a v1 device is rejected.

On v2 the error and fatal mask registers always read back as 0,
regardless of their actual contents, so IRQ_GC_INIT_MASK_CACHE seeds
mask_cache with 0 and makes every source appear unmasked on the next
access. Since al_fic_wire_init() has already masked all sources, override
mask_cache to match for these two groups and let the driver maintain it.
The legacy mask register is not affected, so the legacy group keeps the
register-seeded mask_cache.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
 drivers/irqchip/irq-al-fic.c | 101 ++++++++++++++++++++++++++++++-----
 1 file changed, 89 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 9cf3eedac97f..90a64350be63 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -16,11 +16,14 @@
 /* FIC Registers */
 #define AL_FIC_CAUSE		0x00
 #define AL_FIC_SET_CAUSE	0x08
-#define AL_FIC_MASK		0x10
+#define AL_FIC_LEGACY_MASK	0x10
 #define AL_FIC_CONTROL		0x28
+#define AL_FIC_ERROR_MASK	0x2c
+#define AL_FIC_FATAL_MASK	0x34
 
 #define CONTROL_TRIGGER_RISING	BIT(3)
 #define CONTROL_MASK_MSI_X	BIT(5)
+#define CONTROL_VERSION_ID	GENMASK(29, 28)
 
 #define NR_FIC_IRQS 32
 
@@ -33,12 +36,32 @@ enum al_fic_state {
 	AL_FIC_CONFIGURED_RISING_EDGE,
 };
 
+enum al_fic_version {
+	AL_FIC_VERSION_V1,
+	AL_FIC_VERSION_V2,
+};
+
+enum al_fic_id {
+	AL_FIC_ID_LEGACY,
+	AL_FIC_ID_ERROR,
+	AL_FIC_ID_FATAL,
+	AL_FIC_ID_MAX,		/* keep last */
+};
+
+/* Mask register offset for each interrupt group */
+static const unsigned int al_fic_mask_offset[AL_FIC_ID_MAX] = {
+	[AL_FIC_ID_LEGACY] = AL_FIC_LEGACY_MASK,
+	[AL_FIC_ID_ERROR]  = AL_FIC_ERROR_MASK,
+	[AL_FIC_ID_FATAL]  = AL_FIC_FATAL_MASK,
+};
+
 struct al_fic {
 	void __iomem *base;
 	struct irq_domain *domain;
 	const char *name;
 	unsigned int parent_irq;
 	enum al_fic_state state;
+	enum al_fic_version version;
 };
 
 static void al_fic_set_trigger(struct al_fic *fic,
@@ -126,7 +149,8 @@ static int al_fic_irq_retrigger(struct irq_data *data)
 }
 
 static int al_fic_register(struct device_node *node,
-			   struct al_fic *fic)
+			   struct al_fic *fic,
+			   enum al_fic_id fic_id)
 {
 	struct irq_chip_generic *gc;
 	int ret;
@@ -152,7 +176,7 @@ static int al_fic_register(struct device_node *node,
 
 	gc = irq_get_domain_generic_chip(fic->domain, 0);
 	gc->reg_base = fic->base;
-	gc->chip_types->regs.mask = AL_FIC_MASK;
+	gc->chip_types->regs.mask = al_fic_mask_offset[fic_id];
 	gc->chip_types->regs.ack = AL_FIC_CAUSE;
 	gc->chip_types->chip.irq_mask = irq_gc_mask_set_bit;
 	gc->chip_types->chip.irq_unmask = irq_gc_mask_clr_bit;
@@ -162,6 +186,20 @@ static int al_fic_register(struct device_node *node,
 	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 	gc->private = fic;
 
+	/*
+	 * On FIC v2 the error and fatal mask registers always read back as 0,
+	 * regardless of their actual contents, so IRQ_GC_INIT_MASK_CACHE seeds
+	 * mask_cache with 0 and makes every source appear unmasked on the next
+	 * access. al_fic_wire_init() has already masked all sources, so override
+	 * mask_cache to match and let the driver maintain it from here.
+	 *
+	 * The legacy mask register is not affected, and the erratum is fixed
+	 * from v3 onwards, so those paths keep the register-seeded mask_cache.
+	 */
+	if (fic->version == AL_FIC_VERSION_V2 &&
+	    (fic_id == AL_FIC_ID_ERROR || fic_id == AL_FIC_ID_FATAL))
+		gc->mask_cache = 0xFFFFFFFF;
+
 	ret = request_irq(fic->parent_irq, al_fic_irq_handler,
 			  IRQF_NO_THREAD | IRQF_SHARED, fic->name, fic);
 	if (ret) {
@@ -194,11 +232,13 @@ static int al_fic_register(struct device_node *node,
 static struct al_fic *al_fic_wire_init(struct device_node *node,
 				       void __iomem *base,
 				       const char *name,
-				       unsigned int parent_irq)
+				       unsigned int parent_irq,
+				       enum al_fic_id fic_id)
 {
 	struct al_fic *fic;
+	u32 version_id;
+	u32 control;
 	int ret;
-	u32 control = CONTROL_MASK_MSI_X;
 
 	fic = kzalloc_obj(*fic);
 	if (!fic)
@@ -208,22 +248,37 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 	fic->parent_irq = parent_irq;
 	fic->name = name;
 
+	control = readl_relaxed(fic->base + AL_FIC_CONTROL);
+	version_id = FIELD_GET(CONTROL_VERSION_ID, control);
+	fic->version = version_id;
+	if (version_id == AL_FIC_VERSION_V1 && fic_id != AL_FIC_ID_LEGACY) {
+		pr_err("%s unexpected fic id (%d)\n", fic->name, fic_id);
+		ret = -EINVAL;
+		goto err_free;
+	}
+
 	/* mask out all interrupts */
-	writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_MASK);
+	writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_LEGACY_MASK);
+	if (version_id > AL_FIC_VERSION_V1) {
+		writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_ERROR_MASK);
+		writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_FATAL_MASK);
+	}
 
 	/* clear any pending interrupt */
 	writel_relaxed(0, fic->base + AL_FIC_CAUSE);
 
+	/* make sure the controller works in non msi_x mode */
+	control |= CONTROL_MASK_MSI_X;
 	writel_relaxed(control, fic->base + AL_FIC_CONTROL);
 
-	ret = al_fic_register(node, fic);
+	ret = al_fic_register(node, fic, fic_id);
 	if (ret) {
 		pr_err("fail to register irqchip\n");
 		goto err_free;
 	}
 
-	pr_info("%s initialized successfully in Legacy mode (parent-irq=%u)\n",
-		fic->name, parent_irq);
+	pr_info("%s initialized successfully (fic_id=%d parent-irq=%u)\n",
+		fic->name, fic_id, parent_irq);
 
 	return fic;
 
@@ -233,7 +288,8 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 }
 
 static int __init al_fic_init_dt(struct device_node *node,
-				 struct device_node *parent)
+				 struct device_node *parent,
+				 enum al_fic_id fic_id)
 {
 	int ret;
 	void __iomem *base;
@@ -262,7 +318,8 @@ static int __init al_fic_init_dt(struct device_node *node,
 	fic = al_fic_wire_init(node,
 			       base,
 			       node->full_name,
-			       parent_irq);
+			       parent_irq,
+			       fic_id);
 	if (IS_ERR(fic)) {
 		pr_err("%s: fail to initialize irqchip (%lu)\n",
 		       node->full_name,
@@ -281,4 +338,24 @@ static int __init al_fic_init_dt(struct device_node *node,
 	return ret;
 }
 
-IRQCHIP_DECLARE(al_fic, "amazon,al-fic", al_fic_init_dt);
+static int __init al_fic_init_dt_legacy(struct device_node *node,
+					struct device_node *parent)
+{
+	return al_fic_init_dt(node, parent, AL_FIC_ID_LEGACY);
+}
+
+static int __init al_fic_init_dt_error(struct device_node *node,
+				       struct device_node *parent)
+{
+	return al_fic_init_dt(node, parent, AL_FIC_ID_ERROR);
+}
+
+static int __init al_fic_init_dt_fatal(struct device_node *node,
+				       struct device_node *parent)
+{
+	return al_fic_init_dt(node, parent, AL_FIC_ID_FATAL);
+}
+
+IRQCHIP_DECLARE(al_fic_legacy, "amazon,al-fic", al_fic_init_dt_legacy);
+IRQCHIP_DECLARE(al_fic_error, "amazon,al-fic-error", al_fic_init_dt_error);
+IRQCHIP_DECLARE(al_fic_fatal, "amazon,al-fic-fatal", al_fic_init_dt_fatal);
-- 
2.47.3