linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing
@ 2019-01-25 16:52 Geert Uytterhoeven
  2019-01-25 16:52 ` [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time Geert Uytterhoeven
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

	Hi all,

This patch series contains patches to add more validation of Renesas pin
control tables, and to enable compile-testing on non-Renesas platforms.
Preferably, validation is done at build time.
If not possible, it is done at run time, when the driver is initialized,
and debugging is enabled.

Changes compared to v1:
  - Convert some checks from run-time to build-time checks,
  - Check relations between pin groups and functions,
  - Check PINMUX_DATA_REG() enum IDs,
  - Split run-time checks in two patches (parts before/after enum ID
    absorption), to reduce dependencies,
  - Perform run-time checks even when running on non-Renesas platforms,
  - Improve compile-test support, and move it to separate patches.

As patches 3-5 are very large, I only included the generic part of each
patch, and a sample for one particular SoC.

The full patch series is available in the topic/sh-pfc-validation-v2
branch of my git repository at
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git

Thanks for your comments!

Geert Uytterhoeven (10):
  pinctrl: sh-pfc: Validate fixed-size field widths at build time
  pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging
  pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro
  pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro
  pinctrl: sh-pfc: Absorb enum IDs in PINMUX_DATA_REG() macro
  pinctrl: sh-pfc: Validate enum IDs for regs with fixed-width fields
  pinctrl: sh-pfc: Validate enum IDs for regs with variable-width fields
  pinctrl: sh-pfc: Introduce PINCTRL_SH_FUNC_GPIO helper symbol
  pinctrl: sh-pfc: Add missing #include <linux/errno.h>
  pinctrl: sh-pfc: Allow compile-testing of all drivers

 drivers/pinctrl/sh-pfc/Kconfig           | 204 +++++++++---------
 drivers/pinctrl/sh-pfc/Makefile          |  15 ++
 drivers/pinctrl/sh-pfc/core.c            | 129 ++++++++++++
 drivers/pinctrl/sh-pfc/gpio.c            |   8 +-
 drivers/pinctrl/sh-pfc/pfc-emev2.c       |  67 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c     |  64 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a7740.c     |  56 ++---
 drivers/pinctrl/sh-pfc/pfc-r8a77470.c    | 136 +++++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a7778.c     | 101 +++++----
 drivers/pinctrl/sh-pfc/pfc-r8a7779.c     | 117 ++++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c     | 132 +++++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c     | 156 ++++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7792.c     | 134 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c     | 127 +++++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 125 +++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c     | 130 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c     | 130 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c    | 130 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c    |  68 +++---
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c    |  76 +++----
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c    | 107 +++++-----
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c    |  98 ++++-----
 drivers/pinctrl/sh-pfc/pfc-sh7203.c      | 152 +++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7264.c      | 232 ++++++++++-----------
 drivers/pinctrl/sh-pfc/pfc-sh7269.c      | 252 +++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh73a0.c      |  52 ++---
 drivers/pinctrl/sh-pfc/pfc-sh7720.c      | 144 ++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7722.c      | 220 ++++++++++----------
 drivers/pinctrl/sh-pfc/pfc-sh7723.c      | 200 +++++++++---------
 drivers/pinctrl/sh-pfc/pfc-sh7724.c      | 204 +++++++++---------
 drivers/pinctrl/sh-pfc/pfc-sh7734.c      | 140 +++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7757.c      | 244 +++++++++++-----------
 drivers/pinctrl/sh-pfc/pfc-sh7785.c      | 136 ++++++------
 drivers/pinctrl/sh-pfc/pfc-sh7786.c      |  80 +++----
 drivers/pinctrl/sh-pfc/pfc-shx3.c        |  32 +--
 drivers/pinctrl/sh-pfc/sh_pfc.h          |  73 ++++---
 36 files changed, 2404 insertions(+), 2067 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

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

* [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
@ 2019-01-25 16:52 ` Geert Uytterhoeven
  2019-01-28 12:27   ` Simon Horman
  2019-01-25 16:52 ` [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging Geert Uytterhoeven
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Add a build-time check, to ensure the register and field widths in
descriptors for config registers with fixed-width fields are sane.
This helps catching bugs early.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Convert from run-time to build-time check.
---
 drivers/pinctrl/sh-pfc/sh_pfc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 46d477ff510921f3..56016cb76769c97b 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -126,7 +126,8 @@ struct pinmux_cfg_reg {
  * one for each possible combination of the register field bit values.
  */
 #define PINMUX_CFG_REG(name, r, r_width, f_width) \
-	.reg = r, .reg_width = r_width, .field_width = f_width,		\
+	.reg = r, .reg_width = r_width,					\
+	.field_width = f_width + BUILD_BUG_ON_ZERO(r_width % f_width),	\
 	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])
 
 /*
-- 
2.17.1


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

* [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
  2019-01-25 16:52 ` [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time Geert Uytterhoeven
@ 2019-01-25 16:52 ` Geert Uytterhoeven
  2019-02-15  8:17   ` Simon Horman
  2019-01-25 16:52 ` [PATCH v2 03/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro Geert Uytterhoeven
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Perform some basic sanity checks on all built-in pinmux tables when
DEBUG is defined, to help catching bugs early.

For now the following checks are included:
  - Check register and field widths in descriptors for config registers
    with variable-width fields,
  - Check relations between pin groups and functions:
      - All pin functions must refer to existing pin groups,
      - All pin groups must be referred to by a pin function,
      - Warn if a pin group is referred to by multiple pin functions
	(which is OK for backwards-compatibility aliases),
  - Provide 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,

Note that a dummy non-matching entry is added to the DT match table for
checking r8a7795es1_pinmux_info, as R-Car H3 ES1.0 is matched using
soc_device_match() in r8a7795_pinmux_init(), instead of by the DT match
table.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Drop RFC state,
  - Drop validation of fixed-with config register fields, as this is now
    done at build time,
  - Check relations between pin groups and functions,
  - Move compile-test support out,
  - Move checks depending on enum ID absorption out,
  - Move call to sh_pfc_check_driver() from sh_pfc_probe() to
    sh_pfc_init(), so the checks are even performed on non-native
    platforms when compile-testing.
---
 drivers/pinctrl/sh-pfc/core.c | 123 ++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index f1cfcc8c65446662..f9371e160872f2b4 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
+	{
+		/* For sanity checks only (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,121 @@ 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_check_cfg_reg(const char *drvname,
+				 const struct pinmux_cfg_reg *cfg_reg)
+{
+	unsigned int i, n, rw, fw;
+
+	if (cfg_reg->field_width) {
+		/* Checked at build time */
+		return;
+	}
+
+	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 0x%x: reserved field [%u:%u] can be split to reduce table size\n",
+				drvname, 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",
+		       drvname, cfg_reg->reg, rw, cfg_reg->reg_width);
+		sh_pfc_errors++;
+	}
+}
+
+static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
+{
+	const struct sh_pfc_function *func;
+	const char *drvname = info->name;
+	unsigned int *refcnts;
+	unsigned int i, j, k;
+
+	pr_info("Checking %s\n", drvname);
+
+	/* Check groups and functions */
+	refcnts = kcalloc(info->nr_groups, sizeof(*refcnts), GFP_KERNEL);
+	if (!refcnts)
+		return;
+
+	for (i = 0; func = &info->functions[i], i < info->nr_functions; i++) {
+		for (j = 0; j < func->nr_groups; j++) {
+			for (k = 0; k < info->nr_groups; k++) {
+				if (!strcmp(func->groups[j],
+					    info->groups[k].name)) {
+					refcnts[k]++;
+					goto next;
+				}
+			}
+
+			pr_err("%s: function %s: group %s not found\n",
+			       drvname, func->name, func->groups[j]);
+			sh_pfc_errors++;
+
+next:			;
+		}
+	}
+
+	for (i = 0; i < info->nr_groups; i++) {
+		if (!refcnts[i]) {
+			pr_err("%s: orphan group %s\n", drvname,
+			       info->groups[i].name);
+			sh_pfc_errors++;
+		} else if (refcnts[i] > 1) {
+			pr_err("%s: group %s referred by %u functions\n",
+			       drvname, info->groups[i].name, refcnts[i]);
+			sh_pfc_warnings++;
+		}
+	}
+
+	kfree(refcnts);
+
+	/* Check config register descriptions */
+	for (i = 0; info->cfg_regs && info->cfg_regs[i].reg; i++)
+		sh_pfc_check_cfg_reg(drvname, &info->cfg_regs[i]);
+}
+
+static void sh_pfc_check_driver(const struct platform_driver *pdrv)
+{
+	unsigned int i;
+
+	pr_warn("Checking builtin pinmux tables\n");
+
+	for (i = 0; pdrv->id_table[i].name[0]; i++)
+		sh_pfc_check_info((void *)pdrv->id_table[i].driver_data);
+
+#ifdef CONFIG_OF
+	for (i = 0; pdrv->driver.of_match_table[i].compatible[0]; i++)
+		sh_pfc_check_info(pdrv->driver.of_match_table[i].data);
+#endif
+
+	pr_warn("Detected %u errors and %u warnings\n", sh_pfc_errors,
+		sh_pfc_warnings);
+}
+
+#else /* !DEBUG */
+static inline void sh_pfc_check_driver(struct platform_driver *pdrv) {}
+#endif /* !DEBUG */
+
 static int sh_pfc_probe(struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
@@ -840,6 +962,7 @@ static struct platform_driver sh_pfc_driver = {
 
 static int __init sh_pfc_init(void)
 {
+	sh_pfc_check_driver(&sh_pfc_driver);
 	return platform_driver_register(&sh_pfc_driver);
 }
 postcore_initcall(sh_pfc_init);
-- 
2.17.1


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

* [PATCH v2 03/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
  2019-01-25 16:52 ` [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time Geert Uytterhoeven
  2019-01-25 16:52 ` [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging Geert Uytterhoeven
@ 2019-01-25 16:52 ` Geert Uytterhoeven
  2019-01-25 16:52 ` [PATCH v2 04/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro Geert Uytterhoeven
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, 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.

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

v2:
  - Improve comment.
---
 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 d8b13d4e9bbff7cb..31f51a5d21b68d37 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 56016cb76769c97b..5d7e4ad63b542086 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -115,20 +115,24 @@ 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, all 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 + BUILD_BUG_ON_ZERO(r_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


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

* [PATCH v2 04/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2019-01-25 16:52 ` [PATCH v2 03/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro Geert Uytterhoeven
@ 2019-01-25 16:52 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 05/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_DATA_REG() macro Geert Uytterhoeven
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:52 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, 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.

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

v2:
  - Improve comment.
---
 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-r8a7791.c b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
index 31f51a5d21b68d37..88f8f4cfa4597edb 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
@@ -5700,8 +5700,9 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		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,
-			     1, 1, 1, 1, 1, 1, 1, 1) {
+			     GROUP(1, 2, 2, 2, 2, 2, 2, 3, 1, 1, 1, 1,
+				   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1),
+			     GROUP(
 		/* IP0_31 [1] */
 		0, 0,
 		/* IP0_30_29 [2] */
@@ -5756,10 +5757,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP0_1 [1] */
 		FN_D1, 0,
 		/* IP0_0 [1] */
-		FN_D0, 0, }
+		FN_D0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR1", 0xE6060024, 32,
-			     3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2, 2) {
+			     GROUP(3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2, 2),
+			     GROUP(
 		/* IP1_31_29 [3] */
 		FN_A18, FN_DREQ1, FN_SCIFA1_RXD_C, 0, FN_SCIFB1_RXD_C,
 		0, 0, 0,
@@ -5792,10 +5794,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_A8, FN_MSIOF1_SS1, FN_I2C0_SCL, 0,
 		/* IP1_1_0 [2] */
 		FN_A7, FN_MSIOF1_SYNC,
-		0, 0, }
+		0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR2", 0xE6060028, 32,
-			     2, 3, 2, 2, 2, 2, 3, 3, 3, 3, 2, 2, 3) {
+			     GROUP(2, 3, 2, 2, 2, 2, 3, 3, 3, 3, 2, 2, 3),
+			     GROUP(
 		/* IP2_31_30 [2] */
 		0, 0, 0, 0,
 		/* IP2_29_27 [3] */
@@ -5828,10 +5831,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_A20, FN_SPCLK, 0, 0,
 		/* IP2_2_0 [3] */
 		FN_A19, FN_DACK1, FN_SCIFA1_TXD_C, 0,
-		FN_SCIFB1_TXD_C, 0, FN_SCIFB1_SCK_B, 0, }
+		FN_SCIFB1_TXD_C, 0, FN_SCIFB1_SCK_B, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR3", 0xE606002C, 32,
-			     1, 3, 3, 3, 2, 2, 2, 2, 2, 3, 3, 3, 3) {
+			     GROUP(1, 3, 3, 3, 2, 2, 2, 2, 2, 3, 3, 3, 3),
+			     GROUP(
 		/* IP3_31 [1] */
 		0, 0,
 		/* IP3_30_28 [3] */
@@ -5866,10 +5870,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_SCIFB1_RXD_B, FN_PWM1, FN_TPU_TO1, 0,
 		/* IP3_2_0 [3] */
 		FN_EX_CS4_N, FN_ATARD0_N, FN_MSIOF2_RXD, 0, FN_EX_WAIT2,
-		0, 0, 0, }
+		0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR4", 0xE6060030, 32,
-			     1, 3, 2, 2, 2, 1, 1, 1, 3, 3, 3, 2, 3, 3, 2) {
+			     GROUP(1, 3, 2, 2, 2, 1, 1, 1, 3, 3, 3, 2,
+				   3, 3, 2),
+			     GROUP(
 		/* IP4_31 [1] */
 		0, 0,
 		/* IP4_30_28 [3] */
@@ -5908,10 +5914,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_MSIOF2_SYNC_C, FN_GLO_I0_D,
 		0, 0, 0,
 		/* IP4_1_0 [2] */
-		FN_SSI_SDATA0, FN_I2C0_SCL_B, FN_IIC0_SCL_B, FN_MSIOF2_SCK_C, }
+		FN_SSI_SDATA0, FN_I2C0_SCL_B, FN_IIC0_SCL_B, FN_MSIOF2_SCK_C,
+		))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR5", 0xE6060034, 32,
-			     3, 3, 2, 2, 2, 3, 2, 3, 3, 3, 3, 3) {
+			     GROUP(3, 3, 2, 2, 2, 3, 2, 3, 3, 3, 3, 3),
+			     GROUP(
 		/* IP5_31_29 [3] */
 		FN_SSI_SDATA9, FN_RX3_D, FN_CAN0_RX_D,
 		0, 0, 0, 0, 0,
@@ -5946,10 +5954,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP5_2_0 [3] */
 		FN_SSI_WS5, FN_MSIOF1_SYNC_C, FN_TS_SCK0, FN_GLO_I1,
 		FN_MSIOF2_TXD_D, FN_VI1_R3_B,
-		0, 0, }
+		0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR6", 0xE6060038, 32,
-			     2, 3, 3, 3, 2, 3, 2, 2, 2, 2, 2, 3, 3) {
+			     GROUP(2, 3, 3, 3, 2, 3, 2, 2, 2, 2, 2, 3, 3),
+			     GROUP(
 		/* IP6_31_30 [2] */
 		0, 0, 0, 0,
 		/* IP6_29_27 [3] */
@@ -5986,10 +5995,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP6_2_0 [3] */
 		FN_AUDIO_CLKB, FN_STP_OPWM_0_B, FN_MSIOF1_SCK_B,
 		FN_SCIF_CLK, FN_DVC_MUTE, FN_BPFCLK_E,
-		0, 0, }
+		0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR7", 0xE606003C, 32,
-			     2, 3, 3, 3, 2, 2, 2, 2, 2, 2, 3, 3, 3) {
+			     GROUP(2, 3, 3, 3, 2, 2, 2, 2, 2, 2, 3, 3, 3),
+			     GROUP(
 		/* IP7_31_30 [2] */
 		0, 0, 0, 0,
 		/* IP7_29_27 [3] */
@@ -6027,10 +6037,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP7_2_0 [3] */
 		FN_IRQ9, FN_DU1_DOTCLKIN_B, FN_CAN_CLK_D, FN_GPS_MAG_C,
 		FN_SCIF_CLK_B, FN_GPS_MAG_D,
-		0, 0, }
+		0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR8", 0xE6060040, 32,
-			     1, 3, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3) {
+			     GROUP(1, 3, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3),
+			     GROUP(
 		/* IP8_31 [1] */
 		0, 0,
 		/* IP8_30_28 [3] */
@@ -6070,10 +6081,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		0, 0,
 		/* IP8_2_0 [3] */
 		FN_DU1_DG3, FN_LCDOUT11, FN_VI1_DATA5_B, 0, FN_SSI_WS78_B,
-		0, 0, 0, }
+		0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR9", 0xE6060044, 32,
-			     3, 2, 2, 2, 2, 2, 2, 1, 3, 1, 1, 3, 1, 1, 3, 3) {
+			     GROUP(3, 2, 2, 2, 2, 2, 2, 1, 3, 1, 1, 3,
+				   1, 1, 3, 3),
+			     GROUP(
 		/* IP9_31_29 [3] */
 		FN_VI0_G0, FN_IIC1_SCL, FN_STP_IVCXO27_0_C, FN_I2C4_SCL,
 		FN_HCTS2_N, FN_SCIFB2_CTS_N, FN_ATAWR1_N, 0,
@@ -6113,10 +6126,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		0, 0, 0,
 		/* IP9_2_0 [3] */
 		FN_DU1_DB6, FN_LCDOUT22, FN_I2C3_SCL_C, FN_RX3, FN_SCIFA3_RXD,
-		0, 0, 0, }
+		0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR10", 0xE6060048, 32,
-			     3, 2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 3) {
+			     GROUP(3, 2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 3),
+			     GROUP(
 		/* IP10_31_29 [3] */
 		FN_VI0_R4, FN_VI2_DATA5, FN_GLO_SCLK_B, FN_TX0_C, FN_I2C1_SCL_D,
 		0, 0, 0,
@@ -6150,11 +6164,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		FN_HSCK2, FN_SCIFB2_SCK, FN_ATARD1_N, 0,
 		/* IP10_2_0 [3] */
 		FN_VI0_G1, FN_IIC1_SDA, FN_STP_ISCLK_0_C, FN_I2C4_SDA,
-		FN_HRTS2_N, FN_SCIFB2_RTS_N, FN_ATADIR1_N, 0, }
+		FN_HRTS2_N, FN_SCIFB2_RTS_N, FN_ATADIR1_N, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR11", 0xE606004C, 32,
-			     2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2,
-			     3, 3, 3, 3, 3) {
+			     GROUP(2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
+				   2, 3, 3, 3, 3, 3),
+			     GROUP(
 		/* IP11_31_30 [2] */
 		FN_ETH_CRS_DV, FN_AVB_LINK, FN_I2C2_SDA_C, 0,
 		/* IP11_29_28 [2] */
@@ -6197,10 +6212,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		0, 0, 0,
 		/* IP11_2_0 [3] */
 		FN_VI0_R5, FN_VI2_DATA6, FN_GLO_SDATA_B, FN_RX0_C,
-		FN_I2C1_SDA_D, 0, 0, 0, }
+		FN_I2C1_SDA_D, 0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR12", 0xE6060050, 32,
-			     2, 3, 3, 2, 2, 2, 2, 3, 3, 3, 3, 2, 2) {
+			     GROUP(2, 3, 3, 2, 2, 2, 2, 3, 3, 3, 3, 2, 2),
+			     GROUP(
 		/* IP12_31_30 [2] */
 		0, 0, 0, 0,
 		/* IP12_29_27 [3] */
@@ -6238,11 +6254,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP12_3_2 [2] */
 		FN_ETH_RXD0, FN_AVB_PHY_INT, FN_I2C3_SDA, FN_IIC0_SDA,
 		/* IP12_1_0 [2] */
-		FN_ETH_RX_ER, FN_AVB_CRS, FN_I2C3_SCL, FN_IIC0_SCL, }
+		FN_ETH_RX_ER, FN_AVB_CRS, FN_I2C3_SCL, FN_IIC0_SCL, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR13", 0xE6060054, 32,
-			     1, 3, 1, 1, 1, 2, 1, 3, 3, 1, 1, 1, 1, 1, 1,
-			     3, 2, 2, 3) {
+			     GROUP(1, 3, 1, 1, 1, 2, 1, 3, 3, 1, 1, 1,
+				   1, 1, 1, 3, 2, 2, 3),
+			     GROUP(
 		/* IP13_31 [1] */
 		0, 0,
 		/* IP13_30_28 [3] */
@@ -6289,10 +6306,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP13_2_0 [3] */
 		FN_STP_ISD_0, FN_AVB_TX_ER, FN_SCIFB2_SCK_C,
 		FN_ADICLK_B, FN_MSIOF0_SS1_C,
-		0, 0, 0, }
+		0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR14", 0xE6060058, 32,
-			     3, 3, 3, 3, 3, 3, 3, 3, 1, 1, 1, 1, 1, 1, 2) {
+			     GROUP(3, 3, 3, 3, 3, 3, 3, 3, 1, 1, 1, 1,
+				   1, 1, 2),
+			     GROUP(
 		/* IP14_31_29 [3] */
 		FN_MSIOF0_SS2, FN_MMC_D7, FN_ADICHS2, FN_RX0_E,
 		FN_VI1_VSYNC_N_C, FN_IIC0_SDA_C, FN_VI1_G5_B, 0,
@@ -6332,10 +6351,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP14_2 [1] */
 		FN_SD2_CLK, FN_MMC_CLK,
 		/* IP14_1_0 [2] */
-		FN_SD1_WP, FN_PWM1_B, FN_I2C1_SDA_C, 0, }
+		FN_SD1_WP, FN_PWM1_B, FN_I2C1_SDA_C, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR15", 0xE606005C, 32,
-			     2, 3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2) {
+			     GROUP(2, 3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2),
+			     GROUP(
 		/* IP15_31_30 [2] */
 		0, 0, 0, 0,
 		/* IP15_29_27 [3] */
@@ -6373,10 +6393,11 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP15_3_2 [2] */
 		FN_SIM0_CLK, FN_IECLK, FN_CAN_CLK_C, 0,
 		/* IP15_1_0 [2] */
-		FN_SIM0_RST, FN_IETX, FN_CAN1_TX_D, 0, }
+		FN_SIM0_RST, FN_IETX, FN_CAN1_TX_D, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("IPSR16", 0xE6060160, 32,
-			     4, 4, 4, 4, 4, 2, 2, 2, 3, 3) {
+			     GROUP(4, 4, 4, 4, 4, 2, 2, 2, 3, 3),
+			     GROUP(
 		/* IP16_31_28 [4] */
 		0, 0, 0, 0, 0, 0, 0, 0,
 		0, 0, 0, 0, 0, 0, 0, 0,
@@ -6405,11 +6426,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* IP16_2_0 [3] */
 		FN_HRX1, FN_SCIFB1_RXD, FN_VI1_R0_B,
 		FN_GLO_SDATA_C, FN_VI1_DATA6_C,
-		0, 0, 0, }
+		0, 0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL", 0xE6060090, 32,
-			     1, 2, 2, 2, 3, 2, 1, 1, 1, 1,
-			     3, 2, 2, 2, 1, 2, 2, 2) {
+			     GROUP(1, 2, 2, 2, 3, 2, 1, 1, 1, 1, 3, 2,
+				   2, 2, 1, 2, 2, 2),
+			     GROUP(
 		/* RESERVED [1] */
 		0, 0,
 		/* SEL_SCIF1 [2] */
@@ -6450,11 +6472,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* SEL_TSIF0 [2] */
 		FN_SEL_TSIF0_0, FN_SEL_TSIF0_1, FN_SEL_TSIF0_2, FN_SEL_TSIF0_3,
 		/* SEL_SOF0 [2] */
-		FN_SEL_SOF0_0, FN_SEL_SOF0_1, FN_SEL_SOF0_2, 0, }
+		FN_SEL_SOF0_0, FN_SEL_SOF0_1, FN_SEL_SOF0_2, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL2", 0xE6060094, 32,
-			     3, 1, 1, 3, 2, 1, 1, 2, 2,
-			     1, 3, 2, 1, 2, 2, 2, 1, 1, 1) {
+			     GROUP(3, 1, 1, 3, 2, 1, 1, 2, 2, 1, 3, 2,
+				   1, 2, 2, 2, 1, 1, 1),
+			     GROUP(
 		/* SEL_SCIF0 [3] */
 		FN_SEL_SCIF0_0, FN_SEL_SCIF0_1, FN_SEL_SCIF0_2,
 		FN_SEL_SCIF0_3, FN_SEL_SCIF0_4,
@@ -6498,11 +6521,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* RESERVED [1] */
 		0, 0,
 		/* SEL_SSI8 [1] */
-		FN_SEL_SSI8_0, FN_SEL_SSI8_1, }
+		FN_SEL_SSI8_0, FN_SEL_SSI8_1, ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL3", 0xE6060098, 32,
-			     2, 2, 2, 2, 2, 2, 2, 2,
-			     1, 1, 2, 2, 3, 2, 2, 2, 1) {
+			     GROUP(2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 2, 2,
+				   3, 2, 2, 2, 1),
+			     GROUP(
 		/* SEL_HSCIF2 [2] */
 		FN_SEL_HSCIF2_0, FN_SEL_HSCIF2_1,
 		FN_SEL_HSCIF2_2, FN_SEL_HSCIF2_3,
@@ -6540,11 +6564,12 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* RESERVED [2] */
 		0, 0, 0, 0,
 		/* RESERVED [1] */
-		0, 0, }
+		0, 0, ))
 	},
 	{ PINMUX_CFG_REG_VAR("MOD_SEL4", 0xE606009C, 32,
-			     3, 2, 2, 1, 1, 1, 1, 3, 2,
-			     2, 3, 1, 1, 1, 2, 2, 2, 2) {
+			     GROUP(3, 2, 2, 1, 1, 1, 1, 3, 2, 2, 3, 1,
+				   1, 1, 2, 2, 2, 2),
+			     GROUP(
 		/* SEL_SOF1 [3] */
 		FN_SEL_SOF1_0, FN_SEL_SOF1_1, FN_SEL_SOF1_2, FN_SEL_SOF1_3,
 		FN_SEL_SOF1_4,
@@ -6586,7 +6611,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 		/* RESERVED [2] */
 		0, 0, 0, 0,
 		/* RESERVED [2] */
-		0, 0, 0, 0, }
+		0, 0, 0, 0, ))
 	},
 	{ },
 };
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 5d7e4ad63b542086..4cbde9c7a105e5bc 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -139,16 +139,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, all 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;
@@ -667,7 +668,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 */					\
@@ -679,7 +682,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


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

* [PATCH v2 05/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_DATA_REG() macro
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2019-01-25 16:52 ` [PATCH v2 04/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 06/10] pinctrl: sh-pfc: Validate enum IDs for regs with fixed-width fields Geert Uytterhoeven
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Currently the PINMUX_DATA_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 the GROUP()
macro.

No functional changes.

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

v2:
  - New.
---
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c |  44 ++++++------
 drivers/pinctrl/sh-pfc/pfc-r8a7740.c |  40 +++++------
 drivers/pinctrl/sh-pfc/pfc-sh7203.c  |  28 ++++----
 drivers/pinctrl/sh-pfc/pfc-sh7264.c  |  48 ++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh7269.c  |  48 ++++++-------
 drivers/pinctrl/sh-pfc/pfc-sh73a0.c  |  40 +++++------
 drivers/pinctrl/sh-pfc/pfc-sh7720.c  |  72 +++++++++----------
 drivers/pinctrl/sh-pfc/pfc-sh7722.c  |  92 ++++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh7723.c  |  92 ++++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh7724.c  |  92 ++++++++++++------------
 drivers/pinctrl/sh-pfc/pfc-sh7734.c  |  14 ++--
 drivers/pinctrl/sh-pfc/pfc-sh7757.c  | 104 +++++++++++++--------------
 drivers/pinctrl/sh-pfc/pfc-sh7785.c  |  64 ++++++++---------
 drivers/pinctrl/sh-pfc/pfc-sh7786.c  |  36 +++++-----
 drivers/pinctrl/sh-pfc/pfc-shx3.c    |  16 ++---
 drivers/pinctrl/sh-pfc/sh_pfc.h      |  10 +--
 16 files changed, 420 insertions(+), 420 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
index f07b33c614db2a9e..bf12849defdb74a5 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a73a4.c
@@ -2464,7 +2464,7 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
 
 static const struct pinmux_data_reg pinmux_data_regs[] = {
 
-	{ PINMUX_DATA_REG("PORTL031_000DR", 0xe6054000, 32) {
+	{ PINMUX_DATA_REG("PORTL031_000DR", 0xe6054000, 32, GROUP(
 			0, PORT30_DATA, PORT29_DATA, PORT28_DATA,
 			PORT27_DATA, PORT26_DATA, PORT25_DATA, PORT24_DATA,
 			PORT23_DATA, PORT22_DATA, PORT21_DATA, PORT20_DATA,
@@ -2473,9 +2473,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT11_DATA, PORT10_DATA, PORT9_DATA, PORT8_DATA,
 			PORT7_DATA, PORT6_DATA, PORT5_DATA, PORT4_DATA,
 			PORT3_DATA, PORT2_DATA, PORT1_DATA, PORT0_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTD063_032DR", 0xe6055000, 32) {
+	{ PINMUX_DATA_REG("PORTD063_032DR", 0xe6055000, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
@@ -2484,9 +2484,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			0, 0, 0, PORT40_DATA,
 			PORT39_DATA, PORT38_DATA, PORT37_DATA, PORT36_DATA,
 			PORT35_DATA, PORT34_DATA, PORT33_DATA, PORT32_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTL095_064DR", 0xe6054004, 32) {
+	{ PINMUX_DATA_REG("PORTL095_064DR", 0xe6054004, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, PORT85_DATA, PORT84_DATA,
@@ -2495,9 +2495,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT75_DATA, PORT74_DATA, PORT73_DATA, PORT72_DATA,
 			PORT71_DATA, PORT70_DATA, PORT69_DATA, PORT68_DATA,
 			PORT67_DATA, PORT66_DATA, PORT65_DATA, PORT64_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTD127_096DR", 0xe6055004, 32) {
+	{ PINMUX_DATA_REG("PORTD127_096DR", 0xe6055004, 32, GROUP(
 			0, PORT126_DATA, PORT125_DATA, PORT124_DATA,
 			PORT123_DATA, PORT122_DATA, PORT121_DATA, PORT120_DATA,
 			PORT119_DATA, PORT118_DATA, PORT117_DATA, PORT116_DATA,
@@ -2506,9 +2506,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT107_DATA, PORT106_DATA, PORT105_DATA, PORT104_DATA,
 			PORT103_DATA, PORT102_DATA, PORT101_DATA, PORT100_DATA,
 			PORT99_DATA, PORT98_DATA, PORT97_DATA, PORT96_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTD159_128DR", 0xe6055008, 32) {
+	{ PINMUX_DATA_REG("PORTD159_128DR", 0xe6055008, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
@@ -2517,9 +2517,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			0, 0, 0, 0,
 			0, PORT134_DATA, PORT133_DATA, PORT132_DATA,
 			PORT131_DATA, PORT130_DATA, PORT129_DATA, PORT128_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTR191_160DR", 0xe6056000, 32) {
+	{ PINMUX_DATA_REG("PORTR191_160DR", 0xe6056000, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
@@ -2528,9 +2528,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT171_DATA, PORT170_DATA, PORT169_DATA, PORT168_DATA,
 			PORT167_DATA, PORT166_DATA, PORT165_DATA, PORT164_DATA,
 			PORT163_DATA, PORT162_DATA, PORT161_DATA, PORT160_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTR223_192DR", 0xe6056004, 32) {
+	{ PINMUX_DATA_REG("PORTR223_192DR", 0xe6056004, 32, GROUP(
 			0, PORT222_DATA, PORT221_DATA, PORT220_DATA,
 			PORT219_DATA, PORT218_DATA, PORT217_DATA, PORT216_DATA,
 			PORT215_DATA, PORT214_DATA, PORT213_DATA, PORT212_DATA,
@@ -2539,9 +2539,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT203_DATA, PORT202_DATA, PORT201_DATA, PORT200_DATA,
 			PORT199_DATA, PORT198_DATA, PORT197_DATA, PORT196_DATA,
 			PORT195_DATA, PORT194_DATA, PORT193_DATA, PORT192_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTR255_224DR", 0xe6056008, 32) {
+	{ PINMUX_DATA_REG("PORTR255_224DR", 0xe6056008, 32, GROUP(
 			0, 0, 0, 0,
 			0, PORT250_DATA, PORT249_DATA, PORT248_DATA,
 			PORT247_DATA, PORT246_DATA, PORT245_DATA, PORT244_DATA,
@@ -2550,9 +2550,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT235_DATA, PORT234_DATA, PORT233_DATA, PORT232_DATA,
 			PORT231_DATA, PORT230_DATA, PORT229_DATA, PORT228_DATA,
 			PORT227_DATA, PORT226_DATA, PORT225_DATA, PORT224_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTR287_256DR", 0xe605600C, 32) {
+	{ PINMUX_DATA_REG("PORTR287_256DR", 0xe605600C, 32, GROUP(
 			0, 0, 0, 0,
 			PORT283_DATA, PORT282_DATA, PORT281_DATA, PORT280_DATA,
 			PORT279_DATA, PORT278_DATA, PORT277_DATA, PORT276_DATA,
@@ -2561,9 +2561,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT267_DATA, PORT266_DATA, PORT265_DATA, PORT264_DATA,
 			PORT263_DATA, PORT262_DATA, PORT261_DATA, PORT260_DATA,
 			PORT259_DATA, PORT258_DATA, PORT257_DATA, PORT256_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTU319_288DR", 0xe6057000, 32) {
+	{ PINMUX_DATA_REG("PORTU319_288DR", 0xe6057000, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, PORT308_DATA,
@@ -2572,9 +2572,9 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			PORT299_DATA, PORT298_DATA, PORT297_DATA, PORT296_DATA,
 			PORT295_DATA, PORT294_DATA, PORT293_DATA, PORT292_DATA,
 			PORT291_DATA, PORT290_DATA, PORT289_DATA, PORT288_DATA,
-		}
+		))
 	},
-	{ PINMUX_DATA_REG("PORTU351_320DR", 0xe6057004, 32) {
+	{ PINMUX_DATA_REG("PORTU351_320DR", 0xe6057004, 32, GROUP(
 			0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
@@ -2583,7 +2583,7 @@ static const struct pinmux_data_reg pinmux_data_regs[] = {
 			0, 0, PORT329_DATA, PORT328_DATA,
 			PORT327_DATA, PORT326_DATA, PORT325_DATA, PORT324_DATA,
 			PORT323_DATA, PORT322_DATA, PORT321_DATA, PORT320_DATA,
-		}
+		))
 	},
 	{ },
 };
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 4cbde9c7a105e5bc..77a47d01f82844c7 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -192,12 +192,12 @@ struct pinmux_data_reg {
  *   - name: Register name (unused, for documentation purposes only)
  *   - r: Physical register address
  *   - r_width: Width of the register (in bits)
- * This macro must be followed by initialization data: For each register bit
- * (from left to right, i.e. MSB to LSB), one enum ID must be specified.
+ *   - ids: For each register bit (from left to right, i.e. MSB to LSB), one
+ *          enum ID must be specified, all wrapped using the GROUP() macro.
  */
-#define PINMUX_DATA_REG(name, r, r_width) \
-	.reg = r, .reg_width = r_width,	\
-	.enum_ids = (const u16 [r_width]) \
+#define PINMUX_DATA_REG(name, r, r_width, ids)				\
+	.reg = r, .reg_width = r_width,					\
+	.enum_ids = (const u16 [r_width]) { ids }
 
 struct pinmux_irq {
 	const short *gpios;
-- 
2.17.1


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

* [PATCH v2 06/10] pinctrl: sh-pfc: Validate enum IDs for regs with fixed-width fields
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2019-01-25 16:53 ` [PATCH v2 05/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_DATA_REG() macro Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 07/10] pinctrl: sh-pfc: Validate enum IDs for regs with variable-width fields Geert Uytterhoeven
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Add build-time checks to the PINMUX_CFG_REG() and PINMUX_DATA_REG()
macros, to ensure the number of provided enum IDs is correct.

This helps catching bugs early.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Convert from run-time to build-time check.
  - Add check for PINMUX_DATA_REG().
---
 drivers/pinctrl/sh-pfc/sh_pfc.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 77a47d01f82844c7..dd483dbb71f06be3 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -130,7 +130,9 @@ struct pinmux_cfg_reg {
  */
 #define PINMUX_CFG_REG(name, r, r_width, f_width, ids)			\
 	.reg = r, .reg_width = r_width,					\
-	.field_width = f_width + BUILD_BUG_ON_ZERO(r_width % f_width),	\
+	.field_width = f_width + BUILD_BUG_ON_ZERO(r_width % f_width) +	\
+	BUILD_BUG_ON_ZERO(sizeof((const u16 []) { ids }) / sizeof(u16) != \
+			  (r_width / f_width) * (1 << f_width)),	\
 	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])	\
 		{ ids }
 
@@ -196,7 +198,9 @@ struct pinmux_data_reg {
  *          enum ID must be specified, all wrapped using the GROUP() macro.
  */
 #define PINMUX_DATA_REG(name, r, r_width, ids)				\
-	.reg = r, .reg_width = r_width,					\
+	.reg = r, .reg_width = r_width +				\
+	BUILD_BUG_ON_ZERO(sizeof((const u16 []) { ids }) / sizeof(u16) != \
+			  r_width),					\
 	.enum_ids = (const u16 [r_width]) { ids }
 
 struct pinmux_irq {
-- 
2.17.1


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

* [PATCH v2 07/10] pinctrl: sh-pfc: Validate enum IDs for regs with variable-width fields
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2019-01-25 16:53 ` [PATCH v2 06/10] pinctrl: sh-pfc: Validate enum IDs for regs with fixed-width fields Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 08/10] pinctrl: sh-pfc: Introduce PINCTRL_SH_FUNC_GPIO helper symbol Geert Uytterhoeven
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Add a run-time check to the PINMUX_CFG_REG_VAR() macro, to ensure the
number of provided enum IDs is correct.  This cannot be done at build
time, as the number of values depends on the variable-width fields in
the config register.

This helps catching bugs early.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Extract into its own patch.
---
 drivers/pinctrl/sh-pfc/core.c   |  6 ++++++
 drivers/pinctrl/sh-pfc/sh_pfc.h | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index f9371e160872f2b4..271f190d08a509f8 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -756,6 +756,12 @@ static void sh_pfc_check_cfg_reg(const char *drvname,
 		       drvname, cfg_reg->reg, rw, cfg_reg->reg_width);
 		sh_pfc_errors++;
 	}
+
+	if (n != cfg_reg->nr_enum_ids) {
+		pr_err("%s: reg 0x%x: enum_ids[] has %u instead of %u values\n",
+		       drvname, cfg_reg->reg, cfg_reg->nr_enum_ids, n);
+		sh_pfc_errors++;
+	}
 }
 
 static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index dd483dbb71f06be3..9f5d54b851b14ac9 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;	/* for variable width regs only */
+#endif
 };
 
 #define GROUP(...)	__VA_ARGS__
@@ -148,11 +151,23 @@ struct pinmux_cfg_reg {
  *          combination of the register field bit values, all 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;
-- 
2.17.1


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

* [PATCH v2 08/10] pinctrl: sh-pfc: Introduce PINCTRL_SH_FUNC_GPIO helper symbol
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (6 preceding siblings ...)
  2019-01-25 16:53 ` [PATCH v2 07/10] pinctrl: sh-pfc: Validate enum IDs for regs with variable-width fields Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 09/10] pinctrl: sh-pfc: Add missing #include <linux/errno.h> Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 10/10] pinctrl: sh-pfc: Allow compile-testing of all drivers Geert Uytterhoeven
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Pinctrl drivers for SuperH platforms use legacy function GPIOs.
Currently this support is compiled in based on the SUPERH platform
dependency, which hinders the introduction of compile-testing support
for the affected pinctrl drivers.

Introduce a new Kconfig symbol PINCTRL_SH_FUNC_GPIO, which is
auto-selected when needed.  This symbol in turn selects
PINCTRL_SH_PFC_GPIO, to reduce the number of per-driver selects.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/pinctrl/sh-pfc/Kconfig  | 30 ++++++++++++++++++------------
 drivers/pinctrl/sh-pfc/gpio.c   |  8 ++++----
 drivers/pinctrl/sh-pfc/sh_pfc.h |  2 +-
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/Kconfig b/drivers/pinctrl/sh-pfc/Kconfig
index e941ba60d4b7c775..6a9e4334dbfa7ec0 100644
--- a/drivers/pinctrl/sh-pfc/Kconfig
+++ b/drivers/pinctrl/sh-pfc/Kconfig
@@ -20,6 +20,12 @@ config PINCTRL_SH_PFC_GPIO
 	help
 	  This enables pin control and GPIO drivers for SH/SH Mobile platforms
 
+config PINCTRL_SH_FUNC_GPIO
+	select PINCTRL_SH_PFC_GPIO
+	bool
+	help
+	  This enables legacy function GPIOs for SH platforms
+
 config PINCTRL_PFC_EMEV2
 	def_bool y
 	depends on ARCH_EMEV2
@@ -138,17 +144,17 @@ config PINCTRL_PFC_R8A77995
 config PINCTRL_PFC_SH7203
 	def_bool y
 	depends on CPU_SUBTYPE_SH7203
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7264
 	def_bool y
 	depends on CPU_SUBTYPE_SH7264
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7269
 	def_bool y
 	depends on CPU_SUBTYPE_SH7269
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH73A0
 	def_bool y
@@ -159,45 +165,45 @@ config PINCTRL_PFC_SH73A0
 config PINCTRL_PFC_SH7720
 	def_bool y
 	depends on CPU_SUBTYPE_SH7720
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7722
 	def_bool y
 	depends on CPU_SUBTYPE_SH7722
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7723
 	def_bool y
 	depends on CPU_SUBTYPE_SH7723
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7724
 	def_bool y
 	depends on CPU_SUBTYPE_SH7724
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7734
 	def_bool y
 	depends on CPU_SUBTYPE_SH7734
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7757
 	def_bool y
 	depends on CPU_SUBTYPE_SH7757
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7785
 	def_bool y
 	depends on CPU_SUBTYPE_SH7785
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7786
 	def_bool y
 	depends on CPU_SUBTYPE_SH7786
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SHX3
 	def_bool y
 	depends on CPU_SUBTYPE_SHX3
-	select PINCTRL_SH_PFC_GPIO
+	select PINCTRL_SH_FUNC_GPIO
 endif
diff --git a/drivers/pinctrl/sh-pfc/gpio.c b/drivers/pinctrl/sh-pfc/gpio.c
index 4f3a34ee1cd454b8..97c1332c1045739a 100644
--- a/drivers/pinctrl/sh-pfc/gpio.c
+++ b/drivers/pinctrl/sh-pfc/gpio.c
@@ -252,7 +252,7 @@ static int gpio_pin_setup(struct sh_pfc_chip *chip)
  * Function GPIOs
  */
 
-#ifdef CONFIG_SUPERH
+#ifdef CONFIG_PINCTRL_SH_FUNC_GPIO
 static int gpio_function_request(struct gpio_chip *gc, unsigned offset)
 {
 	static bool __print_once;
@@ -292,7 +292,7 @@ static int gpio_function_setup(struct sh_pfc_chip *chip)
 
 	return 0;
 }
-#endif
+#endif /* CONFIG_PINCTRL_SH_FUNC_GPIO */
 
 /* -----------------------------------------------------------------------------
  * Register/unregister
@@ -369,7 +369,7 @@ int sh_pfc_register_gpiochip(struct sh_pfc *pfc)
 	if (IS_ENABLED(CONFIG_OF) && pfc->dev->of_node)
 		return 0;
 
-#ifdef CONFIG_SUPERH
+#ifdef CONFIG_PINCTRL_SH_FUNC_GPIO
 	/*
 	 * Register the GPIO to pin mappings. As pins with GPIO ports
 	 * must come first in the ranges, skip the pins without GPIO
@@ -397,7 +397,7 @@ int sh_pfc_register_gpiochip(struct sh_pfc *pfc)
 	chip = sh_pfc_add_gpiochip(pfc, gpio_function_setup, NULL);
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
-#endif /* CONFIG_SUPERH */
+#endif /* CONFIG_PINCTRL_SH_FUNC_GPIO */
 
 	return 0;
 }
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index 9f5d54b851b14ac9..9ca43e7b2eff155f 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -285,7 +285,7 @@ struct sh_pfc_soc_info {
 	const struct sh_pfc_function *functions;
 	unsigned int nr_functions;
 
-#ifdef CONFIG_SUPERH
+#ifdef CONFIG_PINCTRL_SH_FUNC_GPIO
 	const struct pinmux_func *func_gpios;
 	unsigned int nr_func_gpios;
 #endif
-- 
2.17.1


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

* [PATCH v2 09/10] pinctrl: sh-pfc: Add missing #include <linux/errno.h>
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (7 preceding siblings ...)
  2019-01-25 16:53 ` [PATCH v2 08/10] pinctrl: sh-pfc: Introduce PINCTRL_SH_FUNC_GPIO helper symbol Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  2019-01-25 16:53 ` [PATCH v2 10/10] pinctrl: sh-pfc: Allow compile-testing of all drivers Geert Uytterhoeven
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Source files using -Exxx error codes should include <linux/errno.h>.
On ARM, this header file is included indirectly, but not on SuperH,
leading to "error: ‘EINVAL’ undeclared" failures when enabling
compile-testing later.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/pinctrl/sh-pfc/pfc-r8a77470.c    | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c     | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c     | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7794.c     | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c     | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c     | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c    | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a77970.c    | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a77980.c    | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c    | 1 +
 drivers/pinctrl/sh-pfc/pfc-r8a77995.c    | 1 +
 12 files changed, 12 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77470.c b/drivers/pinctrl/sh-pfc/pfc-r8a77470.c
index 59384150615f4471..c05dc14904861285 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77470.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77470.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2018 Renesas Electronics Corp.
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "sh_pfc.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index bccdc44828108831..c41a6761cf9d4352 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -8,6 +8,7 @@
  * Copyright (C) 2012  Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
  */
 
+#include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/sys_soc.h>
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
index 88f8f4cfa4597edb..1292ec8d268fc41f 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2014-2017 Cogent Embedded, Inc.
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "sh_pfc.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
index dc0fab281e53ff1e..1ff4969d8381fff4 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2015-2017 Cogent Embedded, Inc. <source@cogentembedded.com>
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/sys_soc.h>
 
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
index cf8aba550945102f..59f636603f13c2cc 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2015-2017  Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "core.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index de9b1d1500940cf6..234f1e33f4fdf628 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2015-2017 Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/sys_soc.h>
 
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
index f49752ddae6518ce..69070aac9c36b809 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
@@ -11,6 +11,7 @@
  * Copyright (C) 2015  Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "core.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
index 88c577e2f6c0d6bb..fc512e6bf577d9b6 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
@@ -12,6 +12,7 @@
  * Copyright (C) 2015  Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "core.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77970.c b/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
index 232e9592122ce33e..d39ddef48d67b0bb 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77970.c
@@ -12,6 +12,7 @@
  * Copyright (C) 2015  Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77980.c b/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
index 6e2bcc92096eb1f3..f8846f431947ed28 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77980.c
@@ -12,6 +12,7 @@
  * Copyright (C) 2015 Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
index 0eacde85ade48dae..dad5a71ca2107b7c 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
@@ -11,6 +11,7 @@
  * Copyright (C) 2016-2017 Renesas Electronics Corp.
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "core.h"
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77995.c b/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
index 71a13c07570d6068..e02e707eaea20072 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77995.c
@@ -11,6 +11,7 @@
  * Copyright (C) 2015  Renesas Electronics Corporation
  */
 
+#include <linux/errno.h>
 #include <linux/kernel.h>
 
 #include "core.h"
-- 
2.17.1


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

* [PATCH v2 10/10] pinctrl: sh-pfc: Allow compile-testing of all drivers
  2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
                   ` (8 preceding siblings ...)
  2019-01-25 16:53 ` [PATCH v2 09/10] pinctrl: sh-pfc: Add missing #include <linux/errno.h> Geert Uytterhoeven
@ 2019-01-25 16:53 ` Geert Uytterhoeven
  9 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-01-25 16:53 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Enable compile-testing of all Renesas SuperH and ARM pin control
drivers, in a similar way as was done before for clock and SoC drivers
in commits 371dd373c6edd557 ("clk: renesas: Allow compile-testing of all
(sub)drivers") and 8be381a131c29c47 ("soc: renesas: Rework Kconfig and
Makefile logic").

The SuperH pin control drivers need specific include files, hence make
sure they are always found when compile-testing.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Add proper COMPILE_TEST support, instead of just enabling all
    drivers unconditionally,
  - Drop RFC state.
---
 drivers/pinctrl/sh-pfc/Kconfig  | 174 ++++++++++++++------------------
 drivers/pinctrl/sh-pfc/Makefile |  15 +++
 2 files changed, 90 insertions(+), 99 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/Kconfig b/drivers/pinctrl/sh-pfc/Kconfig
index 6a9e4334dbfa7ec0..2dd716b016a3f39b 100644
--- a/drivers/pinctrl/sh-pfc/Kconfig
+++ b/drivers/pinctrl/sh-pfc/Kconfig
@@ -3,19 +3,53 @@
 # Renesas SH and SH Mobile PINCTRL drivers
 #
 
-if ARCH_RENESAS || SUPERH
-
 config PINCTRL_SH_PFC
+	bool "Renesas SoC pin control support" if COMPILE_TEST && !(ARCH_RENESAS || SUPERH)
+	default y if ARCH_RENESAS || SUPERH
 	select PINMUX
 	select PINCONF
 	select GENERIC_PINCONF
-	def_bool y
+	select PINCTRL_PFC_EMEV2 if ARCH_EMEV2
+	select PINCTRL_PFC_R8A73A4 if ARCH_R8A73A4
+	select PINCTRL_PFC_R8A7740 if ARCH_R8A7740
+	select PINCTRL_PFC_R8A7743 if ARCH_R8A7743
+	select PINCTRL_PFC_R8A7744 if ARCH_R8A7744
+	select PINCTRL_PFC_R8A7745 if ARCH_R8A7745
+	select PINCTRL_PFC_R8A77470 if ARCH_R8A77470
+	select PINCTRL_PFC_R8A774A1 if ARCH_R8A774A1
+	select PINCTRL_PFC_R8A774C0 if ARCH_R8A774C0
+	select PINCTRL_PFC_R8A7778 if ARCH_R8A7778
+	select PINCTRL_PFC_R8A7779 if ARCH_R8A7779
+	select PINCTRL_PFC_R8A7790 if ARCH_R8A7790
+	select PINCTRL_PFC_R8A7791 if ARCH_R8A7791
+	select PINCTRL_PFC_R8A7792 if ARCH_R8A7792
+	select PINCTRL_PFC_R8A7793 if ARCH_R8A7793
+	select PINCTRL_PFC_R8A7794 if ARCH_R8A7794
+	select PINCTRL_PFC_R8A7795 if ARCH_R8A7795
+	select PINCTRL_PFC_R8A7796 if ARCH_R8A7796
+	select PINCTRL_PFC_R8A77965 if ARCH_R8A77965
+	select PINCTRL_PFC_R8A77970 if ARCH_R8A77970
+	select PINCTRL_PFC_R8A77980 if ARCH_R8A77980
+	select PINCTRL_PFC_R8A77990 if ARCH_R8A77990
+	select PINCTRL_PFC_R8A77995 if ARCH_R8A77995
+	select PINCTRL_PFC_SH7203 if CPU_SUBTYPE_SH7203
+	select PINCTRL_PFC_SH7264 if CPU_SUBTYPE_SH7264
+	select PINCTRL_PFC_SH7269 if CPU_SUBTYPE_SH7269
+	select PINCTRL_PFC_SH73A0 if ARCH_SH73A0
+	select PINCTRL_PFC_SH7720 if CPU_SUBTYPE_SH7720
+	select PINCTRL_PFC_SH7722 if CPU_SUBTYPE_SH7722
+	select PINCTRL_PFC_SH7723 if CPU_SUBTYPE_SH7723
+	select PINCTRL_PFC_SH7724 if CPU_SUBTYPE_SH7724
+	select PINCTRL_PFC_SH7734 if CPU_SUBTYPE_SH7734
+	select PINCTRL_PFC_SH7757 if CPU_SUBTYPE_SH7757
+	select PINCTRL_PFC_SH7785 if CPU_SUBTYPE_SH7785
+	select PINCTRL_PFC_SH7786 if CPU_SUBTYPE_SH7786
+	select PINCTRL_PFC_SHX3 if CPU_SUBTYPE_SHX3
 	help
-	  This enables pin control drivers for SH and SH Mobile platforms
+	  This enables pin control drivers for Renesas SuperH and ARM platforms
 
 config PINCTRL_SH_PFC_GPIO
 	select GPIOLIB
-	select PINCTRL_SH_PFC
 	bool
 	help
 	  This enables pin control and GPIO drivers for SH/SH Mobile platforms
@@ -27,183 +61,125 @@ config PINCTRL_SH_FUNC_GPIO
 	  This enables legacy function GPIOs for SH platforms
 
 config PINCTRL_PFC_EMEV2
-	def_bool y
-	depends on ARCH_EMEV2
-	select PINCTRL_SH_PFC
+	bool "Emma Mobile AV2 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A73A4
-	def_bool y
-	depends on ARCH_R8A73A4
+	bool "R-Mobile APE6 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_R8A7740
-	def_bool y
-	depends on ARCH_R8A7740
+	bool "R-Mobile A1 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_PFC_GPIO
 
 config PINCTRL_PFC_R8A7743
-	def_bool y
-	depends on ARCH_R8A7743
-	select PINCTRL_SH_PFC
+	bool "RZ/G1M pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7744
-	def_bool y
-	depends on ARCH_R8A7744
-	select PINCTRL_SH_PFC
+	bool "RZ/G1N pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7745
-        def_bool y
-        depends on ARCH_R8A7745
-        select PINCTRL_SH_PFC
+	bool "RZ/G1E pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77470
-        def_bool y
-        depends on ARCH_R8A77470
-        select PINCTRL_SH_PFC
+	bool "RZ/G1C pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A774A1
-        def_bool y
-        depends on ARCH_R8A774A1
-        select PINCTRL_SH_PFC
+	bool "RZ/G2M pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A774C0
-        def_bool y
-        depends on ARCH_R8A774C0
-        select PINCTRL_SH_PFC
+	bool "RZ/G2E pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7778
-	def_bool y
-	depends on ARCH_R8A7778
-	select PINCTRL_SH_PFC
+	bool "R-Car M1A pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7779
-	def_bool y
-	depends on ARCH_R8A7779
-	select PINCTRL_SH_PFC
+	bool "R-Car H1 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7790
-	def_bool y
-	depends on ARCH_R8A7790
-	select PINCTRL_SH_PFC
+	bool "R-Car H2 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7791
-	def_bool y
-	depends on ARCH_R8A7791
-	select PINCTRL_SH_PFC
+	bool "R-Car M2-W pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7792
-	def_bool y
-	depends on ARCH_R8A7792
-	select PINCTRL_SH_PFC
+	bool "R-Car V2H pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7793
-	def_bool y
-	depends on ARCH_R8A7793
-	select PINCTRL_SH_PFC
+	bool "R-Car M2-N pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7794
-	def_bool y
-	depends on ARCH_R8A7794
-	select PINCTRL_SH_PFC
+	bool "R-Car E2 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7795
-	def_bool y
-	depends on ARCH_R8A7795
-	select PINCTRL_SH_PFC
+	bool "R-Car H3 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A7796
-        def_bool y
-        depends on ARCH_R8A7796
-        select PINCTRL_SH_PFC
+	bool "R-Car M3-W pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77965
-        def_bool y
-        depends on ARCH_R8A77965
-        select PINCTRL_SH_PFC
+	bool "R-Car M3-N pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77970
-	def_bool y
-	depends on ARCH_R8A77970
-	select PINCTRL_SH_PFC
+	bool "R-Car V3M pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77980
-	def_bool y
-	depends on ARCH_R8A77980
-	select PINCTRL_SH_PFC
+	bool "R-Car V3H pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77990
-        def_bool y
-        depends on ARCH_R8A77990
-        select PINCTRL_SH_PFC
+	bool "R-Car E3 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_R8A77995
-        def_bool y
-        depends on ARCH_R8A77995
-        select PINCTRL_SH_PFC
+	bool "R-Car D3 pin control support" if COMPILE_TEST
 
 config PINCTRL_PFC_SH7203
-	def_bool y
-	depends on CPU_SUBTYPE_SH7203
+	bool "SH7203 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7264
-	def_bool y
-	depends on CPU_SUBTYPE_SH7264
+	bool "SH7264 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7269
-	def_bool y
-	depends on CPU_SUBTYPE_SH7269
+	bool "SH7269 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH73A0
-	def_bool y
-	depends on ARCH_SH73A0
+	bool "SH-Mobile AG5 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_PFC_GPIO
 	select REGULATOR
 
 config PINCTRL_PFC_SH7720
-	def_bool y
-	depends on CPU_SUBTYPE_SH7720
+	bool "SH7720 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7722
-	def_bool y
-	depends on CPU_SUBTYPE_SH7722
+	bool "SH7722 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7723
-	def_bool y
-	depends on CPU_SUBTYPE_SH7723
+	bool "SH-Mobile R2 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7724
-	def_bool y
-	depends on CPU_SUBTYPE_SH7724
+	bool "SH-Mobile R2R pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7734
-	def_bool y
-	depends on CPU_SUBTYPE_SH7734
+	bool "SH7734 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7757
-	def_bool y
-	depends on CPU_SUBTYPE_SH7757
+	bool "SH7757 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7785
-	def_bool y
-	depends on CPU_SUBTYPE_SH7785
+	bool "SH7785 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SH7786
-	def_bool y
-	depends on CPU_SUBTYPE_SH7786
+	bool "SH7786 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
 
 config PINCTRL_PFC_SHX3
-	def_bool y
-	depends on CPU_SUBTYPE_SHX3
+	bool "SH-X3 pin control support" if COMPILE_TEST
 	select PINCTRL_SH_FUNC_GPIO
-endif
diff --git a/drivers/pinctrl/sh-pfc/Makefile b/drivers/pinctrl/sh-pfc/Makefile
index 82ebb2a91ee0f998..8c95abcfcc006371 100644
--- a/drivers/pinctrl/sh-pfc/Makefile
+++ b/drivers/pinctrl/sh-pfc/Makefile
@@ -38,3 +38,18 @@ 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
+
+ifeq ($(CONFIG_COMPILE_TEST),y)
+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
+endif
-- 
2.17.1


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

* Re: [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time
  2019-01-25 16:52 ` [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time Geert Uytterhoeven
@ 2019-01-28 12:27   ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2019-01-28 12:27 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Fri, Jan 25, 2019 at 05:52:56PM +0100, Geert Uytterhoeven wrote:
> Add a build-time check, to ensure the register and field widths in
> descriptors for config registers with fixed-width fields are sane.
> This helps catching bugs early.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Convert from run-time to build-time check.

Nice :)

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

> ---
>  drivers/pinctrl/sh-pfc/sh_pfc.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
> index 46d477ff510921f3..56016cb76769c97b 100644
> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -126,7 +126,8 @@ struct pinmux_cfg_reg {
>   * one for each possible combination of the register field bit values.
>   */
>  #define PINMUX_CFG_REG(name, r, r_width, f_width) \
> -	.reg = r, .reg_width = r_width, .field_width = f_width,		\
> +	.reg = r, .reg_width = r_width,					\
> +	.field_width = f_width + BUILD_BUG_ON_ZERO(r_width % f_width),	\
>  	.enum_ids = (const u16 [(r_width / f_width) * (1 << f_width)])
>  
>  /*
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging
  2019-01-25 16:52 ` [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging Geert Uytterhoeven
@ 2019-02-15  8:17   ` Simon Horman
  2019-02-15  8:43     ` Geert Uytterhoeven
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Horman @ 2019-02-15  8:17 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Fri, Jan 25, 2019 at 05:52:57PM +0100, Geert Uytterhoeven wrote:
> Perform some basic sanity checks on all built-in pinmux tables when
> DEBUG is defined, to help catching bugs early.
> 
> For now the following checks are included:
>   - Check register and field widths in descriptors for config registers
>     with variable-width fields,
>   - Check relations between pin groups and functions:
>       - All pin functions must refer to existing pin groups,
>       - All pin groups must be referred to by a pin function,
>       - Warn if a pin group is referred to by multiple pin functions
> 	(which is OK for backwards-compatibility aliases),
>   - Provide 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,
> 
> Note that a dummy non-matching entry is added to the DT match table for
> checking r8a7795es1_pinmux_info, as R-Car H3 ES1.0 is matched using
> soc_device_match() in r8a7795_pinmux_init(), instead of by the DT match
> table.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Hi Geert,

I noted a few nits below, but in writing them I got to feel
that perhaps its just me not appreciating the merit of
some of the idioms used. So feel free to ignore them.

Nits aside,

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

> ---
> v2:
>   - Drop RFC state,
>   - Drop validation of fixed-with config register fields, as this is now
>     done at build time,
>   - Check relations between pin groups and functions,
>   - Move compile-test support out,
>   - Move checks depending on enum ID absorption out,
>   - Move call to sh_pfc_check_driver() from sh_pfc_probe() to
>     sh_pfc_init(), so the checks are even performed on non-native
>     platforms when compile-testing.
> ---
>  drivers/pinctrl/sh-pfc/core.c | 123 ++++++++++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index f1cfcc8c65446662..f9371e160872f2b4 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
> +	{
> +		/* For sanity checks only (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,121 @@ 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_check_cfg_reg(const char *drvname,
> +				 const struct pinmux_cfg_reg *cfg_reg)
> +{
> +	unsigned int i, n, rw, fw;
> +
> +	if (cfg_reg->field_width) {
> +		/* Checked at build time */
> +		return;
> +	}
> +
> +	for (i = 0, n = 0, rw = 0; (fw = cfg_reg->var_field_width[i]); i++) {

nit: The assignment in the conditional expression in the for loop is
     perhaps not the clearest way to express this, and its
     not clear that the parentheses are required.

> +		if (fw > 3 && is0s(&cfg_reg->enum_ids[n], 1 << fw)) {
> +			pr_warn("%s: reg 0x%x: reserved field [%u:%u] can be split to reduce table size\n",
> +				drvname, 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",
> +		       drvname, cfg_reg->reg, rw, cfg_reg->reg_width);
> +		sh_pfc_errors++;
> +	}
> +}
> +
> +static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> +{
> +	const struct sh_pfc_function *func;
> +	const char *drvname = info->name;
> +	unsigned int *refcnts;
> +	unsigned int i, j, k;
> +
> +	pr_info("Checking %s\n", drvname);
> +
> +	/* Check groups and functions */
> +	refcnts = kcalloc(info->nr_groups, sizeof(*refcnts), GFP_KERNEL);
> +	if (!refcnts)
> +		return;
> +
> +	for (i = 0; func = &info->functions[i], i < info->nr_functions; i++) {

nit: It might be clearer to set (and declare) func inside the loop.

> +		for (j = 0; j < func->nr_groups; j++) {
> +			for (k = 0; k < info->nr_groups; k++) {
> +				if (!strcmp(func->groups[j],
> +					    info->groups[k].name)) {
> +					refcnts[k]++;
> +					goto next;

nit: goto

> +				}
> +			}
> +
> +			pr_err("%s: function %s: group %s not found\n",
> +			       drvname, func->name, func->groups[j]);
> +			sh_pfc_errors++;
> +
> +next:			;
> +		}
> +	}
> +
> +	for (i = 0; i < info->nr_groups; i++) {
> +		if (!refcnts[i]) {
> +			pr_err("%s: orphan group %s\n", drvname,
> +			       info->groups[i].name);
> +			sh_pfc_errors++;
> +		} else if (refcnts[i] > 1) {
> +			pr_err("%s: group %s referred by %u functions\n",
> +			       drvname, info->groups[i].name, refcnts[i]);
> +			sh_pfc_warnings++;
> +		}
> +	}
> +
> +	kfree(refcnts);
> +
> +	/* Check config register descriptions */
> +	for (i = 0; info->cfg_regs && info->cfg_regs[i].reg; i++)
> +		sh_pfc_check_cfg_reg(drvname, &info->cfg_regs[i]);
> +}
> +
> +static void sh_pfc_check_driver(const struct platform_driver *pdrv)
> +{
> +	unsigned int i;
> +
> +	pr_warn("Checking builtin pinmux tables\n");
> +
> +	for (i = 0; pdrv->id_table[i].name[0]; i++)
> +		sh_pfc_check_info((void *)pdrv->id_table[i].driver_data);
> +
> +#ifdef CONFIG_OF
> +	for (i = 0; pdrv->driver.of_match_table[i].compatible[0]; i++)
> +		sh_pfc_check_info(pdrv->driver.of_match_table[i].data);
> +#endif
> +
> +	pr_warn("Detected %u errors and %u warnings\n", sh_pfc_errors,
> +		sh_pfc_warnings);
> +}
> +
> +#else /* !DEBUG */
> +static inline void sh_pfc_check_driver(struct platform_driver *pdrv) {}
> +#endif /* !DEBUG */
> +
>  static int sh_pfc_probe(struct platform_device *pdev)
>  {
>  #ifdef CONFIG_OF
> @@ -840,6 +962,7 @@ static struct platform_driver sh_pfc_driver = {
>  
>  static int __init sh_pfc_init(void)
>  {
> +	sh_pfc_check_driver(&sh_pfc_driver);
>  	return platform_driver_register(&sh_pfc_driver);
>  }
>  postcore_initcall(sh_pfc_init);
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging
  2019-02-15  8:17   ` Simon Horman
@ 2019-02-15  8:43     ` Geert Uytterhoeven
  0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2019-02-15  8:43 UTC (permalink / raw)
  To: Simon Horman
  Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
	open list:GPIO SUBSYSTEM

Hi Simon,

On Fri, Feb 15, 2019 at 9:17 AM Simon Horman <horms@verge.net.au> wrote:
> On Fri, Jan 25, 2019 at 05:52:57PM +0100, Geert Uytterhoeven wrote:
> > Perform some basic sanity checks on all built-in pinmux tables when
> > DEBUG is defined, to help catching bugs early.
> >
> > For now the following checks are included:
> >   - Check register and field widths in descriptors for config registers
> >     with variable-width fields,
> >   - Check relations between pin groups and functions:
> >       - All pin functions must refer to existing pin groups,
> >       - All pin groups must be referred to by a pin function,
> >       - Warn if a pin group is referred to by multiple pin functions
> >       (which is OK for backwards-compatibility aliases),
> >   - Provide 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,
> >
> > Note that a dummy non-matching entry is added to the DT match table for
> > checking r8a7795es1_pinmux_info, as R-Car H3 ES1.0 is matched using
> > soc_device_match() in r8a7795_pinmux_init(), instead of by the DT match
> > table.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Hi Geert,
>
> I noted a few nits below, but in writing them I got to feel
> that perhaps its just me not appreciating the merit of
> some of the idioms used. So feel free to ignore them.
>
> Nits aside,
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Thank you!

> > --- a/drivers/pinctrl/sh-pfc/core.c
> > +++ b/drivers/pinctrl/sh-pfc/core.c

> > +static void sh_pfc_check_cfg_reg(const char *drvname,
> > +                              const struct pinmux_cfg_reg *cfg_reg)
> > +{
> > +     unsigned int i, n, rw, fw;
> > +
> > +     if (cfg_reg->field_width) {
> > +             /* Checked at build time */
> > +             return;
> > +     }
> > +
> > +     for (i = 0, n = 0, rw = 0; (fw = cfg_reg->var_field_width[i]); i++) {
>
> nit: The assignment in the conditional expression in the for loop is
>      perhaps not the clearest way to express this, and its

The alternative is:

        for (i = 0, n = 0, rw = 0; cfg_reg->var_field_width[i]; i++) {
                fw = cfg_reg->var_field_width[i];

Or a while (1) { ... } with a break.
IMHO all more ugly and/or less concise than the above.

>      not clear that the parentheses are required.

Oh yes, just like in the conditions of "if" and "while" statements:

    warning: suggest parentheses around assignment used as truth value

>
> > +             if (fw > 3 && is0s(&cfg_reg->enum_ids[n], 1 << fw)) {
> > +                     pr_warn("%s: reg 0x%x: reserved field [%u:%u] can be split to reduce table size\n",
> > +                             drvname, cfg_reg->reg, rw, rw + fw - 1);
> > +                     sh_pfc_warnings++;
> > +             }
> > +             n += 1 << fw;
> > +             rw += fw;
> > +     }

> > +static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> > +{
> > +     const struct sh_pfc_function *func;
> > +     const char *drvname = info->name;
> > +     unsigned int *refcnts;
> > +     unsigned int i, j, k;
> > +
> > +     pr_info("Checking %s\n", drvname);
> > +
> > +     /* Check groups and functions */
> > +     refcnts = kcalloc(info->nr_groups, sizeof(*refcnts), GFP_KERNEL);
> > +     if (!refcnts)
> > +             return;
> > +
> > +     for (i = 0; func = &info->functions[i], i < info->nr_functions; i++) {
>
> nit: It might be clearer to set (and declare) func inside the loop.

I agree (probably it made sense at some point during development).

> > +             for (j = 0; j < func->nr_groups; j++) {
> > +                     for (k = 0; k < info->nr_groups; k++) {
> > +                             if (!strcmp(func->groups[j],
> > +                                         info->groups[k].name)) {
> > +                                     refcnts[k]++;
> > +                                     goto next;
>
> nit: goto

Yeah, it's a pity C, unlike PERL, doesn't have "next <LABEL>", to escape
out of one or more loop constructs ;-)

The alternative is "break"...

>
> > +                             }
> > +                     }

... and wrap the below in "if (k == info->nr_groups) { ... }", which makes
sense, as it still doesn't make indentation too deep (probably it did at
some point during development).

> > +
> > +                     pr_err("%s: function %s: group %s not found\n",
> > +                            drvname, func->name, func->groups[j]);
> > +                     sh_pfc_errors++;
> > +
> > +next:                        ;
> > +             }

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

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

end of thread, other threads:[~2019-02-15  8:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 16:52 [PATCH v2 00/10] pinctrl: sh-pfc: Validation and compile-testing Geert Uytterhoeven
2019-01-25 16:52 ` [PATCH v2 01/10] pinctrl: sh-pfc: Validate fixed-size field widths at build time Geert Uytterhoeven
2019-01-28 12:27   ` Simon Horman
2019-01-25 16:52 ` [PATCH v2 02/10] pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging Geert Uytterhoeven
2019-02-15  8:17   ` Simon Horman
2019-02-15  8:43     ` Geert Uytterhoeven
2019-01-25 16:52 ` [PATCH v2 03/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG() macro Geert Uytterhoeven
2019-01-25 16:52 ` [PATCH v2 04/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_CFG_REG_VAR() macro Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 05/10 PARTIAL] pinctrl: sh-pfc: Absorb enum IDs in PINMUX_DATA_REG() macro Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 06/10] pinctrl: sh-pfc: Validate enum IDs for regs with fixed-width fields Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 07/10] pinctrl: sh-pfc: Validate enum IDs for regs with variable-width fields Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 08/10] pinctrl: sh-pfc: Introduce PINCTRL_SH_FUNC_GPIO helper symbol Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 09/10] pinctrl: sh-pfc: Add missing #include <linux/errno.h> Geert Uytterhoeven
2019-01-25 16:53 ` [PATCH v2 10/10] pinctrl: sh-pfc: Allow compile-testing of all drivers Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).