Hi Claudiu,
> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Sent: 03 July 2025 09:57
> Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Don't switch to GPIO during resume
>
> Hi, Biju,
>
> On 01.07.2025 13:01, Biju Das wrote:
> > Even though some pins are set correctly during resume(e.g.: PS0), due
> > to the unconditional switch to GPIO for restoring the PFC register is
> > triggering spurious IRQ on RZ/G3E. So avoid switch to GPIO if the pin
> > is configured correctly during resume.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 35
> > ++++++++++++++++---------
> > 1 file changed, 22 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index 2a10ae0bf5bd..09ee771b1e36 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -3118,27 +3118,36 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
> > pm = readw(pctrl->base + PM(off));
> > for_each_set_bit(pin, &pinmap, max_pin) {
> > struct rzg2l_pinctrl_reg_cache *cache = pctrl->cache;
> > + u32 pfc_mask;
> > + u32 pfc_val;
> > + u8 pmc_val;
> >
> > /* Nothing to do if PFC was not configured before. */
> > if (!(cache->pmc[port] & BIT(pin)))
> > continue;
> >
> > - /* Set pin to 'Non-use (Hi-Z input protection)' */
> > - pm &= ~(PM_MASK << (pin * 2));
> > - writew(pm, pctrl->base + PM(off));
> > + pfc_val = readl(pctrl->base + PFC(off));
> > + pmc_val = readb(pctrl->base + PMC(off)) & BIT(pin);
> > + pfc_mask = PFC_MASK << (pin * 4);
> >
> > - /* Temporarily switch to GPIO mode with PMC register */
> > - pmc &= ~BIT(pin);
> > - writeb(pmc, pctrl->base + PMC(off));
> > + if (!pmc_val || ((cache->pfc[port] & pfc_mask) != (pfc_val &
> > +pfc_mask))) {
>
> We should be restoring here the previous Linux state. This function should not be executed for pins
> that weren't previously (before suspending) configured by Linux. This condition:
Yes, but should not generate spurious IRQ.
>
> if (!(cache->pmc[port] & BIT(pin)))
> should allow avoiding this.
>
> Checking pmc_val here takes into account also the settings that were done (while suspend/resume) by
> other applications (e.g. bootloader involved in the resume process).
Currently SLEEP button PS0 has reset value PMC=1, PFC=0
On Resume from STR, the pin is in correct state PMC=1, PFC=0.
But on the resume code, you are making PMC=0, PM=0 to an intermediate switch
which is generating spurious IRQ and then again making it as PMC=1, PFC=0.
Basically, the patch I sent is a fix.
It avoids, intermediate switch to GPIO and avoids spurious IRQ, if the PIN
Is correctly configured during resume.
Cheers,
Biju
