Re: [PATCH v6 7/8] soc: renesas: Add support for Renesas RZ/N1 GPIO Interrupt Multiplexer

[Geert Uytterhoeven]

Hi Hervé,

On Mon, 27 Oct 2025 at 13:36, Herve Codina (Schneider Electric)
<herve.codina@xxxxxxxxxxx> wrote:
> On the Renesas RZ/N1 SoC, GPIOs can generate interruptions. Those
> interruption lines are multiplexed by the GPIO Interrupt Multiplexer in
> order to map 32 * 3 GPIO interrupt lines to 8 GIC interrupt lines.
>
> The GPIO interrupt multiplexer IP does nothing but select 8 GPIO
> IRQ lines out of the 96 available to wire them to the GIC input lines.
>
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@xxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -62,6 +62,7 @@ config ARCH_RZN1
>         select PM
>         select PM_GENERIC_DOMAINS
>         select ARM_AMBA
> +       select RZN1_IRQMUX

I guess it makes some sense to enable this unconditionally...
But as the GPIO driver it relies on is not enabled automatically,
perhaps

    select RZN1_IRQMUX if GPIO_DWAPB

?

>
>  if ARM && ARCH_RENESAS
>
> @@ -459,6 +460,9 @@ config PWC_RZV2M
>  config RST_RCAR
>         bool "Reset Controller support for R-Car" if COMPILE_TEST
>
> +config RZN1_IRQMUX
> +       bool "Renesas RZ/N1 GPIO IRQ multiplexer support" if COMPILE_TEST
> +
>  config SYSC_RZ
>         bool "System controller for RZ SoCs" if COMPILE_TEST
>         select MFD_SYSCON

> --- /dev/null
> +++ b/drivers/soc/renesas/rzn1_irqmux.c

> +static int rzn1_irqmux_parent_args_to_line_index(struct device *dev,
> +                                                const struct of_phandle_args *parent_args)
> +{
> +       /*
> +        * The parent interrupt should be one of the GIC controller.
> +        * Three arguments must be provided.
> +        *  - args[0]: GIC_SPI
> +        *  - args[1]: The GIC interrupt number
> +        *  - args[2]: The interrupt flags
> +        *
> +        * We retrieve the line index based on the GIC interrupt number
> +        * provided.
> +        */
> +
> +       if (parent_args->args_count != 3 ||
> +           parent_args->args[0] != GIC_SPI) {

Fits on a single line.

> +               dev_err(dev, "Invalid interrupt-map item\n");
> +               return -EINVAL;
> +       }
> +
> +       if (parent_args->args[1] < RZN1_IRQMUX_GIC_SPI_BASE ||
> +           parent_args->args[1] >= RZN1_IRQMUX_GIC_SPI_BASE + RZN1_IRQMUX_NUM_OUTPUTS) {
> +               dev_err(dev, "Invalid GIC interrupt %u\n", parent_args->args[1]);
> +               return -EINVAL;
> +       }
> +
> +       return parent_args->args[1] - RZN1_IRQMUX_GIC_SPI_BASE;
> +}
> +
> +static int rzn1_irqmux_setup(struct device *dev, struct device_node *np, u32 __iomem *regs)
> +{
> +       DECLARE_BITMAP(index_done, RZN1_IRQMUX_NUM_OUTPUTS) = {};
> +       struct of_imap_parser imap_parser;
> +       struct of_imap_item imap_item;
> +       int index;
> +       int ret;
> +       u32 tmp;
> +
> +       /* We support only #interrupt-cells = <1> and #address-cells = <0> */
> +       ret = of_property_read_u32(np, "#interrupt-cells", &tmp);
> +       if (ret)
> +               return ret;
> +       if (tmp != 1)
> +               return -EINVAL;
> +
> +       ret = of_property_read_u32(np, "#address-cells", &tmp);
> +       if (ret)
> +               return ret;
> +       if (tmp != 0)
> +               return -EINVAL;
> +
> +       ret = of_imap_parser_init(&imap_parser, np, &imap_item);
> +       if (ret)
> +               return ret;
> +
> +       for_each_of_imap_item(&imap_parser, &imap_item) {
> +               index = rzn1_irqmux_parent_args_to_line_index(dev, &imap_item.parent_args);
> +               if (index < 0) {
> +                       of_node_put(imap_item.parent_args.np);
> +                       return index;
> +               }
> +
> +               if (test_and_set_bit(index, index_done)) {
> +                       of_node_put(imap_item.parent_args.np);
> +                       dev_err(dev, "Mux output line already defined\n");

Perhaps print the actual index? E.g.

    dev_err(dev, "Duplicate GIC_SPI interrupt %u in interrupt-map\n",
            RZN1_IRQMUX_GIC_SPI_BASE + index);

Hmm, having to add the base again would be a reason to let
rzn1_irqmux_parent_args_to_line_index() return the actual GIC interrupt
number instead...

> +                       return -EINVAL;
> +               }
> +
> +               /*
> +                * The child #address-cells is 0 (already checked). The first
> +                * value in imap item is the src hwirq.
> +                */
> +               writel(imap_item.child_imap[0], regs + index);
> +       }
> +
> +       return 0;
> +}
> +
> +static int rzn1_irqmux_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       u32 __iomem *regs;
> +
> +       regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(regs))
> +               return PTR_ERR(regs);
> +
> +       return rzn1_irqmux_setup(dev, np, regs);

As Wolfram already pointed out, this function looks a bit shallow.

> +}

Nothing critical, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds