linux-kernel - RE: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume

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:
 <TY3PR01MB113460BE4B4D20305021D85328609A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Thu, 11 Sep 2025 10:43:20 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Claudiu.Beznea <claudiu.beznea@...on.dev>, "geert+renesas@...der.be"
	<geert+renesas@...der.be>, "linus.walleij@...aro.org"
	<linus.walleij@...aro.org>
CC: Claudiu.Beznea <claudiu.beznea@...on.dev>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Claudiu Beznea
	<claudiu.beznea.uj@...renesas.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume

Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@...on.dev>
> Sent: 08 September 2025 15:43
> Subject: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume
> 
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> 
> Commit 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL in
> gpio_irq_{en,dis}able*()") dropped the configuration of ISEL from
> rzg2l_gpio_irq_enable()/rzg2l_gpio_irq_disable() and moved it to
> rzg2l_gpio_child_to_parent_hwirq()/rzg2l_gpio_irq_domain_free() to fix spurious IRQs.
> 
> The resume code used rzg2l_gpio_irq_enable() (called from
> rzg2l_gpio_irq_restore()) to reconfigure the wakeup interrupts. Some drivers (e.g. Ethernet) may also
> reconfigure interrupts in their own code, eventually calling rzg2l_gpio_irq_enable(), when these are
> not wakeup interrupts.
> 
> After commit 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL in
> gpio_irq_{en,dis}able*()"), ISEL was no longer configured properly after resume.
> 
> Fix this by adding rzg2l_gpio_irq_endisable() back into rzg2l_gpio_irq_enable(), and by using its
> unlocked variant in rzg2l_gpio_irq_restore(). Having IRQs enable in rzg2l_gpio_irq_enable() should be
> safe with respect to spurious IRQs, as in the probe case IRQs are enabled anyway in
> rzg2l_gpio_child_to_parent_hwirq(). No spurious IRQs were detected on suspend/resume tests (executed on
> RZ/G3S).

IIRC, I believe the issue is ISEL is not restored during resume. Can we restore this register just like
Schmitt register suspend/restore[1]

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20250911&id=837afa592c6234be82acb5d23e0a39e9befdaa85

Cheers,
Biju

> 
> Fixes: 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL in gpio_irq_{en,dis}able*(")
> Cc: stable@...r.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index b182b3b8a542..6ae1ee3ffc81 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -2428,7 +2428,7 @@ static int rzg2l_gpio_get_gpioint(unsigned int virq, struct rzg2l_pinctrl
> *pctrl  }
> 
>  static void rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl,
> -				     unsigned int hwirq, bool enable)
> +				     unsigned int hwirq, bool enable, bool lock)
>  {
>  	const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[hwirq];
>  	u64 *pin_data = pin_desc->drv_data;
> @@ -2443,12 +2443,16 @@ static void rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl,
>  		addr += 4;
>  	}
> 
> -	spin_lock_irqsave(&pctrl->lock, flags);
> +	if (lock)
> +		spin_lock_irqsave(&pctrl->lock, flags);
> +
>  	if (enable)
>  		writel(readl(addr) | BIT(bit * 8), addr);
>  	else
>  		writel(readl(addr) & ~BIT(bit * 8), addr);
> -	spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> +	if (lock)
> +		spin_unlock_irqrestore(&pctrl->lock, flags);
>  }
> 
>  static void rzg2l_gpio_irq_disable(struct irq_data *d) @@ -2460,15 +2464,22 @@ static void
> rzg2l_gpio_irq_disable(struct irq_data *d)
>  	gpiochip_disable_irq(gc, hwirq);
>  }
> 
> -static void rzg2l_gpio_irq_enable(struct irq_data *d)
> +static void rzg2l_gpio_irq_enable_helper(struct irq_data *d, bool lock)
>  {
>  	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl,
> +gpio_chip);
>  	unsigned int hwirq = irqd_to_hwirq(d);
> 
>  	gpiochip_enable_irq(gc, hwirq);
> +	rzg2l_gpio_irq_endisable(pctrl, hwirq, true, lock);
>  	irq_chip_enable_parent(d);
>  }
> 
> +static void rzg2l_gpio_irq_enable(struct irq_data *d) {
> +	rzg2l_gpio_irq_enable_helper(d, true); }
> +
>  static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int type)  {
>  	return irq_chip_set_type_parent(d, type); @@ -2570,7 +2581,7 @@ static int
> rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
>  		goto err;
>  	}
> 
> -	rzg2l_gpio_irq_endisable(pctrl, child, true);
> +	rzg2l_gpio_irq_endisable(pctrl, child, true, true);
>  	pctrl->hwirq[irq] = child;
>  	irq += pctrl->data->hwcfg->tint_start_index;
> 
> @@ -2617,7 +2628,7 @@ static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl)
>  		spin_lock_irqsave(&pctrl->lock, flags);
>  		ret = rzg2l_gpio_irq_set_type(data, irqd_get_trigger_type(data));
>  		if (!ret && !irqd_irq_disabled(data))
> -			rzg2l_gpio_irq_enable(data);
> +			rzg2l_gpio_irq_enable_helper(data, false);
>  		spin_unlock_irqrestore(&pctrl->lock, flags);
> 
>  		if (ret)
> @@ -2640,7 +2651,7 @@ static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int v
> 
>  		for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
>  			if (pctrl->hwirq[i] == hwirq) {
> -				rzg2l_gpio_irq_endisable(pctrl, hwirq, false);
> +				rzg2l_gpio_irq_endisable(pctrl, hwirq, false, true);
>  				rzg2l_gpio_free(gc, hwirq);
>  				spin_lock_irqsave(&pctrl->bitmap_lock, flags);
>  				bitmap_release_region(pctrl->tint_slot, i, get_order(1));
> --
> 2.43.0