Re: [PATCH RFC] of: Add of_parse_map_iter() helper for nexus node map iteration

[Kevin Hilman <khilman@baylibre.com>]

Rob Herring <robh@kernel.org> writes:

> +Herve
>
> On Wed, Nov 19, 2025 at 6:41 PM Kevin Hilman (TI.com)
> <khilman@baylibre.com> wrote:
>>
>> Add a new helper function of_parse_map_iter() to iterate over nexus
>> node maps (c.f. DT spec, section 2.5.1.)
>>
>> This function provides an iterator interface for traversing map
>> entries, handling the complexity of variable-sized entries based on
>> <stem>-cells properties, as well as handling the <stem>-skip and
>> <stem>-pass-thru properties.
>>
>> RFC: There's a lot of overlap between this function and
>> of_parse_phandle_with_args_map().  However the key differences are:
>>
>>   - of_parse_phandle_with_args_map() does matching
>>     it searches for an entry that matches specific child args
>>   - of_parse_map_iter() does iteration
>>     it simply walks through all entries sequentially
>
> There's also this in flight for interrupt-map:
>
> https://lore.kernel.org/all/20251027123601.77216-2-herve.codina@bootlin.com/
>
> There's probably enough quirks with interrupt-map that we can't use
> the same code. Though it may boil down to handling #address-cells and
> how the parent is looked up.

Yeah, as Herve pointed out[1], the interrupt-map has enough quirks that
a generic parser isn't going to work.

>> There are likely ways to extract some shared code between these two
>> functions into some shared helpers, but I'm hoping someone more
>> familiar with this OF code can help here.
>
> I would expect of_parse_phandle_with_args_map() could be implemented
> in terms of the iterator.

OK, let's first get the iterator implementation figured out, and then we
can see if that can be implemented by the iterator.

>> However, before refactoring the shared code, it would be good to have
>> some feedback on this approach.
>>
>> Signed-off-by: Kevin Hilman (TI.com) <khilman@baylibre.com>
>> ---
>>  drivers/of/base.c  | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of.h |  13 ++++
>>  2 files changed, 180 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 7043acd971a0..bdb4fde1bfa9 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1594,6 +1594,173 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
>>  }
>>  EXPORT_SYMBOL(of_parse_phandle_with_args_map);
>>
>> +/**
>> + * of_parse_map_iter() - Iterate through entries in a nexus node map
>> + * @np:                        pointer to a device tree node containing the map
>> + * @stem_name:         stem of property names (e.g., "power-domain" for "power-domain-map")
>> + * @index:             pointer to iteration index (set to 0 for first call)
>> + * @child_args:                pointer to structure to fill with child specifier (can be NULL)
>> + * @parent_args:       pointer to structure to fill with parent phandle and specifier
>> + *
>> + * This function iterates through a nexus node map property as defined in DT spec 2.5.1.
>> + * Each map entry has the format: <child_specifier phandle parent_specifier>
>> + *
>> + * On each call, it extracts one map entry and fills child_args (if provided) with the
>> + * child specifier and parent_args with the parent phandle and specifier.
>> + * The index pointer is updated to point to the next entry for the following call.
>> + *
>> + * Example usage::
>> + *
>> + *  int index = 0;
>> + *  struct of_phandle_args child_args, parent_args;
>> + *
>> + *  while (!of_parse_map_iter(np, "power-domain", &index, &child_args, &parent_args)) {
>> + *      // Process child_args and parent_args
>> + *      of_node_put(parent_args.np);
>> + *  }
>> + *
>> + * Caller is responsible for calling of_node_put() on parent_args.np.
>> + *
>> + * Return: 0 on success, -ENOENT when iteration is complete, or negative error code on failure.
>> + */
>> +int of_parse_map_iter(const struct device_node *np,
>> +                      const char *stem_name,
>> +                      int *index,
>> +                      struct of_phandle_args *child_args,
>> +                      struct of_phandle_args *parent_args)
>> +{
>> +       char *cells_name __free(kfree) = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
>> +       char *map_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map", stem_name);
>> +       char *mask_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
>> +       char *pass_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name);
>> +       static const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
>> +       static const __be32 dummy_pass[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(0) };
>> +       const __be32 *map, *mask, *pass;
>> +       __be32 child_spec[MAX_PHANDLE_ARGS];
>> +       u32 child_cells, parent_cells;
>> +       int map_len, i, entry_idx;
>> +
>> +       if (!np || !stem_name || !index || !parent_args)
>> +               return -EINVAL;
>> +
>> +       if (!cells_name || !map_name || !mask_name || !pass_name)
>> +               return -ENOMEM;
>> +
>> +       /* Get the map property */
>> +       map = of_get_property(np, map_name, &map_len);
>> +       if (!map)
>> +               return -ENOENT;
>> +
>> +       map_len /= sizeof(u32);
>> +
>> +       /* Get child #cells */
>> +       if (of_property_read_u32(np, cells_name, &child_cells))
>> +               return -EINVAL;
>> +
>> +       /* Get the mask property (optional) */
>> +       mask = of_get_property(np, mask_name, NULL);
>> +       if (!mask)
>> +               mask = dummy_mask;
>> +
>> +       /* Get the pass-thru property (optional) */
>> +       pass = of_get_property(np, pass_name, NULL);
>> +       if (!pass)
>> +               pass = dummy_pass;
>
> Generally the DT iterators need some state maintained, so there's an
> init function to do all/most of the above and stash that into a state
> struct for the iterator.

OK, I'm assuming you're referring to of_phandle_iterator_init()?

>> +
>> +       /* Iterate through map to find the entry at the requested index */
>> +       entry_idx = 0;
>> +       while (map_len > child_cells + 1) {
>> +               /* If this is the entry we're looking for, extract it */
>> +               if (entry_idx == *index) {
>> +                       /* Save masked child specifier for pass-thru processing */
>> +                       for (i = 0; i < child_cells && i < MAX_PHANDLE_ARGS; i++)
>> +                               child_spec[i] = map[i] & mask[i];
>> +
>> +                       /* Extract child specifier if requested */
>> +                       if (child_args) {
>> +                               child_args->np = (struct device_node *)np;
>> +                               child_args->args_count = child_cells;
>> +                               for (i = 0; i < child_cells && i < MAX_PHANDLE_ARGS; i++)
>> +                                       child_args->args[i] = be32_to_cpu(map[i]);
>> +                       }
>> +
>> +                       /* Move past child specifier */
>> +                       map += child_cells;
>> +                       map_len -= child_cells;
>> +
>> +                       /* Extract parent phandle */
>> +                       parent_args->np = of_find_node_by_phandle(be32_to_cpup(map));
>
> Before you update the parent node, you need to put the previous parent.

OK.

Kevin

[1] https://lore.kernel.org/all/20251125085521.451ea208@bootlin.com/