From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] pinctrl: renesas: r8a779g0: Add pins, groups and functions
Date: Thu, 9 Jun 2022 18:30:48 +0200 [thread overview]
Message-ID: <CAMuHMdUeRtBYp_B_EKd4UkVoUQeOqwAuBCdSeaiiFJdhYS6_nQ@mail.gmail.com> (raw)
In-Reply-To: <87tu8xgu1f.wl-kuninori.morimoto.gx@renesas.com>
Hi Morimoto-san,
On Tue, Jun 7, 2022 at 3:08 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Phong Hoang <phong.hoang.wz@renesas.com>
>
> This patch adds SCIF, I2C, EthernetAVB, HSCIF, MMC, QSPI,
> MSIOF, PWM, CAN-FD, Ethernet-TSN, PCIe pins, groups, and functions
>
> [Morimoto merged above patches into one]
> Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
> Signed-off-by: Hai Pham <hai.pham.ud@renesas.com>
> Signed-off-by: Thanh Quan <thanh.quan.xn@renesas.com>
> Signed-off-by: CongDang <cong.dang.xn@renesas.com>
> Signed-off-by: Kazuya Mizuguch <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Tho Vu <tho.vu.wh@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Thanks for your patch!
> --- a/drivers/pinctrl/renesas/pfc-r8a779g0.c
> +++ b/drivers/pinctrl/renesas/pfc-r8a779g0.c
> +/* - CANFD5 ----------------------------------------------------------------- */
> +static const unsigned int canfd5_data_pins[] = {
> + /* CANFD5_TX, CANFD5_RX */
> + RCAR_GP_PIN(2, 2), RCAR_GP_PIN(2, 3),
> +};
> +static const unsigned int canfd5_data_mux[] = {
> + CANFD5_TX_MARK, CANFD5_RX_MARK,
> +};
Missing CANFD5 B on GP1_[89].
> +/* - HSCIF1 ----------------------------------------------------------------- */
> +static const unsigned int hscif1_data_pins[] = {
> + /* HRX1, HTX1 */
> + RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14),
> +};
> +static const unsigned int hscif1_data_mux[] = {
> + HRX1_MARK, HTX1_MARK,
> +};
> +static const unsigned int hscif1_clk_pins[] = {
> + /* HSCK1 */
> + RCAR_GP_PIN(0, 18),
> +};
> +static const unsigned int hscif1_clk_mux[] = {
> + HSCK1_MARK,
> +};
> +static const unsigned int hscif1_ctrl_pins[] = {
> + /* HRTS1#, HCTS1# */
> + RCAR_GP_PIN(0, 17), RCAR_GP_PIN(0, 16),
> +};
> +static const unsigned int hscif1_ctrl_mux[] = {
> + HRTS1_N_MARK, HCTS1_N_MARK,
> +};
Missing alternative set on GP1_{0[6-7],10}.
How to name it? The CANFD5 alternative set is called "B", the HSCIF3
alternative set is called "A"?
> +/* - HSCIF3 ----------------------------------------------------------------- */
> +static const unsigned int hscif3_data_pins[] = {
> + /* HRX3, HTX3 */
> + RCAR_GP_PIN(1, 24), RCAR_GP_PIN(1, 28),
> +};
> +static const unsigned int hscif3_data_mux[] = {
> + HRX3_MARK, HTX3_MARK,
> +};
> +static const unsigned int hscif3_clk_pins[] = {
> + /* HSCK3 */
> + RCAR_GP_PIN(1, 25),
> +};
> +static const unsigned int hscif3_clk_mux[] = {
> + HSCK3_MARK,
> +};
> +static const unsigned int hscif3_ctrl_pins[] = {
> + /* HRTS3#, HCTS3# */
> + RCAR_GP_PIN(1, 26), RCAR_GP_PIN(1, 27),
> +};
> +static const unsigned int hscif3_ctrl_mux[] = {
> + HRTS3_N_MARK, HCTS3_N_MARK,
> +};
Missing HSCIF3 A on GP1_0[0-4].
> +static const unsigned int mmc_ds_mux[] = {
> + MMC_DS_MARK,
> +};
> +
> +/* - QSPI0 ------------------------------------------------------------------ */
General comment (also applies below): please sort modules
alphabetically.
[...]
> +/* - MSIOF0 ----------------------------------------------------------------- */
> +/* - MSIOF2 ----------------------------------------------------------------- */
> +static const unsigned int msiof2_clk_pins[] = {
> + /* MSIOF2_SCK */
> + RCAR_GP_PIN(0, 17),
> +};
> +static const unsigned int msiof2_clk_mux[] = {
> + MSIOF2_SCK_MARK,
> +};
> +static const unsigned int msiof2_sync_pins[] = {
> + /* MSIOF2_SYNC */
> + RCAR_GP_PIN(0, 15),
> +};
> +static const unsigned int msiof2_sync_mux[] = {
> + MSIOF2_SYNC_MARK,
> +};
> +static const unsigned int msiof2_ss1_pins[] = {
> + /* MSIOF2_SS1 */
> + RCAR_GP_PIN(0, 14),
> +};
> +static const unsigned int msiof2_ss1_mux[] = {
> + MSIOF2_SS1_MARK,
> +};
> +static const unsigned int msiof2_ss2_pins[] = {
> + /* MSIOF1_SS2 */
MSIOF2_SS2
> + RCAR_GP_PIN(0, 13),
> +};
> +static const unsigned int msiof2_ss2_mux[] = {
> + MSIOF2_SS2_MARK,
> +};
> +static const unsigned int msiof2_txd_pins[] = {
> + /* MSIOF1_TXD */
MSIOF2_TXD
> + RCAR_GP_PIN(0, 16),
> +};
> +static const unsigned int msiof2_txd_mux[] = {
> + MSIOF2_TXD_MARK,
> +};
> +static const unsigned int msiof2_rxd_pins[] = {
> + /* MSIOF1_RXD */
MSIOF2_RXD
> + RCAR_GP_PIN(0, 18),
> +};
> +static const unsigned int msiof2_rxd_mux[] = {
> + MSIOF2_RXD_MARK,
> +};
> +/* - PWM0 ------------------------------------------------------------------- */
> +static const unsigned int pwm0_pins[] = {
> + /* PWM0 */
> + RCAR_GP_PIN(1, 15),
How confusing, the datasheet calls this PWM0_A, but there is no PWM0_B.
> +};
> +static const unsigned int pwm0_mux[] = {
> + PWM0_MARK,
> +};
> +
> +/* - PWM1 ------------------------------------------------------------------- */
> +static const unsigned int pwm1_pins[] = {
> + /* PWM1 */
> + RCAR_GP_PIN(2, 13),
This is PWM1_B, and PWM1_A is on GP3_13.
> +};
> +static const unsigned int pwm1_mux[] = {
> + PWM1_MARK,
> +};
> +
> +/* - PWM2 ------------------------------------------------------------------- */
> +static const unsigned int pwm2_pins[] = {
> + /* PWM2 */
> + RCAR_GP_PIN(2, 14),
This is PWM2_B, and PWM2_A is on GP3_14.
> +};
> +static const unsigned int pwm2_mux[] = {
> + PWM2_MARK,
> +};
> +
> +/* - PWM3 ------------------------------------------------------------------- */
> +static const unsigned int pwm3_pins[] = {
> + /* PWM3 */
> + RCAR_GP_PIN(1, 22),
This is PWM3_A, and PWM3_B is on GP2_15.
> +};
> +static const unsigned int pwm3_mux[] = {
> + PWM3_MARK,
> +};
> +
> +/* - PWM4 ------------------------------------------------------------------- */
> +static const unsigned int pwm4_pins[] = {
> + /* PWM4 */
> + RCAR_GP_PIN(2, 16),
> +};
> +static const unsigned int pwm4_mux[] = {
> + PWM4_MARK,
> +};
> +
> +/* - PWM5 ------------------------------------------------------------------- */
> +static const unsigned int pwm5_pins[] = {
> + /* PWM5 */
> + RCAR_GP_PIN(2, 17),
> +};
> +static const unsigned int pwm5_mux[] = {
> + PWM5_MARK,
> +};
> +
> +/* - PWM6 ------------------------------------------------------------------- */
> +static const unsigned int pwm6_pins[] = {
> + /* PWM6 */
> + RCAR_GP_PIN(2, 18),
> +};
> +static const unsigned int pwm6_mux[] = {
> + PWM6_MARK,
> +};
> +
> +/* - PWM7 ------------------------------------------------------------------- */
> +static const unsigned int pwm7_pins[] = {
> + /* PWM7 */
> + RCAR_GP_PIN(2, 19),
> +};
> +static const unsigned int pwm7_mux[] = {
> + PWM7_MARK,
> +};
> +
> +/* - PWM8 ------------------------------------------------------------------- */
> +static const unsigned int pwm8_pins[] = {
> + /* PWM8 */
> + RCAR_GP_PIN(1, 13),
The datasheet calls this PWM8_A (but there is no PWM8_B?).
> +};
> +static const unsigned int pwm8_mux[] = {
> + PWM8_MARK,
> +};
> +
> +/* - PWM9 ------------------------------------------------------------------- */
> +static const unsigned int pwm9_pins[] = {
> + /* PWM9 */
> + RCAR_GP_PIN(1, 14),
The datasheet calls this PWM9_A (but there is no PWM9_B?).
> +};
> +static const unsigned int pwm9_mux[] = {
> + PWM9_MARK,
> +};
> +
> +/* - TSN0 ------------------------------------------------ */
> +static const unsigned int tsn0_link_pins[] = {
> + /* TSN0_LINK */
> + RCAR_GP_PIN(4, 4),
> +};
> +static const unsigned int tsn0_link_mux[] = {
> + TSN0_LINK_MARK,
> +};
> +static const unsigned int tsn0_phy_int_pins[] = {
> + /* TSN0_PHY_INT */
> + RCAR_GP_PIN(4, 3),
> +};
> +static const unsigned int tsn0_phy_int_mux[] = {
> + TSN0_PHY_INT_MARK,
> +};
> +static const unsigned int tsn0_mdio_pins[] = {
> + /* TSN0_MDC, TSN0_MDIO */
> + RCAR_GP_PIN(4, 1), RCAR_GP_PIN(4, 0),
> +};
> +static const unsigned int tsn0_mdio_mux[] = {
> + TSN0_MDC_MARK, TSN0_MDIO_MARK,
> +};
> +static const unsigned int tsn0_rgmii_pins[] = {
> + /*
> + * TSN0_TX_CTL, TSN0_TXC, TSN0_TD0, TSN0_TD1, TSN0_TD2, TSN0_TD3,
> + * TSN0_RX_CTL, TSN0_RXC, TSN0_RD0, TSN0_RD1, TSN0_RD2, TSN0_RD3,
> + */
> + RCAR_GP_PIN(4, 9), RCAR_GP_PIN(4, 12),
> + RCAR_GP_PIN(4, 15), RCAR_GP_PIN(4, 14),
> + RCAR_GP_PIN(4, 19), RCAR_GP_PIN(4, 18),
> + RCAR_GP_PIN(4, 7), RCAR_GP_PIN(4, 11),
> + RCAR_GP_PIN(4, 10), RCAR_GP_PIN(4, 13),
> + RCAR_GP_PIN(4, 17), RCAR_GP_PIN(4, 16),
> +};
> +static const unsigned int tsn0_rgmii_mux[] = {
> + TSN0_TX_CTL_MARK, TSN0_TXC_MARK,
> + TSN0_TD0_MARK, TSN0_TD1_MARK,
> + TSN0_TD2_MARK, TSN0_TD3_MARK,
> + TSN0_RX_CTL_MARK, TSN0_RXC_MARK,
> + TSN0_RD0_MARK, TSN0_RD1_MARK,
> + TSN0_RD2_MARK, TSN0_RD3_MARK,
> +};
> +static const unsigned int tsn0_txcrefclk_pins[] = {
> + /* TSN0_TXCREFCLK */
> + RCAR_GP_PIN(4, 20),
> +};
> +static const unsigned int tsn0_txcrefclk_mux[] = {
> + TSN0_TXCREFCLK_MARK,
> +};
> +static const unsigned int tsn0_avtp_pps_pins[] = {
> + /* TSN0_AVTP_PPS0, TSN0_AVTP_PPS1 */
> + RCAR_GP_PIN(4, 8), RCAR_GP_PIN(4, 2),
The documentation is not very clear on how these are to be used.
Is there a use case where you need only one of PPS0 and PPS1?
If yes, this group should be split.
> +};
> +static const unsigned int tsn0_avtp_pps_mux[] = {
> + TSN0_AVTP_PPS0_MARK, TSN0_AVTP_PPS1_MARK,
> +};
> +static const unsigned int tsn0_avtp_capture_pins[] = {
> + /* TSN0_AVTP_CAPTURE */
> + RCAR_GP_PIN(4, 6),
> +};
> +static const unsigned int tsn0_avtp_capture_mux[] = {
> + TSN0_AVTP_CAPTURE_MARK,
> +};
> +static const unsigned int tsn0_avtp_match_pins[] = {
> + /* TSN0_AVTP_MATCH */
> + RCAR_GP_PIN(4, 5),
> +};
> +static const unsigned int tsn0_avtp_match_mux[] = {
> + TSN0_AVTP_MATCH_MARK,
> +};
Missing blank line.
> +/* - TPU ------------------------------------------------------------------- */
> +static const unsigned int tpu_to0_pins[] = {
> + /* TPU0TO0 */
> + RCAR_GP_PIN(2, 8),
> +};
> +static const unsigned int tpu_to0_mux[] = {
> + TPU0TO0_MARK,
> +};
> +static const unsigned int tpu_to1_pins[] = {
> + /* TPU0TO1 */
> + RCAR_GP_PIN(2, 7),
> +};
> +static const unsigned int tpu_to1_mux[] = {
> + TPU0TO1_MARK,
> +};
> +static const unsigned int tpu_to2_pins[] = {
> + /* TPU0TO2 */
> + RCAR_GP_PIN(2, 12),
> +};
> +static const unsigned int tpu_to2_mux[] = {
> + TPU0TO2_MARK,
> +};
> +static const unsigned int tpu_to3_pins[] = {
> + /* TPU0TO3 */
> + RCAR_GP_PIN(2, 13),
> +};
> +static const unsigned int tpu_to3_mux[] = {
> + TPU0TO3_MARK,
> +};
Missing A pins on GP2_0[01] and GP1_2[56].
> +/* - SCIF1 ------------------------------------------------------------------ */
> +static const unsigned int scif1_data_pins[] = {
> + /* RX, TX */
RX1, TX1
> + RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14),
> +};
> +static const unsigned int scif1_data_mux[] = {
> + RX1_MARK, TX1_MARK,
> +};
> +static const unsigned int scif1_clk_pins[] = {
> + /* SCK1 */
> + RCAR_GP_PIN(0, 18),
> +};
> +static const unsigned int scif1_clk_mux[] = {
> + SCK1_MARK,
> +};
> +static const unsigned int scif1_ctrl_pins[] = {
> + /* RTS1#, CTS1# */
> + RCAR_GP_PIN(0, 17), RCAR_GP_PIN(0, 16),
> +};
> +static const unsigned int scif1_ctrl_mux[] = {
> + RTS1_N_MARK, CTS1_N_MARK,
> +};
Missing alternative set on GP1_{0[6-7],10}.
> +
> +/* - SCIF3 ------------------------------------------------------------------ */
> +static const unsigned int scif3_data_pins[] = {
> + /* RX3, TX3 */
> + RCAR_GP_PIN(1, 1), RCAR_GP_PIN(1, 0),
> +};
> +static const unsigned int scif3_data_mux[] = {
> + RX3_MARK, TX3_MARK,
> +};
> +static const unsigned int scif3_clk_pins[] = {
> + /* SCK3 */
> + RCAR_GP_PIN(1, 4),
> +};
> +static const unsigned int scif3_clk_mux[] = {
> + SCK3_MARK,
> +};
> +static const unsigned int scif3_ctrl_pins[] = {
> + /* RTS3#, CTS3# */
> + RCAR_GP_PIN(1, 2), RCAR_GP_PIN(1, 3),
> +};
> +static const unsigned int scif3_ctrl_mux[] = {
> + RTS3_N_MARK, CTS3_N_MARK,
> +};
Missing SCIF3_A on GP1_2[4-8].
> +
> +/* - SCIF4 ------------------------------------------------------------------ */
> +static const unsigned int scif4_data_pins[] = {
> + /* RX4, TX4 */
> + RCAR_GP_PIN(8, 13), RCAR_GP_PIN(8, 12),
> +};
> +static const unsigned int scif4_data_mux[] = {
> + RX4_MARK, TX4_MARK,
> +};
> +static const unsigned int scif4_clk_pins[] = {
> + /* SCK4 */
> + RCAR_GP_PIN(8, 8),
> +};
> +static const unsigned int scif4_clk_mux[] = {
> + SCK4_MARK,
> +};
> +static const unsigned int scif4_ctrl_pins[] = {
> + /* RTS4#, CTS4# */
> + RCAR_GP_PIN(8, 10), RCAR_GP_PIN(8, 9),
> +};
> +static const unsigned int scif4_ctrl_mux[] = {
> + RTS4_N_MARK, CTS4_N_MARK,
> +};
> +
> +/* - SCIF Clock ------------------------------------------------------------- */
> +static const unsigned int scif_clk_pins[] = {
> + /* SCIF_CLK */
> + RCAR_GP_PIN(1, 17),
> +};
> +static const unsigned int scif_clk_mux[] = {
> + SCIF_CLK_MARK,
> +};
Missing SCIF_CLK2 on GP8_11.
> +
> static const struct sh_pfc_pin_group pinmux_groups[] = {
> + SH_PFC_PIN_GROUP(msiof5_rxd),
> +
> + SH_PFC_PIN_GROUP(qspi0_ctrl),
> + BUS_DATA_PIN_GROUP(qspi0_data, 2),
> + BUS_DATA_PIN_GROUP(qspi0_data, 4),
> + SH_PFC_PIN_GROUP(qspi1_ctrl),
> + BUS_DATA_PIN_GROUP(qspi1_data, 2),
> + BUS_DATA_PIN_GROUP(qspi1_data, 4),
> +
> + SH_PFC_PIN_GROUP(pwm0),
Please sort modules alphabetically (more groups and functions
to sort below).
> + SH_PFC_PIN_GROUP(pwm1),
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
next prev parent reply other threads:[~2022-06-09 16:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-07 1:06 [PATCH v2 0/4] pinctrl: renesas: r8a779g0: Add pins, groups and functions Kuninori Morimoto
2022-06-07 1:06 ` [PATCH v2 1/4] dt-bindings: pinctrl: renesas,pfc: Document r8a779g0 support Kuninori Morimoto
2022-06-09 9:58 ` Geert Uytterhoeven
2022-06-07 1:07 ` [PATCH v2 2/4] pinctrl: renesas: Add PORT_GP_CFG_13 macros Kuninori Morimoto
2022-06-09 9:58 ` Geert Uytterhoeven
2022-06-07 1:07 ` [PATCH v2 3/4] pinctrl: renesas: Initial R8A779G0 (V4H) PFC support Kuninori Morimoto
2022-06-10 11:43 ` Geert Uytterhoeven
2022-06-10 15:57 ` Geert Uytterhoeven
2022-06-12 23:51 ` Kuninori Morimoto
2022-06-13 15:18 ` Geert Uytterhoeven
2022-06-13 23:23 ` Kuninori Morimoto
2022-06-13 23:34 ` Kuninori Morimoto
2022-06-14 6:55 ` Geert Uytterhoeven
2022-06-15 22:17 ` Kuninori Morimoto
2022-06-13 15:08 ` Geert Uytterhoeven
2022-06-07 1:07 ` [PATCH v2 4/4] pinctrl: renesas: r8a779g0: Add pins, groups and functions Kuninori Morimoto
2022-06-09 16:30 ` Geert Uytterhoeven [this message]
2022-06-09 23:28 ` Kuninori Morimoto
2022-06-10 15:53 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMuHMdUeRtBYp_B_EKd4UkVoUQeOqwAuBCdSeaiiFJdhYS6_nQ@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).