All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org
Subject: Re: [PATCH v2 3/4] pinctrl: renesas: Initial R8A779G0 (V4H) PFC support
Date: Mon, 13 Jun 2022 17:08:47 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2206131708100.946510@ramsan.of.borg> (raw)
In-Reply-To: <87v8tdgu1t.wl-kuninori.morimoto.gx@renesas.com>

[-- Attachment #1: Type: text/plain, Size: 19422 bytes --]

 	Hi Morimoto-san,

On Tue, 7 Jun 2022, Kuninori Morimoto 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,
> and cleanuped white space, care reserved bit on each configs,
> fixup setting miss]
> 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!

Below is my second (and final) set of comments.

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

> +/* MOD_SEL4 */			/* 0 */				/* 1 */
> +#define MOD_SEL4_19		FM(SEL_TSN0_TD2_0)		FM(SEL_TSN0_TD2_1)
> +#define MOD_SEL4_18		FM(SEL_TSN0_TD3_0)		FM(SEL_TSN0_TD3_1)
> +#define MOD_SEL4_17		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_16		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_15		FM(SEL_TSN0_TD0_0)		FM(SEL_TSN0_TD0_1)
> +#define MOD_SEL4_14		FM(SEL_TSN0_TD1_0)		FM(SEL_TSN0_TD1_1)
> +#define MOD_SEL4_13		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_12		FM(SEL_TSN0_TXC_0)		FM(SEL_TSN0_TXC_1)
> +#define MOD_SEL4_11		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_10		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_9		FM(SEL_TSN0_TX_CTL_0)		FM(SEL_TSN0_TX_CTL_1)
> +#define MOD_SEL4_8		FM(SEL_TSN0_AVTP_PPS0_0)	FM(SEL_TSN0_AVTP_PPS0_1)
> +#define MOD_SEL4_7		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_6		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_5		FM(SEL_TSN0_AVTP_MATCH_0)	FM(SEL_TSN0_AVTP_MATCH_1)
> +#define MOD_SEL4_4		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_3		F_(0, 0)			F_(0, 0)
> +#define MOD_SEL4_2		FM(SEL_TSN0_AVTP_PPS1_0)	FM(SEL_TSN0_AVTP_PPS1_1)
> +#define MOD_SEL4_1		FM(SEL_TSN0_MDC_0)		FM(SEL_TSN0_MDC_1)
> +#define MOD_SEL4_0		F_(0, 0)			F_(0, 0)

Like for the IPxSRy_n_m() macros, if the definition of a MOD_SELn_m()
macro consists of a series of "F_(0, 0)" only, you can just omit that
definition, and omit its use in the definition of the PINMUX_MOD_SELS()
macro below.

Note that you already omitted MOD_SEL4_n for n > 19.

> +static const u16 pinmux_data[] = {
> +	PINMUX_DATA_GP_ALL(),

Given the inset for I2C selection in Figure 7.1 ("PFC Block Diagram"),
and the documentation for the MODSEL8 register bits in Rev. 0.51 of
the R-Car V4H Series Hardware User’s Manual, I think you need to
override GP_8_[0-12]_FN here to use the GPIO function on I2C-capable
pins.  See also commits 4288caed9a6319b7 ("pinctrl: renesas: r8a779a0:
Fix GPIO function on I2C-capable pins") and 8bdd369dba7ff2f8 ("pinctrl:
renesas: r8a779f0: Fix GPIO function on I2C-capable pins").

And probably you need something similar to configure MODSEL[4567] when
using TSN0 or AVB[012] pins as GPIOs, or when using CC5_OSCOUT?

> +
> +	PINMUX_SINGLE(AVS1),
> +	PINMUX_SINGLE(AVS0),
> +	PINMUX_SINGLE(PCIE1_CLKREQ_N),
> +	PINMUX_SINGLE(PCIE0_CLKREQ_N),
> +	PINMUX_SINGLE(TSN0_TXCREFCLK),
> +	PINMUX_SINGLE(TSN0_TD2),

As using TSN0_{TD[0-3],TXC,TX_CTL,AVTP_PPS[01],AVTP_MATCH,MDC}
needs configuration in MODSEL4, you should use PINMUX_IPSR_NOGM()
instead.

E.g. PINMUX_IPSR_NOGM(0, TSN0_TD2, SEL_TSN0_TD2_1).

> +	PINMUX_SINGLE(TSN0_TD3),
> +	PINMUX_SINGLE(TSN0_RD2),
> +	PINMUX_SINGLE(TSN0_RD3),
> +	PINMUX_SINGLE(TSN0_TD0),
> +	PINMUX_SINGLE(TSN0_TD1),
> +	PINMUX_SINGLE(TSN0_RD1),
> +	PINMUX_SINGLE(TSN0_TXC),
> +	PINMUX_SINGLE(TSN0_RXC),
> +	PINMUX_SINGLE(TSN0_RD0),
> +	PINMUX_SINGLE(TSN0_TX_CTL),
> +	PINMUX_SINGLE(TSN0_AVTP_PPS0),
> +	PINMUX_SINGLE(TSN0_RX_CTL),
> +	PINMUX_SINGLE(TSN0_AVTP_CAPTURE),
> +	PINMUX_SINGLE(TSN0_AVTP_MATCH),
> +	PINMUX_SINGLE(TSN0_LINK),
> +	PINMUX_SINGLE(TSN0_PHY_INT),
> +	PINMUX_SINGLE(TSN0_AVTP_PPS1),
> +	PINMUX_SINGLE(TSN0_MDC),
> +	PINMUX_SINGLE(TSN0_MDIO),
> +
> +	PINMUX_SINGLE(AVB2_RX_CTL),
> +	PINMUX_SINGLE(AVB2_TX_CTL),

As using AVB2_{TX_CTL,TXC,TD[0-3],MDC,MAGIC,AVTP_MATCH,AVTP_CAPTURE}
needs configuration in MODSEL5, you should use PINMUX_IPSR_NOGM()
instead.

E.g. PINMUX_IPSR_NOGM(0, AVB2_TX_CTL, SEL_AVB2_TX_CTL_1).

> +	PINMUX_SINGLE(AVB2_RXC),
> +	PINMUX_SINGLE(AVB2_RD0),
> +	PINMUX_SINGLE(AVB2_TXC),
> +	PINMUX_SINGLE(AVB2_TD0),
> +	PINMUX_SINGLE(AVB2_RD1),
> +	PINMUX_SINGLE(AVB2_RD2),
> +	PINMUX_SINGLE(AVB2_TD1),
> +	PINMUX_SINGLE(AVB2_TD2),
> +	PINMUX_SINGLE(AVB2_MDIO),
> +	PINMUX_SINGLE(AVB2_RD3),
> +	PINMUX_SINGLE(AVB2_TD3),
> +	PINMUX_SINGLE(AVB2_TXCREFCLK),
> +	PINMUX_SINGLE(AVB2_MDC),
> +	PINMUX_SINGLE(AVB2_MAGIC),
> +	PINMUX_SINGLE(AVB2_PHY_INT),
> +	PINMUX_SINGLE(AVB2_LINK),
> +	PINMUX_SINGLE(AVB2_AVTP_MATCH),
> +	PINMUX_SINGLE(AVB2_AVTP_CAPTURE),
> +	PINMUX_SINGLE(AVB2_AVTP_PPS),
> +
> +	/* IP0SR0 */
> +	PINMUX_IPSR_GPSR(IP0SR0_3_0,	ERROROUTC),

Missing TCLK2_A

> +
> +	PINMUX_IPSR_GPSR(IP0SR0_7_4,	MSIOF3_SS1),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_11_8,	MSIOF3_SS2),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_15_12,	IRQ3),
> +	PINMUX_IPSR_GPSR(IP0SR0_15_12,	MSIOF3_SCK),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_19_16,	IRQ2),
> +	PINMUX_IPSR_GPSR(IP0SR0_19_16,	MSIOF3_TXD),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_23_20,	IRQ1),
> +	PINMUX_IPSR_GPSR(IP0SR0_23_20,	MSIOF3_RXD),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_27_24,	IRQ0),
> +	PINMUX_IPSR_GPSR(IP0SR0_27_24,	MSIOF3_SYNC),
> +
> +	PINMUX_IPSR_GPSR(IP0SR0_31_28,	MSIOF5_SS2),
> +
> +	/* IP1SR0 */
> +	PINMUX_IPSR_GPSR(IP1SR0_3_0,	MSIOF5_SS1),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_7_4,	MSIOF5_SYNC),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_11_8,	MSIOF5_TXD),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_15_12,	MSIOF5_SCK),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_19_16,	MSIOF5_RXD),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_23_20,	MSIOF2_SS2),
> +	PINMUX_IPSR_GPSR(IP1SR0_23_20,	TCLK1),

Missing IRQ2_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR0_27_24,	MSIOF2_SS1),
> +	PINMUX_IPSR_GPSR(IP1SR0_27_24,	HTX1),
> +	PINMUX_IPSR_GPSR(IP1SR0_27_24,	TX1),
> +
> +	PINMUX_IPSR_GPSR(IP1SR0_31_28,	MSIOF2_SYNC),
> +	PINMUX_IPSR_GPSR(IP1SR0_31_28,	HRX1),
> +	PINMUX_IPSR_GPSR(IP1SR0_31_28,	RX1),
> +
> +	/* IP2SR0 */
> +	PINMUX_IPSR_GPSR(IP2SR0_3_0,	MSIOF2_TXD),
> +	PINMUX_IPSR_GPSR(IP2SR0_3_0,	HCTS1_N),
> +	PINMUX_IPSR_GPSR(IP2SR0_3_0,	CTS1_N),
> +
> +	PINMUX_IPSR_GPSR(IP2SR0_7_4,	MSIOF2_SCK),
> +	PINMUX_IPSR_GPSR(IP2SR0_7_4,	HRTS1_N),
> +	PINMUX_IPSR_GPSR(IP2SR0_7_4,	RTS1_N),
> +
> +	PINMUX_IPSR_GPSR(IP2SR0_11_8,	MSIOF2_RXD),
> +	PINMUX_IPSR_GPSR(IP2SR0_11_8,	HSCK1),
> +	PINMUX_IPSR_GPSR(IP2SR0_11_8,	SCK1),
> +
> +	/* IP0SR1 */
> +	PINMUX_IPSR_GPSR(IP0SR1_3_0,	MSIOF1_SS2),

Missing HTX3_A and TX3_A

> +	PINMUX_IPSR_GPSR(IP0SR1_7_4,	MSIOF1_SS1),

Missing HCTS3_N_A and RX3_A

> +	PINMUX_IPSR_GPSR(IP0SR1_11_8,	MSIOF1_SYNC),

Missing RTS3_N_A and RTS3_N

> +	PINMUX_IPSR_GPSR(IP0SR1_15_12,	MSIOF1_SCK),

Missing HSCK3_A and CTS3_N

> +	PINMUX_IPSR_GPSR(IP0SR1_19_16,	MSIOF1_TXD),

Missing HRX3_A and SCK3

> +	PINMUX_IPSR_GPSR(IP0SR1_23_20,	MSIOF1_RXD),
> +	PINMUX_IPSR_GPSR(IP0SR1_27_24,	MSIOF0_SS2),

Missing HTX1 and TX1

> +	PINMUX_IPSR_GPSR(IP0SR1_31_28,	MSIOF0_SS1),

Missing HRX1 and TX1

> +
> +	/* IP1SR1 */
> +	PINMUX_IPSR_GPSR(IP1SR1_3_0,	MSIOF0_SYNC),

Missing HCTS1_N, CTS1_N, and CANFD5_TX_B

> +
> +	PINMUX_IPSR_GPSR(IP1SR1_7_4,	MSIOF0_TXD),

Missing HRTS1_N, RTS1_N, and CANFD5_RX_B

> +
> +	PINMUX_IPSR_GPSR(IP1SR1_11_8,	MSIOF0_SCK),

Missing HSCK1 and SCK1

> +
> +	PINMUX_IPSR_GPSR(IP1SR1_15_12,	MSIOF0_RXD),
> +
> +	PINMUX_IPSR_GPSR(IP1SR1_19_16,	HTX0),
> +	PINMUX_IPSR_GPSR(IP1SR1_19_16,	TX0),
> +
> +	PINMUX_IPSR_GPSR(IP1SR1_23_20,	HCTS0_N),
> +	PINMUX_IPSR_GPSR(IP1SR1_23_20,	CTS0_N),
> +	PINMUX_IPSR_GPSR(IP1SR1_23_20,	PWM8),

PWM8_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR1_27_24,	HRTS0_N),
> +	PINMUX_IPSR_GPSR(IP1SR1_27_24,	RTS0_N),
> +	PINMUX_IPSR_GPSR(IP1SR1_27_24,	PWM9),

PWM9_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR1_31_28,	HSCK0),
> +	PINMUX_IPSR_GPSR(IP1SR1_31_28,	SCK0),
> +	PINMUX_IPSR_GPSR(IP1SR1_31_28,	PWM0),

PWM0_A

> +
> +	/* IP2SR1 */
> +	PINMUX_IPSR_GPSR(IP2SR1_3_0,	HRX0),
> +	PINMUX_IPSR_GPSR(IP2SR1_3_0,	RX0),
> +
> +	PINMUX_IPSR_GPSR(IP2SR1_7_4,	SCIF_CLK),
> +	PINMUX_IPSR_GPSR(IP2SR1_7_4,	IRQ4),

IRQ4_A

> +
> +	PINMUX_IPSR_GPSR(IP2SR1_11_8,	SSI_SCK),
> +	PINMUX_IPSR_GPSR(IP2SR1_11_8,	TCLK3),
> +
> +	PINMUX_IPSR_GPSR(IP2SR1_15_12,	SSI_WS),
> +	PINMUX_IPSR_GPSR(IP2SR1_15_12,	TCLK4),
> +
> +	PINMUX_IPSR_GPSR(IP2SR1_19_16,	SSI_SD),

Missing IRQ0_A

> +
> +	PINMUX_IPSR_GPSR(IP2SR1_23_20,	AUDIO_CLKOUT),

Missing IRQ1_A

> +
> +	PINMUX_IPSR_GPSR(IP2SR1_27_24,	AUDIO_CLKIN),
> +	PINMUX_IPSR_GPSR(IP2SR1_27_24,	PWM3),

PWM3_A

> +
> +	PINMUX_IPSR_GPSR(IP2SR1_31_28,	TCLK2),
> +	PINMUX_IPSR_GPSR(IP2SR1_31_28,	MSIOF4_SS1),

Missing IRQ3_B

> +
> +	/* IP3SR1 */
> +	PINMUX_IPSR_GPSR(IP3SR1_3_0,	HRX3),
> +	PINMUX_IPSR_GPSR(IP3SR1_3_0,	SCK3),

SCK3_A

> +	PINMUX_IPSR_GPSR(IP3SR1_3_0,	MSIOF4_SS2),
> +
> +	PINMUX_IPSR_GPSR(IP3SR1_7_4,	HSCK3),
> +	PINMUX_IPSR_GPSR(IP3SR1_7_4,	CTS3_N),

CTS3_N_A

> +	PINMUX_IPSR_GPSR(IP3SR1_7_4,	MSIOF4_SCK),

Missing TPU0TO0_A

> +
> +	PINMUX_IPSR_GPSR(IP3SR1_11_8,	HRTS3_N),
> +	PINMUX_IPSR_GPSR(IP3SR1_11_8,	RTS3_N),

RTS3_N_A

> +	PINMUX_IPSR_GPSR(IP3SR1_11_8,	MSIOF4_TXD),

Missing TPU0TO1_A

> +
> +	PINMUX_IPSR_GPSR(IP3SR1_15_12,	HCTS3_N),
> +	PINMUX_IPSR_GPSR(IP3SR1_15_12,	RX3),

RX3_A

> +	PINMUX_IPSR_GPSR(IP3SR1_15_12,	MSIOF4_RXD),
> +
> +	PINMUX_IPSR_GPSR(IP3SR1_19_16,	HTX3),
> +	PINMUX_IPSR_GPSR(IP3SR1_19_16,	TX3),

TX3_A

> +	PINMUX_IPSR_GPSR(IP3SR1_19_16,	MSIOF4_SYNC),
> +
> +	/* IP0SR2 */
> +	PINMUX_IPSR_GPSR(IP0SR2_3_0,	FXR_TXDA),
> +	PINMUX_IPSR_GPSR(IP0SR2_3_0,	CANFD1_TX),

Missing TPU0TO2_A

> +
> +	PINMUX_IPSR_GPSR(IP0SR2_7_4,	FXR_TXENA_N),
> +	PINMUX_IPSR_GPSR(IP0SR2_7_4,	CANFD1_RX),

Missing TPU0TO3_A

> +
> +	PINMUX_IPSR_GPSR(IP0SR2_11_8,	RXDA_EXTFXR),
> +	PINMUX_IPSR_GPSR(IP0SR2_11_8,	CANFD5_TX),
> +	PINMUX_IPSR_GPSR(IP0SR2_11_8,	IRQ5),
> +
> +	PINMUX_IPSR_GPSR(IP0SR2_15_12,	CLK_EXTFXR),
> +	PINMUX_IPSR_GPSR(IP0SR2_15_12,	CANFD5_RX),

Missing IRQ4_B

> +
> +	PINMUX_IPSR_GPSR(IP0SR2_19_16,	RXDB_EXTFXR),
> +
> +	PINMUX_IPSR_GPSR(IP0SR2_23_20,	FXR_TXENB_N),
> +
> +	PINMUX_IPSR_GPSR(IP0SR2_27_24,	FXR_TXDB),
> +
> +	PINMUX_IPSR_GPSR(IP0SR2_31_28,	TPU0TO1),
> +	PINMUX_IPSR_GPSR(IP0SR2_31_28,	CANFD6_TX),

Missing TCLK2_B

> +
> +	/* IP1SR2 */
> +	PINMUX_IPSR_GPSR(IP1SR2_3_0,	TPU0TO0),
> +	PINMUX_IPSR_GPSR(IP1SR2_3_0,	CANFD6_RX),

Missing TCLK1_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_7_4,	CAN_CLK),

Missing FXR_TXENA_N

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_11_8,	CANFD0_TX),

Missing FXR_TXENB_N

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_15_12,	CANFD0_RX),
> +	PINMUX_IPSR_GPSR(IP1SR2_15_12,	STPWT_EXTFXR),
> +
> +	PINMUX_IPSR_GPSR(IP1SR2_19_16,	CANFD2_TX),
> +	PINMUX_IPSR_GPSR(IP1SR2_19_16,	TPU0TO2),

Missing TCLK3_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_23_20,	CANFD2_RX),
> +	PINMUX_IPSR_GPSR(IP1SR2_23_20,	TPU0TO3),
> +	PINMUX_IPSR_GPSR(IP1SR2_23_20,	PWM1),

PWM1_B

Missing TCLK4_A

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_27_24,	CANFD3_TX),
> +	PINMUX_IPSR_GPSR(IP1SR2_27_24,	PWM2),

PWM2_B

> +
> +	PINMUX_IPSR_GPSR(IP1SR2_31_28,	CANFD3_RX),

Missing PWM3_B

> +
> +	/* IP2SR2 */
> +	PINMUX_IPSR_GPSR(IP2SR2_3_0,	CANFD4_TX),
> +	PINMUX_IPSR_GPSR(IP2SR2_3_0,	PWM4),
> +
> +	PINMUX_IPSR_GPSR(IP2SR2_7_4,	CANFD4_RX),
> +	PINMUX_IPSR_GPSR(IP2SR2_7_4,	PWM5),
> +
> +	PINMUX_IPSR_GPSR(IP2SR2_11_8,	CANFD7_TX),
> +	PINMUX_IPSR_GPSR(IP2SR2_11_8,	PWM6),
> +
> +	PINMUX_IPSR_GPSR(IP2SR2_15_12,	CANFD7_RX),
> +	PINMUX_IPSR_GPSR(IP2SR2_15_12,	PWM7),
> +
> +	/* IP0SR3 */
> +	PINMUX_IPSR_GPSR(IP0SR3_3_0,	MMC_SD_D1),
> +	PINMUX_IPSR_GPSR(IP0SR3_7_4,	MMC_SD_D0),
> +	PINMUX_IPSR_GPSR(IP0SR3_11_8,	MMC_SD_D2),
> +	PINMUX_IPSR_GPSR(IP0SR3_15_12,	MMC_SD_CLK),
> +	PINMUX_IPSR_GPSR(IP0SR3_19_16,	MMC_DS),
> +	PINMUX_IPSR_GPSR(IP0SR3_23_20,	MMC_SD_D3),
> +	PINMUX_IPSR_GPSR(IP0SR3_27_24,	MMC_D5),
> +	PINMUX_IPSR_GPSR(IP0SR3_31_28,	MMC_D4),
> +
> +	/* IP1SR3 */
> +	PINMUX_IPSR_GPSR(IP1SR3_3_0,	MMC_D7),
> +
> +	PINMUX_IPSR_GPSR(IP1SR3_7_4,	MMC_D6),
> +
> +	PINMUX_IPSR_GPSR(IP1SR3_11_8,	MMC_SD_CMD),
> +
> +	PINMUX_IPSR_GPSR(IP1SR3_15_12,	SD_CD),
> +
> +	PINMUX_IPSR_GPSR(IP1SR3_19_16,	SD_WP),
> +
> +	PINMUX_IPSR_GPSR(IP1SR3_23_20,	IPC_CLKIN),
> +	PINMUX_IPSR_GPSR(IP1SR3_23_20,	IPC_CLKEN_IN),

Missing PWM1_A and TCLK3

> +
> +	PINMUX_IPSR_GPSR(IP1SR3_27_24,	IPC_CLKOUT),
> +	PINMUX_IPSR_GPSR(IP1SR3_27_24,	IPC_CLKEN_OUT),

Missing ERROROUTC_A, TCLK4, and PWM2_A (the latter is only listed in the
pin function spreadsheet, as the main PDF file lacks a column for
function 4?).

> +
> +	PINMUX_IPSR_GPSR(IP1SR3_31_28,	QSPI0_SSL),
> +
> +	/* IP2SR3 */
> +	PINMUX_IPSR_GPSR(IP2SR3_3_0,	QSPI0_IO3),
> +	PINMUX_IPSR_GPSR(IP2SR3_7_4,	QSPI0_IO2),
> +	PINMUX_IPSR_GPSR(IP2SR3_11_8,	QSPI0_MISO_IO1),
> +	PINMUX_IPSR_GPSR(IP2SR3_15_12,	QSPI0_MOSI_IO0),
> +	PINMUX_IPSR_GPSR(IP2SR3_19_16,	QSPI0_SPCLK),
> +	PINMUX_IPSR_GPSR(IP2SR3_23_20,	QSPI1_MOSI_IO0),
> +	PINMUX_IPSR_GPSR(IP2SR3_27_24,	QSPI1_SPCLK),
> +	PINMUX_IPSR_GPSR(IP2SR3_31_28,	QSPI1_MISO_IO1),
> +
> +	/* IP3SR3 */
> +	PINMUX_IPSR_GPSR(IP3SR3_3_0,	QSPI1_IO2),
> +	PINMUX_IPSR_GPSR(IP3SR3_7_4,	QSPI1_SSL),
> +	PINMUX_IPSR_GPSR(IP3SR3_11_8,	QSPI1_IO3),
> +	PINMUX_IPSR_GPSR(IP3SR3_15_12,	RPC_RESET_N),
> +	PINMUX_IPSR_GPSR(IP3SR3_19_16,	RPC_WP_N),
> +	PINMUX_IPSR_GPSR(IP3SR3_23_20,	RPC_INT_N),
> +
> +	/* IP0SR6 */
> +	PINMUX_IPSR_GPSR(IP0SR6_3_0,	AVB1_MDIO),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_7_4,	AVB1_MAGIC),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_11_8,	AVB1_MDC),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_15_12,	AVB1_PHY_INT),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_19_16,	AVB1_LINK),
> +	PINMUX_IPSR_GPSR(IP0SR6_19_16,	AVB1_MII_TX_ER),

Hmm, the MII functions for SVB[01] are only documented in the main PDF
file, not in the pin function spreadsheet...

> +
> +	PINMUX_IPSR_GPSR(IP0SR6_23_20,	AVB1_AVTP_MATCH),
> +	PINMUX_IPSR_GPSR(IP0SR6_23_20,	AVB1_MII_RX_ER),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_27_24,	AVB1_TXC),
> +	PINMUX_IPSR_GPSR(IP0SR6_27_24,	AVB1_MII_TXC),
> +
> +	PINMUX_IPSR_GPSR(IP0SR6_31_28,	AVB1_TX_CTL),
> +	PINMUX_IPSR_GPSR(IP0SR6_31_28,	AVB1_MII_TX_EN),
> +
> +	/* IP1SR6 */
> +	PINMUX_IPSR_GPSR(IP1SR6_3_0,	AVB1_RXC),
> +	PINMUX_IPSR_GPSR(IP1SR6_3_0,	AVB1_MII_RXC),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_7_4,	AVB1_RX_CTL),
> +	PINMUX_IPSR_GPSR(IP1SR6_7_4,	AVB1_MII_RX_DV),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_11_8,	AVB1_AVTP_PPS),
> +	PINMUX_IPSR_GPSR(IP1SR6_11_8,	AVB1_MII_COL),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_15_12,	AVB1_AVTP_CAPTURE),
> +	PINMUX_IPSR_GPSR(IP1SR6_15_12,	AVB1_MII_CRS),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_19_16,	AVB1_TD1),
> +	PINMUX_IPSR_GPSR(IP1SR6_19_16,	AVB1_MII_TD1),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_23_20,	AVB1_TD0),
> +	PINMUX_IPSR_GPSR(IP1SR6_23_20,	AVB1_MII_TD0),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_27_24,	AVB1_RD1),
> +	PINMUX_IPSR_GPSR(IP1SR6_27_24,	AVB1_MII_RD1),
> +
> +	PINMUX_IPSR_GPSR(IP1SR6_31_28,	AVB1_RD0),
> +	PINMUX_IPSR_GPSR(IP1SR6_31_28,	AVB1_MII_RD0),
> +
> +	/* IP2SR6 */
> +	PINMUX_IPSR_GPSR(IP2SR6_3_0,	AVB1_TD2),
> +	PINMUX_IPSR_GPSR(IP2SR6_3_0,	AVB1_MII_TD2),
> +
> +	PINMUX_IPSR_GPSR(IP2SR6_7_4,	AVB1_RD2),
> +	PINMUX_IPSR_GPSR(IP2SR6_7_4,	AVB1_MII_RD2),
> +
> +	PINMUX_IPSR_GPSR(IP2SR6_11_8,	AVB1_TD3),
> +	PINMUX_IPSR_GPSR(IP2SR6_11_8,	AVB1_MII_TD3),
> +
> +	PINMUX_IPSR_GPSR(IP2SR6_15_12,	AVB1_RD3),
> +	PINMUX_IPSR_GPSR(IP2SR6_15_12,	AVB1_MII_RD3),
> +
> +	PINMUX_IPSR_GPSR(IP2SR6_19_16,	AVB1_TXCREFCLK),

Some of the above need to configure MODSEL6, so you should use
PINMUX_IPSR_MSEL(..., AVB1_*, SEL_AVB1_*).

> +
> +	/* IP0SR7 */
> +	PINMUX_IPSR_MSEL(IP0SR7_3_0,	AVB0_AVTP_PPS,		SEL_AVB0_AVTP_PPS_1),
> +	PINMUX_IPSR_MSEL(IP0SR7_3_0,	AVB0_MII_COL,		SEL_AVB0_AVTP_PPS_0),
> +
> +	PINMUX_IPSR_GPSR(IP0SR7_7_4,	AVB0_AVTP_CAPTURE),
> +	PINMUX_IPSR_GPSR(IP0SR7_7_4,	AVB0_MII_CRS),
> +
> +	PINMUX_IPSR_MSEL(IP0SR7_11_8,	AVB0_AVTP_MATCH,	SEL_AVB0_AVTP_MATCH_1),
> +	PINMUX_IPSR_MSEL(IP0SR7_11_8,	AVB0_MII_RX_ER,		SEL_AVB0_AVTP_MATCH_0),
> +	PINMUX_IPSR_MSEL(IP0SR7_11_8,	CC5_OSCOUT,		SEL_AVB0_AVTP_MATCH_0),
> +
> +	PINMUX_IPSR_MSEL(IP0SR7_15_12,	AVB0_TD3,		SEL_AVB0_TD3_1),
> +	PINMUX_IPSR_MSEL(IP0SR7_15_12,	AVB0_MII_TD3,		SEL_AVB0_TD3_0),
> +
> +	PINMUX_IPSR_GPSR(IP0SR7_19_16,	AVB0_LINK),
> +	PINMUX_IPSR_GPSR(IP0SR7_19_16,	AVB0_MII_TX_ER),
> +
> +	PINMUX_IPSR_GPSR(IP0SR7_23_20,	AVB0_PHY_INT),
> +
> +	PINMUX_IPSR_MSEL(IP0SR7_27_24,	AVB0_TD2,		SEL_AVB0_TD2_1),
> +	PINMUX_IPSR_MSEL(IP0SR7_27_24,	AVB0_MII_TD2,		SEL_AVB0_TD2_0),
> +
> +	PINMUX_IPSR_MSEL(IP0SR7_31_28,	AVB0_TD1,		SEL_AVB0_TD1_1),
> +	PINMUX_IPSR_MSEL(IP0SR7_31_28,	AVB0_MII_TD1,		SEL_AVB0_TD1_0),
> +
> +	/* IP1SR7 */
> +	PINMUX_IPSR_GPSR(IP1SR7_3_0,	AVB0_RD3),
> +	PINMUX_IPSR_GPSR(IP1SR7_3_0,	AVB0_MII_RD3),
> +
> +	PINMUX_IPSR_GPSR(IP1SR7_7_4,	AVB0_TXCREFCLK),
> +
> +	PINMUX_IPSR_MSEL(IP1SR7_11_8,	AVB0_MAGIC,		SEL_AVB0_MAGIC_1),
> +
> +	PINMUX_IPSR_MSEL(IP1SR7_15_12,	AVB0_TD0,		SEL_AVB0_TD0_1),
> +	PINMUX_IPSR_MSEL(IP1SR7_15_12,	AVB0_MII_TD0,		SEL_AVB0_TD0_0),
> +
> +	PINMUX_IPSR_GPSR(IP1SR7_19_16,	AVB0_RD2),
> +	PINMUX_IPSR_GPSR(IP1SR7_19_16,	AVB0_MII_RD2),
> +
> +	PINMUX_IPSR_MSEL(IP1SR7_23_20,	AVB0_MDC,		SEL_AVB0_MDC_1),
> +
> +	PINMUX_IPSR_GPSR(IP1SR7_27_24,	AVB0_MDIO),
> +
> +	PINMUX_IPSR_MSEL(IP1SR7_31_28,	AVB0_TXC,		SEL_AVB0_TXC_1),
> +	PINMUX_IPSR_MSEL(IP1SR7_31_28,	AVB0_MII_TXC,		SEL_AVB0_TXC_0),
> +
> +	/* IP2SR7 */
> +	PINMUX_IPSR_MSEL(IP2SR7_3_0,	AVB0_TX_CTL,		SEL_AVB0_TX_CTL_1),
> +	PINMUX_IPSR_MSEL(IP2SR7_3_0,	AVB0_MII_TX_EN,		SEL_AVB0_TX_CTL_0),

Are the above SEL_AVB0_* values correct?
I am not an MII expert, but I would expect e.g. both AVB0_TD3 and
AVB0_MII_TD3 to need SEL_AVB0_TD3_1 (= output enabled).

> +
> +	PINMUX_IPSR_GPSR(IP2SR7_7_4,	AVB0_RD1),
> +	PINMUX_IPSR_GPSR(IP2SR7_7_4,	AVB0_MII_RD1),
> +
> +	PINMUX_IPSR_GPSR(IP2SR7_11_8,	AVB0_RD0),
> +	PINMUX_IPSR_GPSR(IP2SR7_11_8,	AVB0_MII_RD0),
> +
> +	PINMUX_IPSR_GPSR(IP2SR7_15_12,	AVB0_RXC),
> +	PINMUX_IPSR_GPSR(IP2SR7_15_12,	AVB0_MII_RXC),
> +
> +	PINMUX_IPSR_GPSR(IP2SR7_19_16,	AVB0_RX_CTL),
> +	PINMUX_IPSR_GPSR(IP2SR7_19_16,	AVB0_MII_RX_DV),
> +
> +	/* IP0SR8 */
> +	PINMUX_IPSR_MSEL(IP0SR8_3_0,	SCL0,			SEL_SCL0_0),

SEL_SCL0_1

> +	PINMUX_IPSR_MSEL(IP0SR8_7_4,	SDA0,			SEL_SDA0_0),

SEL_SDA0_1

> +	PINMUX_IPSR_MSEL(IP0SR8_11_8,	SCL1,			SEL_SCL1_0),

SEL_SCL1_1

> +	PINMUX_IPSR_MSEL(IP0SR8_15_12,	SDA1,			SEL_SDA1_0),

SEL_SDA1_1

> +	PINMUX_IPSR_MSEL(IP0SR8_19_16,	SCL2,			SEL_SCL2_0),

SEL_SCL2_1

> +	PINMUX_IPSR_MSEL(IP0SR8_23_20,	SDA2,			SEL_SDA2_0),

SEL_SDA2_1

> +	PINMUX_IPSR_MSEL(IP0SR8_27_24,	SCL3,			SEL_SCL3_0),

SEL_SCL3_1

> +	PINMUX_IPSR_MSEL(IP0SR8_31_28,	SDA3,			SEL_SDA3_0),

SEL_SDA3_1

> +
> +	/* IP1SR8 */
> +	PINMUX_IPSR_MSEL(IP1SR8_3_0,	SCL4,			SEL_SCL4_0),

SEL_SCL4_1

> +	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),

SEL_SDA4_1

> +	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),

SEL_SCL5_1

> +	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),

SEL_SDA5_1

> +	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

  parent reply	other threads:[~2022-06-13 18:49 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 [this message]
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
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=alpine.DEB.2.22.394.2206131708100.946510@ramsan.of.borg \
    --to=geert@linux-m68k.org \
    --cc=geert+renesas@glider.be \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.