Hi Niklas,
CC Laurent.
On Tue, Sep 15, 2020 at 1:38 AM Niklas Söderlund
<niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx>
>
> This patch adds VIN{4,5} pins, groups and functions to the R8A77951 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx>
> [Niklas: Rework to fit upstream driver]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
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@xxxxxxxxxxxxxx
> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
