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

[Radu Rendec <radu@rendec.net>]

On Sun, 2026-09-27 at 08:06 +0000, Eliav Farber wrote:
> 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.

This is a common problem to all multiplexing irq drivers; not because
the parent interrupt is shared but because changing the affinity of one
child interrupt (by changing the parent interrupt's affinity under the
hood) would have the side effect of changing the affinity of all its
siblings.

I think the correct behavior is to fail with -EINVAL in this case.
If you insist on having the ability to change the child interrupts'
affinity, you may want to look at this patch and do something similar:
https://lore.kernel.org/all/20251128212055.1409093-4-rrendec@redhat.com/

I intentionally provided the lore link instead of the commit id because
the patch is part of a series that added the infrastructure that makes
child interrupt affinity control possible. If you want, you can look at
the previous patches in the series to understand the full context.

> 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;
>