On Sun, 14 Jun 2026 12:32:32 +0200, Jordi Trepat Mur <[email protected]>
wrote:
> single_add_gpio_func() calls ofnode_parse_phandle_with_args() once per
> gpio-range entry. With a flat device tree, every call re-iterates the
> property from index 0 and, because cellname is set, performs an
> fdt_node_offset_by_phandle() (a full-FDT scan) at every step. The total
> cost is therefore quadratic in the number of entries and proportional
> to the size of the device tree.
>
> On a TI J722S board this makes the pinctrl probe take 723 ms
I'm unable to reproduce the delays you have observed with the J722s EVM. Are you
using a custom DT perhaps?
>
>
> diff --git a/drivers/pinctrl/pinctrl-single.c
> b/drivers/pinctrl/pinctrl-single.c
> index 42980e097e0..442979519d4 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -511,27 +511,61 @@ static int single_add_gpio_func(struct udevice *dev)
> const char *cellname = "#pinctrl-single,gpio-range-cells";
> struct single_gpiofunc_range *range;
> struct ofnode_phandle_args gpiospec;
> - int ret, i;
> -
> - for (i = 0; ; i++) {
> - ret = ofnode_parse_phandle_with_args(dev_ofnode(dev), propname,
> - cellname, 0, i, &gpiospec);
> - /* Do not treat it as error. Only treat it as end condition. */
> - if (ret) {
> - ret = 0;
> - break;
> + const __be32 *list;
> + int size, total_cells, cells_per_entry, entries, ret, i;
> + int ret_total = 0;
Why rename 'ret' to 'ret_total' here? From what I can see, it's still being used
for the same purpose.
> +
> + /*
> + * Read the property once and iterate over the raw cell array in
> + * memory. Calling ofnode_parse_phandle_with_args() once per index
> + * re-walks the property from the start on every call and resolves
> + * the phandle (a full-FDT scan) at each step, which is O(N^2) in
> + * the number of entries and very slow pre-relocation with caches
> + * off. Resolve only the first entry's phandle to validate the
> + * cell layout; the target node itself is never used here.
> + */
> + list = ofnode_get_property(dev_ofnode(dev), propname, &size);
> + if (!list || size <= 0)
The size check is redundant here, the size is only negative with error code if
list is NULL.
> [ ... skip 20 lines ... ]
> +
> + for (i = 0; i < entries; i++) {
> + const __be32 *entry = list + i * cells_per_entry;
> + u32 phandle_i = be32_to_cpup(entry + 0);
> +
> + if (phandle_i == 0) {
You could just do `if (!phandle_i)` here. Also, is the validation for a null
phandle required here? Wouldn't a properly configured DT never hit this case?
> + /* empty entry - match original: skip without creating
> range */
> + continue;
> }
> range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
I wonder if we can just allocate a pool of ranges[entries] outside the loop
since we already know how many entries we would need. This would avoid the
repeated calls to malloc and the subsequent null check.
--
Anshul Dalal <[email protected]>
