* [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions
@ 2020-09-13 18:28 Lad Prabhakar
2020-09-14 14:47 ` Geert Uytterhoeven
0 siblings, 1 reply; 5+ messages in thread
From: Lad Prabhakar @ 2020-09-13 18:28 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij, linux-renesas-soc
Cc: linux-gpio, linux-kernel, Biju Das, Lad Prabhakar, Prabhakar
Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
Changes for v2:
* Added complete list of VIN1-B pins
* Renamed vin2_data8_g to vin2_data8g
* Sorted vin1_sync_b pins
v1 - https://patchwork.kernel.org/patch/11761191/
---
drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 114 ++++++++++++++++++++++++++-
1 file changed, 113 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 60f973c5dffe..40c99942925c 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -3866,6 +3866,72 @@ static const unsigned int vin1_data18_mux[] = {
VI1_R4_MARK, VI1_R5_MARK,
VI1_R6_MARK, VI1_R7_MARK,
};
+static const union vin_data vin1_data_b_pins = {
+ .data24 = {
+ /* B */
+ RCAR_GP_PIN(3, 0), RCAR_GP_PIN(3, 1),
+ RCAR_GP_PIN(3, 2), RCAR_GP_PIN(3, 3),
+ RCAR_GP_PIN(3, 4), RCAR_GP_PIN(3, 5),
+ RCAR_GP_PIN(3, 6), RCAR_GP_PIN(3, 7),
+ /* G */
+ RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15),
+ RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 20),
+ RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
+ RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
+ /* R */
+ RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
+ RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 4),
+ RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
+ RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
+ },
+};
+static const union vin_data vin1_data_b_mux = {
+ .data24 = {
+ /* B */
+ VI1_DATA0_VI1_B0_B_MARK, VI1_DATA1_VI1_B1_B_MARK,
+ VI1_DATA2_VI1_B2_B_MARK, VI1_DATA3_VI1_B3_B_MARK,
+ VI1_DATA4_VI1_B4_B_MARK, VI1_DATA5_VI1_B5_B_MARK,
+ VI1_DATA6_VI1_B6_B_MARK, VI1_DATA7_VI1_B7_B_MARK,
+ /* G */
+ VI1_G0_B_MARK, VI1_G1_B_MARK,
+ VI1_G2_B_MARK, VI1_G3_B_MARK,
+ VI1_G4_B_MARK, VI1_G5_B_MARK,
+ VI1_G6_B_MARK, VI1_G7_B_MARK,
+ /* R */
+ VI1_R0_B_MARK, VI1_R1_B_MARK,
+ VI1_R2_B_MARK, VI1_R3_B_MARK,
+ VI1_R4_B_MARK, VI1_R5_B_MARK,
+ VI1_R6_B_MARK, VI1_R7_B_MARK,
+ },
+};
+static const unsigned int vin1_data18_b_pins[] = {
+ /* B */
+ RCAR_GP_PIN(3, 2), RCAR_GP_PIN(3, 3),
+ RCAR_GP_PIN(3, 4), RCAR_GP_PIN(3, 5),
+ RCAR_GP_PIN(3, 6), RCAR_GP_PIN(3, 7),
+ /* G */
+ RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 20),
+ RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
+ RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
+ /* R */
+ RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 4),
+ RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
+ RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
+};
+static const unsigned int vin1_data18_b_mux[] = {
+ /* B */
+ VI1_DATA2_VI1_B2_B_MARK, VI1_DATA3_VI1_B3_B_MARK,
+ VI1_DATA4_VI1_B4_B_MARK, VI1_DATA5_VI1_B5_B_MARK,
+ VI1_DATA6_VI1_B6_B_MARK, VI1_DATA7_VI1_B7_B_MARK,
+ /* G */
+ VI1_G2_B_MARK, VI1_G3_B_MARK,
+ VI1_G4_B_MARK, VI1_G5_B_MARK,
+ VI1_G6_B_MARK, VI1_G7_B_MARK,
+ /* R */
+ VI1_R2_B_MARK, VI1_R3_B_MARK,
+ VI1_R4_B_MARK, VI1_R5_B_MARK,
+ VI1_R6_B_MARK, VI1_R7_B_MARK,
+};
static const unsigned int vin1_sync_pins[] = {
RCAR_GP_PIN(1, 24), /* HSYNC */
RCAR_GP_PIN(1, 25), /* VSYNC */
@@ -3874,6 +3940,14 @@ static const unsigned int vin1_sync_mux[] = {
VI1_HSYNC_N_MARK,
VI1_VSYNC_N_MARK,
};
+static const unsigned int vin1_sync_b_pins[] = {
+ RCAR_GP_PIN(1, 24), /* HSYNC */
+ RCAR_GP_PIN(1, 25), /* VSYNC */
+};
+static const unsigned int vin1_sync_b_mux[] = {
+ VI1_HSYNC_N_B_MARK,
+ VI1_VSYNC_N_B_MARK,
+};
static const unsigned int vin1_field_pins[] = {
RCAR_GP_PIN(1, 13),
};
@@ -3892,6 +3966,12 @@ static const unsigned int vin1_clk_pins[] = {
static const unsigned int vin1_clk_mux[] = {
VI1_CLK_MARK,
};
+static const unsigned int vin1_clk_b_pins[] = {
+ RCAR_GP_PIN(3, 15),
+};
+static const unsigned int vin1_clk_b_mux[] = {
+ VI1_CLK_B_MARK,
+};
/* - VIN2 ----------------------------------------------------------------- */
static const union vin_data vin2_data_pins = {
.data24 = {
@@ -3959,6 +4039,18 @@ static const unsigned int vin2_data18_mux[] = {
VI2_R4_MARK, VI2_R5_MARK,
VI2_R6_MARK, VI2_R7_MARK,
};
+static const unsigned int vin2_data8g_pins[] = {
+ RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
+ RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 10),
+ RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
+ RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
+};
+static const unsigned int vin2_data8g_mux[] = {
+ VI2_G0_MARK, VI2_G1_MARK,
+ VI2_G2_MARK, VI2_G3_MARK,
+ VI2_G4_MARK, VI2_G5_MARK,
+ VI2_G6_MARK, VI2_G7_MARK,
+};
static const unsigned int vin2_sync_pins[] = {
RCAR_GP_PIN(1, 16), /* HSYNC */
RCAR_GP_PIN(1, 21), /* VSYNC */
@@ -4026,7 +4118,7 @@ static const unsigned int vin3_clk_mux[] = {
};
static const struct {
- struct sh_pfc_pin_group common[298];
+ struct sh_pfc_pin_group common[308];
struct sh_pfc_pin_group automotive[1];
} pinmux_groups = {
.common = {
@@ -4310,15 +4402,25 @@ static const struct {
VIN_DATA_PIN_GROUP(vin1_data, 10),
VIN_DATA_PIN_GROUP(vin1_data, 8),
VIN_DATA_PIN_GROUP(vin1_data, 4),
+ VIN_DATA_PIN_GROUP(vin1_data, 24, _b),
+ VIN_DATA_PIN_GROUP(vin1_data, 20, _b),
+ SH_PFC_PIN_GROUP(vin1_data18_b),
+ VIN_DATA_PIN_GROUP(vin1_data, 16, _b),
+ VIN_DATA_PIN_GROUP(vin1_data, 12, _b),
+ VIN_DATA_PIN_GROUP(vin1_data, 10, _b),
+ VIN_DATA_PIN_GROUP(vin1_data, 8, _b),
SH_PFC_PIN_GROUP(vin1_sync),
+ SH_PFC_PIN_GROUP(vin1_sync_b),
SH_PFC_PIN_GROUP(vin1_field),
SH_PFC_PIN_GROUP(vin1_clkenb),
SH_PFC_PIN_GROUP(vin1_clk),
+ SH_PFC_PIN_GROUP(vin1_clk_b),
VIN_DATA_PIN_GROUP(vin2_data, 24),
SH_PFC_PIN_GROUP(vin2_data18),
VIN_DATA_PIN_GROUP(vin2_data, 16),
VIN_DATA_PIN_GROUP(vin2_data, 8),
VIN_DATA_PIN_GROUP(vin2_data, 4),
+ SH_PFC_PIN_GROUP(vin2_data8g),
SH_PFC_PIN_GROUP(vin2_sync),
SH_PFC_PIN_GROUP(vin2_field),
SH_PFC_PIN_GROUP(vin2_clkenb),
@@ -4784,10 +4886,19 @@ static const char * const vin1_groups[] = {
"vin1_data10",
"vin1_data8",
"vin1_data4",
+ "vin1_data24_b",
+ "vin1_data20_b",
+ "vin1_data18_b",
+ "vin1_data16_b",
+ "vin1_data12_b",
+ "vin1_data10_b",
+ "vin1_data8_b",
"vin1_sync",
+ "vin1_sync_b",
"vin1_field",
"vin1_clkenb",
"vin1_clk",
+ "vin1_clk_b",
};
static const char * const vin2_groups[] = {
@@ -4796,6 +4907,7 @@ static const char * const vin2_groups[] = {
"vin2_data16",
"vin2_data8",
"vin2_data4",
+ "vin2_data8g",
"vin2_sync",
"vin2_field",
"vin2_clkenb",
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions
2020-09-13 18:28 [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions Lad Prabhakar
@ 2020-09-14 14:47 ` Geert Uytterhoeven
2020-09-14 23:27 ` Niklas Söderlund
0 siblings, 1 reply; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-09-14 14:47 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Linus Walleij, Linux-Renesas, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Biju Das, Prabhakar, Laurent Pinchart,
Niklas Söderlund
Hi Prabhakar,
CC Laurent, Niklas
On Sun, Sep 13, 2020 at 8:29 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> Changes for v2:
> * Added complete list of VIN1-B pins
> * Renamed vin2_data8_g to vin2_data8g
> * Sorted vin1_sync_b pins
>
> v1 - https://patchwork.kernel.org/patch/11761191/
Thanks for the update!
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -3874,6 +3940,14 @@ static const unsigned int vin1_sync_mux[] = {
> VI1_HSYNC_N_MARK,
> VI1_VSYNC_N_MARK,
> };
> +static const unsigned int vin1_sync_b_pins[] = {
> + RCAR_GP_PIN(1, 24), /* HSYNC */
> + RCAR_GP_PIN(1, 25), /* VSYNC */
> +};
> +static const unsigned int vin1_sync_b_mux[] = {
> + VI1_HSYNC_N_B_MARK,
> + VI1_VSYNC_N_B_MARK,
> +};
> static const unsigned int vin1_field_pins[] = {
> RCAR_GP_PIN(1, 13),
> };
Missing field_b and clkenb_b.
> @@ -3959,6 +4039,18 @@ static const unsigned int vin2_data18_mux[] = {
> VI2_R4_MARK, VI2_R5_MARK,
> VI2_R6_MARK, VI2_R7_MARK,
> };
> +static const unsigned int vin2_data8g_pins[] = {
> + RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
> + RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 10),
> + RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> + RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin2_data8g_mux[] = {
> + VI2_G0_MARK, VI2_G1_MARK,
> + VI2_G2_MARK, VI2_G3_MARK,
> + VI2_G4_MARK, VI2_G5_MARK,
> + VI2_G6_MARK, VI2_G7_MARK,
> +};
Laurent, Niklas: are you happy with the name "vin2_data8g", or do
you have a better suggestion?
> static const unsigned int vin2_sync_pins[] = {
> RCAR_GP_PIN(1, 16), /* HSYNC */
> RCAR_GP_PIN(1, 21), /* VSYNC */
> @@ -4310,15 +4402,25 @@ static const struct {
> VIN_DATA_PIN_GROUP(vin1_data, 10),
> VIN_DATA_PIN_GROUP(vin1_data, 8),
> VIN_DATA_PIN_GROUP(vin1_data, 4),
> + VIN_DATA_PIN_GROUP(vin1_data, 24, _b),
> + VIN_DATA_PIN_GROUP(vin1_data, 20, _b),
> + SH_PFC_PIN_GROUP(vin1_data18_b),
> + VIN_DATA_PIN_GROUP(vin1_data, 16, _b),
> + VIN_DATA_PIN_GROUP(vin1_data, 12, _b),
> + VIN_DATA_PIN_GROUP(vin1_data, 10, _b),
> + VIN_DATA_PIN_GROUP(vin1_data, 8, _b),
Missing vin1_data4_b.
> SH_PFC_PIN_GROUP(vin1_sync),
> + SH_PFC_PIN_GROUP(vin1_sync_b),
> SH_PFC_PIN_GROUP(vin1_field),
> SH_PFC_PIN_GROUP(vin1_clkenb),
> SH_PFC_PIN_GROUP(vin1_clk),
> + SH_PFC_PIN_GROUP(vin1_clk_b),
> VIN_DATA_PIN_GROUP(vin2_data, 24),
> SH_PFC_PIN_GROUP(vin2_data18),
> VIN_DATA_PIN_GROUP(vin2_data, 16),
> VIN_DATA_PIN_GROUP(vin2_data, 8),
> VIN_DATA_PIN_GROUP(vin2_data, 4),
> + SH_PFC_PIN_GROUP(vin2_data8g),
> SH_PFC_PIN_GROUP(vin2_sync),
> SH_PFC_PIN_GROUP(vin2_field),
> SH_PFC_PIN_GROUP(vin2_clkenb),
> @@ -4784,10 +4886,19 @@ static const char * const vin1_groups[] = {
> "vin1_data10",
> "vin1_data8",
> "vin1_data4",
> + "vin1_data24_b",
> + "vin1_data20_b",
> + "vin1_data18_b",
> + "vin1_data16_b",
> + "vin1_data12_b",
> + "vin1_data10_b",
> + "vin1_data8_b",
Missing vin1_data4_b.
> "vin1_sync",
> + "vin1_sync_b",
> "vin1_field",
> "vin1_clkenb",
> "vin1_clk",
> + "vin1_clk_b",
> };
>
> static const char * const vin2_groups[] = {
The rest looks good to me.
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions
2020-09-14 14:47 ` Geert Uytterhoeven
@ 2020-09-14 23:27 ` Niklas Söderlund
2020-09-14 23:40 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Niklas Söderlund @ 2020-09-14 23:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Lad Prabhakar, Linus Walleij, Linux-Renesas,
open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Biju Das,
Prabhakar, Laurent Pinchart
Hi Geert, Lad,
On 2020-09-14 16:47:27 +0200, Geert Uytterhoeven wrote:
> Hi Prabhakar,
>
> CC Laurent, Niklas
>
> On Sun, Sep 13, 2020 at 8:29 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > Changes for v2:
> > * Added complete list of VIN1-B pins
> > * Renamed vin2_data8_g to vin2_data8g
> > * Sorted vin1_sync_b pins
> >
> > v1 - https://patchwork.kernel.org/patch/11761191/
>
> Thanks for the update!
>
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>
> > @@ -3874,6 +3940,14 @@ static const unsigned int vin1_sync_mux[] = {
> > VI1_HSYNC_N_MARK,
> > VI1_VSYNC_N_MARK,
> > };
> > +static const unsigned int vin1_sync_b_pins[] = {
> > + RCAR_GP_PIN(1, 24), /* HSYNC */
> > + RCAR_GP_PIN(1, 25), /* VSYNC */
> > +};
> > +static const unsigned int vin1_sync_b_mux[] = {
> > + VI1_HSYNC_N_B_MARK,
> > + VI1_VSYNC_N_B_MARK,
> > +};
> > static const unsigned int vin1_field_pins[] = {
> > RCAR_GP_PIN(1, 13),
> > };
>
> Missing field_b and clkenb_b.
>
> > @@ -3959,6 +4039,18 @@ static const unsigned int vin2_data18_mux[] = {
> > VI2_R4_MARK, VI2_R5_MARK,
> > VI2_R6_MARK, VI2_R7_MARK,
> > };
> > +static const unsigned int vin2_data8g_pins[] = {
> > + RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
> > + RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 10),
> > + RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> > + RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> > +};
> > +static const unsigned int vin2_data8g_mux[] = {
> > + VI2_G0_MARK, VI2_G1_MARK,
> > + VI2_G2_MARK, VI2_G3_MARK,
> > + VI2_G4_MARK, VI2_G5_MARK,
> > + VI2_G6_MARK, VI2_G7_MARK,
> > +};
>
> Laurent, Niklas: are you happy with the name "vin2_data8g", or do
> you have a better suggestion?
I learnt recently that traditionally for single 8-bit RAW inputs are
named R8 (fist in RGB). But as this really is the G pins and they are
labeled as such I'm OK with the name, but I would like to hear Laurent's
view as well.
>
> > static const unsigned int vin2_sync_pins[] = {
> > RCAR_GP_PIN(1, 16), /* HSYNC */
> > RCAR_GP_PIN(1, 21), /* VSYNC */
>
> > @@ -4310,15 +4402,25 @@ static const struct {
> > VIN_DATA_PIN_GROUP(vin1_data, 10),
> > VIN_DATA_PIN_GROUP(vin1_data, 8),
> > VIN_DATA_PIN_GROUP(vin1_data, 4),
> > + VIN_DATA_PIN_GROUP(vin1_data, 24, _b),
> > + VIN_DATA_PIN_GROUP(vin1_data, 20, _b),
> > + SH_PFC_PIN_GROUP(vin1_data18_b),
> > + VIN_DATA_PIN_GROUP(vin1_data, 16, _b),
> > + VIN_DATA_PIN_GROUP(vin1_data, 12, _b),
> > + VIN_DATA_PIN_GROUP(vin1_data, 10, _b),
> > + VIN_DATA_PIN_GROUP(vin1_data, 8, _b),
>
> Missing vin1_data4_b.
>
> > SH_PFC_PIN_GROUP(vin1_sync),
> > + SH_PFC_PIN_GROUP(vin1_sync_b),
> > SH_PFC_PIN_GROUP(vin1_field),
> > SH_PFC_PIN_GROUP(vin1_clkenb),
> > SH_PFC_PIN_GROUP(vin1_clk),
> > + SH_PFC_PIN_GROUP(vin1_clk_b),
> > VIN_DATA_PIN_GROUP(vin2_data, 24),
> > SH_PFC_PIN_GROUP(vin2_data18),
> > VIN_DATA_PIN_GROUP(vin2_data, 16),
> > VIN_DATA_PIN_GROUP(vin2_data, 8),
> > VIN_DATA_PIN_GROUP(vin2_data, 4),
> > + SH_PFC_PIN_GROUP(vin2_data8g),
> > SH_PFC_PIN_GROUP(vin2_sync),
> > SH_PFC_PIN_GROUP(vin2_field),
> > SH_PFC_PIN_GROUP(vin2_clkenb),
> > @@ -4784,10 +4886,19 @@ static const char * const vin1_groups[] = {
> > "vin1_data10",
> > "vin1_data8",
> > "vin1_data4",
> > + "vin1_data24_b",
> > + "vin1_data20_b",
> > + "vin1_data18_b",
> > + "vin1_data16_b",
> > + "vin1_data12_b",
> > + "vin1_data10_b",
> > + "vin1_data8_b",
>
> Missing vin1_data4_b.
>
> > "vin1_sync",
> > + "vin1_sync_b",
> > "vin1_field",
> > "vin1_clkenb",
> > "vin1_clk",
> > + "vin1_clk_b",
> > };
> >
> > static const char * const vin2_groups[] = {
>
> The rest looks good to me.
>
> 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
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions
2020-09-14 23:27 ` Niklas Söderlund
@ 2020-09-14 23:40 ` Laurent Pinchart
2020-09-15 7:03 ` Geert Uytterhoeven
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2020-09-14 23:40 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Geert Uytterhoeven, Lad Prabhakar, Linus Walleij, Linux-Renesas,
open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Biju Das,
Prabhakar
Hello,
On Tue, Sep 15, 2020 at 01:27:56AM +0200, Niklas Söderlund wrote:
> On 2020-09-14 16:47:27 +0200, Geert Uytterhoeven wrote:
> > On Sun, Sep 13, 2020 at 8:29 PM Lad Prabhakar wrote:
> > > Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > Changes for v2:
> > > * Added complete list of VIN1-B pins
> > > * Renamed vin2_data8_g to vin2_data8g
> > > * Sorted vin1_sync_b pins
> > >
> > > v1 - https://patchwork.kernel.org/patch/11761191/
> >
> > Thanks for the update!
> >
> > > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >
> > > @@ -3874,6 +3940,14 @@ static const unsigned int vin1_sync_mux[] = {
> > > VI1_HSYNC_N_MARK,
> > > VI1_VSYNC_N_MARK,
> > > };
> > > +static const unsigned int vin1_sync_b_pins[] = {
> > > + RCAR_GP_PIN(1, 24), /* HSYNC */
> > > + RCAR_GP_PIN(1, 25), /* VSYNC */
> > > +};
> > > +static const unsigned int vin1_sync_b_mux[] = {
> > > + VI1_HSYNC_N_B_MARK,
> > > + VI1_VSYNC_N_B_MARK,
> > > +};
> > > static const unsigned int vin1_field_pins[] = {
> > > RCAR_GP_PIN(1, 13),
> > > };
> >
> > Missing field_b and clkenb_b.
> >
> > > @@ -3959,6 +4039,18 @@ static const unsigned int vin2_data18_mux[] = {
> > > VI2_R4_MARK, VI2_R5_MARK,
> > > VI2_R6_MARK, VI2_R7_MARK,
> > > };
> > > +static const unsigned int vin2_data8g_pins[] = {
> > > + RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
> > > + RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 10),
> > > + RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> > > + RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> > > +};
> > > +static const unsigned int vin2_data8g_mux[] = {
> > > + VI2_G0_MARK, VI2_G1_MARK,
> > > + VI2_G2_MARK, VI2_G3_MARK,
> > > + VI2_G4_MARK, VI2_G5_MARK,
> > > + VI2_G6_MARK, VI2_G7_MARK,
> > > +};
> >
> > Laurent, Niklas: are you happy with the name "vin2_data8g", or do
> > you have a better suggestion?
>
> I learnt recently that traditionally for single 8-bit RAW inputs are
> named R8 (fist in RGB). But as this really is the G pins and they are
> labeled as such I'm OK with the name, but I would like to hear Laurent's
> view as well.
I think we should match the pin names from the datasheet, so a R suffix
isn't a good option. I have a feeling we will suffer with this though,
as here 'g' refers to the VIN green data pins (g[7:0], a.k.a.
data[15:8]), while below 'b' refers to the second set of VIN data pins,
not the blue data pins. One option would be to use "vin2_data8_shift8",
but I'm not sure I'm very fond of that either. I also wonder whether we
shouldn't call this "vin2_g8" instead of "vin2_data8g" as the pins are
named VIN_G[7:0], not VIN_DATAG[7:0].
> > > static const unsigned int vin2_sync_pins[] = {
> > > RCAR_GP_PIN(1, 16), /* HSYNC */
> > > RCAR_GP_PIN(1, 21), /* VSYNC */
> >
> > > @@ -4310,15 +4402,25 @@ static const struct {
> > > VIN_DATA_PIN_GROUP(vin1_data, 10),
> > > VIN_DATA_PIN_GROUP(vin1_data, 8),
> > > VIN_DATA_PIN_GROUP(vin1_data, 4),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 24, _b),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 20, _b),
> > > + SH_PFC_PIN_GROUP(vin1_data18_b),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 16, _b),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 12, _b),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 10, _b),
> > > + VIN_DATA_PIN_GROUP(vin1_data, 8, _b),
> >
> > Missing vin1_data4_b.
> >
> > > SH_PFC_PIN_GROUP(vin1_sync),
> > > + SH_PFC_PIN_GROUP(vin1_sync_b),
> > > SH_PFC_PIN_GROUP(vin1_field),
> > > SH_PFC_PIN_GROUP(vin1_clkenb),
> > > SH_PFC_PIN_GROUP(vin1_clk),
> > > + SH_PFC_PIN_GROUP(vin1_clk_b),
> > > VIN_DATA_PIN_GROUP(vin2_data, 24),
> > > SH_PFC_PIN_GROUP(vin2_data18),
> > > VIN_DATA_PIN_GROUP(vin2_data, 16),
> > > VIN_DATA_PIN_GROUP(vin2_data, 8),
> > > VIN_DATA_PIN_GROUP(vin2_data, 4),
> > > + SH_PFC_PIN_GROUP(vin2_data8g),
> > > SH_PFC_PIN_GROUP(vin2_sync),
> > > SH_PFC_PIN_GROUP(vin2_field),
> > > SH_PFC_PIN_GROUP(vin2_clkenb),
> > > @@ -4784,10 +4886,19 @@ static const char * const vin1_groups[] = {
> > > "vin1_data10",
> > > "vin1_data8",
> > > "vin1_data4",
> > > + "vin1_data24_b",
> > > + "vin1_data20_b",
> > > + "vin1_data18_b",
> > > + "vin1_data16_b",
> > > + "vin1_data12_b",
> > > + "vin1_data10_b",
> > > + "vin1_data8_b",
> >
> > Missing vin1_data4_b.
> >
> > > "vin1_sync",
> > > + "vin1_sync_b",
> > > "vin1_field",
> > > "vin1_clkenb",
> > > "vin1_clk",
> > > + "vin1_clk_b",
> > > };
> > >
> > > static const char * const vin2_groups[] = {
> >
> > The rest looks good to me.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions
2020-09-14 23:40 ` Laurent Pinchart
@ 2020-09-15 7:03 ` Geert Uytterhoeven
0 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-09-15 7:03 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Niklas Söderlund, Lad Prabhakar, Linus Walleij,
Linux-Renesas, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, Biju Das, Prabhakar
Hi Laurent,
On Tue, Sep 15, 2020 at 1:40 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tue, Sep 15, 2020 at 01:27:56AM +0200, Niklas Söderlund wrote:
> > On 2020-09-14 16:47:27 +0200, Geert Uytterhoeven wrote:
> > > On Sun, Sep 13, 2020 at 8:29 PM Lad Prabhakar wrote:
> > > > Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > ---
> > > > Changes for v2:
> > > > * Added complete list of VIN1-B pins
> > > > * Renamed vin2_data8_g to vin2_data8g
> > > > * Sorted vin1_sync_b pins
> > > >
> > > > v1 - https://patchwork.kernel.org/patch/11761191/
> > > > @@ -3959,6 +4039,18 @@ static const unsigned int vin2_data18_mux[] = {
> > > > VI2_R4_MARK, VI2_R5_MARK,
> > > > VI2_R6_MARK, VI2_R7_MARK,
> > > > };
> > > > +static const unsigned int vin2_data8g_pins[] = {
> > > > + RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28),
> > > > + RCAR_GP_PIN(0, 29), RCAR_GP_PIN(1, 10),
> > > > + RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> > > > + RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> > > > +};
> > > > +static const unsigned int vin2_data8g_mux[] = {
> > > > + VI2_G0_MARK, VI2_G1_MARK,
> > > > + VI2_G2_MARK, VI2_G3_MARK,
> > > > + VI2_G4_MARK, VI2_G5_MARK,
> > > > + VI2_G6_MARK, VI2_G7_MARK,
> > > > +};
> > >
> > > Laurent, Niklas: are you happy with the name "vin2_data8g", or do
> > > you have a better suggestion?
> >
> > I learnt recently that traditionally for single 8-bit RAW inputs are
> > named R8 (fist in RGB). But as this really is the G pins and they are
> > labeled as such I'm OK with the name, but I would like to hear Laurent's
> > view as well.
>
> I think we should match the pin names from the datasheet, so a R suffix
> isn't a good option. I have a feeling we will suffer with this though,
> as here 'g' refers to the VIN green data pins (g[7:0], a.k.a.
> data[15:8]), while below 'b' refers to the second set of VIN data pins,
> not the blue data pins. One option would be to use "vin2_data8_shift8",
> but I'm not sure I'm very fond of that either. I also wonder whether we
> shouldn't call this "vin2_g8" instead of "vin2_data8g" as the pins are
> named VIN_G[7:0], not VIN_DATAG[7:0].
On R-Car H2 and RZ/G1H they're indeed named VIx_G[7:0].
However, looking at other R-Car Gen2 and Gen3 variants, there are
many possibilities, from all-RGB:
1. R[7:0], G[7:0], B[7:0],
over:
2. R[7:0], G[7:0], DATA[7:0]_B[7:0],
3. D[23:16]_R[7:0], D[15:8]_G[7:0]_Y[7:0], D[7:0]_B[7:0]_C[7:0],
to all-DATA:
4. DATA[11:0],
5. DATA[23:0].
Following 1, 24-bit should be called "rgb24", and 18-bit "rgb18"
(I believe this is the only format using discontiguous pins?).
The in-betweens make sense, as YCbCr[7:0] data goes over the 8-bit DATA
or 16-bit D pins, but that sense is lost when considering other formats
that accept 10/12/16/20-bit input.
I guess that's why R-Car Gen3 settled at option 5, which is actually
what we've been doing in the pin control drivers from the beginning.
Nevertheless, I agree "vinX_g8" seems like the best name for this group,
as it's quite obvious from the name what it means, and isn't easily
confused with an alternate set of pins.
Note that the BSP (for R-Car Gen3, no idea about Gen2) uses
"vinX_data8_sft8" (which I never really liked), which Niklas is now
trying to upstream in "[PATCH 0/4] pinctrl: sh-pfc: Add VIN stf8 pins"
(https://lore.kernel.org/linux-renesas-soc/20200914233744.468175-1-niklas.soderlund+renesas@ragnatech.se).
Expect more bikeshedding soon ;-)
> > > > static const unsigned int vin2_sync_pins[] = {
> > > > RCAR_GP_PIN(1, 16), /* HSYNC */
> > > > RCAR_GP_PIN(1, 21), /* VSYNC */
> > >
> > > > @@ -4310,15 +4402,25 @@ static const struct {
> > > > VIN_DATA_PIN_GROUP(vin1_data, 10),
> > > > VIN_DATA_PIN_GROUP(vin1_data, 8),
> > > > VIN_DATA_PIN_GROUP(vin1_data, 4),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 24, _b),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 20, _b),
> > > > + SH_PFC_PIN_GROUP(vin1_data18_b),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 16, _b),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 12, _b),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 10, _b),
> > > > + VIN_DATA_PIN_GROUP(vin1_data, 8, _b),
> > >
> > > Missing vin1_data4_b.
> > >
> > > > SH_PFC_PIN_GROUP(vin1_sync),
> > > > + SH_PFC_PIN_GROUP(vin1_sync_b),
> > > > SH_PFC_PIN_GROUP(vin1_field),
> > > > SH_PFC_PIN_GROUP(vin1_clkenb),
> > > > SH_PFC_PIN_GROUP(vin1_clk),
> > > > + SH_PFC_PIN_GROUP(vin1_clk_b),
> > > > VIN_DATA_PIN_GROUP(vin2_data, 24),
> > > > SH_PFC_PIN_GROUP(vin2_data18),
> > > > VIN_DATA_PIN_GROUP(vin2_data, 16),
> > > > VIN_DATA_PIN_GROUP(vin2_data, 8),
> > > > VIN_DATA_PIN_GROUP(vin2_data, 4),
> > > > + SH_PFC_PIN_GROUP(vin2_data8g),
> > > > SH_PFC_PIN_GROUP(vin2_sync),
> > > > SH_PFC_PIN_GROUP(vin2_field),
> > > > SH_PFC_PIN_GROUP(vin2_clkenb),
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-15 7:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-13 18:28 [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins, groups and functions Lad Prabhakar
2020-09-14 14:47 ` Geert Uytterhoeven
2020-09-14 23:27 ` Niklas Söderlund
2020-09-14 23:40 ` Laurent Pinchart
2020-09-15 7:03 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).