All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements
@ 2019-04-25  9:55 Geert Uytterhoeven
  2019-04-25  9:55 ` [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning Geert Uytterhoeven
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-25  9:55 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

	Hi Linus,

This patch series contains several fixes and enhancements for the
debug code to validate pinctrl tables at run-time.

I intend to queue this in sh-pfc-for-v5.3.

Thanks for your comments!

Geert Uytterhoeven (4):
  pinctrl: sh-pfc: Correct printk level of group referral warning
  pinctrl: sh-pfc: Mark run-time debug code __init
  pinctrl: sh-pfc: Add check for empty pinmux groups/functions
  pinctrl: sh-pfc: Validate pin tables at runtime

 drivers/pinctrl/sh-pfc/core.c | 60 +++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 10 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] 15+ messages in thread

* [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning
  2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
@ 2019-04-25  9:55 ` Geert Uytterhoeven
  2019-04-26  9:21   ` Simon Horman
  2019-04-25  9:55 ` [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init Geert Uytterhoeven
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-25  9:55 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Fixes: 6161b39a14380815 ("pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 3f989f5cb021ecbc..18d7f6d9402093d4 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -803,8 +803,8 @@ static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 			       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]);
+			pr_warn("%s: group %s referred by %u functions\n",
+				drvname, info->groups[i].name, refcnts[i]);
 			sh_pfc_warnings++;
 		}
 	}
-- 
2.17.1


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

* [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init
  2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
  2019-04-25  9:55 ` [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning Geert Uytterhoeven
@ 2019-04-25  9:55 ` Geert Uytterhoeven
  2019-04-26  9:29   ` Simon Horman
  2019-04-25  9:55 ` [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions Geert Uytterhoeven
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-25  9:55 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

All run-time debug code is called from sh_pfc_init(), which is __init.

Fixes: 6161b39a14380815 ("pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/sh-pfc/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 18d7f6d9402093d4..868b9551438efb0a 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -717,7 +717,7 @@ static int sh_pfc_suspend_init(struct sh_pfc *pfc) { return 0; }
 #endif /* CONFIG_PM_SLEEP && CONFIG_ARM_PSCI_FW */
 
 #ifdef DEBUG
-static bool is0s(const u16 *enum_ids, unsigned int n)
+static bool __init is0s(const u16 *enum_ids, unsigned int n)
 {
 	unsigned int i;
 
@@ -728,11 +728,11 @@ static bool is0s(const u16 *enum_ids, unsigned int n)
 	return true;
 }
 
-static unsigned int sh_pfc_errors;
-static unsigned int sh_pfc_warnings;
+static unsigned int sh_pfc_errors __initdata = 0;
+static unsigned int sh_pfc_warnings __initdata = 0;
 
-static void sh_pfc_check_cfg_reg(const char *drvname,
-				 const struct pinmux_cfg_reg *cfg_reg)
+static void __init sh_pfc_check_cfg_reg(const char *drvname,
+					const struct pinmux_cfg_reg *cfg_reg)
 {
 	unsigned int i, n, rw, fw;
 
@@ -764,7 +764,7 @@ static void sh_pfc_check_cfg_reg(const char *drvname,
 	}
 }
 
-static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
+static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 {
 	const struct sh_pfc_function *func;
 	const char *drvname = info->name;
@@ -816,7 +816,7 @@ static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 		sh_pfc_check_cfg_reg(drvname, &info->cfg_regs[i]);
 }
 
-static void sh_pfc_check_driver(const struct platform_driver *pdrv)
+static void __init sh_pfc_check_driver(const struct platform_driver *pdrv)
 {
 	unsigned int i;
 
-- 
2.17.1


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

* [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions
  2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
  2019-04-25  9:55 ` [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning Geert Uytterhoeven
  2019-04-25  9:55 ` [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init Geert Uytterhoeven
@ 2019-04-25  9:55 ` Geert Uytterhoeven
  2019-04-26  9:34   ` Simon Horman
  2019-04-25  9:55 ` [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime Geert Uytterhoeven
  2019-05-03  7:25 ` [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Linus Walleij
  4 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-25  9:55 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

The pinmux groups and functions arrays may contain two parts, to ease
supporting SoCs that expose pin subsets of other related SoCs.  Both
parts need to be declared with explicit sizes, which thus need to be
updated when adding support for more groups and functions.

If a size is too small, the compiler will detect this at build time
("excess elements in array initializer").
If a size is too large, this may go undetected (for pin groups), lead to
pin controller registration failures (for pin functions: "pinmux ops has
no name for functionN"), or crash the optional run-time debug code (for
pin groups).

Extend the run-time debug code with checks to detect this, to help
catching bugs early.

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

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 868b9551438efb0a..889e5af281022e1d 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 
 	for (i = 0; i < info->nr_functions; i++) {
 		func = &info->functions[i];
+		if (!func->name) {
+			pr_err("%s: empty function %u\n", drvname, i);
+			sh_pfc_errors++;
+			continue;
+		}
 		for (j = 0; j < func->nr_groups; j++) {
 			for (k = 0; k < info->nr_groups; k++) {
-				if (!strcmp(func->groups[j],
+				if (info->groups[k].name &&
+				    !strcmp(func->groups[j],
 					    info->groups[k].name)) {
 					refcnts[k]++;
 					break;
@@ -798,6 +804,11 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 	}
 
 	for (i = 0; i < info->nr_groups; i++) {
+		if (!info->groups[i].name) {
+			pr_err("%s: empty group %u\n", drvname, i);
+			sh_pfc_errors++;
+			continue;
+		}
 		if (!refcnts[i]) {
 			pr_err("%s: orphan group %s\n", drvname,
 			       info->groups[i].name);
-- 
2.17.1


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

* [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime
  2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2019-04-25  9:55 ` [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions Geert Uytterhoeven
@ 2019-04-25  9:55 ` Geert Uytterhoeven
  2019-04-26  9:36   ` Simon Horman
  2019-05-03  7:25 ` [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Linus Walleij
  4 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-25  9:55 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-renesas-soc, linux-gpio, Geert Uytterhoeven

Extend the run-time debug code with checks to ensure there are no
conflicting pin names, numbers, or enumeration values.

This helps catching bugs early.

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

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 889e5af281022e1d..3009508f466cf766 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -773,6 +773,35 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
 
 	pr_info("Checking %s\n", drvname);
 
+	/* Check pins */
+	for (i = 0; i < info->nr_pins; i++) {
+		for (j = 0; j < i; j++) {
+			if (!strcmp(info->pins[i].name, info->pins[j].name)) {
+				pr_err("%s: pin %s/%s: name conflict\n",
+				       drvname, info->pins[i].name,
+				       info->pins[j].name);
+				sh_pfc_errors++;
+			}
+
+			if (info->pins[i].pin != (u16)-1 &&
+			    info->pins[i].pin == info->pins[j].pin) {
+				pr_err("%s: pin %s/%s: pin %u conflict\n",
+				       drvname, info->pins[i].name,
+				       info->pins[j].name, info->pins[i].pin);
+				sh_pfc_errors++;
+			}
+
+			if (info->pins[i].enum_id &&
+			    info->pins[i].enum_id == info->pins[j].enum_id) {
+				pr_err("%s: pin %s/%s: enum_id %u conflict\n",
+				       drvname, info->pins[i].name,
+				       info->pins[j].name,
+				       info->pins[i].enum_id);
+				sh_pfc_errors++;
+			}
+		}
+	}
+
 	/* Check groups and functions */
 	refcnts = kcalloc(info->nr_groups, sizeof(*refcnts), GFP_KERNEL);
 	if (!refcnts)
-- 
2.17.1


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

* Re: [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning
  2019-04-25  9:55 ` [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning Geert Uytterhoeven
@ 2019-04-26  9:21   ` Simon Horman
  2019-04-29  8:38     ` Geert Uytterhoeven
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2019-04-26  9:21 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Thu, Apr 25, 2019 at 11:55:39AM +0200, Geert Uytterhoeven wrote:
> Fixes: 6161b39a14380815 ("pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

This looks good to me

Reviewed-by: Simon Horman <simon.horman@netronome.com>

But I do wonder if "referred" should be "referenced"

> ---
>  drivers/pinctrl/sh-pfc/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 3f989f5cb021ecbc..18d7f6d9402093d4 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -803,8 +803,8 @@ static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  			       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]);
> +			pr_warn("%s: group %s referred by %u functions\n",
> +				drvname, info->groups[i].name, refcnts[i]);
>  			sh_pfc_warnings++;
>  		}
>  	}
> -- 
> 2.17.1
> 

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

* Re: [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init
  2019-04-25  9:55 ` [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init Geert Uytterhoeven
@ 2019-04-26  9:29   ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-04-26  9:29 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Thu, Apr 25, 2019 at 11:55:40AM +0200, Geert Uytterhoeven wrote:
> All run-time debug code is called from sh_pfc_init(), which is __init.
> 
> Fixes: 6161b39a14380815 ("pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
>  drivers/pinctrl/sh-pfc/core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 18d7f6d9402093d4..868b9551438efb0a 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -717,7 +717,7 @@ static int sh_pfc_suspend_init(struct sh_pfc *pfc) { return 0; }
>  #endif /* CONFIG_PM_SLEEP && CONFIG_ARM_PSCI_FW */
>  
>  #ifdef DEBUG
> -static bool is0s(const u16 *enum_ids, unsigned int n)
> +static bool __init is0s(const u16 *enum_ids, unsigned int n)
>  {
>  	unsigned int i;
>  
> @@ -728,11 +728,11 @@ static bool is0s(const u16 *enum_ids, unsigned int n)
>  	return true;
>  }
>  
> -static unsigned int sh_pfc_errors;
> -static unsigned int sh_pfc_warnings;
> +static unsigned int sh_pfc_errors __initdata = 0;
> +static unsigned int sh_pfc_warnings __initdata = 0;
>  
> -static void sh_pfc_check_cfg_reg(const char *drvname,
> -				 const struct pinmux_cfg_reg *cfg_reg)
> +static void __init sh_pfc_check_cfg_reg(const char *drvname,
> +					const struct pinmux_cfg_reg *cfg_reg)
>  {
>  	unsigned int i, n, rw, fw;
>  
> @@ -764,7 +764,7 @@ static void sh_pfc_check_cfg_reg(const char *drvname,
>  	}
>  }
>  
> -static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> +static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  {
>  	const struct sh_pfc_function *func;
>  	const char *drvname = info->name;
> @@ -816,7 +816,7 @@ static void sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  		sh_pfc_check_cfg_reg(drvname, &info->cfg_regs[i]);
>  }
>  
> -static void sh_pfc_check_driver(const struct platform_driver *pdrv)
> +static void __init sh_pfc_check_driver(const struct platform_driver *pdrv)
>  {
>  	unsigned int i;
>  
> -- 
> 2.17.1
> 

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

* Re: [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions
  2019-04-25  9:55 ` [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions Geert Uytterhoeven
@ 2019-04-26  9:34   ` Simon Horman
  2019-04-26  9:42     ` Geert Uytterhoeven
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2019-04-26  9:34 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Thu, Apr 25, 2019 at 11:55:41AM +0200, Geert Uytterhoeven wrote:
> The pinmux groups and functions arrays may contain two parts, to ease
> supporting SoCs that expose pin subsets of other related SoCs.  Both
> parts need to be declared with explicit sizes, which thus need to be
> updated when adding support for more groups and functions.
> 
> If a size is too small, the compiler will detect this at build time
> ("excess elements in array initializer").
> If a size is too large, this may go undetected (for pin groups), lead to
> pin controller registration failures (for pin functions: "pinmux ops has
> no name for functionN"), or crash the optional run-time debug code (for
> pin groups).
> 
> Extend the run-time debug code with checks to detect this, to help
> catching bugs early.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

> ---
>  drivers/pinctrl/sh-pfc/core.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 868b9551438efb0a..889e5af281022e1d 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  
>  	for (i = 0; i < info->nr_functions; i++) {
>  		func = &info->functions[i];
> +		if (!func->name) {
> +			pr_err("%s: empty function %u\n", drvname, i);
> +			sh_pfc_errors++;

Not strictly related to this patch but did
you consider not having sh_pfc_errors global to this file?

> +			continue;
> +		}
>  		for (j = 0; j < func->nr_groups; j++) {
>  			for (k = 0; k < info->nr_groups; k++) {
> -				if (!strcmp(func->groups[j],
> +				if (info->groups[k].name &&
> +				    !strcmp(func->groups[j],
>  					    info->groups[k].name)) {
>  					refcnts[k]++;
>  					break;
> @@ -798,6 +804,11 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
>  	}
>  
>  	for (i = 0; i < info->nr_groups; i++) {
> +		if (!info->groups[i].name) {
> +			pr_err("%s: empty group %u\n", drvname, i);
> +			sh_pfc_errors++;
> +			continue;
> +		}
>  		if (!refcnts[i]) {
>  			pr_err("%s: orphan group %s\n", drvname,
>  			       info->groups[i].name);
> -- 
> 2.17.1
> 

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

* Re: [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime
  2019-04-25  9:55 ` [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime Geert Uytterhoeven
@ 2019-04-26  9:36   ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-04-26  9:36 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-renesas-soc, linux-gpio

On Thu, Apr 25, 2019 at 11:55:42AM +0200, Geert Uytterhoeven wrote:
> Extend the run-time debug code with checks to ensure there are no
> conflicting pin names, numbers, or enumeration values.
> 
> This helps catching bugs early.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

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

* Re: [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions
  2019-04-26  9:34   ` Simon Horman
@ 2019-04-26  9:42     ` Geert Uytterhoeven
  2019-04-26  9:53       ` Simon Horman
  0 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-26  9:42 UTC (permalink / raw)
  To: Simon Horman
  Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
	open list:GPIO SUBSYSTEM

Hi Simon,

On Fri, Apr 26, 2019 at 11:35 AM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Apr 25, 2019 at 11:55:41AM +0200, Geert Uytterhoeven wrote:
> > The pinmux groups and functions arrays may contain two parts, to ease
> > supporting SoCs that expose pin subsets of other related SoCs.  Both
> > parts need to be declared with explicit sizes, which thus need to be
> > updated when adding support for more groups and functions.
> >
> > If a size is too small, the compiler will detect this at build time
> > ("excess elements in array initializer").
> > If a size is too large, this may go undetected (for pin groups), lead to
> > pin controller registration failures (for pin functions: "pinmux ops has
> > no name for functionN"), or crash the optional run-time debug code (for
> > pin groups).
> >
> > Extend the run-time debug code with checks to detect this, to help
> > catching bugs early.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> 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
> > @@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> >
> >       for (i = 0; i < info->nr_functions; i++) {
> >               func = &info->functions[i];
> > +             if (!func->name) {
> > +                     pr_err("%s: empty function %u\n", drvname, i);
> > +                     sh_pfc_errors++;
>
> Not strictly related to this patch but did
> you consider not having sh_pfc_errors global to this file?

Do you have an alternative?
I could hide it, and combine it with the pr_err() in a sh_pfc_report_error()
function, but it would still be global.

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] 15+ messages in thread

* Re: [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions
  2019-04-26  9:42     ` Geert Uytterhoeven
@ 2019-04-26  9:53       ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-04-26  9:53 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
	open list:GPIO SUBSYSTEM

On Fri, Apr 26, 2019 at 11:42:11AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Apr 26, 2019 at 11:35 AM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Apr 25, 2019 at 11:55:41AM +0200, Geert Uytterhoeven wrote:
> > > The pinmux groups and functions arrays may contain two parts, to ease
> > > supporting SoCs that expose pin subsets of other related SoCs.  Both
> > > parts need to be declared with explicit sizes, which thus need to be
> > > updated when adding support for more groups and functions.
> > >
> > > If a size is too small, the compiler will detect this at build time
> > > ("excess elements in array initializer").
> > > If a size is too large, this may go undetected (for pin groups), lead to
> > > pin controller registration failures (for pin functions: "pinmux ops has
> > > no name for functionN"), or crash the optional run-time debug code (for
> > > pin groups).
> > >
> > > Extend the run-time debug code with checks to detect this, to help
> > > catching bugs early.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > 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
> > > @@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> > >
> > >       for (i = 0; i < info->nr_functions; i++) {
> > >               func = &info->functions[i];
> > > +             if (!func->name) {
> > > +                     pr_err("%s: empty function %u\n", drvname, i);
> > > +                     sh_pfc_errors++;
> >
> > Not strictly related to this patch but did
> > you consider not having sh_pfc_errors global to this file?
> 
> Do you have an alternative?
> I could hide it, and combine it with the pr_err() in a sh_pfc_report_error()
> function, but it would still be global.

I didn't look carefully for an alternative.
If its not practical to pass around a local variable then
I'm happy with the current setup.

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

* Re: [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning
  2019-04-26  9:21   ` Simon Horman
@ 2019-04-29  8:38     ` Geert Uytterhoeven
  2019-04-29  8:49       ` Simon Horman
  0 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-04-29  8:38 UTC (permalink / raw)
  To: Simon Horman
  Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
	open list:GPIO SUBSYSTEM

Hi Simon,

On Fri, Apr 26, 2019 at 11:21 AM Simon Horman <horms@verge.net.au> wrote:
> On Thu, Apr 25, 2019 at 11:55:39AM +0200, Geert Uytterhoeven wrote:
> > Fixes: 6161b39a14380815 ("pinctrl: sh-pfc: Validate pinmux tables at runtime when debugging")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This looks good to me
>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

Thanks!

Is the atypical email address for sh-pfc reviews intentional?

> But I do wonder if "referred" should be "referenced"

Yes, sounds better.

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] 15+ messages in thread

* Re: [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning
  2019-04-29  8:38     ` Geert Uytterhoeven
@ 2019-04-29  8:49       ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2019-04-29  8:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
	open list:GPIO SUBSYSTEM

On Mon, Apr 29, 2019 at 10:38:52AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Apr 26, 2019 at 11:21 AM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Apr 25, 2019 at 11:55:39AM +0200, Geert Uytterhoeven wrote:

...

> Thanks!
> 
> Is the atypical email address for sh-pfc reviews intentional?

Sorry about that, I meant this:

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

> > But I do wonder if "referred" should be "referenced"
> 
> Yes, sounds better.
> 
> 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] 15+ messages in thread

* Re: [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements
  2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2019-04-25  9:55 ` [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime Geert Uytterhoeven
@ 2019-05-03  7:25 ` Linus Walleij
  2019-05-03  7:33   ` Geert Uytterhoeven
  4 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2019-05-03  7:25 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linux-Renesas, open list:GPIO SUBSYSTEM

On Thu, Apr 25, 2019 at 10:55 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> This patch series contains several fixes and enhancements for the
> debug code to validate pinctrl tables at run-time.
>
> I intend to queue this in sh-pfc-for-v5.3.

I don't mind to take this in for v5.2, but it's your pick.
If you send me a pull request pronto I will pull it in.

Yours,
Linus Walleij

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

* Re: [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements
  2019-05-03  7:25 ` [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Linus Walleij
@ 2019-05-03  7:33   ` Geert Uytterhoeven
  0 siblings, 0 replies; 15+ messages in thread
From: Geert Uytterhoeven @ 2019-05-03  7:33 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Geert Uytterhoeven, Linux-Renesas, open list:GPIO SUBSYSTEM

Hi Linus,

On Fri, May 3, 2019 at 9:26 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Apr 25, 2019 at 10:55 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > This patch series contains several fixes and enhancements for the
> > debug code to validate pinctrl tables at run-time.
> >
> > I intend to queue this in sh-pfc-for-v5.3.
>
> I don't mind to take this in for v5.2, but it's your pick.
> If you send me a pull request pronto I will pull it in.

Thanks, but none of this is critical or urgent, and v5.2 is imminent
(unless you know more ;-).

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] 15+ messages in thread

end of thread, other threads:[~2019-05-03  7:33 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25  9:55 [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Geert Uytterhoeven
2019-04-25  9:55 ` [PATCH 1/4] pinctrl: sh-pfc: Correct printk level of group referral warning Geert Uytterhoeven
2019-04-26  9:21   ` Simon Horman
2019-04-29  8:38     ` Geert Uytterhoeven
2019-04-29  8:49       ` Simon Horman
2019-04-25  9:55 ` [PATCH 2/4] pinctrl: sh-pfc: Mark run-time debug code __init Geert Uytterhoeven
2019-04-26  9:29   ` Simon Horman
2019-04-25  9:55 ` [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions Geert Uytterhoeven
2019-04-26  9:34   ` Simon Horman
2019-04-26  9:42     ` Geert Uytterhoeven
2019-04-26  9:53       ` Simon Horman
2019-04-25  9:55 ` [PATCH 4/4] pinctrl: sh-pfc: Validate pin tables at runtime Geert Uytterhoeven
2019-04-26  9:36   ` Simon Horman
2019-05-03  7:25 ` [PATCH 0/4] pinctrl: sh-pfc: Run-time debug code fixes and enhancements Linus Walleij
2019-05-03  7:33   ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.