Re: [PATCH 1/5] clk: Add support for clock nexus dt bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete clock nexus node implementation breaks assigned clocks and parent resolution.
- [Low] Stale comment referencing replaced function in `of_parse_clkspec()`.
--

commit bfd03486778cd0e1335a28b465c6294bbcca9284
Author: Chancel Liu <chancel.liu@nxp.com>

clk: Add support for clock nexus dt bindings

This commit introduces support for clock nexus nodes by updating
of_parse_clkspec() to use of_parse_phandle_with_args_map(). This allows
remapping phandles and their arguments through nexus nodes to find the
actual clock provider.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 048adfa86a5d0..4240f059bec2b 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5199,15 +5199,15 @@ static int of_parse_clkspec(const struct device_node *np, int index,
>  	/* Walk up the tree of devices looking for a clock property that matches */
>  	while (np) {
>  		/*
>  		 * For named clocks, first look up the name in the
>  		 * "clock-names" property.  If it cannot be found, then index
>  		 * will be an error code and of_parse_phandle_with_args() will
>  		 * return -EINVAL.
>  		 */

This isn't a bug, but should the comment here be updated to refer to the new
of_parse_phandle_with_args_map() function variant?

>  		if (name)
>  			index = of_property_match_string(np, "clock-names", name);
> -		ret = of_parse_phandle_with_args(np, "clocks", "#clock-cells",
> -						 index, out_args);
> +		ret = of_parse_phandle_with_args_map(np, "clocks", "clock",
> +						     index, out_args);

If a device tree utilizes nexus nodes for assigned-clocks or
assigned-clock-parents, how do the core clock paths handle parent
resolution?

Looking at __set_clk_parents() and __set_clk_rates() in
drivers/clk/clk-conf.c, and of_clk_get_parent_name() in drivers/clk/clk.c,
these sites still appear to use the unmapped of_parse_phandle_with_args()
function.

When using nexus nodes for these properties, won't the specifier resolve to
the nexus node itself rather than the mapped clock provider?

Since the nexus node is not registered as a clock provider, it looks like
of_clk_get_from_provider() will return -EPROBE_DEFER, which could cause the
consumer device to silently defer probing forever.

Are there plans to update these core paths to use
of_parse_phandle_with_args_map() to fully support clock nexus nodes
system-wide without causing regressions for valid configurations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509024846.2094049-1-chancel.liu@nxp.com?part=1