Re: [PATCH v3 03/21] pinctrl: renesas: Initial R8A779G0 (V4H) PFC support

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Morimoto-san,

On Tue, Jun 14, 2022 at 7:58 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: LUU HOAI <hoai.luu.ub@renesas.com>
>
> This patch adds initial pinctrl support for the R8A779G0 (V4H) SoC,
> including bias, drive strength and voltage control.
>
> [Morimoto merged Kihara-san's MODSEL8 fixup patch,
>  cleanuped white space, care reserved bit on each configs,
>  fixup comments, etc]
> Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pinctrl/renesas/pfc-r8a779g0.c

> +static const u16 pinmux_data[] = {

> +       /* IP0SR8 */
> +       PINMUX_IPSR_MSEL(IP0SR8_3_0,    SCL0,                   SEL_SCL0_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_7_4,    SDA0,                   SEL_SDA0_0),

These are definitely still wrong, and not fixed by any of your
subsequent patches: to enable I2C0 functionality, the corresponding
SEL_{SCL,SDA}0 bit should be set to 1, not 0.
Same for I2C[1-5] below.

> +       PINMUX_IPSR_MSEL(IP0SR8_11_8,   SCL1,                   SEL_SCL1_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_15_12,  SDA1,                   SEL_SDA1_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_19_16,  SCL2,                   SEL_SCL2_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_23_20,  SDA2,                   SEL_SDA2_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_27_24,  SCL3,                   SEL_SCL3_0),
> +       PINMUX_IPSR_MSEL(IP0SR8_31_28,  SDA3,                   SEL_SDA3_0),
> +
> +       /* IP1SR8 */
> +       PINMUX_IPSR_MSEL(IP1SR8_3_0,    SCL4,                   SEL_SCL4_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_3_0,    HRX2,                   SEL_SCL4_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_3_0,    SCK4,                   SEL_SCL4_0),
> +
> +       PINMUX_IPSR_MSEL(IP1SR8_7_4,    SDA4,                   SEL_SDA4_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_7_4,    HTX2,                   SEL_SDA4_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_7_4,    CTS4_N,                 SEL_SDA4_0),
> +
> +       PINMUX_IPSR_MSEL(IP1SR8_11_8,   SCL5,                   SEL_SCL5_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_11_8,   HRTS2_N,                SEL_SCL5_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_11_8,   RTS4_N,                 SEL_SCL5_0),
> +
> +       PINMUX_IPSR_MSEL(IP1SR8_15_12,  SDA5,                   SEL_SDA5_0),
> +       PINMUX_IPSR_MSEL(IP1SR8_15_12,  SCIF_CLK2,              SEL_SDA5_0),
> +
> +       PINMUX_IPSR_GPSR(IP1SR8_19_16,  HCTS2_N),
> +       PINMUX_IPSR_GPSR(IP1SR8_19_16,  TX4),
> +
> +       PINMUX_IPSR_GPSR(IP1SR8_23_20,  HSCK2),
> +       PINMUX_IPSR_GPSR(IP1SR8_23_20,  RX4),
> +};

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