Re: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Biju,

On 9/11/25 13:43, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu <claudiu.beznea@tuxon.dev>
>> Sent: 08 September 2025 15:43
>> Subject: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.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.

Yes

> Can we restore this register just like
> Schmitt register suspend/restore[1]

The IA55 would have to be configured for interrupts as well. Doing it in
other order will lead to spurious interrupts while resuming. The commit
254203f9a94c ("pinctrl: renesas: rzg2l: Add suspend/resume support") that
introduced this approach, mentions the following:

    Because interrupt signals are routed to IA55 interrupt controller and
    IA55 interrupt controller resumes before pin controller, patch restores
    also the configured interrupts just after pin settings are restored to
    avoid invalid interrupts while resuming.

Thank you,
Claudiu

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