[PATCH 00/15] pinctrl: sh-pfc: Fix config register descriptions

[PATCH 00/15] pinctrl: sh-pfc: Fix config register descriptions

[Geert Uytterhoeven]

	Hi all,

This patch series contains enhancements for config register descriptions
and their handling in the Renesas pinctrl drivers.
  - Patches 1-10 are fixes for config register descriptions, of which all
    but 6 and 10 were found by the pinmux table validation in the RFC
    patches at the end of the series,
  - Patches 11-12 are small improvements for debugging and memory
    consumption,
  - Patches 13-15 are an RFC, to add the option of doing some validation
    of all builtin pinmux tables at runtime.  While these aren't ready
    for prime-time yet, they were very valuable in finding most of the
    issues fixed above.  Hence I think it would be good to integrate an
    evolved form of them later, to help catching bugs early.
    Note that patches 13 and 14 are incomplete (the real ones are boring
    and very large), but just serve as an example.

Probably I will queue the most important fixes in sh-pfc-for-v4.21.
The rest will have to wait for v4.22.

Thanks for your comments!

Geert Uytterhoeven (15):
  pinctrl: sh-pfc: r8a77994: Remove bogus IPSR9 field
  pinctrl: sh-pfc: r8a779970: Add missing MOD_SEL0 field
  pinctrl: sh-pfc: r8a779980: Add missing MOD_SEL0 field
  pinctrl: sh-pfc: sh7734: Add missing IPSR11 field
  pinctrl: sh-pfc: r8a77990: Fix MOD_SEL0 SEL_I2C1 field width
  pinctrl: sh-pfc: r8a77995: Remove bogus SEL_PWM[0-3]_3 configurations
  pinctrl: sh-pfc: sh7264: Fix PFCR3 and PFCR0 register configuration
  pinctrl: sh-pfc: sh7269: Add missing PCIOR0 field
  pinctrl: sh-pfc: sh7734: Remove bogus IPSR10 value
  pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10
  pinctrl: sh-pfc: Print actual field width for variable-width fields
  pinctrl: sh-pfc: Make pinmux_cfg_reg.var_field_width[] variable-length
  [RFC/INCOMPLETE] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro
  [RFC/INCOMPLETE] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro
  [RFC] pinctrl: sh-pfc: Validate pinmux tables at runtime when
    debugging

 drivers/pinctrl/sh-pfc/Kconfig           |  36 ----
 drivers/pinctrl/sh-pfc/Makefile          |  13 ++
 drivers/pinctrl/sh-pfc/core.c            | 104 +++++++++++-
 drivers/pinctrl/sh-pfc/pfc-emev2.c       |  67 +++++---
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c     |  20 +--
 drivers/pinctrl/sh-pfc/pfc-r8a7740.c     |  16 +-
 drivers/pinctrl/sh-pfc/pfc-r8a77470.c    | 135 ++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7778.c     | 101 ++++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a7779.c     | 117 +++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c     | 131 ++++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c     | 155 ++++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7792.c     | 134 ++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c     | 126 ++++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 124 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c     | 129 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c     | 129 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c    | 129 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c    |  67 ++++----
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c    |  75 +++++----
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c    | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c    | 105 ++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7203.c      | 124 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7264.c      | 191 +++++++++++----------
 drivers/pinctrl/sh-pfc/pfc-sh7269.c      | 206 +++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh73a0.c      |  12 +-
 drivers/pinctrl/sh-pfc/pfc-sh7720.c      |  72 ++++----
 drivers/pinctrl/sh-pfc/pfc-sh7722.c      | 128 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7723.c      | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7724.c      | 112 ++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7734.c      | 144 +++++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7757.c      | 140 +++++++--------
 drivers/pinctrl/sh-pfc/pfc-sh7785.c      |  72 ++++----
 drivers/pinctrl/sh-pfc/pfc-sh7786.c      |  44 ++---
 drivers/pinctrl/sh-pfc/pfc-shx3.c        |  16 +-
 drivers/pinctrl/sh-pfc/sh_pfc.h          |  68 ++++++--
 35 files changed, 1873 insertions(+), 1585 deletions(-)

-- 
2.17.1

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

[PATCH 01/15] pinctrl: sh-pfc: r8a77994: Remove bogus IPSR9 field

[Geert Uytterhoeven]

The Peripheral Function Select Register 9 contains 12 fields, but the
variable field descriptor contains a 13th bogus field of 3 bits.

Fixes: 43c4436e2f1890a7 ("pinctrl: sh-pfc: add R8A7794 PFC support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index b96a3cc79084ddcc..fcf1339c40584385 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -5212,7 +5212,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_AVB_MDC, FN_SSI_SDATA6_B, 0, 0, }
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR9", 0xE6060044, 32,
-			     1, 3, 3, 3, 3, 2, 2, 3, 3, 3, 3, 3, 3) {
+			     1, 3, 3, 3, 3, 2, 2, 3, 3, 3, 3, 3) {
 		/* IP9_31 [1] */
 		0, 0,
 		/* IP9_30_28 [3] */
-- 
2.17.1

[PATCH 02/15] pinctrl: sh-pfc: r8a779970: Add missing MOD_SEL0 field

[Geert Uytterhoeven]

The Module Select Register 0 contains 20 (= 5 x 4) reserved bits, and 12
single-bit fields, but the variable field descriptor lacks a field of 4
reserved bits.

Fixes: b92ac66a1819602b ("pinctrl: sh-pfc: Add R8A77970 PFC support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77970.c b/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
index 2ab80d1e65bc96fc..ee3a2ba44134a374 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
@@ -2422,7 +2422,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 #define F_(x, y)	x,
 #define FM(x)		FN_##x,
 	{ PINMUX_CFG_REG_VAR("MOD_SEL0", 0xe6060500, 32,
-			     4, 4, 4, 4,
+			     4, 4, 4, 4, 4,
 			     1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) {
 		/* RESERVED 31, 30, 29, 28 */
 		0, 0, 0, 0, 0, 0, 0, 0,	0, 0, 0, 0, 0, 0, 0, 0,
-- 
2.17.1

[PATCH 03/15] pinctrl: sh-pfc: r8a779980: Add missing MOD_SEL0 field

[Geert Uytterhoeven]

The Module Select Register 0 contains 20 (= 5 x 4) reserved bits, and 12
single-bit fields, but the variable field descriptor lacks a field of 4
reserved bits.

Fixes: f59125248a691dfe ("pinctrl: sh-pfc: Add R8A77980 PFC support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77980.c b/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
index 69cc571e00aec6c1..91523a5ed2a89c7d 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
@@ -2822,7 +2822,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 #define F_(x, y)	x,
 #define FM(x)		FN_##x,
 	{ PINMUX_CFG_REG_VAR("MOD_SEL0", 0xe6060500, 32,
-			     4, 4, 4, 4,
+			     4, 4, 4, 4, 4,
 			     1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1) {
 		/* RESERVED 31, 30, 29, 28 */
 		0, 0, 0, 0, 0, 0, 0, 0,	0, 0, 0, 0, 0, 0, 0, 0,
-- 
2.17.1

[PATCH 04/15] pinctrl: sh-pfc: sh7734: Add missing IPSR11 field

[Geert Uytterhoeven]

The Peripheral Function Select Register 11 contains 3 reserved bits and
15 variable-width fields, but the variable field descriptor does not
contain the 3-bit field IP11[25:23].

Fixes: 856cb4bb337ee504 ("sh: Add support pinmux for SH7734")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-sh7734.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
index b0533c86053a6467..04062f642d3e62de 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
@@ -2234,7 +2234,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_LCD_DATA15_B, 0, 0, 0 }
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR11", 0xFFFC0048, 32,
-			3, 1, 2, 2, 2, 3, 3, 1, 2, 3, 3, 1, 1, 1, 1) {
+			3, 1, 2, 3, 2, 2, 3, 3, 1, 2, 3, 3, 1, 1, 1, 1) {
 	    /* IP11_31_29 [3] */
 	    0, 0, 0, 0, 0, 0, 0, 0,
 	    /* IP11_28 [1] */
-- 
2.17.1

[PATCH 05/15] pinctrl: sh-pfc: r8a77990: Fix MOD_SEL0 SEL_I2C1 field width

[Geert Uytterhoeven]

The SEL_I2C1 (MOD_SEL0[21:20]) field in Module Select Register 0 has a
width of 2 bits, i.e. it allows programming one out of 4 different
configurations.
However, the MOD_SEL0_21_20 macro contains 8 values instead of 4,
overflowing into the subsequent fields in the register, and thus breaking
the configuration of the latter.

Fix this by dropping the bogus last 4 values, including the non-existent
SEL_I2C1_4 configuration.

Fixes: 6d4036a1e3b3ac0f ("pinctrl: sh-pfc: Initial R8A77990 PFC support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
index d5b7a83e8b614370..e40908dc37e06264 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
@@ -399,7 +399,7 @@ FM(IP12_31_28)	IP12_31_28	FM(IP13_31_28)	IP13_31_28	FM(IP14_31_28)	IP14_31_28	FM
 #define MOD_SEL0_24		FM(SEL_HSCIF0_0)		FM(SEL_HSCIF0_1)
 #define MOD_SEL0_23		FM(SEL_HSCIF1_0)		FM(SEL_HSCIF1_1)
 #define MOD_SEL0_22		FM(SEL_HSCIF2_0)		FM(SEL_HSCIF2_1)
-#define MOD_SEL0_21_20		FM(SEL_I2C1_0)			FM(SEL_I2C1_1)			FM(SEL_I2C1_2)			FM(SEL_I2C1_3)		FM(SEL_I2C1_4)		F_(0, 0)	F_(0, 0)	F_(0, 0)
+#define MOD_SEL0_21_20		FM(SEL_I2C1_0)			FM(SEL_I2C1_1)			FM(SEL_I2C1_2)			FM(SEL_I2C1_3)
 #define MOD_SEL0_19_18_17	FM(SEL_I2C2_0)			FM(SEL_I2C2_1)			FM(SEL_I2C2_2)			FM(SEL_I2C2_3)		FM(SEL_I2C2_4)		F_(0, 0)	F_(0, 0)	F_(0, 0)
 #define MOD_SEL0_16		FM(SEL_NDFC_0)			FM(SEL_NDFC_1)
 #define MOD_SEL0_15		FM(SEL_PWM0_0)			FM(SEL_PWM0_1)
-- 
2.17.1

[PATCH 06/15] pinctrl: sh-pfc: r8a77995: Remove bogus SEL_PWM[0-3]_3 configurations

[Geert Uytterhoeven]

While the SEL_PWM[0-3] fields in the Module Select Register 0 support 4
possible configurations per PWM pin, only the first 3 are valid.

Replace the invalid and unused configurations for SEL_PWM[0-3]_3 by
dummies.

Fixes: 794a6711764658a1 ("pinctrl: sh-pfc: Initial R8A77995 PFC support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77995.c b/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
index e457539a61c55bb9..84d78db381e30249 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
@@ -388,10 +388,10 @@ FM(IP12_31_28)	IP12_31_28 \
 #define MOD_SEL0_27		FM(SEL_MSIOF3_0)	FM(SEL_MSIOF3_1)
 #define MOD_SEL0_26		FM(SEL_HSCIF3_0)	FM(SEL_HSCIF3_1)
 #define MOD_SEL0_25		FM(SEL_SCIF4_0)		FM(SEL_SCIF4_1)
-#define MOD_SEL0_24_23		FM(SEL_PWM0_0)		FM(SEL_PWM0_1)		FM(SEL_PWM0_2)		FM(SEL_PWM0_3)
-#define MOD_SEL0_22_21		FM(SEL_PWM1_0)		FM(SEL_PWM1_1)		FM(SEL_PWM1_2)		FM(SEL_PWM1_3)
-#define MOD_SEL0_20_19		FM(SEL_PWM2_0)		FM(SEL_PWM2_1)		FM(SEL_PWM2_2)		FM(SEL_PWM2_3)
-#define MOD_SEL0_18_17		FM(SEL_PWM3_0)		FM(SEL_PWM3_1)		FM(SEL_PWM3_2)		FM(SEL_PWM3_3)
+#define MOD_SEL0_24_23		FM(SEL_PWM0_0)		FM(SEL_PWM0_1)		FM(SEL_PWM0_2)		F_(0, 0)
+#define MOD_SEL0_22_21		FM(SEL_PWM1_0)		FM(SEL_PWM1_1)		FM(SEL_PWM1_2)		F_(0, 0)
+#define MOD_SEL0_20_19		FM(SEL_PWM2_0)		FM(SEL_PWM2_1)		FM(SEL_PWM2_2)		F_(0, 0)
+#define MOD_SEL0_18_17		FM(SEL_PWM3_0)		FM(SEL_PWM3_1)		FM(SEL_PWM3_2)		F_(0, 0)
 #define MOD_SEL0_15		FM(SEL_IRQ_0_0)		FM(SEL_IRQ_0_1)
 #define MOD_SEL0_14		FM(SEL_IRQ_1_0)		FM(SEL_IRQ_1_1)
 #define MOD_SEL0_13		FM(SEL_IRQ_2_0)		FM(SEL_IRQ_2_1)
-- 
2.17.1

[PATCH 07/15] pinctrl: sh-pfc: sh7264: Fix PFCR3 and PFCR0 register configuration

[Geert Uytterhoeven]

The Port F Control Register 3 (PFCR3) contains only a single field.
However, counting from left to right, it is the fourth field, not the
first field.
Insert the missing dummy configuration values (3 fields of 16 values) to
fix this.

The descriptor for the Port F Control Register 0 (PFCR0) lacks the
description for the 4th field (PF0 Mode, PF0MD[2:0]).
Add the missing configuration values to fix this.

Fixes: a8d42fc4217b1ea1 ("sh-pfc: Add sh7264 pinmux support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Based on the SH7264 Hardware User Manual Rev. 4.00.

The odd location of the closing curly brace in the PFCR0 section
suggests an editing accident.
---
 drivers/pinctrl/sh-pfc/pfc-sh7264.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7264.c b/drivers/pinctrl/sh-pfc/pfc-sh7264.c
index 4f44ce0d7237faa9..501de63e6c5f4be2 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7264.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7264.c
@@ -1713,6 +1713,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	},
 
 	{ PINMUX_CFG_REG("PFCR3", 0xfffe38a8, 16, 4) {
+		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 		PF12MD_000, PF12MD_001, 0, PF12MD_011,
 		PF12MD_100, PF12MD_101, 0, 0,
 		0, 0, 0, 0, 0, 0, 0, 0 }
@@ -1756,8 +1759,10 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		0, 0, 0, 0, 0, 0, 0, 0,
 		PF1MD_000, PF1MD_001, PF1MD_010, PF1MD_011,
 		PF1MD_100, PF1MD_101, 0, 0,
-		0, 0, 0, 0, 0, 0, 0, 0
-	 }
+		0, 0, 0, 0, 0, 0, 0, 0,
+		PF0MD_000, PF0MD_001, PF0MD_010, PF0MD_011,
+		PF0MD_100, PF0MD_101, 0, 0,
+		0, 0, 0, 0, 0, 0, 0, 0 }
 	},
 
 	{ PINMUX_CFG_REG("PFIOR0", 0xfffe38b2, 16, 1) {
-- 
2.17.1

[PATCH 08/15] pinctrl: sh-pfc: sh7269: Add missing PCIOR0 field

[Geert Uytterhoeven]

The Port C I/O Register 0 contains 7 reserved bits, but the descriptor
contains only dummy configuration values for 6 reserved bits, thus
breaking the configuration of all subsequent fields in the register.

Fix this by adding the two missing configuration values.

Fixes: f5e811f2a43117b2 ("sh-pfc: Add sh7269 pinmux support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-sh7269.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7269.c b/drivers/pinctrl/sh-pfc/pfc-sh7269.c
index 5b48a0368e55b33a..a95997a389a4e873 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7269.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7269.c
@@ -2116,7 +2116,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	},
 
 	{ PINMUX_CFG_REG("PCIOR0", 0xfffe3852, 16, 1) {
-		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 		PC8_IN, PC8_OUT,
 		PC7_IN, PC7_OUT,
 		PC6_IN, PC6_OUT,
-- 
2.17.1

[PATCH 09/15] pinctrl: sh-pfc: sh7734: Remove bogus IPSR10 value

[Geert Uytterhoeven]

The IP10[5:3] field in Peripheral Function Select Register 10 has a
width of 3 bits, i.e. it allows programming one out of 8 different
configurations.
However, 9 values are provided instead of 8, overflowing into the
subsequent field in the register, and thus breaking the configuration of
the latter.

Fix this by dropping a bogus zero value.

Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/pfc-sh7734.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
index 04062f642d3e62de..cad70f9cf5699f0c 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
@@ -2228,7 +2228,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_LCD_CL1_B, 0, 0, 0,
 	    /* IP10_5_3 [3] */
 		FN_SSI_WS23, FN_VI1_5_B, FN_TX1_D, FN_HSCK0_C, FN_FALE_B,
-		FN_LCD_DON_B, 0, 0, 0,
+		FN_LCD_DON_B, 0, 0,
 	    /* IP10_2_0 [3] */
 		FN_SSI_SCK23, FN_VI1_4_B, FN_RX1_D, FN_FCLE_B,
 		FN_LCD_DATA15_B, 0, 0, 0 }
-- 
2.17.1

[PATCH 10/15] pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10

[Geert Uytterhoeven]

Some values in the Peripheral Function Select Register 10 descriptor are
shifted by one position, which may cause a peripheral function to be
programmed incorrectly.

Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
already the case for the HSCK0 pin in field IP10[5:3].

Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Based on the SH7734 Hardware User's Manual Rev. 1.00.
Compile-tested only.

Noticed when investigating a different bug in the same register
description.
---
 drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
index cad70f9cf5699f0c..748a32a3af82d368 100644
--- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
+++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
@@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 	    /* IP10_22 [1] */
 		FN_CAN_CLK_A, FN_RX4_D,
 	    /* IP10_21_19 [3] */
-		FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
-		FN_LCD_M_DISP_B, 0, 0, 0,
+		FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
+		FN_LCD_M_DISP_B, 0, 0,
 	    /* IP10_18_16 [3] */
-		FN_AUDIO_CLKC, FN_SCK1_E, FN_HCTS0_C, FN_FRB_B,
-		FN_LCD_VEPWC_B, 0, 0, 0,
+		FN_AUDIO_CLKC, FN_SCK1_E, 0, FN_HCTS0_C, FN_FRB_B,
+		FN_LCD_VEPWC_B, 0, 0,
 	    /* IP10_15 [1] */
 		FN_AUDIO_CLKB_A, FN_LCD_CLK_B,
 	    /* IP10_14_12 [3] */
 		FN_AUDIO_CLKA_A, FN_VI1_CLK_B, FN_SCK1_D, FN_IECLK_B,
 		FN_LCD_FLM_B, 0, 0, 0,
 	    /* IP10_11_9 [3] */
-		FN_SSI_SDATA3, FN_VI1_7_B, FN_HTX0_C, FN_FWE_B,
-		FN_LCD_CL2_B, 0, 0, 0,
+		FN_SSI_SDATA3, FN_VI1_7_B, 0, FN_HTX0_C, FN_FWE_B,
+		FN_LCD_CL2_B, 0, 0,
 	    /* IP10_8_6 [3] */
-		FN_SSI_SDATA2, FN_VI1_6_B, FN_HRX0_C, FN_FRE_B,
-		FN_LCD_CL1_B, 0, 0, 0,
+		FN_SSI_SDATA2, FN_VI1_6_B, 0, FN_HRX0_C, FN_FRE_B,
+		FN_LCD_CL1_B, 0, 0,
 	    /* IP10_5_3 [3] */
 		FN_SSI_WS23, FN_VI1_5_B, FN_TX1_D, FN_HSCK0_C, FN_FALE_B,
 		FN_LCD_DON_B, 0, 0,
-- 
2.17.1

[PATCH 11/15] pinctrl: sh-pfc: Print actual field width for variable-width fields

[Geert Uytterhoeven]

The debug code in sh_pfc_write_config_reg() prints the width of the
field being modified.

However, registers with a variable-width field layout are identified by
pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
of the actual field widths.

Fix this by printing the Hamming height of the field mask instead, which
is correct for both fixed-width and variable-width fields.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index a10f7050a74f35ff..f1cfcc8c65446662 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -221,7 +221,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
 
 	dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
 		"r_width = %u, f_width = %u\n",
-		crp->reg, value, field, crp->reg_width, crp->field_width);
+		crp->reg, value, field, crp->reg_width, hweight32(mask));
 
 	mask = ~(mask << pos);
 	value = value << pos;
-- 
2.17.1

[PATCH 12/15] pinctrl: sh-pfc: Make pinmux_cfg_reg.var_field_width[] variable-length

[Geert Uytterhoeven]

pinmux_cfg_reg.var_field_width[] is actually a variable-length array,
terminated by a zero, and counting at most r_width entries.
Usually the number of entries is much smaller than r_width, so the
ability to catch bugs at compile time through an "excess elements in
array initializer" warning is fairly limited.

Hence make the array variable-length, decreasing kernel size slightly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/sh_pfc.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index a2c5d530aaa1c0b3..273b55de9fd67b03 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -142,8 +142,7 @@ struct pinmux_cfg_reg {
  */
 #define PINMUX_CFG_REG_VAR(name, r, r_width, var_fw0, var_fwn...) \
 	.reg = r, .reg_width = r_width,	\
-	.var_field_width = (const u8 [r_width]) \
-		{ var_fw0, var_fwn, 0 }, \
+	.var_field_width = (const u8 []) { var_fw0, var_fwn, 0 }, \
 	.enum_ids = (const u16 [])
 
 struct pinmux_drive_reg_field {
-- 
2.17.1

[PATCH 13/15] [RFC] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro

[Geert Uytterhoeven]

Currently the PINMUX_CFG_REG() macro must be followed by initialization
data, specifying all enum IDs.  Hence the macro itself does not know
anything about the enum IDs, preventing the macro from performing any
validation on it.

Make the macro accept the enum IDs as a parameter, and update all users.
Note that array data enclosed by curly braces cannot be passed to a
macro as a parameter, hence the enum IDs are wrapped using a new macro
GROUPS().

No functional changes.

Not-yet-signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This patch is incomplete!
It contains only the generic and r8a7791 parts.

 drivers/pinctrl/sh-pfc/pfc-emev2.c       |  20 +--
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c     |  20 +--
 drivers/pinctrl/sh-pfc/pfc-r8a7740.c     |  16 +-
 drivers/pinctrl/sh-pfc/pfc-r8a77470.c    |  24 +--
 drivers/pinctrl/sh-pfc/pfc-r8a7778.c     |  20 +--
 drivers/pinctrl/sh-pfc/pfc-r8a7779.c     |  28 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c     |  24 +--
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c     |  32 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a7792.c     |  48 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c     |  28 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 104 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c     | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c     | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c    | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c    |  60 +++----
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c    |  68 ++++----
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c    |  92 +++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c    |  84 +++++-----
 drivers/pinctrl/sh-pfc/pfc-sh7203.c      | 124 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7264.c      | 184 ++++++++++----------
 drivers/pinctrl/sh-pfc/pfc-sh7269.c      | 204 +++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh73a0.c      |  12 +-
 drivers/pinctrl/sh-pfc/pfc-sh7720.c      |  72 ++++----
 drivers/pinctrl/sh-pfc/pfc-sh7722.c      | 128 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7723.c      | 108 ++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7724.c      | 112 ++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7734.c      |  45 ++---
 drivers/pinctrl/sh-pfc/pfc-sh7757.c      | 140 ++++++++--------
 drivers/pinctrl/sh-pfc/pfc-sh7785.c      |  72 ++++----
 drivers/pinctrl/sh-pfc/pfc-sh7786.c      |  44 ++---
 drivers/pinctrl/sh-pfc/pfc-shx3.c        |  16 +-
 drivers/pinctrl/sh-pfc/sh_pfc.h          |  14 +-
 32 files changed, 1138 insertions(+), 1129 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
index 2859231aaffc4546..62e615080099b034 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
@@ -5427,7 +5427,7 @@ static const struct {
 };
 
 static const struct pinmux_cfg_reg pinmux_config_regs[] = {
-	{ PINMUX_CFG_REG("GPSR0", 0xE6060004, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR0", 0xE6060004, 32, 1, GROUP(
 		GP_0_31_FN, FN_IP1_22_20,
 		GP_0_30_FN, FN_IP1_19_17,
 		GP_0_29_FN, FN_IP1_16_14,
@@ -5459,9 +5459,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_0_3_FN, FN_IP0_3,
 		GP_0_2_FN, FN_IP0_2,
 		GP_0_1_FN, FN_IP0_1,
-		GP_0_0_FN, FN_IP0_0, }
+		GP_0_0_FN, FN_IP0_0, ))
 	},
-	{ PINMUX_CFG_REG("GPSR1", 0xE6060008, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR1", 0xE6060008, 32, 1, GROUP(
 		0, 0,
 		0, 0,
 		0, 0,
@@ -5493,9 +5493,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_1_3_FN, FN_IP2_2_0,
 		GP_1_2_FN, FN_IP1_31_29,
 		GP_1_1_FN, FN_IP1_28_26,
-		GP_1_0_FN, FN_IP1_25_23, }
+		GP_1_0_FN, FN_IP1_25_23, ))
 	},
-	{ PINMUX_CFG_REG("GPSR2", 0xE606000C, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR2", 0xE606000C, 32, 1, GROUP(
 		GP_2_31_FN, FN_IP6_7_6,
 		GP_2_30_FN, FN_IP6_5_3,
 		GP_2_29_FN, FN_IP6_2_0,
@@ -5527,9 +5527,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_2_3_FN, FN_IP4_4_2,
 		GP_2_2_FN, FN_IP4_1_0,
 		GP_2_1_FN, FN_IP3_30_28,
-		GP_2_0_FN, FN_IP3_27_25 }
+		GP_2_0_FN, FN_IP3_27_25 ))
 	},
-	{ PINMUX_CFG_REG("GPSR3", 0xE6060010, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR3", 0xE6060010, 32, 1, GROUP(
 		GP_3_31_FN, FN_IP9_18_17,
 		GP_3_30_FN, FN_IP9_16,
 		GP_3_29_FN, FN_IP9_15_13,
@@ -5561,9 +5561,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_3_3_FN, FN_IP7_12_11,
 		GP_3_2_FN, FN_IP7_10_9,
 		GP_3_1_FN, FN_IP7_8_6,
-		GP_3_0_FN, FN_IP7_5_3 }
+		GP_3_0_FN, FN_IP7_5_3 ))
 	},
-	{ PINMUX_CFG_REG("GPSR4", 0xE6060014, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR4", 0xE6060014, 32, 1, GROUP(
 		GP_4_31_FN, FN_IP15_5_4,
 		GP_4_30_FN, FN_IP15_3_2,
 		GP_4_29_FN, FN_IP15_1_0,
@@ -5595,9 +5595,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_4_3_FN, FN_IP9_24_23,
 		GP_4_2_FN, FN_IP9_22_21,
 		GP_4_1_FN, FN_IP9_20_19,
-		GP_4_0_FN, FN_VI0_CLK }
+		GP_4_0_FN, FN_VI0_CLK ))
 	},
-	{ PINMUX_CFG_REG("GPSR5", 0xE6060018, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR5", 0xE6060018, 32, 1, GROUP(
 		GP_5_31_FN, FN_IP3_24_22,
 		GP_5_30_FN, FN_IP13_9_7,
 		GP_5_29_FN, FN_IP13_6_5,
@@ -5629,9 +5629,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_5_3_FN, FN_IP11_18_17,
 		GP_5_2_FN, FN_IP11_16_15,
 		GP_5_1_FN, FN_IP11_14_12,
-		GP_5_0_FN, FN_IP11_11_9 }
+		GP_5_0_FN, FN_IP11_11_9 ))
 	},
-	{ PINMUX_CFG_REG("GPSR6", 0xE606001C, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR6", 0xE606001C, 32, 1, GROUP(
 		GP_6_31_FN, FN_DU0_DOTCLKIN,
 		GP_6_30_FN, FN_USB1_OVC,
 		GP_6_29_FN, FN_IP14_31_29,
@@ -5663,9 +5663,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_6_3_FN, FN_IP13_13,
 		GP_6_2_FN, FN_IP13_12,
 		GP_6_1_FN, FN_IP13_11,
-		GP_6_0_FN, FN_IP13_10 }
+		GP_6_0_FN, FN_IP13_10 ))
 	},
-	{ PINMUX_CFG_REG("GPSR7", 0xE6060074, 32, 1) {
+	{ PINMUX_CFG_REG("GPSR7", 0xE6060074, 32, 1, GROUP(
 		0, 0,
 		0, 0,
 		0, 0,
@@ -5697,7 +5697,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		GP_7_3_FN, FN_IP15_26_24,
 		GP_7_2_FN, FN_IP15_23_21,
 		GP_7_1_FN, FN_IP15_20_18,
-		GP_7_0_FN, FN_IP15_17_15 }
+		GP_7_0_FN, FN_IP15_17_15 ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR0", 0xE6060020, 32,
 			     1, 2, 2, 2, 2, 2, 2, 3, 1, 1, 1, 1, 1, 1, 1, 1,
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 273b55de9fd67b03..84468bba9c30e675 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -115,19 +115,23 @@ struct pinmux_cfg_reg {
 	const u8 *var_field_width;
 };
 
+#define GROUP(...)	__VA_ARGS__
+
 /*
  * Describe a config register consisting of several fields of the same width
  *   - name: Register name (unused, for documentation purposes only)
  *   - r: Physical register address
  *   - r_width: Width of the register (in bits)
  *   - f_width: Width of the fixed-width register fields (in bits)
- * This macro must be followed by initialization data: For each register field
- * (from left to right, i.e. MSB to LSB), 2^f_width enum IDs must be specified,
- * one for each possible combination of the register field bit values.
+ *   - ids: For each register field (from left to right, i.e. MSB to LSB),
+ *          2^f_width enum IDs must be specified, one for each possible
+ *          combination of the register field bit values, wrapped using the
+ *          GROUP() macro.
  */
-#define PINMUX_CFG_REG(name, r, r_width, f_width) \
+#define PINMUX_CFG_REG(name, r, r_width, f_width, ids)			\
 	.reg = r, .reg_width = r_width, .field_width = f_width,		\
-	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])
+	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])	\
+		{ ids }
 
 /*
  * Describe a config register consisting of several fields of different widths
-- 
2.17.1

[PATCH 14/15] [RFC] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro

[Geert Uytterhoeven]

Currently the PINMUX_CFG_REG_VAR() macro must be followed by
initialization data, specifying all enum IDs.  Hence the macro itself
does not know anything about the enum IDs, preventing the macro from
performing any validation on it.

Make the macro accept the enum IDs as a parameter, and update all users.
Note that array data enclosed by curly braces cannot be passed to a
macro as a parameter, hence both the register field widths and the enum
IDs are wrapped using the GROUP() macro.

No functional changes.

Not-yet-signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This patch is incomplete!
It contains only the generic and r8a7795 parts.

 drivers/pinctrl/sh-pfc/pfc-emev2.c       |  47 +++++----
 drivers/pinctrl/sh-pfc/pfc-r8a77470.c    | 111 +++++++++++---------
 drivers/pinctrl/sh-pfc/pfc-r8a7778.c     |  81 +++++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7779.c     |  89 +++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c     | 107 ++++++++++++--------
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c     | 123 ++++++++++++++---------
 drivers/pinctrl/sh-pfc/pfc-r8a7792.c     |  86 +++++++++-------
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c     |  98 ++++++++++--------
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c |  20 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c     |  21 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c     |  21 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c    |  21 ++--
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c    |   7 +-
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c    |   7 +-
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c    |  14 +--
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c    |  13 +--
 drivers/pinctrl/sh-pfc/pfc-sh7734.c      |  81 +++++++++------
 drivers/pinctrl/sh-pfc/sh_pfc.h          |  25 +++--
 18 files changed, 571 insertions(+), 401 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 2c1d5d56628a1156..abf6e0167ef6053a 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -5669,8 +5669,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 #define F_(x, y)	x,
 #define FM(x)		FN_##x,
 	{ PINMUX_CFG_REG_VAR("MOD_SEL0", 0xe6060500, 32,
-			     3, 2, 3, 1, 1, 1, 1, 1, 2, 1,
-			     1, 2, 1, 1, 1, 2, 2, 1, 2, 3) {
+			     GROUP(3, 2, 3, 1, 1, 1, 1, 1, 2, 1, 1, 2,
+				   1, 1, 1, 2, 2, 1, 2, 3),
+			     GROUP(
 		MOD_SEL0_31_30_29
 		MOD_SEL0_28_27
 		MOD_SEL0_26_25_24
@@ -5691,11 +5692,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		MOD_SEL0_5
 		MOD_SEL0_4_3
 		/* RESERVED 2, 1, 0 */
-		0, 0, 0, 0, 0, 0, 0, 0 }
+		0, 0, 0, 0, 0, 0, 0, 0 ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL1", 0xe6060504, 32,
-			     2, 3, 1, 2, 3, 1, 1, 2, 1,
-			     2, 1, 1, 1, 1, 1, 2, 1, 1, 1, 1, 1, 1, 1) {
+			     GROUP(2, 3, 1, 2, 3, 1, 1, 2, 1, 2, 1, 1,
+				   1, 1, 1, 2, 1, 1, 1, 1, 1, 1, 1),
+			     GROUP(
 		MOD_SEL1_31_30
 		MOD_SEL1_29_28_27
 		MOD_SEL1_26
@@ -5718,11 +5720,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		MOD_SEL1_3
 		MOD_SEL1_2
 		MOD_SEL1_1
-		MOD_SEL1_0 }
+		MOD_SEL1_0 ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL2", 0xe6060508, 32,
-			     1, 1, 1, 2, 1, 3, 1, 1, 1, 1, 1, 1, 1,
-			     4, 4, 4, 3, 1) {
+			     GROUP(1, 1, 1, 2, 1, 3, 1, 1, 1, 1, 1, 1,
+				   1, 4, 4, 4, 3, 1),
+			     GROUP(
 		MOD_SEL2_31
 		MOD_SEL2_30
 		MOD_SEL2_29
@@ -5749,7 +5752,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		0, 0, 0, 0, 0, 0, 0, 0,
 		/* RESERVED 3, 2, 1 */
 		0, 0, 0, 0, 0, 0, 0, 0,
-		MOD_SEL2_0 }
+		MOD_SEL2_0 ))
 	},
 	{ },
 };
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 84468bba9c30e675..584d58d954961018 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -138,16 +138,17 @@ struct pinmux_cfg_reg {
  *   - name: Register name (unused, for documentation purposes only)
  *   - r: Physical register address
  *   - r_width: Width of the register (in bits)
- *   - var_fw0, var_fwn...: List of widths of the register fields (in bits),
- *                          From left to right (i.e. MSB to LSB)
- * This macro must be followed by initialization data: For each register field
- * (from left to right, i.e. MSB to LSB), 2^var_fwi enum IDs must be specified,
- * one for each possible combination of the register field bit values.
+ *   - f_widths: List of widths of the register fields (in bits), from left
+ *               to right (i.e. MSB to LSB), wrapped using the GROUP() macro.
+ *   - ids: For each register field (from left to right, i.e. MSB to LSB),
+ *          2^f_widths[i] enum IDs must be specified, one for each possible
+ *          combination of the register field bit values, wrapped using the
+ *          GROUP() macro.
  */
-#define PINMUX_CFG_REG_VAR(name, r, r_width, var_fw0, var_fwn...) \
-	.reg = r, .reg_width = r_width,	\
-	.var_field_width = (const u8 []) { var_fw0, var_fwn, 0 }, \
-	.enum_ids = (const u16 [])
+#define PINMUX_CFG_REG_VAR(name, r, r_width, f_widths, ids)		\
+	.reg = r, .reg_width = r_width,					\
+	.var_field_width = (const u8 []) { f_widths, 0 },		\
+	.enum_ids = (const u16 []) { ids }
 
 struct pinmux_drive_reg_field {
 	u16 pin;
@@ -666,7 +667,9 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
  */
 #define PORTCR(nr, reg)							\
 	{								\
-		PINMUX_CFG_REG_VAR("PORT" nr "CR", reg, 8, 2, 2, 1, 3) {\
+		PINMUX_CFG_REG_VAR("PORT" nr "CR", reg, 8,		\
+				   GROUP(2, 2, 1, 3),			\
+				   GROUP(				\
 			/* PULMD[1:0], handled by .set_bias() */	\
 			0, 0, 0, 0,					\
 			/* IE and OE */					\
@@ -678,7 +681,7 @@ extern const struct sh_pfc_soc_info shx3_pinmux_info;
 			PORT##nr##_FN2, PORT##nr##_FN3,			\
 			PORT##nr##_FN4, PORT##nr##_FN5,			\
 			PORT##nr##_FN6, PORT##nr##_FN7			\
-		}							\
+		))							\
 	}
 
 /*
-- 
2.17.1

[PATCH 15/15] [RFC] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging

[Geert Uytterhoeven]

Do some basic sanity checks on all built-in pinmux tables (for SuperH
and Renesas ARM) when DEBUG is defined.

For now this is limited to checking config register descriptors:
  - Fixed-width and variable-width field checks,
  - Number of enum_ids must match the number of possible configurations,
  - Suggestions for reducing table sizes: reserved fields of more than 3
    bits can better be split in smaller subfields, as the storage need
    is proportional to the square of the width of the (sub)field).

This helps catching bugs early.

Probably the individual pinctrl Kconfig symbols should start depending
on COMPILE_TEST, instead of always enabling all of them.

Not-yet-signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/Kconfig  |  36 -----------
 drivers/pinctrl/sh-pfc/Makefile |  13 ++++
 drivers/pinctrl/sh-pfc/core.c   | 102 ++++++++++++++++++++++++++++++++
 drivers/pinctrl/sh-pfc/sh_pfc.h |  28 ++++++++-
 4 files changed, 142 insertions(+), 37 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/Kconfig b/drivers/pinctrl/sh-pfc/Kconfig
index e941ba60d4b7c775..7ccece42e2fd6d9a 100644
--- a/drivers/pinctrl/sh-pfc/Kconfig
+++ b/drivers/pinctrl/sh-pfc/Kconfig
@@ -22,182 +22,146 @@ config PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_EMEV2
 	def_bool y
-	depends on ARCH_EMEV2
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A73A4
 	def_bool y
-	depends on ARCH_R8A73A4
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_R8A7740
 	def_bool y
-	depends on ARCH_R8A7740
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_R8A7743
 	def_bool y
-	depends on ARCH_R8A7743
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7744
 	def_bool y
-	depends on ARCH_R8A7744
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7745
         def_bool y
-        depends on ARCH_R8A7745
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77470
         def_bool y
-        depends on ARCH_R8A77470
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A774A1
         def_bool y
-        depends on ARCH_R8A774A1
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A774C0
         def_bool y
-        depends on ARCH_R8A774C0
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7778
 	def_bool y
-	depends on ARCH_R8A7778
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7779
 	def_bool y
-	depends on ARCH_R8A7779
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7790
 	def_bool y
-	depends on ARCH_R8A7790
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7791
 	def_bool y
-	depends on ARCH_R8A7791
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7792
 	def_bool y
-	depends on ARCH_R8A7792
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7793
 	def_bool y
-	depends on ARCH_R8A7793
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7794
 	def_bool y
-	depends on ARCH_R8A7794
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7795
 	def_bool y
-	depends on ARCH_R8A7795
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A7796
         def_bool y
-        depends on ARCH_R8A7796
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77965
         def_bool y
-        depends on ARCH_R8A77965
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77970
 	def_bool y
-	depends on ARCH_R8A77970
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77980
 	def_bool y
-	depends on ARCH_R8A77980
 	select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77990
         def_bool y
-        depends on ARCH_R8A77990
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_R8A77995
         def_bool y
-        depends on ARCH_R8A77995
         select PINCTRL_SH_PFC
 
 config PINCTRL_PFC_SH7203
 	def_bool y
-	depends on CPU_SUBTYPE_SH7203
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7264
 	def_bool y
-	depends on CPU_SUBTYPE_SH7264
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7269
 	def_bool y
-	depends on CPU_SUBTYPE_SH7269
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH73A0
 	def_bool y
-	depends on ARCH_SH73A0
 	select PINCTRL_SH_PFC_GPIO
 	select REGULATOR
 
 config PINCTRL_PFC_SH7720
 	def_bool y
-	depends on CPU_SUBTYPE_SH7720
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7722
 	def_bool y
-	depends on CPU_SUBTYPE_SH7722
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7723
 	def_bool y
-	depends on CPU_SUBTYPE_SH7723
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7724
 	def_bool y
-	depends on CPU_SUBTYPE_SH7724
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7734
 	def_bool y
-	depends on CPU_SUBTYPE_SH7734
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7757
 	def_bool y
-	depends on CPU_SUBTYPE_SH7757
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7785
 	def_bool y
-	depends on CPU_SUBTYPE_SH7785
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SH7786
 	def_bool y
-	depends on CPU_SUBTYPE_SH7786
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_SHX3
 	def_bool y
-	depends on CPU_SUBTYPE_SHX3
 	select PINCTRL_SH_PFC_GPIO
 endif
diff --git a/drivers/pinctrl/sh-pfc/Makefile b/drivers/pinctrl/sh-pfc/Makefile
index 82ebb2a91ee0f998..0cba62ad8accf611 100644
--- a/drivers/pinctrl/sh-pfc/Makefile
+++ b/drivers/pinctrl/sh-pfc/Makefile
@@ -38,3 +38,16 @@ obj-$(CONFIG_PINCTRL_PFC_SH7757)	+= pfc-sh7757.o
 obj-$(CONFIG_PINCTRL_PFC_SH7785)	+= pfc-sh7785.o
 obj-$(CONFIG_PINCTRL_PFC_SH7786)	+= pfc-sh7786.o
 obj-$(CONFIG_PINCTRL_PFC_SHX3)		+= pfc-shx3.o
+
+CFLAGS_pfc-sh7203.o	+= -I$(srctree)/arch/sh/include/cpu-sh2a
+CFLAGS_pfc-sh7264.o	+= -I$(srctree)/arch/sh/include/cpu-sh2a
+CFLAGS_pfc-sh7269.o	+= -I$(srctree)/arch/sh/include/cpu-sh2a
+CFLAGS_pfc-sh7720.o	+= -I$(srctree)/arch/sh/include/cpu-sh3
+CFLAGS_pfc-sh7722.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7723.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7724.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7734.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7757.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7785.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-sh7786.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
+CFLAGS_pfc-shx3.o	+= -I$(srctree)/arch/sh/include/cpu-sh4
diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index f1cfcc8c65446662..a0616992e8a6b09a 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -571,6 +571,13 @@ static const struct of_device_id sh_pfc_of_table[] = {
 		.compatible = "renesas,pfc-r8a7795",
 		.data = &r8a7795_pinmux_info,
 	},
+#ifdef DEBUG
+	{
+		/* Exercise sanity checks (nothing matches against this) */
+		.compatible = "renesas,pfc-r8a77950",	/* R-Car H3 ES1.0 */
+		.data = &r8a7795es1_pinmux_info,
+	},
+#endif /* DEBUG */
 #endif
 #ifdef CONFIG_PINCTRL_PFC_R8A7796
 	{
@@ -709,6 +716,100 @@ static int sh_pfc_suspend_init(struct sh_pfc *pfc) { return 0; }
 #define DEV_PM_OPS	NULL
 #endif /* CONFIG_PM_SLEEP && CONFIG_ARM_PSCI_FW */
 
+#ifdef DEBUG
+static bool is0s(const u16 *enum_ids, unsigned int n)
+{
+	unsigned int i;
+
+	for (i = 0; i < n; i++)
+		if (enum_ids[i])
+			return false;
+
+	return true;
+}
+
+static unsigned int sh_pfc_errors;
+static unsigned int sh_pfc_warnings;
+
+static void sh_pfc_validate_cfg_reg(const char *name,
+				    const struct pinmux_cfg_reg *cfg_reg)
+{
+	unsigned int rw, fw, i, n;
+
+	fw = cfg_reg->field_width;
+	if (fw) {
+		rw = cfg_reg->reg_width;
+		if (rw % fw) {
+			pr_err("%s reg 0x%x: reg_width %u is not a multiple of field_width %u\n",
+			       name, cfg_reg->reg, rw, fw);
+			sh_pfc_errors++;
+		}
+		n = rw / fw * (1 << fw);
+	} else {
+		for (i = 0, n = 0, rw = 0; (fw = cfg_reg->var_field_width[i]);
+		     i++) {
+			if (fw > 3 && is0s(&cfg_reg->enum_ids[n], 1 << fw)) {
+				pr_warn("%s reg %x: Reserved field [%u:%u] should be split to reduce table size\n",
+					name, cfg_reg->reg, rw, rw + fw - 1);
+				sh_pfc_warnings++;
+			}
+			n += 1 << fw;
+			rw += fw;
+		}
+
+		if (rw != cfg_reg->reg_width) {
+			pr_err("%s reg 0x%x: var_field_width declares %u instead of %u bits\n",
+			       name, cfg_reg->reg, rw, cfg_reg->reg_width);
+			sh_pfc_errors++;
+		}
+
+	}
+	if (cfg_reg->nr_enum_ids != n) {
+		pr_err("%s reg 0x%x: enum_ids[] contains %u instead of %u values\n",
+		       name, cfg_reg->reg, cfg_reg->nr_enum_ids, n);
+		sh_pfc_errors++;
+	}
+}
+
+static void sh_pfc_validate_info(const char *name,
+				 const struct sh_pfc_soc_info *info)
+{
+	const struct pinmux_cfg_reg *cfg_regs;
+	unsigned int i;
+
+	pr_info("%s: Validating %s/%s (%ps)\n", __func__, name, info->name,
+		info);
+
+	cfg_regs = info->cfg_regs;
+	for (i = 0; cfg_regs && cfg_regs[i].reg; i++)
+		sh_pfc_validate_cfg_reg(info->name, &cfg_regs[i]);
+}
+
+static void sh_pfc_validate_driver(const struct device_driver *drv)
+{
+	const struct platform_driver *pdrv = to_platform_driver(drv);
+	unsigned int i;
+
+	pr_warn("%s: Checking builtin pinmux tables\n", __func__);
+
+	for (i = 0; pdrv->id_table[i].name[0]; i++)
+		sh_pfc_validate_info(pdrv->id_table[i].name,
+				     (void *)pdrv->id_table[i].driver_data);
+
+#ifdef CONFIG_OF
+	for (i = 0; drv->of_match_table[i].compatible[0]; i++)
+		sh_pfc_validate_info(drv->of_match_table[i].compatible,
+				     drv->of_match_table[i].data);
+#endif
+
+	pr_warn("%s: Detected %u errors and %u warnings\n", __func__,
+		sh_pfc_errors, sh_pfc_warnings);
+}
+
+#else
+static inline void sh_pfc_validate_driver(struct device_driver *driver) {}
+#endif
+
 static int sh_pfc_probe(struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
@@ -718,6 +819,7 @@ static int sh_pfc_probe(struct platform_device *pdev)
 	struct sh_pfc *pfc;
 	int ret;
 
+	sh_pfc_validate_driver(pdev->dev.driver);
 #ifdef CONFIG_OF
 	if (np)
 		info = of_device_get_match_data(&pdev->dev);
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 584d58d954961018..e1310a751175f71b 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -113,6 +113,9 @@ struct pinmux_cfg_reg {
 	u8 reg_width, field_width;
 	const u16 *enum_ids;
 	const u8 *var_field_width;
+#ifdef DEBUG
+	unsigned int nr_enum_ids;
+#endif
 };
 
 #define GROUP(...)	__VA_ARGS__
@@ -128,10 +131,21 @@ struct pinmux_cfg_reg {
  *          combination of the register field bit values, wrapped using the
  *          GROUP() macro.
  */
+#ifdef DEBUG
+
+#define PINMUX_CFG_REG(name, r, r_width, f_width, ids)			\
+	.reg = r, .reg_width = r_width, .field_width = f_width,		\
+	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])	\
+		{ ids }, \
+	.nr_enum_ids = sizeof((const u16 []) { ids }) / sizeof(u16)
+
+#else
+
 #define PINMUX_CFG_REG(name, r, r_width, f_width, ids)			\
 	.reg = r, .reg_width = r_width, .field_width = f_width,		\
 	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])	\
 		{ ids }
+#endif
 
 /*
  * Describe a config register consisting of several fields of different widths
@@ -145,11 +159,23 @@ struct pinmux_cfg_reg {
  *          combination of the register field bit values, wrapped using the
  *          GROUP() macro.
  */
+#ifdef DEBUG
+
+#define PINMUX_CFG_REG_VAR(name, r, r_width, f_widths, ids)		\
+	.reg = r, .reg_width = r_width,					\
+	.var_field_width = (const u8 []) { f_widths, 0 },		\
+	.enum_ids = (const u16 []) { ids },				\
+	.nr_enum_ids = sizeof((const u16 []) { ids }) / sizeof(u16)
+
+#else
+
 #define PINMUX_CFG_REG_VAR(name, r, r_width, f_widths, ids)		\
 	.reg = r, .reg_width = r_width,					\
 	.var_field_width = (const u8 []) { f_widths, 0 },		\
 	.enum_ids = (const u16 []) { ids }
 
+#endif
+
 struct pinmux_drive_reg_field {
 	u16 pin;
 	u8 offset;
@@ -265,7 +291,7 @@ struct sh_pfc_soc_info {
 	const struct sh_pfc_function *functions;
 	unsigned int nr_functions;
 
-#ifdef CONFIG_SUPERH
+#if 1 //def CONFIG_SUPERH
 	const struct pinmux_func *func_gpios;
 	unsigned int nr_func_gpios;
 #endif
-- 
2.17.1

Re: [PATCH 01/15] pinctrl: sh-pfc: r8a77994: Remove bogus IPSR9 field

[Sergei Shtylyov]

Hello!

    like I told you on IRC, the subjects of this and next 2 patches has 
duplicate 9 in the model #s...

MBR, Sergei

Re: [PATCH 01/15] pinctrl: sh-pfc: r8a77994: Remove bogus IPSR9 field

[Geert Uytterhoeven]

Hi Sergei,

On Fri, Dec 14, 2018 at 9:00 AM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>     like I told you on IRC, the subjects of this and next 2 patches has
> duplicate 9 in the model #s...

Thanks, already fixed locally, as I told you on IRC ;-)

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

Re: [PATCH 01/15] pinctrl: sh-pfc: r8a77994: Remove bogus IPSR9 field

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:00PM +0100, Geert Uytterhoeven wrote:
> The Peripheral Function Select Register 9 contains 12 fields, but the
> variable field descriptor contains a 13th bogus field of 3 bits.
> 
> Fixes: 43c4436e2f1890a7 ("pinctrl: sh-pfc: add R8A7794 PFC support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 02/15] pinctrl: sh-pfc: r8a779970: Add missing MOD_SEL0 field

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:01PM +0100, Geert Uytterhoeven wrote:
> The Module Select Register 0 contains 20 (= 5 x 4) reserved bits, and 12
> single-bit fields, but the variable field descriptor lacks a field of 4
> reserved bits.
> 
> Fixes: b92ac66a1819602b ("pinctrl: sh-pfc: Add R8A77970 PFC support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 03/15] pinctrl: sh-pfc: r8a779980: Add missing MOD_SEL0 field

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:02PM +0100, Geert Uytterhoeven wrote:
> The Module Select Register 0 contains 20 (= 5 x 4) reserved bits, and 12
> single-bit fields, but the variable field descriptor lacks a field of 4
> reserved bits.
> 
> Fixes: f59125248a691dfe ("pinctrl: sh-pfc: Add R8A77980 PFC support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 05/15] pinctrl: sh-pfc: r8a77990: Fix MOD_SEL0 SEL_I2C1 field width

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:04PM +0100, Geert Uytterhoeven wrote:
> The SEL_I2C1 (MOD_SEL0[21:20]) field in Module Select Register 0 has a
> width of 2 bits, i.e. it allows programming one out of 4 different
> configurations.
> However, the MOD_SEL0_21_20 macro contains 8 values instead of 4,
> overflowing into the subsequent fields in the register, and thus breaking
> the configuration of the latter.
> 
> Fix this by dropping the bogus last 4 values, including the non-existent
> SEL_I2C1_4 configuration.
> 
> Fixes: 6d4036a1e3b3ac0f ("pinctrl: sh-pfc: Initial R8A77990 PFC support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 04/15] pinctrl: sh-pfc: sh7734: Add missing IPSR11 field

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:03PM +0100, Geert Uytterhoeven wrote:
> The Peripheral Function Select Register 11 contains 3 reserved bits and
> 15 variable-width fields, but the variable field descriptor does not
> contain the 3-bit field IP11[25:23].
> 
> Fixes: 856cb4bb337ee504 ("sh: Add support pinmux for SH7734")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 06/15] pinctrl: sh-pfc: r8a77995: Remove bogus SEL_PWM[0-3]_3 configurations

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:05PM +0100, Geert Uytterhoeven wrote:
> While the SEL_PWM[0-3] fields in the Module Select Register 0 support 4

s/support/supports

> possible configurations per PWM pin, only the first 3 are valid.
> 
> Replace the invalid and unused configurations for SEL_PWM[0-3]_3 by
> dummies.
> 
> Fixes: 794a6711764658a1 ("pinctrl: sh-pfc: Initial R8A77995 PFC support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 06/15] pinctrl: sh-pfc: r8a77995: Remove bogus SEL_PWM[0-3]_3 configurations

[Geert Uytterhoeven]

Hi Simon,

On Mon, Dec 17, 2018 at 3:07 PM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 13, 2018 at 07:27:05PM +0100, Geert Uytterhoeven wrote:
> > While the SEL_PWM[0-3] fields in the Module Select Register 0 support 4
>
> s/support/supports

"fields" is plural, hence "support".

> > possible configurations per PWM pin, only the first 3 are valid.
> >
> > Replace the invalid and unused configurations for SEL_PWM[0-3]_3 by
> > dummies.
> >
> > Fixes: 794a6711764658a1 ("pinctrl: sh-pfc: Initial R8A77995 PFC support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thanks!

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

Re: [PATCH 07/15] pinctrl: sh-pfc: sh7264: Fix PFCR3 and PFCR0 register configuration

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:06PM +0100, Geert Uytterhoeven wrote:
> The Port F Control Register 3 (PFCR3) contains only a single field.
> However, counting from left to right, it is the fourth field, not the
> first field.
> Insert the missing dummy configuration values (3 fields of 16 values) to
> fix this.
> 
> The descriptor for the Port F Control Register 0 (PFCR0) lacks the
> description for the 4th field (PF0 Mode, PF0MD[2:0]).
> Add the missing configuration values to fix this.
> 
> Fixes: a8d42fc4217b1ea1 ("sh-pfc: Add sh7264 pinmux support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 08/15] pinctrl: sh-pfc: sh7269: Add missing PCIOR0 field

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:07PM +0100, Geert Uytterhoeven wrote:
> The Port C I/O Register 0 contains 7 reserved bits, but the descriptor
> contains only dummy configuration values for 6 reserved bits, thus
> breaking the configuration of all subsequent fields in the register.
> 
> Fix this by adding the two missing configuration values.
> 
> Fixes: f5e811f2a43117b2 ("sh-pfc: Add sh7269 pinmux support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 09/15] pinctrl: sh-pfc: sh7734: Remove bogus IPSR10 value

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:08PM +0100, Geert Uytterhoeven wrote:
> The IP10[5:3] field in Peripheral Function Select Register 10 has a
> width of 3 bits, i.e. it allows programming one out of 8 different
> configurations.
> However, 9 values are provided instead of 8, overflowing into the
> subsequent field in the register, and thus breaking the configuration of
> the latter.
> 
> Fix this by dropping a bogus zero value.
> 
> Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 10/15] pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> Some values in the Peripheral Function Select Register 10 descriptor are
> shifted by one position, which may cause a peripheral function to be
> programmed incorrectly.
> 
> Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> already the case for the HSCK0 pin in field IP10[5:3].
> 
> Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Based on the SH7734 Hardware User's Manual Rev. 1.00.
> Compile-tested only.
> 
> Noticed when investigating a different bug in the same register
> description.
> ---
>  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> index cad70f9cf5699f0c..748a32a3af82d368 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
>  	    /* IP10_22 [1] */
>  		FN_CAN_CLK_A, FN_RX4_D,
>  	    /* IP10_21_19 [3] */
> -		FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> -		FN_LCD_M_DISP_B, 0, 0, 0,
> +		FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,

FN_FSE_B does no appear to be documented for this field in Users' Manual
Hardware v1.00 (Jun 12, 2012). Should it be 0?

Otherwise this patch looks good.


Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


> +		FN_LCD_M_DISP_B, 0, 0,
>  	    /* IP10_18_16 [3] */
> -		FN_AUDIO_CLKC, FN_SCK1_E, FN_HCTS0_C, FN_FRB_B,
> -		FN_LCD_VEPWC_B, 0, 0, 0,
> +		FN_AUDIO_CLKC, FN_SCK1_E, 0, FN_HCTS0_C, FN_FRB_B,
> +		FN_LCD_VEPWC_B, 0, 0,
>  	    /* IP10_15 [1] */
>  		FN_AUDIO_CLKB_A, FN_LCD_CLK_B,
>  	    /* IP10_14_12 [3] */
>  		FN_AUDIO_CLKA_A, FN_VI1_CLK_B, FN_SCK1_D, FN_IECLK_B,
>  		FN_LCD_FLM_B, 0, 0, 0,
>  	    /* IP10_11_9 [3] */
> -		FN_SSI_SDATA3, FN_VI1_7_B, FN_HTX0_C, FN_FWE_B,
> -		FN_LCD_CL2_B, 0, 0, 0,
> +		FN_SSI_SDATA3, FN_VI1_7_B, 0, FN_HTX0_C, FN_FWE_B,
> +		FN_LCD_CL2_B, 0, 0,
>  	    /* IP10_8_6 [3] */
> -		FN_SSI_SDATA2, FN_VI1_6_B, FN_HRX0_C, FN_FRE_B,
> -		FN_LCD_CL1_B, 0, 0, 0,
> +		FN_SSI_SDATA2, FN_VI1_6_B, 0, FN_HRX0_C, FN_FRE_B,
> +		FN_LCD_CL1_B, 0, 0,
>  	    /* IP10_5_3 [3] */
>  		FN_SSI_WS23, FN_VI1_5_B, FN_TX1_D, FN_HSCK0_C, FN_FALE_B,
>  		FN_LCD_DON_B, 0, 0,
> -- 
> 2.17.1
> 

Re: [PATCH 10/15] pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10

[Geert Uytterhoeven]

Hi Simon,

On Mon, Dec 17, 2018 at 3:37 PM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> > Some values in the Peripheral Function Select Register 10 descriptor are
> > shifted by one position, which may cause a peripheral function to be
> > programmed incorrectly.
> >
> > Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> > already the case for the HSCK0 pin in field IP10[5:3].
> >
> > Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Based on the SH7734 Hardware User's Manual Rev. 1.00.
> > Compile-tested only.
> >
> > Noticed when investigating a different bug in the same register
> > description.
> > ---
> >  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > index cad70f9cf5699f0c..748a32a3af82d368 100644
> > --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> >           /* IP10_22 [1] */
> >               FN_CAN_CLK_A, FN_RX4_D,
> >           /* IP10_21_19 [3] */
> > -             FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> > -             FN_LCD_M_DISP_B, 0, 0, 0,
> > +             FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
>
> FN_FSE_B does no appear to be documented for this field in Users' Manual
> Hardware v1.00 (Jun 12, 2012). Should it be 0?

None of the FSE bits seem to be documented in the public datasheet, so I
just retained it, maintaining consistency with the surrounding fields.

> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thanks!

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

Re: [PATCH 11/15] pinctrl: sh-pfc: Print actual field width for variable-width fields

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> The debug code in sh_pfc_write_config_reg() prints the width of the
> field being modified.
> 
> However, registers with a variable-width field layout are identified by
> pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> of the actual field widths.
> 
> Fix this by printing the Hamming height of the field mask instead, which
> is correct for both fixed-width and variable-width fields.

s/height/width ?

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

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/pinctrl/sh-pfc/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index a10f7050a74f35ff..f1cfcc8c65446662 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -221,7 +221,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
>  
>  	dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
>  		"r_width = %u, f_width = %u\n",
> -		crp->reg, value, field, crp->reg_width, crp->field_width);
> +		crp->reg, value, field, crp->reg_width, hweight32(mask));
>  
>  	mask = ~(mask << pos);
>  	value = value << pos;
> -- 
> 2.17.1
> 

Re: [PATCH 12/15] pinctrl: sh-pfc: Make pinmux_cfg_reg.var_field_width[] variable-length

[Simon Horman]

On Thu, Dec 13, 2018 at 07:27:11PM +0100, Geert Uytterhoeven wrote:
> pinmux_cfg_reg.var_field_width[] is actually a variable-length array,
> terminated by a zero, and counting at most r_width entries.
> Usually the number of entries is much smaller than r_width, so the
> ability to catch bugs at compile time through an "excess elements in
> array initializer" warning is fairly limited.
> 
> Hence make the array variable-length, decreasing kernel size slightly.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 11/15] pinctrl: sh-pfc: Print actual field width for variable-width fields

[Geert Uytterhoeven]

Hi Simon,

On Mon, Dec 17, 2018 at 3:46 PM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> > The debug code in sh_pfc_write_config_reg() prints the width of the
> > field being modified.
> >
> > However, registers with a variable-width field layout are identified by
> > pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> > of the actual field widths.
> >
> > Fix this by printing the Hamming height of the field mask instead, which
> > is correct for both fixed-width and variable-width fields.
>
> s/height/width ?

Nah, weight. Will fix.
https://en.wikipedia.org/wiki/Hamming_weight

> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thanks!

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

Re: [PATCH 11/15] pinctrl: sh-pfc: Print actual field width for variable-width fields

[Simon Horman]

On Mon, Dec 17, 2018 at 03:58:52PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Dec 17, 2018 at 3:46 PM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 13, 2018 at 07:27:10PM +0100, Geert Uytterhoeven wrote:
> > > The debug code in sh_pfc_write_config_reg() prints the width of the
> > > field being modified.
> > >
> > > However, registers with a variable-width field layout are identified by
> > > pinmux_cfg_reg.field_width being zero, hence zeroes are printed instead
> > > of the actual field widths.
> > >
> > > Fix this by printing the Hamming height of the field mask instead, which
> > > is correct for both fixed-width and variable-width fields.
> >
> > s/height/width ?
> 
> Nah, weight. Will fix.
> https://en.wikipedia.org/wiki/Hamming_weight

I was thinking weight :)

> 
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thanks!
> 
> 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
> 

Re: [PATCH 10/15] pinctrl: sh-pfc: sh7734: Fix shifted values in IPSR10

[Simon Horman]

On Mon, Dec 17, 2018 at 03:45:26PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Dec 17, 2018 at 3:37 PM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 13, 2018 at 07:27:09PM +0100, Geert Uytterhoeven wrote:
> > > Some values in the Peripheral Function Select Register 10 descriptor are
> > > shifted by one position, which may cause a peripheral function to be
> > > programmed incorrectly.
> > >
> > > Fixing this makes all HSCIF0 pins use Function 4 (value 3), like was
> > > already the case for the HSCK0 pin in field IP10[5:3].
> > >
> > > Fixes: ac1ebc2190f575fc ("sh-pfc: Add sh7734 pinmux support")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > Based on the SH7734 Hardware User's Manual Rev. 1.00.
> > > Compile-tested only.
> > >
> > > Noticed when investigating a different bug in the same register
> > > description.
> > > ---
> > >  drivers/pinctrl/sh-pfc/pfc-sh7734.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/sh-pfc/pfc-sh7734.c b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > index cad70f9cf5699f0c..748a32a3af82d368 100644
> > > --- a/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > +++ b/drivers/pinctrl/sh-pfc/pfc-sh7734.c
> > > @@ -2210,22 +2210,22 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
> > >           /* IP10_22 [1] */
> > >               FN_CAN_CLK_A, FN_RX4_D,
> > >           /* IP10_21_19 [3] */
> > > -             FN_AUDIO_CLKOUT, FN_TX1_E, FN_HRTS0_C, FN_FSE_B,
> > > -             FN_LCD_M_DISP_B, 0, 0, 0,
> > > +             FN_AUDIO_CLKOUT, FN_TX1_E, 0, FN_HRTS0_C, FN_FSE_B,
> >
> > FN_FSE_B does no appear to be documented for this field in Users' Manual
> > Hardware v1.00 (Jun 12, 2012). Should it be 0?
> 
> None of the FSE bits seem to be documented in the public datasheet, so I
> just retained it, maintaining consistency with the surrounding fields.

That is fine by me.

> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thanks!
> 
> 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
> 

Re: [PATCH 06/15] pinctrl: sh-pfc: r8a77995: Remove bogus SEL_PWM[0-3]_3 configurations

[Simon Horman]

On Mon, Dec 17, 2018 at 03:12:27PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Dec 17, 2018 at 3:07 PM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Dec 13, 2018 at 07:27:05PM +0100, Geert Uytterhoeven wrote:
> > > While the SEL_PWM[0-3] fields in the Module Select Register 0 support 4
> >
> > s/support/supports
> 
> "fields" is plural, hence "support".

Yes, I see that now. Sorry for the noise.

> > > possible configurations per PWM pin, only the first 3 are valid.
> > >
> > > Replace the invalid and unused configurations for SEL_PWM[0-3]_3 by
> > > dummies.
> > >
> > > Fixes: 794a6711764658a1 ("pinctrl: sh-pfc: Initial R8A77995 PFC support")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thanks!
> 
> 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
>