Message-ID: <7hh5s85dbu.fsf@baylibre.com>
Date: Mon, 26 Jan 2026 12:27:49 -0800
From: Kevin Hilman <khilman@...libre.com>
To: Rob Herring <robh@...nel.org>, Herve Codina <herve.codina@...tlin.com>
Cc: Krzysztof Kozlowski <krzk+dt@...nel.org>, devicetree@...r.kernel.org,
 Ulf Hansson <ulf.hansson@...aro.org>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2] of: Add of_parse_map_iter() helper for nexus
 node map iteration

Rob Herring <robh@...nel.org> writes:

> On Wed, Jan 21, 2026 at 5:55 PM Kevin Hilman (TI) <khilman@...libre.com> wrote:
>>
>> Add a new helper function of_parse_map_iter() to simplify parsing of
>> nexus node maps as defined in the 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. Each map entry follows the format:
>>   <child_specifier phandle parent_specifier>
>>
>> The iterator extracts both the child specifier and parent phandle+args
>> for each entry, managing all the details of:
>> - Reading #<stem>-cells from both child and parent nodes
>> - Calculating variable entry sizes
>> - Resolving phandles
>> - Proper node reference management
>>
>> This eliminates the need for subsystems to manually parse map properties,
>> reducing code duplication and potential bugs.
>>
>> This code was developed in collaboration with Claude Code (model:
>> Sonnet 4.5), which needed some guidance to use existing OF helpers,
>> iterators etc.
>>
>> Signed-off-by: Kevin Hilman (TI) <khilman@...libre.com>
>> ---
>> Changes in v2:
>> - Use helpers of_phandle_iterator_init() and of_phandle_iterator_next()
>> - add missing of_node_put() pointed out in v1
>> - Link to v1: https://patch.msgid.link/20251119-topic-lpm-of-map-iterator-v6-18-v1-1-1f0075d771a3@baylibre.com
>> ---
>>  drivers/of/base.c  | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of.h |  13 +++++++++++++
>>  2 files changed, 176 insertions(+)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 0b65039ece53..8392fe54cf60 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1641,6 +1641,169 @@ 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);
>
> It's not great that we're doing allocs and frees on each iteration.
>
> Can't we follow the same design Herve did for interrupt-map? You have
> an init function you call once up front and then an iterator define
> (e.g. for_each_of_imap_item()). The complication in this case would be
> if we do the allocs in the init function, then we need a way to free
> them. If they are part of the for loop init, then we could use the
> scoped cleanup.
>
> On thing I noticed is I think of_phandle_iterator and of_imap_parser
> should probably be merged to one struct. They basically hold the same
> information (pointers to property data).
>
> Let me see if I can come up with something next week.

Thank you!

I would greatly appreciate some help getting this going in the right
direction as I'm definitely out of my comfort zone in this drivers/of
code.

Kevin

Powered by blists - more mailing lists