of: property: stop creating callback for each pinctrl-N property

[PATCH] of: property: stop creating callback for each pinctrl-N property

Posted by Rasmus Villemoes 1 month, 2 weeks ago

While not a lot in the grand scheme of things, this eliminates 8*2
pointless function calls for almost every property present in the
device tree (the exception are the few properties that were already
matched). It also seems to reduce .text by about 1.5K - why gcc
decides to inline parse_prop_cells() in every instantiation I don't know.

Supporting double-digit suffixes would still require tweaking, but it
does match pinctrl-9.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/of/property.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index c1feb631e383..7c6fe4529d73 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -21,6 +21,7 @@
 
 #define pr_fmt(fmt)	"OF: " fmt
 
+#include <linux/ctype.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
@@ -1358,15 +1359,6 @@ DEFINE_SIMPLE_PROP(extcon, "extcon", NULL)
 DEFINE_SIMPLE_PROP(nvmem_cells, "nvmem-cells", "#nvmem-cell-cells")
 DEFINE_SIMPLE_PROP(phys, "phys", "#phy-cells")
 DEFINE_SIMPLE_PROP(wakeup_parent, "wakeup-parent", NULL)
-DEFINE_SIMPLE_PROP(pinctrl0, "pinctrl-0", NULL)
-DEFINE_SIMPLE_PROP(pinctrl1, "pinctrl-1", NULL)
-DEFINE_SIMPLE_PROP(pinctrl2, "pinctrl-2", NULL)
-DEFINE_SIMPLE_PROP(pinctrl3, "pinctrl-3", NULL)
-DEFINE_SIMPLE_PROP(pinctrl4, "pinctrl-4", NULL)
-DEFINE_SIMPLE_PROP(pinctrl5, "pinctrl-5", NULL)
-DEFINE_SIMPLE_PROP(pinctrl6, "pinctrl-6", NULL)
-DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL)
-DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
 DEFINE_SIMPLE_PROP(pwms, "pwms", "#pwm-cells")
 DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
 DEFINE_SIMPLE_PROP(leds, "leds", NULL)
@@ -1380,6 +1372,18 @@ DEFINE_SIMPLE_PROP(power_supplies, "power-supplies", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
 
+static struct device_node *parse_pinctrl_n(struct device_node *np,
+					   const char *prop_name, int index)
+{
+	if (!strstarts(prop_name, "pinctrl-"))
+		return NULL;
+
+	if (!isdigit(prop_name[strlen("pinctrl-")]))
+		return NULL;
+
+	return of_parse_phandle(np, prop_name, index);
+}
+
 static struct device_node *parse_gpios(struct device_node *np,
 				       const char *prop_name, int index)
 {
@@ -1503,15 +1507,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_nvmem_cells, },
 	{ .parse_prop = parse_phys, },
 	{ .parse_prop = parse_wakeup_parent, },
-	{ .parse_prop = parse_pinctrl0, },
-	{ .parse_prop = parse_pinctrl1, },
-	{ .parse_prop = parse_pinctrl2, },
-	{ .parse_prop = parse_pinctrl3, },
-	{ .parse_prop = parse_pinctrl4, },
-	{ .parse_prop = parse_pinctrl5, },
-	{ .parse_prop = parse_pinctrl6, },
-	{ .parse_prop = parse_pinctrl7, },
-	{ .parse_prop = parse_pinctrl8, },
+	{ .parse_prop = parse_pinctrl_n, },
 	{
 		.parse_prop = parse_remote_endpoint,
 		.get_con_dev = of_graph_get_port_parent,
-- 
2.52.0

Re: [PATCH] of: property: stop creating callback for each pinctrl-N property

Posted by Rob Herring (Arm) 1 month ago

On Fri, 19 Dec 2025 13:18:11 +0100, Rasmus Villemoes wrote:
> While not a lot in the grand scheme of things, this eliminates 8*2
> pointless function calls for almost every property present in the
> device tree (the exception are the few properties that were already
> matched). It also seems to reduce .text by about 1.5K - why gcc
> decides to inline parse_prop_cells() in every instantiation I don't know.
> 
> Supporting double-digit suffixes would still require tweaking, but it
> does match pinctrl-9.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/of/property.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
> 

Applied, thanks!

Re: [PATCH] of: property: stop creating callback for each pinctrl-N property

Posted by Rob Herring 1 month, 2 weeks ago

On Fri, Dec 19, 2025 at 6:18 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> While not a lot in the grand scheme of things, this eliminates 8*2
> pointless function calls for almost every property present in the
> device tree (the exception are the few properties that were already
> matched). It also seems to reduce .text by about 1.5K - why gcc
> decides to inline parse_prop_cells() in every instantiation I don't know.

Presumably it is still doing that with all the other cases? Perhaps we
should add a noinline attr.

> Supporting double-digit suffixes would still require tweaking, but it
> does match pinctrl-9.

Looks to me like double-digit suffixes would work. Even pinctrl-0foo
would work (which is fine IMO).

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/of/property.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index c1feb631e383..7c6fe4529d73 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -21,6 +21,7 @@
>
>  #define pr_fmt(fmt)    "OF: " fmt
>
> +#include <linux/ctype.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_device.h>
> @@ -1358,15 +1359,6 @@ DEFINE_SIMPLE_PROP(extcon, "extcon", NULL)
>  DEFINE_SIMPLE_PROP(nvmem_cells, "nvmem-cells", "#nvmem-cell-cells")
>  DEFINE_SIMPLE_PROP(phys, "phys", "#phy-cells")
>  DEFINE_SIMPLE_PROP(wakeup_parent, "wakeup-parent", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl0, "pinctrl-0", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl1, "pinctrl-1", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl2, "pinctrl-2", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl3, "pinctrl-3", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl4, "pinctrl-4", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl5, "pinctrl-5", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl6, "pinctrl-6", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL)
> -DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
>  DEFINE_SIMPLE_PROP(pwms, "pwms", "#pwm-cells")
>  DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
>  DEFINE_SIMPLE_PROP(leds, "leds", NULL)
> @@ -1380,6 +1372,18 @@ DEFINE_SIMPLE_PROP(power_supplies, "power-supplies", NULL)
>  DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
>  DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
>
> +static struct device_node *parse_pinctrl_n(struct device_node *np,
> +                                          const char *prop_name, int index)
> +{
> +       if (!strstarts(prop_name, "pinctrl-"))
> +               return NULL;
> +
> +       if (!isdigit(prop_name[strlen("pinctrl-")]))
> +               return NULL;
> +
> +       return of_parse_phandle(np, prop_name, index);
> +}
> +
>  static struct device_node *parse_gpios(struct device_node *np,
>                                        const char *prop_name, int index)
>  {
> @@ -1503,15 +1507,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
>         { .parse_prop = parse_nvmem_cells, },
>         { .parse_prop = parse_phys, },
>         { .parse_prop = parse_wakeup_parent, },
> -       { .parse_prop = parse_pinctrl0, },
> -       { .parse_prop = parse_pinctrl1, },
> -       { .parse_prop = parse_pinctrl2, },
> -       { .parse_prop = parse_pinctrl3, },
> -       { .parse_prop = parse_pinctrl4, },
> -       { .parse_prop = parse_pinctrl5, },
> -       { .parse_prop = parse_pinctrl6, },
> -       { .parse_prop = parse_pinctrl7, },
> -       { .parse_prop = parse_pinctrl8, },
> +       { .parse_prop = parse_pinctrl_n, },
>         {
>                 .parse_prop = parse_remote_endpoint,
>                 .get_con_dev = of_graph_get_port_parent,
> --
> 2.52.0
>

Re: [PATCH] of: property: stop creating callback for each pinctrl-N property

Posted by Rasmus Villemoes 1 month, 2 weeks ago

On Fri, Dec 19 2025, Rob Herring <robh@kernel.org> wrote:

> On Fri, Dec 19, 2025 at 6:18 AM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>>
>> While not a lot in the grand scheme of things, this eliminates 8*2
>> pointless function calls for almost every property present in the
>> device tree (the exception are the few properties that were already
>> matched). It also seems to reduce .text by about 1.5K - why gcc
>> decides to inline parse_prop_cells() in every instantiation I don't know.
>
> Presumably it is still doing that with all the other cases? Perhaps we
> should add a noinline attr.

Yes, it does, parse_prop_cells() does not exist at all in the generated
code (I've just tested with arm64 defconfig and arm
imx_v6_v7_defconfig).

Adding noinline to just parse_prop_cells (on top of current patch) does

$ size drivers/of/property.o.{old,new}
   text    data     bss     dec     hex filename
  18098     278       0   18376    47c8 drivers/of/property.o.old
  14962     278       0   15240    3b88 drivers/of/property.o.new

There are only two uses of DEFINE_SUFFIX_PROP, so giving
parse_suffix_prop_cells() the same treatment doesn't result in that much
(merely 300 bytes), but should probably be done as well for consistency.

I'll send a proper patch.

>> Supporting double-digit suffixes would still require tweaking, but it
>> does match pinctrl-9.
>
> Looks to me like double-digit suffixes would work. Even pinctrl-0foo
> would work (which is fine IMO).

Ah, yes, if matching ^pinctrl-[0-9] is sufficient and not
^pinctrl-[0-9]+$ (or something even worse to
avoid-leading-0-unless-exactly-0), it does work, and I agree that that's
unlikely to ever be a problem.

Rasmus