Re: [RFC v11 3/4] soc: microchip: add mpfs gpio interrupt mux driver

[Herve Codina <herve.codina@bootlin.com>]

Hi Conor,

On Fri, 27 Feb 2026 14:52:29 +0000
Conor Dooley <conor@kernel.org> wrote:

> From: Conor Dooley <conor.dooley@microchip.com>
> 
> On PolarFire SoC there are more GPIO interrupts than there are interrupt
> lines available on the PLIC, and a runtime configurable mux is used to
> decide which interrupts are assigned direct connections to the PLIC &
> which are relegated to sharing a line.
> 
> Add a driver so that Linux can set the mux based on the interrupt
> mapping in the devicetree.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
...

> --- a/drivers/soc/microchip/Kconfig
> +++ b/drivers/soc/microchip/Kconfig
> @@ -1,3 +1,14 @@
> +config POLARFIRE_SOC_IRQ_MUX
> +	bool "Microchip PolarFire SoC's GPIO IRQ Mux"
> +	depends on ARCH_MICROCHIP
> +	select REGMAP
> +	select REGMAP_MMIO
> +	default y
> +	help
> +	  Support for the interrupt mux on Polarfire SoC. It sits between
> +	  the GPIO controllers and the PLIC, as only 35 interrupts are shared
> +	  between 3 GPIO controllers with 32 interrupts each.

35 interrupts ?

Previously (other patches) you mentionned 41 (38 + 3).

Also 32 interrutps on each (3 * 32 = 96) but you talked about 70 on previous
patches.

Can you double check or clarify those numbers ?

...
> +++ b/drivers/soc/microchip/mpfs-irqmux.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Largely copied from rzn1_irqmux.c
> + */
> +
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MPFS_IRQMUX_CR		0x54
> +#define MPFS_IRQMUX_NUM_OUTPUTS	70

Is 70 really the outputs ?

According to previous patches, I would say 41 (38+3).

...
> +static int mpfs_irqmux_probe(struct platform_device *pdev)
> +{
> +	DECLARE_BITMAP(line_done, MPFS_IRQMUX_NUM_OUTPUTS) = {};
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct of_imap_parser imap_parser;
> +	struct of_imap_item imap_item;
> +	struct regmap *regmap;
> +	int ret, direct_mode, line, controller, gpio;

Reverse Xmas tree.

> +	u32 tmp, val = 0, old;

...
> +	for_each_of_imap_item(&imap_parser, &imap_item) {
> +
> +		direct_mode = mpfs_irqmux_is_direct_mode(dev, &imap_item.parent_args);
> +		if (direct_mode < 0) {
> +			of_node_put(imap_item.parent_args.np);
> +			return direct_mode;
> +		}
> +
> +		line = imap_item.child_imap[0];
> +		gpio = line % 32;
> +		controller = line / 32;
> +
> +		if (controller > 2) {
> +			of_node_put(imap_item.parent_args.np);
> +			dev_err(dev, "child interrupt number too large: %d\n", line);
> +			return -EINVAL;
> +		}
> +
> +		if (test_and_set_bit(line, line_done)) {

Your bitmap size is MPFS_IRQMUX_NUM_OUTPUTS but you your line variable can
have values from 0 to 95.

Maybe some checks on imap_item.child_imap[0] or line could be added in
order to be be sure that line value will fit in the bitmap.


> +			of_node_put(imap_item.parent_args.np);
> +			dev_err(dev, "Mux output line %d already defined in interrupt-map\n",
> +				line);

line is computed from imap_item.child_imap[0]. It is the input and not the
output.

In rzn1-irqmux.c, the bitmap is used to avoid multiple input lines using the same
output line. Bitmap bits represent outputs.

> +			return -EINVAL;
> +		}
> +
> +		/*
> +		 * There are 41 interrupts assigned to GPIOs, of which 38 are "direct". Since the
> +		 * mux has 32 bits only, 6 of these exclusive/"direct" interrupts remain. These
> +		 * are used by GPIO controller 1's lines 18 to 23. Nothing needs to be done
> +		 * for these interrupts.
> +		 */
> +		if (controller == 1 && gpio >= 18)
> +			continue;
> +
> +		/*
> +		 * The mux has a single register, where bits 0 to 13 mux between GPIO controller
> +		 * 1's 14 GPIOs and GPIO controller 2's first 14 GPIOs. The remaining bits mux
> +		 * between the first 18 GPIOs of controller 1 and the last 18 GPIOS of
> +		 * controller 2. If a bit in the mux's control register is set, the
> +		 * corresponding interrupt line for GPIO controller 0 or 1 will be put in
> +		 * "non-direct" mode. If cleared, the "fabric" controller's will.
> +		 *
> +		 * Register layout:
> +		 *    GPIO 1 interrupt line 17 | mux bit 31 | GPIO 2 interrupt line 31
> +		 *    ...                      | ...        | ...
> +		 *    ...                      | ...        | ...
> +		 *    GPIO 1 interrupt line  0 | mux bit 14 | GPIO 2 interrupt line 14
> +		 *    GPIO 0 interrupt line 13 | mux bit 13 | GPIO 2 interrupt line 13
> +		 *    ...                      | ...        | ...
> +		 *    ...                      | ...        | ...
> +		 *    GPIO 0 interrupt line  0 | mux bit  0 | GPIO 2 interrupt line  0
> +		 *
> +		 * As the binding mandates 70 items, one for each GPIO line, there's no need to
> +		 * handle anything for GPIO controller 2, since the bit will be set for the
> +		 * corresponding line in GPIO controller 0 or 1.

Hum, what happen if the interrupts property is set in the GPIO controller 2 and not
GPIO controllers 0 or 1.

Is it legit ?

If so, should lines coming from GPIO controller 2 be took into account ?

Maybe my comment is not relevant due to some misunderstanding in the not
so obvious mapping.

> +		 */
> +		if (controller == 2)
> +			continue;
> +
> +		/*
> +		 * If in direct mode, the bit is cleared, nothing needs to be done as val is zero
> +		 * initialised and that's the direct mode setting for GPIO controller 0 and 1.
> +		 */
> +		if (direct_mode)
> +			continue;
> +
> +		if (controller == 0)
> +			val |= 1U << gpio;
> +		else
> +			val |= 1U << (gpio + 14);
> +	}
> +
> +	regmap_read(regmap, MPFS_IRQMUX_CR, &old);
> +	regmap_write(regmap, MPFS_IRQMUX_CR, val);
> +
> +	if (val != old)
> +		dev_info(dev, "firmware mux setting of 0x%x overwritten to 0x%x\n", old, val);
> +
> +	return 0;
> +}
> +

Best regards,
Hervé