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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Miquel Raynal (2026-01-29 12:10:03)
> From: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
> 
> A nexus node is some kind of parent device abstracting the outer
> connexions. They are particularly useful for describing connectors-like

connections?

> interfaces but not only. Certain IP blocks will typically include inner
> blocks and distribute resources to them.
> 
> In the case of clocks, there is already the concept of clock controller,
> but this usually indicates some kind of control over the said clock,
> ie. gate or rate control. When there is none of this, an existing
> approach is to reference the upper clock, which is wrong from a hardware
> point of view.
> 
> Nexus nodes are already part of the device-tree specification and clocks
> are already mentioned:
> https://github.com/devicetree-org/devicetree-specification/blob/v0.4/source/chapter2-devicetree-basics.rst#nexus-nodes-and-specifier-mapping
> 
> Following the introductions of nexus nodes support for interrupts, gpios
> and pwms, here is the same logic applied again to the clk subsystem,
> just by transitioning from of_parse_phandle_with_args() to
> of_parse_phandle_with_args_map():
> 
> * Nexus OF support:
> bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node")

Throwback!

> * GPIO adoption:
> c11e6f0f04db ("gpio: Support gpio nexus dt bindings")
> * PWM adoption:
> e71e46a6f19c ("pwm: Add support for pwm nexus dt bindings")
> 
> Expected Nexus properties supported:
> - clock-map: maps inner clocks to inlet clocks,
> - clock-map-mask: specifier cell(s) which will be remapped,
> - clock-map-pass-thru: specifier cell(s) not used for remapping,
>   forwarded as-is.
> 
> In my own usage I had to deal with controllers where clock-map-mask and
> clock-map-pass-thru were not relevant, but here is a made up example
> showing how all these properties could go together:
> 
> Example:
>     soc_clk: clock-controller {
>         #clock-cells = <2>;
>     };
> 
>     container: container {
>         #clock-cells = <2>;
>         clock-map = <0 0 &soc_clk 2 0>,
>                     <1 0 &soc_clk 6 0>;
>         clock-map-mask = <0xffffffff 0x0>;
>         clock-map-pass-thru = <0x0 0xffffffff>;
> 
>         child_device {

child-device

>             clocks = <&container 1 0>;
>             /* This is equivalent to <&soc_clk 6 0> */
>         };
>     };
> 
> The child device does not need to know about the outer implementation,
> and only knows about what the nexus provides. The nexus acts as a
> pass-through, with no extra control.
> 
> Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
> ---
>  drivers/clk/clk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 85d2f2481acf..bb52837a9f76 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5213,13 +5213,13 @@ static int of_parse_clkspec(const struct device_node *np, int index,
>                 /*
>                  * 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
> +                * will be an error code and of_parse_phandle_with_args_map() will
>                  * return -EINVAL.
>                  */
>                 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);

Looks simple enough. Please add KUnit tests for this though. Should be
doable with something like of_clk_get_hw() assuming it is exported as a
symbol.

>                 if (!ret)
>                         break;
>                 if (name && index >= 0)
> @@ -5286,7 +5286,7 @@ of_clk_get_hw_from_clkspec(struct of_phandle_args *clkspec)
>   *
>   * This function looks up a struct clk from the registered list of clock
>   * providers, an input is a clock specifier data structure as returned
> - * from the of_parse_phandle_with_args() function call.
> + * from the of_parse_phandle_with_args_map() function call.

The comment is bad if it needs to be updated for this. Can you rewrite
it to be more generic and not use specific function names?

>   */
>  struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
>  {
> @@ -5374,8 +5374,8 @@ const char *of_clk_get_parent_name(const struct device_node *np, int index)
>         int count;
>         struct clk *clk;
>  
> -       rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
> -                                       &clkspec);
> +       rc = of_parse_phandle_with_args_map(np, "clocks", "clock", index,
> +                                           &clkspec);

And this shows that we do this in two places. We probably want a better
wrapper here to parse a clk specifier. Are we missing parsing
"clock-ranges" for of_clk_get_parent_name()? Maybe this should just be
of_parse_clkspec() with a NULL name argument. Similarly we can get tests
for this function too.