[PATCH 0/4] pinctrl: renesas: rzg2l: Macro Updates and Reorganization for Pin Configuration

[PATCH 0/4] pinctrl: renesas: rzg2l: Macro Updates and Reorganization for Pin Configuration

[Prabhakar]

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

Hi All,

This patch series updates and reorganizes several macros in the
Renesas RZ/G2L pinctrl driver to enhance clarity, align with the
current configuration requirements, and address code structure
improvements.

Cheers,
Prabhakar

Lad Prabhakar (4):
  pinctrl: renesas: rzg2l: Update PIN_CFG_MASK() macro to be 32-bit wide
  pinctrl: renesas: rzg2l: Adjust bit masks for PIN_CFG_VARIABLE to use
    BIT(62)
  pinctrl: renesas: rzg2l: Move RZG2L_SINGLE_PIN definition to top of
    the file
  pinctrl: renesas: rzg2l: Reorganize variable configuration macro

 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 59 ++++++++++++++-----------
 1 file changed, 32 insertions(+), 27 deletions(-)

-- 
2.34.1

[PATCH 1/4] pinctrl: renesas: rzg2l: Update PIN_CFG_MASK() macro to be 32-bit wide

[Prabhakar]

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

Modify the `PIN_CFG_MASK()` macro to be 32-bit wide. The current maximum
value for `PIN_CFG_*` is `BIT(21)`, which fits within a 32-bit mask.

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

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 32945d4c8dc0..bfaeeb00ac4a 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -89,7 +89,7 @@
 
 #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)
+#define PIN_CFG_MASK			GENMASK_ULL(31, 0)
 
 /*
  * m indicates the bitmap of supported pins, a is the register index
@@ -1187,7 +1187,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
 	u64 *pin_data = pin->drv_data;
 	unsigned int arg = 0;
 	u32 off;
-	u64 cfg;
+	u32 cfg;
 	int ret;
 	u8 bit;
 
@@ -1322,7 +1322,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
 	u64 *pin_data = pin->drv_data;
 	unsigned int i, arg, index;
 	u32 off, param;
-	u64 cfg;
+	u32 cfg;
 	int ret;
 	u8 bit;
 
@@ -2755,9 +2755,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;
-		u64 cfg, caps;
+		u32 off, caps;
 		u8 pincnt;
-		u32 off;
+		u64 cfg;
 
 		cfg = pctrl->data->port_pin_configs[port];
 		off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg);
@@ -2801,7 +2801,7 @@ 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 caps;
 	u32 i;
 
 	/*
-- 
2.34.1

[PATCH 2/4] pinctrl: renesas: rzg2l: Adjust bit masks for PIN_CFG_VARIABLE to use BIT(62)

[Prabhakar]

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

Shift the bit masks for `PIN_CFG_PIN_MAP_MASK` and `PIN_CFG_PIN_REG_MASK`,
to accommodate `PIN_CFG_VARIABLE` using `BIT(62)`.

Previously, these bit masks were placed higher up in the bit range, which
did not leave room for `PIN_CFG_VARIABLE` at `BIT(62)`. By adjusting these
masks, we ensure that `PIN_CFG_VARIABLE` can occupy `BIT(62)` without any
conflicts. The updated masks are now:
- `PIN_CFG_PIN_MAP_MASK`: `GENMASK_ULL(61, 54)` (was `GENMASK_ULL(62, 55)`)
- `PIN_CFG_PIN_REG_MASK`: `GENMASK_ULL(53, 46)` (was `GENMASK_ULL(54, 47)`)

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

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index bfaeeb00ac4a..b79dd1ea2616 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -87,8 +87,8 @@
 					 PIN_CFG_FILNUM | \
 					 PIN_CFG_FILCLKSEL)
 
-#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(62, 55)
-#define PIN_CFG_PIN_REG_MASK		GENMASK_ULL(54, 47)
+#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(61, 54)
+#define PIN_CFG_PIN_REG_MASK		GENMASK_ULL(53, 46)
 #define PIN_CFG_MASK			GENMASK_ULL(31, 0)
 
 /*
-- 
2.34.1

[PATCH 3/4] pinctrl: renesas: rzg2l: Move RZG2L_SINGLE_PIN definition to top of the file

[Prabhakar]

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

Define `RZG2L_SINGLE_PIN` at the top of the file to clarify its use for
dedicated pins for improved readability.

While at it update the comment for `RZG2L_SINGLE_PIN_PACK` macro and place
it just above the macro for clarity.

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

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index b79dd1ea2616..37a99d33400d 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -64,6 +64,8 @@
 #define PIN_CFG_ELC			BIT(20)
 #define PIN_CFG_IOLH_RZV2H		BIT(21)
 
+#define RZG2L_SINGLE_PIN		BIT_ULL(63)	/* Dedicated pin */
+
 #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
 					(PIN_CFG_IOLH_##group | \
 					 PIN_CFG_PUPD | \
@@ -105,15 +107,13 @@
  */
 #define RZG2L_GPIO_PORT_PACK(n, a, f)	RZG2L_GPIO_PORT_SPARSE_PACK((1ULL << (n)) - 1, (a), (f))
 
-/*
- * BIT(63) indicates dedicated pin, p is the register index while
- * referencing to SR/IEN/IOLH/FILxx registers, b is the register bits
- * (b * 8) and f is the pin configuration capabilities supported.
- */
-#define RZG2L_SINGLE_PIN		BIT_ULL(63)
 #define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK_ULL(62, 56)
 #define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK_ULL(55, 53)
-
+/*
+ * p is the register index while referencing to SR/IEN/IOLH/FILxx
+ * registers, b is the register bits (b * 8) and f is the pin
+ * configuration capabilities supported.
+ */
 #define RZG2L_SINGLE_PIN_PACK(p, b, f)	(RZG2L_SINGLE_PIN | \
 					 FIELD_PREP_CONST(RZG2L_SINGLE_PIN_INDEX_MASK, (p)) | \
 					 FIELD_PREP_CONST(RZG2L_SINGLE_PIN_BITS_MASK, (b)) | \
-- 
2.34.1

[PATCH 4/4] pinctrl: renesas: rzg2l: Reorganize variable configuration macro

[Prabhakar]

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

The `PIN_CFG_VARIABLE` macro did not indicate the capabilities of a pin
but served as a flag indicating that the pins of a port have different
capabilities.

To better reflect its purpose, move the `PIN_CFG_VARIABLE` macro beside
`RZG2L_SINGLE_PIN` and rename it to `RZG2L_CFG_VARIABLE`. Additionally,
introduce new macros for packing variable port configurations:

- `RZG2L_GPIO_PORT_PACK_VARIABLE(n, a)`: Combines `RZG2L_CFG_VARIABLE`
  with `RZG2L_GPIO_PORT_PACK` to handle variable pin configurations
  for a packed port.
- `RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(m, a)`: Combines
  `RZG2L_CFG_VARIABLE` with `RZG2L_GPIO_PORT_SPARSE_PACK` to handle
  variable pin configurations for a sparse port.

Due to the above change the configuration macros have been reorganized
as follows:
- Shift the bit positions of `PIN_CFG_NOGPIO_INT`, `PIN_CFG_NOD`,
  `PIN_CFG_SMT`, `PIN_CFG_ELC`, and `PIN_CFG_IOLH_RZV2H` down by one
  to accommodate the removal of `PIN_CFG_VARIABLE`.

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

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 37a99d33400d..9a67de960470 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -57,14 +57,14 @@
 #define PIN_CFG_IOLH_C			BIT(13)
 #define PIN_CFG_SOFT_PS			BIT(14)
 #define PIN_CFG_OEN			BIT(15)
-#define PIN_CFG_VARIABLE		BIT(16)
-#define PIN_CFG_NOGPIO_INT		BIT(17)
-#define PIN_CFG_NOD			BIT(18)	/* N-ch Open Drain */
-#define PIN_CFG_SMT			BIT(19)	/* Schmitt-trigger input control */
-#define PIN_CFG_ELC			BIT(20)
-#define PIN_CFG_IOLH_RZV2H		BIT(21)
+#define PIN_CFG_NOGPIO_INT		BIT(16)
+#define PIN_CFG_NOD			BIT(17)	/* N-ch Open Drain */
+#define PIN_CFG_SMT			BIT(18)	/* Schmitt-trigger input control */
+#define PIN_CFG_ELC			BIT(19)
+#define PIN_CFG_IOLH_RZV2H		BIT(20)
 
 #define RZG2L_SINGLE_PIN		BIT_ULL(63)	/* Dedicated pin */
+#define RZG2L_CFG_VARIABLE		BIT_ULL(62)	/* Variable cfg for port pins */
 
 #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
 					(PIN_CFG_IOLH_##group | \
@@ -100,12 +100,17 @@
 #define RZG2L_GPIO_PORT_SPARSE_PACK(m, a, f)	(FIELD_PREP_CONST(PIN_CFG_PIN_MAP_MASK, (m)) | \
 						 FIELD_PREP_CONST(PIN_CFG_PIN_REG_MASK, (a)) | \
 						 FIELD_PREP_CONST(PIN_CFG_MASK, (f)))
+#define RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(m, a)	\
+						(RZG2L_CFG_VARIABLE | \
+						 RZG2L_GPIO_PORT_SPARSE_PACK(m, a, 0))
 
 /*
  * n indicates number of pins in the port, a is the register index
  * and f is pin configuration capabilities supported.
  */
 #define RZG2L_GPIO_PORT_PACK(n, a, f)	RZG2L_GPIO_PORT_SPARSE_PACK((1ULL << (n)) - 1, (a), (f))
+#define RZG2L_GPIO_PORT_PACK_VARIABLE(n, a)	(RZG2L_CFG_VARIABLE | \
+						 RZG2L_GPIO_PORT_PACK(n, a, 0))
 
 #define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK_ULL(62, 56)
 #define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK_ULL(55, 53)
@@ -371,7 +376,7 @@ static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
 
 		if (FIELD_GET(VARIABLE_PIN_CFG_PORT_MASK, cfg) == port &&
 		    FIELD_GET(VARIABLE_PIN_CFG_PIN_MASK, cfg) == pin)
-			return (pincfg & ~PIN_CFG_VARIABLE) | FIELD_GET(PIN_CFG_MASK, cfg);
+			return (pincfg & ~RZG2L_CFG_VARIABLE) | FIELD_GET(PIN_CFG_MASK, cfg);
 	}
 
 	return 0;
@@ -1835,13 +1840,13 @@ static const u64 r9a07g043_gpio_configs[] = {
 	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x06, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
 				    PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
 				    PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),			/* P19 */
-	RZG2L_GPIO_PORT_PACK(8, 0x07, PIN_CFG_VARIABLE),				/* P20 */
+	RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x07),						/* P20 */
 	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x08, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
 				    PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),			/* P21 */
 	RZG2L_GPIO_PORT_PACK(4, 0x09, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
 			     PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),				/* P22 */
-	RZG2L_GPIO_PORT_SPARSE_PACK(0x3e, 0x0a, PIN_CFG_VARIABLE),			/* P23 */
-	RZG2L_GPIO_PORT_PACK(6, 0x0b, PIN_CFG_VARIABLE),				/* P24 */
+	RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(0x3e, 0x0a),				/* P23 */
+	RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x0b),						/* P24 */
 	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x0c, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_FILONOFF |
 				    PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
 				    PIN_CFG_NOGPIO_INT),				/* P25 */
@@ -1913,7 +1918,7 @@ static const u64 r9a09g057_gpio_configs[] = {
 				      PIN_CFG_ELC),		/* P8 */
 	RZG2L_GPIO_PORT_PACK(8, 0x29, RZV2H_MPXED_PIN_FUNCS),	/* P9 */
 	RZG2L_GPIO_PORT_PACK(8, 0x2a, RZV2H_MPXED_PIN_FUNCS),	/* PA */
-	RZG2L_GPIO_PORT_PACK(6, 0x2b, PIN_CFG_VARIABLE),	/* PB */
+	RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x2b),			/* PB */
 };
 
 static const struct {
@@ -2637,7 +2642,7 @@ 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];
-		if (pin_data[i] & PIN_CFG_VARIABLE)
+		if (pin_data[i] & RZG2L_CFG_VARIABLE)
 			pin_data[i] = rzg2l_pinctrl_get_variable_pin_cfg(pctrl,
 									 pin_data[i],
 									 j,
-- 
2.34.1

Re: [PATCH 1/4] pinctrl: renesas: rzg2l: Update PIN_CFG_MASK() macro to be 32-bit wide

[Geert Uytterhoeven]

On Tue, Jun 18, 2024 at 7:48 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Modify the `PIN_CFG_MASK()` macro to be 32-bit wide. The current maximum
> value for `PIN_CFG_*` is `BIT(21)`, which fits within a 32-bit mask.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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 2/4] pinctrl: renesas: rzg2l: Adjust bit masks for PIN_CFG_VARIABLE to use BIT(62)

[Geert Uytterhoeven]

On Tue, Jun 18, 2024 at 7:48 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Shift the bit masks for `PIN_CFG_PIN_MAP_MASK` and `PIN_CFG_PIN_REG_MASK`,
> to accommodate `PIN_CFG_VARIABLE` using `BIT(62)`.
>
> Previously, these bit masks were placed higher up in the bit range, which
> did not leave room for `PIN_CFG_VARIABLE` at `BIT(62)`. By adjusting these
> masks, we ensure that `PIN_CFG_VARIABLE` can occupy `BIT(62)` without any
> conflicts. The updated masks are now:
> - `PIN_CFG_PIN_MAP_MASK`: `GENMASK_ULL(61, 54)` (was `GENMASK_ULL(62, 55)`)
> - `PIN_CFG_PIN_REG_MASK`: `GENMASK_ULL(53, 46)` (was `GENMASK_ULL(54, 47)`)
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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 3/4] pinctrl: renesas: rzg2l: Move RZG2L_SINGLE_PIN definition to top of the file

[Geert Uytterhoeven]

On Tue, Jun 18, 2024 at 7:48 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Define `RZG2L_SINGLE_PIN` at the top of the file to clarify its use for
> dedicated pins for improved readability.
>
> While at it update the comment for `RZG2L_SINGLE_PIN_PACK` macro and place
> it just above the macro for clarity.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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 4/4] pinctrl: renesas: rzg2l: Reorganize variable configuration macro

[Geert Uytterhoeven]

Hi Prabhakar,

Thanks for your patch!

On Tue, Jun 18, 2024 at 7:48 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The `PIN_CFG_VARIABLE` macro did not indicate the capabilities of a pin
> but served as a flag indicating that the pins of a port have different
> capabilities.
>
> To better reflect its purpose, move the `PIN_CFG_VARIABLE` macro beside
> `RZG2L_SINGLE_PIN` and rename it to `RZG2L_CFG_VARIABLE`. Additionally,

Do you mind me renaming it to RZG2L_VARIABLE_CFG while applying?

> introduce new macros for packing variable port configurations:
>
> - `RZG2L_GPIO_PORT_PACK_VARIABLE(n, a)`: Combines `RZG2L_CFG_VARIABLE`
>   with `RZG2L_GPIO_PORT_PACK` to handle variable pin configurations
>   for a packed port.
> - `RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(m, a)`: Combines
>   `RZG2L_CFG_VARIABLE` with `RZG2L_GPIO_PORT_SPARSE_PACK` to handle
>   variable pin configurations for a sparse port.
>
> Due to the above change the configuration macros have been reorganized
> as follows:
> - Shift the bit positions of `PIN_CFG_NOGPIO_INT`, `PIN_CFG_NOD`,
>   `PIN_CFG_SMT`, `PIN_CFG_ELC`, and `PIN_CFG_IOLH_RZV2H` down by one
>   to accommodate the removal of `PIN_CFG_VARIABLE`.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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 4/4] pinctrl: renesas: rzg2l: Reorganize variable configuration macro

[Lad, Prabhakar]

Hi Geert,

On Fri, Jun 21, 2024 at 1:17 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Tue, Jun 18, 2024 at 7:48 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The `PIN_CFG_VARIABLE` macro did not indicate the capabilities of a pin
> > but served as a flag indicating that the pins of a port have different
> > capabilities.
> >
> > To better reflect its purpose, move the `PIN_CFG_VARIABLE` macro beside
> > `RZG2L_SINGLE_PIN` and rename it to `RZG2L_CFG_VARIABLE`. Additionally,
>
> Do you mind me renaming it to RZG2L_VARIABLE_CFG while applying?
>
Fine by me.

Cheers,
Prabhakar

Re: [PATCH 1/4] pinctrl: renesas: rzg2l: Update PIN_CFG_MASK() macro to be 32-bit wide

[claudiu beznea]

On 18.06.2024 20:48, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Modify the `PIN_CFG_MASK()` macro to be 32-bit wide. The current maximum
> value for `PIN_CFG_*` is `BIT(21)`, which fits within a 32-bit mask.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 32945d4c8dc0..bfaeeb00ac4a 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -89,7 +89,7 @@
>  
>  #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)
> +#define PIN_CFG_MASK			GENMASK_ULL(31, 0)
>  
>  /*
>   * m indicates the bitmap of supported pins, a is the register index
> @@ -1187,7 +1187,7 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
>  	u64 *pin_data = pin->drv_data;
>  	unsigned int arg = 0;
>  	u32 off;
> -	u64 cfg;
> +	u32 cfg;
>  	int ret;
>  	u8 bit;
>  
> @@ -1322,7 +1322,7 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
>  	u64 *pin_data = pin->drv_data;
>  	unsigned int i, arg, index;
>  	u32 off, param;
> -	u64 cfg;
> +	u32 cfg;
>  	int ret;
>  	u8 bit;
>  
> @@ -2755,9 +2755,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;
> -		u64 cfg, caps;
> +		u32 off, caps;
>  		u8 pincnt;
> -		u32 off;
> +		u64 cfg;
>  
>  		cfg = pctrl->data->port_pin_configs[port];
>  		off = RZG2L_PIN_CFG_TO_PORT_OFFSET(cfg);
> @@ -2801,7 +2801,7 @@ 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 caps;
>  	u32 i;
>  
>  	/*

Re: [PATCH 2/4] pinctrl: renesas: rzg2l: Adjust bit masks for PIN_CFG_VARIABLE to use BIT(62)

[claudiu beznea]

On 18.06.2024 20:48, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Shift the bit masks for `PIN_CFG_PIN_MAP_MASK` and `PIN_CFG_PIN_REG_MASK`,
> to accommodate `PIN_CFG_VARIABLE` using `BIT(62)`.
> 
> Previously, these bit masks were placed higher up in the bit range, which
> did not leave room for `PIN_CFG_VARIABLE` at `BIT(62)`. By adjusting these
> masks, we ensure that `PIN_CFG_VARIABLE` can occupy `BIT(62)` without any
> conflicts. The updated masks are now:
> - `PIN_CFG_PIN_MAP_MASK`: `GENMASK_ULL(61, 54)` (was `GENMASK_ULL(62, 55)`)
> - `PIN_CFG_PIN_REG_MASK`: `GENMASK_ULL(53, 46)` (was `GENMASK_ULL(54, 47)`)
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index bfaeeb00ac4a..b79dd1ea2616 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -87,8 +87,8 @@
>  					 PIN_CFG_FILNUM | \
>  					 PIN_CFG_FILCLKSEL)
>  
> -#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(62, 55)
> -#define PIN_CFG_PIN_REG_MASK		GENMASK_ULL(54, 47)
> +#define PIN_CFG_PIN_MAP_MASK		GENMASK_ULL(61, 54)
> +#define PIN_CFG_PIN_REG_MASK		GENMASK_ULL(53, 46)
>  #define PIN_CFG_MASK			GENMASK_ULL(31, 0)
>  
>  /*

Re: [PATCH 3/4] pinctrl: renesas: rzg2l: Move RZG2L_SINGLE_PIN definition to top of the file

[claudiu beznea]

On 18.06.2024 20:48, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Define `RZG2L_SINGLE_PIN` at the top of the file to clarify its use for
> dedicated pins for improved readability.
> 
> While at it update the comment for `RZG2L_SINGLE_PIN_PACK` macro and place
> it just above the macro for clarity.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>


> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index b79dd1ea2616..37a99d33400d 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -64,6 +64,8 @@
>  #define PIN_CFG_ELC			BIT(20)
>  #define PIN_CFG_IOLH_RZV2H		BIT(21)
>  
> +#define RZG2L_SINGLE_PIN		BIT_ULL(63)	/* Dedicated pin */
> +
>  #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
>  					(PIN_CFG_IOLH_##group | \
>  					 PIN_CFG_PUPD | \
> @@ -105,15 +107,13 @@
>   */
>  #define RZG2L_GPIO_PORT_PACK(n, a, f)	RZG2L_GPIO_PORT_SPARSE_PACK((1ULL << (n)) - 1, (a), (f))
>  
> -/*
> - * BIT(63) indicates dedicated pin, p is the register index while
> - * referencing to SR/IEN/IOLH/FILxx registers, b is the register bits
> - * (b * 8) and f is the pin configuration capabilities supported.
> - */
> -#define RZG2L_SINGLE_PIN		BIT_ULL(63)
>  #define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK_ULL(62, 56)
>  #define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK_ULL(55, 53)
> -
> +/*
> + * p is the register index while referencing to SR/IEN/IOLH/FILxx
> + * registers, b is the register bits (b * 8) and f is the pin
> + * configuration capabilities supported.
> + */
>  #define RZG2L_SINGLE_PIN_PACK(p, b, f)	(RZG2L_SINGLE_PIN | \
>  					 FIELD_PREP_CONST(RZG2L_SINGLE_PIN_INDEX_MASK, (p)) | \
>  					 FIELD_PREP_CONST(RZG2L_SINGLE_PIN_BITS_MASK, (b)) | \

Re: [PATCH 4/4] pinctrl: renesas: rzg2l: Reorganize variable configuration macro

[claudiu beznea]

On 18.06.2024 20:48, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The `PIN_CFG_VARIABLE` macro did not indicate the capabilities of a pin
> but served as a flag indicating that the pins of a port have different
> capabilities.
> 
> To better reflect its purpose, move the `PIN_CFG_VARIABLE` macro beside
> `RZG2L_SINGLE_PIN` and rename it to `RZG2L_CFG_VARIABLE`. Additionally,
> introduce new macros for packing variable port configurations:
> 
> - `RZG2L_GPIO_PORT_PACK_VARIABLE(n, a)`: Combines `RZG2L_CFG_VARIABLE`
>   with `RZG2L_GPIO_PORT_PACK` to handle variable pin configurations
>   for a packed port.
> - `RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(m, a)`: Combines
>   `RZG2L_CFG_VARIABLE` with `RZG2L_GPIO_PORT_SPARSE_PACK` to handle
>   variable pin configurations for a sparse port.
> 
> Due to the above change the configuration macros have been reorganized
> as follows:
> - Shift the bit positions of `PIN_CFG_NOGPIO_INT`, `PIN_CFG_NOD`,
>   `PIN_CFG_SMT`, `PIN_CFG_ELC`, and `PIN_CFG_IOLH_RZV2H` down by one
>   to accommodate the removal of `PIN_CFG_VARIABLE`.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 29 +++++++++++++++----------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index 37a99d33400d..9a67de960470 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -57,14 +57,14 @@
>  #define PIN_CFG_IOLH_C			BIT(13)
>  #define PIN_CFG_SOFT_PS			BIT(14)
>  #define PIN_CFG_OEN			BIT(15)
> -#define PIN_CFG_VARIABLE		BIT(16)
> -#define PIN_CFG_NOGPIO_INT		BIT(17)
> -#define PIN_CFG_NOD			BIT(18)	/* N-ch Open Drain */
> -#define PIN_CFG_SMT			BIT(19)	/* Schmitt-trigger input control */
> -#define PIN_CFG_ELC			BIT(20)
> -#define PIN_CFG_IOLH_RZV2H		BIT(21)
> +#define PIN_CFG_NOGPIO_INT		BIT(16)
> +#define PIN_CFG_NOD			BIT(17)	/* N-ch Open Drain */
> +#define PIN_CFG_SMT			BIT(18)	/* Schmitt-trigger input control */
> +#define PIN_CFG_ELC			BIT(19)
> +#define PIN_CFG_IOLH_RZV2H		BIT(20)
>  
>  #define RZG2L_SINGLE_PIN		BIT_ULL(63)	/* Dedicated pin */
> +#define RZG2L_CFG_VARIABLE		BIT_ULL(62)	/* Variable cfg for port pins */
>  
>  #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
>  					(PIN_CFG_IOLH_##group | \
> @@ -100,12 +100,17 @@
>  #define RZG2L_GPIO_PORT_SPARSE_PACK(m, a, f)	(FIELD_PREP_CONST(PIN_CFG_PIN_MAP_MASK, (m)) | \
>  						 FIELD_PREP_CONST(PIN_CFG_PIN_REG_MASK, (a)) | \
>  						 FIELD_PREP_CONST(PIN_CFG_MASK, (f)))
> +#define RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(m, a)	\
> +						(RZG2L_CFG_VARIABLE | \
> +						 RZG2L_GPIO_PORT_SPARSE_PACK(m, a, 0))
>  
>  /*
>   * n indicates number of pins in the port, a is the register index
>   * and f is pin configuration capabilities supported.
>   */
>  #define RZG2L_GPIO_PORT_PACK(n, a, f)	RZG2L_GPIO_PORT_SPARSE_PACK((1ULL << (n)) - 1, (a), (f))
> +#define RZG2L_GPIO_PORT_PACK_VARIABLE(n, a)	(RZG2L_CFG_VARIABLE | \
> +						 RZG2L_GPIO_PORT_PACK(n, a, 0))
>  
>  #define RZG2L_SINGLE_PIN_INDEX_MASK	GENMASK_ULL(62, 56)
>  #define RZG2L_SINGLE_PIN_BITS_MASK	GENMASK_ULL(55, 53)
> @@ -371,7 +376,7 @@ static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
>  
>  		if (FIELD_GET(VARIABLE_PIN_CFG_PORT_MASK, cfg) == port &&
>  		    FIELD_GET(VARIABLE_PIN_CFG_PIN_MASK, cfg) == pin)
> -			return (pincfg & ~PIN_CFG_VARIABLE) | FIELD_GET(PIN_CFG_MASK, cfg);
> +			return (pincfg & ~RZG2L_CFG_VARIABLE) | FIELD_GET(PIN_CFG_MASK, cfg);
>  	}
>  
>  	return 0;
> @@ -1835,13 +1840,13 @@ static const u64 r9a07g043_gpio_configs[] = {
>  	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x06, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
>  				    PIN_CFG_FILONOFF | PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
>  				    PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),			/* P19 */
> -	RZG2L_GPIO_PORT_PACK(8, 0x07, PIN_CFG_VARIABLE),				/* P20 */
> +	RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x07),						/* P20 */
>  	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x08, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
>  				    PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),			/* P21 */
>  	RZG2L_GPIO_PORT_PACK(4, 0x09, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_PUPD |
>  			     PIN_CFG_IEN | PIN_CFG_NOGPIO_INT),				/* P22 */
> -	RZG2L_GPIO_PORT_SPARSE_PACK(0x3e, 0x0a, PIN_CFG_VARIABLE),			/* P23 */
> -	RZG2L_GPIO_PORT_PACK(6, 0x0b, PIN_CFG_VARIABLE),				/* P24 */
> +	RZG2L_GPIO_PORT_SPARSE_PACK_VARIABLE(0x3e, 0x0a),				/* P23 */
> +	RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x0b),						/* P24 */
>  	RZG2L_GPIO_PORT_SPARSE_PACK(0x2, 0x0c, PIN_CFG_IOLH_B | PIN_CFG_SR | PIN_CFG_FILONOFF |
>  				    PIN_CFG_FILNUM | PIN_CFG_FILCLKSEL |
>  				    PIN_CFG_NOGPIO_INT),				/* P25 */
> @@ -1913,7 +1918,7 @@ static const u64 r9a09g057_gpio_configs[] = {
>  				      PIN_CFG_ELC),		/* P8 */
>  	RZG2L_GPIO_PORT_PACK(8, 0x29, RZV2H_MPXED_PIN_FUNCS),	/* P9 */
>  	RZG2L_GPIO_PORT_PACK(8, 0x2a, RZV2H_MPXED_PIN_FUNCS),	/* PA */
> -	RZG2L_GPIO_PORT_PACK(6, 0x2b, PIN_CFG_VARIABLE),	/* PB */
> +	RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x2b),			/* PB */
>  };
>  
>  static const struct {
> @@ -2637,7 +2642,7 @@ 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];
> -		if (pin_data[i] & PIN_CFG_VARIABLE)
> +		if (pin_data[i] & RZG2L_CFG_VARIABLE)
>  			pin_data[i] = rzg2l_pinctrl_get_variable_pin_cfg(pctrl,
>  									 pin_data[i],
>  									 j,