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

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

On 9/11/25 16:39, Biju Das wrote:
> HI Caludiu,
> 
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> Sent: 11 September 2025 14:24
>> Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Fix ISEL restore on resume
>>
>> 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.
> 
> OK. So enable/disable Keep ISEL configuration as it is, so the pin gpio int always.

Yes

> Which commit 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoidconfiguring ISEL in gpio_irq_{en,dis}able*()")
> is doing.
> 
> The new addition is suspend/resume restores ISEL along with reconfiguring interrupts.
> 
> Is it correct?

This commit only fixes the ISEL restore on resume. The rest of interrupt
reconfiguration on resume was in place from previous commits.

Thank you,
Claudiu

> 
> Cheers,
> Biju
> 
> 
>