Re: [PATCH v3 10/21] pinctrl: renesas: r8a779g0: add missing IRQx_A/IRQx_B

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Morimoto-san,

On Tue, Jun 14, 2022 at 7:59 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>
> This patch adds missing IRQx_A/IRQx_B
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pfc-r8a779g0.c
> +++ b/drivers/pinctrl/renesas/pfc-r8a779g0.c
> @@ -278,7 +278,7 @@
>  #define IP1SR0_11_8    FM(MSIOF5_TXD)          F_(0, 0)                F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
>  #define IP1SR0_15_12   FM(MSIOF5_SCK)          F_(0, 0)                F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
>  #define IP1SR0_19_16   FM(MSIOF5_RXD)          F_(0, 0)                F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> -#define IP1SR0_23_20   FM(MSIOF2_SS2)          FM(TCLK1)               F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
> +#define IP1SR0_23_20   FM(MSIOF2_SS2)          FM(TCLK1)               FM(IRQ2_A)      F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
>  #define IP1SR0_27_24   FM(MSIOF2_SS1)          FM(HTX1)                FM(TX1)         F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
>  #define IP1SR0_31_28   FM(MSIOF2_SYNC)         FM(HRX1)                FM(RX1)         F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
>
> @@ -313,10 +313,10 @@
>  #define IP2SR1_7_4     FM(SCIF_CLK)            FM(IRQ4)                F_(0, 0)        F_(0, 0)        F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)

"IRQ4" here should be "IRQ4_A", also in the corresponding
PINMUX_IPSR_GPSR() definition.  Probably this should be fixed in
"[PATCH v3 03/21] pinctrl: renesas: Initial R8A779G0 (V4H) PFC
support"?

The rest LGTM, as it matches the documentation, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

However, given the inconsistent naming of pins for IRQs with two
possible pin alternatives:
  - IRQ2 and IRQ2_A,
  - IRQ3 and IRQ3_B,
  - IRQ4_A and IRQ4_B,
I expect several of these to be renamed in future revisions of the
documentation.  As these names become part of the DT ABI when defining
pin groups, that happens sooner rather than later...

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