diff mbox series

[1/4] pinctrl: sh-pfc: r8a77951: Add VIN pins, groups and functions

Message ID 20200914233744.468175-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series pinctrl: sh-pfc: Add VIN stf8 pins | expand

Commit Message

Niklas Söderlund Sept. 14, 2020, 11:37 p.m. UTC 
From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds VIN{4,5} pins, groups and functions to the R8A77951 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[Niklas: Rework to fit upstream driver]
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77951.c | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Sergei Shtylyov Sept. 15, 2020, 8:23 a.m. UTC | #1 
On 15.09.2020 2:37, Niklas Söderlund wrote:

> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

    Re: subject -- you seem to be only adding the SFT8 pins/groups/funcs...

> This patch adds VIN{4,5} pins, groups and functions to the R8A77951 SoC.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Niklas: Rework to fit upstream driver]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>   drivers/pinctrl/sh-pfc/pfc-r8a77951.c | 28 +++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77951.c b/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
> index a94ebe0bf5d06127..afba52ae92009f17 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
> @@ -4074,6 +4074,18 @@ static const union vin_data vin4_data_b_mux = {
>   		VI4_DATA22_MARK, VI4_DATA23_MARK,
>   	},
>   };
> +static const unsigned int vin4_data8_sft8_pins[] = {
> +	RCAR_GP_PIN(1, 0),  RCAR_GP_PIN(1, 1),
> +	RCAR_GP_PIN(1, 2),  RCAR_GP_PIN(1, 3),
> +	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin4_data8_sft8_mux[] = {
> +	VI4_DATA8_MARK,  VI4_DATA9_MARK,
> +	VI4_DATA10_MARK, VI4_DATA11_MARK,
> +	VI4_DATA12_MARK, VI4_DATA13_MARK,
> +	VI4_DATA14_MARK, VI4_DATA15_MARK,
> +};
>   static const unsigned int vin4_sync_pins[] = {
>   	/* HSYNC#, VSYNC# */
>   	RCAR_GP_PIN(1, 18), RCAR_GP_PIN(1, 17),
> @@ -4128,6 +4140,18 @@ static const union vin_data16 vin5_data_mux = {
>   		VI5_DATA14_MARK, VI5_DATA15_MARK,
>   	},
>   };
> +static const unsigned int vin5_data8_sft8_pins[] = {
> +	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13),
> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15),
> +	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin5_data8_sft8_mux[] = {
> +	VI5_DATA8_MARK,  VI5_DATA9_MARK,
> +	VI5_DATA10_MARK, VI5_DATA11_MARK,
> +	VI5_DATA12_MARK, VI5_DATA13_MARK,
> +	VI5_DATA14_MARK, VI5_DATA15_MARK,
> +};
>   static const unsigned int vin5_sync_pins[] = {
>   	/* HSYNC#, VSYNC# */
>   	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 9),
> @@ -4470,6 +4494,7 @@ static const struct {
>   		SH_PFC_PIN_GROUP(vin4_data18_b),
>   		VIN_DATA_PIN_GROUP(vin4_data, 20, _b),
>   		VIN_DATA_PIN_GROUP(vin4_data, 24, _b),
> +		SH_PFC_PIN_GROUP(vin4_data8_sft8),
>   		SH_PFC_PIN_GROUP(vin4_sync),
>   		SH_PFC_PIN_GROUP(vin4_field),
>   		SH_PFC_PIN_GROUP(vin4_clkenb),
> @@ -4478,6 +4503,7 @@ static const struct {
>   		VIN_DATA_PIN_GROUP(vin5_data, 10),
>   		VIN_DATA_PIN_GROUP(vin5_data, 12),
>   		VIN_DATA_PIN_GROUP(vin5_data, 16),
> +		SH_PFC_PIN_GROUP(vin5_data8_sft8),
>   		SH_PFC_PIN_GROUP(vin5_sync),
>   		SH_PFC_PIN_GROUP(vin5_field),
>   		SH_PFC_PIN_GROUP(vin5_clkenb),
> @@ -5022,6 +5048,7 @@ static const char * const vin4_groups[] = {
>   	"vin4_data18_b",
>   	"vin4_data20_b",
>   	"vin4_data24_b",
> +	"vin4_data8_sft8",
>   	"vin4_sync",
>   	"vin4_field",
>   	"vin4_clkenb",
> @@ -5033,6 +5060,7 @@ static const char * const vin5_groups[] = {
>   	"vin5_data10",
>   	"vin5_data12",
>   	"vin5_data16",
> +	"vin5_data8_sft8",
>   	"vin5_sync",
>   	"vin5_field",
>   	"vin5_clkenb",

MBR, Sergei
Geert Uytterhoeven Sept. 23, 2020, 9:57 a.m. UTC | #2 
Hi Niklas,

CC Laurent.

On Tue, Sep 15, 2020 at 1:38 AM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds VIN{4,5} pins, groups and functions to the R8A77951 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Niklas: Rework to fit upstream driver]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thanks for your patch!

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
> @@ -4074,6 +4074,18 @@ static const union vin_data vin4_data_b_mux = {
>                 VI4_DATA22_MARK, VI4_DATA23_MARK,
>         },
>  };
> +static const unsigned int vin4_data8_sft8_pins[] = {
> +       RCAR_GP_PIN(1, 0),  RCAR_GP_PIN(1, 1),
> +       RCAR_GP_PIN(1, 2),  RCAR_GP_PIN(1, 3),
> +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +       RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin4_data8_sft8_mux[] = {
> +       VI4_DATA8_MARK,  VI4_DATA9_MARK,
> +       VI4_DATA10_MARK, VI4_DATA11_MARK,
> +       VI4_DATA12_MARK, VI4_DATA13_MARK,
> +       VI4_DATA14_MARK, VI4_DATA15_MARK,
> +};

I'm not so fond of the "sft8" suffix.
What about using "vin4_g8" instead of "vin4_data8_sft8", cfr. "[PATCH v3]
pinctrl: renesas: r8a7790: Add VIN1-B and VIN2-G pins, groups and
functions"?
https://lore.kernel.org/linux-renesas-soc/20200917195924.20384-1-prabhakar.mahadev-lad.rj@bp.renesas.com

>  static const unsigned int vin4_sync_pins[] = {
>         /* HSYNC#, VSYNC# */
>         RCAR_GP_PIN(1, 18), RCAR_GP_PIN(1, 17),
> @@ -4128,6 +4140,18 @@ static const union vin_data16 vin5_data_mux = {
>                 VI5_DATA14_MARK, VI5_DATA15_MARK,
>         },
>  };
> +static const unsigned int vin5_data8_sft8_pins[] = {
> +       RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13),
> +       RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15),
> +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +       RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin5_data8_sft8_mux[] = {
> +       VI5_DATA8_MARK,  VI5_DATA9_MARK,
> +       VI5_DATA10_MARK, VI5_DATA11_MARK,
> +       VI5_DATA12_MARK, VI5_DATA13_MARK,
> +       VI5_DATA14_MARK, VI5_DATA15_MARK,
> +};

I'm not so fond of the "sft8" suffix.
However, as this is a 16-bit instead of a 24-bit interface, "vin5_g8"
doesn't sound appropriate to me.
Anyone with a good suggestion?

>  static const unsigned int vin5_sync_pins[] = {
>         /* HSYNC#, VSYNC# */
>         RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 9),
> @@ -4470,6 +4494,7 @@ static const struct {
>                 SH_PFC_PIN_GROUP(vin4_data18_b),
>                 VIN_DATA_PIN_GROUP(vin4_data, 20, _b),
>                 VIN_DATA_PIN_GROUP(vin4_data, 24, _b),
> +               SH_PFC_PIN_GROUP(vin4_data8_sft8),

warning: excess elements in array initializer

You have to increase the size of the .common[] array, to match the
actual number of entries.
Booting a kernel with CONFIG_DEBUG_PINCTRL=y also helps.

The same comments apply to patches 2-4.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77951.c b/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
index a94ebe0bf5d06127..afba52ae92009f17 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77951.c
@@ -4074,6 +4074,18 @@  static const union vin_data vin4_data_b_mux = {
 		VI4_DATA22_MARK, VI4_DATA23_MARK,
 	},
 };
+static const unsigned int vin4_data8_sft8_pins[] = {
+	RCAR_GP_PIN(1, 0),  RCAR_GP_PIN(1, 1),
+	RCAR_GP_PIN(1, 2),  RCAR_GP_PIN(1, 3),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+};
+static const unsigned int vin4_data8_sft8_mux[] = {
+	VI4_DATA8_MARK,  VI4_DATA9_MARK,
+	VI4_DATA10_MARK, VI4_DATA11_MARK,
+	VI4_DATA12_MARK, VI4_DATA13_MARK,
+	VI4_DATA14_MARK, VI4_DATA15_MARK,
+};
 static const unsigned int vin4_sync_pins[] = {
 	/* HSYNC#, VSYNC# */
 	RCAR_GP_PIN(1, 18), RCAR_GP_PIN(1, 17),
@@ -4128,6 +4140,18 @@  static const union vin_data16 vin5_data_mux = {
 		VI5_DATA14_MARK, VI5_DATA15_MARK,
 	},
 };
+static const unsigned int vin5_data8_sft8_pins[] = {
+	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13),
+	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+};
+static const unsigned int vin5_data8_sft8_mux[] = {
+	VI5_DATA8_MARK,  VI5_DATA9_MARK,
+	VI5_DATA10_MARK, VI5_DATA11_MARK,
+	VI5_DATA12_MARK, VI5_DATA13_MARK,
+	VI5_DATA14_MARK, VI5_DATA15_MARK,
+};
 static const unsigned int vin5_sync_pins[] = {
 	/* HSYNC#, VSYNC# */
 	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 9),
@@ -4470,6 +4494,7 @@  static const struct {
 		SH_PFC_PIN_GROUP(vin4_data18_b),
 		VIN_DATA_PIN_GROUP(vin4_data, 20, _b),
 		VIN_DATA_PIN_GROUP(vin4_data, 24, _b),
+		SH_PFC_PIN_GROUP(vin4_data8_sft8),
 		SH_PFC_PIN_GROUP(vin4_sync),
 		SH_PFC_PIN_GROUP(vin4_field),
 		SH_PFC_PIN_GROUP(vin4_clkenb),
@@ -4478,6 +4503,7 @@  static const struct {
 		VIN_DATA_PIN_GROUP(vin5_data, 10),
 		VIN_DATA_PIN_GROUP(vin5_data, 12),
 		VIN_DATA_PIN_GROUP(vin5_data, 16),
+		SH_PFC_PIN_GROUP(vin5_data8_sft8),
 		SH_PFC_PIN_GROUP(vin5_sync),
 		SH_PFC_PIN_GROUP(vin5_field),
 		SH_PFC_PIN_GROUP(vin5_clkenb),
@@ -5022,6 +5048,7 @@  static const char * const vin4_groups[] = {
 	"vin4_data18_b",
 	"vin4_data20_b",
 	"vin4_data24_b",
+	"vin4_data8_sft8",
 	"vin4_sync",
 	"vin4_field",
 	"vin4_clkenb",
@@ -5033,6 +5060,7 @@  static const char * const vin5_groups[] = {
 	"vin5_data10",
 	"vin5_data12",
 	"vin5_data16",
+	"vin5_data8_sft8",
 	"vin5_sync",
 	"vin5_field",
 	"vin5_clkenb",