linux-kernel - Re: [PATCH v5 7/8] soc: renesas: Add support for Renesas RZ/N1 GPIO Interrupt Multiplexer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMuHMdX=_6MGgti2NEL6FaChBhefFLBdjeam5Zts3+Yf-Ps3Gg@mail.gmail.com>
Date: Fri, 24 Oct 2025 09:24:25 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Wolfram Sang <wsa+renesas@...g-engineering.com>, 
	Hoan Tran <hoan@...amperecomputing.com>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Magnus Damm <magnus.damm@...il.com>, Saravana Kannan <saravanak@...gle.com>, 
	Serge Semin <fancer.lancer@...il.com>, Phil Edworthy <phil.edworthy@...esas.com>, 
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	Pascal Eberhard <pascal.eberhard@...com>, Miquel Raynal <miquel.raynal@...tlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v5 7/8] soc: renesas: Add support for Renesas RZ/N1 GPIO
 Interrupt Multiplexer

Hi Hervé,

On Thu, 23 Oct 2025 at 15:21, Herve Codina <herve.codina@...tlin.com> wrote:
> On Thu, 23 Oct 2025 13:30:53 +0200
> Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > > I have in mind a use case that can lead to a non-contiguous mapping.
> > >
> > > The RZ/N1 SoC embeds a Cortex-M3 CPU. This CPU can use GPIOs and
> > > some of them for interrupt purpose. In that case, those GPIOs have
> > > to be routed to the interrupt line expected by the Cortex-M3.
> > >
> > > And so, we have some interrupts reserved for CPUs running Linux and
> > > some others for the Cortex-M3.
> > >
> > > Among those reserved interrupts may some are not used.
> > >
> > > for instance:
> > >   Interrupt 103, 102: Reserved and used by Linux
> > >   Interrupt 103: Reserved for Linux but not used -> Hole in the mapping
> > >   Interrupt 104: Reserved and used my Cortex-M3 (need to be routed by Linux)
> >
> > 102 does not seem to  be correct?
>
> My bad, my example was wrong.
>    Interrupt 103, 104: Reserved and used by Linux
>    Interrupt 105: Reserved for Linux but not used -> Hole in the mapping
>    Interrupt 106: Reserved and used my Cortex-M3 (need to be routed by Linux)

OK, much better!

> > > I don't know if this use case is relevant but I think we should be too restrictive
> > > on the mapping and so accept holes.
> > >
> > > With that in mind, I let you confirm that you still prefer to have a mapping
> > > without any holes. A future patch to support that is always possible.
> >
> > While that would indeed be a non-discontiguous mapping, I do not see how
> > it is related to rzn1_irqmux_output_lines[] in the driver.  That array
> > would still contain the same contiguous values 103..110, right?
>
> The array rzn1_irqmux_output_lines is still contiguous yes but the mapping
> defined in irq-map property no.
>
> Looking back again at your proposal, indeed I can remove the following loop:
>         for (i = 0; i < output_lines_count; i++) {
>                if (parent_args->args[1] == output_lines[i])
>                        return i;
>         }
>
> With just
>         if (parent_args->args[1] >= RZN1_IRQMUX_SPI_BASE &&
>             parent_args->args[1] < RZN1_IRQMUX_SPI_BASE + RZN1_IRQMUX_NUM_IRQS) {
>                 return parent_args->args[1] - RZN1_IRQMUX_SPI_BASE;
>
>         dev_err(dev, "Invalid GIC interrupt %u\n", parent_args->args[1]);
>         return -EINVAL;

Good. I like simpler code ;-)

BTW, please invert the logic, i.e. bail out early in case of error.

> > Sorry, I haven't been following the development of this driver that
> > closely (RZ/N1 is completely different from e.g. R-Car, and I never
> > had access to an RZ/N1 platform), so perhaps I am missing something.
> > Why does the user have to specify an interrupt-map in DT? Can't the
> > driver create the mapping dynamically, based actual usage of the
> > GPIOs? I.e. the first 8 GPIOs that ask for interrupt functionality
> > receive it, and are mapped to an available GIC interrupt?
> > I believe this is how rzg2l-irqc works, mapping up to 32 GPIO interrupts
> > to 32 GIC (TINT) interrupts.
>
> I think the main difference with rzg2l-irqc is that the RZ/N1 irq mux is
> clearly not an interrupt controller.
>
> It is just a mux with 96 inputs (GPIO lines coming from several GPIO
> controller) and 8 outputs (connected to the GIC).
>
> It is represented as an interrupt nexus node and has an interrupt-map property.
> to describe the routing.
>
> The interrupt-map property cannot be dynamically created.
>
> Also, the routing is necessary even if the related GPIO is not used by Linux.
> This GPIO can be used as a GPIO input interrupt line by the Cortex M3.
>
> If the irq mux driver performs the routing only on Linux GPIO usage, it will
> not route GPIOs depending on Cortex M3 internal usage.

Thanks, makes sense!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

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