linux-kernel - RE: [PATCH 4/8] pinctrl: renesas: rzt2h: add GPIO IRQ chip to handle interrupts

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <TYYPR01MB1395557CE23154BC30C454C8585A6A@TYYPR01MB13955.jpnprd01.prod.outlook.com>
Date: Thu, 4 Dec 2025 15:36:39 +0000
From: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
To: Linus Walleij <linusw@...nel.org>
CC: Geert Uytterhoeven <geert+renesas@...der.be>, Linus Walleij
	<linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	magnus.damm <magnus.damm@...il.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 4/8] pinctrl: renesas: rzt2h: add GPIO IRQ chip to handle
 interrupts

> From: Linus Walleij <linusw@...nel.org>
> Sent: Thursday, December 4, 2025 1:34 AM
> 
> Hi Cosmin,
> 
> thanks for your patch!
> 

Hi Linus, thank you for your feedback.

> On Fri, Nov 21, 2025 at 12:27 PM Cosmin Tanislav
> <cosmin-gabriel.tanislav.xa@...esas.com> wrote:
> 
> > The Renesas RZ/T2H (R9A09G077) and Renesas RZ/N2H (R9A09G087) SoCs have
> > IRQ-capable pins handled by the ICU, which forwards them to the GIC.
> >
> > The ICU supports 16 IRQ lines, the pins map to these lines arbitrarily,
> > and the mapping is not configurable.
> >
> > Add a GPIO IRQ chip that can be used to configure these pins as IRQ
> > lines.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> 
> Mention in the commit that this is achieved with a
> hierarchical IRQ domain please. (I really like how this
> was done!) Also mention that wakeup capability is
> also implemented as part of the patch.
> 

Ack, I will reword the commit message and include your suggestions.

> You probably need a:
> 
> select IRQ_DOMAIN_HIERARCHY
> 
> In the rzt2h Kconfig entry?
> 

Yes, along with a select GPIOLIB_IRQCHIP.

I will add both in the next version.

> > +static int rzt2h_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
> > +{
> > +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +       struct rzt2h_pinctrl *pctrl = container_of(gc, struct rzt2h_pinctrl, gpio_chip);
> > +       int ret;
> > +
> > +       ret = irq_chip_set_wake_parent(d, on);
> > +       if (ret)
> > +               return ret;
> > +
> 
> Add a comment here:
> 
> /*
>  * If any of the IRQs are in use, then put the entire pin controller
>  * on the device wakeup path.
>  */
> 

Ack.

> > +       if (on)
> > +               atomic_inc(&pctrl->wakeup_path);
> > +       else
> > +               atomic_dec(&pctrl->wakeup_path);
> 
> BTW this is an elegant piece of code I think a lot of other drivers
> need...
> 
> > +static int rzt2h_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> > +                                           unsigned int child,
> > +                                           unsigned int child_type,
> > +                                           unsigned int *parent,
> > +                                           unsigned int *parent_type)
> > +{
> > +       struct rzt2h_pinctrl *pctrl = gpiochip_get_data(gc);
> > +       u8 port = RZT2H_PIN_ID_TO_PORT(child);
> > +       u8 pin = RZT2H_PIN_ID_TO_PIN(child);
> > +       u8 parent_irq;
> > +
> > +       parent_irq = rzt2h_gpio_irq_map[child];
> > +       if (parent_irq < RZT2H_INTERRUPTS_START)
> > +               return -EINVAL;
> > +
> > +       if (test_and_set_bit(parent_irq - RZT2H_INTERRUPTS_START,
> > +                            pctrl->used_irqs))
> > +               return -EBUSY;
> > +
> > +       rzt2h_pinctrl_set_pfc_mode(pctrl, port, pin, PFC_FUNC_INTERRUPT);
> > +
> > +       *parent = parent_irq;
> > +       *parent_type = child_type;
> > +
> > +       return 0;
> > +}
> 
> Complex, but easy to follow, understand and debug.
> Good job here!
> 

Thank you!

> Yours,
> Linus Walleij