Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Biju Das <[email protected]>
---
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
Hi Prabhakar,
CC Laurent, Niklas
On Sun, Sep 13, 2020 at 8:29 PM Lad Prabhakar
<[email protected]> wrote:
> Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Biju Das <[email protected]>
> ---
> 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 -- [email protected]
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
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
> <[email protected]> wrote:
> > Add pins, groups and functions for the VIN1-B [data/clk/sync] and VIN2-G [data].
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Biju Das <[email protected]>
> > ---
> > 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 -- [email protected]
>
> 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
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 <[email protected]>
> > > Reviewed-by: Biju Das <[email protected]>
> > > ---
> > > 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
Hi Laurent,
On Tue, Sep 15, 2020 at 1:40 AM Laurent Pinchart
<[email protected]> 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 <[email protected]>
> > > > Reviewed-by: Biju Das <[email protected]>
> > > > ---
> > > > 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/[email protected]).
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 -- [email protected]
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