[PATCH v2 0/5] pinctrl: renesas: basic R8A779A0 (V3U) support

[PATCH v2 0/5] pinctrl: renesas: basic R8A779A0 (V3U) support

[Ulrich Hecht]

Hi!

This series provides basic V3U pin control support, up to and including the
SCIF pins.

This revision includes fixes for numerous issues found by Geert in his
review; see below for details.

CU
Uli


Changes since v1:
- add support for different voltage levels
- add more PORT_GP_CFG_{2,31} macros
- add non-GP pins
- add A/B pins/groups for TCLK{1,2}, {RX,TX}1, FXR_TXDA, RXDA_EXTFXR
- add SEL_I2C*_0 to MOD_SEL2
- add PINMUX_PHYS, fix multiplexing of S{DA,CL}[0-6]
- add AVB{0,1}_{MAGIC,MDC,MDIO,TXREFCLK}
- remove undocumented POC3
- add human-readable pin names to pinmux_bias_regs[]
- use generic rcar_pinmux_{get,set}_bias() ops
- tweak coding style and commit messages
- add Reviewed-Bys where applicable


Ulrich Hecht (5):
  pinctrl: renesas: implement unlock register masks
  pinctrl: renesas: add I/O voltage level flag
  pinctrl: renesas: add PORT_GP_CFG_{2,31} macros
  pinctrl: renesas: Initial R8A779A0 (V3U) PFC support
  pinctrl: renesas: r8a779a0: Add SCIF pins, groups and functions

 drivers/pinctrl/renesas/Kconfig        |    5 +
 drivers/pinctrl/renesas/Makefile       |    1 +
 drivers/pinctrl/renesas/core.c         |   34 +-
 drivers/pinctrl/renesas/pfc-r8a779a0.c | 2683 ++++++++++++++++++++++++
 drivers/pinctrl/renesas/pinctrl.c      |   16 +-
 drivers/pinctrl/renesas/sh_pfc.h       |   23 +-
 6 files changed, 2748 insertions(+), 14 deletions(-)
 create mode 100644 drivers/pinctrl/renesas/pfc-r8a779a0.c

-- 
2.20.1

[PATCH v2 1/5] pinctrl: renesas: implement unlock register masks

[Ulrich Hecht]

The V3U SoC has several unlock registers, one per register group. They
reside at offset zero in each 0x200 bytes-sized block.

To avoid adding yet another table to the PFC implementation, this
patch adds the option to specify an address mask instead of the fixed
address in sh_pfc_soc_info::unlock_reg.

Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/pinctrl/renesas/core.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c
index 2cc457279345..4cd95e220900 100644
--- a/drivers/pinctrl/renesas/core.c
+++ b/drivers/pinctrl/renesas/core.c
@@ -175,13 +175,25 @@ u32 sh_pfc_read(struct sh_pfc *pfc, u32 reg)
 	return sh_pfc_read_raw_reg(sh_pfc_phys_to_virt(pfc, reg), 32);
 }
 
-void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data)
+static void sh_pfc_unlock_reg(struct sh_pfc *pfc, u32 reg, u32 data)
 {
-	if (pfc->info->unlock_reg)
-		sh_pfc_write_raw_reg(
-			sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg), 32,
-			~data);
+	u32 unlock;
+
+	if (!pfc->info->unlock_reg)
+		return;
 
+	if (pfc->info->unlock_reg >= 0x80000000UL)
+		unlock = pfc->info->unlock_reg;
+	else
+		/* unlock_reg is a mask */
+		unlock = reg & ~pfc->info->unlock_reg;
+
+	sh_pfc_write_raw_reg(sh_pfc_phys_to_virt(pfc, unlock), 32, ~data);
+}
+
+void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data)
+{
+	sh_pfc_unlock_reg(pfc, reg, data);
 	sh_pfc_write_raw_reg(sh_pfc_phys_to_virt(pfc, reg), 32, data);
 }
 
@@ -227,11 +239,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
 	data &= mask;
 	data |= value;
 
-	if (pfc->info->unlock_reg)
-		sh_pfc_write_raw_reg(
-			sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg), 32,
-			~data);
-
+	sh_pfc_unlock_reg(pfc, crp->reg, data);
 	sh_pfc_write_raw_reg(mapped_reg, crp->reg_width, data);
 }
 
-- 
2.20.1

[PATCH v2 2/5] pinctrl: renesas: add I/O voltage level flag

[Ulrich Hecht]

This patch adds config macros describing the voltage levels available on
a pin. The current default (3.3V/1.8V) maps to zero to avoid having to
change existing PFC implementations.

Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/pinctrl/renesas/pinctrl.c | 16 ++++++++++++++--
 drivers/pinctrl/renesas/sh_pfc.h  |  9 +++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl.c b/drivers/pinctrl/renesas/pinctrl.c
index ac542d278a38..85a182191d7d 100644
--- a/drivers/pinctrl/renesas/pinctrl.c
+++ b/drivers/pinctrl/renesas/pinctrl.c
@@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
 	}
 
 	case PIN_CONFIG_POWER_SOURCE: {
+		int idx = sh_pfc_get_pin_index(pfc, _pin);
+		const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
+		int lower_voltage;
 		u32 pocctrl, val;
 		int bit;
 
@@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
 		val = sh_pfc_read(pfc, pocctrl);
 		spin_unlock_irqrestore(&pfc->lock, flags);
 
-		arg = (val & BIT(bit)) ? 3300 : 1800;
+		lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ?
+			2500 : 1800;
+
+		arg = (val & BIT(bit)) ? 3300 : lower_voltage;
 		break;
 	}
 
@@ -702,6 +708,9 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin,
 
 		case PIN_CONFIG_POWER_SOURCE: {
 			unsigned int mV = pinconf_to_config_argument(configs[i]);
+			int idx = sh_pfc_get_pin_index(pfc, _pin);
+			const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
+			int lower_voltage;
 			u32 pocctrl, val;
 			int bit;
 
@@ -712,7 +721,10 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin,
 			if (WARN(bit < 0, "invalid pin %#x", _pin))
 				return bit;
 
-			if (mV != 1800 && mV != 3300)
+			lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ?
+				2500 : 1800;
+
+			if (mV != lower_voltage && mV != 3300)
 				return -EINVAL;
 
 			spin_lock_irqsave(&pfc->lock, flags);
diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h
index dc484c13f59c..00bfda90a7b7 100644
--- a/drivers/pinctrl/renesas/sh_pfc.h
+++ b/drivers/pinctrl/renesas/sh_pfc.h
@@ -31,6 +31,15 @@ enum {
 					 SH_PFC_PIN_CFG_PULL_DOWN)
 #define SH_PFC_PIN_CFG_IO_VOLTAGE	(1 << 4)
 #define SH_PFC_PIN_CFG_DRIVE_STRENGTH	(1 << 5)
+
+#define SH_PFC_PIN_VOLTAGE_18_33	(0 << 6)
+#define SH_PFC_PIN_VOLTAGE_25_33	(1 << 6)
+
+#define SH_PFC_PIN_CFG_IO_VOLTAGE_18_33	(SH_PFC_PIN_CFG_IO_VOLTAGE | \
+					 SH_PFC_PIN_VOLTAGE_18_33)
+#define SH_PFC_PIN_CFG_IO_VOLTAGE_25_33	(SH_PFC_PIN_CFG_IO_VOLTAGE | \
+					 SH_PFC_PIN_VOLTAGE_25_33)
+
 #define SH_PFC_PIN_CFG_NO_GPIO		(1 << 31)
 
 struct sh_pfc_pin {
-- 
2.20.1

[PATCH v2 3/5] pinctrl: renesas: add PORT_GP_CFG_{2,31} macros

[Ulrich Hecht]

Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/pinctrl/renesas/sh_pfc.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h
index 00bfda90a7b7..747dfdb331bd 100644
--- a/drivers/pinctrl/renesas/sh_pfc.h
+++ b/drivers/pinctrl/renesas/sh_pfc.h
@@ -460,6 +460,11 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
 	fn(bank, pin, GP_##bank##_##pin, sfx, cfg)
 #define PORT_GP_1(bank, pin, fn, sfx)	PORT_GP_CFG_1(bank, pin, fn, sfx, 0)
 
+#define PORT_GP_CFG_2(bank, fn, sfx, cfg)				\
+	PORT_GP_CFG_1(bank, 0,  fn, sfx, cfg),				\
+	PORT_GP_CFG_1(bank, 1,  fn, sfx, cfg)
+#define PORT_GP_2(bank, fn, sfx)	PORT_GP_CFG_2(bank, fn, sfx, 0)
+
 #define PORT_GP_CFG_4(bank, fn, sfx, cfg)				\
 	PORT_GP_CFG_1(bank, 0,  fn, sfx, cfg),				\
 	PORT_GP_CFG_1(bank, 1,  fn, sfx, cfg),				\
@@ -581,9 +586,13 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
 	PORT_GP_CFG_1(bank, 29, fn, sfx, cfg)
 #define PORT_GP_30(bank, fn, sfx)	PORT_GP_CFG_30(bank, fn, sfx, 0)
 
-#define PORT_GP_CFG_32(bank, fn, sfx, cfg)				\
+#define PORT_GP_CFG_31(bank, fn, sfx, cfg)				\
 	PORT_GP_CFG_30(bank, fn, sfx, cfg),				\
-	PORT_GP_CFG_1(bank, 30, fn, sfx, cfg),				\
+	PORT_GP_CFG_1(bank, 30, fn, sfx, cfg)
+#define PORT_GP_31(bank, fn, sfx)	PORT_GP_CFG_31(bank, fn, sfx, 0)
+
+#define PORT_GP_CFG_32(bank, fn, sfx, cfg)				\
+	PORT_GP_CFG_31(bank, fn, sfx, cfg),				\
 	PORT_GP_CFG_1(bank, 31, fn, sfx, cfg)
 #define PORT_GP_32(bank, fn, sfx)	PORT_GP_CFG_32(bank, fn, sfx, 0)
 
-- 
2.20.1

[PATCH v2 5/5] pinctrl: renesas: r8a779a0: Add SCIF pins, groups and functions

[Ulrich Hecht]

This patch adds SCIF0, 1, 3 and 4 pins, groups and functions for the
R8A779A0 (V3U) SoC.

Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/pinctrl/renesas/pfc-r8a779a0.c | 156 +++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/drivers/pinctrl/renesas/pfc-r8a779a0.c b/drivers/pinctrl/renesas/pfc-r8a779a0.c
index 9e09d1db8b43..253387e47b55 100644
--- a/drivers/pinctrl/renesas/pfc-r8a779a0.c
+++ b/drivers/pinctrl/renesas/pfc-r8a779a0.c
@@ -1233,10 +1233,166 @@ static const struct sh_pfc_pin pinmux_pins[] = {
 	PINMUX_GPIO_GP_ALL(),
 };
 
+/* - SCIF0 ------------------------------------------------------------------ */
+static const unsigned int scif0_data_pins[] = {
+	/* RX0, TX0 */
+	RCAR_GP_PIN(1, 1), RCAR_GP_PIN(1, 5),
+};
+static const unsigned int scif0_data_mux[] = {
+	RX0_MARK, TX0_MARK,
+};
+static const unsigned int scif0_clk_pins[] = {
+	/* SCK0 */
+	RCAR_GP_PIN(1, 2),
+};
+static const unsigned int scif0_clk_mux[] = {
+	SCK0_MARK,
+};
+static const unsigned int scif0_ctrl_pins[] = {
+	/* RTS0#, CTS0# */
+	RCAR_GP_PIN(1, 3), RCAR_GP_PIN(1, 4),
+};
+static const unsigned int scif0_ctrl_mux[] = {
+	RTS0_N_MARK, CTS0_N_MARK,
+};
+
+/* - SCIF1 ------------------------------------------------------------------ */
+static const unsigned int scif1_data_a_pins[] = {
+	/* RX, TX */
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+};
+static const unsigned int scif1_data_a_mux[] = {
+	RX1_A_MARK, TX1_A_MARK,
+};
+static const unsigned int scif1_data_b_pins[] = {
+	/* RX, TX */
+	RCAR_GP_PIN(3, 2), RCAR_GP_PIN(3, 1),
+};
+static const unsigned int scif1_data_b_mux[] = {
+	RX1_B_MARK, TX1_B_MARK,
+};
+static const unsigned int scif1_clk_pins[] = {
+	/* SCK1 */
+	RCAR_GP_PIN(1, 18),
+};
+static const unsigned int scif1_clk_mux[] = {
+	SCK1_MARK,
+};
+static const unsigned int scif1_ctrl_pins[] = {
+	/* RTS1#, CTS1# */
+	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 19),
+};
+static const unsigned int scif1_ctrl_mux[] = {
+	RTS1_N_MARK, CTS1_N_MARK,
+};
+
+/* - SCIF3 ------------------------------------------------------------------ */
+static const unsigned int scif3_data_pins[] = {
+	/* RX3, TX3 */
+	RCAR_GP_PIN(1, 16), RCAR_GP_PIN(1, 17),
+};
+static const unsigned int scif3_data_mux[] = {
+	RX3_MARK, TX3_MARK,
+};
+static const unsigned int scif3_clk_pins[] = {
+	/* SCK3 */
+	RCAR_GP_PIN(1, 13),
+};
+static const unsigned int scif3_clk_mux[] = {
+	SCK3_MARK,
+};
+static const unsigned int scif3_ctrl_pins[] = {
+	/* RTS3#, CTS3# */
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14),
+};
+static const unsigned int scif3_ctrl_mux[] = {
+	RTS3_N_MARK, CTS3_N_MARK,
+};
+
+/* - SCIF4 ------------------------------------------------------------------ */
+static const unsigned int scif4_data_pins[] = {
+	/* RX4, TX4 */
+	RCAR_GP_PIN(2, 8), RCAR_GP_PIN(2, 9),
+};
+static const unsigned int scif4_data_mux[] = {
+	RX4_MARK, TX4_MARK,
+};
+static const unsigned int scif4_clk_pins[] = {
+	/* SCK4 */
+	RCAR_GP_PIN(2, 5),
+};
+static const unsigned int scif4_clk_mux[] = {
+	SCK4_MARK,
+};
+static const unsigned int scif4_ctrl_pins[] = {
+	/* RTS4#, CTS4# */
+	RCAR_GP_PIN(2, 7), RCAR_GP_PIN(2, 6),
+};
+static const unsigned int scif4_ctrl_mux[] = {
+	RTS4_N_MARK, CTS4_N_MARK,
+};
+
+/* - SCIF Clock ------------------------------------------------------------- */
+static const unsigned int scif_clk_pins[] = {
+	/* SCIF_CLK */
+	RCAR_GP_PIN(1, 0),
+};
+static const unsigned int scif_clk_mux[] = {
+	SCIF_CLK_MARK,
+};
+
 static const struct sh_pfc_pin_group pinmux_groups[] = {
+	SH_PFC_PIN_GROUP(scif0_data),
+	SH_PFC_PIN_GROUP(scif0_clk),
+	SH_PFC_PIN_GROUP(scif0_ctrl),
+	SH_PFC_PIN_GROUP(scif1_data_a),
+	SH_PFC_PIN_GROUP(scif1_data_b),
+	SH_PFC_PIN_GROUP(scif1_clk),
+	SH_PFC_PIN_GROUP(scif1_ctrl),
+	SH_PFC_PIN_GROUP(scif3_data),
+	SH_PFC_PIN_GROUP(scif3_clk),
+	SH_PFC_PIN_GROUP(scif3_ctrl),
+	SH_PFC_PIN_GROUP(scif4_data),
+	SH_PFC_PIN_GROUP(scif4_clk),
+	SH_PFC_PIN_GROUP(scif4_ctrl),
+	SH_PFC_PIN_GROUP(scif_clk),
+};
+
+static const char * const scif0_groups[] = {
+	"scif0_data",
+	"scif0_clk",
+	"scif0_ctrl",
+};
+
+static const char * const scif1_groups[] = {
+	"scif1_data_a",
+	"scif1_data_b",
+	"scif1_clk",
+	"scif1_ctrl",
+};
+
+static const char * const scif3_groups[] = {
+	"scif3_data",
+	"scif3_clk",
+	"scif3_ctrl",
+};
+
+static const char * const scif4_groups[] = {
+	"scif4_data",
+	"scif4_clk",
+	"scif4_ctrl",
+};
+
+static const char * const scif_clk_groups[] = {
+	"scif_clk",
 };
 
 static const struct sh_pfc_function pinmux_functions[] = {
+	SH_PFC_FUNCTION(scif0),
+	SH_PFC_FUNCTION(scif1),
+	SH_PFC_FUNCTION(scif3),
+	SH_PFC_FUNCTION(scif4),
+	SH_PFC_FUNCTION(scif_clk),
 };
 
 static const struct pinmux_cfg_reg pinmux_config_regs[] = {
-- 
2.20.1

Re: [PATCH v2 5/5] pinctrl: renesas: r8a779a0: Add SCIF pins, groups and functions

[Geert Uytterhoeven]

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> This patch adds SCIF0, 1, 3 and 4 pins, groups and functions for the
> R8A779A0 (V3U) SoC.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

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 1/5] pinctrl: renesas: implement unlock register masks

[Geert Uytterhoeven]

Hi Uli,

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> The V3U SoC has several unlock registers, one per register group. They
> reside at offset zero in each 0x200 bytes-sized block.
>
> To avoid adding yet another table to the PFC implementation, this
> patch adds the option to specify an address mask instead of the fixed
> address in sh_pfc_soc_info::unlock_reg.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

My comments for v1 are still valid:

Perhaps a comment should be added to sh_pfc_soc_info.unlock_reg,
to document this dual behavior?
Or should we just always use masking, as that seems to be suited
for all SoCs using unlock_reg?

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 3/5] pinctrl: renesas: add PORT_GP_CFG_{2,31} macros

[Geert Uytterhoeven]

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

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

One suggestion below...

> --- a/drivers/pinctrl/renesas/sh_pfc.h
> +++ b/drivers/pinctrl/renesas/sh_pfc.h
> @@ -460,6 +460,11 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
>         fn(bank, pin, GP_##bank##_##pin, sfx, cfg)
>  #define PORT_GP_1(bank, pin, fn, sfx)  PORT_GP_CFG_1(bank, pin, fn, sfx, 0)
>
> +#define PORT_GP_CFG_2(bank, fn, sfx, cfg)                              \
> +       PORT_GP_CFG_1(bank, 0,  fn, sfx, cfg),                          \
> +       PORT_GP_CFG_1(bank, 1,  fn, sfx, cfg)
> +#define PORT_GP_2(bank, fn, sfx)       PORT_GP_CFG_2(bank, fn, sfx, 0)
> +
>  #define PORT_GP_CFG_4(bank, fn, sfx, cfg)                              \
>         PORT_GP_CFG_1(bank, 0,  fn, sfx, cfg),                          \
>         PORT_GP_CFG_1(bank, 1,  fn, sfx, cfg),                          \

This can now start using PORT_GP_CFG_2().

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 2/5] pinctrl: renesas: add I/O voltage level flag

[Geert Uytterhoeven]

Hi Uli,

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> This patch adds config macros describing the voltage levels available on
> a pin. The current default (3.3V/1.8V) maps to zero to avoid having to
> change existing PFC implementations.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl.c
> +++ b/drivers/pinctrl/renesas/pinctrl.c
> @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
>         }
>
>         case PIN_CONFIG_POWER_SOURCE: {
> +               int idx = sh_pfc_get_pin_index(pfc, _pin);

I guess this cannot fail when we get here?

> +               const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
> +               int lower_voltage;

unsigned int mV_low?

>                 u32 pocctrl, val;
>                 int bit;
>
> @@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
>                 val = sh_pfc_read(pfc, pocctrl);
>                 spin_unlock_irqrestore(&pfc->lock, flags);
>
> -               arg = (val & BIT(bit)) ? 3300 : 1800;
> +               lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ?
> +                       2500 : 1800;
> +
> +               arg = (val & BIT(bit)) ? 3300 : lower_voltage;
>                 break;
>         }
>
> @@ -702,6 +708,9 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin,
>
>                 case PIN_CONFIG_POWER_SOURCE: {
>                         unsigned int mV = pinconf_to_config_argument(configs[i]);
> +                       int idx = sh_pfc_get_pin_index(pfc, _pin);
> +                       const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
> +                       int lower_voltage;

unsigned int mV_low?

>                         u32 pocctrl, val;
>                         int bit;
>

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 4/5] pinctrl: renesas: Initial R8A779A0 (V3U) PFC support

[Geert Uytterhoeven]

Hi Uli,

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> This patch adds initial pinctrl support for the R8A779A0 (V3U) SoC,
> including bias, drive strength and voltage control.
>
> Based on patch by LUU HOAI <hoai.luu.ub@renesas.com>.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pinctrl/renesas/pfc-r8a779a0.c
> @@ -0,0 +1,2527 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * R8A779A0 processor support - PFC hardware block.
> + *
> + * Copyright (C) 2020 Renesas Electronics Corp.
> + *
> + * This file is based on the drivers/pinctrl/renesas/pfc-r8a7795.c

based on drivers/pinctrl/renesas/pfc-r8a77951.c?

> +#define CPU_ALL_GP(fn, sfx)    \
> +       PORT_GP_CFG_15(0, fn, sfx, CFG_FLAGS),  \
> +       PORT_GP_CFG_1(0, 15, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 16, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 17, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 18, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 19, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 20, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 21, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 22, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 23, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 24, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 25, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 26, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(0, 27, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_31(1, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),      \
> +       PORT_GP_CFG_2(2, fn, sfx, CFG_FLAGS),                                   \
> +       PORT_GP_CFG_1(2, 2, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 3, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 4, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 5, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 6, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 7, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 8, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 9, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),    \
> +       PORT_GP_CFG_1(2, 10, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(2, 11, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(2, 12, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(2, 13, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(2, 14, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \
> +       PORT_GP_CFG_1(2, 15, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE),   \

I guess it wouldn't hurt to make the voltage options explicit, and start using
SH_PFC_PIN_CFG_IO_VOLTAGE_18_33?

> +       PORT_GP_CFG_1(2, 16, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 17, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 21, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 22, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 23, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(2, 24, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_17(3, fn, sfx, CFG_FLAGS),  \
> +       PORT_GP_CFG_18(4, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(4, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 21, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 22, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 23, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 24, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 25, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(4, 26, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_18(5, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(5, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(5, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(5, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_18(6, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(6, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(6, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(6, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_18(7, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(7, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(7, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(7, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_18(8, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(8, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(8, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(8, 20, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_18(9, fn, sfx, CFG_FLAGS | SH_PFC_PIN_CFG_IO_VOLTAGE_25_33),\
> +       PORT_GP_CFG_1(9, 18, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(9, 19, fn, sfx, CFG_FLAGS),       \
> +       PORT_GP_CFG_1(9, 20, fn, sfx, CFG_FLAGS)

> +/* GPSR1 */
> +#define GPSR1_30       F_(GP1_30,              IP3SR1_27_24)
> +#define GPSR1_29       F_(GP1_29,              IP3SR1_23_20)
> +#define GPSR1_28       F_(GP1_28,              IP3SR1_19_16)
> +#define GPSR1_27       F_(IRQ3,                IP3SR1_15_12)
> +#define GPSR1_26       F_(IRQ2,                IP3SR1_11_8)
> +#define GPSR1_25       F_(IRQ1,                IP3SR1_7_4)
> +#define GPSR1_24       F_(IRQ0,                IP3SR1_3_0)

Please align definitions (also for other GPSR below)

> +#define GPSR1_23       F_(MSIOF2_SS2,  IP2SR1_31_28)
> +#define GPSR1_22       F_(MSIOF2_SS1,  IP2SR1_27_24)
> +#define GPSR1_21       F_(MSIOF2_SYNC, IP2SR1_23_20)
> +#define GPSR1_20       F_(MSIOF2_SCK,  IP2SR1_19_16)
> +#define GPSR1_19       F_(MSIOF2_TXD,  IP2SR1_15_12)
> +#define GPSR1_18       F_(MSIOF2_RXD,  IP2SR1_11_8)
> +#define GPSR1_17       F_(MSIOF1_SS2,  IP2SR1_7_4)
> +#define GPSR1_16       F_(MSIOF1_SS1,  IP2SR1_3_0)
> +#define GPSR1_15       F_(MSIOF1_SYNC, IP1SR1_31_28)
> +#define GPSR1_14       F_(MSIOF1_SCK,  IP1SR1_27_24)
> +#define GPSR1_13       F_(MSIOF1_TXD,  IP1SR1_23_20)
> +#define GPSR1_12       F_(MSIOF1_RXD,  IP1SR1_19_16)
> +#define GPSR1_11       F_(MSIOF0_SS2,  IP1SR1_15_12)
> +#define GPSR1_10       F_(MSIOF0_SS1,  IP1SR1_11_8)
> +#define GPSR1_9                F_(MSIOF0_SYNC, IP1SR1_7_4)
> +#define GPSR1_8                F_(MSIOF0_SCK,  IP1SR1_3_0)
> +#define GPSR1_7                F_(MSIOF0_TXD,  IP0SR1_31_28)
> +#define GPSR1_6                F_(MSIOF0_RXD,  IP0SR1_27_24)
> +#define GPSR1_5                F_(HTX0,                IP0SR1_23_20)
> +#define GPSR1_4                F_(HCTS0_N,             IP0SR1_19_16)
> +#define GPSR1_3                F_(HRTS0_N,             IP0SR1_15_12)
> +#define GPSR1_2                F_(HSCK0,               IP0SR1_11_8)
> +#define GPSR1_1                F_(HRX0,                IP0SR1_7_4)
> +#define GPSR1_0                F_(SCIF_CLK,    IP0SR1_3_0)

> +/* IP3SR1 */           /* 0 */                 /* 1 */         /* 2 */         /* 3 */         /* 4 */         /* 5 */         /* 6 - F */
> +#define IP3SR1_3_0     FM(IRQ0)                F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(DU_DOTCLKOUT)        FM(A24) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_7_4     FM(IRQ1)                F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(DU_HSYNC)    FM(A25) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_11_8    FM(IRQ2)                F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(DU_VSYNC)    FM(CS1_N_A26)   F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_15_12   FM(IRQ3)                F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(DU_ODDF_DISP_CDE)    FM(CS0_N)       F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)

Please align columns

> +#define IP3SR1_19_16   FM(GP1_28)              F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(D0)          F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_23_20   FM(GP1_29)              F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(D1)          F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_27_24   FM(GP1_30)              F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0)        FM(D2)          F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP3SR1_31_28   F_(0, 0)                F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)

> +       { PINMUX_DRIVE_REG("DRV2CTRL2", 0xe6050888) {
> +               { RCAR_GP_PIN(2, 23), 28, 3 },  /* TCLK1_A */
> +               { RCAR_GP_PIN(2, 22), 24, 3 },  /* TPU0TO1 */
> +               { RCAR_GP_PIN(2, 21), 20, 3 },  /* TPU0TO0 */
> +               { RCAR_GP_PIN(2, 20), 16, 3 },  /* CLK_EXTFXR */
> +               { RCAR_GP_PIN(2, 19), 12, 3 },  /* RXDB_EXTFXR */
> +               { RCAR_GP_PIN(2, 18),  8, 3 },  /* FXR_TXDB */
> +               { RCAR_GP_PIN(2, 17),  4, 3 },  /* RXDA_EXTFXR */

RXDA_EXTFXR_A

> +               { RCAR_GP_PIN(2, 16),  0, 3 },  /* FXR_TXDA */

FXR_TXDA_A

> +       } },

> +       { PINMUX_BIAS_REG("PUEN2", 0xe60508c0, "PUD2", 0xe60508e0) {
> +               [ 0] = RCAR_GP_PIN(2,  0),      /* IPC_CLKIN */
> +               [ 1] = RCAR_GP_PIN(2,  1),      /* IPC_CLKOUT */
> +               [ 2] = RCAR_GP_PIN(2,  2),      /* GP2_02 */
> +               [ 3] = RCAR_GP_PIN(2,  3),      /* GP2_03 */
> +               [ 4] = RCAR_GP_PIN(2,  4),      /* GP2_04 */
> +               [ 5] = RCAR_GP_PIN(2,  5),      /* GP2_05 */
> +               [ 6] = RCAR_GP_PIN(2,  6),      /* GP2_06 */
> +               [ 7] = RCAR_GP_PIN(2,  7),      /* GP2_07 */
> +               [ 8] = RCAR_GP_PIN(2,  8),      /* GP2_08 */
> +               [ 9] = RCAR_GP_PIN(2,  9),      /* GP2_09 */
> +               [10] = RCAR_GP_PIN(2, 10),      /* GP2_10 */
> +               [11] = RCAR_GP_PIN(2, 11),      /* GP2_11 */
> +               [12] = RCAR_GP_PIN(2, 12),      /* GP2_12 */
> +               [13] = RCAR_GP_PIN(2, 13),      /* GP2_13 */
> +               [14] = RCAR_GP_PIN(2, 14),      /* GP2_14 */
> +               [15] = RCAR_GP_PIN(2, 15),      /* GP2_15 */
> +               [16] = RCAR_GP_PIN(2, 16),      /* FXR_TXDA */

FXR_TXDA_A

> +               [17] = RCAR_GP_PIN(2, 17),      /* RXDA_EXTFXR */

RXDA_EXTFXR_A

> +               [18] = RCAR_GP_PIN(2, 18),      /* FXR_TXDB */
> +               [19] = RCAR_GP_PIN(2, 19),      /* RXDB_EXTFXR */
> +               [20] = RCAR_GP_PIN(2, 20),      /* CLK_EXTFXR */
> +               [21] = RCAR_GP_PIN(2, 21),      /* TPU0TO0 */
> +               [22] = RCAR_GP_PIN(2, 22),      /* TPU0TO1 */
> +               [23] = RCAR_GP_PIN(2, 23),      /* TCLK1_A */
> +               [24] = RCAR_GP_PIN(2, 24),      /* TCLK2_A */
> +               [25] = SH_PFC_PIN_NONE,
> +               [26] = SH_PFC_PIN_NONE,
> +               [27] = SH_PFC_PIN_NONE,
> +               [28] = SH_PFC_PIN_NONE,
> +               [29] = SH_PFC_PIN_NONE,
> +               [30] = SH_PFC_PIN_NONE,
> +               [31] = SH_PFC_PIN_NONE,
> +       } },

> +       { PINMUX_BIAS_REG("PUEN9", 0xe60698c0, "PUD9", 0xe60698e0) {
> +               [ 0] = RCAR_GP_PIN(9,  0),      /* AVB5_RX_CTL */
> +               [ 1] = RCAR_GP_PIN(9,  1),      /* AVB5_RXC */
> +               [ 2] = RCAR_GP_PIN(9,  2),      /* AVB5_RD0 */
> +               [ 3] = RCAR_GP_PIN(9,  3),      /* AVB5_RD1 */
> +               [ 4] = RCAR_GP_PIN(9,  4),      /* AVB5_RD2 */
> +               [ 5] = RCAR_GP_PIN(9,  5),      /* AVB5_RD3 */
> +               [ 6] = RCAR_GP_PIN(9,  6),      /* AVB5_TX_CTL */
> +               [ 7] = RCAR_GP_PIN(9,  7),      /* AVB5_TXC */
> +               [ 8] = RCAR_GP_PIN(9,  8),      /* AVB5_TD0 */
> +               [ 9] = RCAR_GP_PIN(9,  9),      /* AVB5_TD1 */
> +               [10] = RCAR_GP_PIN(9, 10),      /* AVB5_TD2 */
> +               [11] = RCAR_GP_PIN(9, 11),      /* AVB5_TD3 */
> +               [12] = RCAR_GP_PIN(9, 12),      /* AVB5_TXCREFCLK */
> +               [13] = RCAR_GP_PIN(9, 13),      /* AVB5_MDIO */
> +               [14] = RCAR_GP_PIN(9, 14),      /* AVB5_MDC */
> +               [15] = RCAR_GP_PIN(9, 15),      /* AVB5_MAGIC */
> +               [16] = RCAR_GP_PIN(9, 16),      /* AVB5_PHY_INT */
> +               [17] = RCAR_GP_PIN(9, 17),      /* AVB5_LINK */
> +               [18] = RCAR_GP_PIN(9, 18),      /* AVB5_AVTP_MATCH */
> +               [19] = RCAR_GP_PIN(9, 19),      /* AVB5_AVTP_CAPTURE */
> +               [20] = RCAR_GP_PIN(9, 20),      /* AVB5_AVTP_PPS */
> +               [21] = SH_PFC_PIN_NONE,
> +               [22] = SH_PFC_PIN_NONE,
> +               [23] = SH_PFC_PIN_NONE,
> +               [24] = SH_PFC_PIN_NONE,
> +               [25] = SH_PFC_PIN_NONE,
> +               [26] = SH_PFC_PIN_NONE,
> +               [27] = SH_PFC_PIN_NONE,
> +               [28] = SH_PFC_PIN_NONE,
> +               [29] = SH_PFC_PIN_NONE,
> +               [30] = SH_PFC_PIN_NONE,
> +               [31] = SH_PFC_PIN_NONE,
> +               [21] = SH_PFC_PIN_NONE,
> +               [22] = SH_PFC_PIN_NONE,
> +               [23] = SH_PFC_PIN_NONE,
> +               [24] = SH_PFC_PIN_NONE,
> +               [25] = SH_PFC_PIN_NONE,
> +               [26] = SH_PFC_PIN_NONE,
> +               [27] = SH_PFC_PIN_NONE,
> +               [28] = SH_PFC_PIN_NONE,
> +               [29] = SH_PFC_PIN_NONE,
> +               [30] = SH_PFC_PIN_NONE,
> +               [31] = SH_PFC_PIN_NONE,

Duplicated entries.

> +       } },

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 2/5] pinctrl: renesas: add I/O voltage level flag

[Geert Uytterhoeven]

Hi Uli,

On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote:
> This patch adds config macros describing the voltage levels available on
> a pin. The current default (3.3V/1.8V) maps to zero to avoid having to
> change existing PFC implementations.
>
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>
> --- a/drivers/pinctrl/renesas/pinctrl.c
> +++ b/drivers/pinctrl/renesas/pinctrl.c
> @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
>         }
>
>         case PIN_CONFIG_POWER_SOURCE: {
> +               int idx = sh_pfc_get_pin_index(pfc, _pin);
> +               const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
> +               int lower_voltage;
>                 u32 pocctrl, val;
>                 int bit;
>
> @@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
>                 val = sh_pfc_read(pfc, pocctrl);
>                 spin_unlock_irqrestore(&pfc->lock, flags);
>
> -               arg = (val & BIT(bit)) ? 3300 : 1800;
> +               lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ?
> +                       2500 : 1800;
> +
> +               arg = (val & BIT(bit)) ? 3300 : lower_voltage;

Alternatively, .pin_to_pocctrl() could return mV_low and mV_high?
That would require updating all subdrivers, though.

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 2/5] pinctrl: renesas: add I/O voltage level flag

[Ulrich Hecht]

> On 12/22/2020 11:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > --- a/drivers/pinctrl/renesas/pinctrl.c
> > +++ b/drivers/pinctrl/renesas/pinctrl.c
> > @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin,
> >         }
> >
> >         case PIN_CONFIG_POWER_SOURCE: {
> > +               int idx = sh_pfc_get_pin_index(pfc, _pin);
> 
> I guess this cannot fail when we get here?

That would require a bug elsewhere, I think.

> > +               const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
> > +               int lower_voltage;
> 
> unsigned int

Fair enough...

> mV_low?

That, though, seems ambiguous to me because it could refer to the logical-zero voltage.
(Are internal capital letters even permitted in identifiers?)

CU
Uli

Re: [PATCH v2 2/5] pinctrl: renesas: add I/O voltage level flag

[Ulrich Hecht]

> On 12/22/2020 12:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Alternatively, .pin_to_pocctrl() could return mV_low and mV_high?
> That would require updating all subdrivers, though.

Exactly. :)

CU
Uli

Re: [PATCH v2 2/5] pinctrl: renesas: add I/O voltage level flag

[Geert Uytterhoeven]

Hi Uli,

On Tue, Dec 22, 2020 at 5:51 PM Ulrich Hecht <uli@fpond.eu> wrote:
> > On 12/22/2020 11:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > --- a/drivers/pinctrl/renesas/pinctrl.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl.c
> > > +               const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
> > > +               int lower_voltage;
> >
> > unsigned int
>
> Fair enough...
>
> > mV_low?
>
> That, though, seems ambiguous to me because it could refer to the logical-zero voltage.

True.

> (Are internal capital letters even permitted in identifiers?)

We already have "mV".

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 0/5] pinctrl: renesas: basic R8A779A0 (V3U) support

[Wolfram Sang]

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


> This series provides basic V3U pin control support, up to and including the
> SCIF pins.
> 
> This revision includes fixes for numerous issues found by Geert in his
> review; see below for details.

I use this series to enable other devices. SCIF0, I2C0-6, and AVB0
passed all my tests, so far. So, I think I can say:

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


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

Re: [PATCH v2 4/5] pinctrl: renesas: Initial R8A779A0 (V3U) PFC support

[Wolfram Sang]

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


I got a correct report from a build bot:

> +	{ PINMUX_BIAS_REG("PUEN9", 0xe60698c0, "PUD9", 0xe60698e0) {
> +		[ 0] = RCAR_GP_PIN(9,  0),	/* AVB5_RX_CTL */
> +		[ 1] = RCAR_GP_PIN(9,  1),	/* AVB5_RXC */
> +		[ 2] = RCAR_GP_PIN(9,  2),	/* AVB5_RD0 */
> +		[ 3] = RCAR_GP_PIN(9,  3),	/* AVB5_RD1 */
> +		[ 4] = RCAR_GP_PIN(9,  4),	/* AVB5_RD2 */
> +		[ 5] = RCAR_GP_PIN(9,  5),	/* AVB5_RD3 */
> +		[ 6] = RCAR_GP_PIN(9,  6),	/* AVB5_TX_CTL */
> +		[ 7] = RCAR_GP_PIN(9,  7),	/* AVB5_TXC */
> +		[ 8] = RCAR_GP_PIN(9,  8),	/* AVB5_TD0 */
> +		[ 9] = RCAR_GP_PIN(9,  9),	/* AVB5_TD1 */
> +		[10] = RCAR_GP_PIN(9, 10),	/* AVB5_TD2 */
> +		[11] = RCAR_GP_PIN(9, 11),	/* AVB5_TD3 */
> +		[12] = RCAR_GP_PIN(9, 12),	/* AVB5_TXCREFCLK */
> +		[13] = RCAR_GP_PIN(9, 13),	/* AVB5_MDIO */
> +		[14] = RCAR_GP_PIN(9, 14),	/* AVB5_MDC */
> +		[15] = RCAR_GP_PIN(9, 15),	/* AVB5_MAGIC */
> +		[16] = RCAR_GP_PIN(9, 16),	/* AVB5_PHY_INT */
> +		[17] = RCAR_GP_PIN(9, 17),	/* AVB5_LINK */
> +		[18] = RCAR_GP_PIN(9, 18),	/* AVB5_AVTP_MATCH */
> +		[19] = RCAR_GP_PIN(9, 19),	/* AVB5_AVTP_CAPTURE */
> +		[20] = RCAR_GP_PIN(9, 20),	/* AVB5_AVTP_PPS */
> +		[21] = SH_PFC_PIN_NONE,
> +		[22] = SH_PFC_PIN_NONE,
> +		[23] = SH_PFC_PIN_NONE,
> +		[24] = SH_PFC_PIN_NONE,
> +		[25] = SH_PFC_PIN_NONE,
> +		[26] = SH_PFC_PIN_NONE,
> +		[27] = SH_PFC_PIN_NONE,
> +		[28] = SH_PFC_PIN_NONE,
> +		[29] = SH_PFC_PIN_NONE,
> +		[30] = SH_PFC_PIN_NONE,
> +		[31] = SH_PFC_PIN_NONE,
> +		[21] = SH_PFC_PIN_NONE,
> +		[22] = SH_PFC_PIN_NONE,
> +		[23] = SH_PFC_PIN_NONE,
> +		[24] = SH_PFC_PIN_NONE,
> +		[25] = SH_PFC_PIN_NONE,
> +		[26] = SH_PFC_PIN_NONE,
> +		[27] = SH_PFC_PIN_NONE,
> +		[28] = SH_PFC_PIN_NONE,
> +		[29] = SH_PFC_PIN_NONE,
> +		[30] = SH_PFC_PIN_NONE,
> +		[31] = SH_PFC_PIN_NONE,
> +	} },

Pins 21-31 are set to none twice.


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

Re: [PATCH v2 5/5] pinctrl: renesas: r8a779a0: Add SCIF pins, groups and functions

[Wolfram Sang]

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

On Mon, Dec 21, 2020 at 05:54:48PM +0100, Ulrich Hecht wrote:
> This patch adds SCIF0, 1, 3 and 4 pins, groups and functions for the
> R8A779A0 (V3U) SoC.
> 
> Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu>

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> # for SCIF0 and SCIF_CLK


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

Re: [PATCH v2 4/5] pinctrl: renesas: Initial R8A779A0 (V3U) PFC support

[Ulrich Hecht]

> On 12/22/2020 12:42 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > --- /dev/null
> > +++ b/drivers/pinctrl/renesas/pfc-r8a779a0.c
> > @@ -0,0 +1,2527 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * R8A779A0 processor support - PFC hardware block.
> > + *
> > + * Copyright (C) 2020 Renesas Electronics Corp.
> > + *
> > + * This file is based on the drivers/pinctrl/renesas/pfc-r8a7795.c
> 
> based on drivers/pinctrl/renesas/pfc-r8a77951.c?

I'm inclined to leave that as it is because that's what the file was called when the original patch was created.

CU
Uli