All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
@ 2016-09-13 14:03 Niklas Söderlund
  2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-13 14:03 UTC (permalink / raw)
  To: geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij, Niklas Söderlund

Hi,

This series adds support to control the drive strength for none GPIO 
pins. All pins expect one (FSCLKST) which can have its drive strength 
controlled is now supported. I have also added the new pins to the 
correct groups, or added groups to mimic other sh-pfc drivers. One 
notable exception is the group avb_mdc which on other SoC are called 
avb_mdio, see commit for explanation.

I did not add FSCLKST since I can't figure out which physical pin it's 
mapped to. Looking at the code that is already there and documentation 
it should be a GPIO pin controlled by IPSR7[15:12] but the documentation 
and code is lacking that part and I can't with a 100% certainty figure 
out which physical pin it is.

The series is based on top of v4.8-rc5 and tested on Salvator-X. My test 
is a bit crude and is setting a few of the AVB pins to a higher 
drive-strength value then the others and observing that the AVB fails to 
work after the pfc settings are applied (NFS root failing to mount after 
kernel is fetched over TFTP).

Niklas Söderlund (4):
  pinctrl: sh-pfc: Support named pins with custom configuration
  pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable
    drive-strength
  pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins
  pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins

 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 254 ++++++++++++++++++++++++++++++++---
 drivers/pinctrl/sh-pfc/sh_pfc.h      |   8 ++
 2 files changed, 243 insertions(+), 19 deletions(-)

-- 
2.9.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration
  2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
@ 2016-09-13 14:03 ` Niklas Söderlund
  2016-09-13 14:28   ` Laurent Pinchart
  2016-10-04 19:08   ` Geert Uytterhoeven
  2016-09-13 14:03 ` [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-13 14:03 UTC (permalink / raw)
  To: geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij, Niklas Söderlund

Pins not associated with a GPIO port can still have other configuration
parameters. Add a new macro SH_PFC_PIN_NAMED_CFG which allows for named
pins to be declared with a set of configurations. The new macro is an
modification of SH_PFC_PIN_NAMED to allow for optional configuration to
be assigned.

The flag SH_PFC_PIN_CFG_NO_GPIO is still enforced as this should only be
used to define pins not associated with a GPIO port.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/pinctrl/sh-pfc/sh_pfc.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 5e966c0..a6a2346 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -507,6 +507,14 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
 		.configs = SH_PFC_PIN_CFG_NO_GPIO,			\
 	}
 
+/* SH_PFC_PIN_NAMED_CFG - Expand to a sh_pfc_pin entry with the given name */
+#define SH_PFC_PIN_NAMED_CFG(row, col, _name, cfgs)			\
+	{								\
+		.pin = PIN_NUMBER(row, col),				\
+		.name = __stringify(PIN_##_name),			\
+		.configs = SH_PFC_PIN_CFG_NO_GPIO | cfgs,		\
+	}
+
 /* PINMUX_DATA_ALL - Expand to a list of PORT_name_DATA, PORT_name_FN0,
  *		     PORT_name_OUT, PORT_name_IN marks
  */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
  2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
@ 2016-09-13 14:03 ` Niklas Söderlund
  2016-10-04 19:13   ` Geert Uytterhoeven
  2016-09-13 14:03 ` [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins Niklas Söderlund
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-13 14:03 UTC (permalink / raw)
  To: geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij, Niklas Söderlund

There are pins on the Salvator-X which is not part of a GPIO bank nor
can be muxed between different functions. They do however allow for the
drive-strength to be configured. Add those pins to the list of pins and
to the drive-strength configuration registers.

The pins can now be referred to in DT by there physical location and the
drive-strength modified.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 155 +++++++++++++++++++++++++++++++----
 1 file changed, 139 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index b74cdd3..4a60f15 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -518,7 +518,22 @@ MOD_SEL0_3		MOD_SEL1_3 \
 MOD_SEL0_2_1		MOD_SEL1_2 \
 			MOD_SEL1_1 \
 			MOD_SEL1_0		MOD_SEL2_0
-
+/*
+ * These pins are not able to be muxed but have other properties
+ * that can be set, such as drive-strength or pull-up/pull-down enable.
+ */
+#define PINMUX_STATIC \
+	FM(QSPI0_SPCLK) FM(QSPI0_SSL) FM(QSPI0_MOSI_IO0) FM(QSPI0_MISO_IO1) \
+	FM(QSPI0_IO2) FM(QSPI0_IO3) \
+	FM(QSPI1_SPCLK) FM(QSPI1_SSL) FM(QSPI1_MOSI_IO0) FM(QSPI1_MISO_IO1) \
+	FM(QSPI1_IO2) FM(QSPI1_IO3) \
+	FM(RPC_INT) FM(RPC_WP) FM(RPC_RESET) \
+	FM(AVB_TX_CTL) FM(AVB_TXC) FM(AVB_TD0) FM(AVB_TD1) FM(AVB_TD2) FM(AVB_TD3) \
+	FM(AVB_RX_CTL) FM(AVB_RXC) FM(AVB_RD0) FM(AVB_RD1) FM(AVB_RD2) FM(AVB_RD3) \
+	FM(AVB_TXREFCLK) FM(AVB_MDIO) \
+	FM(CLKOUT) FM(PRESETOUT) \
+	FM(DU_DOTCLKIN0) FM(DU_DOTCLKIN1) FM(DU_DOTCLKIN2) FM(DU_DOTCLKIN3) \
+	FM(TMS) FM(TDO) FM(ASEBRK) FM(MLB_REF)
 
 enum {
 	PINMUX_RESERVED = 0,
@@ -544,6 +559,7 @@ enum {
 	PINMUX_GPSR
 	PINMUX_IPSR
 	PINMUX_MOD_SELS
+	PINMUX_STATIC
 	PINMUX_MARK_END,
 #undef F_
 #undef FM
@@ -1408,10 +1424,70 @@ static const u16 pinmux_data[] = {
 	PINMUX_IPSR_MSEL(IP17_7_4,	STP_ISSYNC_0_E,		SEL_SSP1_0_4),
 	PINMUX_IPSR_MSEL(IP17_7_4,	RIF2_D1_B,		SEL_DRIF2_1),
 	PINMUX_IPSR_GPSR(IP17_7_4,	TPU0TO3),
+
+/*
+ * Static pins can not be muxed between different functions but
+ * still needs a mark entry in the pinmux list. Add each static
+ * pin to the list without an associated function. The sh-pfc
+ * core will do the right thing and skip trying to mux then pin
+ * while still applying configuration to it
+ */
+#define FM(x)	PINMUX_DATA(x##_MARK, 0),
+	PINMUX_STATIC
+#undef FM
 };
 
+/*
+ * R8A7795 has 7 banks with 32 PGIOS in each = 224 GPIOs.
+ * Physical layout rows: A - AW, cols: 1 - 39.
+ */
+#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
+#define PIN_NUMBER(r, c) (((r) - 'A') * 39 + (c) + 300)
+#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
+
 static const struct sh_pfc_pin pinmux_pins[] = {
 	PINMUX_GPIO_GP_ALL(),
+
+	/* Pins not associated with a GPIO port */
+	SH_PFC_PIN_NAMED_CFG('A',  8,  A8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TX_CTL */
+	SH_PFC_PIN_NAMED_CFG('A',  9,  A9, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_MDIO */
+	SH_PFC_PIN_NAMED_CFG('A', 12, A12, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TXCREFCLK */
+	SH_PFC_PIN_NAMED_CFG('A', 13, A13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RD0 */
+	SH_PFC_PIN_NAMED_CFG('A', 14, A14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RD2 */
+	SH_PFC_PIN_NAMED_CFG('A', 16, A16, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RX_CTL */
+	SH_PFC_PIN_NAMED_CFG('A', 17, A17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TD2 */
+	SH_PFC_PIN_NAMED_CFG('A', 18, A18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TD0 */
+	SH_PFC_PIN_NAMED_CFG('A', 19, A19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TXC */
+	SH_PFC_PIN_NAMED_CFG('B', 13, B13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RD1 */
+	SH_PFC_PIN_NAMED_CFG('B', 14, B14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RD3 */
+	SH_PFC_PIN_NAMED_CFG('B', 17, B17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TD3 */
+	SH_PFC_PIN_NAMED_CFG('B', 18, B18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_TD1 */
+	SH_PFC_PIN_NAMED_CFG('B', 19, B19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* AVB_RXC */
+	SH_PFC_PIN_NAMED_CFG('C',  1,  C1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* PRESETOUT# */
+	SH_PFC_PIN_NAMED_CFG('F',  1,  F1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* CLKOUT */
+	SH_PFC_PIN_NAMED_CFG('H', 37, H37, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* MLB_REF */
+	SH_PFC_PIN_NAMED_CFG('V',  3,  V3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* QSPI1_SPCLK */
+	SH_PFC_PIN_NAMED_CFG('V',  5,  V5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* QSPI1_SSL */
+	SH_PFC_PIN_NAMED_CFG('V',  6,  V6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* RPC_WP# */
+	SH_PFC_PIN_NAMED_CFG('V',  7,  V7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* RPC_RESET# */
+	SH_PFC_PIN_NAMED_CFG('W',  3,  W3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* QSPI0_SPCLK */
+	SH_PFC_PIN_NAMED_CFG('Y',  3,  Y3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* QSPI0_SSL */
+	SH_PFC_PIN_NAMED_CFG('Y',  6,  Y6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* QSPI0_IO2 */
+	SH_PFC_PIN_NAMED_CFG('Y',  7,  Y7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),			/* RPC_INT# */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  4,  AB4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI0_MISO_IO1 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  6,  AB6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI0_IO3 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  3,  AC3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI1_IO3 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  5,  AC5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI0_MOSI_IO0 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  7,  AC7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI1_MOSI_IO0 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI1_IO2 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* QSPI1_MISO_IO1 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* DU_DOTCLKIN0 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* DU_DOTCLKIN1 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* DU_DOTCLKIN2 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* DU_DOTCLKIN3 */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* TMS */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* TDO */
+	SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),	/* ASEBRK */
 };
 
 /* - AUDIO CLOCK ------------------------------------------------------------ */
@@ -4858,10 +4934,45 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 };
 
 static const struct pinmux_drive_reg pinmux_drive_regs[] = {
+	{ PINMUX_DRIVE_REG("DRVCTRL0", 0xe6060300) {
+		{ PIN_NUMBER('W', 3),   28, 2 },	/* QSPI0_SPCLK */
+		{ PIN_A_NUMBER('C', 5), 24, 2 },	/* QSPI0_MOSI_IO0 */
+		{ PIN_A_NUMBER('B', 4), 20, 2 },	/* QSPI0_MISO_IO1 */
+		{ PIN_NUMBER('Y', 6),   16, 2 },	/* QSPI0_IO2 */
+		{ PIN_A_NUMBER('B', 6), 12, 2 },	/* QSPI0_IO3 */
+		{ PIN_NUMBER('Y', 3),    8, 2 },	/* QSPI0_SSL */
+		{ PIN_NUMBER('V', 3),    4, 2 },	/* QSPI1_SPCLK */
+		{ PIN_A_NUMBER('C', 7),  0, 2 },	/* QSPI1_MOSI_IO0 */
+	} },
+	{ PINMUX_DRIVE_REG("DRVCTRL1", 0xe6060304) {
+		{ PIN_A_NUMBER('E', 5), 28, 2 },	/* QSPI1_MISO_IO1 */
+		{ PIN_A_NUMBER('E', 4), 24, 2 },	/* QSPI1_IO2 */
+		{ PIN_A_NUMBER('C', 3), 20, 2 },	/* QSPI1_IO3 */
+		{ PIN_NUMBER('V', 5),   16, 2 },	/* QSPI1_SSL */
+		{ PIN_NUMBER('Y', 7),   12, 2 },	/* RPC_INT# */
+		{ PIN_NUMBER('V', 6),    8, 2 },	/* RPC_WP# */
+		{ PIN_NUMBER('V', 7),    4, 2 },	/* RPC_RESET# */
+		{ PIN_NUMBER('A', 16),   0, 3 },	/* AVB_RX_CTL */
+	} },
+	{ PINMUX_DRIVE_REG("DRVCTRL2", 0xe6060308) {
+		{ PIN_NUMBER('B', 19),  28, 3 },	/* AVB_RXC */
+		{ PIN_NUMBER('A', 13),  24, 3 },	/* AVB_RD0 */
+		{ PIN_NUMBER('B', 13),  20, 3 },	/* AVB_RD1 */
+		{ PIN_NUMBER('A', 14),  16, 3 },	/* AVB_RD2 */
+		{ PIN_NUMBER('B', 14),  12, 3 },	/* AVB_RD3 */
+		{ PIN_NUMBER('A', 8),    8, 3 },	/* AVB_TX_CTL */
+		{ PIN_NUMBER('A', 19),   4, 3 },	/* AVB_TXC */
+		{ PIN_NUMBER('A', 18),   0, 3 },	/* AVB_TD0 */
+	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL3", 0xe606030c) {
-		{ RCAR_GP_PIN(2,  9),  8, 3 },	/* AVB_MDC */
-		{ RCAR_GP_PIN(2, 10),  4, 3 },	/* AVB_MAGIC */
-		{ RCAR_GP_PIN(2, 11),  0, 3 },	/* AVB_PHY_INT */
+		{ PIN_NUMBER('B', 18),  28, 3 },	/* AVB_TD1 */
+		{ PIN_NUMBER('A', 17),  24, 3 },	/* AVB_TD2 */
+		{ PIN_NUMBER('B', 17),  20, 3 },	/* AVB_TD3 */
+		{ PIN_NUMBER('A', 12),  16, 3 },	/* AVB_TXCREFCLK */
+		{ PIN_NUMBER('A', 9),   12, 3 },	/* AVB_MDIO */
+		{ RCAR_GP_PIN(2,  9),    8, 3 },	/* AVB_MDC */
+		{ RCAR_GP_PIN(2, 10),    4, 3 },	/* AVB_MAGIC */
+		{ RCAR_GP_PIN(2, 11),    0, 3 },	/* AVB_PHY_INT */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL4", 0xe6060310) {
 		{ RCAR_GP_PIN(2, 12), 28, 3 },	/* AVB_LINK */
@@ -4904,6 +5015,7 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
 		{ RCAR_GP_PIN(1, 19),  0, 3 },	/* A19 */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL8", 0xe6060320) {
+		{ PIN_NUMBER('F', 1), 28, 3 },	/* CLKOUT */
 		{ RCAR_GP_PIN(1, 20), 24, 3 },	/* CS0 */
 		{ RCAR_GP_PIN(1, 21), 20, 3 },	/* CS1_A26 */
 		{ RCAR_GP_PIN(1, 22), 16, 3 },	/* BS */
@@ -4914,6 +5026,7 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL9", 0xe6060324) {
 		{ RCAR_GP_PIN(1, 27), 28, 3 },	/* EX_WAIT0 */
+		{ PIN_NUMBER('C', 1), 24, 3 },	/* PRESETOUT# */
 		{ RCAR_GP_PIN(0,  0), 20, 3 },	/* D0 */
 		{ RCAR_GP_PIN(0,  1), 16, 3 },	/* D1 */
 		{ RCAR_GP_PIN(0,  2), 12, 3 },	/* D2 */
@@ -4932,20 +5045,29 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
 		{ RCAR_GP_PIN(0, 13),  0, 3 },	/* D13 */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL11", 0xe606032c) {
-		{ RCAR_GP_PIN(0, 14), 28, 3 },	/* D14 */
-		{ RCAR_GP_PIN(0, 15), 24, 3 },	/* D15 */
-		{ RCAR_GP_PIN(7,  0), 20, 3 },	/* AVS1 */
-		{ RCAR_GP_PIN(7,  1), 16, 3 },	/* AVS2 */
-		{ RCAR_GP_PIN(7,  2), 12, 3 },	/* HDMI0_CEC */
-		{ RCAR_GP_PIN(7,  3),  8, 3 },	/* HDMI1_CEC */
+		{ RCAR_GP_PIN(0, 14),   28, 3 },	/* D14 */
+		{ RCAR_GP_PIN(0, 15),   24, 3 },	/* D15 */
+		{ RCAR_GP_PIN(7,  0),   20, 3 },	/* AVS1 */
+		{ RCAR_GP_PIN(7,  1),   16, 3 },	/* AVS2 */
+		{ RCAR_GP_PIN(7,  2),   12, 3 },	/* HDMI0_CEC */
+		{ RCAR_GP_PIN(7,  3),    8, 3 },	/* HDMI1_CEC */
+		{ PIN_A_NUMBER('P', 7),  4, 2 },	/* DU_DOTCLKIN0 */
+		{ PIN_A_NUMBER('P', 8),  0, 2 },	/* DU_DOTCLKIN1 */
+	} },
+	{ PINMUX_DRIVE_REG("DRVCTRL12", 0xe6060330) {
+		{ PIN_A_NUMBER('R', 7),  28, 2 },	/* DU_DOTCLKIN2 */
+		{ PIN_A_NUMBER('R', 8),  24, 2 },	/* DU_DOTCLKIN3 */
+		{ PIN_A_NUMBER('R', 30),  4, 2 },	/* TMS */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL13", 0xe6060334) {
-		{ RCAR_GP_PIN(3,  0), 20, 3 },	/* SD0_CLK */
-		{ RCAR_GP_PIN(3,  1), 16, 3 },	/* SD0_CMD */
-		{ RCAR_GP_PIN(3,  2), 12, 3 },	/* SD0_DAT0 */
-		{ RCAR_GP_PIN(3,  3),  8, 3 },	/* SD0_DAT1 */
-		{ RCAR_GP_PIN(3,  4),  4, 3 },	/* SD0_DAT2 */
-		{ RCAR_GP_PIN(3,  5),  0, 3 },	/* SD0_DAT3 */
+		{ PIN_A_NUMBER('T', 28), 28, 2 },	/* TDO */
+		{ PIN_A_NUMBER('T', 30), 24, 2 },	/* ASEBRK */
+		{ RCAR_GP_PIN(3,  0),    20, 3 },	/* SD0_CLK */
+		{ RCAR_GP_PIN(3,  1),    16, 3 },	/* SD0_CMD */
+		{ RCAR_GP_PIN(3,  2),    12, 3 },	/* SD0_DAT0 */
+		{ RCAR_GP_PIN(3,  3),     8, 3 },	/* SD0_DAT1 */
+		{ RCAR_GP_PIN(3,  4),     4, 3 },	/* SD0_DAT2 */
+		{ RCAR_GP_PIN(3,  5),     0, 3 },	/* SD0_DAT3 */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL14", 0xe6060338) {
 		{ RCAR_GP_PIN(3,  6), 28, 3 },	/* SD1_CLK */
@@ -5014,6 +5136,7 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
 		{ RCAR_GP_PIN(5, 23), 16, 3 },	/* MLB_CLK */
 		{ RCAR_GP_PIN(5, 24), 12, 3 },	/* MLB_SIG */
 		{ RCAR_GP_PIN(5, 25),  8, 3 },	/* MLB_DAT */
+		{ PIN_NUMBER('H', 37),  4, 3 },	/* MLB_REF */
 		{ RCAR_GP_PIN(6,  0),  0, 3 },	/* SSI_SCK01239 */
 	} },
 	{ PINMUX_DRIVE_REG("DRVCTRL21", 0xe6060354) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins
  2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
  2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
  2016-09-13 14:03 ` [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
@ 2016-09-13 14:03 ` Niklas Söderlund
  2016-09-14  9:05   ` Sergei Shtylyov
  2016-10-05  7:41   ` Geert Uytterhoeven
  2016-09-13 14:03 ` [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins Niklas Söderlund
  2016-10-04 19:09 ` [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Geert Uytterhoeven
  4 siblings, 2 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-13 14:03 UTC (permalink / raw)
  To: geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij, Niklas Söderlund

Group the AVB pins into similar groups found in other sh-pfc drivers.
The pins can not be muxed between functions other then AVB but there
drive strength can be controlled.

The group avb_mdc containing ADV_MDC and ADV_MDIO are on other SoCs
called avb_mdio. In pfc-r8a7795 the adv_mdc group already existed and
is in use in DT. Therefor the ADV_MDIO pin to the existing group
instead of renaming it.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 4a60f15..7b9a355 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -1635,11 +1635,11 @@ static const unsigned int avb_phy_int_mux[] = {
 	AVB_PHY_INT_MARK,
 };
 static const unsigned int avb_mdc_pins[] = {
-	/* AVB_MDC */
-	RCAR_GP_PIN(2, 9),
+	/* AVB_MDC, AVB_MDIO */
+	RCAR_GP_PIN(2, 9), PIN_NUMBER('A', 9),
 };
 static const unsigned int avb_mdc_mux[] = {
-	AVB_MDC_MARK,
+	AVB_MDC_MARK, AVB_MDIO_MARK,
 };
 static const unsigned int avb_avtp_pps_pins[] = {
 	/* AVB_AVTP_PPS */
@@ -1676,6 +1676,28 @@ static const unsigned int avb_avtp_capture_b_pins[] = {
 static const unsigned int avb_avtp_capture_b_mux[] = {
 	AVB_AVTP_CAPTURE_B_MARK,
 };
+static const unsigned int avb_mii_pins[] = {
+	/*
+	 * AVB_TX_CTL, AVB_TXC, AVB_TD0,
+	 * AVB_TD1, AVB_TD2, AVB_TD3,
+	 * AVB_RX_CTL, AVB_RXC, AVB_RD0,
+	 * AVB_RD1, AVB_RD2, AVB_RD3,
+	 * AVB_TXREFCLK
+	 */
+	PIN_NUMBER('A', 8), PIN_NUMBER('A', 19), PIN_NUMBER('A', 18),
+	PIN_NUMBER('B', 18), PIN_NUMBER('A', 17), PIN_NUMBER('B', 17),
+	PIN_NUMBER('A', 16), PIN_NUMBER('B', 19), PIN_NUMBER('A', 13),
+	PIN_NUMBER('B', 13), PIN_NUMBER('A', 14), PIN_NUMBER('B', 14),
+	PIN_NUMBER('A', 12),
+
+};
+static const unsigned int avb_mii_mux[] = {
+	AVB_TX_CTL_MARK, AVB_TXC_MARK, AVB_TD0_MARK,
+	AVB_TD1_MARK, AVB_TD2_MARK, AVB_TD3_MARK,
+	AVB_RX_CTL_MARK, AVB_RXC_MARK, AVB_RD0_MARK,
+	AVB_RD1_MARK, AVB_RD2_MARK, AVB_RD3_MARK,
+	AVB_TXREFCLK_MARK,
+};
 
 /* - CAN ------------------------------------------------------------------ */
 static const unsigned int can0_data_a_pins[] = {
@@ -3632,6 +3654,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(avb_avtp_capture_a),
 	SH_PFC_PIN_GROUP(avb_avtp_match_b),
 	SH_PFC_PIN_GROUP(avb_avtp_capture_b),
+	SH_PFC_PIN_GROUP(avb_mii),
 	SH_PFC_PIN_GROUP(can0_data_a),
 	SH_PFC_PIN_GROUP(can0_data_b),
 	SH_PFC_PIN_GROUP(can1_data),
@@ -3928,6 +3951,7 @@ static const char * const avb_groups[] = {
 	"avb_avtp_capture_a",
 	"avb_avtp_match_b",
 	"avb_avtp_capture_b",
+	"avb_mii",
 };
 
 static const char * const can0_groups[] = {
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins
  2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
                   ` (2 preceding siblings ...)
  2016-09-13 14:03 ` [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins Niklas Söderlund
@ 2016-09-13 14:03 ` Niklas Söderlund
  2016-10-05  7:33   ` Geert Uytterhoeven
  2016-10-04 19:09 ` [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Geert Uytterhoeven
  4 siblings, 1 reply; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-13 14:03 UTC (permalink / raw)
  To: geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij, Niklas Söderlund

Group the QSPI0 and QSPI1 pins into similar groups found in other sh-pfc
drivers.  The pins can not be muxed between functions other then QSPI
but there drive strength can be controlled.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 7b9a355..192e272 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -3627,6 +3627,55 @@ static const unsigned int usb2_mux[] = {
 	USB2_PWEN_MARK, USB2_OVC_MARK,
 };
 
+/* - QSPI0 ------------------------------------------------------------------ */
+static const unsigned int qspi0_ctrl_pins[] = {
+	/* QSPI0_SPCLK, QSPI0_SSL */
+	PIN_NUMBER('W', 3), PIN_NUMBER('Y', 3),
+};
+static const unsigned int qspi0_ctrl_mux[] = {
+	QSPI0_SPCLK_MARK, QSPI0_SSL_MARK,
+};
+static const unsigned int qspi0_data2_pins[] = {
+	/* QSPI0_MOSI_IO0, QSPI0_MISO_IO1 */
+	PIN_A_NUMBER('C', 5), PIN_A_NUMBER('B', 4),
+};
+static const unsigned int qspi0_data2_mux[] = {
+	QSPI0_MOSI_IO0_MARK, QSPI0_MISO_IO1_MARK,
+};
+static const unsigned int qspi0_data4_pins[] = {
+	/* QSPI0_MOSI_IO0, QSPI0_MISO_IO1, QSPI0_IO2, QSPI0_IO3 */
+	PIN_A_NUMBER('C', 5), PIN_A_NUMBER('B', 4),
+	PIN_NUMBER('Y', 6), PIN_A_NUMBER('B', 6),
+};
+static const unsigned int qspi0_data4_mux[] = {
+	QSPI0_MOSI_IO0_MARK, QSPI0_MISO_IO1_MARK,
+	QSPI0_IO2_MARK, QSPI0_IO3_MARK,
+};
+/* - QSPI1 ------------------------------------------------------------------ */
+static const unsigned int qspi1_ctrl_pins[] = {
+	/* QSPI1_SPCLK, QSPI1_SSL */
+	PIN_NUMBER('V', 3), PIN_NUMBER('V', 5),
+};
+static const unsigned int qspi1_ctrl_mux[] = {
+	QSPI1_SPCLK_MARK, QSPI1_SSL_MARK,
+};
+static const unsigned int qspi1_data2_pins[] = {
+	/* QSPI1_MOSI_IO0, QSPI1_MISO_IO1 */
+	PIN_A_NUMBER('C', 7), PIN_A_NUMBER('E', 5),
+};
+static const unsigned int qspi1_data2_mux[] = {
+	QSPI1_MOSI_IO0_MARK, QSPI1_MISO_IO1_MARK,
+};
+static const unsigned int qspi1_data4_pins[] = {
+	/* QSPI1_MOSI_IO0, QSPI1_MISO_IO1, QSPI1_IO2, QSPI1_IO3 */
+	PIN_A_NUMBER('C', 7), PIN_A_NUMBER('E', 5),
+	PIN_A_NUMBER('E', 4), PIN_A_NUMBER('C', 3),
+};
+static const unsigned int qspi1_data4_mux[] = {
+	QSPI1_MOSI_IO0_MARK, QSPI1_MISO_IO1_MARK,
+	QSPI1_IO2_MARK, QSPI1_IO3_MARK,
+};
+
 static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(audio_clk_a_a),
 	SH_PFC_PIN_GROUP(audio_clk_a_b),
@@ -3919,6 +3968,12 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(usb0),
 	SH_PFC_PIN_GROUP(usb1),
 	SH_PFC_PIN_GROUP(usb2),
+	SH_PFC_PIN_GROUP(qspi0_ctrl),
+	SH_PFC_PIN_GROUP(qspi0_data2),
+	SH_PFC_PIN_GROUP(qspi0_data4),
+	SH_PFC_PIN_GROUP(qspi1_ctrl),
+	SH_PFC_PIN_GROUP(qspi1_data2),
+	SH_PFC_PIN_GROUP(qspi1_data4),
 };
 
 static const char * const audio_clk_groups[] = {
@@ -4353,6 +4408,18 @@ static const char * const usb2_groups[] = {
 	"usb2",
 };
 
+static const char * const qspi0_groups[] = {
+	"qspi0_ctrl",
+	"qspi0_data2",
+	"qspi0_data4",
+};
+
+static const char * const qspi1_groups[] = {
+	"qspi1_ctrl",
+	"qspi1_data2",
+	"qspi1_data4",
+};
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(audio_clk),
 	SH_PFC_FUNCTION(avb),
@@ -4401,6 +4468,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(usb0),
 	SH_PFC_FUNCTION(usb1),
 	SH_PFC_FUNCTION(usb2),
+	SH_PFC_FUNCTION(qspi0),
+	SH_PFC_FUNCTION(qspi1),
 };
 
 static const struct pinmux_cfg_reg pinmux_config_regs[] = {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration
  2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
@ 2016-09-13 14:28   ` Laurent Pinchart
  2016-10-04 19:08   ` Geert Uytterhoeven
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2016-09-13 14:28 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: geert+renesas, linux-renesas-soc, linux-gpio, linus.walleij

Hi Niklas,

Thank you for the patch

On Tuesday 13 Sep 2016 16:03:11 Niklas Söderlund wrote:
> Pins not associated with a GPIO port can still have other configuration
> parameters. Add a new macro SH_PFC_PIN_NAMED_CFG which allows for named
> pins to be declared with a set of configurations. The new macro is an
> modification of SH_PFC_PIN_NAMED to allow for optional configuration to
> be assigned.
> 
> The flag SH_PFC_PIN_CFG_NO_GPIO is still enforced as this should only be
> used to define pins not associated with a GPIO port.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/pinctrl/sh-pfc/sh_pfc.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h
> b/drivers/pinctrl/sh-pfc/sh_pfc.h index 5e966c0..a6a2346 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -507,6 +507,14 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
>  		.configs = SH_PFC_PIN_CFG_NO_GPIO,			\
>  	}
> 
> +/* SH_PFC_PIN_NAMED_CFG - Expand to a sh_pfc_pin entry with the given name
> */ +#define SH_PFC_PIN_NAMED_CFG(row, col, _name, cfgs)			
\
> +	{								\
> +		.pin = PIN_NUMBER(row, col),				\
> +		.name = __stringify(PIN_##_name),			\
> +		.configs = SH_PFC_PIN_CFG_NO_GPIO | cfgs,		\
> +	}
> +
>  /* PINMUX_DATA_ALL - Expand to a list of PORT_name_DATA, PORT_name_FN0,
>   *		     PORT_name_OUT, PORT_name_IN marks
>   */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins
  2016-09-13 14:03 ` [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins Niklas Söderlund
@ 2016-09-14  9:05   ` Sergei Shtylyov
  2016-10-05  7:41   ` Geert Uytterhoeven
  1 sibling, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2016-09-14  9:05 UTC (permalink / raw)
  To: Niklas Söderlund, geert+renesas, linux-renesas-soc, linux-gpio
  Cc: laurent.pinchart, linus.walleij

On 9/13/2016 5:03 PM, Niklas Söderlund wrote:

> Group the AVB pins into similar groups found in other sh-pfc drivers.
> The pins can not be muxed between functions other then AVB but there
> drive strength can be controlled.
>
> The group avb_mdc containing ADV_MDC and ADV_MDIO are on other SoCs
> called avb_mdio. In pfc-r8a7795 the adv_mdc group already existed and
                                       ^^^^^^^ avb_mdc?

> is in use in DT. Therefor the ADV_MDIO pin to the existing group

    Therefore. And you missed a verb.

> instead of renaming it.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> index 4a60f15..7b9a355 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
[...]
>  static const unsigned int can0_data_a_pins[] = {
> @@ -3632,6 +3654,7 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(avb_avtp_capture_a),
>  	SH_PFC_PIN_GROUP(avb_avtp_match_b),
>  	SH_PFC_PIN_GROUP(avb_avtp_capture_b),
> +	SH_PFC_PIN_GROUP(avb_mii),
>  	SH_PFC_PIN_GROUP(can0_data_a),
>  	SH_PFC_PIN_GROUP(can0_data_b),
>  	SH_PFC_PIN_GROUP(can1_data),
> @@ -3928,6 +3951,7 @@ static const char * const avb_groups[] = {
>  	"avb_avtp_capture_a",
>  	"avb_avtp_match_b",
>  	"avb_avtp_capture_b",
> +	"avb_mii",

    I'd prefer if this group preceded the capture/match groups.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration
  2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
  2016-09-13 14:28   ` Laurent Pinchart
@ 2016-10-04 19:08   ` Geert Uytterhoeven
  1 sibling, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-04 19:08 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Pins not associated with a GPIO port can still have other configuration
> parameters. Add a new macro SH_PFC_PIN_NAMED_CFG which allows for named
> pins to be declared with a set of configurations. The new macro is an
> modification of SH_PFC_PIN_NAMED to allow for optional configuration to
> be assigned.
>
> The flag SH_PFC_PIN_CFG_NO_GPIO is still enforced as this should only be
> used to define pins not associated with a GPIO port.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
                   ` (3 preceding siblings ...)
  2016-09-13 14:03 ` [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins Niklas Söderlund
@ 2016-10-04 19:09 ` Geert Uytterhoeven
  4 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-04 19:09 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

Hi Niklas,

On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> I did not add FSCLKST since I can't figure out which physical pin it's
> mapped to. Looking at the code that is already there and documentation
> it should be a GPIO pin controlled by IPSR7[15:12] but the documentation
> and code is lacking that part and I can't with a 100% certainty figure
> out which physical pin it is.

FSCLKST is pin AD38, according to R-CarH3SiP_pin_arrangement rev1.01.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-09-13 14:03 ` [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
@ 2016-10-04 19:13   ` Geert Uytterhoeven
  2016-10-05  8:33       ` Niklas Söderlund
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-04 19:13 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

Hi Niklas,

On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> There are pins on the Salvator-X which is not part of a GPIO bank nor

s/Salvator-X/r8a7795/
s/is/are/

> can be muxed between different functions. They do however allow for the
> drive-strength to be configured. Add those pins to the list of pins and
> to the drive-strength configuration registers.
>
> The pins can now be referred to in DT by there physical location and the

s/there/their/

> drive-strength modified.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 155 +++++++++++++++++++++++++++++++----
>  1 file changed, 139 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> index b74cdd3..4a60f15 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -518,7 +518,22 @@ MOD_SEL0_3         MOD_SEL1_3 \
>  MOD_SEL0_2_1           MOD_SEL1_2 \
>                         MOD_SEL1_1 \
>                         MOD_SEL1_0              MOD_SEL2_0
> -
> +/*
> + * These pins are not able to be muxed but have other properties
> + * that can be set, such as drive-strength or pull-up/pull-down enable.
> + */
> +#define PINMUX_STATIC \
> +       FM(QSPI0_SPCLK) FM(QSPI0_SSL) FM(QSPI0_MOSI_IO0) FM(QSPI0_MISO_IO1) \
> +       FM(QSPI0_IO2) FM(QSPI0_IO3) \
> +       FM(QSPI1_SPCLK) FM(QSPI1_SSL) FM(QSPI1_MOSI_IO0) FM(QSPI1_MISO_IO1) \
> +       FM(QSPI1_IO2) FM(QSPI1_IO3) \
> +       FM(RPC_INT) FM(RPC_WP) FM(RPC_RESET) \
> +       FM(AVB_TX_CTL) FM(AVB_TXC) FM(AVB_TD0) FM(AVB_TD1) FM(AVB_TD2) FM(AVB_TD3) \
> +       FM(AVB_RX_CTL) FM(AVB_RXC) FM(AVB_RD0) FM(AVB_RD1) FM(AVB_RD2) FM(AVB_RD3) \
> +       FM(AVB_TXREFCLK) FM(AVB_MDIO) \
> +       FM(CLKOUT) FM(PRESETOUT) \
> +       FM(DU_DOTCLKIN0) FM(DU_DOTCLKIN1) FM(DU_DOTCLKIN2) FM(DU_DOTCLKIN3) \
> +       FM(TMS) FM(TDO) FM(ASEBRK) FM(MLB_REF)

FM(FSCLKST)

> @@ -1408,10 +1424,70 @@ static const u16 pinmux_data[] = {
>         PINMUX_IPSR_MSEL(IP17_7_4,      STP_ISSYNC_0_E,         SEL_SSP1_0_4),
>         PINMUX_IPSR_MSEL(IP17_7_4,      RIF2_D1_B,              SEL_DRIF2_1),
>         PINMUX_IPSR_GPSR(IP17_7_4,      TPU0TO3),
> +
> +/*
> + * Static pins can not be muxed between different functions but
> + * still needs a mark entry in the pinmux list. Add each static
> + * pin to the list without an associated function. The sh-pfc
> + * core will do the right thing and skip trying to mux then pin
> + * while still applying configuration to it
> + */
> +#define FM(x)  PINMUX_DATA(x##_MARK, 0),
> +       PINMUX_STATIC
> +#undef FM
>  };
>
> +/*
> + * R8A7795 has 7 banks with 32 PGIOS in each = 224 GPIOs.

No, it has 8 banks => 256 GPIOs

> + * Physical layout rows: A - AW, cols: 1 - 39.
> + */
> +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> +#define PIN_NUMBER(r, c) (((r) - 'A') * 39 + (c) + 300)

Fortunately 300 >= 256 :-)

> +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> +
>  static const struct sh_pfc_pin pinmux_pins[] = {
>         PINMUX_GPIO_GP_ALL(),
> +
> +       /* Pins not associated with a GPIO port */
> +       SH_PFC_PIN_NAMED_CFG('A',  8,  A8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TX_CTL */
> +       SH_PFC_PIN_NAMED_CFG('A',  9,  A9, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_MDIO */
> +       SH_PFC_PIN_NAMED_CFG('A', 12, A12, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXCREFCLK */
> +       SH_PFC_PIN_NAMED_CFG('A', 13, A13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD0 */
> +       SH_PFC_PIN_NAMED_CFG('A', 14, A14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD2 */
> +       SH_PFC_PIN_NAMED_CFG('A', 16, A16, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RX_CTL */
> +       SH_PFC_PIN_NAMED_CFG('A', 17, A17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD2 */
> +       SH_PFC_PIN_NAMED_CFG('A', 18, A18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD0 */
> +       SH_PFC_PIN_NAMED_CFG('A', 19, A19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXC */
> +       SH_PFC_PIN_NAMED_CFG('B', 13, B13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD1 */
> +       SH_PFC_PIN_NAMED_CFG('B', 14, B14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD3 */
> +       SH_PFC_PIN_NAMED_CFG('B', 17, B17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD3 */
> +       SH_PFC_PIN_NAMED_CFG('B', 18, B18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD1 */
> +       SH_PFC_PIN_NAMED_CFG('B', 19, B19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RXC */
> +       SH_PFC_PIN_NAMED_CFG('C',  1,  C1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* PRESETOUT# */
> +       SH_PFC_PIN_NAMED_CFG('F',  1,  F1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* CLKOUT */
> +       SH_PFC_PIN_NAMED_CFG('H', 37, H37, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* MLB_REF */
> +       SH_PFC_PIN_NAMED_CFG('V',  3,  V3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SPCLK */
> +       SH_PFC_PIN_NAMED_CFG('V',  5,  V5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SSL */
> +       SH_PFC_PIN_NAMED_CFG('V',  6,  V6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_WP# */
> +       SH_PFC_PIN_NAMED_CFG('V',  7,  V7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_RESET# */
> +       SH_PFC_PIN_NAMED_CFG('W',  3,  W3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SPCLK */
> +       SH_PFC_PIN_NAMED_CFG('Y',  3,  Y3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SSL */
> +       SH_PFC_PIN_NAMED_CFG('Y',  6,  Y6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_IO2 */
> +       SH_PFC_PIN_NAMED_CFG('Y',  7,  Y7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_INT# */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  4,  AB4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MISO_IO1 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  6,  AB6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_IO3 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  3,  AC3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO3 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  5,  AC5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MOSI_IO0 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  7,  AC7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MOSI_IO0 */

+ SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('D'),  38,  AD38,
SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* FSCLKST */

> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */

All these pin numbers match R-Car H3SiP, while there exists also a plain
R-Car H3, which uses completely different pin numbers.

How are we gonna distinguish these two variants?
Perhaps we can refer to these pins in some other way, to have consistent
numbering?

Or don't we have to? Are these numbers visible in userspace (sysfs)?

> @@ -4932,20 +5045,29 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
>                 { RCAR_GP_PIN(0, 13),  0, 3 },  /* D13 */
>         } },
>         { PINMUX_DRIVE_REG("DRVCTRL11", 0xe606032c) {
> -               { RCAR_GP_PIN(0, 14), 28, 3 },  /* D14 */
> -               { RCAR_GP_PIN(0, 15), 24, 3 },  /* D15 */
> -               { RCAR_GP_PIN(7,  0), 20, 3 },  /* AVS1 */
> -               { RCAR_GP_PIN(7,  1), 16, 3 },  /* AVS2 */
> -               { RCAR_GP_PIN(7,  2), 12, 3 },  /* HDMI0_CEC */
> -               { RCAR_GP_PIN(7,  3),  8, 3 },  /* HDMI1_CEC */
> +               { RCAR_GP_PIN(0, 14),   28, 3 },        /* D14 */
> +               { RCAR_GP_PIN(0, 15),   24, 3 },        /* D15 */
> +               { RCAR_GP_PIN(7,  0),   20, 3 },        /* AVS1 */
> +               { RCAR_GP_PIN(7,  1),   16, 3 },        /* AVS2 */
> +               { RCAR_GP_PIN(7,  2),   12, 3 },        /* HDMI0_CEC */
> +               { RCAR_GP_PIN(7,  3),    8, 3 },        /* HDMI1_CEC */
> +               { PIN_A_NUMBER('P', 7),  4, 2 },        /* DU_DOTCLKIN0 */
> +               { PIN_A_NUMBER('P', 8),  0, 2 },        /* DU_DOTCLKIN1 */
> +       } },
> +       { PINMUX_DRIVE_REG("DRVCTRL12", 0xe6060330) {
> +               { PIN_A_NUMBER('R', 7),  28, 2 },       /* DU_DOTCLKIN2 */
> +               { PIN_A_NUMBER('R', 8),  24, 2 },       /* DU_DOTCLKIN3 */

{ PIN_A_NUMBER('D', 38), 20, 2 },        /* FSCLKST */

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins
  2016-09-13 14:03 ` [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins Niklas Söderlund
@ 2016-10-05  7:33   ` Geert Uytterhoeven
  0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-05  7:33 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Group the QSPI0 and QSPI1 pins into similar groups found in other sh-pfc
> drivers.  The pins can not be muxed between functions other then QSPI
> but there drive strength can be controlled.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins
  2016-09-13 14:03 ` [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins Niklas Söderlund
  2016-09-14  9:05   ` Sergei Shtylyov
@ 2016-10-05  7:41   ` Geert Uytterhoeven
  1 sibling, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-05  7:41 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Group the AVB pins into similar groups found in other sh-pfc drivers.
> The pins can not be muxed between functions other then AVB but there

s/there/their/

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-10-04 19:13   ` Geert Uytterhoeven
@ 2016-10-05  8:33       ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-10-05  8:33 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

Hi Geert,

Thanks for your feedback.

On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > There are pins on the Salvator-X which is not part of a GPIO bank nor
> 
> s/Salvator-X/r8a7795/
> s/is/are/

Thanks.

> 
> > can be muxed between different functions. They do however allow for the
> > drive-strength to be configured. Add those pins to the list of pins and
> > to the drive-strength configuration registers.
> >
> > The pins can now be referred to in DT by there physical location and the
> 
> s/there/their/

Thanks.

> 
> > drive-strength modified.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 155 +++++++++++++++++++++++++++++++----
> >  1 file changed, 139 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > index b74cdd3..4a60f15 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > @@ -518,7 +518,22 @@ MOD_SEL0_3         MOD_SEL1_3 \
> >  MOD_SEL0_2_1           MOD_SEL1_2 \
> >                         MOD_SEL1_1 \
> >                         MOD_SEL1_0              MOD_SEL2_0
> > -
> > +/*
> > + * These pins are not able to be muxed but have other properties
> > + * that can be set, such as drive-strength or pull-up/pull-down enable.
> > + */
> > +#define PINMUX_STATIC \
> > +       FM(QSPI0_SPCLK) FM(QSPI0_SSL) FM(QSPI0_MOSI_IO0) FM(QSPI0_MISO_IO1) \
> > +       FM(QSPI0_IO2) FM(QSPI0_IO3) \
> > +       FM(QSPI1_SPCLK) FM(QSPI1_SSL) FM(QSPI1_MOSI_IO0) FM(QSPI1_MISO_IO1) \
> > +       FM(QSPI1_IO2) FM(QSPI1_IO3) \
> > +       FM(RPC_INT) FM(RPC_WP) FM(RPC_RESET) \
> > +       FM(AVB_TX_CTL) FM(AVB_TXC) FM(AVB_TD0) FM(AVB_TD1) FM(AVB_TD2) FM(AVB_TD3) \
> > +       FM(AVB_RX_CTL) FM(AVB_RXC) FM(AVB_RD0) FM(AVB_RD1) FM(AVB_RD2) FM(AVB_RD3) \
> > +       FM(AVB_TXREFCLK) FM(AVB_MDIO) \
> > +       FM(CLKOUT) FM(PRESETOUT) \
> > +       FM(DU_DOTCLKIN0) FM(DU_DOTCLKIN1) FM(DU_DOTCLKIN2) FM(DU_DOTCLKIN3) \
> > +       FM(TMS) FM(TDO) FM(ASEBRK) FM(MLB_REF)
> 
> FM(FSCLKST)

This won't work as FSCLKST is already defined as a part of IP7_15_12, so 
adding it here will result in 'error: redeclaration of enumerator 
'FSCLKST_MARK'. It matches the datasheet I have that FSCLKST is part of 
the ISPR7 register, but I can't find it as GPIO so I'm a bit confused 
how to handle it and chose to leave it out. Any suggestion on how to 
handle it is appreciated.

> 
> > @@ -1408,10 +1424,70 @@ static const u16 pinmux_data[] = {
> >         PINMUX_IPSR_MSEL(IP17_7_4,      STP_ISSYNC_0_E,         SEL_SSP1_0_4),
> >         PINMUX_IPSR_MSEL(IP17_7_4,      RIF2_D1_B,              SEL_DRIF2_1),
> >         PINMUX_IPSR_GPSR(IP17_7_4,      TPU0TO3),
> > +
> > +/*
> > + * Static pins can not be muxed between different functions but
> > + * still needs a mark entry in the pinmux list. Add each static
> > + * pin to the list without an associated function. The sh-pfc
> > + * core will do the right thing and skip trying to mux then pin
> > + * while still applying configuration to it
> > + */
> > +#define FM(x)  PINMUX_DATA(x##_MARK, 0),
> > +       PINMUX_STATIC
> > +#undef FM
> >  };
> >
> > +/*
> > + * R8A7795 has 7 banks with 32 PGIOS in each = 224 GPIOs.
> 
> No, it has 8 banks => 256 GPIOs

Yes, sometime calculating is hard for me, GPSR0-7 is 8 banks not 7...  
Thanks for pointing this out.

> 
> > + * Physical layout rows: A - AW, cols: 1 - 39.
> > + */
> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 39 + (c) + 300)
> 
> Fortunately 300 >= 256 :-)

:-)

> 
> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> > +
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> >         PINMUX_GPIO_GP_ALL(),
> > +
> > +       /* Pins not associated with a GPIO port */
> > +       SH_PFC_PIN_NAMED_CFG('A',  8,  A8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TX_CTL */
> > +       SH_PFC_PIN_NAMED_CFG('A',  9,  A9, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_MDIO */
> > +       SH_PFC_PIN_NAMED_CFG('A', 12, A12, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXCREFCLK */
> > +       SH_PFC_PIN_NAMED_CFG('A', 13, A13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD0 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 14, A14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD2 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 16, A16, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RX_CTL */
> > +       SH_PFC_PIN_NAMED_CFG('A', 17, A17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD2 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 18, A18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD0 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 19, A19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXC */
> > +       SH_PFC_PIN_NAMED_CFG('B', 13, B13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD1 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 14, B14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD3 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 17, B17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD3 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 18, B18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD1 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 19, B19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RXC */
> > +       SH_PFC_PIN_NAMED_CFG('C',  1,  C1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* PRESETOUT# */
> > +       SH_PFC_PIN_NAMED_CFG('F',  1,  F1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* CLKOUT */
> > +       SH_PFC_PIN_NAMED_CFG('H', 37, H37, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* MLB_REF */
> > +       SH_PFC_PIN_NAMED_CFG('V',  3,  V3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SPCLK */
> > +       SH_PFC_PIN_NAMED_CFG('V',  5,  V5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SSL */
> > +       SH_PFC_PIN_NAMED_CFG('V',  6,  V6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_WP# */
> > +       SH_PFC_PIN_NAMED_CFG('V',  7,  V7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_RESET# */
> > +       SH_PFC_PIN_NAMED_CFG('W',  3,  W3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SPCLK */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  3,  Y3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SSL */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  6,  Y6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_IO2 */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  7,  Y7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_INT# */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  4,  AB4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MISO_IO1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  6,  AB6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_IO3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  3,  AC3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  5,  AC5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MOSI_IO0 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  7,  AC7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MOSI_IO0 */
> 
> + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('D'),  38,  AD38,
> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* FSCLKST */
> 
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> 
> All these pin numbers match R-Car H3SiP, while there exists also a plain
> R-Car H3, which uses completely different pin numbers.
> 
> How are we gonna distinguish these two variants?
> Perhaps we can refer to these pins in some other way, to have consistent
> numbering?
> 
> Or don't we have to? Are these numbers visible in userspace (sysfs)?

Unfortunately both the number and name are show in sysfs under 
'/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:

<snip>
pin 1906 (PIN_AP7) sh-pfc
pin 1907 (PIN_AP8) sh-pfc
pin 1984 (PIN_AR7) sh-pfc
pin 1985 (PIN_AR8) sh-pfc
pin 2007 (PIN_AR30) sh-pfc
pin 2083 (PIN_AT28) sh-pfc
pin 2085 (PIN_AT30) sh-pfc
</snip>

So yes a way to present consistent names is needed if this driver should 
match both H3 variants. But I'm not sure the numbers needs to be 
correlated to the pin matrix they only need to be unique I think, please 
correct me if I'm wrong. And if that is the case then maybe a solution 
to the problem is to simply change the name of the pins from there pin 
matrix location to there function:

- SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
+ SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK, SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */

That would keep the names and numbers consistent on both H3 varinats.  
The names would correlate to function and the numbers simply serve as a 
pin identifier which is unique and derived from the H3SiP pin layout, 
probably a comment about this in the source is a good idea :-)

Let me know what you think about this solution and I will work it in to 
the next version of this series.

> 
> > @@ -4932,20 +5045,29 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
> >                 { RCAR_GP_PIN(0, 13),  0, 3 },  /* D13 */
> >         } },
> >         { PINMUX_DRIVE_REG("DRVCTRL11", 0xe606032c) {
> > -               { RCAR_GP_PIN(0, 14), 28, 3 },  /* D14 */
> > -               { RCAR_GP_PIN(0, 15), 24, 3 },  /* D15 */
> > -               { RCAR_GP_PIN(7,  0), 20, 3 },  /* AVS1 */
> > -               { RCAR_GP_PIN(7,  1), 16, 3 },  /* AVS2 */
> > -               { RCAR_GP_PIN(7,  2), 12, 3 },  /* HDMI0_CEC */
> > -               { RCAR_GP_PIN(7,  3),  8, 3 },  /* HDMI1_CEC */
> > +               { RCAR_GP_PIN(0, 14),   28, 3 },        /* D14 */
> > +               { RCAR_GP_PIN(0, 15),   24, 3 },        /* D15 */
> > +               { RCAR_GP_PIN(7,  0),   20, 3 },        /* AVS1 */
> > +               { RCAR_GP_PIN(7,  1),   16, 3 },        /* AVS2 */
> > +               { RCAR_GP_PIN(7,  2),   12, 3 },        /* HDMI0_CEC */
> > +               { RCAR_GP_PIN(7,  3),    8, 3 },        /* HDMI1_CEC */
> > +               { PIN_A_NUMBER('P', 7),  4, 2 },        /* DU_DOTCLKIN0 */
> > +               { PIN_A_NUMBER('P', 8),  0, 2 },        /* DU_DOTCLKIN1 */
> > +       } },
> > +       { PINMUX_DRIVE_REG("DRVCTRL12", 0xe6060330) {
> > +               { PIN_A_NUMBER('R', 7),  28, 2 },       /* DU_DOTCLKIN2 */
> > +               { PIN_A_NUMBER('R', 8),  24, 2 },       /* DU_DOTCLKIN3 */
> 
> { PIN_A_NUMBER('D', 38), 20, 2 },        /* FSCLKST */
> 
> 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

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
@ 2016-10-05  8:33       ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-10-05  8:33 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

Hi Geert,

Thanks for your feedback.

On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Tue, Sep 13, 2016 at 4:03 PM, Niklas S�derlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > There are pins on the Salvator-X which is not part of a GPIO bank nor
> 
> s/Salvator-X/r8a7795/
> s/is/are/

Thanks.

> 
> > can be muxed between different functions. They do however allow for the
> > drive-strength to be configured. Add those pins to the list of pins and
> > to the drive-strength configuration registers.
> >
> > The pins can now be referred to in DT by there physical location and the
> 
> s/there/their/

Thanks.

> 
> > drive-strength modified.
> >
> > Signed-off-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 155 +++++++++++++++++++++++++++++++----
> >  1 file changed, 139 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > index b74cdd3..4a60f15 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > @@ -518,7 +518,22 @@ MOD_SEL0_3         MOD_SEL1_3 \
> >  MOD_SEL0_2_1           MOD_SEL1_2 \
> >                         MOD_SEL1_1 \
> >                         MOD_SEL1_0              MOD_SEL2_0
> > -
> > +/*
> > + * These pins are not able to be muxed but have other properties
> > + * that can be set, such as drive-strength or pull-up/pull-down enable.
> > + */
> > +#define PINMUX_STATIC \
> > +       FM(QSPI0_SPCLK) FM(QSPI0_SSL) FM(QSPI0_MOSI_IO0) FM(QSPI0_MISO_IO1) \
> > +       FM(QSPI0_IO2) FM(QSPI0_IO3) \
> > +       FM(QSPI1_SPCLK) FM(QSPI1_SSL) FM(QSPI1_MOSI_IO0) FM(QSPI1_MISO_IO1) \
> > +       FM(QSPI1_IO2) FM(QSPI1_IO3) \
> > +       FM(RPC_INT) FM(RPC_WP) FM(RPC_RESET) \
> > +       FM(AVB_TX_CTL) FM(AVB_TXC) FM(AVB_TD0) FM(AVB_TD1) FM(AVB_TD2) FM(AVB_TD3) \
> > +       FM(AVB_RX_CTL) FM(AVB_RXC) FM(AVB_RD0) FM(AVB_RD1) FM(AVB_RD2) FM(AVB_RD3) \
> > +       FM(AVB_TXREFCLK) FM(AVB_MDIO) \
> > +       FM(CLKOUT) FM(PRESETOUT) \
> > +       FM(DU_DOTCLKIN0) FM(DU_DOTCLKIN1) FM(DU_DOTCLKIN2) FM(DU_DOTCLKIN3) \
> > +       FM(TMS) FM(TDO) FM(ASEBRK) FM(MLB_REF)
> 
> FM(FSCLKST)

This won't work as FSCLKST is already defined as a part of IP7_15_12, so 
adding it here will result in 'error: redeclaration of enumerator 
'FSCLKST_MARK'. It matches the datasheet I have that FSCLKST is part of 
the ISPR7 register, but I can't find it as GPIO so I'm a bit confused 
how to handle it and chose to leave it out. Any suggestion on how to 
handle it is appreciated.

> 
> > @@ -1408,10 +1424,70 @@ static const u16 pinmux_data[] = {
> >         PINMUX_IPSR_MSEL(IP17_7_4,      STP_ISSYNC_0_E,         SEL_SSP1_0_4),
> >         PINMUX_IPSR_MSEL(IP17_7_4,      RIF2_D1_B,              SEL_DRIF2_1),
> >         PINMUX_IPSR_GPSR(IP17_7_4,      TPU0TO3),
> > +
> > +/*
> > + * Static pins can not be muxed between different functions but
> > + * still needs a mark entry in the pinmux list. Add each static
> > + * pin to the list without an associated function. The sh-pfc
> > + * core will do the right thing and skip trying to mux then pin
> > + * while still applying configuration to it
> > + */
> > +#define FM(x)  PINMUX_DATA(x##_MARK, 0),
> > +       PINMUX_STATIC
> > +#undef FM
> >  };
> >
> > +/*
> > + * R8A7795 has 7 banks with 32 PGIOS in each = 224 GPIOs.
> 
> No, it has 8 banks => 256 GPIOs

Yes, sometime calculating is hard for me, GPSR0-7 is 8 banks not 7...  
Thanks for pointing this out.

> 
> > + * Physical layout rows: A - AW, cols: 1 - 39.
> > + */
> > +#define ROW_GROUP_A(r) ('Z' - 'A' + 1 + (r))
> > +#define PIN_NUMBER(r, c) (((r) - 'A') * 39 + (c) + 300)
> 
> Fortunately 300 >= 256 :-)

:-)

> 
> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
> > +
> >  static const struct sh_pfc_pin pinmux_pins[] = {
> >         PINMUX_GPIO_GP_ALL(),
> > +
> > +       /* Pins not associated with a GPIO port */
> > +       SH_PFC_PIN_NAMED_CFG('A',  8,  A8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TX_CTL */
> > +       SH_PFC_PIN_NAMED_CFG('A',  9,  A9, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_MDIO */
> > +       SH_PFC_PIN_NAMED_CFG('A', 12, A12, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXCREFCLK */
> > +       SH_PFC_PIN_NAMED_CFG('A', 13, A13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD0 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 14, A14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD2 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 16, A16, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RX_CTL */
> > +       SH_PFC_PIN_NAMED_CFG('A', 17, A17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD2 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 18, A18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD0 */
> > +       SH_PFC_PIN_NAMED_CFG('A', 19, A19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXC */
> > +       SH_PFC_PIN_NAMED_CFG('B', 13, B13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD1 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 14, B14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD3 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 17, B17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD3 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 18, B18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD1 */
> > +       SH_PFC_PIN_NAMED_CFG('B', 19, B19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RXC */
> > +       SH_PFC_PIN_NAMED_CFG('C',  1,  C1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* PRESETOUT# */
> > +       SH_PFC_PIN_NAMED_CFG('F',  1,  F1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* CLKOUT */
> > +       SH_PFC_PIN_NAMED_CFG('H', 37, H37, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* MLB_REF */
> > +       SH_PFC_PIN_NAMED_CFG('V',  3,  V3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SPCLK */
> > +       SH_PFC_PIN_NAMED_CFG('V',  5,  V5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SSL */
> > +       SH_PFC_PIN_NAMED_CFG('V',  6,  V6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_WP# */
> > +       SH_PFC_PIN_NAMED_CFG('V',  7,  V7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_RESET# */
> > +       SH_PFC_PIN_NAMED_CFG('W',  3,  W3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SPCLK */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  3,  Y3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SSL */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  6,  Y6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_IO2 */
> > +       SH_PFC_PIN_NAMED_CFG('Y',  7,  Y7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_INT# */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  4,  AB4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MISO_IO1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  6,  AB6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_IO3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  3,  AC3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  5,  AC5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MOSI_IO0 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  7,  AC7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MOSI_IO0 */
> 
> + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('D'),  38,  AD38,
> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* FSCLKST */
> 
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> 
> All these pin numbers match R-Car H3SiP, while there exists also a plain
> R-Car H3, which uses completely different pin numbers.
> 
> How are we gonna distinguish these two variants?
> Perhaps we can refer to these pins in some other way, to have consistent
> numbering?
> 
> Or don't we have to? Are these numbers visible in userspace (sysfs)?

Unfortunately both the number and name are show in sysfs under 
'/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:

<snip>
pin 1906 (PIN_AP7) sh-pfc
pin 1907 (PIN_AP8) sh-pfc
pin 1984 (PIN_AR7) sh-pfc
pin 1985 (PIN_AR8) sh-pfc
pin 2007 (PIN_AR30) sh-pfc
pin 2083 (PIN_AT28) sh-pfc
pin 2085 (PIN_AT30) sh-pfc
</snip>

So yes a way to present consistent names is needed if this driver should 
match both H3 variants. But I'm not sure the numbers needs to be 
correlated to the pin matrix they only need to be unique I think, please 
correct me if I'm wrong. And if that is the case then maybe a solution 
to the problem is to simply change the name of the pins from there pin 
matrix location to there function:

- SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
+ SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK, SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */

That would keep the names and numbers consistent on both H3 varinats.  
The names would correlate to function and the numbers simply serve as a 
pin identifier which is unique and derived from the H3SiP pin layout, 
probably a comment about this in the source is a good idea :-)

Let me know what you think about this solution and I will work it in to 
the next version of this series.

> 
> > @@ -4932,20 +5045,29 @@ static const struct pinmux_drive_reg pinmux_drive_regs[] = {
> >                 { RCAR_GP_PIN(0, 13),  0, 3 },  /* D13 */
> >         } },
> >         { PINMUX_DRIVE_REG("DRVCTRL11", 0xe606032c) {
> > -               { RCAR_GP_PIN(0, 14), 28, 3 },  /* D14 */
> > -               { RCAR_GP_PIN(0, 15), 24, 3 },  /* D15 */
> > -               { RCAR_GP_PIN(7,  0), 20, 3 },  /* AVS1 */
> > -               { RCAR_GP_PIN(7,  1), 16, 3 },  /* AVS2 */
> > -               { RCAR_GP_PIN(7,  2), 12, 3 },  /* HDMI0_CEC */
> > -               { RCAR_GP_PIN(7,  3),  8, 3 },  /* HDMI1_CEC */
> > +               { RCAR_GP_PIN(0, 14),   28, 3 },        /* D14 */
> > +               { RCAR_GP_PIN(0, 15),   24, 3 },        /* D15 */
> > +               { RCAR_GP_PIN(7,  0),   20, 3 },        /* AVS1 */
> > +               { RCAR_GP_PIN(7,  1),   16, 3 },        /* AVS2 */
> > +               { RCAR_GP_PIN(7,  2),   12, 3 },        /* HDMI0_CEC */
> > +               { RCAR_GP_PIN(7,  3),    8, 3 },        /* HDMI1_CEC */
> > +               { PIN_A_NUMBER('P', 7),  4, 2 },        /* DU_DOTCLKIN0 */
> > +               { PIN_A_NUMBER('P', 8),  0, 2 },        /* DU_DOTCLKIN1 */
> > +       } },
> > +       { PINMUX_DRIVE_REG("DRVCTRL12", 0xe6060330) {
> > +               { PIN_A_NUMBER('R', 7),  28, 2 },       /* DU_DOTCLKIN2 */
> > +               { PIN_A_NUMBER('R', 8),  24, 2 },       /* DU_DOTCLKIN3 */
> 
> { PIN_A_NUMBER('D', 38), 20, 2 },        /* FSCLKST */
> 
> 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

-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-10-05  8:33       ` Niklas Söderlund
  (?)
@ 2016-10-05  9:51       ` Geert Uytterhoeven
  2016-10-05 10:12         ` Laurent Pinchart
  -1 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-10-05  9:51 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linux-Renesas, linux-gpio, Laurent Pinchart,
	Linus Walleij

Hi Niklas,

On Wed, Oct 5, 2016 at 10:33 AM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
>> On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund
>> <niklas.soderlund+renesas@ragnatech.se> wrote:
>> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> > @@ -518,7 +518,22 @@ MOD_SEL0_3         MOD_SEL1_3 \
>> >  MOD_SEL0_2_1           MOD_SEL1_2 \
>> >                         MOD_SEL1_1 \
>> >                         MOD_SEL1_0              MOD_SEL2_0
>> > -
>> > +/*
>> > + * These pins are not able to be muxed but have other properties
>> > + * that can be set, such as drive-strength or pull-up/pull-down enable.
>> > + */
>> > +#define PINMUX_STATIC \
>> > +       FM(QSPI0_SPCLK) FM(QSPI0_SSL) FM(QSPI0_MOSI_IO0) FM(QSPI0_MISO_IO1) \
>> > +       FM(QSPI0_IO2) FM(QSPI0_IO3) \
>> > +       FM(QSPI1_SPCLK) FM(QSPI1_SSL) FM(QSPI1_MOSI_IO0) FM(QSPI1_MISO_IO1) \
>> > +       FM(QSPI1_IO2) FM(QSPI1_IO3) \
>> > +       FM(RPC_INT) FM(RPC_WP) FM(RPC_RESET) \
>> > +       FM(AVB_TX_CTL) FM(AVB_TXC) FM(AVB_TD0) FM(AVB_TD1) FM(AVB_TD2) FM(AVB_TD3) \
>> > +       FM(AVB_RX_CTL) FM(AVB_RXC) FM(AVB_RD0) FM(AVB_RD1) FM(AVB_RD2) FM(AVB_RD3) \
>> > +       FM(AVB_TXREFCLK) FM(AVB_MDIO) \
>> > +       FM(CLKOUT) FM(PRESETOUT) \
>> > +       FM(DU_DOTCLKIN0) FM(DU_DOTCLKIN1) FM(DU_DOTCLKIN2) FM(DU_DOTCLKIN3) \
>> > +       FM(TMS) FM(TDO) FM(ASEBRK) FM(MLB_REF)
>>
>> FM(FSCLKST)
>
> This won't work as FSCLKST is already defined as a part of IP7_15_12, so
> adding it here will result in 'error: redeclaration of enumerator
> 'FSCLKST_MARK'. It matches the datasheet I have that FSCLKST is part of
> the ISPR7 register, but I can't find it as GPIO so I'm a bit confused
> how to handle it and chose to leave it out. Any suggestion on how to
> handle it is appreciated.

IPSR is not about selecting GPIO vs. another function (we have GPSR for that),
but about selecting between multiple functions.
This pin seems to need to be programmed for this function, while it is
documented to be single function (or secret multi function ;-)

So I think we can just use the existing FSCLKST_MARK definition?

>> > +#define PIN_A_NUMBER(r, c) PIN_NUMBER(ROW_GROUP_A(r), c)
>> > +
>> >  static const struct sh_pfc_pin pinmux_pins[] = {
>> >         PINMUX_GPIO_GP_ALL(),
>> > +
>> > +       /* Pins not associated with a GPIO port */
>> > +       SH_PFC_PIN_NAMED_CFG('A',  8,  A8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TX_CTL */
>> > +       SH_PFC_PIN_NAMED_CFG('A',  9,  A9, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_MDIO */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 12, A12, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXCREFCLK */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 13, A13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD0 */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 14, A14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD2 */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 16, A16, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RX_CTL */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 17, A17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD2 */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 18, A18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD0 */
>> > +       SH_PFC_PIN_NAMED_CFG('A', 19, A19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TXC */
>> > +       SH_PFC_PIN_NAMED_CFG('B', 13, B13, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD1 */
>> > +       SH_PFC_PIN_NAMED_CFG('B', 14, B14, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RD3 */
>> > +       SH_PFC_PIN_NAMED_CFG('B', 17, B17, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD3 */
>> > +       SH_PFC_PIN_NAMED_CFG('B', 18, B18, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_TD1 */
>> > +       SH_PFC_PIN_NAMED_CFG('B', 19, B19, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* AVB_RXC */
>> > +       SH_PFC_PIN_NAMED_CFG('C',  1,  C1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* PRESETOUT# */
>> > +       SH_PFC_PIN_NAMED_CFG('F',  1,  F1, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* CLKOUT */
>> > +       SH_PFC_PIN_NAMED_CFG('H', 37, H37, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* MLB_REF */
>> > +       SH_PFC_PIN_NAMED_CFG('V',  3,  V3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SPCLK */
>> > +       SH_PFC_PIN_NAMED_CFG('V',  5,  V5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI1_SSL */
>> > +       SH_PFC_PIN_NAMED_CFG('V',  6,  V6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_WP# */
>> > +       SH_PFC_PIN_NAMED_CFG('V',  7,  V7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_RESET# */
>> > +       SH_PFC_PIN_NAMED_CFG('W',  3,  W3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SPCLK */
>> > +       SH_PFC_PIN_NAMED_CFG('Y',  3,  Y3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_SSL */
>> > +       SH_PFC_PIN_NAMED_CFG('Y',  6,  Y6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* QSPI0_IO2 */
>> > +       SH_PFC_PIN_NAMED_CFG('Y',  7,  Y7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),                      /* RPC_INT# */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  4,  AB4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MISO_IO1 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('B'),  6,  AB6, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_IO3 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  3,  AC3, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO3 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  5,  AC5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI0_MOSI_IO0 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('C'),  7,  AC7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MOSI_IO0 */
>>
>> + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('D'),  38,  AD38,
>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* FSCLKST */
>>
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
>> > +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
>>
>> All these pin numbers match R-Car H3SiP, while there exists also a plain
>> R-Car H3, which uses completely different pin numbers.
>>
>> How are we gonna distinguish these two variants?
>> Perhaps we can refer to these pins in some other way, to have consistent
>> numbering?
>>
>> Or don't we have to? Are these numbers visible in userspace (sysfs)?
>
> Unfortunately both the number and name are show in sysfs under
> '/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:
>
> <snip>
> pin 1906 (PIN_AP7) sh-pfc
> pin 1907 (PIN_AP8) sh-pfc
> pin 1984 (PIN_AR7) sh-pfc
> pin 1985 (PIN_AR8) sh-pfc
> pin 2007 (PIN_AR30) sh-pfc
> pin 2083 (PIN_AT28) sh-pfc
> pin 2085 (PIN_AT30) sh-pfc
> </snip>

Thanks for checking!

> So yes a way to present consistent names is needed if this driver should
> match both H3 variants. But I'm not sure the numbers needs to be
> correlated to the pin matrix they only need to be unique I think, please
> correct me if I'm wrong. And if that is the case then maybe a solution

Yes, I also think they just have to be unique.
Having some system to make it easier to have unique numbers is nice.

> to the problem is to simply change the name of the pins from there pin
> matrix location to there function:
>
> - SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30, SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK, SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */
>
> That would keep the names and numbers consistent on both H3 varinats.
> The names would correlate to function and the numbers simply serve as a
> pin identifier which is unique and derived from the H3SiP pin layout,
> probably a comment about this in the source is a good idea :-)

So "the system" would be H3SiP pin numbers.
Looks good to me.

Laurent, do you agree?

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-10-05  9:51       ` Geert Uytterhoeven
@ 2016-10-05 10:12         ` Laurent Pinchart
  2016-10-06  9:27             ` Niklas Söderlund
  0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2016-10-05 10:12 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Söderlund, Geert Uytterhoeven, Linux-Renesas,
	linux-gpio, Linus Walleij

Hi Geert,

On Wednesday 05 Oct 2016 11:51:49 Geert Uytterhoeven wrote:
> On Wed, Oct 5, 2016 at 10:33 AM, Niklas Söderlund wrote:
> > On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
> >> On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund wrote:
> >> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> >> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c

[snip]

> >>> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */>
> >>
> >> All these pin numbers match R-Car H3SiP, while there exists also a plain
> >> R-Car H3, which uses completely different pin numbers.
> >> 
> >> How are we gonna distinguish these two variants?
> >> Perhaps we can refer to these pins in some other way, to have consistent
> >> numbering?
> >> 
> >> Or don't we have to? Are these numbers visible in userspace (sysfs)?
> > 
> > Unfortunately both the number and name are show in sysfs under
> > '/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:
> > 
> > <snip>
> > pin 1906 (PIN_AP7) sh-pfc
> > pin 1907 (PIN_AP8) sh-pfc
> > pin 1984 (PIN_AR7) sh-pfc
> > pin 1985 (PIN_AR8) sh-pfc
> > pin 2007 (PIN_AR30) sh-pfc
> > pin 2083 (PIN_AT28) sh-pfc
> > pin 2085 (PIN_AT30) sh-pfc
> > </snip>
> 
> Thanks for checking!
> 
> > So yes a way to present consistent names is needed if this driver should
> > match both H3 variants. But I'm not sure the numbers needs to be
> > correlated to the pin matrix they only need to be unique I think, please
> > correct me if I'm wrong. And if that is the case then maybe a solution
> 
> Yes, I also think they just have to be unique.
> Having some system to make it easier to have unique numbers is nice.
> 
> > to the problem is to simply change the name of the pins from there pin
> > matrix location to there function:
> > 
> > - SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> > SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> > + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK,
> > SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */
> > 
> > That would keep the names and numbers consistent on both H3 varinats.
> > The names would correlate to function and the numbers simply serve as a
> > pin identifier which is unique and derived from the H3SiP pin layout,
> > probably a comment about this in the source is a good idea :-)
> 
> So "the system" would be H3SiP pin numbers.
> Looks good to me.
> 
> Laurent, do you agree?

I'm fine with that.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
  2016-10-05 10:12         ` Laurent Pinchart
@ 2016-10-06  9:27             ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-10-06  9:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Geert Uytterhoeven, Linux-Renesas,
	linux-gpio, Linus Walleij

On 2016-10-05 13:12:43 +0300, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Wednesday 05 Oct 2016 11:51:49 Geert Uytterhoeven wrote:
> > On Wed, Oct 5, 2016 at 10:33 AM, Niklas Söderlund wrote:
> > > On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
> > >> On Tue, Sep 13, 2016 at 4:03 PM, Niklas Söderlund wrote:
> > >> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > >> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> 
> [snip]
> 
> > >>> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */>
> > >>
> > >> All these pin numbers match R-Car H3SiP, while there exists also a plain
> > >> R-Car H3, which uses completely different pin numbers.
> > >> 
> > >> How are we gonna distinguish these two variants?
> > >> Perhaps we can refer to these pins in some other way, to have consistent
> > >> numbering?
> > >> 
> > >> Or don't we have to? Are these numbers visible in userspace (sysfs)?
> > > 
> > > Unfortunately both the number and name are show in sysfs under
> > > '/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:
> > > 
> > > <snip>
> > > pin 1906 (PIN_AP7) sh-pfc
> > > pin 1907 (PIN_AP8) sh-pfc
> > > pin 1984 (PIN_AR7) sh-pfc
> > > pin 1985 (PIN_AR8) sh-pfc
> > > pin 2007 (PIN_AR30) sh-pfc
> > > pin 2083 (PIN_AT28) sh-pfc
> > > pin 2085 (PIN_AT30) sh-pfc
> > > </snip>
> > 
> > Thanks for checking!
> > 
> > > So yes a way to present consistent names is needed if this driver should
> > > match both H3 variants. But I'm not sure the numbers needs to be
> > > correlated to the pin matrix they only need to be unique I think, please
> > > correct me if I'm wrong. And if that is the case then maybe a solution
> > 
> > Yes, I also think they just have to be unique.
> > Having some system to make it easier to have unique numbers is nice.
> > 
> > > to the problem is to simply change the name of the pins from there pin
> > > matrix location to there function:
> > > 
> > > - SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> > > SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> > > + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK,
> > > SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */
> > > 
> > > That would keep the names and numbers consistent on both H3 varinats.
> > > The names would correlate to function and the numbers simply serve as a
> > > pin identifier which is unique and derived from the H3SiP pin layout,
> > > probably a comment about this in the source is a good idea :-)
> > 
> > So "the system" would be H3SiP pin numbers.
> > Looks good to me.
> > 
> > Laurent, do you agree?
> 
> I'm fine with that.

OK, thanks for the feedback guys. I will updated the series and send out 
a new version.

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength
@ 2016-10-06  9:27             ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-10-06  9:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Geert Uytterhoeven, Linux-Renesas,
	linux-gpio, Linus Walleij

On 2016-10-05 13:12:43 +0300, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Wednesday 05 Oct 2016 11:51:49 Geert Uytterhoeven wrote:
> > On Wed, Oct 5, 2016 at 10:33 AM, Niklas S�derlund wrote:
> > > On 2016-10-04 21:13:18 +0200, Geert Uytterhoeven wrote:
> > >> On Tue, Sep 13, 2016 at 4:03 PM, Niklas S�derlund wrote:
> > >> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> > >> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> 
> [snip]
> 
> > >>> +       SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  4,  AE4,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_IO2 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('E'),  5,  AE5,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* QSPI1_MISO_IO1 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  7,  AP7,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN0 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('P'),  8,  AP8,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN1 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  7,  AR7,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN2 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'),  8,  AR8,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* DU_DOTCLKIN3 */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('R'), 30, AR30,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TMS */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 28, AT28,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* TDO */
> > >>> +      SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> > >>> SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */>
> > >>
> > >> All these pin numbers match R-Car H3SiP, while there exists also a plain
> > >> R-Car H3, which uses completely different pin numbers.
> > >> 
> > >> How are we gonna distinguish these two variants?
> > >> Perhaps we can refer to these pins in some other way, to have consistent
> > >> numbering?
> > >> 
> > >> Or don't we have to? Are these numbers visible in userspace (sysfs)?
> > > 
> > > Unfortunately both the number and name are show in sysfs under
> > > '/sys/kernel/debug/pinctrl/e6060000.pfc/*', example from the pins node:
> > > 
> > > <snip>
> > > pin 1906 (PIN_AP7) sh-pfc
> > > pin 1907 (PIN_AP8) sh-pfc
> > > pin 1984 (PIN_AR7) sh-pfc
> > > pin 1985 (PIN_AR8) sh-pfc
> > > pin 2007 (PIN_AR30) sh-pfc
> > > pin 2083 (PIN_AT28) sh-pfc
> > > pin 2085 (PIN_AT30) sh-pfc
> > > </snip>
> > 
> > Thanks for checking!
> > 
> > > So yes a way to present consistent names is needed if this driver should
> > > match both H3 variants. But I'm not sure the numbers needs to be
> > > correlated to the pin matrix they only need to be unique I think, please
> > > correct me if I'm wrong. And if that is the case then maybe a solution
> > 
> > Yes, I also think they just have to be unique.
> > Having some system to make it easier to have unique numbers is nice.
> > 
> > > to the problem is to simply change the name of the pins from there pin
> > > matrix location to there function:
> > > 
> > > - SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, AT30,
> > > SH_PFC_PIN_CFG_DRIVE_STRENGTH),        /* ASEBRK */
> > > + SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK,
> > > SH_PFC_PIN_CFG_DRIVE_STRENGTH),      /* ASEBRK */
> > > 
> > > That would keep the names and numbers consistent on both H3 varinats.
> > > The names would correlate to function and the numbers simply serve as a
> > > pin identifier which is unique and derived from the H3SiP pin layout,
> > > probably a comment about this in the source is a good idea :-)
> > 
> > So "the system" would be H3SiP pin numbers.
> > Looks good to me.
> > 
> > Laurent, do you agree?
> 
> I'm fine with that.

OK, thanks for the feedback guys. I will updated the series and send out 
a new version.

-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-10-06  9:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-13 14:03 [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
2016-09-13 14:03 ` [PATCH 1/4] pinctrl: sh-pfc: Support named pins with custom configuration Niklas Söderlund
2016-09-13 14:28   ` Laurent Pinchart
2016-10-04 19:08   ` Geert Uytterhoeven
2016-09-13 14:03 ` [PATCH 2/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Niklas Söderlund
2016-10-04 19:13   ` Geert Uytterhoeven
2016-10-05  8:33     ` Niklas Söderlund
2016-10-05  8:33       ` Niklas Söderlund
2016-10-05  9:51       ` Geert Uytterhoeven
2016-10-05 10:12         ` Laurent Pinchart
2016-10-06  9:27           ` Niklas Söderlund
2016-10-06  9:27             ` Niklas Söderlund
2016-09-13 14:03 ` [PATCH 3/4] pinctrl: sh-pfc: r8a7795: Add group for AVB MDIO and MII pins Niklas Söderlund
2016-09-14  9:05   ` Sergei Shtylyov
2016-10-05  7:41   ` Geert Uytterhoeven
2016-09-13 14:03 ` [PATCH 4/4] pinctrl: sh-pfc: r8a7795: Add group for QSPI0 and QSPI1 pins Niklas Söderlund
2016-10-05  7:33   ` Geert Uytterhoeven
2016-10-04 19:09 ` [PATCH 0/4] pinctrl: sh-pfc: r8a7795: Support none GPIO pins with configurable drive-strength Geert Uytterhoeven

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.