[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <7hjyzedgoc.fsf@baylibre.com>
Date: Mon, 24 Nov 2025 17:50:11 -0800
From: Kevin Hilman <khilman@...libre.com>
To: Rob Herring <robh@...nel.org>, Herve Codina <herve.codina@...tlin.com>
Cc: Ulf Hansson <ulf.hansson@...aro.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, devicetree@...r.kernel.org,
linux-pm@...r.kernel.org, arm-scmi@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] of: Add of_parse_map_iter() helper for nexus node
map iteration
Rob Herring <robh@...nel.org> writes:
> +Herve
>
> On Wed, Nov 19, 2025 at 6:41 PM Kevin Hilman (TI.com)
> <khilman@...libre.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.
Hmm, I wasn't aware of this, thanks for point it out. It looks very
similar to what i need, except for it's hard-coding the properties as
"#interrupt-*".
Seems like this should be generalized to handle the generic nexus-node
map. But it also seems to rely on an existing function
of_irq_parse_imap_parent() which is also specific to interrupt maps.
That being said, I'm not sure if interrupt-maps are really special, or
if they are just a specific case of the nexus node map. This drivers/of
code is breaking my brain, so it's more likely that I simply don't
understand enough of it to know how to do this correctly.
Any more detailed help/guidance for how to go forward here would be
greatly appreciated.
>> 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.
I'm not really sure how because the of_parse_phandle* stuff just has to
handle a single phandle, where what I need (and what the imap stuff is
doing) is iterating over the whole map.
>> 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@...libre.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.
Are you 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
Powered by blists - more mailing lists