* [PATCH v5 0/7] clk: Add support for clock nexus
@ 2026-08-07 15:24 Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Frank Li, Herve Codina
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/
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
---
Changes in v5:
- Collected tags.
- Added the of_ prefix on the new kunit helper.
- Reworded a commit message.
- All remaining points raised by Sashiko do not seem relevant.
- Link to v4: https://lore.kernel.org/r/20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com
Changes in v4:
- Fixed the kernel doc of of_find_node_by_name_kunit() which was lacking
the fact that it could return NULL upon lookup failure. Also improved
the commit message by mentioning the addition of that helper.
- All other Sashiko reports have either been disregarded because they
were way too unrelated or just not relevant enough in my opinion.
- Link to v3: https://lore.kernel.org/r/20260716-schneider-v7-2-rc1-eip201-upstream-v3-0-98454e83845c@bootlin.com
Changes in v3:
- Prevent out-of-bounds clock-ranges fallback in of_parse_clkspec()
- tests: Check of_clk_add_hw_provider_kunit() return value and fix node
ref leaks
- Link to v2: https://lore.kernel.org/r/20260710-schneider-v7-2-rc1-eip201-upstream-v2-0-b4680787377e@bootlin.com
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
---
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 | 15 ++-
drivers/clk/clk_kunit_helpers.c | 31 +++++
drivers/clk/clk_test.c | 139 +++++++++++++++++++++
drivers/clk/kunit_clk_parse_clkspec.dtso | 31 +++++
include/kunit/clk.h | 2 +
8 files changed, 249 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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:33 ` sashiko-bot
2026-08-07 15:24 ` [PATCH v5 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing Miquel Raynal (Schneider Electric)
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Frank Li
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>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name() Miquel Raynal (Schneider Electric)
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Frank Li
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>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
drivers/clk/Makefile | 1 +
drivers/clk/clk.c | 2 +
drivers/clk/clk_test.c | 110 +++++++++++++++++++++++++++++++
drivers/clk/kunit_clk_parse_clkspec.dtso | 21 ++++++
4 files changed, 134 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..f47f81b7d72a 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3541,10 +3541,119 @@ 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);
+ of_node_put_kunit(test, prov1_np);
+
+ KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov1_np, hw1));
+ KUNIT_ASSERT_EQ(test, 0, of_clk_add_hw_provider_kunit(test, prov1_np, of_clk_hw_simple_get, hw1));
+
+ /* 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);
+ of_node_put_kunit(test, prov2_np);
+
+ KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, prov2_np, hw2));
+ KUNIT_ASSERT_EQ(test, 0, of_clk_add_hw_provider_kunit(test, prov2_np, of_clk_hw_simple_get, hw2));
+
+ ctx->cons_np = of_find_compatible_node(NULL, NULL, "test,clock-consumer");
+ KUNIT_ASSERT_NOT_NULL(test, ctx->cons_np);
+ of_node_put_kunit(test, ctx->cons_np);
+
+ return 0;
+}
+
+/* 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,
+ .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 +3671,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name()
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 4/7] clk: Improve a couple of comments Miquel Raynal (Schneider Electric)
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Frank Li
Ensure this helper remains correct before making upcoming changes to it.
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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 f47f81b7d72a..4084559e157d 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3637,9 +3637,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 4/7] clk: Improve a couple of comments
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
` (2 preceding siblings ...)
2026-08-07 15:24 ` [PATCH v5 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name() Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 16:23 ` Frank Li
2026-08-07 15:24 ` [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
` (3 preceding siblings ...)
2026-08-07 15:24 ` [PATCH v5 4/7] clk: Improve a couple of comments Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:41 ` sashiko-bot
2026-08-07 16:26 ` Frank Li
2026-08-07 15:24 ` [PATCH v5 6/7] clk: Add support for clock nexus dt bindings Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes Miquel Raynal (Schneider Electric)
6 siblings, 2 replies; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric)
Ideally, there should be one single entry in the OF world. This way, the
DT parsing logic is always identical. At this stage, we call
of_parse_phandle_with_args() in:
- of_parse_clkspec()
- of_clk_get_parent_name()
which kinds of defeats the above statement. Furthermore, when this call
is done from of_clk_get_parent_name(), we fail to parse the
'clock-ranges' property.
Make sure DT world is entered only once by calling of_parse_clkspec()
from of_clk_get_parent_name(). Doing so implies that we do not try the
"clock-ranges" path if we already found a "clocks" property.
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 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7d63d81ebc09..45f5d7a4ccc1 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -5219,6 +5219,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
* has a "clock-ranges" property, then we can try one of its
* clocks.
*/
+ if (of_property_present(np, "clocks"))
+ break;
np = np->parent;
if (np && !of_property_present(np, "clock-ranges"))
break;
@@ -5364,8 +5366,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 6/7] clk: Add support for clock nexus dt bindings
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
` (4 preceding siblings ...)
2026-08-07 15:24 ` [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:35 ` sashiko-bot
2026-08-07 15:24 ` [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes Miquel Raynal (Schneider Electric)
6 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Herve Codina, Frank Li
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>
Reviewed-by: Frank Li <Frank.Li@nxp.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 45f5d7a4ccc1..6acf042673c3 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
` (5 preceding siblings ...)
2026-08-07 15:24 ` [PATCH v5 6/7] clk: Add support for clock nexus dt bindings Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:24 ` Miquel Raynal (Schneider Electric)
2026-08-07 15:33 ` sashiko-bot
6 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal (Schneider Electric) @ 2026-08-07 15:24 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar
Cc: Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang, Fran Li,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev,
Miquel Raynal (Schneider Electric), Frank Li
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.
Create an of_find_node_by_name() like kunit helper to garbage collect the
node automatically in case of failed assertion.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Reviewed-by: Frank Li <Frank.Li@nxp.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..ab6b2cffcad6 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 *);
+
+/**
+ * clk_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_node is managed by
+ * the test case and is automatically put after the test case concludes.
+ *
+ * Return: the device_node on success, NULL if not found, or an error pointer on failure.
+ */
+struct device_node *clk_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(clk_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 4084559e157d..37e4137d37c9 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3650,10 +3650,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 = clk_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..0b5ab8e971c9 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 *clk_of_find_node_by_name_kunit(struct kunit *test, struct device_node *from,
+ const char *name);
#endif
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes
2026-08-07 15:24 ` [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:33 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-07 15:33 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric); +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] The clock-nexus-node schema omits definitions for standard nexus node properties `clock-map-mask`, `clock-map-pass-thru`, and `#clock-cells`, breaking schema validation.
Pre-existing issues:
- [High] The Linux clock subsystem lacks support for nexus node parsing, rendering this device tree binding unusable for Linux drivers.
--
--- Patch [1]: [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes ---
commit 0bad30dd59cfc8ff93ceabdefc7264eea3d9d045
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
dt-bindings: clock: Introduce nexus nodes
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.
> 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
[ ... ]
> +select: true
> +
> +properties:
> + clock-map:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
[Severity: High]
This isn't a bug introduced by this patch, but does the core clock framework
actually support nexus node parsing yet?
Looking at drivers/clk/clk.c:of_parse_clkspec():
ret = of_parse_phandle_with_args(np, "clocks", "#clock-cells",
index, out_args);
It appears the core clock framework uses of_parse_phandle_with_args() instead
of of_parse_phandle_with_args_map().
If a device tree author configures a clock-map and a consumer driver attempts
to request the mapped clock, wouldn't the clock subsystem incorrectly return
the nexus node itself as the provider instead of mapping to the underlying
clock provider?
> +
> +dependentRequired:
> + clock-map: ['#clock-cells']
[Severity: Medium]
Are we missing definitions for standard nexus node properties here?
The properties block omits clock-map-mask, clock-map-pass-thru, and
#clock-cells. Since additionalProperties is true, any device tree nodes using
clock-map-mask or clock-map-pass-thru will bypass type validation.
Should #clock-cells be included and constrained in the properties block as
dictated by subsystem guidelines for providers?
> +
> +additionalProperties: true
> +
> +# See the original pwm-nexus-node.yaml description for examples
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes
2026-08-07 15:24 ` [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:33 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-07 15:33 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric); +Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `clk_of_find_node_by_name_kunit` test helper introduces a double-free vulnerability if a KUnit-managed device node is passed as the `from` argument.
--
--- Patch [7]: [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes ---
commit 46e51b3734a9b5d623eb4fdcf34b7fd8442c735e
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
clk: tests: Add Kunit testing for nexus nodes
This commit adds a nexus node with a child requesting a mapped clock in the
fake DT overlay to verify parsing works correctly. It also introduces a new
KUnit helper similar to of_find_node_by_name() to automatically garbage collect
nodes during testing.
> diff --git a/drivers/clk/clk_kunit_helpers.c b/drivers/clk/clk_kunit_helpers.c
> index 68a28e70bb61c..ab6b2cffcad60 100644
> --- a/drivers/clk/clk_kunit_helpers.c
> +++ b/drivers/clk/clk_kunit_helpers.c
[ ... ]
> +struct device_node *clk_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);
[Severity: Medium]
Since of_find_node_by_name() drops the reference to the from node, what
happens if a test author chains calls and passes a KUnit-managed node as the
from argument?
Will the underlying function drop the reference while KUnit's deferred
teardown attempts a second of_node_put() on the same node when the test
finishes, leading to a double-free?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com?part=7
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 6/7] clk: Add support for clock nexus dt bindings
2026-08-07 15:24 ` [PATCH v5 6/7] clk: Add support for clock nexus dt bindings Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:35 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-07 15:35 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric); +Cc: conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing length validation on `*-map-mask` and `*-map-pass-thru` arrays in `of_parse_phandle_with_args_map()` allows out-of-bounds memory reads.
- [High] Lack of cycle detection in `of_parse_phandle_with_args_map()` allows a malicious device tree to cause an infinite loop in the kernel.
--
--- Patch [6]: [PATCH v5 6/7] clk: Add support for clock nexus dt bindings ---
commit a3b2f5d6812aa21284326e875a1ad049deda57c7
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
clk: Add support for clock nexus dt bindings
This patch introduces support for nexus nodes in the clock subsystem. It
transitions the parsing logic from of_parse_phandle_with_args() to
of_parse_phandle_with_args_map() to support clock-map properties.
> 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: High]
This isn't a bug introduced by this patch, but changing to
of_parse_phandle_with_args_map() exposes the clock subsystem to a
pre-existing issue. Does of_parse_phandle_with_args_map() properly
validate the lengths of the map-mask and map-pass-thru arrays?
Looking at of_parse_phandle_with_args_map() in drivers/of/base.c, it seems
there are loops that unconditionally access mask[i] and pass[i] up to
list_size:
for (i = 0; i < list_size; i++, map_len--)
match &= !((match_array[i] ^ *map++) & mask[i]);
and later:
if (i < list_size) {
val &= ~pass[i];
val |= cpu_to_be32(out_args->args[i]) & pass[i];
}
If a device tree node provides clock-map-mask or clock-map-pass-thru properties
that are smaller than the expected size dictated by #clock-cells (list_size),
can this cause an out-of-bounds memory read?
[Severity: High]
This is a pre-existing issue, but does of_parse_phandle_with_args_map()
prevent infinite loops when parsing maliciously or accidentally crafted
device tree overlays?
In drivers/of/base.c:of_parse_phandle_with_args_map(), the main loop replaces
the current node with the new mapped node without tracking visited nodes or
limiting the mapping depth:
while (cur) {
/* Get the <list>-map property */
If a cyclic nexus mapping exists (e.g. node A maps to node B, and node B maps
back to node A), could this loop run indefinitely and hang the kernel?
> if (rc < 0) {
> /* skip empty (null) phandles */
> if (rc == -ENOENT)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com?part=6
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place
2026-08-07 15:24 ` [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
@ 2026-08-07 15:41 ` sashiko-bot
2026-08-07 16:26 ` Frank Li
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-07 15:41 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric); +Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] The switch to `of_parse_clkspec` in `of_clk_get_parent_name` introduces broken positional index lookups during `clock-ranges` fallback and creates an API mismatch with `of_clk_get_parent_count`.
Pre-existing issues:
- [High] Returning a pointer to the device node's name (`clkspec.np->name`) after calling `of_node_put(clkspec.np)` causes a potential Use-After-Free.
--
--- Patch [5]: [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place ---
commit 83137fb481bf2605bb7ce30c009986e0de4d1253
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Subject: clk: Use the generic OF phandle parsing in only one place
This patch consolidates the device tree parsing logic by replacing
of_parse_phandle_with_args() with of_parse_clkspec() inside
of_clk_get_parent_name(). This change provides uniform handling and aims
to support the clock-ranges property during parent clock resolution.
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 7d63d81ebc096..45f5d7a4ccc17 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5219,6 +5219,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
> * has a "clock-ranges" property, then we can try one of its
> * clocks.
> */
> + if (of_property_present(np, "clocks"))
> + break;
> np = np->parent;
> if (np && !of_property_present(np, "clock-ranges"))
> break;
[Severity: Medium]
When falling back to the parent node due to clock-ranges, does resetting
the index to 0 break positional index lookups?
Right below this snippet in of_parse_clkspec(), the index is explicitly
reset to 0 before the loop continues:
np = np->parent;
if (np && !of_property_present(np, "clock-ranges"))
break;
index = 0;
If a driver calls of_clk_get_parent_name() with an index > 0 for a node
that lacks a clocks property but whose parent has clock-ranges, this reset
causes the parent's 0th clock to be returned repeatedly instead of the
subsequently requested clocks.
> @@ -5364,8 +5366,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;
[Severity: Medium]
Does this change create an API mismatch with of_clk_get_parent_count()?
Currently, of_clk_get_parent_count() relies on of_count_phandle_with_args()
which does not check for the clock-ranges fallback. Now that
of_clk_get_parent_name() uses of_parse_clkspec() and supports clock-ranges,
will drivers relying on of_clk_get_parent_count() receive a count of 0 and
fail to initialize?
> [ ... ]
> clk = of_clk_get_from_provider(&clkspec);
> if (IS_ERR(clk)) {
> if (clkspec.args_count == 0)
> clk_name = clkspec.np->name;
> else
> clk_name = NULL;
> } else {
> clk_name = __clk_get_name(clk);
> clk_put(clk);
> }
> }
>
>
> of_node_put(clkspec.np);
> return clk_name;
[Severity: High]
This is a pre-existing issue, but can this result in a use-after-free of the
node's name?
If the clock is unregistered and has a clock-cells value of 0, clk_name is
assigned directly to clkspec.np->name. The node reference is then dropped
immediately via of_node_put().
If the node was dynamically allocated, such as through a device tree
overlay, dropping the last reference here frees the node and its string
properties, leaving clk_name pointing to freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-schneider-v7-2-rc1-eip201-upstream-v5-0-8785f9531cf0@bootlin.com?part=5
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 4/7] clk: Improve a couple of comments
2026-08-07 15:24 ` [PATCH v5 4/7] clk: Improve a couple of comments Miquel Raynal (Schneider Electric)
@ 2026-08-07 16:23 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-08-07 16:23 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric)
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar, Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev
On Fri, Aug 07, 2026 at 05:24:05PM +0200, Miquel Raynal (Schneider Electric) wrote:
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place
2026-08-07 15:24 ` [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
2026-08-07 15:41 ` sashiko-bot
@ 2026-08-07 16:26 ` Frank Li
1 sibling, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-08-07 16:26 UTC (permalink / raw)
To: Miquel Raynal (Schneider Electric)
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brendan Higgins, David Gow,
Rae Moar, Thomas Petazzoni, Pascal EBERHARD, Wolfram Sang,
linux-clk, devicetree, linux-kernel, linux-kselftest, kunit-dev
On Fri, Aug 07, 2026 at 05:24:06PM +0200, Miquel Raynal (Schneider Electric) wrote:
> Ideally, there should be one single entry in the OF world. This way, the
> DT parsing logic is always identical. At this stage, we call
> of_parse_phandle_with_args() in:
> - of_parse_clkspec()
> - of_clk_get_parent_name()
> which kinds of defeats the above statement. Furthermore, when this call
> is done from of_clk_get_parent_name(), we fail to parse the
> 'clock-ranges' property.
>
> Make sure DT world is entered only once by calling of_parse_clkspec()
> from of_clk_get_parent_name(). Doing so implies that we do not try the
> "clock-ranges" path if we already found a "clocks" property.
>
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/clk/clk.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 7d63d81ebc09..45f5d7a4ccc1 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5219,6 +5219,8 @@ static int of_parse_clkspec(const struct device_node *np, int index,
> * has a "clock-ranges" property, then we can try one of its
> * clocks.
> */
> + if (of_property_present(np, "clocks"))
> + break;
> np = np->parent;
> if (np && !of_property_present(np, "clock-ranges"))
> break;
> @@ -5364,8 +5366,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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-07 16:26 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 15:24 [PATCH v5 0/7] clk: Add support for clock nexus Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 1/7] dt-bindings: clock: Introduce nexus nodes Miquel Raynal (Schneider Electric)
2026-08-07 15:33 ` sashiko-bot
2026-08-07 15:24 ` [PATCH v5 2/7] clk: tests: Add clk_parse_clkspec() Kunit testing Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name() Miquel Raynal (Schneider Electric)
2026-08-07 15:24 ` [PATCH v5 4/7] clk: Improve a couple of comments Miquel Raynal (Schneider Electric)
2026-08-07 16:23 ` Frank Li
2026-08-07 15:24 ` [PATCH v5 5/7] clk: Use the generic OF phandle parsing in only one place Miquel Raynal (Schneider Electric)
2026-08-07 15:41 ` sashiko-bot
2026-08-07 16:26 ` Frank Li
2026-08-07 15:24 ` [PATCH v5 6/7] clk: Add support for clock nexus dt bindings Miquel Raynal (Schneider Electric)
2026-08-07 15:35 ` sashiko-bot
2026-08-07 15:24 ` [PATCH v5 7/7] clk: tests: Add Kunit testing for nexus nodes Miquel Raynal (Schneider Electric)
2026-08-07 15:33 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).