| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200914234000.GG15543@pendragon.ideasonboard.com>
Date: Tue, 15 Sep 2020 02:40:00 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Linus Walleij <linus.walleij@...aro.org>,
Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Biju Das <biju.das.jz@...renesas.com>,
Prabhakar <prabhakar.csengg@...il.com>
Subject: Re: [PATCH v2] pinctrl: sh-pfc: r8a7790: Add VIN1-B and VIN2-G pins,
groups and functions
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@...renesas.com>
> > > Reviewed-by: Biju Das <biju.das.jz@...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
Powered by blists - more mailing lists