[PATCH v2 00/13] Add PFC support for Renesas RZ/V2H(P) SoC

[PATCH v2 00/13] Add PFC support for Renesas RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to add PFC (Pin Function Controller) support for
Renesas RZ/V2H(P) SoC. The PFC block on RZ/V2H(P) is almost similar to
one found on the RZ/G2L family with couple of differences. To able to
re-use the use the existing driver for RZ/V2H(P) SoC function pointers
are introduced based on the SoC changes.


RFC->v2
- Fixed review comments pointed by Rob
- Incorporated changes suggested by Claudiu
- Fixed build error reported for m68K
- Dropped IOLH groups as we will be passing register values
- Fixed configs for dedicated pins
- Added support for slew-rate and bias settings
- Added support for OEN

RFC: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240326222844.1422948-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (13):
  dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Remove the check from the
    object
  dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC
  pinctrl: renesas: pinctrl-rzg2l: Allow more bits for pin configuration
  pinctrl: renesas: pinctrl-rzg2l: Allow parsing of variable
    configuration for all architectures
  pinctrl: renesas: pinctrl-rzg2l: Validate power registers for SD and
    ETH
  pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
    locking/unlocking the PFC register
  pinctrl: renesas: pinctrl-rzg2l: Add function pointer for writing to
    PMC register
  pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
    reading/writing OEN register
  pinctrl: renesas: pinctrl-rzg2l: Add support to configure the
    slew-rate
  pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down
    the pins
  pinctrl: renesas: pinctrl-rzg2l: Pass pincontrol device pointer to
    pinconf_generic_parse_dt_config()
  pinctrl: renesas: pinctrl-rzg2l: Add support for custom parameters
  pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC

 .../pinctrl/renesas,rzg2l-pinctrl.yaml        |  40 +-
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 640 ++++++++++++++++--
 2 files changed, 617 insertions(+), 63 deletions(-)

-- 
2.34.1

[PATCH v2 01/13] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Remove the check from the object

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Drop the bogus check from object as this didn't really add restriction
check.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
RFC->v2
- Updated commit message
- Collected RB tag from Rob
---
 .../bindings/pinctrl/renesas,rzg2l-pinctrl.yaml   | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
index 4d5a957fa232..881e992adca3 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
@@ -79,21 +79,6 @@ additionalProperties:
         - $ref: pincfg-node.yaml#
         - $ref: pinmux-node.yaml#
 
-        - if:
-            properties:
-              compatible:
-                contains:
-                  enum:
-                    - renesas,r9a08g045-pinctrl
-          then:
-            properties:
-              drive-strength: false
-              output-impedance-ohms: false
-              slew-rate: false
-          else:
-            properties:
-              drive-strength-microamp: false
-
       description:
         Pin controller client devices use pin configuration subnodes (children
         and grandchildren) for desired pin configuration.
-- 
2.34.1

[PATCH v2 02/13] dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add documentation for the pin controller found on the Renesas RZ/V2H(P)
(R9A09G057) SoC. The RZ/V2H PFC varies slightly compared to the RZ/G2L
family:
- Additional bits need to be set during pinmuxing.
- The GPIO pin count is different.

Hence, a SoC-specific compatible string, 'renesas,r9a09g057-pinctrl', is
added for the RZ/V2H(P) SoC.

Also, add the 'renesas,output-impedance' property. The drive strength
setting on RZ/V2H(P) depends on the different power rails coming out from
the PMIC (connected via I2C). These power rails (required for drive
strength) can be 1.2V, 1.8V, or 3.3V.

Pins are grouped into 4 groups:

Group 1: Impedance
- 150/75/38/25 ohms (at 3.3V)
- 130/65/33/22 ohms (at 1.8V)

Group 2: Impedance
- 50/40/33/25 ohms (at 1.8V)

Group 3: Impedance
- 150/75/37.5/25 ohms (at 3.3V)
- 130/65/33/22 ohms (at 1.8V)

Group 4: Impedance
- 110/55/30/20 ohms (at 1.8V)
- 150/75/38/25 ohms (at 1.2V)

The 'renesas,output-impedance' property, as documented, can be
[0, 1, 2, 3], indicating x1/x2/x3/x4 drive strength.

As power rail information may not be available very early in the boot
process, the 'renesas,output-impedance' property is added instead of
reusing the 'output-impedance-ohms' property.

Also, allow bias-disable, bias-pull-down and bias-pull-up properties
as these can be used to configure the pins.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- Renamed renesas-rzv2h,output-impedance -> renesas,output-impedance
- Updated values for renesas,output-impedance
- Added bias properties
---
 .../pinctrl/renesas,rzg2l-pinctrl.yaml        | 25 ++++++++++++++++---
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
index 881e992adca3..089b3ce61bac 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
@@ -26,6 +26,7 @@ properties:
               - renesas,r9a07g043-pinctrl # RZ/G2UL{Type-1,Type-2} and RZ/Five
               - renesas,r9a07g044-pinctrl # RZ/G2{L,LC}
               - renesas,r9a08g045-pinctrl # RZ/G3S
+              - renesas,r9a09g057-pinctrl # RZ/V2H(P)
 
       - items:
           - enum:
@@ -66,10 +67,14 @@ properties:
     maxItems: 1
 
   resets:
-    items:
-      - description: GPIO_RSTN signal
-      - description: GPIO_PORT_RESETN signal
-      - description: GPIO_SPARE_RESETN signal
+    oneOf:
+      - items:
+          - description: GPIO_RSTN signal
+          - description: GPIO_PORT_RESETN signal
+          - description: GPIO_SPARE_RESETN signal
+      - items:
+          - description: PFC main reset
+          - description: Reset for the control register related to WDTUDFCA and WDTUDFFCM pins
 
 additionalProperties:
   anyOf:
@@ -111,6 +116,18 @@ additionalProperties:
         output-high: true
         output-low: true
         line-name: true
+        bias-disable: true
+        bias-pull-down: true
+        bias-pull-up: true
+        renesas,output-impedance:
+          description: |
+            Output impedance for pins on RZ/V2H(P) SoC.
+            0: Corresponds to x1
+            1: Corresponds to x2
+            2: Corresponds to x3
+            3: Corresponds to x4
+          $ref: /schemas/types.yaml#/definitions/uint32
+          enum: [0, 1, 2, 3]
 
     - type: object
       additionalProperties:
-- 
2.34.1

[PATCH v2 03/13] pinctrl: renesas: pinctrl-rzg2l: Allow more bits for pin configuration

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The pin configuration bits have been growing for every new SoCs being
added for the pinctrl-rzg2l driver which would mean updating the macros
every time for each new configuration. To avoid this allocate additional
bits for pin configuration by relocating the known fixed bits to the very
end of the configuration.

Also update the size of 'cfg' to 'u64' to allow more configuration bits in
the 'struct rzg2l_variable_pin_cfg'.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- Merged the macros and rzg2l_variable_pin_cfg changes into single patch
- Updated types for the config changes
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 30 ++++++++++++++-----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index dbcf009837ef..9bb9cc63f9df 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -78,9 +78,9 @@
 					 PIN_CFG_FILNUM | \
 					 PIN_CFG_FILCLKSEL)
 
-#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(35, 28)
-#define PIN_CFG_PIN_REG_MASK		GENMASK(27, 20)
-#define PIN_CFG_MASK			GENMASK(19, 0)
+#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(62, 55)
+#define PIN_CFG_PIN_REG_MASK		GENMASK_ULL(54, 47)
+#define PIN_CFG_MASK			GENMASK_ULL(46, 0)
 
 /*
  * m indicates the bitmap of supported pins, a is the register index
@@ -102,8 +102,8 @@
  * (b * 8) and f is the pin configuration capabilities supported.
  */
 #define RZG2L_SINGLE_PIN		BIT_ULL(63)
-#define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK(30, 24)
-#define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK(22, 20)
+#define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK_ULL(62, 56)
+#define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK_ULL(55, 53)
 
 #define RZG2L_SINGLE_PIN_PACK(p, b, f)	(RZG2L_SINGLE_PIN | \
 					 FIELD_PREP_CONST(RZG2L_SINGLE_PIN_INDEX_MASK, (p)) | \
@@ -241,9 +241,9 @@ struct rzg2l_dedicated_configs {
  * @pin: port pin
  */
 struct rzg2l_variable_pin_cfg {
-	u32 cfg:20;
-	u32 port:5;
-	u32 pin:3;
+	u64 cfg:46;
+	u64 port:5;
+	u64 pin:3;
 };
 
 struct rzg2l_pinctrl_data {
@@ -1082,7 +1082,8 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 	const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[_pin];
 	u64 *pin_data = pin->drv_data;
 	unsigned int arg = 0;
-	u32 off, cfg;
+	u32 off;
+	u64 cfg;
 	int ret;
 	u8 bit;
 
@@ -1186,7 +1187,8 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 	u64 *pin_data = pin->drv_data;
 	enum pin_config_param param;
 	unsigned int i, arg, index;
-	u32 cfg, off;
+	u32 off;
+	u64 cfg;
 	int ret;
 	u8 bit;
 
@@ -2414,9 +2416,9 @@ static void rzg2l_pinctrl_pm_setup_regs(struct rzg2l_pinctrl *pctrl, bool suspen
 
 	for (u32 port = 0; port < nports; port++) {
 		bool has_iolh, has_ien;
-		u32 off, caps;
+		u64 cfg, caps;
 		u8 pincnt;
-		u64 cfg;
+		u32 off;
 
 		cfg = pctrl->data->port_pin_configs[port];
 		off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg);
@@ -2460,12 +2462,14 @@ static void rzg2l_pinctrl_pm_setup_regs(struct rzg2l_pinctrl *pctrl, bool suspen
 static void rzg2l_pinctrl_pm_setup_dedicated_regs(struct rzg2l_pinctrl *pctrl, bool suspend)
 {
 	struct rzg2l_pinctrl_reg_cache *cache = pctrl->dedicated_cache;
+	u64 caps;
+	u32 i;
 
 	/*
 	 * Make sure entries in pctrl->data->n_dedicated_pins[] having the same
 	 * port offset are close together.
 	 */
-	for (u32 i = 0, caps = 0; i < pctrl->data->n_dedicated_pins; i++) {
+	for (i = 0, caps = 0; i < pctrl->data->n_dedicated_pins; i++) {
 		bool has_iolh, has_ien;
 		u32 off, next_off = 0;
 		u64 cfg, next_cfg;
-- 
2.34.1

[PATCH v2 04/13] pinctrl: renesas: pinctrl-rzg2l: Allow parsing of variable configuration for all architectures

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Enable parsing of variable configuration for all architectures. This patch
is in preparation for adding support for the RZ/V2H SoC, which utilizes the
ARM64 architecture and features port pins with variable configuration.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- No change
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 9bb9cc63f9df..c944d94b9a36 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -322,7 +322,6 @@ struct rzg2l_pinctrl {
 
 static const u16 available_ps[] = { 1800, 2500, 3300 };
 
-#ifdef CONFIG_RISCV
 static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
 					      u64 pincfg,
 					      unsigned int port,
@@ -339,6 +338,7 @@ static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
 	return 0;
 }
 
+#ifdef CONFIG_RISCV
 static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
 	{
 		.port = 20,
@@ -2299,13 +2299,11 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl)
 		if (i && !(i % RZG2L_PINS_PER_PORT))
 			j++;
 		pin_data[i] = pctrl->data->port_pin_configs[j];
-#ifdef CONFIG_RISCV
 		if (pin_data[i] & PIN_CFG_VARIABLE)
 			pin_data[i] = rzg2l_pinctrl_get_variable_pin_cfg(pctrl,
 									 pin_data[i],
 									 j,
 									 i % RZG2L_PINS_PER_PORT);
-#endif
 		pins[i].drv_data = &pin_data[i];
 	}
 
-- 
2.34.1

[PATCH v2 05/13] pinctrl: renesas: pinctrl-rzg2l: Validate power registers for SD and ETH

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

On RZ/V2H(P) SoC, the power registers for SD and ETH do not exist,
resulting in invalid register offsets. Ensure that the register offsets
are valid before any read/write operations are performed. If the power
registers are not available, both SD and ETH will be set to '0'.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- Update check to != 0 instead of -EINVAL
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index c944d94b9a36..bec4685b4681 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2583,8 +2583,10 @@ static int rzg2l_pinctrl_suspend_noirq(struct device *dev)
 	rzg2l_pinctrl_pm_setup_dedicated_regs(pctrl, true);
 
 	for (u8 i = 0; i < 2; i++) {
-		cache->sd_ch[i] = readb(pctrl->base + SD_CH(regs->sd_ch, i));
-		cache->eth_poc[i] = readb(pctrl->base + ETH_POC(regs->eth_poc, i));
+		if (regs->sd_ch)
+			cache->sd_ch[i] = readb(pctrl->base + SD_CH(regs->sd_ch, i));
+		if (regs->eth_poc)
+			cache->eth_poc[i] = readb(pctrl->base + ETH_POC(regs->eth_poc, i));
 	}
 
 	cache->qspi = readb(pctrl->base + QSPI);
@@ -2615,8 +2617,10 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
 	writeb(cache->qspi, pctrl->base + QSPI);
 	writeb(cache->eth_mode, pctrl->base + ETH_MODE);
 	for (u8 i = 0; i < 2; i++) {
-		writeb(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i));
-		writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
+		if (regs->sd_ch)
+			writeb(cache->sd_ch[i], pctrl->base + SD_CH(regs->sd_ch, i));
+		if (regs->eth_poc)
+			writeb(cache->eth_poc[i], pctrl->base + ETH_POC(regs->eth_poc, i));
 	}
 
 	rzg2l_pinctrl_pm_setup_pfc(pctrl);
-- 
2.34.1

[PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls
writing to both PFC and PMC registers. Additionally, BIT(7) B0WI is
undocumented for the PWPR register on RZ/V2H(P) SoC. To accommodate these
differences across SoC variants, introduce the set_pfc_mode() and
pm_set_pfc() function pointers.

Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is now
called before PMC read/write and pwpr_pfc_lock() call is now called after
PMC read/write this is to keep changes minimal for RZ/V2H(P).

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- Introduced function pointer for (un)lock
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 51 ++++++++++++++++---------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index bec4685b4681..0840fda7ca69 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -246,6 +246,8 @@ struct rzg2l_variable_pin_cfg {
 	u64 pin:3;
 };
 
+struct rzg2l_pinctrl;
+
 struct rzg2l_pinctrl_data {
 	const char * const *port_pins;
 	const u64 *port_pin_configs;
@@ -256,6 +258,8 @@ struct rzg2l_pinctrl_data {
 	const struct rzg2l_hwcfg *hwcfg;
 	const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
 	unsigned int n_variable_pin_cfg;
+	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
+	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
 };
 
 /**
@@ -462,7 +466,6 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
 static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 				       u8 pin, u8 off, u8 func)
 {
-	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
 	unsigned long flags;
 	u32 reg;
 
@@ -473,27 +476,23 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 	reg &= ~(PM_MASK << (pin * 2));
 	writew(reg, pctrl->base + PM(off));
 
+	pctrl->data->pwpr_pfc_unlock(pctrl);
+
 	/* Temporarily switch to GPIO mode with PMC register */
 	reg = readb(pctrl->base + PMC(off));
 	writeb(reg & ~BIT(pin), pctrl->base + PMC(off));
 
-	/* Set the PWPR register to allow PFC register to write */
-	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
-	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
-
 	/* 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));
 
-	/* Set the PWPR register to be write-protected */
-	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
-	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
-
 	/* Switch to Peripheral pin function with PMC register */
 	reg = readb(pctrl->base + PMC(off));
 	writeb(reg | BIT(pin), pctrl->base + PMC(off));
 
+	pctrl->data->pwpr_pfc_lock(pctrl);
+
 	spin_unlock_irqrestore(&pctrl->lock, flags);
 };
 
@@ -2519,12 +2518,8 @@ static void rzg2l_pinctrl_pm_setup_dedicated_regs(struct rzg2l_pinctrl *pctrl, b
 static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
 {
 	u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT;
-	const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
-	const struct rzg2l_register_offsets *regs = &hwcfg->regs;
 
-	/* Set the PWPR register to allow PFC register to write. */
-	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
-	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
+	pctrl->data->pwpr_pfc_unlock(pctrl);
 
 	/* Restore port registers. */
 	for (u32 port = 0; port < nports; port++) {
@@ -2567,9 +2562,7 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
 		}
 	}
 
-	/* Set the PWPR register to be write-protected. */
-	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
-	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
+	pctrl->data->pwpr_pfc_lock(pctrl);
 }
 
 static int rzg2l_pinctrl_suspend_noirq(struct device *dev)
@@ -2631,6 +2624,24 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
 	return 0;
 }
 
+static void rzg2l_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl)
+{
+	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
+
+	/* Set the PWPR register to allow PFC register to write */
+	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
+	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
+}
+
+static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
+{
+	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
+
+	/* Set the PWPR register to be write-protected */
+	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
+	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
+}
+
 static const struct rzg2l_hwcfg rzg2l_hwcfg = {
 	.regs = {
 		.pwpr = 0x3014,
@@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
 	.variable_pin_cfg = r9a07g043f_variable_pin_cfg,
 	.n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
 #endif
+	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
+	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 };
 
 static struct rzg2l_pinctrl_data r9a07g044_data = {
@@ -2699,6 +2712,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
 	.n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
 		ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
 	.hwcfg = &rzg2l_hwcfg,
+	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
+	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 };
 
 static struct rzg2l_pinctrl_data r9a08g045_data = {
@@ -2709,6 +2724,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
 	.n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
 	.n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
 	.hwcfg = &rzg3s_hwcfg,
+	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
+	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 };
 
 static const struct of_device_id rzg2l_pinctrl_of_table[] = {
-- 
2.34.1

[PATCH v2 07/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointer for writing to PMC register

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

This patch introduces a function pointer, pmc_writeb(), in the
struct rzg2l_pinctrl_data to facilitate writing to the PMC register. On
the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_A bit before writing to PMC
registers is required, whereas this is not the case for the existing
RZ/G2L family. This addition enables the reuse of existing code for
RZ/V2H(P). Additionally, this patch populates this function pointer with
appropriate data for existing SoCs.

Note that this functionality is only handled in rzg2l_gpio_request(), as
PMC unlock/lock during PFC setup will be taken care of in the
pwpr_pfc_unlock/pwpr_pfc_lock.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- No change
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 0840fda7ca69..e6d986b84be6 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -260,6 +260,7 @@ struct rzg2l_pinctrl_data {
 	unsigned int n_variable_pin_cfg;
 	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
 	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
+	void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
 };
 
 /**
@@ -463,6 +464,11 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
 };
 #endif
 
+static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
+{
+	writeb(val, addr);
+}
+
 static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 				       u8 pin, u8 off, u8 func)
 {
@@ -1410,7 +1416,7 @@ static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset)
 	/* Select GPIO mode in PMC Register */
 	reg8 = readb(pctrl->base + PMC(off));
 	reg8 &= ~BIT(bit);
-	writeb(reg8, pctrl->base + PMC(off));
+	pctrl->data->pmc_writeb(pctrl, reg8, pctrl->base + PMC(off));
 
 	spin_unlock_irqrestore(&pctrl->lock, flags);
 
@@ -2701,6 +2707,7 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
 #endif
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
+	.pmc_writeb = &rzg2l_pmc_writeb,
 };
 
 static struct rzg2l_pinctrl_data r9a07g044_data = {
@@ -2714,6 +2721,7 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
 	.hwcfg = &rzg2l_hwcfg,
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
+	.pmc_writeb = &rzg2l_pmc_writeb,
 };
 
 static struct rzg2l_pinctrl_data r9a08g045_data = {
@@ -2726,6 +2734,7 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
 	.hwcfg = &rzg3s_hwcfg,
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
+	.pmc_writeb = &rzg2l_pmc_writeb,
 };
 
 static const struct of_device_id rzg2l_pinctrl_of_table[] = {
-- 
2.34.1

[PATCH v2 08/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for reading/writing OEN register

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

This patch introduces function pointers, read_oen() and write_oen(), in the
struct rzg2l_pinctrl_data to facilitate reading and writing to the PFC_OEN
register. On the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_B bit before
writing to the PFC_OEN register is necessary, and the PFC_OEN register has
more bits compared to the RZ/G2L family. To handle these differences
between RZ/G2L and RZ/V2H(P) and to reuse the existing code for RZ/V2H(P),
these function pointers are introduced.

Additionally, this patch populates these function pointers with appropriate
data for existing SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- No change
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index e6d986b84be6..64648a951323 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -261,6 +261,8 @@ struct rzg2l_pinctrl_data {
 	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
 	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
 	void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
+	u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
+	int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
 };
 
 /**
@@ -1116,7 +1118,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		break;
 
 	case PIN_CONFIG_OUTPUT_ENABLE:
-		arg = rzg2l_read_oen(pctrl, cfg, _pin, bit);
+		arg = pctrl->data->read_oen(pctrl, cfg, _pin, bit);
 		if (!arg)
 			return -EINVAL;
 		break;
@@ -1225,7 +1227,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 
 		case PIN_CONFIG_OUTPUT_ENABLE:
 			arg = pinconf_to_config_argument(_configs[i]);
-			ret = rzg2l_write_oen(pctrl, cfg, _pin, bit, !!arg);
+			ret = pctrl->data->write_oen(pctrl, cfg, _pin, bit, !!arg);
 			if (ret)
 				return ret;
 			break;
@@ -2708,6 +2710,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 	.pmc_writeb = &rzg2l_pmc_writeb,
+	.read_oen = &rzg2l_read_oen,
+	.write_oen = &rzg2l_write_oen,
 };
 
 static struct rzg2l_pinctrl_data r9a07g044_data = {
@@ -2722,6 +2726,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 	.pmc_writeb = &rzg2l_pmc_writeb,
+	.read_oen = &rzg2l_read_oen,
+	.write_oen = &rzg2l_write_oen,
 };
 
 static struct rzg2l_pinctrl_data r9a08g045_data = {
@@ -2735,6 +2741,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
 	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
 	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
 	.pmc_writeb = &rzg2l_pmc_writeb,
+	.read_oen = &rzg2l_read_oen,
+	.write_oen = &rzg2l_write_oen,
 };
 
 static const struct of_device_id rzg2l_pinctrl_of_table[] = {
-- 
2.34.1

[PATCH v2 09/13] pinctrl: renesas: pinctrl-rzg2l: Add support to configure the slew-rate

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support to configure slew-rate property of the pin.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- New patch
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 64648a951323..102fa75c71d3 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -120,6 +120,7 @@
 #define PFC(off)		(0x0400 + (off) * 4)
 #define PIN(off)		(0x0800 + (off))
 #define IOLH(off)		(0x1000 + (off) * 8)
+#define SR(off)			(0x1400 + (off) * 8)
 #define IEN(off)		(0x1800 + (off) * 8)
 #define ISEL(off)		(0x2C00 + (off) * 8)
 #define SD_CH(off, ch)		((off) + (ch) * 4)
@@ -138,6 +139,7 @@
 #define PFC_MASK		0x07
 #define IEN_MASK		0x01
 #define IOLH_MASK		0x03
+#define SR_MASK			0x01
 
 #define PM_INPUT		0x1
 #define PM_OUTPUT		0x2
@@ -1130,6 +1132,13 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		arg = ret;
 		break;
 
+	case PIN_CONFIG_SLEW_RATE:
+		if (!(cfg & PIN_CFG_SR))
+			return -EINVAL;
+
+		arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
+		break;
+
 	case PIN_CONFIG_DRIVE_STRENGTH: {
 		unsigned int index;
 
@@ -1236,6 +1245,15 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 			settings.power_source = pinconf_to_config_argument(_configs[i]);
 			break;
 
+		case PIN_CONFIG_SLEW_RATE:
+			arg = pinconf_to_config_argument(_configs[i]);
+
+			if (!(cfg & PIN_CFG_SR) || arg > 1)
+				return -EINVAL;
+
+			rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg);
+			break;
+
 		case PIN_CONFIG_DRIVE_STRENGTH:
 			arg = pinconf_to_config_argument(_configs[i]);
 
-- 
2.34.1

[PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support to configure bias-disable, bias-pull-up and bias-pull-down
properties of the pin.

Two new function pointers get_bias_param() and get_bias_val() are
introduced as the values in PUPD register differ when compared to
RZ/G2L family and RZ/V2H(P) SoC,

Value | RZ/G2L        | RZ/V2H
---------------------------------
00b:  | Bias Disabled | Pull up/down disabled
01b:  | Pull-up       | Pull up/down disabled
10b:  | Pull-down     | Pull-down
11b:  | Prohibited    | Pull-up

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- New patch
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 102fa75c71d3..c144bf43522b 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -122,6 +122,7 @@
 #define IOLH(off)		(0x1000 + (off) * 8)
 #define SR(off)			(0x1400 + (off) * 8)
 #define IEN(off)		(0x1800 + (off) * 8)
+#define PUPD(off)		(0x1C00 + (off) * 8)
 #define ISEL(off)		(0x2C00 + (off) * 8)
 #define SD_CH(off, ch)		((off) + (ch) * 4)
 #define ETH_POC(off, ch)	((off) + (ch) * 4)
@@ -140,6 +141,7 @@
 #define IEN_MASK		0x01
 #define IOLH_MASK		0x03
 #define SR_MASK			0x01
+#define PUPD_MASK		0x03
 
 #define PM_INPUT		0x1
 #define PM_OUTPUT		0x2
@@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
 	void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
 	u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
 	int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
+	int (*get_bias_param)(u8 val);
+	int (*get_bias_val)(enum pin_config_param param);
 };
 
 /**
@@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8
 	return 0;
 }
 
+static int rzg2l_get_bias_param(u8 val)
+{
+	switch (val) {
+	case 0:
+		return PIN_CONFIG_BIAS_DISABLE;
+	case 1:
+		return PIN_CONFIG_BIAS_PULL_UP;
+	case 2:
+		return PIN_CONFIG_BIAS_PULL_DOWN;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int rzg2l_get_bias_val(enum pin_config_param param)
+{
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		return 0;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		return 1;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		return 2;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
 static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 				     unsigned int _pin,
 				     unsigned long *config)
@@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
 		break;
 
+	case PIN_CONFIG_BIAS_DISABLE:
+	case PIN_CONFIG_BIAS_PULL_UP:
+	case PIN_CONFIG_BIAS_PULL_DOWN: {
+		if (!(cfg & PIN_CFG_PUPD))
+			return -EINVAL;
+
+		ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
+									PUPD(off),
+									bit, PUPD_MASK));
+		if (ret < 0)
+			return ret;
+
+		if (ret != param)
+			return -EINVAL;
+		/* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */
+		arg = 1;
+		break;
+	}
+
 	case PIN_CONFIG_DRIVE_STRENGTH: {
 		unsigned int index;
 
@@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 			rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg);
 			break;
 
+		case PIN_CONFIG_BIAS_DISABLE:
+		case PIN_CONFIG_BIAS_PULL_UP:
+		case PIN_CONFIG_BIAS_PULL_DOWN: {
+			if (!(cfg & PIN_CFG_PUPD))
+				return -EINVAL;
+
+			ret = pctrl->data->get_bias_val(param);
+			if (ret < 0)
+				return ret;
+
+			rzg2l_rmw_pin_config(pctrl, PUPD(off), bit, PUPD_MASK, ret);
+			break;
+		}
+
 		case PIN_CONFIG_DRIVE_STRENGTH:
 			arg = pinconf_to_config_argument(_configs[i]);
 
@@ -2746,6 +2815,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
 	.pmc_writeb = &rzg2l_pmc_writeb,
 	.read_oen = &rzg2l_read_oen,
 	.write_oen = &rzg2l_write_oen,
+	.get_bias_param = &rzg2l_get_bias_param,
+	.get_bias_val = &rzg2l_get_bias_val,
 };
 
 static struct rzg2l_pinctrl_data r9a08g045_data = {
@@ -2761,6 +2832,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
 	.pmc_writeb = &rzg2l_pmc_writeb,
 	.read_oen = &rzg2l_read_oen,
 	.write_oen = &rzg2l_write_oen,
+	.get_bias_param = &rzg2l_get_bias_param,
+	.get_bias_val = &rzg2l_get_bias_val,
 };
 
 static const struct of_device_id rzg2l_pinctrl_of_table[] = {
-- 
2.34.1

[PATCH v2 11/13] pinctrl: renesas: pinctrl-rzg2l: Pass pincontrol device pointer to pinconf_generic_parse_dt_config()

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Pass pincontrol device pointer to pinconf_generic_parse_dt_config() in
prepration for passing custom params.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- No change
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index c144bf43522b..f3c5e8982623 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -612,7 +612,7 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		return -EINVAL;
 	}
 
-	ret = pinconf_generic_parse_dt_config(np, NULL, &configs, &num_configs);
+	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs, &num_configs);
 	if (ret < 0)
 		return ret;
 
-- 
2.34.1

[PATCH v2 12/13] pinctrl: renesas: pinctrl-rzg2l: Add support for custom parameters

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

In preparation for passing custom params for RZ/V2H(P) SoC assign the
custom params that is being passed via struct rzg2l_pinctrl_data.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- No change
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index f3c5e8982623..7e3ed18e0745 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -262,6 +262,9 @@ struct rzg2l_pinctrl_data {
 	const struct rzg2l_hwcfg *hwcfg;
 	const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
 	unsigned int n_variable_pin_cfg;
+	unsigned int num_custom_params;
+	const struct pinconf_generic_params *custom_params;
+	const struct pin_config_item *custom_conf_items;
 	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
 	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
 	void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
@@ -2374,6 +2377,13 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl)
 	pctrl->desc.pmxops = &rzg2l_pinctrl_pmxops;
 	pctrl->desc.confops = &rzg2l_pinctrl_confops;
 	pctrl->desc.owner = THIS_MODULE;
+	if (pctrl->data->num_custom_params) {
+		pctrl->desc.num_custom_params = pctrl->data->num_custom_params;
+		pctrl->desc.custom_params = pctrl->data->custom_params;
+#ifdef CONFIG_DEBUG_FS
+		pctrl->desc.custom_conf_items = pctrl->data->custom_conf_items;
+#endif
+	}
 
 	pins = devm_kcalloc(pctrl->dev, pctrl->desc.npins, sizeof(*pins), GFP_KERNEL);
 	if (!pins)
-- 
2.34.1

[PATCH v2 13/13] pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add pinctrl driver support for RZ/V2H(P) SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
- Renamed renesas-rzv2h,output-impedance -> renesas,output-impedance
- Dropped IOLH groups
- Fixed dedicated pin configs
- Updated r9a09g057_variable_pin_cfg
- Added support OEN
- Added support for bias settings
- Added function pointers for pwpr (un)lock
- Added support for slew-rate
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 419 +++++++++++++++++++++++-
 1 file changed, 415 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 7e3ed18e0745..ee0dcdd7921a 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -59,6 +59,10 @@
 #define PIN_CFG_OEN			BIT(15)
 #define PIN_CFG_VARIABLE		BIT(16)
 #define PIN_CFG_NOGPIO_INT		BIT(17)
+#define PIN_CFG_OPEN_DRAIN		BIT(18)
+#define PIN_CFG_SCHMIT_CTRL		BIT(19)
+#define PIN_CFG_ELC			BIT(20)
+#define PIN_CFG_IOLH_RZV2H		BIT(21)
 
 #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
 					(PIN_CFG_IOLH_##group | \
@@ -73,6 +77,10 @@
 #define RZG3S_MPXED_PIN_FUNCS(group)	(RZG2L_MPXED_COMMON_PIN_FUNCS(group) | \
 					 PIN_CFG_SOFT_PS)
 
+#define RZV2H_MPXED_PIN_FUNCS		(RZG2L_MPXED_COMMON_PIN_FUNCS(RZV2H) | \
+					 PIN_CFG_OPEN_DRAIN | \
+					 PIN_CFG_SR)
+
 #define RZG2L_MPXED_ETH_PIN_FUNCS(x)	((x) | \
 					 PIN_CFG_FILONOFF | \
 					 PIN_CFG_FILNUM | \
@@ -128,13 +136,15 @@
 #define ETH_POC(off, ch)	((off) + (ch) * 4)
 #define QSPI			(0x3008)
 #define ETH_MODE		(0x3018)
+#define PFC_OEN			(0x3C40) /* known on RZ/V2H(P) only */
 
 #define PVDD_2500		2	/* I/O domain voltage 2.5V */
 #define PVDD_1800		1	/* I/O domain voltage <= 1.8V */
 #define PVDD_3300		0	/* I/O domain voltage >= 3.3V */
 
 #define PWPR_B0WI		BIT(7)	/* Bit Write Disable */
-#define PWPR_PFCWE		BIT(6)	/* PFC Register Write Enable */
+#define PWPR_PFCWE		BIT(6)	/* PFC (and PMC on RZ/V2H) Register Write Enable */
+#define PWPR_REGWE_B		BIT(5)	/* OEN Register Write Enable, known only in RZ/V2H(P) */
 
 #define PM_MASK			0x03
 #define PFC_MASK		0x07
@@ -153,6 +163,19 @@
 #define RZG2L_TINT_IRQ_START_INDEX	9
 #define RZG2L_PACK_HWIRQ(t, i)		(((t) << 16) | (i))
 
+/* Custom pinconf parameters */
+#define RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE	(PIN_CONFIG_END + 1)
+
+static const struct pinconf_generic_params renesas_rzv2h_custom_bindings[] = {
+	{ "renesas,output-impedance", RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE, 1 },
+};
+
+#ifdef CONFIG_DEBUG_FS
+static const struct pin_config_item renesas_rzv2h_conf_items[] = {
+	PCONFDUMP(RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE, "output-impedance", "x", true),
+};
+#endif
+
 /* Read/write 8 bits register */
 #define RZG2L_PCTRL_REG_ACCESS8(_read, _addr, _val)	\
 	do {						\
@@ -330,6 +353,8 @@ struct rzg2l_pinctrl {
 	spinlock_t			lock; /* lock read/write registers */
 	struct mutex			mutex; /* serialize adding groups and functions */
 
+	raw_spinlock_t			pwpr_lock; /* serialize PWPR register access */
+
 	struct rzg2l_pinctrl_pin_settings *settings;
 	struct rzg2l_pinctrl_reg_cache	*cache;
 	struct rzg2l_pinctrl_reg_cache	*dedicated_cache;
@@ -354,6 +379,39 @@ static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
 	return 0;
 }
 
+static const struct rzg2l_variable_pin_cfg r9a09g057_variable_pin_cfg[] = {
+	{
+		.port = 11,
+		.pin = 0,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL,
+	},
+	{
+		.port = 11,
+		.pin = 1,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
+	},
+	{
+		.port = 11,
+		.pin = 2,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
+	},
+	{
+		.port = 11,
+		.pin = 3,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
+	},
+	{
+		.port = 11,
+		.pin = 4,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
+	},
+	{
+		.port = 11,
+		.pin = 5,
+		.cfg = RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL | PIN_CFG_IEN,
+	},
+};
+
 #ifdef CONFIG_RISCV
 static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
 	{
@@ -480,6 +538,19 @@ static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *
 	writeb(val, addr);
 }
 
+static void rzv2h_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
+{
+	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
+	u8 pwpr;
+
+	raw_spin_lock(&pctrl->pwpr_lock);
+	pwpr = readb(pctrl->base + regs->pwpr);
+	writeb(pwpr | PWPR_PFCWE, pctrl->base + regs->pwpr);
+	writeb(val, addr);
+	writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);
+	raw_spin_unlock(&pctrl->pwpr_lock);
+}
+
 static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
 				       u8 pin, u8 off, u8 func)
 {
@@ -1120,14 +1191,107 @@ static int rzg2l_get_bias_val(enum pin_config_param param)
 	return -EINVAL;
 }
 
+static int rzv2h_get_bias_param(u8 val)
+{
+	switch (val) {
+	case 0:
+	case 1:
+		return PIN_CONFIG_BIAS_DISABLE;
+	case 2:
+		return PIN_CONFIG_BIAS_PULL_DOWN;
+	case 3:
+		return PIN_CONFIG_BIAS_PULL_UP;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int rzv2h_get_bias_val(enum pin_config_param param)
+{
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		return 0;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		return 2;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		return 3;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static u8 rzv2h_pin_to_oen_bit(struct rzg2l_pinctrl *pctrl, u32 offset)
+{
+	static const char * const pin_names[] = { "ET0_TXC_TXCLK", "ET1_TXC_TXCLK",
+						  "XSPI0_RESET0N", "XSPI0_CS0N",
+						  "XSPI0_CKN", "XSPI0_CKP" };
+	const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
+	u8 bit_array[] = { 0, 1, 2, 3, 4, 5 };
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(bit_array); i++) {
+		if (!strcmp(pin_desc->name, pin_names[i]))
+			return bit_array[i];
+	}
+
+	/* Should not happen. */
+	return 0;
+}
+
+static u32 rzv2h_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin)
+{
+	u8 bit;
+
+	if (!(caps & PIN_CFG_OEN))
+		return 0;
+
+	bit = rzv2h_pin_to_oen_bit(pctrl, offset);
+
+	return !(readb(pctrl->base + PFC_OEN) & BIT(bit));
+}
+
+static int rzv2h_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen)
+{
+	const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
+	const struct rzg2l_register_offsets *regs = &hwcfg->regs;
+	unsigned long flags;
+	u8 val, bit;
+	u8 pwpr;
+
+	if (!(caps & PIN_CFG_OEN))
+		return -EINVAL;
+
+	bit = rzv2h_pin_to_oen_bit(pctrl, offset);
+	spin_lock_irqsave(&pctrl->lock, flags);
+	val = readb(pctrl->base + PFC_OEN);
+	if (oen)
+		val &= ~BIT(bit);
+	else
+		val |= BIT(bit);
+
+	raw_spin_lock(&pctrl->pwpr_lock);
+	pwpr = readb(pctrl->base + regs->pwpr);
+	writeb(pwpr | PWPR_REGWE_B, pctrl->base + regs->pwpr);
+	writeb(val, pctrl->base + PFC_OEN);
+	writeb(pwpr & ~PWPR_REGWE_B, pctrl->base + regs->pwpr);
+	raw_spin_unlock(&pctrl->pwpr_lock);
+	spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	return 0;
+}
+
 static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 				     unsigned int _pin,
 				     unsigned long *config)
 {
 	struct rzg2l_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
-	enum pin_config_param param = pinconf_to_config_param(*config);
 	const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
 	const struct pinctrl_pin_desc *pin = &pctrl->desc.pins[_pin];
+	u32 param = pinconf_to_config_param(*config);
 	u64 *pin_data = pin->drv_data;
 	unsigned int arg = 0;
 	u32 off;
@@ -1240,6 +1404,14 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 		break;
 	}
 
+	case RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE: {
+		if (!(cfg & PIN_CFG_IOLH_RZV2H))
+			return -EINVAL;
+
+		arg = rzg2l_read_pin_config(pctrl, IOLH(off), bit, IOLH_MASK);
+		break;
+	}
+
 	default:
 		return -ENOTSUPP;
 	}
@@ -1259,9 +1431,8 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 	const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
 	struct rzg2l_pinctrl_pin_settings settings = pctrl->settings[_pin];
 	u64 *pin_data = pin->drv_data;
-	enum pin_config_param param;
 	unsigned int i, arg, index;
-	u32 off;
+	u32 off, param;
 	u64 cfg;
 	int ret;
 	u8 bit;
@@ -1367,6 +1538,17 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 			rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, index);
 			break;
 
+		case RENESAS_RZV2H_PIN_CONFIG_OUTPUT_IMPEDANCE:
+			arg = pinconf_to_config_argument(_configs[i]);
+
+			if (!(cfg & PIN_CFG_IOLH_RZV2H))
+				return -EINVAL;
+
+			if (arg > 3)
+				return -EINVAL;
+			rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, arg);
+			break;
+
 		default:
 			return -EOPNOTSUPP;
 		}
@@ -1814,6 +1996,39 @@ static const u64 r9a08g045_gpio_configs[] = {
 	RZG2L_GPIO_PORT_PACK(6, 0x2a, RZG3S_MPXED_PIN_FUNCS(A)),			/* P18 */
 };
 
+static const char * const rzv2h_gpio_names[] = {
+	"P00", "P01", "P02", "P03", "P04", "P05", "P06", "P07",
+	"P10", "P11", "P12", "P13", "P14", "P15", "P16", "P17",
+	"P20", "P21", "P22", "P23", "P24", "P25", "P26", "P27",
+	"P30", "P31", "P32", "P33", "P34", "P35", "P36", "P37",
+	"P40", "P41", "P42", "P43", "P44", "P45", "P46", "P47",
+	"P50", "P51", "P52", "P53", "P54", "P55", "P56", "P57",
+	"P60", "P61", "P62", "P63", "P64", "P65", "P66", "P67",
+	"P70", "P71", "P72", "P73", "P74", "P75", "P76", "P77",
+	"P80", "P81", "P82", "P83", "P84", "P85", "P86", "P87",
+	"P90", "P91", "P92", "P93", "P94", "P95", "P96", "P97",
+	"PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", "PA7",
+	"PB0", "PB1", "PB2", "PB3", "PB4", "PB5", "PB6", "PB7",
+};
+
+static const u64 r9a09g057_gpio_configs[] = {
+	RZG2L_GPIO_PORT_PACK(8, 0x20, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P0 */
+	RZG2L_GPIO_PORT_PACK(6, 0x21, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P1 */
+	RZG2L_GPIO_PORT_PACK(2, 0x22, RZG2L_MPXED_COMMON_PIN_FUNCS(RZV2H) |
+				      PIN_CFG_OPEN_DRAIN),				/* P2 */
+	RZG2L_GPIO_PORT_PACK(8, 0x23, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P3 */
+	RZG2L_GPIO_PORT_PACK(8, 0x24, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P4 */
+	RZG2L_GPIO_PORT_PACK(8, 0x25, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P5 */
+	RZG2L_GPIO_PORT_PACK(8, 0x26, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL |
+				      PIN_CFG_ELC),					/* P6 */
+	RZG2L_GPIO_PORT_PACK(8, 0x27, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P7 */
+	RZG2L_GPIO_PORT_PACK(8, 0x28, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL |
+				      PIN_CFG_ELC),					/* P8 */
+	RZG2L_GPIO_PORT_PACK(8, 0x29, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* P9 */
+	RZG2L_GPIO_PORT_PACK(8, 0x2a, RZV2H_MPXED_PIN_FUNCS | PIN_CFG_SCHMIT_CTRL),	/* PA */
+	RZG2L_GPIO_PORT_PACK(6, 0x2b, PIN_CFG_VARIABLE),				/* PB */
+};
+
 static const struct {
 	struct rzg2l_dedicated_configs common[35];
 	struct rzg2l_dedicated_configs rzg2l_pins[7];
@@ -1940,6 +2155,138 @@ static const struct rzg2l_dedicated_configs rzg3s_dedicated_pins[] = {
 						       PIN_CFG_IO_VMC_SD1)) },
 };
 
+static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
+	{ "NMI", RZG2L_SINGLE_PIN_PACK(0x1, 0, (PIN_CFG_FILONOFF | PIN_CFG_FILNUM |
+						PIN_CFG_FILCLKSEL)) },
+	{ "TMS_SWDIO", RZG2L_SINGLE_PIN_PACK(0x3, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_IEN)) },
+	{ "TDO", RZG2L_SINGLE_PIN_PACK(0x3, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "WDTUDFCA", RZG2L_SINGLE_PIN_PACK(0x5, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD | PIN_CFG_OPEN_DRAIN)) },
+	{ "WDTUDFCM", RZG2L_SINGLE_PIN_PACK(0x5, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD | PIN_CFG_OPEN_DRAIN)) },
+	{ "SCIF_RXD", RZG2L_SINGLE_PIN_PACK(0x6, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD)) },
+	{ "SCIF_TXD", RZG2L_SINGLE_PIN_PACK(0x6, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD)) },
+	{ "XSPI0_CKP", RZG2L_SINGLE_PIN_PACK(0x7, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "XSPI0_CKN", RZG2L_SINGLE_PIN_PACK(0x7, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "XSPI0_CS0N", RZG2L_SINGLE_PIN_PACK(0x7, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						       PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "XSPI0_DS", RZG2L_SINGLE_PIN_PACK(0x7, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD)) },
+	{ "XSPI0_RESET0N", RZG2L_SINGLE_PIN_PACK(0x7, 4, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+							  PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "XSPI0_RSTO0N", RZG2L_SINGLE_PIN_PACK(0x7, 5, (PIN_CFG_PUPD)) },
+	{ "XSPI0_INT0N", RZG2L_SINGLE_PIN_PACK(0x7, 6, (PIN_CFG_PUPD)) },
+	{ "XSPI0_ECS0N", RZG2L_SINGLE_PIN_PACK(0x7, 7, (PIN_CFG_PUPD)) },
+	{ "XSPI0_IO0", RZG2L_SINGLE_PIN_PACK(0x8, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO1", RZG2L_SINGLE_PIN_PACK(0x8, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO2", RZG2L_SINGLE_PIN_PACK(0x8, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO3", RZG2L_SINGLE_PIN_PACK(0x8, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO4", RZG2L_SINGLE_PIN_PACK(0x8, 4, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO5", RZG2L_SINGLE_PIN_PACK(0x8, 5, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO6", RZG2L_SINGLE_PIN_PACK(0x8, 6, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "XSPI0_IO7", RZG2L_SINGLE_PIN_PACK(0x8, 7, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "SD0CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "SD0CMD", RZG2L_SINGLE_PIN_PACK(0x9, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						   PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0RSTN", RZG2L_SINGLE_PIN_PACK(0x9, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "SD0DAT0", RZG2L_SINGLE_PIN_PACK(0xa, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT1", RZG2L_SINGLE_PIN_PACK(0xa, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT2", RZG2L_SINGLE_PIN_PACK(0xa, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT3", RZG2L_SINGLE_PIN_PACK(0xa, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT4", RZG2L_SINGLE_PIN_PACK(0xa, 4, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT5", RZG2L_SINGLE_PIN_PACK(0xa, 5, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT6", RZG2L_SINGLE_PIN_PACK(0xa, 6, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD0DAT7", RZG2L_SINGLE_PIN_PACK(0xa, 7, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD1CLK", RZG2L_SINGLE_PIN_PACK(0xb, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "SD1CMD", RZG2L_SINGLE_PIN_PACK(0xb, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						   PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD1DAT0", RZG2L_SINGLE_PIN_PACK(0xc, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD1DAT1", RZG2L_SINGLE_PIN_PACK(0xc, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD1DAT2", RZG2L_SINGLE_PIN_PACK(0xc, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "SD1DAT3", RZG2L_SINGLE_PIN_PACK(0xc, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "PCIE0_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
+	{ "ET0_MDIO", RZG2L_SINGLE_PIN_PACK(0xf, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "ET0_MDC", RZG2L_SINGLE_PIN_PACK(0xf, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						    PIN_CFG_PUPD)) },
+	{ "ET0_RXCTL_RXDV", RZG2L_SINGLE_PIN_PACK(0x10, 0, (PIN_CFG_PUPD)) },
+	{ "ET0_TXCTL_TXEN", RZG2L_SINGLE_PIN_PACK(0x10, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+							    PIN_CFG_PUPD)) },
+	{ "ET0_TXER", RZG2L_SINGLE_PIN_PACK(0x10, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET0_RXER", RZG2L_SINGLE_PIN_PACK(0x10, 3, (PIN_CFG_PUPD)) },
+	{ "ET0_RXC_RXCLK", RZG2L_SINGLE_PIN_PACK(0x10, 4, (PIN_CFG_PUPD)) },
+	{ "ET0_TXC_TXCLK", RZG2L_SINGLE_PIN_PACK(0x10, 5, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+							   PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "ET0_CRS", RZG2L_SINGLE_PIN_PACK(0x10, 6, (PIN_CFG_PUPD)) },
+	{ "ET0_COL", RZG2L_SINGLE_PIN_PACK(0x10, 7, (PIN_CFG_PUPD)) },
+	{ "ET0_TXD0", RZG2L_SINGLE_PIN_PACK(0x11, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET0_TXD1", RZG2L_SINGLE_PIN_PACK(0x11, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET0_TXD2", RZG2L_SINGLE_PIN_PACK(0x11, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET0_TXD3", RZG2L_SINGLE_PIN_PACK(0x11, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET0_RXD0", RZG2L_SINGLE_PIN_PACK(0x11, 4, (PIN_CFG_PUPD)) },
+	{ "ET0_RXD1", RZG2L_SINGLE_PIN_PACK(0x11, 5, (PIN_CFG_PUPD)) },
+	{ "ET0_RXD2", RZG2L_SINGLE_PIN_PACK(0x11, 6, (PIN_CFG_PUPD)) },
+	{ "ET0_RXD3", RZG2L_SINGLE_PIN_PACK(0x11, 7, (PIN_CFG_PUPD)) },
+	{ "ET1_MDIO", RZG2L_SINGLE_PIN_PACK(0x12, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_IEN | PIN_CFG_PUPD)) },
+	{ "ET1_MDC", RZG2L_SINGLE_PIN_PACK(0x12, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						     PIN_CFG_PUPD)) },
+	{ "ET1_RXCTL_RXDV", RZG2L_SINGLE_PIN_PACK(0x13, 0, (PIN_CFG_PUPD)) },
+	{ "ET1_TXCTL_TXEN", RZG2L_SINGLE_PIN_PACK(0x13, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+							    PIN_CFG_PUPD)) },
+	{ "ET1_TXER", RZG2L_SINGLE_PIN_PACK(0x13, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						       PIN_CFG_PUPD)) },
+	{ "ET1_RXER", RZG2L_SINGLE_PIN_PACK(0x13, 3, (PIN_CFG_PUPD)) },
+	{ "ET1_RXC_RXCLK", RZG2L_SINGLE_PIN_PACK(0x13, 4, (PIN_CFG_PUPD)) },
+	{ "ET1_TXC_TXCLK", RZG2L_SINGLE_PIN_PACK(0x13, 5, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+							   PIN_CFG_PUPD | PIN_CFG_OEN)) },
+	{ "ET1_CRS", RZG2L_SINGLE_PIN_PACK(0x13, 6, (PIN_CFG_PUPD)) },
+	{ "ET1_COL", RZG2L_SINGLE_PIN_PACK(0x13, 7, (PIN_CFG_PUPD)) },
+	{ "ET1_TXD0", RZG2L_SINGLE_PIN_PACK(0x14, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET1_TXD1", RZG2L_SINGLE_PIN_PACK(0x14, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET1_TXD2", RZG2L_SINGLE_PIN_PACK(0x14, 2, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET1_TXD3", RZG2L_SINGLE_PIN_PACK(0x14, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
+						      PIN_CFG_PUPD)) },
+	{ "ET1_RXD0", RZG2L_SINGLE_PIN_PACK(0x14, 4, (PIN_CFG_PUPD)) },
+	{ "ET1_RXD1", RZG2L_SINGLE_PIN_PACK(0x14, 5, (PIN_CFG_PUPD)) },
+	{ "ET1_RXD2", RZG2L_SINGLE_PIN_PACK(0x14, 6, (PIN_CFG_PUPD)) },
+	{ "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD)) },
+};
+
 static int rzg2l_gpio_get_gpioint(unsigned int virq, struct rzg2l_pinctrl *pctrl)
 {
 	const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[virq];
@@ -2476,6 +2823,9 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
 	BUILD_BUG_ON(ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT >
 		     ARRAY_SIZE(rzg2l_gpio_names));
 
+	BUILD_BUG_ON(ARRAY_SIZE(r9a09g057_gpio_configs) * RZG2L_PINS_PER_PORT >
+		     ARRAY_SIZE(rzv2h_gpio_names));
+
 	pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL);
 	if (!pctrl)
 		return -ENOMEM;
@@ -2498,6 +2848,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
 
 	spin_lock_init(&pctrl->lock);
 	spin_lock_init(&pctrl->bitmap_lock);
+	raw_spin_lock_init(&pctrl->pwpr_lock);
 	mutex_init(&pctrl->mutex);
 	atomic_set(&pctrl->wakeup_path, 0);
 
@@ -2747,6 +3098,32 @@ static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
 	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
 }
 
+static void rzv2h_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl)
+{
+	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
+	u8 pwpr;
+
+	/*
+	 * lock is acquired in pfc unlock call back and then released in
+	 * pfc lock callback
+	 */
+	raw_spin_lock(&pctrl->pwpr_lock);
+	/* Set the PWPR register to allow PFC and PMC register to write */
+	pwpr = readb(pctrl->base + regs->pwpr);
+	writeb(PWPR_PFCWE | pwpr, pctrl->base + regs->pwpr);
+}
+
+static void rzv2h_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
+{
+	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
+	u8 pwpr;
+
+	/* Set the PWPR register to be write-protected */
+	pwpr = readb(pctrl->base + regs->pwpr);
+	writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);
+	raw_spin_unlock(&pctrl->pwpr_lock);
+}
+
 static const struct rzg2l_hwcfg rzg2l_hwcfg = {
 	.regs = {
 		.pwpr = 0x3014,
@@ -2792,6 +3169,12 @@ static const struct rzg2l_hwcfg rzg3s_hwcfg = {
 	.oen_max_port = 7, /* P7_1 is the maximum OEN port. */
 };
 
+static const struct rzg2l_hwcfg rzv2h_hwcfg = {
+	.regs = {
+		.pwpr = 0x3c04,
+	},
+};
+
 static struct rzg2l_pinctrl_data r9a07g043_data = {
 	.port_pins = rzg2l_gpio_names,
 	.port_pin_configs = r9a07g043_gpio_configs,
@@ -2846,6 +3229,30 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
 	.get_bias_val = &rzg2l_get_bias_val,
 };
 
+static struct rzg2l_pinctrl_data r9a09g057_data = {
+	.port_pins = rzv2h_gpio_names,
+	.port_pin_configs = r9a09g057_gpio_configs,
+	.n_ports = ARRAY_SIZE(r9a09g057_gpio_configs),
+	.dedicated_pins = rzv2h_dedicated_pins,
+	.n_port_pins = ARRAY_SIZE(r9a09g057_gpio_configs) * RZG2L_PINS_PER_PORT,
+	.n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins),
+	.hwcfg = &rzv2h_hwcfg,
+	.variable_pin_cfg = r9a09g057_variable_pin_cfg,
+	.n_variable_pin_cfg = ARRAY_SIZE(r9a09g057_variable_pin_cfg),
+	.num_custom_params = ARRAY_SIZE(renesas_rzv2h_custom_bindings),
+	.custom_params = renesas_rzv2h_custom_bindings,
+#ifdef CONFIG_DEBUG_FS
+	.custom_conf_items = renesas_rzv2h_conf_items,
+#endif
+	.pwpr_pfc_unlock = &rzv2h_pwpr_pfc_unlock,
+	.pwpr_pfc_lock = &rzv2h_pwpr_pfc_lock,
+	.pmc_writeb = &rzv2h_pmc_writeb,
+	.read_oen = &rzv2h_read_oen,
+	.write_oen = &rzv2h_write_oen,
+	.get_bias_param = &rzv2h_get_bias_param,
+	.get_bias_val = &rzv2h_get_bias_val,
+};
+
 static const struct of_device_id rzg2l_pinctrl_of_table[] = {
 	{
 		.compatible = "renesas,r9a07g043-pinctrl",
@@ -2859,6 +3266,10 @@ static const struct of_device_id rzg2l_pinctrl_of_table[] = {
 		.compatible = "renesas,r9a08g045-pinctrl",
 		.data = &r9a08g045_data,
 	},
+	{
+		.compatible = "renesas,r9a09g057-pinctrl",
+		.data = &r9a09g057_data,
+	},
 	{ /* sentinel */ }
 };
 
-- 
2.34.1

RE: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Biju Das]

Hi Prabhakar,

Thanks for the patch.

> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Tuesday, April 23, 2024 6:59 PM
> Subject: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
> locking/unlocking the PFC register
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls writing to both PFC and
> PMC registers. Additionally, BIT(7) B0WI is undocumented for the PWPR register on RZ/V2H(P) SoC. To
> accommodate these differences across SoC variants, introduce the set_pfc_mode() and
> pm_set_pfc() function pointers.
> 
> Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is now called before PMC
> read/write and pwpr_pfc_lock() call is now called after PMC read/write this is to keep changes
> minimal for RZ/V2H(P).
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - Introduced function pointer for (un)lock
> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 51 ++++++++++++++++---------
>  1 file changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index bec4685b4681..0840fda7ca69 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -246,6 +246,8 @@ struct rzg2l_variable_pin_cfg {
>  	u64 pin:3;
>  };
> 
> +struct rzg2l_pinctrl;
> +
>  struct rzg2l_pinctrl_data {
>  	const char * const *port_pins;
>  	const u64 *port_pin_configs;
> @@ -256,6 +258,8 @@ struct rzg2l_pinctrl_data {
>  	const struct rzg2l_hwcfg *hwcfg;
>  	const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
>  	unsigned int n_variable_pin_cfg;
> +	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
> +	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
>  };
> 
>  /**
> @@ -462,7 +466,6 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] =
> {  static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
>  				       u8 pin, u8 off, u8 func)
>  {
> -	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
>  	unsigned long flags;
>  	u32 reg;
> 
> @@ -473,27 +476,23 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
>  	reg &= ~(PM_MASK << (pin * 2));
>  	writew(reg, pctrl->base + PM(off));
> 
> +	pctrl->data->pwpr_pfc_unlock(pctrl);
> +
>  	/* Temporarily switch to GPIO mode with PMC register */
>  	reg = readb(pctrl->base + PMC(off));
>  	writeb(reg & ~BIT(pin), pctrl->base + PMC(off));
> 
> -	/* Set the PWPR register to allow PFC register to write */
> -	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> -	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
> -
>  	/* 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));
> 
> -	/* Set the PWPR register to be write-protected */
> -	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> -	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
> -
>  	/* Switch to Peripheral pin function with PMC register */
>  	reg = readb(pctrl->base + PMC(off));
>  	writeb(reg | BIT(pin), pctrl->base + PMC(off));
> 
> +	pctrl->data->pwpr_pfc_lock(pctrl);
> +
>  	spin_unlock_irqrestore(&pctrl->lock, flags);  };
> 
> @@ -2519,12 +2518,8 @@ static void rzg2l_pinctrl_pm_setup_dedicated_regs(struct rzg2l_pinctrl
> *pctrl, b  static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)  {
>  	u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT;
> -	const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> -	const struct rzg2l_register_offsets *regs = &hwcfg->regs;
> 
> -	/* Set the PWPR register to allow PFC register to write. */
> -	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> -	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
> +	pctrl->data->pwpr_pfc_unlock(pctrl);
> 
>  	/* Restore port registers. */
>  	for (u32 port = 0; port < nports; port++) { @@ -2567,9 +2562,7 @@ static void
> rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
>  		}
>  	}
> 
> -	/* Set the PWPR register to be write-protected. */
> -	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> -	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
> +	pctrl->data->pwpr_pfc_lock(pctrl);
>  }
> 
>  static int rzg2l_pinctrl_suspend_noirq(struct device *dev) @@ -2631,6 +2624,24 @@ static int
> rzg2l_pinctrl_resume_noirq(struct device *dev)
>  	return 0;
>  }
> 
> +static void rzg2l_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl) {
> +	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> +
> +	/* Set the PWPR register to allow PFC register to write */
> +	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> +	writel(PWPR_PFCWE, pctrl->base + regs->pwpr);	/* B0WI=0, PFCWE=1 */
> +}
> +
> +static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl) {
> +	const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> +
> +	/* Set the PWPR register to be write-protected */
> +	writel(0x0, pctrl->base + regs->pwpr);		/* B0WI=0, PFCWE=0 */
> +	writel(PWPR_B0WI, pctrl->base + regs->pwpr);	/* B0WI=1, PFCWE=0 */
> +}
> +
>  static const struct rzg2l_hwcfg rzg2l_hwcfg = {
>  	.regs = {
>  		.pwpr = 0x3014,
> @@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
>  	.variable_pin_cfg = r9a07g043f_variable_pin_cfg,
>  	.n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
>  #endif
> +	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> +	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
>  };
> 
>  static struct rzg2l_pinctrl_data r9a07g044_data = { @@ -2699,6 +2712,8 @@ static struct
> rzg2l_pinctrl_data r9a07g044_data = {
>  	.n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
>  		ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
>  	.hwcfg = &rzg2l_hwcfg,
> +	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> +	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
>  };
> 
>  static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2709,6 +2724,8 @@ static struct
> rzg2l_pinctrl_data r9a08g045_data = {
>  	.n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
>  	.n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
>  	.hwcfg = &rzg3s_hwcfg,
> +	.pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> +	.pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,

Some memory can be saved by avoiding duplication of data by using
a single pointer for structure containing function pointers??

struct rzg2l_pinctrl_fns {
	void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
	void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
}

Cheers,
Biju

>  };
> 
>  static const struct of_device_id rzg2l_pinctrl_of_table[] = {
> --
> 2.34.1

Re: [PATCH v2 02/13] dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC

[Geert Uytterhoeven]

Hi Prabhakar,

Thanks for your patch!

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add documentation for the pin controller found on the Renesas RZ/V2H(P)
> (R9A09G057) SoC. The RZ/V2H PFC varies slightly compared to the RZ/G2L
> family:
> - Additional bits need to be set during pinmuxing.
> - The GPIO pin count is different.
>
> Hence, a SoC-specific compatible string, 'renesas,r9a09g057-pinctrl', is
> added for the RZ/V2H(P) SoC.
>
> Also, add the 'renesas,output-impedance' property. The drive strength
> setting on RZ/V2H(P) depends on the different power rails coming out from
> the PMIC (connected via I2C). These power rails (required for drive
> strength) can be 1.2V, 1.8V, or 3.3V.
>
> Pins are grouped into 4 groups:
>
> Group 1: Impedance
> - 150/75/38/25 ohms (at 3.3V)
> - 130/65/33/22 ohms (at 1.8V)
>
> Group 2: Impedance
> - 50/40/33/25 ohms (at 1.8V)
>
> Group 3: Impedance
> - 150/75/37.5/25 ohms (at 3.3V)
> - 130/65/33/22 ohms (at 1.8V)
>
> Group 4: Impedance
> - 110/55/30/20 ohms (at 1.8V)
> - 150/75/38/25 ohms (at 1.2V)
>
> The 'renesas,output-impedance' property, as documented, can be
> [0, 1, 2, 3], indicating x1/x2/x3/x4 drive strength.

The documentation says "x1/x2/x4/x6" for all but Group 4 (P20/P21).
Still, the actual values don't seem to match the magnification factors...

> As power rail information may not be available very early in the boot
> process, the 'renesas,output-impedance' property is added instead of
> reusing the 'output-impedance-ohms' property.
>
> Also, allow bias-disable, bias-pull-down and bias-pull-up properties
> as these can be used to configure the pins.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> --- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> @@ -111,6 +116,18 @@ additionalProperties:
>          output-high: true
>          output-low: true
>          line-name: true
> +        bias-disable: true
> +        bias-pull-down: true
> +        bias-pull-up: true
> +        renesas,output-impedance:
> +          description: |
> +            Output impedance for pins on RZ/V2H(P) SoC.
> +            0: Corresponds to x1
> +            1: Corresponds to x2
> +            2: Corresponds to x3
> +            3: Corresponds to x4

Hence I'd drop the multiplication factors, and just say the meaning
of the value is pin-dependent?

> +          $ref: /schemas/types.yaml#/definitions/uint32
> +          enum: [0, 1, 2, 3]
>
>      - type: object
>        additionalProperties:

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 v2 02/13] dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, Apr 24, 2024 at 10:06 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add documentation for the pin controller found on the Renesas RZ/V2H(P)
> > (R9A09G057) SoC. The RZ/V2H PFC varies slightly compared to the RZ/G2L
> > family:
> > - Additional bits need to be set during pinmuxing.
> > - The GPIO pin count is different.
> >
> > Hence, a SoC-specific compatible string, 'renesas,r9a09g057-pinctrl', is
> > added for the RZ/V2H(P) SoC.
> >
> > Also, add the 'renesas,output-impedance' property. The drive strength
> > setting on RZ/V2H(P) depends on the different power rails coming out from
> > the PMIC (connected via I2C). These power rails (required for drive
> > strength) can be 1.2V, 1.8V, or 3.3V.
> >
> > Pins are grouped into 4 groups:
> >
> > Group 1: Impedance
> > - 150/75/38/25 ohms (at 3.3V)
> > - 130/65/33/22 ohms (at 1.8V)
> >
> > Group 2: Impedance
> > - 50/40/33/25 ohms (at 1.8V)
> >
> > Group 3: Impedance
> > - 150/75/37.5/25 ohms (at 3.3V)
> > - 130/65/33/22 ohms (at 1.8V)
> >
> > Group 4: Impedance
> > - 110/55/30/20 ohms (at 1.8V)
> > - 150/75/38/25 ohms (at 1.2V)
> >
> > The 'renesas,output-impedance' property, as documented, can be
> > [0, 1, 2, 3], indicating x1/x2/x3/x4 drive strength.
>
> The documentation says "x1/x2/x4/x6" for all but Group 4 (P20/P21).
> Still, the actual values don't seem to match the magnification factors...
>
> > As power rail information may not be available very early in the boot
> > process, the 'renesas,output-impedance' property is added instead of
> > reusing the 'output-impedance-ohms' property.
> >
> > Also, allow bias-disable, bias-pull-down and bias-pull-up properties
> > as these can be used to configure the pins.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > --- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> > @@ -111,6 +116,18 @@ additionalProperties:
> >          output-high: true
> >          output-low: true
> >          line-name: true
> > +        bias-disable: true
> > +        bias-pull-down: true
> > +        bias-pull-up: true
> > +        renesas,output-impedance:
> > +          description: |
> > +            Output impedance for pins on RZ/V2H(P) SoC.
> > +            0: Corresponds to x1
> > +            1: Corresponds to x2
> > +            2: Corresponds to x3
> > +            3: Corresponds to x4
>
> Hence I'd drop the multiplication factors, and just say the meaning
> of the value is pin-dependent?
>
Agreed, I will update the description "Output impedance for pins on
RZ/V2H(P) SoC. 0/1/2/3 corresponds to register bit values that can be
set in PFC_IOLH_mn register which adjusts the drive strength value and
is pin-dependent"

Cheers,
Prabhakar

> > +          $ref: /schemas/types.yaml#/definitions/uint32
> > +          enum: [0, 1, 2, 3]
> >
> >      - type: object
> >        additionalProperties:
>
> 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 v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Lad, Prabhakar]

Hi Biju,

Thank you for the review.

On Tue, Apr 23, 2024 at 7:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: Prabhakar <prabhakar.csengg@gmail.com>
> > Sent: Tuesday, April 23, 2024 6:59 PM
> > Subject: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
> > locking/unlocking the PFC register
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> > However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls writing to both PFC and
> > PMC registers. Additionally, BIT(7) B0WI is undocumented for the PWPR register on RZ/V2H(P) SoC. To
> > accommodate these differences across SoC variants, introduce the set_pfc_mode() and
> > pm_set_pfc() function pointers.
> >
> > Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is now called before PMC
> > read/write and pwpr_pfc_lock() call is now called after PMC read/write this is to keep changes
> > minimal for RZ/V2H(P).
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - Introduced function pointer for (un)lock
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 51 ++++++++++++++++---------
> >  1 file changed, 34 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index bec4685b4681..0840fda7ca69 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -246,6 +246,8 @@ struct rzg2l_variable_pin_cfg {
> >       u64 pin:3;
> >  };
> >
> > +struct rzg2l_pinctrl;
> > +
> >  struct rzg2l_pinctrl_data {
> >       const char * const *port_pins;
> >       const u64 *port_pin_configs;
> > @@ -256,6 +258,8 @@ struct rzg2l_pinctrl_data {
> >       const struct rzg2l_hwcfg *hwcfg;
> >       const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
> >       unsigned int n_variable_pin_cfg;
> > +     void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
> > +     void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
> >  };
> >
> >  /**
> > @@ -462,7 +466,6 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] =
> > {  static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> >                                      u8 pin, u8 off, u8 func)
> >  {
> > -     const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> >       unsigned long flags;
> >       u32 reg;
> >
> > @@ -473,27 +476,23 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> >       reg &= ~(PM_MASK << (pin * 2));
> >       writew(reg, pctrl->base + PM(off));
> >
> > +     pctrl->data->pwpr_pfc_unlock(pctrl);
> > +
> >       /* Temporarily switch to GPIO mode with PMC register */
> >       reg = readb(pctrl->base + PMC(off));
> >       writeb(reg & ~BIT(pin), pctrl->base + PMC(off));
> >
> > -     /* Set the PWPR register to allow PFC register to write */
> > -     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > -     writel(PWPR_PFCWE, pctrl->base + regs->pwpr);   /* B0WI=0, PFCWE=1 */
> > -
> >       /* 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));
> >
> > -     /* Set the PWPR register to be write-protected */
> > -     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > -     writel(PWPR_B0WI, pctrl->base + regs->pwpr);    /* B0WI=1, PFCWE=0 */
> > -
> >       /* Switch to Peripheral pin function with PMC register */
> >       reg = readb(pctrl->base + PMC(off));
> >       writeb(reg | BIT(pin), pctrl->base + PMC(off));
> >
> > +     pctrl->data->pwpr_pfc_lock(pctrl);
> > +
> >       spin_unlock_irqrestore(&pctrl->lock, flags);  };
> >
> > @@ -2519,12 +2518,8 @@ static void rzg2l_pinctrl_pm_setup_dedicated_regs(struct rzg2l_pinctrl
> > *pctrl, b  static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)  {
> >       u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT;
> > -     const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> > -     const struct rzg2l_register_offsets *regs = &hwcfg->regs;
> >
> > -     /* Set the PWPR register to allow PFC register to write. */
> > -     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > -     writel(PWPR_PFCWE, pctrl->base + regs->pwpr);   /* B0WI=0, PFCWE=1 */
> > +     pctrl->data->pwpr_pfc_unlock(pctrl);
> >
> >       /* Restore port registers. */
> >       for (u32 port = 0; port < nports; port++) { @@ -2567,9 +2562,7 @@ static void
> > rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
> >               }
> >       }
> >
> > -     /* Set the PWPR register to be write-protected. */
> > -     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > -     writel(PWPR_B0WI, pctrl->base + regs->pwpr);    /* B0WI=1, PFCWE=0 */
> > +     pctrl->data->pwpr_pfc_lock(pctrl);
> >  }
> >
> >  static int rzg2l_pinctrl_suspend_noirq(struct device *dev) @@ -2631,6 +2624,24 @@ static int
> > rzg2l_pinctrl_resume_noirq(struct device *dev)
> >       return 0;
> >  }
> >
> > +static void rzg2l_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl) {
> > +     const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> > +
> > +     /* Set the PWPR register to allow PFC register to write */
> > +     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > +     writel(PWPR_PFCWE, pctrl->base + regs->pwpr);   /* B0WI=0, PFCWE=1 */
> > +}
> > +
> > +static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl) {
> > +     const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> > +
> > +     /* Set the PWPR register to be write-protected */
> > +     writel(0x0, pctrl->base + regs->pwpr);          /* B0WI=0, PFCWE=0 */
> > +     writel(PWPR_B0WI, pctrl->base + regs->pwpr);    /* B0WI=1, PFCWE=0 */
> > +}
> > +
> >  static const struct rzg2l_hwcfg rzg2l_hwcfg = {
> >       .regs = {
> >               .pwpr = 0x3014,
> > @@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> >       .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
> >       .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
> >  #endif
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> >  };
> >
> >  static struct rzg2l_pinctrl_data r9a07g044_data = { @@ -2699,6 +2712,8 @@ static struct
> > rzg2l_pinctrl_data r9a07g044_data = {
> >       .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> >               ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
> >       .hwcfg = &rzg2l_hwcfg,
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> >  };
> >
> >  static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2709,6 +2724,8 @@ static struct
> > rzg2l_pinctrl_data r9a08g045_data = {
> >       .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
> >       .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
> >       .hwcfg = &rzg3s_hwcfg,
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
>
> Some memory can be saved by avoiding duplication of data by using
> a single pointer for structure containing function pointers??
>
> struct rzg2l_pinctrl_fns {
>         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
>         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
> }
>
Ok makes sense, I will do that in the next version.

Cheers,
Prabhakar

Re: [PATCH v2 00/13] Add PFC support for Renesas RZ/V2H(P) SoC

[Lad, Prabhakar]

On Tue, Apr 23, 2024 at 6:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Hi All,
>
> This patch series aims to add PFC (Pin Function Controller) support for
> Renesas RZ/V2H(P) SoC. The PFC block on RZ/V2H(P) is almost similar to
> one found on the RZ/G2L family with couple of differences. To able to
> re-use the use the existing driver for RZ/V2H(P) SoC function pointers
> are introduced based on the SoC changes.
>
>
> RFC->v2
> - Fixed review comments pointed by Rob
> - Incorporated changes suggested by Claudiu
> - Fixed build error reported for m68K
> - Dropped IOLH groups as we will be passing register values
> - Fixed configs for dedicated pins
> - Added support for slew-rate and bias settings
> - Added support for OEN
>
> RFC: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240326222844.1422948-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (13):
>   dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Remove the check from the
>     object
>   dt-bindings: pinctrl: renesas: Document RZ/V2H(P) SoC
>   pinctrl: renesas: pinctrl-rzg2l: Allow more bits for pin configuration
>   pinctrl: renesas: pinctrl-rzg2l: Allow parsing of variable
>     configuration for all architectures
>   pinctrl: renesas: pinctrl-rzg2l: Validate power registers for SD and
>     ETH
>   pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
>     locking/unlocking the PFC register
>   pinctrl: renesas: pinctrl-rzg2l: Add function pointer for writing to
>     PMC register
>   pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
>     reading/writing OEN register
>   pinctrl: renesas: pinctrl-rzg2l: Add support to configure the
>     slew-rate
>   pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down
>     the pins
>   pinctrl: renesas: pinctrl-rzg2l: Pass pincontrol device pointer to
>     pinconf_generic_parse_dt_config()
>   pinctrl: renesas: pinctrl-rzg2l: Add support for custom parameters
>   pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC
>
Gentle ping.

Cheers,
Prabhakar

>  .../pinctrl/renesas,rzg2l-pinctrl.yaml        |  40 +-
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 640 ++++++++++++++++--
>  2 files changed, 617 insertions(+), 63 deletions(-)
>
> --
> 2.34.1
>

Re: [PATCH v2 01/13] dt-bindings: pinctrl: renesas,rzg2l-pinctrl: Remove the check from the object

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Drop the bogus check from object as this didn't really add restriction
> check.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> RFC->v2
> - Updated commit message
> - Collected RB tag from Rob

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

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 v2 03/13] pinctrl: renesas: pinctrl-rzg2l: Allow more bits for pin configuration

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The pin configuration bits have been growing for every new SoCs being
> added for the pinctrl-rzg2l driver which would mean updating the macros
> every time for each new configuration. To avoid this allocate additional
> bits for pin configuration by relocating the known fixed bits to the very
> end of the configuration.
>
> Also update the size of 'cfg' to 'u64' to allow more configuration bits in
> the 'struct rzg2l_variable_pin_cfg'.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - Merged the macros and rzg2l_variable_pin_cfg changes into single patch
> - Updated types for the config changes

Thanks for the update!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -78,9 +78,9 @@
>                                          PIN_CFG_FILNUM | \
>                                          PIN_CFG_FILCLKSEL)
>
> -#define PIN_CFG_PIN_MAP_MASK           GENMASK_ULL(35, 28)
> -#define PIN_CFG_PIN_REG_MASK           GENMASK(27, 20)
> -#define PIN_CFG_MASK                   GENMASK(19, 0)
> +#define PIN_CFG_PIN_MAP_MASK           GENMASK_ULL(62, 55)
> +#define PIN_CFG_PIN_REG_MASK           GENMASK_ULL(54, 47)
> +#define PIN_CFG_MASK                   GENMASK_ULL(46, 0)
>
>  /*
>   * m indicates the bitmap of supported pins, a is the register index

> @@ -241,9 +241,9 @@ struct rzg2l_dedicated_configs {
>   * @pin: port pin
>   */
>  struct rzg2l_variable_pin_cfg {
> -       u32 cfg:20;
> -       u32 port:5;
> -       u32 pin:3;
> +       u64 cfg:46;

47, to match PIN_CFG_MASK()?

> +       u64 port:5;
> +       u64 pin:3;
>  };

To avoid such mistakes, and to increase uniformity, I think it would
be good to get rid of this structure, and replace it by masks, to be
used with FIELD_GET() and FIELD_PREP_CONST().

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 v2 04/13] pinctrl: renesas: pinctrl-rzg2l: Allow parsing of variable configuration for all architectures

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Enable parsing of variable configuration for all architectures. This patch
> is in preparation for adding support for the RZ/V2H SoC, which utilizes the
> ARM64 architecture and features port pins with variable configuration.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - No change

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 05/13] pinctrl: renesas: pinctrl-rzg2l: Validate power registers for SD and ETH

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On RZ/V2H(P) SoC, the power registers for SD and ETH do not exist,
> resulting in invalid register offsets. Ensure that the register offsets
> are valid before any read/write operations are performed. If the power
> registers are not available, both SD and ETH will be set to '0'.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - Update check to != 0 instead of -EINVAL

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls
> writing to both PFC and PMC registers. Additionally, BIT(7) B0WI is
> undocumented for the PWPR register on RZ/V2H(P) SoC. To accommodate these
> differences across SoC variants, introduce the set_pfc_mode() and
> pm_set_pfc() function pointers.
>
> Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is now
> called before PMC read/write and pwpr_pfc_lock() call is now called after
> PMC read/write this is to keep changes minimal for RZ/V2H(P).
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - Introduced function pointer for (un)lock

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Geert Uytterhoeven]

Hi Biju,

On Tue, Apr 23, 2024 at 8:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Prabhakar <prabhakar.csengg@gmail.com>
> > Sent: Tuesday, April 23, 2024 6:59 PM
> > Subject: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
> > locking/unlocking the PFC register
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> > However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit controls writing to both PFC and
> > PMC registers. Additionally, BIT(7) B0WI is undocumented for the PWPR register on RZ/V2H(P) SoC. To
> > accommodate these differences across SoC variants, introduce the set_pfc_mode() and
> > pm_set_pfc() function pointers.
> >
> > Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is now called before PMC
> > read/write and pwpr_pfc_lock() call is now called after PMC read/write this is to keep changes
> > minimal for RZ/V2H(P).
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - Introduced function pointer for (un)lock

> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> >       .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
> >       .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
> >  #endif
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> >  };
> >
> >  static struct rzg2l_pinctrl_data r9a07g044_data = { @@ -2699,6 +2712,8 @@ static struct
> > rzg2l_pinctrl_data r9a07g044_data = {
> >       .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> >               ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
> >       .hwcfg = &rzg2l_hwcfg,
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> >  };
> >
> >  static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2709,6 +2724,8 @@ static struct
> > rzg2l_pinctrl_data r9a08g045_data = {
> >       .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
> >       .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
> >       .hwcfg = &rzg3s_hwcfg,
> > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
>
> Some memory can be saved by avoiding duplication of data by using
> a single pointer for structure containing function pointers??
>
> struct rzg2l_pinctrl_fns {
>         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
>         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
> }

So that would replace 3 (4 after adding RZ/V2H support) x 2 pointers in
rzg2l_pinctrl_data structures by 3 (4) pointers in rzg2l_pinctrl_data
structures + 1 (2) x 2 pointers in rzg2l_pinctrl_fns structures, and
code size would increase due to extra pointer dereferences before
each call.
Am I missing something?

Merging rzg2l_pwpr_pfc_{,un}lock() into a single function (taking a
"bool lock" flag) might be a better solution to reduce rzg2l_pinctrl_data size.

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 v2 07/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointer for writing to PMC register

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This patch introduces a function pointer, pmc_writeb(), in the
> struct rzg2l_pinctrl_data to facilitate writing to the PMC register. On
> the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_A bit before writing to PMC
> registers is required, whereas this is not the case for the existing
> RZ/G2L family. This addition enables the reuse of existing code for
> RZ/V2H(P). Additionally, this patch populates this function pointer with
> appropriate data for existing SoCs.
>
> Note that this functionality is only handled in rzg2l_gpio_request(), as
> PMC unlock/lock during PFC setup will be taken care of in the
> pwpr_pfc_unlock/pwpr_pfc_lock.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - No change

Thanks for the update!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -463,6 +464,11 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
>  };
>  #endif
>
> +static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)

Please pass the register offset instead of the virtual register address.
You do have pctrl->base here, and rzv2h_pmc_writeb() will need to use
pctrl->base for all other register writes anyway.

> +{
> +       writeb(val, addr);
> +}

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 v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Wednesday, May 22, 2024 1:23 PM
> Subject: Re: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
> locking/unlocking the PFC register
> 
> Hi Biju,
> 
> On Tue, Apr 23, 2024 at 8:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Prabhakar <prabhakar.csengg@gmail.com>
> > > Sent: Tuesday, April 23, 2024 6:59 PM
> > > Subject: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add
> > > function pointers for locking/unlocking the PFC register
> > >
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> > > However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit
> > > controls writing to both PFC and PMC registers. Additionally, BIT(7)
> > > B0WI is undocumented for the PWPR register on RZ/V2H(P) SoC. To
> > > accommodate these differences across SoC variants, introduce the
> > > set_pfc_mode() and
> > > pm_set_pfc() function pointers.
> > >
> > > Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is
> > > now called before PMC read/write and pwpr_pfc_lock() call is now
> > > called after PMC read/write this is to keep changes minimal for RZ/V2H(P).
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > RFC->v2
> > > - Introduced function pointer for (un)lock
> 
> > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > @@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> > >       .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
> > >       .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
> > >  #endif
> > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> > >  };
> > >
> > >  static struct rzg2l_pinctrl_data r9a07g044_data = { @@ -2699,6
> > > +2712,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
> > >       .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> > >               ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
> > >       .hwcfg = &rzg2l_hwcfg,
> > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> > >  };
> > >
> > >  static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2709,6
> > > +2724,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
> > >       .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
> > >       .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
> > >       .hwcfg = &rzg3s_hwcfg,
> > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> >
> > Some memory can be saved by avoiding duplication of data by using a
> > single pointer for structure containing function pointers??
> >
> > struct rzg2l_pinctrl_fns {
> >         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
> >         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl); }
> 
> So that would replace 3 (4 after adding RZ/V2H support) x 2 pointers in rzg2l_pinctrl_data
> structures by 3 (4) pointers in rzg2l_pinctrl_data structures + 1 (2) x 2 pointers in
> rzg2l_pinctrl_fns structures, and code size would increase due to extra pointer dereferences before
> each call.
> Am I missing something?

Current case
3 * 2 pointers = 6 pointers

Suggestion
3 * 1 pointer + 1 * 2 pointer = 5 pointers

As you said,  code size would increase due to extra pointer dereferences before
each call.


> 
> Merging rzg2l_pwpr_pfc_{,un}lock() into a single function (taking a "bool lock" flag) might be a
> better solution to reduce rzg2l_pinctrl_data size.

I agree.

Cheers,
Biju

Re: [PATCH v2 08/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for reading/writing OEN register

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This patch introduces function pointers, read_oen() and write_oen(), in the
> struct rzg2l_pinctrl_data to facilitate reading and writing to the PFC_OEN
> register. On the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_B bit before
> writing to the PFC_OEN register is necessary, and the PFC_OEN register has
> more bits compared to the RZ/G2L family. To handle these differences
> between RZ/G2L and RZ/V2H(P) and to reuse the existing code for RZ/V2H(P),
> these function pointers are introduced.
>
> Additionally, this patch populates these function pointers with appropriate
> data for existing SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - No change

Thanks for the update!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -261,6 +261,8 @@ struct rzg2l_pinctrl_data {
>         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
>         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
>         void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> +       u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
> +       int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);

Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering,
"read_oen" uses <verb>_<noun> ordering.

The rest LGTM.

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 v2 09/13] pinctrl: renesas: pinctrl-rzg2l: Add support to configure the slew-rate

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add support to configure slew-rate property of the pin.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - New patch

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add support to configure bias-disable, bias-pull-up and bias-pull-down
> properties of the pin.
>
> Two new function pointers get_bias_param() and get_bias_val() are
> introduced as the values in PUPD register differ when compared to
> RZ/G2L family and RZ/V2H(P) SoC,
>
> Value | RZ/G2L        | RZ/V2H
> ---------------------------------
> 00b:  | Bias Disabled | Pull up/down disabled
> 01b:  | Pull-up       | Pull up/down disabled
> 10b:  | Pull-down     | Pull-down
> 11b:  | Prohibited    | Pull-up
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - New patch

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
>                 arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
>                 break;
>
> +       case PIN_CONFIG_BIAS_DISABLE:
> +       case PIN_CONFIG_BIAS_PULL_UP:
> +       case PIN_CONFIG_BIAS_PULL_DOWN: {
> +               if (!(cfg & PIN_CFG_PUPD))
> +                       return -EINVAL;
> +
> +               ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
> +                                                                       PUPD(off),
> +                                                                       bit, PUPD_MASK));

This is rather long, so please split it in two parts, like is done in
other cases in this function:

    arg = rzg2l_read_pin_config(pctrl, PUPD(off), bit, PUPD_MASK);
    ret = pctrl->data->get_bias_param(arg);

> +               if (ret < 0)
> +                       return ret;
> +
> +               if (ret != param)
> +                       return -EINVAL;
> +               /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */
> +               arg = 1;
> +               break;
> +       }
> +

The rest LGTM.

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 v2 11/13] pinctrl: renesas: pinctrl-rzg2l: Pass pincontrol device pointer to pinconf_generic_parse_dt_config()

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Pass pincontrol device pointer to pinconf_generic_parse_dt_config() in
> prepration for passing custom params.

preparation

>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - No change

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v2 12/13] pinctrl: renesas: pinctrl-rzg2l: Add support for custom parameters

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> In preparation for passing custom params for RZ/V2H(P) SoC assign the
> custom params that is being passed via struct rzg2l_pinctrl_data.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - No change

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -262,6 +262,9 @@ struct rzg2l_pinctrl_data {
>         const struct rzg2l_hwcfg *hwcfg;
>         const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
>         unsigned int n_variable_pin_cfg;
> +       unsigned int num_custom_params;
> +       const struct pinconf_generic_params *custom_params;
> +       const struct pin_config_item *custom_conf_items;

Perhaps this should be protected by #ifdef CONFIG_DEBUG_FS, too?

>         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
>         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
>         void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> @@ -2374,6 +2377,13 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl)
>         pctrl->desc.pmxops = &rzg2l_pinctrl_pmxops;
>         pctrl->desc.confops = &rzg2l_pinctrl_confops;
>         pctrl->desc.owner = THIS_MODULE;
> +       if (pctrl->data->num_custom_params) {
> +               pctrl->desc.num_custom_params = pctrl->data->num_custom_params;
> +               pctrl->desc.custom_params = pctrl->data->custom_params;
> +#ifdef CONFIG_DEBUG_FS
> +               pctrl->desc.custom_conf_items = pctrl->data->custom_conf_items;
> +#endif
> +       }
>
>         pins = devm_kcalloc(pctrl->dev, pctrl->desc.npins, sizeof(*pins), GFP_KERNEL);
>         if (!pins)

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 v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Geert Uytterhoeven]

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add support to configure bias-disable, bias-pull-up and bias-pull-down
> properties of the pin.
>
> Two new function pointers get_bias_param() and get_bias_val() are
> introduced as the values in PUPD register differ when compared to
> RZ/G2L family and RZ/V2H(P) SoC,
>
> Value | RZ/G2L        | RZ/V2H
> ---------------------------------
> 00b:  | Bias Disabled | Pull up/down disabled
> 01b:  | Pull-up       | Pull up/down disabled
> 10b:  | Pull-down     | Pull-down
> 11b:  | Prohibited    | Pull-up
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - New patch
> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 102fa75c71d3..c144bf43522b 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -122,6 +122,7 @@
>  #define IOLH(off)              (0x1000 + (off) * 8)
>  #define SR(off)                        (0x1400 + (off) * 8)
>  #define IEN(off)               (0x1800 + (off) * 8)
> +#define PUPD(off)              (0x1C00 + (off) * 8)
>  #define ISEL(off)              (0x2C00 + (off) * 8)
>  #define SD_CH(off, ch)         ((off) + (ch) * 4)
>  #define ETH_POC(off, ch)       ((off) + (ch) * 4)
> @@ -140,6 +141,7 @@
>  #define IEN_MASK               0x01
>  #define IOLH_MASK              0x03
>  #define SR_MASK                        0x01
> +#define PUPD_MASK              0x03
>
>  #define PM_INPUT               0x1
>  #define PM_OUTPUT              0x2
> @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
>         void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
>         u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
>         int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
> +       int (*get_bias_param)(u8 val);
> +       int (*get_bias_val)(enum pin_config_param param);

Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering,
"get_bias_pararm" uses <verb>_<noun> ordering.

Perhaps "hw_to_bias_param()" and "bias_param_to_hw()?"

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 v2 13/13] pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add pinctrl driver support for RZ/V2H(P) SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - Renamed renesas-rzv2h,output-impedance -> renesas,output-impedance
> - Dropped IOLH groups
> - Fixed dedicated pin configs
> - Updated r9a09g057_variable_pin_cfg
> - Added support OEN
> - Added support for bias settings
> - Added function pointers for pwpr (un)lock
> - Added support for slew-rate

Thanks for the update!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -59,6 +59,10 @@
>  #define PIN_CFG_OEN                    BIT(15)
>  #define PIN_CFG_VARIABLE               BIT(16)
>  #define PIN_CFG_NOGPIO_INT             BIT(17)
> +#define PIN_CFG_OPEN_DRAIN             BIT(18)

Or PIN_CFG_NOD, to match the docs?
You can always add a comment if the meaning is unclear:
/* N-ch Open Drain */

> +#define PIN_CFG_SCHMIT_CTRL            BIT(19)

SCHMITT (double T). Or just call it PIN_CFG_SMT, to match the docs?
/* Schmitt-trigger input control */

> +#define PIN_CFG_ELC                    BIT(20)

/* Event Link Control */

> +#define PIN_CFG_IOLH_RZV2H             BIT(21)
>
>  #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
>                                         (PIN_CFG_IOLH_##group | \
> @@ -73,6 +77,10 @@
>  #define RZG3S_MPXED_PIN_FUNCS(group)   (RZG2L_MPXED_COMMON_PIN_FUNCS(group) | \
>                                          PIN_CFG_SOFT_PS)
>
> +#define RZV2H_MPXED_PIN_FUNCS          (RZG2L_MPXED_COMMON_PIN_FUNCS(RZV2H) | \
> +                                        PIN_CFG_OPEN_DRAIN | \
> +                                        PIN_CFG_SR)

I think you can include PIN_CFG_SCHMIT_CTRL here, and thus drop it
from all tables below?

> +
>  #define RZG2L_MPXED_ETH_PIN_FUNCS(x)   ((x) | \
>                                          PIN_CFG_FILONOFF | \
>                                          PIN_CFG_FILNUM | \
> @@ -128,13 +136,15 @@
>  #define ETH_POC(off, ch)       ((off) + (ch) * 4)
>  #define QSPI                   (0x3008)
>  #define ETH_MODE               (0x3018)
> +#define PFC_OEN                        (0x3C40) /* known on RZ/V2H(P) only */
>
>  #define PVDD_2500              2       /* I/O domain voltage 2.5V */
>  #define PVDD_1800              1       /* I/O domain voltage <= 1.8V */
>  #define PVDD_3300              0       /* I/O domain voltage >= 3.3V */
>
>  #define PWPR_B0WI              BIT(7)  /* Bit Write Disable */

FWIW, this should be PWPR_BOWI (O like in Oscar, not 0 = Zero).

> -#define PWPR_PFCWE             BIT(6)  /* PFC Register Write Enable */
> +#define              BIT(6)  /* PFC (and PMC on RZ/V2H) Register Write Enable */

As this bit is called differently on RZ/V2H, and there are different
code paths to handle PWPR on RZ/V2H vs. RZ/G2L, please add an extra
definition for PWPR_REGWE_A, and use that in RZ/V2H-specific
functions.

> +#define PWPR_REGWE_B           BIT(5)  /* OEN Register Write Enable, known only in RZ/V2H(P) */
>
>  #define PM_MASK                        0x03
>  #define PFC_MASK               0x07
                                   \
> @@ -330,6 +353,8 @@ struct rzg2l_pinctrl {
>         spinlock_t                      lock; /* lock read/write registers */
>         struct mutex                    mutex; /* serialize adding groups and functions */
>
> +       raw_spinlock_t                  pwpr_lock; /* serialize PWPR register access */

Do you need this lock?
I.e. can't you use the existing .lock above instead? (see below)

> +
>         struct rzg2l_pinctrl_pin_settings *settings;
>         struct rzg2l_pinctrl_reg_cache  *cache;
>         struct rzg2l_pinctrl_reg_cache  *dedicated_cache;

> @@ -480,6 +538,19 @@ static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *
>         writeb(val, addr);
>  }
>
> +static void rzv2h_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
> +{
> +       const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> +       u8 pwpr;
> +
> +       raw_spin_lock(&pctrl->pwpr_lock);

rzg2l_pinctrl_data.pmc_write() is always called with rzg2l_pinctrl.lock
held.

> +       pwpr = readb(pctrl->base + regs->pwpr);
> +       writeb(pwpr | PWPR_PFCWE, pctrl->base + regs->pwpr);

PWPR_REGWE_A

> +       writeb(val, addr);
> +       writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);

PWPR_REGWE_A

> +       raw_spin_unlock(&pctrl->pwpr_lock);
> +}
> +
>  static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
>                                        u8 pin, u8 off, u8 func)
>  {

> +static u8 rzv2h_pin_to_oen_bit(struct rzg2l_pinctrl *pctrl, u32 offset)
> +{
> +       static const char * const pin_names[] = { "ET0_TXC_TXCLK", "ET1_TXC_TXCLK",
> +                                                 "XSPI0_RESET0N", "XSPI0_CS0N",
> +                                                 "XSPI0_CKN", "XSPI0_CKP" };

        static const char * const pin_names[] = {
                "ET0_TXC_TXCLK", "ET1_TXC_TXCLK", "XSPI0_RESET0N",
                "XSPI0_CS0N", "XSPI0_CKN", "XSPI0_CKP"
        };

> +       const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
> +       u8 bit_array[] = { 0, 1, 2, 3, 4, 5 };

Do you need this identity-transforming array? ;-)

> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(bit_array); i++) {

ARRAY_SIZE(pin_names)

> +               if (!strcmp(pin_desc->name, pin_names[i]))
> +                       return bit_array[i];

return i;

> +       }
> +
> +       /* Should not happen. */
> +       return 0;
> +}
> +
> +static u32 rzv2h_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin)
> +{
> +       u8 bit;
> +
> +       if (!(caps & PIN_CFG_OEN))
> +               return 0;
> +
> +       bit = rzv2h_pin_to_oen_bit(pctrl, offset);
> +
> +       return !(readb(pctrl->base + PFC_OEN) & BIT(bit));
> +}
> +
> +static int rzv2h_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen)
> +{
> +       const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> +       const struct rzg2l_register_offsets *regs = &hwcfg->regs;
> +       unsigned long flags;
> +       u8 val, bit;
> +       u8 pwpr;
> +
> +       if (!(caps & PIN_CFG_OEN))
> +               return -EINVAL;
> +
> +       bit = rzv2h_pin_to_oen_bit(pctrl, offset);
> +       spin_lock_irqsave(&pctrl->lock, flags);
> +       val = readb(pctrl->base + PFC_OEN);
> +       if (oen)
> +               val &= ~BIT(bit);
> +       else
> +               val |= BIT(bit);
> +
> +       raw_spin_lock(&pctrl->pwpr_lock);

rzg2l_pinctrl.lock is taken above.

> +       pwpr = readb(pctrl->base + regs->pwpr);
> +       writeb(pwpr | PWPR_REGWE_B, pctrl->base + regs->pwpr);
> +       writeb(val, pctrl->base + PFC_OEN);
> +       writeb(pwpr & ~PWPR_REGWE_B, pctrl->base + regs->pwpr);
> +       raw_spin_unlock(&pctrl->pwpr_lock);
> +       spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> +       return 0;
> +}

> @@ -2747,6 +3098,32 @@ static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
>         writel(PWPR_B0WI, pctrl->base + regs->pwpr);    /* B0WI=1, PFCWE=0 */
>  }
>
> +static void rzv2h_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl)
> +{
> +       const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> +       u8 pwpr;
> +
> +       /*
> +        * lock is acquired in pfc unlock call back and then released in
> +        * pfc lock callback
> +        */
> +       raw_spin_lock(&pctrl->pwpr_lock);

Except for rzg2l_pinctrl_pm_setup_pfc(), this function is always
called while holding rzg2l_pinctrl.lock.  So I think you can just
take rzg2l_pinctrl.lock in rzg2l_pinctrl_pm_setup_pfc(), and get rid
of pwpr_lock?

> +       /* Set the PWPR register to allow PFC and PMC register to write */
> +       pwpr = readb(pctrl->base + regs->pwpr);
> +       writeb(PWPR_PFCWE | pwpr, pctrl->base + regs->pwpr);

PWPR_REGWE_A

> +}
> +
> +static void rzv2h_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
> +{
> +       const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> +       u8 pwpr;
> +
> +       /* Set the PWPR register to be write-protected */
> +       pwpr = readb(pctrl->base + regs->pwpr);
> +       writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);

PWPR_REGWE_A

> +       raw_spin_unlock(&pctrl->pwpr_lock);
> +}
> +
>  static const struct rzg2l_hwcfg rzg2l_hwcfg = {
>         .regs = {
>                 .pwpr = 0x3014,

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 v2 03/13] pinctrl: renesas: pinctrl-rzg2l: Allow more bits for pin configuration

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 11:19 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The pin configuration bits have been growing for every new SoCs being
> > added for the pinctrl-rzg2l driver which would mean updating the macros
> > every time for each new configuration. To avoid this allocate additional
> > bits for pin configuration by relocating the known fixed bits to the very
> > end of the configuration.
> >
> > Also update the size of 'cfg' to 'u64' to allow more configuration bits in
> > the 'struct rzg2l_variable_pin_cfg'.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - Merged the macros and rzg2l_variable_pin_cfg changes into single patch
> > - Updated types for the config changes
>
> Thanks for the update!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -78,9 +78,9 @@
> >                                          PIN_CFG_FILNUM | \
> >                                          PIN_CFG_FILCLKSEL)
> >
> > -#define PIN_CFG_PIN_MAP_MASK           GENMASK_ULL(35, 28)
> > -#define PIN_CFG_PIN_REG_MASK           GENMASK(27, 20)
> > -#define PIN_CFG_MASK                   GENMASK(19, 0)
> > +#define PIN_CFG_PIN_MAP_MASK           GENMASK_ULL(62, 55)
> > +#define PIN_CFG_PIN_REG_MASK           GENMASK_ULL(54, 47)
> > +#define PIN_CFG_MASK                   GENMASK_ULL(46, 0)
> >
> >  /*
> >   * m indicates the bitmap of supported pins, a is the register index
>
> > @@ -241,9 +241,9 @@ struct rzg2l_dedicated_configs {
> >   * @pin: port pin
> >   */
> >  struct rzg2l_variable_pin_cfg {
> > -       u32 cfg:20;
> > -       u32 port:5;
> > -       u32 pin:3;
> > +       u64 cfg:46;
>
> 47, to match PIN_CFG_MASK()?
>
Oops, I missed that.

> > +       u64 port:5;
> > +       u64 pin:3;
> >  };
>
> To avoid such mistakes, and to increase uniformity, I think it would
> be good to get rid of this structure, and replace it by masks, to be
> used with FIELD_GET() and FIELD_PREP_CONST().
>
Agreed, I will make a patch on top of this patch (so that its easier
for review).

Cheers,
Prabhakar

Re: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for locking/unlocking the PFC register

[Lad, Prabhakar]

Hi Biju, Geert,

Thank you for the review.

On Wed, May 22, 2024 at 1:40 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Geert,
>
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: Wednesday, May 22, 2024 1:23 PM
> > Subject: Re: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for
> > locking/unlocking the PFC register
> >
> > Hi Biju,
> >
> > On Tue, Apr 23, 2024 at 8:12 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > -----Original Message-----
> > > > From: Prabhakar <prabhakar.csengg@gmail.com>
> > > > Sent: Tuesday, April 23, 2024 6:59 PM
> > > > Subject: [PATCH v2 06/13] pinctrl: renesas: pinctrl-rzg2l: Add
> > > > function pointers for locking/unlocking the PFC register
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > On the RZ/G2L SoC, the PFCWE bit controls writing to PFC registers.
> > > > However, on the RZ/V2H(P) SoC, the PFCWE (REGWE_A on RZ/V2H) bit
> > > > controls writing to both PFC and PMC registers. Additionally, BIT(7)
> > > > B0WI is undocumented for the PWPR register on RZ/V2H(P) SoC. To
> > > > accommodate these differences across SoC variants, introduce the
> > > > set_pfc_mode() and
> > > > pm_set_pfc() function pointers.
> > > >
> > > > Note, in rzg2l_pinctrl_set_pfc_mode() the pwpr_pfc_unlock() call is
> > > > now called before PMC read/write and pwpr_pfc_lock() call is now
> > > > called after PMC read/write this is to keep changes minimal for RZ/V2H(P).
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > > RFC->v2
> > > > - Introduced function pointer for (un)lock
> >
> > > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > > @@ -2688,6 +2699,8 @@ static struct rzg2l_pinctrl_data r9a07g043_data = {
> > > >       .variable_pin_cfg = r9a07g043f_variable_pin_cfg,
> > > >       .n_variable_pin_cfg = ARRAY_SIZE(r9a07g043f_variable_pin_cfg),
> > > >  #endif
> > > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> > > >  };
> > > >
> > > >  static struct rzg2l_pinctrl_data r9a07g044_data = { @@ -2699,6
> > > > +2712,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
> > > >       .n_dedicated_pins = ARRAY_SIZE(rzg2l_dedicated_pins.common) +
> > > >               ARRAY_SIZE(rzg2l_dedicated_pins.rzg2l_pins),
> > > >       .hwcfg = &rzg2l_hwcfg,
> > > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> > > >  };
> > > >
> > > >  static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2709,6
> > > > +2724,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
> > > >       .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT,
> > > >       .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins),
> > > >       .hwcfg = &rzg3s_hwcfg,
> > > > +     .pwpr_pfc_unlock = &rzg2l_pwpr_pfc_unlock,
> > > > +     .pwpr_pfc_lock = &rzg2l_pwpr_pfc_lock,
> > >
> > > Some memory can be saved by avoiding duplication of data by using a
> > > single pointer for structure containing function pointers??
> > >
> > > struct rzg2l_pinctrl_fns {
> > >         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
> > >         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl); }
> >
> > So that would replace 3 (4 after adding RZ/V2H support) x 2 pointers in rzg2l_pinctrl_data
> > structures by 3 (4) pointers in rzg2l_pinctrl_data structures + 1 (2) x 2 pointers in
> > rzg2l_pinctrl_fns structures, and code size would increase due to extra pointer dereferences before
> > each call.
> > Am I missing something?
>
> Current case
> 3 * 2 pointers = 6 pointers
>
> Suggestion
> 3 * 1 pointer + 1 * 2 pointer = 5 pointers
>
> As you said,  code size would increase due to extra pointer dereferences before
> each call.
>
>
> >
> > Merging rzg2l_pwpr_pfc_{,un}lock() into a single function (taking a "bool lock" flag) might be a
> > better solution to reduce rzg2l_pinctrl_data size.
>
> I agree.
>
OK, I will introduce a single function pointer (pwpr_pfc_lock_unlock)
in this patch.

Cheers,
Prabhakar

Re: [PATCH v2 07/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointer for writing to PMC register

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 1:39 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > This patch introduces a function pointer, pmc_writeb(), in the
> > struct rzg2l_pinctrl_data to facilitate writing to the PMC register. On
> > the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_A bit before writing to PMC
> > registers is required, whereas this is not the case for the existing
> > RZ/G2L family. This addition enables the reuse of existing code for
> > RZ/V2H(P). Additionally, this patch populates this function pointer with
> > appropriate data for existing SoCs.
> >
> > Note that this functionality is only handled in rzg2l_gpio_request(), as
> > PMC unlock/lock during PFC setup will be taken care of in the
> > pwpr_pfc_unlock/pwpr_pfc_lock.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - No change
>
> Thanks for the update!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -463,6 +464,11 @@ static const struct rzg2l_variable_pin_cfg r9a07g043f_variable_pin_cfg[] = {
> >  };
> >  #endif
> >
> > +static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
>
> Please pass the register offset instead of the virtual register address.
> You do have pctrl->base here, and rzv2h_pmc_writeb() will need to use
> pctrl->base for all other register writes anyway.
>
Agreed,  I will pass the register offset (u16 offset) instead of
virtual address.

Cheers,
Prabhakar

Re: [PATCH v2 08/13] pinctrl: renesas: pinctrl-rzg2l: Add function pointers for reading/writing OEN register

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 1:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > This patch introduces function pointers, read_oen() and write_oen(), in the
> > struct rzg2l_pinctrl_data to facilitate reading and writing to the PFC_OEN
> > register. On the RZ/V2H(P) SoC, unlocking the PWPR.REGWE_B bit before
> > writing to the PFC_OEN register is necessary, and the PFC_OEN register has
> > more bits compared to the RZ/G2L family. To handle these differences
> > between RZ/G2L and RZ/V2H(P) and to reuse the existing code for RZ/V2H(P),
> > these function pointers are introduced.
> >
> > Additionally, this patch populates these function pointers with appropriate
> > data for existing SoCs.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - No change
>
> Thanks for the update!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -261,6 +261,8 @@ struct rzg2l_pinctrl_data {
> >         void (*pwpr_pfc_unlock)(struct rzg2l_pinctrl *pctrl);
> >         void (*pwpr_pfc_lock)(struct rzg2l_pinctrl *pctrl);
> >         void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> > +       u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
> > +       int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
>
> Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering,
> "read_oen" uses <verb>_<noun> ordering.
>
Ok, I'll rename them to oen_read() and oen_write().

Cheers,
Prabhakar

Re: [PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 2:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support to configure bias-disable, bias-pull-up and bias-pull-down
> > properties of the pin.
> >
> > Two new function pointers get_bias_param() and get_bias_val() are
> > introduced as the values in PUPD register differ when compared to
> > RZ/G2L family and RZ/V2H(P) SoC,
> >
> > Value | RZ/G2L        | RZ/V2H
> > ---------------------------------
> > 00b:  | Bias Disabled | Pull up/down disabled
> > 01b:  | Pull-up       | Pull up/down disabled
> > 10b:  | Pull-down     | Pull-down
> > 11b:  | Prohibited    | Pull-up
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - New patch
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >                 arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
> >                 break;
> >
> > +       case PIN_CONFIG_BIAS_DISABLE:
> > +       case PIN_CONFIG_BIAS_PULL_UP:
> > +       case PIN_CONFIG_BIAS_PULL_DOWN: {
> > +               if (!(cfg & PIN_CFG_PUPD))
> > +                       return -EINVAL;
> > +
> > +               ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
> > +                                                                       PUPD(off),
> > +                                                                       bit, PUPD_MASK));
>
> This is rather long, so please split it in two parts, like is done in
> other cases in this function:
>
>     arg = rzg2l_read_pin_config(pctrl, PUPD(off), bit, PUPD_MASK);
>     ret = pctrl->data->get_bias_param(arg);
>
Agreed, I'll update it as per your suggestion.

Cheers,
Prabhakar

Re: [PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 2:26 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support to configure bias-disable, bias-pull-up and bias-pull-down
> > properties of the pin.
> >
> > Two new function pointers get_bias_param() and get_bias_val() are
> > introduced as the values in PUPD register differ when compared to
> > RZ/G2L family and RZ/V2H(P) SoC,
> >
> > Value | RZ/G2L        | RZ/V2H
> > ---------------------------------
> > 00b:  | Bias Disabled | Pull up/down disabled
> > 01b:  | Pull-up       | Pull up/down disabled
> > 10b:  | Pull-down     | Pull-down
> > 11b:  | Prohibited    | Pull-up
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - New patch
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index 102fa75c71d3..c144bf43522b 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -122,6 +122,7 @@
> >  #define IOLH(off)              (0x1000 + (off) * 8)
> >  #define SR(off)                        (0x1400 + (off) * 8)
> >  #define IEN(off)               (0x1800 + (off) * 8)
> > +#define PUPD(off)              (0x1C00 + (off) * 8)
> >  #define ISEL(off)              (0x2C00 + (off) * 8)
> >  #define SD_CH(off, ch)         ((off) + (ch) * 4)
> >  #define ETH_POC(off, ch)       ((off) + (ch) * 4)
> > @@ -140,6 +141,7 @@
> >  #define IEN_MASK               0x01
> >  #define IOLH_MASK              0x03
> >  #define SR_MASK                        0x01
> > +#define PUPD_MASK              0x03
> >
> >  #define PM_INPUT               0x1
> >  #define PM_OUTPUT              0x2
> > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
> >         void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> >         u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
> >         int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
> > +       int (*get_bias_param)(u8 val);
> > +       int (*get_bias_val)(enum pin_config_param param);
>
> Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering,
> "get_bias_pararm" uses <verb>_<noun> ordering.
>
> Perhaps "hw_to_bias_param()" and "bias_param_to_hw()?"
>
Ok, I'll rename as suggested above.

Cheers,
Prabhakar

Re: [PATCH v2 12/13] pinctrl: renesas: pinctrl-rzg2l: Add support for custom parameters

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 2:21 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > In preparation for passing custom params for RZ/V2H(P) SoC assign the
> > custom params that is being passed via struct rzg2l_pinctrl_data.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - No change
>
> Thanks for your patch!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -262,6 +262,9 @@ struct rzg2l_pinctrl_data {
> >         const struct rzg2l_hwcfg *hwcfg;
> >         const struct rzg2l_variable_pin_cfg *variable_pin_cfg;
> >         unsigned int n_variable_pin_cfg;
> > +       unsigned int num_custom_params;
> > +       const struct pinconf_generic_params *custom_params;
> > +       const struct pin_config_item *custom_conf_items;
>
> Perhaps this should be protected by #ifdef CONFIG_DEBUG_FS, too?
>
Agreed, I'll protect custom_conf_items by #ifdef CONFIG_DEBUG_FS.

Cheers,
Prabhakar

Re: [PATCH v2 13/13] pinctrl: renesas: pinctrl-rzg2l: Add support for RZ/V2H SoC

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 22, 2024 at 4:30 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add pinctrl driver support for RZ/V2H(P) SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - Renamed renesas-rzv2h,output-impedance -> renesas,output-impedance
> > - Dropped IOLH groups
> > - Fixed dedicated pin configs
> > - Updated r9a09g057_variable_pin_cfg
> > - Added support OEN
> > - Added support for bias settings
> > - Added function pointers for pwpr (un)lock
> > - Added support for slew-rate
>
> Thanks for the update!
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -59,6 +59,10 @@
> >  #define PIN_CFG_OEN                    BIT(15)
> >  #define PIN_CFG_VARIABLE               BIT(16)
> >  #define PIN_CFG_NOGPIO_INT             BIT(17)
> > +#define PIN_CFG_OPEN_DRAIN             BIT(18)
>
> Or PIN_CFG_NOD, to match the docs?
> You can always add a comment if the meaning is unclear:
> /* N-ch Open Drain */
>
Agreed, I will update as above.

> > +#define PIN_CFG_SCHMIT_CTRL            BIT(19)
>
> SCHMITT (double T). Or just call it PIN_CFG_SMT, to match the docs?
> /* Schmitt-trigger input control */
>
Agreed, I will update as above.

> > +#define PIN_CFG_ELC                    BIT(20)
>
> /* Event Link Control */
>
> > +#define PIN_CFG_IOLH_RZV2H             BIT(21)
> >
> >  #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
> >                                         (PIN_CFG_IOLH_##group | \
> > @@ -73,6 +77,10 @@
> >  #define RZG3S_MPXED_PIN_FUNCS(group)   (RZG2L_MPXED_COMMON_PIN_FUNCS(group) | \
> >                                          PIN_CFG_SOFT_PS)
> >
> > +#define RZV2H_MPXED_PIN_FUNCS          (RZG2L_MPXED_COMMON_PIN_FUNCS(RZV2H) | \
> > +                                        PIN_CFG_OPEN_DRAIN | \
> > +                                        PIN_CFG_SR)
>
> I think you can include PIN_CFG_SCHMIT_CTRL here, and thus drop it
> from all tables below?
>
Agreed.

> > +
> >  #define RZG2L_MPXED_ETH_PIN_FUNCS(x)   ((x) | \
> >                                          PIN_CFG_FILONOFF | \
> >                                          PIN_CFG_FILNUM | \
> > @@ -128,13 +136,15 @@
> >  #define ETH_POC(off, ch)       ((off) + (ch) * 4)
> >  #define QSPI                   (0x3008)
> >  #define ETH_MODE               (0x3018)
> > +#define PFC_OEN                        (0x3C40) /* known on RZ/V2H(P) only */
> >
> >  #define PVDD_2500              2       /* I/O domain voltage 2.5V */
> >  #define PVDD_1800              1       /* I/O domain voltage <= 1.8V */
> >  #define PVDD_3300              0       /* I/O domain voltage >= 3.3V */
> >
> >  #define PWPR_B0WI              BIT(7)  /* Bit Write Disable */
>
> FWIW, this should be PWPR_BOWI (O like in Oscar, not 0 = Zero).
>
Indeed, I'll make a seperate patch for this.

> > -#define PWPR_PFCWE             BIT(6)  /* PFC Register Write Enable */
> > +#define              BIT(6)  /* PFC (and PMC on RZ/V2H) Register Write Enable */
>
> As this bit is called differently on RZ/V2H, and there are different
> code paths to handle PWPR on RZ/V2H vs. RZ/G2L, please add an extra
> definition for PWPR_REGWE_A, and use that in RZ/V2H-specific
> functions.
>
Agreed, I will add PWPR_REGWE_A.

> > +#define PWPR_REGWE_B           BIT(5)  /* OEN Register Write Enable, known only in RZ/V2H(P) */
> >
> >  #define PM_MASK                        0x03
> >  #define PFC_MASK               0x07
>                                    \
> > @@ -330,6 +353,8 @@ struct rzg2l_pinctrl {
> >         spinlock_t                      lock; /* lock read/write registers */
> >         struct mutex                    mutex; /* serialize adding groups and functions */
> >
> > +       raw_spinlock_t                  pwpr_lock; /* serialize PWPR register access */
>
> Do you need this lock?
> I.e. can't you use the existing .lock above instead? (see below)
>
> > +
> >         struct rzg2l_pinctrl_pin_settings *settings;
> >         struct rzg2l_pinctrl_reg_cache  *cache;
> >         struct rzg2l_pinctrl_reg_cache  *dedicated_cache;
>
> > @@ -480,6 +538,19 @@ static void rzg2l_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *
> >         writeb(val, addr);
> >  }
> >
> > +static void rzv2h_pmc_writeb(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr)
> > +{
> > +       const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> > +       u8 pwpr;
> > +
> > +       raw_spin_lock(&pctrl->pwpr_lock);
>
> rzg2l_pinctrl_data.pmc_write() is always called with rzg2l_pinctrl.lock
> held.
>
> > +       pwpr = readb(pctrl->base + regs->pwpr);
> > +       writeb(pwpr | PWPR_PFCWE, pctrl->base + regs->pwpr);
>
> PWPR_REGWE_A
>
> > +       writeb(val, addr);
> > +       writeb(pwpr & ~PWPR_PFCWE, pctrl->base + regs->pwpr);
>
> PWPR_REGWE_A
>
> > +       raw_spin_unlock(&pctrl->pwpr_lock);
> > +}
> > +
> >  static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> >                                        u8 pin, u8 off, u8 func)
> >  {
>
> > +static u8 rzv2h_pin_to_oen_bit(struct rzg2l_pinctrl *pctrl, u32 offset)
> > +{
> > +       static const char * const pin_names[] = { "ET0_TXC_TXCLK", "ET1_TXC_TXCLK",
> > +                                                 "XSPI0_RESET0N", "XSPI0_CS0N",
> > +                                                 "XSPI0_CKN", "XSPI0_CKP" };
>
>         static const char * const pin_names[] = {
>                 "ET0_TXC_TXCLK", "ET1_TXC_TXCLK", "XSPI0_RESET0N",
>                 "XSPI0_CS0N", "XSPI0_CKN", "XSPI0_CKP"
>         };
>
> > +       const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
> > +       u8 bit_array[] = { 0, 1, 2, 3, 4, 5 };
>
> Do you need this identity-transforming array? ;-)
>
Oops, it can be simplified.

> > +       unsigned int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(bit_array); i++) {
>
> ARRAY_SIZE(pin_names)
>
> > +               if (!strcmp(pin_desc->name, pin_names[i]))
> > +                       return bit_array[i];
>
> return i;
>
> > +       }
> > +
> > +       /* Should not happen. */
> > +       return 0;
> > +}
> > +
> > +static u32 rzv2h_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin)
> > +{
> > +       u8 bit;
> > +
> > +       if (!(caps & PIN_CFG_OEN))
> > +               return 0;
> > +
> > +       bit = rzv2h_pin_to_oen_bit(pctrl, offset);
> > +
> > +       return !(readb(pctrl->base + PFC_OEN) & BIT(bit));
> > +}
> > +
> > +static int rzv2h_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen)
> > +{
> > +       const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
> > +       const struct rzg2l_register_offsets *regs = &hwcfg->regs;
> > +       unsigned long flags;
> > +       u8 val, bit;
> > +       u8 pwpr;
> > +
> > +       if (!(caps & PIN_CFG_OEN))
> > +               return -EINVAL;
> > +
> > +       bit = rzv2h_pin_to_oen_bit(pctrl, offset);
> > +       spin_lock_irqsave(&pctrl->lock, flags);
> > +       val = readb(pctrl->base + PFC_OEN);
> > +       if (oen)
> > +               val &= ~BIT(bit);
> > +       else
> > +               val |= BIT(bit);
> > +
> > +       raw_spin_lock(&pctrl->pwpr_lock);
>
> rzg2l_pinctrl.lock is taken above.
>
> > +       pwpr = readb(pctrl->base + regs->pwpr);
> > +       writeb(pwpr | PWPR_REGWE_B, pctrl->base + regs->pwpr);
> > +       writeb(val, pctrl->base + PFC_OEN);
> > +       writeb(pwpr & ~PWPR_REGWE_B, pctrl->base + regs->pwpr);
> > +       raw_spin_unlock(&pctrl->pwpr_lock);
> > +       spin_unlock_irqrestore(&pctrl->lock, flags);
> > +
> > +       return 0;
> > +}
>
> > @@ -2747,6 +3098,32 @@ static void rzg2l_pwpr_pfc_lock(struct rzg2l_pinctrl *pctrl)
> >         writel(PWPR_B0WI, pctrl->base + regs->pwpr);    /* B0WI=1, PFCWE=0 */
> >  }
> >
> > +static void rzv2h_pwpr_pfc_unlock(struct rzg2l_pinctrl *pctrl)
> > +{
> > +       const struct rzg2l_register_offsets *regs = &pctrl->data->hwcfg->regs;
> > +       u8 pwpr;
> > +
> > +       /*
> > +        * lock is acquired in pfc unlock call back and then released in
> > +        * pfc lock callback
> > +        */
> > +       raw_spin_lock(&pctrl->pwpr_lock);
>
> Except for rzg2l_pinctrl_pm_setup_pfc(), this function is always
> called while holding rzg2l_pinctrl.lock.  So I think you can just
> take rzg2l_pinctrl.lock in rzg2l_pinctrl_pm_setup_pfc(), and get rid
> of pwpr_lock?
>
Agreed.

Cheers,
Prabhakar

Re: [PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[claudiu beznea]

Hi, Prabhakar,

On 23.04.2024 20:58, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support to configure bias-disable, bias-pull-up and bias-pull-down
> properties of the pin.
> 
> Two new function pointers get_bias_param() and get_bias_val() are
> introduced as the values in PUPD register differ when compared to
> RZ/G2L family and RZ/V2H(P) SoC,
> 
> Value | RZ/G2L        | RZ/V2H
> ---------------------------------
> 00b:  | Bias Disabled | Pull up/down disabled
> 01b:  | Pull-up       | Pull up/down disabled
> 10b:  | Pull-down     | Pull-down
> 11b:  | Prohibited    | Pull-up
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> - New patch
> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 102fa75c71d3..c144bf43522b 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -122,6 +122,7 @@
>  #define IOLH(off)		(0x1000 + (off) * 8)
>  #define SR(off)			(0x1400 + (off) * 8)
>  #define IEN(off)		(0x1800 + (off) * 8)
> +#define PUPD(off)		(0x1C00 + (off) * 8)
>  #define ISEL(off)		(0x2C00 + (off) * 8)
>  #define SD_CH(off, ch)		((off) + (ch) * 4)
>  #define ETH_POC(off, ch)	((off) + (ch) * 4)
> @@ -140,6 +141,7 @@
>  #define IEN_MASK		0x01
>  #define IOLH_MASK		0x03
>  #define SR_MASK			0x01
> +#define PUPD_MASK		0x03
>  
>  #define PM_INPUT		0x1
>  #define PM_OUTPUT		0x2
> @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
>  	void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
>  	u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
>  	int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
> +	int (*get_bias_param)(u8 val);
> +	int (*get_bias_val)(enum pin_config_param param);
>  };
>  
>  /**
> @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8
>  	return 0;
>  }
>  
> +static int rzg2l_get_bias_param(u8 val)
> +{
> +	switch (val) {
> +	case 0:
> +		return PIN_CONFIG_BIAS_DISABLE;
> +	case 1:
> +		return PIN_CONFIG_BIAS_PULL_UP;
> +	case 2:
> +		return PIN_CONFIG_BIAS_PULL_DOWN;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int rzg2l_get_bias_val(enum pin_config_param param)
> +{
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		return 0;
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		return 1;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		return 2;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
>  				     unsigned int _pin,
>  				     unsigned long *config)
> @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
>  		arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
>  		break;
>  
> +	case PIN_CONFIG_BIAS_DISABLE:
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +	case PIN_CONFIG_BIAS_PULL_DOWN: {

Block { } can be removed here.

> +		if (!(cfg & PIN_CFG_PUPD))
> +			return -EINVAL;
> +
> +		ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
> +									PUPD(off),
> +									bit, PUPD_MASK));
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret != param)
> +			return -EINVAL;

Can this happen? Otherwise it can be removed.

> +		/* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */

What about bias disable? I haven't checked in detail, is it OK to do
arg = 1 here?

> +		arg = 1;
> +		break;
> +	}
> +
>  	case PIN_CONFIG_DRIVE_STRENGTH: {
>  		unsigned int index;
>  
> @@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
>  			rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg);
>  			break;
>  
> +		case PIN_CONFIG_BIAS_DISABLE:
> +		case PIN_CONFIG_BIAS_PULL_UP:
> +		case PIN_CONFIG_BIAS_PULL_DOWN: {

Block { } can be removed in this case.

> +			if (!(cfg & PIN_CFG_PUPD))
> +				return -EINVAL;
> +
> +			ret = pctrl->data->get_bias_val(param);
> +			if (ret < 0)
> +				return ret;
> +
> +			rzg2l_rmw_pin_config(pctrl, PUPD(off), bit, PUPD_MASK, ret);
> +			break;
> +		}
> +
>  		case PIN_CONFIG_DRIVE_STRENGTH:
>  			arg = pinconf_to_config_argument(_configs[i]);
>  
> @@ -2746,6 +2815,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = {
>  	.pmc_writeb = &rzg2l_pmc_writeb,
>  	.read_oen = &rzg2l_read_oen,
>  	.write_oen = &rzg2l_write_oen,
> +	.get_bias_param = &rzg2l_get_bias_param,
> +	.get_bias_val = &rzg2l_get_bias_val,
>  };
>  
>  static struct rzg2l_pinctrl_data r9a08g045_data = {
> @@ -2761,6 +2832,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = {
>  	.pmc_writeb = &rzg2l_pmc_writeb,
>  	.read_oen = &rzg2l_read_oen,
>  	.write_oen = &rzg2l_write_oen,
> +	.get_bias_param = &rzg2l_get_bias_param,
> +	.get_bias_val = &rzg2l_get_bias_val,
>  };
>  
>  static const struct of_device_id rzg2l_pinctrl_of_table[] = {

Re: [PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Lad, Prabhakar]

Hi Claudiu,

Thank you for the review.

On Thu, May 30, 2024 at 8:48 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Prabhakar,
>
> On 23.04.2024 20:58, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support to configure bias-disable, bias-pull-up and bias-pull-down
> > properties of the pin.
> >
> > Two new function pointers get_bias_param() and get_bias_val() are
> > introduced as the values in PUPD register differ when compared to
> > RZ/G2L family and RZ/V2H(P) SoC,
> >
> > Value | RZ/G2L        | RZ/V2H
> > ---------------------------------
> > 00b:  | Bias Disabled | Pull up/down disabled
> > 01b:  | Pull-up       | Pull up/down disabled
> > 10b:  | Pull-down     | Pull-down
> > 11b:  | Prohibited    | Pull-up
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - New patch
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index 102fa75c71d3..c144bf43522b 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -122,6 +122,7 @@
> >  #define IOLH(off)            (0x1000 + (off) * 8)
> >  #define SR(off)                      (0x1400 + (off) * 8)
> >  #define IEN(off)             (0x1800 + (off) * 8)
> > +#define PUPD(off)            (0x1C00 + (off) * 8)
> >  #define ISEL(off)            (0x2C00 + (off) * 8)
> >  #define SD_CH(off, ch)               ((off) + (ch) * 4)
> >  #define ETH_POC(off, ch)     ((off) + (ch) * 4)
> > @@ -140,6 +141,7 @@
> >  #define IEN_MASK             0x01
> >  #define IOLH_MASK            0x03
> >  #define SR_MASK                      0x01
> > +#define PUPD_MASK            0x03
> >
> >  #define PM_INPUT             0x1
> >  #define PM_OUTPUT            0x2
> > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
> >       void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> >       u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
> >       int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
> > +     int (*get_bias_param)(u8 val);
> > +     int (*get_bias_val)(enum pin_config_param param);
> >  };
> >
> >  /**
> > @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8
> >       return 0;
> >  }
> >
> > +static int rzg2l_get_bias_param(u8 val)
> > +{
> > +     switch (val) {
> > +     case 0:
> > +             return PIN_CONFIG_BIAS_DISABLE;
> > +     case 1:
> > +             return PIN_CONFIG_BIAS_PULL_UP;
> > +     case 2:
> > +             return PIN_CONFIG_BIAS_PULL_DOWN;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +static int rzg2l_get_bias_val(enum pin_config_param param)
> > +{
> > +     switch (param) {
> > +     case PIN_CONFIG_BIAS_DISABLE:
> > +             return 0;
> > +     case PIN_CONFIG_BIAS_PULL_UP:
> > +             return 1;
> > +     case PIN_CONFIG_BIAS_PULL_DOWN:
> > +             return 2;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> >  static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >                                    unsigned int _pin,
> >                                    unsigned long *config)
> > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >               arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
> >               break;
> >
> > +     case PIN_CONFIG_BIAS_DISABLE:
> > +     case PIN_CONFIG_BIAS_PULL_UP:
> > +     case PIN_CONFIG_BIAS_PULL_DOWN: {
>
> Block { } can be removed here.
>
Agreed, I will drop it.

> > +             if (!(cfg & PIN_CFG_PUPD))
> > +                     return -EINVAL;
> > +
> > +             ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
> > +                                                                     PUPD(off),
> > +                                                                     bit, PUPD_MASK));
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             if (ret != param)
> > +                     return -EINVAL;
>
> Can this happen? Otherwise it can be removed.
>
> > +             /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */
>
> What about bias disable? I haven't checked in detail, is it OK to do
> arg = 1 here?
>
For BIAS_DISABLE config there isn't any argument, hence the above
comment mentions only for UP/DOWN. Passing arg = 1 for BIAS_DISABLE
has no effect.

> > +             arg = 1;
> > +             break;
> > +     }
> > +
> >       case PIN_CONFIG_DRIVE_STRENGTH: {
> >               unsigned int index;
> >
> > @@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
> >                       rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg);
> >                       break;
> >
> > +             case PIN_CONFIG_BIAS_DISABLE:
> > +             case PIN_CONFIG_BIAS_PULL_UP:
> > +             case PIN_CONFIG_BIAS_PULL_DOWN: {
>
> Block { } can be removed in this case.
>
Agreed, I will drop it.

Cheers,
Prabhakar

Re: [PATCH v2 10/13] pinctrl: renesas: pinctrl-rzg2l: Add support to set pulling up/down the pins

[Lad, Prabhakar]

On Thu, May 30, 2024 at 8:48 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Prabhakar,
>
> On 23.04.2024 20:58, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support to configure bias-disable, bias-pull-up and bias-pull-down
> > properties of the pin.
> >
> > Two new function pointers get_bias_param() and get_bias_val() are
> > introduced as the values in PUPD register differ when compared to
> > RZ/G2L family and RZ/V2H(P) SoC,
> >
> > Value | RZ/G2L        | RZ/V2H
> > ---------------------------------
> > 00b:  | Bias Disabled | Pull up/down disabled
> > 01b:  | Pull-up       | Pull up/down disabled
> > 10b:  | Pull-down     | Pull-down
> > 11b:  | Prohibited    | Pull-up
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > - New patch
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index 102fa75c71d3..c144bf43522b 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -122,6 +122,7 @@
> >  #define IOLH(off)            (0x1000 + (off) * 8)
> >  #define SR(off)                      (0x1400 + (off) * 8)
> >  #define IEN(off)             (0x1800 + (off) * 8)
> > +#define PUPD(off)            (0x1C00 + (off) * 8)
> >  #define ISEL(off)            (0x2C00 + (off) * 8)
> >  #define SD_CH(off, ch)               ((off) + (ch) * 4)
> >  #define ETH_POC(off, ch)     ((off) + (ch) * 4)
> > @@ -140,6 +141,7 @@
> >  #define IEN_MASK             0x01
> >  #define IOLH_MASK            0x03
> >  #define SR_MASK                      0x01
> > +#define PUPD_MASK            0x03
> >
> >  #define PM_INPUT             0x1
> >  #define PM_OUTPUT            0x2
> > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data {
> >       void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr);
> >       u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin);
> >       int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen);
> > +     int (*get_bias_param)(u8 val);
> > +     int (*get_bias_val)(enum pin_config_param param);
> >  };
> >
> >  /**
> > @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8
> >       return 0;
> >  }
> >
> > +static int rzg2l_get_bias_param(u8 val)
> > +{
> > +     switch (val) {
> > +     case 0:
> > +             return PIN_CONFIG_BIAS_DISABLE;
> > +     case 1:
> > +             return PIN_CONFIG_BIAS_PULL_UP;
> > +     case 2:
> > +             return PIN_CONFIG_BIAS_PULL_DOWN;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +static int rzg2l_get_bias_val(enum pin_config_param param)
> > +{
> > +     switch (param) {
> > +     case PIN_CONFIG_BIAS_DISABLE:
> > +             return 0;
> > +     case PIN_CONFIG_BIAS_PULL_UP:
> > +             return 1;
> > +     case PIN_CONFIG_BIAS_PULL_DOWN:
> > +             return 2;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> >  static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >                                    unsigned int _pin,
> >                                    unsigned long *config)
> > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
> >               arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK);
> >               break;
> >
> > +     case PIN_CONFIG_BIAS_DISABLE:
> > +     case PIN_CONFIG_BIAS_PULL_UP:
> > +     case PIN_CONFIG_BIAS_PULL_DOWN: {
>
> Block { } can be removed here.
>
> > +             if (!(cfg & PIN_CFG_PUPD))
> > +                     return -EINVAL;
> > +
> > +             ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl,
> > +                                                                     PUPD(off),
> > +                                                                     bit, PUPD_MASK));
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             if (ret != param)
> > +                     return -EINVAL;
>
> Can this happen? Otherwise it can be removed.
>
Yes this can happen (and is needed) as we want to report only the
current BIAS setting of the pin.

For example without this condition I get the below for ET1_RXD3 pin:
pin 173 (ET1_RXD3): input bias disabled, input bias pull down (0x1
ohms), input bias pull up (0x1 ohms)
with the check included:
pin 173 (ET1_RXD3): input bias disabled

Cheers,
Prabhakar