[PATCH v2 0/8] irqchip/al-fic: shared parent IRQ, error/fatal outputs and affinity

[PATCH v2 0/8] irqchip/al-fic: shared parent IRQ, error/fatal outputs and affinity

[Eliav Farber]

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

The block the driver calls a FIC is the generic Annapurna Labs interrupt
controller, built from groups of up to 32 triggers each. A device tree node
describes one group, not a whole controller, and the groups of one
controller share that controller's output line toward the parent GIC SPI.
Later revisions add two interrupt outputs on top of the original "info"
output - an "error" output and a "fatal" output - each with its own mask
register. Which output a group drives is fixed when the SoC is wired.

The series is organised as follows:

  1. Fix an argument alignment and a repeated word flagged by checkpatch.
     No functional change.
  2. Use %pOF in log messages and raise the successful-init message to
     pr_info, so each FIC instance is identifiable at boot.
  3. Keep the device_node in struct al_fic instead of a cached name string
     that aliased node storage.
  4. Request the parent interrupt as shared (IRQF_SHARED) instead of a
     chained handler, so the several nodes of one controller can share one
     GIC SPI.
  5. Extend the binding with an optional amazon,al-fic-mask property
     (info, error or fatal; default info) selecting which output a group
     drives.
  6. Add error/fatal output support and FIC v2: version detection from the
     control register, the two extra mask registers, and a mask_cache
     workaround for a v2 erratum where the error and fatal mask registers
     always read back as 0.
  7. 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.
  8. 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.

Patches 4 and 8 are co-developed with Talel Shenhar, the al-fic driver
maintainer, and carry his Signed-off-by.

Changes since v1:
 - Binding (patch 5): drop the two proposed per-output compatible strings
   (amazon,al-fic-error / amazon,al-fic-fatal). A node describes one group
   and a controller reports its revision in the group control register
   (bits 29-28), so no per-revision or per-output compatible is needed.
   Instead select the output with a new optional amazon,al-fic-mask
   property (info, error, fatal; default info), which keeps existing
   amazon,al-fic nodes behaving as before. This addresses Conor Dooley's
   review of the v1 binding patch, and the commit message now describes
   the hardware and the group granularity as he asked. The YAML gains an
   example fragment.
 - FIX (patch 4): the request_irq() error path called
   irq_free_generic_chip(gc), which is kfree(gc) on an interior pointer
   into a single allocation - invalid free, reachable when request_irq()
   fails at probe. Replace it with irq_domain_remove_generic_chips()
   before irq_domain_remove().
 - FIX (patch 6): the v2 mask_cache workaround was dead. mask_cache is not
   seeded until the first child mapping, so the override was overwritten
   with 0 and the first unmask then cleared the whole mask register,
   unmasking all 32 sources. Drop IRQ_GC_INIT_MASK_CACHE for the error and
   fatal outputs and seed mask_cache from the value the driver programmed.
 - New patch 1: checkpatch cleanup (alignment + repeated word), at the head
   of the series.
 - New patch 3: keep the device_node rather than a cached name string;
   reorders the irqchip prep so every commit builds on its own.
 - Reworded commit messages throughout to describe hardware rather than
   device tree mechanics.

v1: https://lore.kernel.org/all/20260924062311.37308-1-farbere@amazon.com/

Eliav Farber (8):
  irqchip/al-fic: fix argument alignment and a repeated word
  irqchip/al-fic: use %pOF and raise init log level
  irqchip/al-fic: keep the device_node instead of a cached name string
  irqchip/al-fic: switch to shared parent interrupt
  dt-bindings: interrupt-controller: amazon,al-fic: add mask selection
  irqchip/al-fic: support error and fatal outputs and FIC v2
  irqchip/al-fic: add support for FIC v3
  irqchip/al-fic: add irq_set_affinity callback

 .../interrupt-controller/amazon,al-fic.yaml   |  16 +-
 drivers/irqchip/irq-al-fic.c                  | 233 ++++++++++++++----
 2 files changed, 206 insertions(+), 43 deletions(-)


base-commit: 6828157ab3e96cd1c957b78d995c5e508a2eb2c2
-- 
2.47.3

[PATCH v2 1/8] irqchip/al-fic: fix argument alignment and a repeated word

[Eliav Farber]

The continuation lines of the irq_domain_create_linear() call sit three
columns short of the open parenthesis. They were aligned for
irq_domain_add_linear(); commit affdc0d1bdfa ("irqchip: Switch to
irq_domain_create_*()") renamed the function to one three characters longer
and left the indentation as it was. checkpatch --strict reports "Alignment
should match open parenthesis".

al_fic_wire_init()'s kernel-doc reads "the fic hardware to to work in wire
mode". checkpatch reports "Possible repeated word: 'to'".

No functional change.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: new patch. checkpatch --strict cleanups (argument alignment and a
    repeated word) split out ahead of the functional changes; the touched
    lines also appear as context in patches 3 and 6.

 drivers/irqchip/irq-al-fic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index d10ac9b63c99..35581332b40e 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -132,9 +132,9 @@ static int al_fic_register(struct device_node *node,
 	int ret;
 
 	fic->domain = irq_domain_create_linear(of_fwnode_handle(node),
-					    NR_FIC_IRQS,
-					    &irq_generic_chip_ops,
-					    fic);
+					       NR_FIC_IRQS,
+					       &irq_generic_chip_ops,
+					       fic);
 	if (!fic->domain) {
 		pr_err("fail to add irq domain\n");
 		return -ENOMEM;
@@ -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.
-- 
2.47.3

[PATCH v2 2/8] irqchip/al-fic: use %pOF and raise init log level

[Eliav Farber]

Use %pOF in the driver's log messages, so that both the success and the
error paths identify the specific FIC instance by its full devicetree
path rather than by its bare node name, e.g.:

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

A single controller is described by several FIC nodes that share the same
bare node name (e.g. "interrupt-controller"), so the bare name cannot tell
one instance from another. The full path includes the unit address and
keeps the instances distinguishable.

While converting the parent-missing message, fix a grammar typo in it
("device require a parent" -> "device requires a parent").

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.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: reworded to explain why %pOF matters (several nodes of one controller
    share the same bare node name, so only the full path with the unit
    address distinguishes them). Also switched from full_name string to
    %pOF. No functional change from v1's "use full node name" patch beyond
    the log level and the grammar fix.

 drivers/irqchip/irq-al-fic.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 35581332b40e..760bd08dcff4 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -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("%pOF initialized successfully in Legacy mode (parent-irq=%u)\n",
+		node, parent_irq);
 
 	return fic;
 
@@ -235,20 +235,19 @@ static int __init al_fic_init_dt(struct device_node *node,
 	struct al_fic *fic;
 
 	if (!parent) {
-		pr_err("%s: unsupported - device require a parent\n",
-		       node->name);
+		pr_err("%pOF: unsupported - device requires a parent\n", node);
 		return -EINVAL;
 	}
 
 	base = of_iomap(node, 0);
 	if (!base) {
-		pr_err("%s: fail to map memory\n", node->name);
+		pr_err("%pOF: fail to map memory\n", node);
 		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("%pOF: fail to map irq\n", node);
 		ret = -EINVAL;
 		goto err_unmap;
 	}
@@ -258,9 +257,8 @@ static int __init al_fic_init_dt(struct device_node *node,
 			       node->name,
 			       parent_irq);
 	if (IS_ERR(fic)) {
-		pr_err("%s: fail to initialize irqchip (%lu)\n",
-		       node->name,
-		       PTR_ERR(fic));
+		pr_err("%pOF: fail to initialize irqchip (%lu)\n",
+		       node, PTR_ERR(fic));
 		ret = PTR_ERR(fic);
 		goto err_irq_dispose;
 	}
-- 
2.47.3

[PATCH v2 3/8] irqchip/al-fic: keep the device_node instead of a cached name string

[Eliav Farber]

struct al_fic cached a "const char *name" that al_fic_wire_init() received
as a separate argument and set from node->name. That string was never owned
by the driver: it aliased storage inside the device_node and stayed valid
only as long as the node did, yet nothing in the struct held the node to
express that dependency. Keep the device_node in the struct instead: it
holds the owning object rather than a bare pointer into it, lets each site
derive the name on demand, and gives the driver the node it needs in the
next change, which requests the parent interrupt by the node's full_name.

The irqchip callback that has no device_node in scope now prints the
instance with %pOF, which formats the node on demand, and the name argument
threaded through al_fic_wire_init() goes away.

irq_alloc_domain_generic_chips() keeps the pointer it is given, so it now
uses node->full_name. This changes the generic chip name from the bare
node name (e.g. "interrupt-controller") to the full node name including
its unit address (e.g. "interrupt-controller@fd8a8500"), which keeps
instances that share a bare name distinguishable.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: new patch. Keep the device_node in struct al_fic instead of a cached
    name string that aliased node storage. Introduced here so the struct
    holds the node before the next patch requests the parent interrupt by
    node->full_name, keeping every commit buildable on its own.

 drivers/irqchip/irq-al-fic.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 760bd08dcff4..c7cc2631caf8 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -36,7 +36,7 @@ enum al_fic_state {
 struct al_fic {
 	void __iomem *base;
 	struct irq_domain *domain;
-	const char *name;
+	struct device_node *node;
 	unsigned int parent_irq;
 	enum al_fic_state state;
 };
@@ -89,7 +89,7 @@ static int al_fic_irq_set_type(struct irq_data *data, unsigned int flow_type)
 	if (fic->state == AL_FIC_UNCONFIGURED) {
 		al_fic_set_trigger(fic, gc, new_state);
 	} else if (fic->state != new_state) {
-		pr_debug("fic %s state already configured to %d\n", fic->name, fic->state);
+		pr_debug("fic %pOF state already configured to %d\n", fic->node, fic->state);
 		return -EINVAL;
 	}
 	return 0;
@@ -142,7 +142,7 @@ static int al_fic_register(struct device_node *node,
 
 	ret = irq_alloc_domain_generic_chips(fic->domain,
 					     NR_FIC_IRQS,
-					     1, fic->name,
+					     1, fic->node->full_name,
 					     handle_level_irq,
 					     0, 0, IRQ_GC_INIT_MASK_CACHE);
 	if (ret) {
@@ -175,9 +175,8 @@ static int al_fic_register(struct device_node *node,
 
 /*
  * al_fic_wire_init() - initialize and configure fic in wire mode
- * @of_node: optional pointer to interrupt controller's device tree node.
+ * @node: pointer to the interrupt controller's device tree node
  * @base: mmio to fic register
- * @name: name of the fic
  * @parent_irq: interrupt of parent
  *
  * This API will configure the fic hardware to work in wire mode.
@@ -187,7 +186,6 @@ 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)
 {
 	struct al_fic *fic;
@@ -200,7 +198,7 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 
 	fic->base = base;
 	fic->parent_irq = parent_irq;
-	fic->name = name;
+	fic->node = node;
 
 	/* mask out all interrupts */
 	writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_MASK);
@@ -254,7 +252,6 @@ static int __init al_fic_init_dt(struct device_node *node,
 
 	fic = al_fic_wire_init(node,
 			       base,
-			       node->name,
 			       parent_irq);
 	if (IS_ERR(fic)) {
 		pr_err("%pOF: fail to initialize irqchip (%lu)\n",
-- 
2.47.3

[PATCH v2 4/8] 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.

A FIC controller is built from groups, each described by its own DT node,
and the groups of one controller share that controller's output line
toward the parent. So a real devicetree has several FIC nodes on one
parent GIC SPI, and a chained handler can only be installed once per
parent. Cascading compounds this: an aggregating group collects several
peripherals' outputs onto the line above it.

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. IRQF_NO_THREAD is set because the handler only demultiplexes to
the child domain and must not be forced-threaded; all instances sharing a
parent line agree on this flag, as the shared-IRQ core requires.

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.

request_irq() can fail, unlike irq_set_chained_handler_and_data(), so add
an error path for it. It calls irq_domain_remove_generic_chips() before
irq_domain_remove(), because irq_domain_remove() frees the generic chips
only when the domain carries IRQ_DOMAIN_FLAG_DESTROY_GC.

Co-developed-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2:
 - Fix the request_irq() error path: v1 called irq_free_generic_chip(gc),
   which is kfree(gc) on an interior pointer into the single allocation
   made by irq_domain_alloc_generic_chips() - an invalid free reachable
   when request_irq() fails at probe. Replace it with
   irq_domain_remove_generic_chips() before irq_domain_remove(), and add a
   commit-message paragraph explaining the teardown ordering.
 - Add Co-developed-by/Signed-off-by: Talel Shenhar.
 - Reworded to state the hardware reason for the shared parent (the groups
   of one controller share that controller's output line).

 drivers/irqchip/irq-al-fic.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index c7cc2631caf8..091a06abc0bb 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,18 @@ 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->node->full_name,
+			  fic);
+	if (ret) {
+		pr_err("fail to request irq (%d)\n", ret);
+		goto err_remove_generic_chips;
+	}
+
 	return 0;
 
+err_remove_generic_chips:
+	irq_domain_remove_generic_chips(fic->domain);
 err_domain_remove:
 	irq_domain_remove(fic->domain);
 
-- 
2.47.3

[PATCH v2 5/8] dt-bindings: interrupt-controller: amazon,al-fic: add mask selection

[Eliav Farber]

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

A node in this binding 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 triggers. A controller with several groups appears as several
nodes, and controllers cascade when a tree needs more triggers than one
controller has.

A group has one cause register and three mask registers, which decide
which output a set cause bit drives:

  0x10  info mask    gates the info output
  0x2c  error mask   gates the error output
  0x34  fatal mask   gates the fatal output

The three outputs are aggregated into three separate interrupt trees
(info, error, fatal); mapping one event to more than one severity is
forbidden by the interrupt methodology, so a group belongs to exactly one
tree. Which output a group drives is fixed when the SoC is wired and is
not discoverable from any register, so it must be described in the
devicetree.

The error and fatal outputs, and their mask registers, exist only on
controller revisions that implement them; earlier revisions provide the
info output only. A controller reports its revision in the group control
register (bits 29-28), so the revision itself does not need to appear in
the devicetree and no per-revision compatible string is needed.

Describe which output a group drives with a new optional property,
amazon,al-fic-mask (info, error or fatal; default info).

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: redesigned in response to Conor Dooley's review of v1.
 - Drop the two proposed per-output compatible strings
   (amazon,al-fic-error / amazon,al-fic-fatal). A node describes one group,
   and a controller reports its revision in the group control register
   (bits 29-28), so no per-revision or per-output compatible is needed.
 - Instead add an optional amazon,al-fic-mask property (info, error, fatal;
   default info) selecting which output a group drives. Existing
   amazon,al-fic nodes keep behaving as before.
 - Rewrite the commit message to describe the hardware and the group
   granularity, and add an example fragment to the YAML, as requested.

 .../interrupt-controller/amazon,al-fic.yaml      | 16 +++++++++++++++-
 1 file changed, 15 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..1ebf9a427f2f 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
@@ -24,6 +24,19 @@ properties:
   interrupts:
     maxItems: 1
 
+  amazon,al-fic-mask:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Selects which of the controller's severity outputs this group drives,
+      and thus which mask register the driver programs. A group has one cause
+      register and three severity outputs - info, error and fatal - each
+      gated by its own mask register. Each output is aggregated into a
+      separate interrupt tree, and a group belongs to exactly one of them.
+      The error and fatal outputs exist only on controller revisions that
+      implement them; earlier revisions provide the info output only.
+    enum: [info, error, fatal]
+    default: info
+
 required:
   - compatible
   - reg
@@ -39,8 +52,9 @@ examples:
 
     interrupt-controller@fd8a8500 {
         compatible = "amazon,al-fic";
-        reg = <0xfd8a8500 0x1000>;
+        reg = <0xfd8a8500 0x40>;
         interrupt-controller;
         #interrupt-cells = <2>;
         interrupts = <GIC_SPI 0x0 IRQ_TYPE_LEVEL_HIGH>;
+        amazon,al-fic-mask = "error";
     };
-- 
2.47.3

[PATCH v2 6/8] irqchip/al-fic: support error and fatal outputs and FIC v2

[Eliav Farber]

FIC v2 hardware adds two interrupt outputs on top of the info output: an
error output and a fatal output, each with its own mask register
(AL_FIC_ERROR_MASK, AL_FIC_FATAL_MASK). A group drives exactly one of the
three outputs.

Read which output a group drives from the amazon,al-fic-mask devicetree
property (info, error or fatal; absent means info) and program the
matching mask register. Detect the hardware revision from the CONTROL
register version field (bits 28-29). The revision is not in the
devicetree, so requesting the error or fatal output on a v1 device - which
has neither - is rejected at probe against the register.

Name the selected output in the probe log line, in place of the "Legacy
mode" text it replaces. A booted system then shows which output each group
drives.

The "v1" and "v2" names are this driver's labels for the CONTROL
version field encoding (0 and 1).

On v2 the error and fatal mask registers always read back as 0, regardless
of their actual contents. IRQ_GC_INIT_MASK_CACHE seeds mask_cache from the
mask register on the first child mapping, so on those two outputs it would
seed 0: every source would appear unmasked, and the first unmask would
write that 0 back and clear the whole mask register. Drop the flag for
those two outputs and seed mask_cache with the value al_fic_wire_init()
programmed instead. The info mask register is not affected, so the info
output keeps the register-seeded mask_cache.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2:
 - Fix the v2 mask_cache workaround, which was dead in v1. mask_cache is
   not seeded until the first child mapping (irq_map_generic_chip), so the
   v1 override was overwritten with 0 and the first unmask then cleared the
   whole mask register, unmasking all 32 sources. Drop
   IRQ_GC_INIT_MASK_CACHE for the error and fatal outputs and seed
   mask_cache from the value al_fic_wire_init() programmed. Commit message
   rewritten to state the real timing and consequence.
 - Read the output from the new amazon,al-fic-mask property (was per-output
   compatible in v1).
 - Label the mask in the probe log line (mask=%s).

 drivers/irqchip/irq-al-fic.c | 137 ++++++++++++++++++++++++++++++++---
 1 file changed, 126 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 091a06abc0bb..35e366b4da30 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_INFO_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,6 +36,37 @@ enum al_fic_state {
 	AL_FIC_CONFIGURED_RISING_EDGE,
 };
 
+/*
+ * FIC hardware revision, as reported by the CONTROL register version field
+ * (CONTROL_VERSION_ID, bits 29-28). These are this driver's names for that
+ * field's encoding.
+ */
+enum al_fic_version {
+	AL_FIC_VERSION_V1,
+	AL_FIC_VERSION_V2,
+};
+
+enum al_fic_id {
+	AL_FIC_ID_INFO,
+	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_INFO]  = AL_FIC_INFO_MASK,
+	[AL_FIC_ID_ERROR] = AL_FIC_ERROR_MASK,
+	[AL_FIC_ID_FATAL] = AL_FIC_FATAL_MASK,
+};
+
+/* amazon,al-fic-mask property value for each interrupt group */
+static const char * const al_fic_mask_name[AL_FIC_ID_MAX] = {
+	[AL_FIC_ID_INFO]  = "info",
+	[AL_FIC_ID_ERROR] = "error",
+	[AL_FIC_ID_FATAL] = "fatal",
+};
+
 struct al_fic {
 	void __iomem *base;
 	struct irq_domain *domain;
@@ -126,11 +160,32 @@ 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,
+			   enum al_fic_version version)
 {
 	struct irq_chip_generic *gc;
+	enum irq_gc_flags gc_flags;
 	int ret;
 
+	/*
+	 * On FIC v2 the error and fatal mask registers always read back as 0,
+	 * regardless of their actual contents. IRQ_GC_INIT_MASK_CACHE seeds
+	 * mask_cache from the mask register on the first child mapping, so on
+	 * those two outputs it would seed 0 and make every source appear
+	 * unmasked - and the first unmask would then clear the whole mask
+	 * register. Suppress the seeding there and set mask_cache below to
+	 * match what al_fic_wire_init() programmed.
+	 *
+	 * The info mask register is not affected, so the info output keeps the
+	 * register-seeded mask_cache.
+	 */
+	if (version == AL_FIC_VERSION_V2 &&
+	    (fic_id == AL_FIC_ID_ERROR || fic_id == AL_FIC_ID_FATAL))
+		gc_flags = 0;
+	else
+		gc_flags = IRQ_GC_INIT_MASK_CACHE;
+
 	fic->domain = irq_domain_create_linear(of_fwnode_handle(node),
 					       NR_FIC_IRQS,
 					       &irq_generic_chip_ops,
@@ -144,7 +199,7 @@ static int al_fic_register(struct device_node *node,
 					     NR_FIC_IRQS,
 					     1, fic->node->full_name,
 					     handle_level_irq,
-					     0, 0, IRQ_GC_INIT_MASK_CACHE);
+					     0, 0, gc_flags);
 	if (ret) {
 		pr_err("fail to allocate generic chip (%d)\n", ret);
 		goto err_domain_remove;
@@ -152,7 +207,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 +217,13 @@ static int al_fic_register(struct device_node *node,
 	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 	gc->private = fic;
 
+	/*
+	 * Seed the mask cache the driver maintains itself, matching the mask
+	 * al_fic_wire_init() programmed (see the gc_flags comment above).
+	 */
+	if (!(gc_flags & IRQ_GC_INIT_MASK_CACHE))
+		gc->mask_cache = 0xFFFFFFFF;
+
 	ret = request_irq(fic->parent_irq, al_fic_irq_handler,
 			  IRQF_NO_THREAD | IRQF_SHARED, fic->node->full_name,
 			  fic);
@@ -185,6 +247,8 @@ static int al_fic_register(struct device_node *node,
  * @node: pointer to the interrupt controller's device tree node
  * @base: mmio to fic register
  * @parent_irq: interrupt of parent
+ * @fic_id: which of the controller's outputs (info, error or fatal) this
+ *          group drives
  *
  * This API will configure the fic hardware to work in wire mode.
  * In wire mode, fic hardware is generating a wire ("wired") interrupt.
@@ -193,11 +257,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,
-				       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)
@@ -207,22 +273,37 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 	fic->parent_irq = parent_irq;
 	fic->node = node;
 
+	control = readl_relaxed(fic->base + AL_FIC_CONTROL);
+	version_id = FIELD_GET(CONTROL_VERSION_ID, control);
+	if (version_id == AL_FIC_VERSION_V1 && fic_id != AL_FIC_ID_INFO) {
+		pr_err("%pOF: amazon,al-fic-mask = \"%s\" not available on FIC v1\n",
+		       node, al_fic_mask_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_INFO_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, version_id);
 	if (ret) {
 		pr_err("fail to register irqchip\n");
 		goto err_free;
 	}
 
-	pr_info("%pOF initialized successfully in Legacy mode (parent-irq=%u)\n",
-		node, parent_irq);
+	pr_info("%pOF initialized successfully (mask=%s parent-irq=%u)\n",
+		node, al_fic_mask_name[fic_id], parent_irq);
 
 	return fic;
 
@@ -231,11 +312,38 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 	return ERR_PTR(ret);
 }
 
+/*
+ * Parse the amazon,al-fic-mask property into an enum al_fic_id, selecting
+ * which of the controller's outputs this group drives. The property is
+ * optional; an absent property means the info output.
+ */
+static int al_fic_parse_mask(struct device_node *node, enum al_fic_id *fic_id)
+{
+	const char *mask;
+	int ret;
+
+	ret = of_property_read_string(node, "amazon,al-fic-mask", &mask);
+	if (ret == -EINVAL) {
+		*fic_id = AL_FIC_ID_INFO;
+		return 0;
+	}
+	if (ret)
+		return ret;
+
+	ret = match_string(al_fic_mask_name, AL_FIC_ID_MAX, mask);
+	if (ret < 0)
+		return ret;
+
+	*fic_id = ret;
+	return 0;
+}
+
 static int __init al_fic_init_dt(struct device_node *node,
 				 struct device_node *parent)
 {
 	int ret;
 	void __iomem *base;
+	enum al_fic_id fic_id;
 	unsigned int parent_irq;
 	struct al_fic *fic;
 
@@ -244,6 +352,12 @@ static int __init al_fic_init_dt(struct device_node *node,
 		return -EINVAL;
 	}
 
+	ret = al_fic_parse_mask(node, &fic_id);
+	if (ret) {
+		pr_err("%pOF: invalid amazon,al-fic-mask\n", node);
+		return ret;
+	}
+
 	base = of_iomap(node, 0);
 	if (!base) {
 		pr_err("%pOF: fail to map memory\n", node);
@@ -259,7 +373,8 @@ static int __init al_fic_init_dt(struct device_node *node,
 
 	fic = al_fic_wire_init(node,
 			       base,
-			       parent_irq);
+			       parent_irq,
+			       fic_id);
 	if (IS_ERR(fic)) {
 		pr_err("%pOF: fail to initialize irqchip (%lu)\n",
 		       node, PTR_ERR(fic));
-- 
2.47.3

[PATCH v2 7/8] irqchip/al-fic: add support for FIC v3

[Eliav Farber]

FIC v3 reports version id 2 in the control register version field. It
exposes the same info/error/fatal groups as v2, so add the version to
enum al_fic_version; the existing "version_id > V1" handling masks the
error and fatal groups at init as for v2.

Unlike v2, the v3 error and fatal mask registers read back correctly, so
v3 does not need the v2 mask_cache workaround: the workaround is gated on
AL_FIC_VERSION_V2 and v3 keeps the register-seeded mask_cache.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: the "erratum fixed from v3 onwards" code comment moved here (where
    AL_FIC_VERSION_V3 is introduced) from the FIC v2 patch, so the claim
    that v3 keeps the register-seeded mask_cache has its code in the same
    commit.

 drivers/irqchip/irq-al-fic.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 35e366b4da30..322e41ffa2fa 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -44,6 +44,7 @@ enum al_fic_state {
 enum al_fic_version {
 	AL_FIC_VERSION_V1,
 	AL_FIC_VERSION_V2,
+	AL_FIC_VERSION_V3,
 };
 
 enum al_fic_id {
@@ -177,8 +178,8 @@ static int al_fic_register(struct device_node *node,
 	 * register. Suppress the seeding there and set mask_cache below to
 	 * match what al_fic_wire_init() programmed.
 	 *
-	 * The info mask register is not affected, so the info output keeps the
-	 * register-seeded mask_cache.
+	 * The info mask register is not affected, and the erratum is fixed
+	 * from v3 onwards, so those paths keep the register-seeded mask_cache.
 	 */
 	if (version == AL_FIC_VERSION_V2 &&
 	    (fic_id == AL_FIC_ID_ERROR || fic_id == AL_FIC_ID_FATAL))
-- 
2.47.3

[PATCH v2 8/8] irqchip/al-fic: add irq_set_affinity callback

[Eliav Farber]

The FIC aggregates up to 32 interrupt sources into a single parent GIC
SPI and has no per-child affinity control; affinity is determined by the
parent GIC SPI routing.

irq_set_affinity() fails with -EINVAL when the target irqchip has no
irq_set_affinity callback, which prevents drivers that call it from
probing when their interrupt is routed through a FIC instance. Add an
irq_set_affinity callback that returns IRQ_SET_MASK_OK_DONE
unconditionally.

Affinity requests cannot be forwarded to the parent GIC SPI, because it
may be shared by several FIC instances and changing its affinity for one
child would silently move all the others. A pr_warn_once() is emitted
when the requested affinity differs from the parent's current affinity,
giving visibility without failing the caller.

Co-developed-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v2: add Co-developed-by/Signed-off-by: Talel Shenhar. No code change from
    v1.

 drivers/irqchip/irq-al-fic.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 322e41ffa2fa..a558e0d88697 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -160,6 +160,36 @@ static int al_fic_irq_retrigger(struct irq_data *data)
 	return 1;
 }
 
+/*
+ * The FIC aggregates up to 32 child interrupts into a single parent GIC SPI.
+ * It has no hardware affinity control - all children inherit the parent's
+ * CPU routing. Affinity requests cannot be forwarded to the parent, because
+ * a parent GIC SPI may be shared by multiple FIC instances and changing its
+ * affinity for one child would silently move all the others.
+ *
+ * Return IRQ_SET_MASK_OK_DONE unconditionally so that callers using
+ * irq_set_affinity() do not fail with -EINVAL for lack of this callback.
+ */
+static int al_fic_irq_set_affinity(struct irq_data *data,
+				   const struct cpumask *mask, bool force)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	const struct cpumask *parent_affinity;
+	struct al_fic *fic = gc->private;
+	struct irq_data *parent_data;
+
+	parent_data = irq_get_irq_data(fic->parent_irq);
+	if (!parent_data)
+		return -EINVAL;
+
+	parent_affinity = irq_data_get_affinity_mask(parent_data);
+	if (!cpumask_equal(mask, parent_affinity))
+		pr_warn_once("%pOF: affinity change requested but FIC is fixed to parent IRQ %u CPU routing\n",
+			     fic->node, fic->parent_irq);
+
+	return IRQ_SET_MASK_OK_DONE;
+}
+
 static int al_fic_register(struct device_node *node,
 			   struct al_fic *fic,
 			   enum al_fic_id fic_id,
@@ -215,6 +245,7 @@ static int al_fic_register(struct device_node *node,
 	gc->chip_types->chip.irq_ack = irq_gc_ack_clr_bit;
 	gc->chip_types->chip.irq_set_type = al_fic_irq_set_type;
 	gc->chip_types->chip.irq_retrigger = al_fic_irq_retrigger;
+	gc->chip_types->chip.irq_set_affinity = al_fic_irq_set_affinity;
 	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 	gc->private = fic;
 
-- 
2.47.3

Re: [PATCH v2 5/8] dt-bindings: interrupt-controller: amazon,al-fic: add mask selection

[Conor Dooley]

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

On Sun, Sep 27, 2026 at 08:06:34AM +0000, Eliav Farber wrote:
> The block this driver calls a FIC is the generic Annapurna Labs interrupt
> controller. It is built from groups of up to 32 triggers each; the number
> of groups differs from one controller to another, and each group has its
> own 0x40 register block.
> 
> A node in this binding 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 triggers. A controller with several groups appears as several
> nodes, and controllers cascade when a tree needs more triggers than one
> controller has.
> 
> A group has one cause register and three mask registers, which decide
> which output a set cause bit drives:
> 
>   0x10  info mask    gates the info output
>   0x2c  error mask   gates the error output
>   0x34  fatal mask   gates the fatal output
> 
> The three outputs are aggregated into three separate interrupt trees
> (info, error, fatal); mapping one event to more than one severity is
> forbidden by the interrupt methodology, so a group belongs to exactly one
> tree. Which output a group drives is fixed when the SoC is wired and is
> not discoverable from any register, so it must be described in the
> devicetree.
> 
> The error and fatal outputs, and their mask registers, exist only on
> controller revisions that implement them; earlier revisions provide the
> info output only. A controller reports its revision in the group control
> register (bits 29-28), so the revision itself does not need to appear in
> the devicetree and no per-revision compatible string is needed.
> 
> Describe which output a group drives with a new optional property,
> amazon,al-fic-mask (info, error or fatal; default info).
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> v2: redesigned in response to Conor Dooley's review of v1.
>  - Drop the two proposed per-output compatible strings
>    (amazon,al-fic-error / amazon,al-fic-fatal). A node describes one group,
>    and a controller reports its revision in the group control register
>    (bits 29-28), so no per-revision or per-output compatible is needed.
>  - Instead add an optional amazon,al-fic-mask property (info, error, fatal;
>    default info) selecting which output a group drives. Existing
>    amazon,al-fic nodes keep behaving as before.
>  - Rewrite the commit message to describe the hardware and the group
>    granularity, and add an example fragment to the YAML, as requested.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

Thanks,
Conor.

> 
>  .../interrupt-controller/amazon,al-fic.yaml      | 16 +++++++++++++++-
>  1 file changed, 15 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..1ebf9a427f2f 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/amazon,al-fic.yaml
> @@ -24,6 +24,19 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  amazon,al-fic-mask:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Selects which of the controller's severity outputs this group drives,
> +      and thus which mask register the driver programs. A group has one cause
> +      register and three severity outputs - info, error and fatal - each
> +      gated by its own mask register. Each output is aggregated into a
> +      separate interrupt tree, and a group belongs to exactly one of them.
> +      The error and fatal outputs exist only on controller revisions that
> +      implement them; earlier revisions provide the info output only.
> +    enum: [info, error, fatal]
> +    default: info
> +
>  required:
>    - compatible
>    - reg
> @@ -39,8 +52,9 @@ examples:
>  
>      interrupt-controller@fd8a8500 {
>          compatible = "amazon,al-fic";
> -        reg = <0xfd8a8500 0x1000>;
> +        reg = <0xfd8a8500 0x40>;
>          interrupt-controller;
>          #interrupt-cells = <2>;
>          interrupts = <GIC_SPI 0x0 IRQ_TYPE_LEVEL_HIGH>;
> +        amazon,al-fic-mask = "error";
>      };
> -- 
> 2.47.3
> 

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