[PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations

[PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

There is no need to reconfigure the pin if the pin's configuration values
are same as the reset values. E.g.: PS0 pin configuration for NMI function
is PMC = 1 and PFC = 0 and is same as that of reset values. Currently the
code is first setting it to GPIO HI-Z state and then again reconfiguring
to NMI function leading to spurious IRQ. Drop the unnecessary pin
configurations from the driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Dropped extra space before the == operator.
 * Moved spinlock acquire before reading pfc value.
 * Make sure it is configured for function in PMC register for
   skipping GPIO switch.
v1->v2:
 * Updated commit header and description.
 * Added check in rzg2l_pinctrl_set_pfc_mode() to avoid unnecessary
   configuration
 * Updated rzg2l_pinctrl_pm_setup_pfc() to make changes minimal
 Logs:
 
 STR using SLEEP button:
 root@smarc-rzg3e:~# cat /proc/interrupts | grep SLEEP
129:          0          0          0          0 rzv2h-icu   0 Edge      SLEEP
root@smarc-rzg3e:~# [  103.459191] PM: suspend entry (deep)
[  103.462898] Filesystems sync: 0.000 seconds
[  103.468988] Freezing user space processes
[  103.474815] Freezing user space processes completed (elapsed 0.001 seconds)
[  103.481817] OOM killer disabled.
[  103.485056] Freezing remaining freezable tasks
[  103.490977] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[  103.498387] printk: Suspending console(s) (use no_console_suspend to debug)
NOTICE:  BL2: v2.10.5(release):2.10.5/rz_soc_dev-169-g1410189b0
NOTICE:  BL2: Built : 12:53:12, Jul 15 2025
NOTICE:  BL2: SYS_LSI_MODE: 0x13e06
NOTICE:  BL2: SYS_LSI_DEVID: 0x8679447
NOTICE:  BL2: SYS_LSI_PRR: 0x0
NOTICE:  BL2: Booting BL31
[  103.556250] renesas-gbeth 15c30000.ethernet end0: Link is Down
[  103.568430] Disabling non-boot CPUs ...
[  103.573306] psci: CPU3 killed (polled 0 ms)
[  103.579244] psci: CPU2 killed (polled 4 ms)
[  103.585717] psci: CPU1 killed (polled 0 ms)
[  103.590320] Enabling non-boot CPUs ...
[  103.590529] Detected VIPT I-cache on CPU1
[  103.590577] GICv3: CPU1: found redistributor 100 region 0:0x0000000014960000
[  103.590616] CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
[  103.591462] CPU1 is up
[  103.591560] Detected VIPT I-cache on CPU2
[  103.591583] GICv3: CPU2: found redistributor 200 region 0:0x0000000014980000
[  103.591605] CPU2: Booted secondary processor 0x0000000200 [0x412fd050]
[  103.592176] CPU2 is up
[  103.592275] Detected VIPT I-cache on CPU3
[  103.592298] GICv3: CPU3: found redistributor 300 region 0:0x00000000149a0000
[  103.592319] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[  103.593020] CPU3 is up
[  103.607003] dwmac4: Master AXI performs fixed burst length
[  103.607903] renesas-gbeth 15c30000.ethernet end0: No Safety Features support found
[  103.607923] renesas-gbeth 15c30000.ethernet end0: IEEE 1588-2008 Advanced Timestamp supported
[  103.611435] renesas-gbeth 15c30000.ethernet end0: configuring for phy/rgmii-id link mode
[  103.626991] dwmac4: Master AXI performs fixed burst length
[  103.627881] renesas-gbeth 15c40000.ethernet end1: No Safety Features support found
[  103.627896] renesas-gbeth 15c40000.ethernet end1: IEEE 1588-2008 Advanced Timestamp supported
[  103.631423] renesas-gbeth 15c40000.ethernet end1: configuring for phy/rgmii-id link mode
[  103.824301] OOM killer enabled.
[  103.827439] Restarting tasks: Starting
[  103.831759] Restarting tasks: Done
[  103.835223] random: crng reseeded on system resumption
[  103.840452] PM: suspend exit

root@smarc-rzg3e:~# [  106.180406] renesas-gbeth 15c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx

root@smarc-rzg3e:~# cat /proc/interrupts | grep SLEEP
129:          1          0          0          0 rzv2h-icu   0 Edge      SLEEP
root@smarc-rzg3e:~#

STR using cmdline:
root@smarc-rzg3e:~# echo deep > /sys/power/mem_sleep
echo mem > /sys/power/stateroot@smarc-rzg3e:~# echo mem > /sys/power/state
[ 1196.663529] PM: suspend entry (deep)
[ 1196.667543] Filesystems sync: 0.000 seconds
[ 1196.673064] Freezing user space processes
[ 1196.679409] Freezing user space processes completed (elapsed 0.002 seconds)
[ 1196.686556] OOM killer disabled.
[ 1196.689910] Freezing remaining freezable tasks
[ 1196.696144] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[ 1196.703737] printk: Suspending console(s) (use no_console_suspend to debug)
NOTICE:  BL2: v2.10.5(release):2.10.5/rz_soc_dev-169-g1410189b0
NOTICE:  BL2: Built : 12:53:12, Jul 15 2025
NOTICE:  BL2: SYS_LSI_MODE: 0x13e06
NOTICE:  BL2: SYS_LSI_DEVID: 0x8679447
NOTICE:  BL2: SYS_LSI_PRR: 0x0
NOTICE:  BL2: Booting BL31
[ 1196.747763] renesas-gbeth 15c30000.ethernet end0: Link is Down
[ 1196.760273] Disabling non-boot CPUs ...
[ 1196.764975] psci: CPU3 killed (polled 0 ms)
[ 1196.771159] psci: CPU2 killed (polled 4 ms)
[ 1196.777676] psci: CPU1 killed (polled 0 ms)
[ 1196.780129] Enabling non-boot CPUs ...
[ 1196.780343] Detected VIPT I-cache on CPU1
[ 1196.780391] GICv3: CPU1: found redistributor 100 region 0:0x0000000014960000
[ 1196.780430] CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
[ 1196.781219] CPU1 is up
[ 1196.781315] Detected VIPT I-cache on CPU2
[ 1196.781337] GICv3: CPU2: found redistributor 200 region 0:0x0000000014980000
[ 1196.781358] CPU2: Booted secondary processor 0x0000000200 [0x412fd050]
[ 1196.781867] CPU2 is up
[ 1196.781966] Detected VIPT I-cache on CPU3
[ 1196.781988] GICv3: CPU3: found redistributor 300 region 0:0x00000000149a0000
[ 1196.782010] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[ 1196.782643] CPU3 is up
[ 1196.798955] dwmac4: Master AXI performs fixed burst length
[ 1196.799861] renesas-gbeth 15c30000.ethernet end0: No Safety Features support found
[ 1196.799881] renesas-gbeth 15c30000.ethernet end0: IEEE 1588-2008 Advanced Timestamp supported
[ 1196.803386] renesas-gbeth 15c30000.ethernet end0: configuring for phy/rgmii-id link mode
[ 1196.818938] dwmac4: Master AXI performs fixed burst length
[ 1196.819832] renesas-gbeth 15c40000.ethernet end1: No Safety Features support found
[ 1196.819848] renesas-gbeth 15c40000.ethernet end1: IEEE 1588-2008 Advanced Timestamp supported
[ 1196.823391] renesas-gbeth 15c40000.ethernet end1: configuring for phy/rgmii-id link mode
[ 1197.020126] OOM killer enabled.
[ 1197.023265] Restarting tasks: Starting
[ 1197.027513] Restarting tasks: Done
[ 1197.030980] random: crng reseeded on system resumption
[ 1197.036203] PM: suspend exit
root@smarc-rzg3e:~# echo deep > /sys/power/mem_sleep[ 1199.390705] renesas-gbeth 15c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx
root@smarc-rzg3e:~# cat /proc/interrupts | grep SLEEP
129:          1          0          0          0 rzv2h-icu   0 Edge      SLEEP
root@smarc-rzg3e:~#
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index f524af6f586f..c7eb5566b340 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -541,9 +541,16 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 				       u8 pin, u8 off, u8 func)
 {
 	unsigned long flags;
-	u32 reg;
+	u32 reg, pfc;
 
+	/* Switching to GPIO is not required if reset value is same as func */
+	reg = readb(pctrl->base + PMC(off));
 	spin_lock_irqsave(&pctrl->lock, flags);
+	pfc = readl(pctrl->base + PFC(off));
+	if ((reg & BIT(pin)) && (((pfc >> (pin * 4)) & PFC_MASK) == func)) {
+		spin_unlock_irqrestore(&pctrl->lock, flags);
+		return;
+	}
 
 	/* Set pin to 'Non-use (Hi-Z input protection)'  */
 	reg = readw(pctrl->base + PM(off));
@@ -557,9 +564,8 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 	writeb(reg & ~BIT(pin), pctrl->base + PMC(off));
 
 	/* Select Pin function mode with PFC register */
-	reg = readl(pctrl->base + PFC(off));
-	reg &= ~(PFC_MASK << (pin * 4));
-	writel(reg | (func << (pin * 4)), pctrl->base + PFC(off));
+	pfc &= ~(PFC_MASK << (pin * 4));
+	writel(pfc | (func << (pin * 4)), pctrl->base + PFC(off));
 
 	/* Switch to Peripheral pin function with PMC register */
 	reg = readb(pctrl->base + PMC(off));
@@ -3113,11 +3119,18 @@ 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_val, pfc_mask;
 
 			/* Nothing to do if PFC was not configured before. */
 			if (!(cache->pmc[port] & BIT(pin)))
 				continue;
 
+			pfc_val = readl(pctrl->base + PFC(off));
+			pfc_mask = PFC_MASK << (pin * 4);
+			/* Nothing to do if reset value of the pin is same as cached value */
+			if ((cache->pfc[port] & pfc_mask) == (pfc_val & pfc_mask))
+				continue;
+
 			/* Set pin to 'Non-use (Hi-Z input protection)' */
 			pm &= ~(PM_MASK << (pin * 2));
 			writew(pm, pctrl->base + PM(off));
@@ -3127,8 +3140,8 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
 			writeb(pmc, pctrl->base + PMC(off));
 
 			/* Select Pin function mode. */
-			pfc &= ~(PFC_MASK << (pin * 4));
-			pfc |= (cache->pfc[port] & (PFC_MASK << (pin * 4)));
+			pfc &= ~pfc_mask;
+			pfc |= (cache->pfc[port] & pfc_mask);
 			writel(pfc, pctrl->base + PFC(off));
 
 			/* Switch to Peripheral pin function. */
-- 
2.43.0

Re: [PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations

[Geert Uytterhoeven]

On Tue, 9 Sept 2025 at 12:42, Biju <biju.das.au@gmail.com> wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> There is no need to reconfigure the pin if the pin's configuration values
> are same as the reset values. E.g.: PS0 pin configuration for NMI function
> is PMC = 1 and PFC = 0 and is same as that of reset values. Currently the
> code is first setting it to GPIO HI-Z state and then again reconfiguring
> to NMI function leading to spurious IRQ. Drop the unnecessary pin
> configurations from the driver.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Dropped extra space before the == operator.
>  * Moved spinlock acquire before reading pfc value.
>  * Make sure it is configured for function in PMC register for
>    skipping GPIO switch.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-pinctrl for v6.19.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations

[Geert Uytterhoeven]

Hi Biju,

On Tue, 9 Sept 2025 at 12:42, Biju <biju.das.au@gmail.com> wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> There is no need to reconfigure the pin if the pin's configuration values
> are same as the reset values. E.g.: PS0 pin configuration for NMI function
> is PMC = 1 and PFC = 0 and is same as that of reset values. Currently the
> code is first setting it to GPIO HI-Z state and then again reconfiguring
> to NMI function leading to spurious IRQ. Drop the unnecessary pin
> configurations from the driver.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -541,9 +541,16 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
>                                        u8 pin, u8 off, u8 func)
>  {
>         unsigned long flags;
> -       u32 reg;
> +       u32 reg, pfc;
>
> +       /* Switching to GPIO is not required if reset value is same as func */
> +       reg = readb(pctrl->base + PMC(off));

I am updating the commit to move this assignment inside the spinlock
below.

>         spin_lock_irqsave(&pctrl->lock, flags);
> +       pfc = readl(pctrl->base + PFC(off));
> +       if ((reg & BIT(pin)) && (((pfc >> (pin * 4)) & PFC_MASK) == func)) {
> +               spin_unlock_irqrestore(&pctrl->lock, flags);
> +               return;
> +       }

To ease backporting "[PATCH v2] pinctrl: renesas: rzg2l: Fix ISEL
restore on resume"[1], I am rebasing this commit on top of the latter.

>
>         /* Set pin to 'Non-use (Hi-Z input protection)'  */
>         reg = readw(pctrl->base + PM(off));

[1] https://lore.kernel.org/20250912095308.3603704-1-claudiu.beznea.uj@bp.renesas.com

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Subject: Re: [PATCH v3] pinctrl: renesas: rzg2l: Drop the unnecessary pin configurations
> 
> Hi Biju,
> 
> On Tue, 9 Sept 2025 at 12:42, Biju <biju.das.au@gmail.com> wrote:
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > There is no need to reconfigure the pin if the pin's configuration
> > values are same as the reset values. E.g.: PS0 pin configuration for
> > NMI function is PMC = 1 and PFC = 0 and is same as that of reset
> > values. Currently the code is first setting it to GPIO HI-Z state and
> > then again reconfiguring to NMI function leading to spurious IRQ. Drop
> > the unnecessary pin configurations from the driver.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -541,9 +541,16 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> >                                        u8 pin, u8 off, u8 func)  {
> >         unsigned long flags;
> > -       u32 reg;
> > +       u32 reg, pfc;
> >
> > +       /* Switching to GPIO is not required if reset value is same as func */
> > +       reg = readb(pctrl->base + PMC(off));
> 
> I am updating the commit to move this assignment inside the spinlock
> below.
> 
> >         spin_lock_irqsave(&pctrl->lock, flags);
> > +       pfc = readl(pctrl->base + PFC(off));
> > +       if ((reg & BIT(pin)) && (((pfc >> (pin * 4)) & PFC_MASK) == func)) {
> > +               spin_unlock_irqrestore(&pctrl->lock, flags);
> > +               return;
> > +       }
> 
> To ease backporting "[PATCH v2] pinctrl: renesas: rzg2l: Fix ISEL
> restore on resume"[1], I am rebasing this commit on top of the latter.


It is ok to me.

Cheers,
Biju