[PATCH] pinctrl: renesas: rza1: Check pin state before configuring

[PATCH] pinctrl: renesas: rza1: Check pin state before configuring

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add code to the RZ/A1 pinctrl driver to check the state of the pin before
writing any registers. As it is without this patch, resetting the pin state
for every pin regardless of preious state might negatively be affecting
certain shared pins like for instance address and data bus pins.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

  This makes the following patch work with Linux:
  [PATCH] Update r7s72100 Genmai DTS to include NOR Flash pinctrl

  In U-Boot the above DTS change works without any changes most likely
  because the external SDRAM is not in use and on-chip RAM is used instead.

  For the Linux case the SDRAM has been setup and reconfiguring shared
  pins will mess with the memory bus pins and cause the system to lock up.

  Did I get the MUX_FLAGS_SWIO_INPUT | MUX_FLAGS_SWIO_OUTPUT handling right?

  After enabling DEBUG and checking the "Genmai DTS NOR Flash pinctrl" patch
  above it becomes obvious that most pins are skipped however the following
  pins still seem to get configured:
  
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 8 8
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 8 9
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 8 10
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 8 11
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 8 12
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 7 8
   pinctrl-rza1 fcfe3000.pinctrl: Configuring pinmux port pin 7 0

 That translates to A16, A17, A18, A19, A20, RD and CS0.
 The CFI detection is still working on both CS0 and CS1 NOR flash banks.

 drivers/pinctrl/renesas/pinctrl-rza1.c |   83 ++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

--- 0001/drivers/pinctrl/renesas/pinctrl-rza1.c
+++ work/drivers/pinctrl/renesas/pinctrl-rza1.c	2025-07-12 23:51:15.000275500 +0900
@@ -663,6 +663,75 @@ static inline int rza1_pin_get(struct rz
 }
 
 /**
+ * rza1_pin_mux_needs_update() - check pin multiplexing on a single pin
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ * @mux_conf: pin multiplexing descriptor
+ */
+static int rza1_pin_mux_needs_update(struct rza1_pinctrl *rza1_pctl,
+				     struct rza1_mux_conf *mux_conf)
+{
+	struct rza1_port *port = &rza1_pctl->ports[mux_conf->port];
+	unsigned int pin = mux_conf->pin;
+	u8 mux_func = mux_conf->mux_func;
+	u8 mux_flags = mux_conf->mux_flags;
+	u8 mux_flags_from_table;
+
+	/* follow register write logic from rza1_pin_mux_single()
+	 * but instead of programming the hardware check if the
+	 * pin actually needs to be configured or not
+	 *
+	 * we read the register settings and in case it does not
+	 * match the expected value we return 1 right away
+	 *
+	 * return value 0 means all registers are matching
+	 * and no need to perform any register update
+	 */
+
+	/* SWIO pinmux flags coming from DT are high precedence */
+	mux_flags_from_table = rza1_pinmux_get_flags(port->id, pin, mux_func,
+						     rza1_pctl);
+	if (mux_flags)
+		mux_flags |= (mux_flags_from_table & MUX_FLAGS_BIDIR);
+	else
+		mux_flags = mux_flags_from_table;
+
+	mux_func -= 1;
+
+	/* return 1 in case register bit does not match MUX_FLAGS/FUNC */
+
+	if (!!(mux_flags & MUX_FLAGS_BIDIR) !=
+	  !!rza1_get_bit(port, RZA1_PBDC_REG, pin))
+		return 1;
+
+	if (!!(mux_func & MUX_FUNC_PFC_MASK) !=
+	    !!rza1_get_bit(port, RZA1_PFC_REG, pin))
+		return 1;
+
+	if (!!(mux_func & MUX_FUNC_PFCE_MASK) !=
+	    !!rza1_get_bit(port, RZA1_PFCE_REG, pin))
+		return 1;
+
+	if (!!(mux_func & MUX_FUNC_PFCEA_MASK) !=
+	    !!rza1_get_bit(port, RZA1_PFCEA_REG, pin))
+		return 1;
+
+	if (mux_flags & (MUX_FLAGS_SWIO_INPUT | MUX_FLAGS_SWIO_OUTPUT)) {
+		if (!!(mux_func & MUX_FLAGS_SWIO_INPUT) !=
+		    !!rza1_get_bit(port, RZA1_PM_REG, pin))
+			return 1;
+	} else {
+		if (!rza1_get_bit(port, RZA1_PIPC_REG, pin))
+			return 1;
+	}
+
+	if (!rza1_get_bit(port, RZA1_PMC_REG, pin))
+		return 1;
+
+	return 0;
+}
+
+/**
  * rza1_pin_mux_single() - configure pin multiplexing on a single pin
  *
  * @rza1_pctl: RZ/A1 pin controller device
@@ -677,6 +746,20 @@ static int rza1_pin_mux_single(struct rz
 	u8 mux_flags = mux_conf->mux_flags;
 	u8 mux_flags_from_table;
 
+	/* Before touching the hardware check if it is actually needed.
+	 * The reason for doing this is that some pins may be used
+	 * already while the driver operates, for instance address bus
+	 * for a NOR flash might be shared with SDRAM or similar.
+	 * Reconfiguring such a pin might cause the system to lock up.
+	 */
+	if (!rza1_pin_mux_needs_update(rza1_pctl, mux_conf)) {
+		dev_dbg(rza1_pctl->dev, "Skipping pinmux port pin %d %d\n",
+			mux_conf->port, pin);
+		return 0;
+	}
+	dev_dbg(rza1_pctl->dev, "Configuring pinmux port pin %d %d\n",
+		mux_conf->port, pin);
+
 	rza1_pin_reset(port, pin);
 
 	/* SWIO pinmux flags coming from DT are high precedence */

Re: [PATCH] pinctrl: renesas: rza1: Check pin state before configuring

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

Hi Magnus,

On Sat, Jul 12, 2025 at 05:25:38PM +0200, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add code to the RZ/A1 pinctrl driver to check the state of the pin before
> writing any registers. As it is without this patch, resetting the pin state
> for every pin regardless of preious state might negatively be affecting
> certain shared pins like for instance address and data bus pins.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
> 
>   This makes the following patch work with Linux:
>   [PATCH] Update r7s72100 Genmai DTS to include NOR Flash pinctrl

True that.

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

I can't say if it is feasible to have this code in the driver. I leave
this for Geert or the RZ team. But I can say it works with the above DTS
patch.

> +static int rza1_pin_mux_needs_update(struct rza1_pinctrl *rza1_pctl,
> +				     struct rza1_mux_conf *mux_conf)

Minor: I'd make this a bool, though...

> +	if (!!(mux_flags & MUX_FLAGS_BIDIR) !=
> +	  !!rza1_get_bit(port, RZA1_PBDC_REG, pin))
> +		return 1;

... and treturn true / false.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] pinctrl: renesas: rza1: Check pin state before configuring

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Bummer, I overlooked a local modification which prevented the pinmux
settings from being applied :(

So, it still doesn't boot for me. Is it my old U-Boot? I have

"U-Boot 2013.01-rc3 (Sep 09 2013 - 14:57:28)"

Sorry for the noise!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]