* [PATCH] of: property: stop creating callback for each pinctrl-N property
@ 2025-12-19 12:18 Rasmus Villemoes
2025-12-19 14:18 ` Rob Herring
0 siblings, 1 reply; 3+ messages in thread
From: Rasmus Villemoes @ 2025-12-19 12:18 UTC (permalink / raw)
To: Rob Herring; +Cc: Saravana Kannan, devicetree, linux-kernel, Rasmus Villemoes
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] of: property: stop creating callback for each pinctrl-N property
2025-12-19 12:18 [PATCH] of: property: stop creating callback for each pinctrl-N property Rasmus Villemoes
@ 2025-12-19 14:18 ` Rob Herring
2025-12-22 10:18 ` Rasmus Villemoes
0 siblings, 1 reply; 3+ messages in thread
From: Rob Herring @ 2025-12-19 14:18 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: Saravana Kannan, devicetree, linux-kernel
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] of: property: stop creating callback for each pinctrl-N property
2025-12-19 14:18 ` Rob Herring
@ 2025-12-22 10:18 ` Rasmus Villemoes
0 siblings, 0 replies; 3+ messages in thread
From: Rasmus Villemoes @ 2025-12-22 10:18 UTC (permalink / raw)
To: Rob Herring; +Cc: Saravana Kannan, devicetree, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-22 10:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19 12:18 [PATCH] of: property: stop creating callback for each pinctrl-N property Rasmus Villemoes
2025-12-19 14:18 ` Rob Herring
2025-12-22 10:18 ` Rasmus Villemoes
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).