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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <175233393885.19419.10468322450742766513.sendpatchset@Bjoern-Magnuss-MacBook-Pro.local>
Date: Sat, 12 Jul 2025 17:25:38 +0200
From: Magnus Damm <damm@...nsource.se>
To: linux-renesas-soc@...r.kernel.org
Cc: Magnus Damm <damm@...nsource.se>,geert+renesas@...der.be,linux-gpio@...r.kernel.org,linus.walleij@...aro.org,linux-kernel@...r.kernel.org,wsa+renesas@...g-engineering.com
Subject: [PATCH] pinctrl: renesas: rza1: Check pin state before configuring

From: Magnus Damm <damm@...nsource.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@...nsource.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 */