[PATCH v2 0/7] clk: Add support for clock nexus

[PATCH v2 0/7] clk: Add support for clock nexus

[Miquel Raynal (Schneider Electric)]

Adding support for clock nexus had some prerequisites requested by
Stephen, such as adding new selftests, the initial discussions come from:

Link: https://lore.kernel.org/linux-clk/20260129201003.288605-1-miquel.raynal@bootlin.com/

Then there has been a bigger series (including patches for other
subsystems) to show the big picture, but let's split again and focus on
one subsystem at a time:

Changes in v2:
- Collected tags.
- Rebased on top of v7.2-rc1
- Exported (test) clock symbols using the _IF_KUNIT() variant, and
  imported the NS in the test file.
- Fixed a leaking node reported by Brian in the tests. Also used
  existing helpers to simplify the test implementation as he wisely
  suggested.
- Reworded the nexus node binding description as suggested by Stephen.
- Dropped redundant and non restraining properties from the Nexus
  node as suggested by Rob.
- Assigned clock/clock-rate properties now traverse Nexus nodes.
- Rename helpers following Stephen proposals.
- Rework tests following Stephen hints.
- Link to v1: https://lore.kernel.org/r/20260327-schneider-v7-0-rc1-crypto-v1-0-5e6ff7853994@bootlin.com

Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
Miquel Raynal (Schneider Electric) (7):
      dt-bindings: clock: Introduce nexus nodes
      clk: tests: Add clk_parse_clkspec() Kunit testing
      clk: tests: Add Kunit testing for of_clk_get_parent_name()
      clk: Improve a couple of comments
      clk: Use the generic OF phandle parsing in only one place
      clk: Add support for clock nexus dt bindings
      clk: tests: Add Kunit testing for nexus nodes

 .../bindings/clock/clock-nexus-node.yaml           |  30 +++++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-conf.c                             |  12 +-
 drivers/clk/clk.c                                  |  13 +-
 drivers/clk/clk_kunit_helpers.c                    |  31 +++++
 drivers/clk/clk_test.c                             | 146 +++++++++++++++++++++
 drivers/clk/kunit_clk_parse_clkspec.dtso           |  31 +++++
 include/kunit/clk.h                                |   2 +
 8 files changed, 254 insertions(+), 12 deletions(-)
---
base-commit: 7f26e010764df304602f33912a0550dcf46e72c2
change-id: 20260710-schneider-v7-2-rc1-eip201-upstream-a470e5eab92d

Best regards,
-- 
Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

[PATCH v2 1/7] dt-bindings: clock: Introduce nexus nodes

[Miquel Raynal (Schneider Electric)]

Hardware containers can just decouple external resources like clock
without any more control. Nexus nodes already exist for PWM and GPIOs,
add a binding to allow them for clocks as well.

No examples are given, the file is litteraly a copy-paste from Hervé
Codina's work on PWM Nexus nodes, hence we just point to the examples
there which already illustrate very clearly the concept of the various
properties.

Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
 .../bindings/clock/clock-nexus-node.yaml           | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
new file mode 100644
index 000000000000..ff3283bd5fb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/clock-nexus-node.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock Nexus
+
+description: >
+  A nexus node allows to remap a clock specifier list in a consumer node
+  through a container or a connector node in a generic way. With this
+  remapping, the consumer node needs to know only about the nexus
+  node. Resources behind the nexus node are decoupled by the nexus node
+  itself.
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+select: true
+
+properties:
+  clock-map:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+dependentRequired:
+  clock-map: ['#clock-cells']
+
+additionalProperties: true
+
+# See the original pwm-nexus-node.yaml description for examples

-- 
2.54.0

Re: [PATCH v2 1/7] dt-bindings: clock: Introduce nexus nodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The clock nexus node schema omits standard nexus map properties (`clock-map-mask` and `clock-map-pass-thru`) and `#clock-cells`.
--

--- Patch [1]: [PATCH v2 1/7] dt-bindings: clock: Introduce nexus nodes ---
commit 4092325fdc1397916665dc9c21bd4330df6026ce
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

dt-bindings: clock: Introduce nexus nodes

This commit introduces a new device tree binding for clock nexus nodes,
allowing hardware containers to remap clock specifier lists from consumer
nodes in a generic way. It applies the concept from existing PWM and GPIO
nexus nodes to clocks.

> diff --git a/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
> new file mode 100644
> index 0000000000000..ff3283bd5fb11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
[ ... ]
> +properties:
> +  clock-map:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix

[Severity: Medium]
Does this schema omit the definition for #clock-cells and the standard
nexus map properties like clock-map-mask and clock-map-pass-thru?

Without these properties defined in the properties block, there is no type
checking for clock-map-mask and clock-map-pass-thru. Furthermore, if a
consumer device tree node uses them and specifies unevaluatedProperties: false
in its own binding, could it spuriously fail schema validation during
dt_binding_check?

> +
> +dependentRequired:
> +  clock-map: ['#clock-cells']
> +
> +additionalProperties: true

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-schneider-v7-2-rc1-eip201-upstream-v2-0-b4680787377e@bootlin.com?part=1

[PATCH v2 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing

[Miquel Raynal (Schneider Electric)]

Create a new set of kunit tests to make sure clk_parse_clkspec() is
working as expected. We currently verify if we get a proper device when
using indexes and names. If we make an out of bounds request we expect
an error.

For testing purposes, we must ensure of_clk_get_hw()'s symbol is
exported.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
 drivers/clk/Makefile                     |   1 +
 drivers/clk/clk.c                        |   2 +
 drivers/clk/clk_test.c                   | 117 +++++++++++++++++++++++++++++++
 drivers/clk/kunit_clk_parse_clkspec.dtso |  21 ++++++
 4 files changed, 141 insertions(+)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index cc108a75a900..035b984db70d 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -19,6 +19,7 @@ clk-test-y			:= clk_test.o \
 				   kunit_clk_assigned_rates_zero.dtbo.o \
 				   kunit_clk_assigned_rates_zero_consumer.dtbo.o \
 				   kunit_clk_hw_get_dev_of_node.dtbo.o \
+				   kunit_clk_parse_clkspec.dtbo.o \
 				   kunit_clk_parent_data_test.dtbo.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 048adfa86a5d..08874cf9b561 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,6 +24,7 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/stringhash.h>
+#include <kunit/visibility.h>
 
 #include "clk.h"
 
@@ -5300,6 +5301,7 @@ struct clk_hw *of_clk_get_hw(struct device_node *np, int index,
 
 	return hw;
 }
+EXPORT_SYMBOL_IF_KUNIT(of_clk_get_hw);
 
 static struct clk *__of_clk_get(struct device_node *np,
 				int index, const char *dev_id,
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index b1961daac5e2..fddb2bb9618f 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3541,10 +3541,126 @@ static struct kunit_suite clk_hw_get_dev_of_node_test_suite = {
 	.test_cases = clk_hw_get_dev_of_node_test_cases,
 };
 
+static const struct clk_init_data clk_parse_clkspec_1_init_data = {
+	.name = "clk_parse_clkspec_1",
+	.ops = &empty_clk_ops,
+};
+
+static const struct clk_init_data clk_parse_clkspec_2_init_data = {
+	.name = "clk_parse_clkspec_2",
+	.ops = &empty_clk_ops,
+};
+
+struct clk_parse_clkspec_ctx {
+	struct device_node *cons_np;
+};
+
+static int clk_parse_clkspec_init(struct kunit *test)
+{
+	struct device_node *prov1_np, *prov2_np;
+	struct clk_parse_clkspec_ctx *ctx;
+	struct clk_hw *hw1, *hw2;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+	test->priv = ctx;
+
+	KUNIT_ASSERT_EQ(test, 0, of_overlay_apply_kunit(test, kunit_clk_parse_clkspec));
+
+	/* Register provider 1 */
+	hw1 = kunit_kzalloc(test, sizeof(*hw1), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw1);
+	hw1->init = &clk_parse_clkspec_1_init_data;
+
+	prov1_np = of_find_compatible_node(NULL, NULL, "test,clock-provider1");
+	KUNIT_ASSERT_NOT_NULL(test, prov1_np);
+
+	KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov1_np, hw1));
+	of_clk_add_hw_provider_kunit(test, prov1_np, of_clk_hw_simple_get, hw1);
+	of_node_put(prov1_np);
+
+	/* Register provider 2 */
+	hw2 = kunit_kzalloc(test, sizeof(*hw2), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw2);
+	hw2->init = &clk_parse_clkspec_2_init_data;
+
+	prov2_np = of_find_compatible_node(NULL, NULL, "test,clock-provider2");
+	KUNIT_ASSERT_NOT_NULL(test, prov2_np);
+
+	KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov2_np, hw2));
+	of_clk_add_hw_provider_kunit(test, prov2_np, of_clk_hw_simple_get, hw2);
+	of_node_put(prov2_np);
+
+	ctx->cons_np = of_find_compatible_node(NULL, NULL, "test,clock-consumer");
+	KUNIT_ASSERT_NOT_NULL(test, ctx->cons_np);
+
+	return 0;
+}
+
+static void clk_parse_clkspec_exit(struct kunit *test)
+{
+	struct clk_parse_clkspec_ctx *ctx = test->priv;
+
+	of_node_put(ctx->cons_np);
+}
+
+/* Test DT phandle lookups using correct index or name succeed */
+static void clk_parse_clkspec_with_correct_index_and_name(struct kunit *test)
+{
+	struct clk_parse_clkspec_ctx *ctx = test->priv;
+	struct clk_hw *hw1, *hw2, *hw3, *hw4;
+
+	/* Get clocks by index */
+	hw1 = of_clk_get_hw(ctx->cons_np, 0, NULL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw1);
+
+	hw2 = of_clk_get_hw(ctx->cons_np, 1, NULL);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw2);
+	KUNIT_EXPECT_PTR_NE(test, hw1, hw2);
+
+	/* Get clocks by name */
+	hw3 = of_clk_get_hw(ctx->cons_np, 0, "first_clock");
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw3);
+	KUNIT_EXPECT_PTR_EQ(test, hw1, hw3);
+
+	hw4 = of_clk_get_hw(ctx->cons_np, 0, "second_clock");
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw4);
+	KUNIT_EXPECT_PTR_EQ(test, hw2, hw4);
+}
+
+/* Test DT phandle lookups using wrong index or name fail */
+static void clk_parse_clkspec_with_incorrect_index_and_name(struct kunit *test)
+{
+	struct clk_parse_clkspec_ctx *ctx = test->priv;
+	struct clk_hw *hw;
+
+	/* Get clock by index */
+	hw = of_clk_get_hw(ctx->cons_np, 2, NULL);
+	KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
+
+	/* Get clock by name */
+	hw = of_clk_get_hw(ctx->cons_np, 0, "third_clock");
+	KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
+}
+
+static struct kunit_case clk_parse_clkspec_test_cases[] = {
+	KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
+	KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
+	{}
+};
+
+/* Test suite to verify clk_parse_clkspec() */
+static struct kunit_suite clk_parse_clkspec_test_suite = {
+	.name = "clk_parse_clkspec",
+	.init = clk_parse_clkspec_init,
+	.exit = clk_parse_clkspec_exit,
+	.test_cases = clk_parse_clkspec_test_cases,
+};
 
 kunit_test_suites(
 	&clk_assigned_rates_suite,
 	&clk_hw_get_dev_of_node_test_suite,
+	&clk_parse_clkspec_test_suite,
 	&clk_leaf_mux_set_rate_parent_test_suite,
 	&clk_test_suite,
 	&clk_multiple_parents_mux_test_suite,
@@ -3562,4 +3678,5 @@ kunit_test_suites(
 	&clk_uncached_test_suite,
 );
 MODULE_DESCRIPTION("Kunit tests for clk framework");
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/kunit_clk_parse_clkspec.dtso b/drivers/clk/kunit_clk_parse_clkspec.dtso
new file mode 100644
index 000000000000..c93feb93e101
--- /dev/null
+++ b/drivers/clk/kunit_clk_parse_clkspec.dtso
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	kunit_clock_provider1: kunit-clock-provider1 {
+		compatible = "test,clock-provider1";
+		#clock-cells = <1>;
+	};
+
+	kunit_clock_provider2: kunit-clock-provider2 {
+		compatible = "test,clock-provider2";
+		#clock-cells = <1>;
+	};
+
+	kunit-clock-consumer {
+		compatible = "test,clock-consumer";
+		clocks = <&kunit_clock_provider1 0>, <&kunit_clock_provider2 0>;
+		clock-names = "first_clock", "second_clock";
+	};
+};

-- 
2.54.0

Re: [PATCH v2 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Ignored return value of `of_clk_add_hw_provider_kunit()`.
--

--- Patch [2]: [PATCH v2 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing ---
commit 4b569be51db687b32469c5799d300307be42e0b7
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

clk: tests: Add clk_parse_clkspec() Kunit testing

This commit introduces a new set of KUnit tests to verify that
clk_parse_clkspec() functions properly. It validates whether valid lookups
by index and name return the correct devices, and checks that out-of-bounds
requests properly result in an error.

> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index b1961daac5e22..fddb2bb9618fd 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
[ ... ]
> @@ -3541,10 +3541,126 @@ static struct kunit_suite clk_hw_get_dev_of_node_test_suite = {
[ ... ]
> +static int clk_parse_clkspec_init(struct kunit *test)
> +{
> +	struct device_node *prov1_np, *prov2_np;
> +	struct clk_parse_clkspec_ctx *ctx;
> +	struct clk_hw *hw1, *hw2;
> +
> +	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +	test->priv = ctx;
> +
> +	KUNIT_ASSERT_EQ(test, 0, of_overlay_apply_kunit(test, kunit_clk_parse_clkspec));
> +
> +	/* Register provider 1 */
> +	hw1 = kunit_kzalloc(test, sizeof(*hw1), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw1);
> +	hw1->init = &clk_parse_clkspec_1_init_data;
> +
> +	prov1_np = of_find_compatible_node(NULL, NULL, "test,clock-provider1");
> +	KUNIT_ASSERT_NOT_NULL(test, prov1_np);
> +
> +	KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov1_np, hw1));
> +	of_clk_add_hw_provider_kunit(test, prov1_np, of_clk_hw_simple_get, hw1);

[Severity: Medium]
Should the return value of of_clk_add_hw_provider_kunit() be verified here?

If the provider fails to register during setup, test initialization will 
continue as if it succeeded. This could mask the actual setup failure and
cause downstream tests to fail unpredictably. Adding a KUNIT_ASSERT_EQ check
similar to the of_clk_hw_register_kunit() call right above it might prevent 
this.

> +	of_node_put(prov1_np);
> +
> +	/* Register provider 2 */
> +	hw2 = kunit_kzalloc(test, sizeof(*hw2), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw2);
> +	hw2->init = &clk_parse_clkspec_2_init_data;
> +
> +	prov2_np = of_find_compatible_node(NULL, NULL, "test,clock-provider2");
> +	KUNIT_ASSERT_NOT_NULL(test, prov2_np);
> +
> +	KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov2_np, hw2));
> +	of_clk_add_hw_provider_kunit(test, prov2_np, of_clk_hw_simple_get, hw2);

[Severity: Medium]
Is it also necessary to check the return value of this second call to
of_clk_add_hw_provider_kunit() for the same reason?

> +	of_node_put(prov2_np);
> +
> +	ctx->cons_np = of_find_compatible_node(NULL, NULL, "test,clock-consumer");
> +	KUNIT_ASSERT_NOT_NULL(test, ctx->cons_np);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-schneider-v7-2-rc1-eip201-upstream-v2-0-b4680787377e@bootlin.com?part=2

[PATCH v2 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name()

[Miquel Raynal (Schneider Electric)]

Make sure this helper is never broken, especially since we will soon
make some changes in it.

Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
 drivers/clk/clk_test.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index fddb2bb9618f..5f3ffda033c3 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3643,9 +3643,23 @@ static void clk_parse_clkspec_with_incorrect_index_and_name(struct kunit *test)
 	KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
 }
 
+/*
+ * Verify that of_clk_get_parent_name() returns the correct clock name when
+ * looking up by index through the consumer's clocks property.
+ */
+static void of_clk_get_parent_name_gets_parent_name(struct kunit *test)
+{
+	struct clk_parse_clkspec_ctx *ctx = test->priv;
+	const char *expected_name = "clk_parse_clkspec_1";
+
+	KUNIT_EXPECT_STREQ(test, expected_name,
+			   of_clk_get_parent_name(ctx->cons_np, 0));
+}
+
 static struct kunit_case clk_parse_clkspec_test_cases[] = {
 	KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
 	KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
+	KUNIT_CASE(of_clk_get_parent_name_gets_parent_name),
 	{}
 };
 

-- 
2.54.0

[PATCH v2 4/7] clk: Improve a couple of comments

[Miquel Raynal (Schneider Electric)]

Avoid mentioning the function names directly in the comments, it makes
them easily out of sync with the rest of the code. Use a more generic
wording.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/clk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 08874cf9b561..7d63d81ebc09 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5202,7 +5202,7 @@ 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 the OF phandle parser will
 		 * return -EINVAL.
 		 */
 		if (name)
@@ -5275,7 +5275,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 phandle parser.
  */
 struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
 {

-- 
2.54.0

[PATCH v2 5/7] clk: Use the generic OF phandle parsing in only one place

[Miquel Raynal (Schneider Electric)]

There should be one single entry in the OF world, so that the way we
parse the DT is always the same. make sure this is the case by avoid
calling of_parse_phandle_with_args() from of_clk_get_parent_name(). This
is even more relevant as we currently fail to parse clock-ranges. As a
result, it seems to be safer to directly call of_parse_clkspec() there.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Fixes: 4472287a3b2f5 ("clk: Introduce of_clk_get_hw_from_clkspec()")
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/clk.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7d63d81ebc09..dad4bac10ad4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5364,8 +5364,7 @@ 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_clkspec(np, index, NULL, &clkspec);
 	if (rc)
 		return NULL;
 

-- 
2.54.0

Re: [PATCH v2 5/7] clk: Use the generic OF phandle parsing in only one place

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Switching to `of_parse_clkspec` breaks out-of-bounds indexed clock lookups on nodes with `clock-ranges`, causing infinite loops and topology corruption.
- [High] API mismatch between `of_clk_get_parent_name` and `of_clk_get_parent_count` regarding `clock-ranges` support.
--

--- Patch [5]: [PATCH v2 5/7] clk: Use the generic OF phandle parsing in only one place ---
commit bc84db19056243e06a72beb625e47749500a0824
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

clk: Use the generic OF phandle parsing in only one place

This patch refactors of_clk_get_parent_name() to use of_parse_clkspec()
instead of of_parse_phandle_with_args(). The stated goal is to consolidate
Device Tree parsing and ensure consistent handling, particularly to support
the parsing of the clock-ranges property.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 7d63d81ebc096..dad4bac10ad45 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5364,8 +5364,7 @@ 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_clkspec(np, index, NULL, &clkspec);

[Severity: High]
Does this change break out-of-bounds indexed clock lookups on nodes with
clock-ranges?

When falling back to the parent node, of_parse_clkspec() unconditionally
resets the index:

drivers/clk/clk.c:of_parse_clkspec() {
	...
	np = np->parent;
	if (np && !of_property_present(np, "clock-ranges"))
		break;
	index = 0;
	...
}

Iterators like of_clk_parent_fill() rely on of_clk_get_parent_name()
returning NULL to terminate when they exceed the available clock indices:

drivers/clk/clk.c:of_clk_parent_fill() {
	...
	while (i < size && (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
		i++;
	...
}

Because the index is reset to 0 upon fallback, when the iterator exceeds
the child's valid clock indices, won't this successfully return the
parent's 0-th clock instead of failing?

If so, this could fill the array with duplicate copies of the parent's
0-th clock and corrupt the clock tree topology.

[Severity: High]
Does this create an API mismatch with of_clk_get_parent_count() regarding
clock-ranges support?

of_clk_get_parent_count() strictly counts the elements in the immediate
child node's clocks property and returns 0 if absent:

drivers/clk/clk.c:of_clk_get_parent_count() {
	...
	count = of_count_phandle_with_args(np, "clocks", "#clock-cells");
	...
}

Since of_clk_get_parent_name() now falls back to the parent node via
clock-ranges, what happens if a node relies entirely on clock-ranges to
inherit clocks?

If drivers dynamically allocate parent arrays based on the count, won't
they size them to 0 and skip clock initialization entirely, rendering
clock-ranges inheritance ineffective?

>  	if (rc)
>  		return NULL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-schneider-v7-2-rc1-eip201-upstream-v2-0-b4680787377e@bootlin.com?part=5

[PATCH v2 6/7] clk: Add support for clock nexus dt bindings

[Miquel Raynal (Schneider Electric)]

A nexus node is some kind of parent device abstracting the outer
connections. They are particularly useful for describing connectors-like
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:
commit bd6f2fd5a1d5 ("of: Support parsing phandle argument lists through a nexus node")
* GPIO adoption:
commit c11e6f0f04db ("gpio: Support gpio nexus dt bindings")
* PWM adoption:
commit e71e46a6f19c ("pwm: Add support for pwm nexus dt bindings")

Only expected Nexus property:
- clock-map: maps inner clocks to inlet clocks
(the other properties have been judged not relevant for clocks)

Here is an example:

Example:
    soc_clk: clock-controller {
        #clock-cells = <1>;
    };

    container: container {
        #clock-cells = <1>;
        clock-map = <0 &soc_clk 2>,
                    <1 &soc_clk 6>;

        child-device {
            clocks = <&container 1>;
	    /* This is equivalent to <&soc_clk 6> */
        };
    };

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>
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/clk-conf.c | 12 ++++++------
 drivers/clk/clk.c      |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 303a0bb26e54..5380d43b56a4 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -25,8 +25,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
 		       node);
 
 	for (index = 0; index < num_parents; index++) {
-		rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
-					"#clock-cells",	index, &clkspec);
+		rc = of_parse_phandle_with_args_map(node, "assigned-clock-parents",
+						    "clock", index, &clkspec);
 		if (rc < 0) {
 			/* skip empty (null) phandles */
 			if (rc == -ENOENT)
@@ -47,8 +47,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
 			return PTR_ERR(pclk);
 		}
 
-		rc = of_parse_phandle_with_args(node, "assigned-clocks",
-					"#clock-cells", index, &clkspec);
+		rc = of_parse_phandle_with_args_map(node, "assigned-clocks",
+						    "clock", index, &clkspec);
 		if (rc < 0)
 			goto err;
 		if (clkspec.np == node && !clk_supplier) {
@@ -121,8 +121,8 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
 			rate = rates[index];
 
 		if (rate) {
-			rc = of_parse_phandle_with_args(node, "assigned-clocks",
-					"#clock-cells",	index, &clkspec);
+			rc = of_parse_phandle_with_args_map(node, "assigned-clocks",
+							    "clock", index, &clkspec);
 			if (rc < 0) {
 				/* skip empty (null) phandles */
 				if (rc == -ENOENT)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index dad4bac10ad4..73ddd9a6aa2e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5207,8 +5207,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
 		 */
 		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 (!ret)
 			break;
 		if (name && index >= 0)

-- 
2.54.0

Re: [PATCH v2 6/7] clk: Add support for clock nexus dt bindings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Stack buffer overflow in `of_parse_phandle_with_args_map` when `#<list>-cells` exceeds `MAX_PHANDLE_ARGS`.
- [High] Infinite loop in `of_parse_phandle_with_args_map` due to lack of cycle detection.
- [High] Unconditional dereference of optional `out_args` pointer in `of_parse_phandle_with_args_map`.
--

--- Patch [6]: [PATCH v2 6/7] clk: Add support for clock nexus dt bindings ---
commit 2eb2cbfe08397c2ed1b0b80363cc91255e359912
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

clk: Add support for clock nexus dt bindings

This commit introduces support for nexus nodes in the clock subsystem. It
maps inner clocks to inlet clocks by transitioning from
of_parse_phandle_with_args() to of_parse_phandle_with_args_map().

> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a..5380d43b56a42 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -25,8 +25,8 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
>  		       node);
>  
>  	for (index = 0; index < num_parents; index++) {
> -		rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
> -					"#clock-cells",	index, &clkspec);
> +		rc = of_parse_phandle_with_args_map(node, "assigned-clock-parents",
> +						    "clock", index, &clkspec);

[Severity: Critical]
This is a pre-existing issue, but by adopting of_parse_phandle_with_args_map(),
does this expose the clock subsystem to a stack buffer overflow?

When a device tree node specifies a cell count greater than MAX_PHANDLE_ARGS,
__of_parse_phandle_with_args() correctly clamps the count.

However, of_parse_phandle_with_args_map() in drivers/of/base.c re-reads the
unclamped cell count into list_size:

drivers/of/base.c:of_parse_phandle_with_args_map() {
    ...
    ret = of_property_read_u32(cur, cells_name, &list_size);
    if (ret < 0)
        goto put;

    /* Precalculate the match array - this simplifies match loop */
    for (i = 0; i < list_size; i++)
        initial_match_array[i] = cpu_to_be32(out_args->args[i]);
    ...
}

Could this overflow initial_match_array[] if list_size exceeds
MAX_PHANDLE_ARGS?

[Severity: High]
This is a pre-existing issue, but does switching to this API also expose the
subsystem to an infinite loop if a device tree overlay introduces a cycle?

In drivers/of/base.c:of_parse_phandle_with_args_map(), the map resolution loop
continuously transitions to the next node:

drivers/of/base.c:of_parse_phandle_with_args_map() {
    ...
    while (cur) {
        /* Get the <list>-map property */
        map = of_get_property(cur, map_name, &map_len);
    ...
}

Does this while loop lack cycle detection or a maximum depth bound, potentially
allowing malformed input to hang the CPU?

[Severity: High]
This is a pre-existing issue, but does of_parse_phandle_with_args_map()
unconditionally dereference out_args?

While this patch passes a valid pointer, the API is documented as accepting
an optional out_args pointer. If a future caller passes NULL to validate a
mapping, it appears a NULL pointer dereference will occur:

drivers/of/base.c:of_parse_phandle_with_args_map() {
    ...
    /* Get the #<list>-cells property */
    cur = out_args->np;
    ...
}

Can this cause a panic if out_args is NULL?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-schneider-v7-2-rc1-eip201-upstream-v2-0-b4680787377e@bootlin.com?part=6

[PATCH v2 7/7] clk: tests: Add Kunit testing for nexus nodes

[Miquel Raynal (Schneider Electric)]

Add a nexus node with a child requesting a mapped clock in the fake DT
overlay to verify that the parsing is also correctly working.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
 drivers/clk/clk_kunit_helpers.c          | 31 +++++++++++++++++++++++++++++++
 drivers/clk/clk_test.c                   | 15 +++++++++++++++
 drivers/clk/kunit_clk_parse_clkspec.dtso | 10 ++++++++++
 include/kunit/clk.h                      |  2 ++
 4 files changed, 58 insertions(+)

diff --git a/drivers/clk/clk_kunit_helpers.c b/drivers/clk/clk_kunit_helpers.c
index 68a28e70bb61..151f62ab57c1 100644
--- a/drivers/clk/clk_kunit_helpers.c
+++ b/drivers/clk/clk_kunit_helpers.c
@@ -233,5 +233,36 @@ int of_clk_add_hw_provider_kunit(struct kunit *test, struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(of_clk_add_hw_provider_kunit);
 
+KUNIT_DEFINE_ACTION_WRAPPER(of_node_put_wrapper, of_node_put, struct device_node *);
+
+/**
+ * of_find_node_by_name_kunit() - Test managed of_find_node_by_name()
+ * @test: The test context
+ * @from: Parent device node to start searching from, or NULL to search from root
+ * @name: The name string to match against
+ *
+ * Just like of_find_node_by_name(), except the device_noded is managed by
+ * the test case and is automatically put after the test case concludes.
+ *
+ * Return: the device_node on success or a negative errno value on failure.
+ */
+struct device_node *of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
+					       const char *name)
+{
+	struct device_node *np;
+	int ret;
+
+	np = of_find_node_by_name(from, name);
+	if (!np)
+		return NULL;
+
+	ret = kunit_add_action_or_reset(test, of_node_put_wrapper, np);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return np;
+}
+EXPORT_SYMBOL_GPL(of_find_node_by_name_kunit);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("KUnit helpers for clk providers and consumers");
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 5f3ffda033c3..bebfd928ab36 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3656,10 +3656,25 @@ static void of_clk_get_parent_name_gets_parent_name(struct kunit *test)
 			   of_clk_get_parent_name(ctx->cons_np, 0));
 }
 
+static void of_clk_get_hw_maps_thru_nexus(struct kunit *test)
+{
+	struct clk_parse_clkspec_ctx *ctx = test->priv;
+	struct clk_hw *expected;
+	struct device_node *np;
+
+	np = of_find_node_by_name_kunit(test, NULL, "kunit-clock-nexus-child");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np);
+	expected = of_clk_get_hw(ctx->cons_np, 1, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, expected);
+
+	KUNIT_EXPECT_PTR_EQ(test, expected, of_clk_get_hw(np, 0, NULL));
+}
+
 static struct kunit_case clk_parse_clkspec_test_cases[] = {
 	KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
 	KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
 	KUNIT_CASE(of_clk_get_parent_name_gets_parent_name),
+	KUNIT_CASE(of_clk_get_hw_maps_thru_nexus),
 	{}
 };
 
diff --git a/drivers/clk/kunit_clk_parse_clkspec.dtso b/drivers/clk/kunit_clk_parse_clkspec.dtso
index c93feb93e101..a4115216d2aa 100644
--- a/drivers/clk/kunit_clk_parse_clkspec.dtso
+++ b/drivers/clk/kunit_clk_parse_clkspec.dtso
@@ -18,4 +18,14 @@ kunit-clock-consumer {
 		clocks = <&kunit_clock_provider1 0>, <&kunit_clock_provider2 0>;
 		clock-names = "first_clock", "second_clock";
 	};
+
+	kunit_clock_nexus: kunit-clock-nexus {
+		clocks = <&kunit_clock_provider2 0>;
+		clock-map = <&kunit_clock_provider2 0>;
+		#clock-cells = <0>;
+
+		kunit-clock-nexus-child {
+			clocks = <&kunit_clock_nexus>;
+		};
+	};
 };
diff --git a/include/kunit/clk.h b/include/kunit/clk.h
index f226044cc78d..ab24f7747516 100644
--- a/include/kunit/clk.h
+++ b/include/kunit/clk.h
@@ -29,5 +29,7 @@ int of_clk_hw_register_kunit(struct kunit *test, struct device_node *node,
 int of_clk_add_hw_provider_kunit(struct kunit *test, struct device_node *np,
 				 struct clk_hw *(*get)(struct of_phandle_args *clkspec, void *data),
 				 void *data);
+struct device_node *of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
+					       const char *name);
 
 #endif

-- 
2.54.0

Re: [PATCH v2 0/7] clk: Add support for clock nexus

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

Hi Miquel,

> Then there has been a bigger series (including patches for other
> subsystems) to show the big picture, but let's split again and focus on
> one subsystem at a time:

This is a good idea IMO. Sadly, for this series, I won't have time for
review. If you have a branch available for testing, I could provide
test-tags, though.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]