Re: [PATCH RFC v2 2/2] pmdomain: core: use DT map to support hierarchy

[Kevin Hilman <khilman@baylibre.com>]

Hi Rob,

Rob Herring <robh@kernel.org> writes:

> On Wed, May 28, 2025 at 02:58:52PM -0700, Kevin Hilman wrote:
>> Currently, PM domains can only support hierarchy for simple
>> providers (e.g. ones with #power-domain-cells = 0).
>> 
>> Add support for oncell providers as well by adding support for a nexus
>> node map, as described in section 2.5.1 of the DT spec.
>> 
>> For example, an SCMI PM domain provider might be a subdomain of
>> multiple parent domains. In this example, the parent domains are
>> MAIN_PD and WKUP_PD:
>> 
>>     scmi_pds: protocol@11 {
>>         reg = <0x11>;
>>         #power-domain-cells = <1>;
>>         power-domain-map = <15 &MAIN_PD>,
>>                            <19 &WKUP_PD>;
>>     };
>> 
>> With the new map, child domain 15 (scmi_pds 15) becomes a
>> subdomain of MAIN_PD, and child domain 19 (scmi_pds 19) becomes a
>> subdomain of WKUP_PD.
>> 
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>

[...]

>> +/**
>> + * of_genpd_parse_domains_map() - Parse power-domains-map property for Nexus mapping
>> + * @np: Device node pointer associated with the PM domain provider.
>> + * @data: Pointer to the onecell data associated with the PM domain provider.
>> + *
>> + * Parse the power-domains-map property to establish parent-child relationships
>> + * for PM domains using Nexus node mapping as defined in the device tree
>> + * specification section v2.5.1.
>> + *
>> + * The power-domains-map property format is:
>> + * power-domains-map = <child_specifier target_phandle [target_specifier]>, ...;
>> + *
>> + * Where:
>> + * - child_specifier: The child domain ID that should be mapped
>> + * - target_phandle: Phandle to the parent PM domain provider
>> + * - target_specifier: Optional arguments for the parent provider (if it has #power-domain-cells > 0)
>> + *
>> + * Returns 0 on success, -ENOENT if property doesn't exist, or negative error code.
>> + */
>> +static int of_genpd_parse_domains_map(struct device_node *np,
>> +				      struct genpd_onecell_data *data)
>> +{
>> +	struct of_phandle_args parent_args;
>> +	struct generic_pm_domain *parent_genpd, *child_genpd;
>> +	u32 *map_entries;
>> +	int map_len, child_cells, i, ret;
>> +	u32 child_id;
>> +
>> +	/* Check if power-domains-map property exists */
>> +	map_len = of_property_count_u32_elems(np, "power-domains-map");
>> +	if (map_len <= 0)
>> +		return -ENOENT;
>
> Don't implement your own map parsing. Use or extend 
> of_parse_phandle_with_args_map().

So I've been wrestling with this for a bit, and I need some guidance.
TBH, these "nexus node maps" and of_parse_phandle_with_args_map() are
breaking my brain.

So, my node looks like this:

	scmi_pds: protocol@11 {
		reg = <0x11>;
		#power-domain-cells = <1>;
		bootph-all;

		power-domain-map = <15 &MAIN_PD>,
				   <19 &WKUP_PD>;
	};

my first attempt was to iterate over the child domains by calling:

  of_parse_phandle_with_args_map(np, "power-domains", "power-domain", i,  &mapped_args);  

but this doesn't find any entries because my node doesn't have the
"base" property.  So I gathered (perhaps mistakenly) that I was missing
something, so I added:

		power-domains = <&MAIN_PD>, <&WKUP_PD>;

to that node and try to iterate again. Now I got a match for i=0 (it
returns the node for MAIN_PD) and i=1 (it returns the node for WKUP_PD)

So I gather from that that the index arg to of_parse_phandle_with_args_map()
is the index into the -map array.  OK, fine.

So I know that the 0th entry in my -map points to &MAIN_PD, an dthe 1st
entry in the -map points to &WKUP_PD, but I don't see how to get the
child ID without (re)parsing the -map again myself because
of_parse_phandle_with_args_map() doesn't give me any information about
the child ID.

I can maybe see that in other usecases, the caller might not need the
child ID because it's being (re)mapped, but I need the child ID because
it's the pmdomain belonging to the child ID that I need to add as a
subdomain to the pmdomain of the parent.

However, thinking through this, I'm now realizing that maybe the problem
is that I cannot have a sparse -map table.  For this to work properly,
maybe my <15 &MAIN_PD> needs to be the 15th entry in the table (or
technically 16th if it's zero based)?

But before I go down any more rabbit holes, I wanted to check with folks
who understand this stuff and see if I'm on the right track or if I've
missed the boat on how to use of_parse_phandle_with_args_map().

Guidance, suggetions and/or public riducle are welcome. ;)

Kevin