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

[PATCH v5 0/9] 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. Print the init error with %pe instead of %lu, so a failure reports a
     legible error code.
  3. Use %pOF in log messages and raise the successful-init message to
     pr_info, so each FIC instance is identifiable at boot.
  4. Keep the device_node in struct al_fic instead of a cached name string
     that aliased node storage, and take a reference on it.
  5. 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. The handler filters the cause register through the generic
     chip's mask cache, so the cache is seeded from the value the driver
     programmed before the parent is requested.
  6. Extend the binding with an optional amazon,al-fic-mask property
     (info, error or fatal; default info) selecting which output a group
     drives.
  7. Add error/fatal output support and FIC v2: version detection from the
     control register and the two extra mask registers.
  8. Add FIC v3 support. v3 reports version id 2 and exposes the same
     group layout as v2, which the existing handling already covers.
  9. Use the interrupt redirection infrastructure to give each child real
     affinity. irq_set_affinity() fails with -EINVAL on a FIC-routed
     interrupt because the FIC has no hardware affinity control and a
     child's handler normally runs in the parent's context; use
     irq_chip_redirect_set_affinity() and generic_handle_demux_domain_irq()
     so a child whose affinity excludes the parent's CPU runs on its
     target CPU in IRQ work context instead.

Changes in v5 - a maintainer rejection and a review thread settled on patch
9, two sashiko-bot findings on the v4 posting, one review comment from
Radu Rendec on patch 4, and two tags. The new patch 2 shifts the
numbering of every later patch by one.
 - patch 9 (was 8): REWRITTEN. Thomas Gleixner rejected the
   IRQ_SET_MASK_OK_DONE approach ("a horrible hack ..."). This version
   instead implements per-child affinity using that infrastructure, per Radu
   Rendec's confirmation of the shape (irq_chip_redirect_set_affinity(),
   generic_handle_demux_domain_irq(), and an irq_pre_redirect callback
   that moves the ack/mask into parent context) and the dw_pci driver he
   and Thomas referenced. Tested on our hardware.
 - new patch 2: print the init error with %pe. al_fic_init_dt() logged
   PTR_ERR(fic) with %lu, which turns a negative error code into a large
   positive number. The bug predates this series and is only visible here
   as context in patch 3.
 - patch 4 (was 3): take a reference on the device_node the driver keeps.
   of_irq_init() stopped holding one past the init callback in commit
   30724547b221 ("of/irq: Fix remaining refcount leaks in of_irq_init()"),
   so the commit message no longer argues from that behaviour. Radu
   Rendec's Reviewed-by is dropped: the code changed.
 - patch 5 (was 4): Radu Rendec's Reviewed-by added.
 - patch 7 (was 6): Radu Rendec's Reviewed-by added.
 - rebased onto v7.3-rc6.

Changes in v4 - all from a sashiko-bot finding on the v3 posting of patch
4. Patches 1, 2, 3, 5 and 8 are unchanged.
 - patch 4: seed gc->mask_cache before the parent is requested and drop
   IRQ_GC_INIT_MASK_CACHE, which only seeds on the first child mapping -
   too late once the handler is shared, and never at all for a group with
   no consumer in the devicetree, leaving the cache at 0 so the handler
   claims interrupts it did not service. Radu Rendec's Reviewed-by is
   dropped: functional change.
 - patch 6: the gc_flags condition and the enum al_fic_version parameter
   go, since patch 4 no longer asks for the flag.
 - patch 7: reduced to the enum addition; the comment it reworded went
   with patch 6's condition. Reviewed-by retained.

Changes in v3 - all from Radu Rendec's review of the v2 series. Patches
1, 2, 5 and 7 are unchanged and carry the tags he and Conor Dooley gave
them; patches 3, 4 and 6 changed; patch 8 is unchanged and its open
question is still being discussed on the v2 thread.
 - patch 3: use of_node_full_name() instead of reaching into
   node->full_name. The commit message is rewritten to say plainly that
   this patch does not fix a lifetime bug - of_irq_init() pins the node
   for the life of the system on a successful init, so node->name was
   never at risk of dangling. The value is making the dependency
   explicit.
 - patch 4: al_fic_irq_handler() no longer derives its return value from
   generic_handle_domain_irq(), which only reports whether the hwirq
   mapping succeeded. It now returns IRQ_HANDLED when the masked CAUSE
   snapshot is non-zero, which is the correct signal for a shared
   interrupt.
 - patch 4: set IRQ_DOMAIN_FLAG_DESTROY_GC on the domain and let
   irq_domain_remove() free the generic chips, instead of calling
   irq_domain_remove_generic_chips() by hand. Both error paths collapse
   into one label. The invalid-free fix from v2 is unaffected; only the
   teardown mechanism changed.
 - patch 4: use of_node_full_name() in the request_irq() call.
 - patch 6: initialise gc_flags to IRQ_GC_INIT_MASK_CACHE at its
   declaration and only clear it on the v2 error/fatal path, dropping the
   else branch.
 - patch 6: use ~0U instead of 0xFFFFFFFF for the mask_cache seed and the
   three mask register writes.
 - patch 6: the commit message now explains the control register
   read-modify-write. Every writable bit in that register resets to 0, so
   preserving the other bits is equivalent to the previous plain write at
   probe time; it is not a behaviour fix.

Changes in v2 - Conor Dooley's review of the v1 binding, plus two bugs
found while re-reading the series.
 - patch 5 (binding): 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.

v4: https://lore.kernel.org/all/20261008090058.38591-1-farbere@amazon.com/
v3: https://lore.kernel.org/all/20261005112458.22291-1-farbere@amazon.com/
v2: https://lore.kernel.org/all/20260927080637.27285-1-farbere@amazon.com/
v1: https://lore.kernel.org/all/20260924062311.37308-1-farbere@amazon.com/

Eliav Farber (9):
  irqchip/al-fic: fix argument alignment and a repeated word
  irqchip/al-fic: use %pe to print init error
  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: support per-child affinity via interrupt redirection

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


base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
-- 
2.47.3

[PATCH v5 1/9] 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>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
v5: no change.

v4: no change.

v3: no change. Added Reviewed-by: Radu Rendec.

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 v5 2/9] irqchip/al-fic: use %pe to print init error

[Eliav Farber]

al_fic_init_dt() logs PTR_ERR(fic) with %lu when al_fic_wire_init()
fails. PTR_ERR() returns a signed long, so printing it with an
unsigned format specifier turns a negative error code into a large
positive value, making it harder to read.

Pass fic to %pe directly instead of unwrapping it with PTR_ERR()
first. %pe accepts the ERR_PTR-encoded pointer itself and prints a
symbolic error name (e.g. -EINVAL) when one is known, falling back
to the plain decimal value otherwise.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v5: new patch.

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

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 35581332b40e..41f7a8390328 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -258,9 +258,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("%s: fail to initialize irqchip (%pe)\n",
+		       node->name, fic);
 		ret = PTR_ERR(fic);
 		goto err_irq_dispose;
 	}
-- 
2.47.3

[PATCH v5 3/9] 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>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
v5: no functional change. The hunk touching the init-failure message
    now has %pe and fic on both sides, because the new patch 2 ahead of
    this one fixed that argument first. This patch still only converts %s
    and node->name to %pOF and node, as reviewed.

v4: no change.

v3: no change. Added Reviewed-by: Radu Rendec.

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 | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 41f7a8390328..1a7f851731b2 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,8 +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 (%pe)\n",
-		       node->name, fic);
+		pr_err("%pOF: fail to initialize irqchip (%pe)\n",
+		       node, fic);
 		ret = PTR_ERR(fic);
 		goto err_irq_dispose;
 	}
-- 
2.47.3

[PATCH v5 4/9] 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() set
from node->name. That string pointed into the device_node, so the
driver already depended on the node outliving init, but nothing in the
struct said so.

Keep the device_node itself and derive the name where it is needed. The
dependency does not go away: of_node_full_name() also returns a pointer
into the node, and irq_alloc_domain_generic_chips() keeps the string it
is given.

The driver therefore needs its own reference. of_irq_init() holds one
across the init callback, but since commit 30724547b221 ("of/irq: Fix
remaining refcount leaks in of_irq_init()") it drops that reference
again when it drains its parent list. Take a reference where the
pointer is stored and drop it on the error path. The driver is
__init-only with no teardown, so on success it is held for the life of
the system.

Deriving the name this way also changes the generic chip name from the
bare node name (e.g. "interrupt-controller") to the full name including
the unit address (e.g. "interrupt-controller@fd8a8500"), which keeps
instances that share a bare name distinguishable.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v5:
 - Take a reference on the device_node where the driver stores it, and
   drop it on the error path. of_irq_init() no longer holds one past the
   init callback: commit 30724547b221 ("of/irq: Fix remaining refcount
   leaks in of_irq_init()") drops it when the parent list is drained.
 - Rewrite the commit message accordingly. It previously argued that
   of_irq_init() never drops the reference on a successful init, which is
   no longer true, and that this patch therefore fixed no lifetime bug.

v4: no change.

v3:
 - Use of_node_full_name() instead of reaching into node->full_name
   directly, as Radu Rendec suggested.
 - Rewrite the commit message to say plainly that this patch does not fix
   a lifetime bug. of_irq_init() takes a reference on the node before
   calling the driver's init callback and does not drop it on a
   successful init, so the node, and the storage node->name points into,
   is pinned for the life of the system either way. The value of keeping
   the device_node is making that dependency explicit, not closing a
   real one.

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 | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 1a7f851731b2..32d8098699a4 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, of_node_full_name(fic->node),
 					     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 = of_node_get(node);
 
 	/* mask out all interrupts */
 	writel_relaxed(0xFFFFFFFF, fic->base + AL_FIC_MASK);
@@ -222,6 +220,7 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 	return fic;
 
 err_free:
+	of_node_put(fic->node);
 	kfree(fic);
 	return ERR_PTR(ret);
 }
@@ -254,7 +253,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 (%pe)\n",
-- 
2.47.3

[PATCH v5 5/9] 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 this instance had
anything pending, so the shared-IRQ core can tell which instance on the
line raised the interrupt. 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.

The handler reads the group's cause register and returns IRQ_HANDLED when
any unmasked cause bit is set, IRQ_NONE otherwise. That is the signal the
shared-IRQ core needs - "did this instance's hardware raise the line" -
rather than the result of dispatching to the child domain.

For that filter to be right, gc->mask_cache must be valid before the parent
is requested. IRQ_GC_INIT_MASK_CACHE seeds it from the mask register only
on the first child mapping, and never at all for a group with no consumer
in the devicetree; until then the cache reads 0 and every latched cause bit
passes the filter. Seed it from the value al_fic_wire_init() programmed and
drop the flag; nothing can change the register in between, so the read
could only have returned that same value.

request_irq() can fail, unlike irq_set_chained_handler_and_data(), so add
an error path for it. Set IRQ_DOMAIN_FLAG_DESTROY_GC on the domain after
creating it, so irq_domain_remove() tears the generic chips down too and
the single call suffices for both the chip-allocation and request_irq()
failure paths.

Co-developed-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Talel Shenhar <talel@amazon.com>
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
v5: no change. Added Reviewed-by: Radu Rendec, given on the v4 posting.

v4:
 - Seed gc->mask_cache before request_irq() and drop
   IRQ_GC_INIT_MASK_CACHE. The flag only seeds the cache on the first
   child mapping, which is too late once the handler is shared and never
   happens for a group with no consumer in the devicetree. Found by
   sashiko-bot on the v3 posting.
 - Drop Radu Rendec's Reviewed-by, since the above is a functional
   change.
 - The commit message no longer claims the domain-sized dispatch loop
   always succeeds; with the cache seeded, only mapped bits reach it.

v3:
 - al_fic_irq_handler() no longer derives IRQ_HANDLED/IRQ_NONE from
   generic_handle_domain_irq(), whose return value only reports whether
   the hwirq to virq mapping succeeded - and since the loop iterates
   exactly NR_FIC_IRQS bits, which is the domain's own size, that mapping
   always succeeds. Return IRQ_HANDLED when the masked CAUSE snapshot is
   non-zero instead, which is the correct signal for a shared interrupt.
 - Set IRQ_DOMAIN_FLAG_DESTROY_GC on the domain and let
   irq_domain_remove() free the generic chips, instead of calling
   irq_domain_remove_generic_chips() by hand. Both error paths now go
   through one label. The invalid-free fix from v2 is unaffected; only the
   teardown mechanism changed.
 - Use of_node_full_name() in the request_irq() call.

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 | 38 +++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 32d8098699a4..8a7eb9009515 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,21 @@ 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);
 	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);
 
-	chained_irq_exit(irqchip, desc);
+	return pending ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static int al_fic_irq_retrigger(struct irq_data *data)
@@ -140,11 +137,17 @@ static int al_fic_register(struct device_node *node,
 		return -ENOMEM;
 	}
 
+	/*
+	 * Let irq_domain_remove() free the generic chips on either error path
+	 * below, instead of calling irq_domain_remove_generic_chips() by hand.
+	 */
+	fic->domain->flags |= IRQ_DOMAIN_FLAG_DESTROY_GC;
+
 	ret = irq_alloc_domain_generic_chips(fic->domain,
 					     NR_FIC_IRQS,
 					     1, of_node_full_name(fic->node),
 					     handle_level_irq,
-					     0, 0, IRQ_GC_INIT_MASK_CACHE);
+					     0, 0, 0);
 	if (ret) {
 		pr_err("fail to allocate generic chip (%d)\n", ret);
 		goto err_domain_remove;
@@ -162,9 +165,22 @@ 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);
+	/*
+	 * Seed the mask cache with the value al_fic_wire_init() programmed,
+	 * rather than having the generic chip read the register back on the
+	 * first child mapping: that is later than the parent is requested, and
+	 * never happens at all for a group with no consumer in the devicetree.
+	 */
+	gc->mask_cache = ~0U;
+
+	ret = request_irq(fic->parent_irq, al_fic_irq_handler,
+			  IRQF_NO_THREAD | IRQF_SHARED,
+			  of_node_full_name(fic->node), fic);
+	if (ret) {
+		pr_err("fail to request irq (%d)\n", ret);
+		goto err_domain_remove;
+	}
+
 	return 0;
 
 err_domain_remove:
-- 
2.47.3

[PATCH v5 6/9] 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>
Reviewed-by: Radu Rendec <radu@rendec.net>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
v5: no change.

v4: no change.

v3: no change. Added Reviewed-by: Radu Rendec and Acked-by: Conor Dooley.

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 v5 7/9] 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).

The driver seeds mask_cache from the value it programs rather than having
the generic chip read the mask register back, so the error and fatal mask
registers are never read - which also sidesteps the v2 erratum where they
always read as 0 regardless of their contents.

Reading CONTROL to get the version field turns the write that follows into
a read-modify-write instead of a value built from CONTROL_MASK_MSI_X
alone. Every RW bit in this register resets to 0, so the two are
equivalent at probe time; the read-modify-write is kept anyway as the
better practice; it costs nothing and does not depend on the reset value
staying 0.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
v5: no change to this patch's own hunks; the new patch 2 shifted the
    init-failure line it carries as context. Added Reviewed-by: Radu
    Rendec, given on the v4 posting.

v4: drop the gc_flags local and the revision-gated condition that cleared
    IRQ_GC_INIT_MASK_CACHE for the v2 error and fatal outputs. Patch 4 now
    seeds mask_cache unconditionally and does not ask for the flag at all,
    so there is nothing left for this patch to gate. The enum
    al_fic_version parameter on al_fic_register() goes with it, since the
    condition was its only user. The v2 read-back erratum is now stated in
    the commit message as a consequence of seeding rather than as the
    reason for a per-revision workaround.

v3:
 - Initialise gc_flags to IRQ_GC_INIT_MASK_CACHE at its declaration and
   only clear it on the FIC v2 error/fatal path, dropping the else
   branch.
 - Use ~0U instead of 0xFFFFFFFF, for the mask_cache seed and for the
   three mask register writes.
 - Explain the control register read-modify-write in the commit message.
   Every writable bit in that register resets to 0, so preserving the
   other bits is equivalent to the previous plain write at probe time. It
   is better practice, not a behaviour fix.

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 | 108 +++++++++++++++++++++++++++++++----
 1 file changed, 98 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 8a7eb9009515..1d524b6aba42 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;
@@ -123,7 +157,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;
@@ -155,7 +190,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;
@@ -194,6 +229,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.
@@ -202,11 +239,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)
@@ -216,22 +255,37 @@ static struct al_fic *al_fic_wire_init(struct device_node *node,
 	fic->parent_irq = parent_irq;
 	fic->node = of_node_get(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(~0U, fic->base + AL_FIC_INFO_MASK);
+	if (version_id > AL_FIC_VERSION_V1) {
+		writel_relaxed(~0U, fic->base + AL_FIC_ERROR_MASK);
+		writel_relaxed(~0U, 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("%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;
 
@@ -241,11 +295,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;
 
@@ -254,6 +335,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);
@@ -269,7 +356,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 (%pe)\n",
 		       node, fic);
-- 
2.47.3

[PATCH v5 8/9] 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.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Radu Rendec <radu@rendec.net>
---
v5: no change.

v4: reduced to the enum addition. The comment hunk this patch carried
    reworded the gc_flags comment in al_fic_register(), which patch 6 no
    longer adds, so there is nothing left to reword. The commit message
    paragraph about the v2 mask_cache workaround not applying to v3 goes
    with it. The enum addition itself is unchanged, so Reviewed-by: Radu
    Rendec is retained.

v3: no change. Added Reviewed-by: Radu Rendec.

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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 1d524b6aba42..13a945a1d312 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 {
-- 
2.47.3

[PATCH v5 9/9] irqchip/al-fic: support per-child affinity via interrupt redirection

[Eliav Farber]

The FIC multiplexes up to 32 sources into one parent GIC SPI, and a
child's handler runs in the parent's context, so a child has no affinity
control of its own. irq_set_affinity() therefore fails with -EINVAL, and
drivers that treat that as fatal cannot probe when their interrupt
arrives through a FIC group. The parent routing cannot be changed on
behalf of one child either, because several FIC groups may share one SPI
and all of them would move with it.

Use the interrupt redirection infrastructure. Dispatch children with
generic_handle_demux_domain_irq() and set irq_set_affinity to
irq_chip_redirect_set_affinity(), so a child whose affinity excludes the
CPU taking the parent interrupt runs on its target CPU in IRQ work
context. The interrupt is still taken on the parent's CPU; only the
child's handler body moves.

Redirection defers the whole flow handler, so the cause bit is cleared in
al_fic_irq_pre_redirect(), which runs in parent context on both the
inline and the redirected path. A level configured group is masked there
too, because its cause bit re-latches while the input stays asserted; the
deferred handle_level_irq() masks again and unmasks when the handler is
done. An edge configured group is only acked, since handle_edge_irq() has
no unconditional unmask. irq_ack is a noop because irq_pre_redirect runs
on every dispatch and the ack must not happen twice.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
v5: rewritten from scratch. Thomas Gleixner rejected the
    IRQ_SET_MASK_OK_DONE approach below ("That's a horrible hack and just
    pretends to solve a problem. We have the interrupt redirection
    infrastructure to handle that correctly."). This patch instead wires
    al-fic into that infrastructure (irq_chip_redirect_set_affinity(),
    generic_handle_demux_domain_irq(), and an irq_pre_redirect callback
    that moves the ack - and, for a level configured group, the mask -
    into parent context), per Radu Rendec's confirmation of this shape on
    the v2 thread and the dw_pci driver he and Thomas pointed at as a
    reference. Tested on our hardware: with a child's affinity pinned
    away from the CPU taking the parent SPI, the handler body runs on the
    target CPU and the ack still happens on the parent's CPU, with no
    re-fire storm on a level configured group, and no change to the
    inline (non-redirected) path. This replaces every version of the
    irq_set_affinity callback below; the v2-v4 history is kept for the
    record of how the approach got here.

v4: no change. The question Radu Rendec raised on this patch - whether an
    unconditional IRQ_SET_MASK_OK_DONE is the right contract, or whether
    it should fail with -EINVAL - is still open.

v3: no change. The question Radu Rendec raised on this patch - whether an
    unconditional IRQ_SET_MASK_OK_DONE is the right contract, or whether
    it should fail with -EINVAL - is still under discussion on the v2
    thread and is answered there.

v2: add Co-developed-by/Signed-off-by: Talel Shenhar. No code change from
    v1.

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

diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
index 13a945a1d312..77cff6308eba 100644
--- a/drivers/irqchip/irq-al-fic.c
+++ b/drivers/irqchip/irq-al-fic.c
@@ -142,7 +142,13 @@ static irqreturn_t al_fic_irq_handler(int irq, void *data)
 	pending &= ~gc->mask_cache;
 
 	for_each_set_bit(hwirq, &pending, NR_FIC_IRQS)
-		generic_handle_domain_irq(domain, hwirq);
+		/*
+		 * Redirect a child whose affinity excludes this CPU to its
+		 * target CPU via irq_work. The ack moves to
+		 * al_fic_irq_pre_redirect(), because the whole flow handler is
+		 * deferred when that happens.
+		 */
+		generic_handle_demux_domain_irq(domain, hwirq);
 
 	return pending ? IRQ_HANDLED : IRQ_NONE;
 }
@@ -157,6 +163,32 @@ static int al_fic_irq_retrigger(struct irq_data *data)
 	return 1;
 }
 
+#ifdef CONFIG_SMP
+/* The ack happens in al_fic_irq_pre_redirect() on both paths instead. */
+static void al_fic_irq_noop(struct irq_data *data) { }
+
+/*
+ * Runs in the parent interrupt context on every dispatch, redirected or not.
+ * Clear the cause bit here: once the flow handler is deferred, the FIC keeps
+ * driving its output until the irq_work runs, and a level parent SPI re-fires.
+ *
+ * A level configured group is masked too, because its cause bit re-latches
+ * while the input stays asserted; the deferred handle_level_irq() unmasks
+ * when the handler is done. An edge configured group is only acked, since
+ * handle_edge_irq() never unmasks unconditionally and the mask would stay.
+ */
+static void al_fic_irq_pre_redirect(struct irq_data *data)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	struct al_fic *fic = gc->private;
+
+	if (fic->state == AL_FIC_CONFIGURED_LEVEL)
+		irq_gc_mask_set_bit(data);
+
+	irq_gc_ack_clr_bit(data);
+}
+#endif /* CONFIG_SMP */
+
 static int al_fic_register(struct device_node *node,
 			   struct al_fic *fic,
 			   enum al_fic_id fic_id)
@@ -195,9 +227,19 @@ static int al_fic_register(struct device_node *node,
 	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;
-	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;
+#ifdef CONFIG_SMP
+	/*
+	 * The FIC has no hardware affinity control, so give each child real
+	 * affinity by redirecting its handler to the target CPU.
+	 */
+	gc->chip_types->chip.irq_ack = al_fic_irq_noop;
+	gc->chip_types->chip.irq_pre_redirect = al_fic_irq_pre_redirect;
+	gc->chip_types->chip.irq_set_affinity = irq_chip_redirect_set_affinity;
+#else
+	gc->chip_types->chip.irq_ack = irq_gc_ack_clr_bit;
+#endif
 	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 	gc->private = fic;
 
-- 
2.47.3