[PATCH v4 0/5] clk: Support spread spectrum and use it in clk-scmi

[PATCH v4 0/5] clk: Support spread spectrum and use it in clk-scmi

[Peng Fan]

Since the assigned-clock-sscs property [1] has been accepted into the device
tree schema, we can now support it in the Linux clock driver. Therefore,
I’ve picked up the previously submitted work [2] titled “clk: Support
spread spectrum and use it in clk-pll144x and clk-scmi.”
As more than six months have passed since [2] was posted, I’m treating this
patchset as a new submission rather than a v3.

- Introduce clk_set_spread_spectrum to set the parameters for enabling
  spread spectrum of a clock.
- Parse 'assigned-clock-sscs' and configure it by default before using the
  clock. This property is parsed before parsing clock rate.
- Enable this feature for clk-scmi on i.MX95.

Because SCMI spec will not include spread spectrum as a standard
extension, we still need to use NXP i.MX OEM extension.

[1] https://github.com/devicetree-org/dt-schema/pull/154
[2] https://lore.kernel.org/all/20250205-clk-ssc-v2-0-fa73083caa92@nxp.com/

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v4:
- Add R-b for patch 1 from Brian
- Drop unecessary change in patch 4 Per Brian
- Link to v3: https://lore.kernel.org/r/20250912-clk-ssc-version1-v3-0-fd1e07476ba1@nxp.com

Changes in v3:
- New patch 1 for dt-bindings per comment from Brian
  https://lore.kernel.org/all/aLeEFzXkPog_dt2B@x1/
  This might not be good to add a new dt-binding file in v3. But this is
  quite a simple file that just has four macros to encode modulation
  method. So hope this is fine for DT maintainers.
- Add Brain's R-b for patch 2
- New patch 3 to add Kunit test per Brain. Since Brain helped
  draft part of the code, I added Co-developed-by tag from Brain.
- Link to v2: https://lore.kernel.org/r/20250901-clk-ssc-version1-v2-0-1d0a486dffe6@nxp.com

Changes in v2:
- Simplify the code in patch 2 per Dan Carpenter and Brian Masney
- Rebased to next-20250829
- Link to v1: https://lore.kernel.org/r/20250812-clk-ssc-version1-v1-0-cef60f20d770@nxp.com

---
Peng Fan (5):
      dt-bindings: clock: Add spread spectrum definition
      clk: Introduce clk_hw_set_spread_spectrum
      clk: conf: Support assigned-clock-sscs
      clk: Add KUnit tests for assigned-clock-sscs
      clk: scmi: Support Spread Spectrum for NXP i.MX95

 drivers/clk/Makefile                               |   6 +
 drivers/clk/clk-conf.c                             |  69 ++++++++++++
 drivers/clk/clk-scmi.c                             |  64 ++++++++++-
 drivers/clk/clk.c                                  |  26 +++++
 drivers/clk/clk_test.c                             | 121 ++++++++++++++++++++-
 drivers/clk/kunit_clk_assigned_rates.h             |  10 ++
 drivers/clk/kunit_clk_assigned_rates_multiple.dtso |   6 +
 ...kunit_clk_assigned_rates_multiple_consumer.dtso |   6 +
 drivers/clk/kunit_clk_assigned_rates_one.dtso      |   3 +
 .../clk/kunit_clk_assigned_rates_one_consumer.dtso |   3 +
 .../clk/kunit_clk_assigned_rates_u64_multiple.dtso |   6 +
 ...t_clk_assigned_rates_u64_multiple_consumer.dtso |   6 +
 drivers/clk/kunit_clk_assigned_rates_u64_one.dtso  |   3 +
 .../kunit_clk_assigned_rates_u64_one_consumer.dtso |   3 +
 drivers/clk/kunit_clk_assigned_sscs_null.dtso      |  16 +++
 .../clk/kunit_clk_assigned_sscs_null_consumer.dtso |  20 ++++
 drivers/clk/kunit_clk_assigned_sscs_without.dtso   |  15 +++
 .../kunit_clk_assigned_sscs_without_consumer.dtso  |  19 ++++
 drivers/clk/kunit_clk_assigned_sscs_zero.dtso      |  12 ++
 .../clk/kunit_clk_assigned_sscs_zero_consumer.dtso |  16 +++
 include/dt-bindings/clock/clock.h                  |  14 +++
 include/linux/clk-provider.h                       |  22 ++++
 include/linux/scmi_protocol.h                      |   5 +
 23 files changed, 464 insertions(+), 7 deletions(-)
---
base-commit: 8941e75c0f122fdd76dc54ed45c4ce917587e006
change-id: 20250812-clk-ssc-version1-acf6f6efbd96

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH v4 1/5] dt-bindings: clock: Add spread spectrum definition

[Peng Fan]

Per dt-schema, the modulation methods are: down-spread(3), up-spread(2),
center-spread(1), no-spread(0). So define them in dt-bindings to avoid
write the magic number in device tree.

Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 include/dt-bindings/clock/clock.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/dt-bindings/clock/clock.h b/include/dt-bindings/clock/clock.h
new file mode 100644
index 0000000000000000000000000000000000000000..155e2653a120bf10747bd7f4d47f25e0493e0464
--- /dev/null
+++ b/include/dt-bindings/clock/clock.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_H
+#define __DT_BINDINGS_CLOCK_H
+
+#define CLK_SSC_NO_SPREAD	0
+#define CLK_SSC_CENTER_SPREAD	1
+#define CLK_SSC_UP_SPREAD	2
+#define CLK_SSC_DOWN_SPREAD	3
+
+#endif	/* __DT_BINDINGS_CLOCK_H */

-- 
2.37.1

[PATCH v4 2/5] clk: Introduce clk_hw_set_spread_spectrum

[Peng Fan]

Add clk_hw_set_spread_spectrum to configure a clock to enable spread
spectrum feature. set_spread_spectrum ops is added for clk drivers to
have their own hardware specific implementation.

Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk.c            | 26 ++++++++++++++++++++++++++
 include/linux/clk-provider.h | 22 ++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b821b2cdb155331c85fafbd2fac8ab3703a08e4d..06db8918a1b35e3280e565272bc4603a88295a92 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2802,6 +2802,32 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_set_max_rate);
 
+int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf)
+{
+	struct clk_core *core;
+	int ret;
+
+	if (!hw)
+		return 0;
+
+	core = hw->core;
+
+	clk_prepare_lock();
+
+	ret = clk_pm_runtime_get(core);
+	if (ret)
+		goto fail;
+
+	if (core->ops->set_spread_spectrum)
+		ret = core->ops->set_spread_spectrum(hw, conf);
+
+	clk_pm_runtime_put(core);
+
+fail:
+	clk_prepare_unlock();
+	return ret;
+}
+
 /**
  * clk_get_parent - return the parent of a clk
  * @clk: the clk whose parent gets returned
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 630705a47129453c241f1b1755f2c2f2a7ed8f77..4f48a4df95a1c54638a0e91e0a449fcc8aa40b80 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -84,6 +84,19 @@ struct clk_duty {
 	unsigned int den;
 };
 
+/**
+ * struct clk_spread_spectrum - Structure encoding spread spectrum of a clock
+ *
+ * @modfreq_hz:		Modulation frequency
+ * @spread_bp:		Modulation percent in permyriad
+ * @method:		Modulation method
+ */
+struct clk_spread_spectrum {
+	u32 modfreq_hz;
+	u32 spread_bp;
+	u32 method;
+};
+
 /**
  * struct clk_ops -  Callback operations for hardware clocks; these are to
  * be provided by the clock implementation, and will be called by drivers
@@ -178,6 +191,12 @@ struct clk_duty {
  *		separately via calls to .set_parent and .set_rate.
  *		Returns 0 on success, -EERROR otherwise.
  *
+ * @set_spread_spectrum: Optional callback used to configure the spread
+ *		spectrum modulation frequency, percentage, and method
+ *		to reduce EMI by spreading the clock frequency over a
+ *		wider range.
+ *		Returns 0 on success, -EERROR otherwise.
+ *
  * @recalc_accuracy: Recalculate the accuracy of this clock. The clock accuracy
  *		is expressed in ppb (parts per billion). The parent accuracy is
  *		an input parameter.
@@ -255,6 +274,8 @@ struct clk_ops {
 	int		(*set_rate_and_parent)(struct clk_hw *hw,
 				    unsigned long rate,
 				    unsigned long parent_rate, u8 index);
+	int		(*set_spread_spectrum)(struct clk_hw *hw,
+					       struct clk_spread_spectrum *clk_ss);
 	unsigned long	(*recalc_accuracy)(struct clk_hw *hw,
 					   unsigned long parent_accuracy);
 	int		(*get_phase)(struct clk_hw *hw);
@@ -1430,6 +1451,7 @@ void clk_hw_get_rate_range(struct clk_hw *hw, unsigned long *min_rate,
 			   unsigned long *max_rate);
 void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
 			   unsigned long max_rate);
+int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf);
 
 static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)
 {

-- 
2.37.1

[PATCH v4 3/5] clk: conf: Support assigned-clock-sscs

[Peng Fan]

Parse the Spread Spectrum Configuration(SSC) from device tree and configure
them before using the clock.

Each SSC is three u32 elements which means '<modfreq spreaddepth
modmethod>', so assigned-clock-sscs is an array of multiple three u32
elements.

Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-conf.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 303a0bb26e54a95655ce094a35b989c97ebc6fd8..dd6083597db3f8f27d86abf5640dfc3fb39a9b88 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -155,6 +155,71 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
 	return 0;
 }
 
+static int __set_clk_spread_spectrum(struct device_node *node, bool clk_supplier)
+{
+	struct clk_spread_spectrum *sscs __free(kfree) = NULL;
+	u32 elem_size = sizeof(struct clk_spread_spectrum);
+	struct of_phandle_args clkspec;
+	int rc, count, index;
+	struct clk *clk;
+
+	/* modfreq, spreadPercent, modmethod */
+	count = of_property_count_elems_of_size(node, "assigned-clock-sscs", elem_size);
+	if (count <= 0)
+		return 0;
+
+	sscs = kcalloc(count, elem_size, GFP_KERNEL);
+	if (!sscs)
+		return -ENOMEM;
+
+	rc = of_property_read_u32_array(node, "assigned-clock-sscs", (u32 *)sscs,
+					count * 3);
+	if (rc)
+		return rc;
+
+	for (index = 0; index < count; index++) {
+		struct clk_spread_spectrum *conf = &sscs[index];
+		struct clk_hw *hw;
+
+		if (!conf->modfreq_hz && !conf->spread_bp && !conf->method)
+			continue;
+
+		rc = of_parse_phandle_with_args(node, "assigned-clocks", "#clock-cells",
+						index, &clkspec);
+		if (rc < 0) {
+			/* skip empty (null) phandles */
+			if (rc == -ENOENT)
+				continue;
+			else
+				return rc;
+		}
+
+		if (clkspec.np == node && !clk_supplier) {
+			of_node_put(clkspec.np);
+			return 0;
+		}
+
+		clk = of_clk_get_from_provider(&clkspec);
+		of_node_put(clkspec.np);
+		if (IS_ERR(clk)) {
+			if (PTR_ERR(clk) != -EPROBE_DEFER)
+				pr_warn("clk: couldn't get clock %d for %pOF\n",
+					index, node);
+			return PTR_ERR(clk);
+		}
+
+		hw = __clk_get_hw(clk);
+		rc = clk_hw_set_spread_spectrum(hw, conf);
+		if (rc < 0)
+			pr_err("clk: couldn't set %s clk spread spectrum %u %u %u: %d\n",
+			       __clk_get_name(clk), conf->modfreq_hz, conf->spread_bp,
+			       conf->method, rc);
+		clk_put(clk);
+	}
+
+	return 0;
+}
+
 /**
  * of_clk_set_defaults() - parse and set assigned clocks configuration
  * @node: device node to apply clock settings for
@@ -174,6 +239,10 @@ int of_clk_set_defaults(struct device_node *node, bool clk_supplier)
 	if (!node)
 		return 0;
 
+	rc = __set_clk_spread_spectrum(node, clk_supplier);
+	if (rc < 0)
+		return rc;
+
 	rc = __set_clk_parents(node, clk_supplier);
 	if (rc < 0)
 		return rc;

-- 
2.37.1

[PATCH v4 4/5] clk: Add KUnit tests for assigned-clock-sscs

[Peng Fan]

Spread spectrum configuration is part of clock frequency settings,
and its behavior can be validated similarly to assigned clock rates.

Extend the existing KUnit tests for assigned-clock-rates to cover
assigned-clock-sscs by reusing the test framework. Add new test
device trees:
  - kunit_clk_assigned_sscs_null.dtso
  - kunit_clk_assigned_sscs_null_consumer.dtso
  - kunit_clk_assigned_sscs_without.dtso
  - kunit_clk_assigned_sscs_without_consumer.dtso
  - kunit_clk_assigned_sscs_zero.dtso
  - kunit_clk_assigned_sscs_zero_consumer.dtso

These tests cover various invalid configurations of assigned-clock-sscs,
ensuring robustness and consistent error handling, similar to the coverage
provided for assigned-clock-rates.

Co-developed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/Makefile                               |   6 +
 drivers/clk/clk_test.c                             | 121 ++++++++++++++++++++-
 drivers/clk/kunit_clk_assigned_rates.h             |  10 ++
 drivers/clk/kunit_clk_assigned_rates_multiple.dtso |   6 +
 ...kunit_clk_assigned_rates_multiple_consumer.dtso |   6 +
 drivers/clk/kunit_clk_assigned_rates_one.dtso      |   3 +
 .../clk/kunit_clk_assigned_rates_one_consumer.dtso |   3 +
 .../clk/kunit_clk_assigned_rates_u64_multiple.dtso |   6 +
 ...t_clk_assigned_rates_u64_multiple_consumer.dtso |   6 +
 drivers/clk/kunit_clk_assigned_rates_u64_one.dtso  |   3 +
 .../kunit_clk_assigned_rates_u64_one_consumer.dtso |   3 +
 drivers/clk/kunit_clk_assigned_sscs_null.dtso      |  16 +++
 .../clk/kunit_clk_assigned_sscs_null_consumer.dtso |  20 ++++
 drivers/clk/kunit_clk_assigned_sscs_without.dtso   |  15 +++
 .../kunit_clk_assigned_sscs_without_consumer.dtso  |  19 ++++
 drivers/clk/kunit_clk_assigned_sscs_zero.dtso      |  12 ++
 .../clk/kunit_clk_assigned_sscs_zero_consumer.dtso |  16 +++
 17 files changed, 268 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 18ed29cfdc1133b6c254190c6092eb263366d5ac..ea93e16c5a0080f610c8cfaccc1ac68fefa481c2 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -18,6 +18,12 @@ clk-test-y			:= clk_test.o \
 				   kunit_clk_assigned_rates_without_consumer.dtbo.o \
 				   kunit_clk_assigned_rates_zero.dtbo.o \
 				   kunit_clk_assigned_rates_zero_consumer.dtbo.o \
+				   kunit_clk_assigned_sscs_null.dtbo.o \
+				   kunit_clk_assigned_sscs_null_consumer.dtbo.o \
+				   kunit_clk_assigned_sscs_without.dtbo.o \
+				   kunit_clk_assigned_sscs_without_consumer.dtbo.o \
+				   kunit_clk_assigned_sscs_zero.dtbo.o \
+				   kunit_clk_assigned_sscs_zero_consumer.dtbo.o \
 				   kunit_clk_hw_get_dev_of_node.dtbo.o \
 				   kunit_clk_parent_data_test.dtbo.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index a268d7b5d4cb28ec1f029f828c31107f8e130556..eb28f0a61ef0057bcc9c1d75653357a442bff81d 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -28,6 +28,7 @@ static const struct clk_ops empty_clk_ops = { };
 struct clk_dummy_context {
 	struct clk_hw hw;
 	unsigned long rate;
+	struct clk_spread_spectrum sscs;
 };
 
 static unsigned long clk_dummy_recalc_rate(struct clk_hw *hw,
@@ -83,6 +84,17 @@ static int clk_dummy_set_rate(struct clk_hw *hw,
 	return 0;
 }
 
+static int clk_dummy_set_spread_spectrum(struct clk_hw *hw,
+					 struct clk_spread_spectrum *conf)
+{
+	struct clk_dummy_context *ctx =
+		container_of(hw, struct clk_dummy_context, hw);
+
+	ctx->sscs = *conf;
+
+	return 0;
+}
+
 static int clk_dummy_single_set_parent(struct clk_hw *hw, u8 index)
 {
 	if (index >= clk_hw_get_num_parents(hw))
@@ -100,18 +112,21 @@ static const struct clk_ops clk_dummy_rate_ops = {
 	.recalc_rate = clk_dummy_recalc_rate,
 	.determine_rate = clk_dummy_determine_rate,
 	.set_rate = clk_dummy_set_rate,
+	.set_spread_spectrum = clk_dummy_set_spread_spectrum,
 };
 
 static const struct clk_ops clk_dummy_maximize_rate_ops = {
 	.recalc_rate = clk_dummy_recalc_rate,
 	.determine_rate = clk_dummy_maximize_rate,
 	.set_rate = clk_dummy_set_rate,
+	.set_spread_spectrum = clk_dummy_set_spread_spectrum,
 };
 
 static const struct clk_ops clk_dummy_minimize_rate_ops = {
 	.recalc_rate = clk_dummy_recalc_rate,
 	.determine_rate = clk_dummy_minimize_rate,
 	.set_rate = clk_dummy_set_rate,
+	.set_spread_spectrum = clk_dummy_set_spread_spectrum,
 };
 
 static const struct clk_ops clk_dummy_single_parent_ops = {
@@ -3097,6 +3112,7 @@ struct clk_assigned_rates_context {
  * @overlay_end: Pointer to end of DT overlay to apply for test
  * @rate0: Initial rate of first clk
  * @rate1: Initial rate of second clk
+ * @sscs: Initial spread spectrum settings
  * @consumer_test: true if a consumer is being tested
  */
 struct clk_assigned_rates_test_param {
@@ -3105,6 +3121,7 @@ struct clk_assigned_rates_test_param {
 	u8 *overlay_end;
 	unsigned long rate0;
 	unsigned long rate1;
+	struct clk_spread_spectrum sscs;
 	bool consumer_test;
 };
 
@@ -3116,7 +3133,7 @@ static void
 clk_assigned_rates_register_clk(struct kunit *test,
 				struct clk_dummy_context *ctx,
 				struct device_node *np, const char *name,
-				unsigned long rate)
+				unsigned long rate, const struct clk_spread_spectrum *sscs)
 {
 	struct clk_init_data init = { };
 
@@ -3124,6 +3141,7 @@ clk_assigned_rates_register_clk(struct kunit *test,
 	init.ops = &clk_dummy_rate_ops;
 	ctx->hw.init = &init;
 	ctx->rate = rate;
+	ctx->sscs = *sscs;
 
 	KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, np, &ctx->hw));
 	KUNIT_ASSERT_EQ(test, ctx->rate, rate);
@@ -3167,14 +3185,16 @@ static int clk_assigned_rates_test_init(struct kunit *test)
 	KUNIT_ASSERT_LT(test, clk_cells, 2);
 
 	clk_assigned_rates_register_clk(test, &ctx->clk0, np,
-					"test_assigned_rate0", test_param->rate0);
+					"test_assigned_rate0", test_param->rate0,
+					&test_param->sscs);
 	if (clk_cells == 0) {
 		KUNIT_ASSERT_EQ(test, 0,
 				of_clk_add_hw_provider_kunit(test, np, of_clk_hw_simple_get,
 							     &ctx->clk0.hw));
 	} else if (clk_cells == 1) {
 		clk_assigned_rates_register_clk(test, &ctx->clk1, np,
-						"test_assigned_rate1", test_param->rate1);
+						"test_assigned_rate1", test_param->rate1,
+						&test_param->sscs);
 
 		KUNIT_ASSERT_NOT_ERR_OR_NULL(test,
 			data = kunit_kzalloc(test, struct_size(data, hws, 2), GFP_KERNEL));
@@ -3203,6 +3223,9 @@ static void clk_assigned_rates_assigns_one(struct kunit *test)
 	struct clk_assigned_rates_context *ctx = test->priv;
 
 	KUNIT_EXPECT_EQ(test, ctx->clk0.rate, ASSIGNED_RATES_0_RATE);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
 }
 
 static void clk_assigned_rates_assigns_multiple(struct kunit *test)
@@ -3210,7 +3233,13 @@ static void clk_assigned_rates_assigns_multiple(struct kunit *test)
 	struct clk_assigned_rates_context *ctx = test->priv;
 
 	KUNIT_EXPECT_EQ(test, ctx->clk0.rate, ASSIGNED_RATES_0_RATE);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
 	KUNIT_EXPECT_EQ(test, ctx->clk1.rate, ASSIGNED_RATES_1_RATE);
+	KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.modfreq_hz, ASSIGNED_SSCS_1_MODFREQ);
+	KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.spread_bp, ASSIGNED_SSCS_1_SPREAD);
+	KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.method, ASSIGNED_SSCS_1_METHOD);
 }
 
 static void clk_assigned_rates_skips(struct kunit *test)
@@ -3222,6 +3251,19 @@ static void clk_assigned_rates_skips(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, ctx->clk0.rate, test_param->rate0);
 }
 
+static void clk_assigned_sscs_skips(struct kunit *test)
+{
+	struct clk_assigned_rates_context *ctx = test->priv;
+	const struct clk_assigned_rates_test_param *test_param = test->param_value;
+
+	KUNIT_EXPECT_NE(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
+	KUNIT_EXPECT_NE(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
+	KUNIT_EXPECT_NE(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, test_param->sscs.modfreq_hz);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, test_param->sscs.spread_bp);
+	KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, test_param->sscs.method);
+}
+
 OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_one);
 OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_one_consumer);
 OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_u64_one);
@@ -3384,6 +3426,77 @@ KUNIT_ARRAY_PARAM_DESC(clk_assigned_rates_skips,
 		       clk_assigned_rates_skips_test_params,
 		       desc)
 
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_without);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_without_consumer);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_zero);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_zero_consumer);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_null);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_null_consumer);
+
+/* Test cases that skip changing the sscs due to malformed DT */
+static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_params[] = {
+	{
+		/*
+		 * Test that an assigned-clock-sscs property without an assigned-clocks
+		 * property fails when the property is in the provider.
+		 */
+		.desc = "provider missing assigned-clocks",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without),
+		.sscs = {50000, 60000, 3},
+	},
+	{
+		/*
+		 * Test that an assigned-clock-rates property without an assigned-clocks
+		 * property fails when the property is in the consumer.
+		 */
+		.desc = "consumer missing assigned-clocks",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without_consumer),
+		.sscs = {50000, 60000, 3},
+		.consumer_test = true,
+	},
+	{
+		/*
+		 * Test that an assigned-clock-rates property of zero doesn't
+		 * set a rate when the property is in the provider.
+		 */
+		.desc = "provider assigned-clock-sscs of zero",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero),
+		.sscs = {50000, 60000, 3},
+	},
+	{
+		/*
+		 * Test that an assigned-clock-rates property of zero doesn't
+		 * set a rate when the property is in the consumer.
+		 */
+		.desc = "consumer assigned-clock-sscs of zero",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero_consumer),
+		.sscs = {50000, 60000, 3},
+		.consumer_test = true,
+	},
+	{
+		/*
+		 * Test that an assigned-clocks property with a null phandle
+		 * doesn't set a rate when the property is in the provider.
+		 */
+		.desc = "provider assigned-clocks null phandle",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null),
+		.sscs = {50000, 60000, 3},
+	},
+	{
+		/*
+		 * Test that an assigned-clocks property with a null phandle
+		 * doesn't set a rate when the property is in the consumer.
+		 */
+		.desc = "provider assigned-clocks null phandle",
+		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null_consumer),
+		.sscs = {50000, 60000, 3},
+		.consumer_test = true,
+	},
+};
+KUNIT_ARRAY_PARAM_DESC(clk_assigned_sscs_skips,
+		       clk_assigned_sscs_skips_test_params,
+		       desc)
+
 static struct kunit_case clk_assigned_rates_test_cases[] = {
 	KUNIT_CASE_PARAM(clk_assigned_rates_assigns_one,
 			 clk_assigned_rates_assigns_one_gen_params),
@@ -3391,6 +3504,8 @@ static struct kunit_case clk_assigned_rates_test_cases[] = {
 			 clk_assigned_rates_assigns_multiple_gen_params),
 	KUNIT_CASE_PARAM(clk_assigned_rates_skips,
 			 clk_assigned_rates_skips_gen_params),
+	KUNIT_CASE_PARAM(clk_assigned_sscs_skips,
+			 clk_assigned_sscs_skips_gen_params),
 	{}
 };
 
diff --git a/drivers/clk/kunit_clk_assigned_rates.h b/drivers/clk/kunit_clk_assigned_rates.h
index df2d84dcaa93511694b6da842debdc3cfd3a6c19..d7ae5ec2d25bed79b8438e8ce580872131ce4286 100644
--- a/drivers/clk/kunit_clk_assigned_rates.h
+++ b/drivers/clk/kunit_clk_assigned_rates.h
@@ -1,8 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+
+#include <dt-bindings/clock/clock.h>
+
 #ifndef _KUNIT_CLK_ASSIGNED_RATES_H
 #define _KUNIT_CLK_ASSIGNED_RATES_H
 
 #define ASSIGNED_RATES_0_RATE		1600000
 #define ASSIGNED_RATES_1_RATE		9700000
 
+#define ASSIGNED_SSCS_0_MODFREQ		10000
+#define ASSIGNED_SSCS_0_SPREAD		30000
+#define ASSIGNED_SSCS_0_METHOD		CLK_SSC_CENTER_SPREAD
+#define ASSIGNED_SSCS_1_MODFREQ		20000
+#define ASSIGNED_SSCS_1_SPREAD		40000
+#define ASSIGNED_SSCS_1_METHOD		CLK_SSC_UP_SPREAD
+
 #endif
diff --git a/drivers/clk/kunit_clk_assigned_rates_multiple.dtso b/drivers/clk/kunit_clk_assigned_rates_multiple.dtso
index e600736e70f5041ddeb1bfb0d6074746a064e08a..6c54d65444d5d779c9fa4bb2a79c4742dd88f6d0 100644
--- a/drivers/clk/kunit_clk_assigned_rates_multiple.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_multiple.dtso
@@ -12,5 +12,11 @@ clk: kunit-clock {
 				  <&clk 1>;
 		assigned-clock-rates = <ASSIGNED_RATES_0_RATE>,
 				       <ASSIGNED_RATES_1_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>,
+				      <ASSIGNED_SSCS_1_MODFREQ
+				       ASSIGNED_SSCS_1_SPREAD
+				       ASSIGNED_SSCS_1_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_multiple_consumer.dtso b/drivers/clk/kunit_clk_assigned_rates_multiple_consumer.dtso
index 260aba458daf2bc57fde46b5442453e7de10faac..b1fee396c4b1e51341a411168569d8351bb23b12 100644
--- a/drivers/clk/kunit_clk_assigned_rates_multiple_consumer.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_multiple_consumer.dtso
@@ -16,5 +16,11 @@ kunit-clock-consumer {
 				  <&clk 1>;
 		assigned-clock-rates = <ASSIGNED_RATES_0_RATE>,
 				       <ASSIGNED_RATES_1_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>,
+				      <ASSIGNED_SSCS_1_MODFREQ
+				       ASSIGNED_SSCS_1_SPREAD
+				       ASSIGNED_SSCS_1_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_one.dtso b/drivers/clk/kunit_clk_assigned_rates_one.dtso
index dd95ec9b1cf977883f71564a94602ae518937132..da6e91b9e6bda0ef2c8f601a08aef1f10fda4baa 100644
--- a/drivers/clk/kunit_clk_assigned_rates_one.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_one.dtso
@@ -10,5 +10,8 @@ clk: kunit-clock {
 		#clock-cells = <0>;
 		assigned-clocks = <&clk>;
 		assigned-clock-rates = <ASSIGNED_RATES_0_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso b/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
index a41dca806318b031187c1b8739fcf71eb088a480..4b6e06048f863d014aed8222652d6d9d38e9238b 100644
--- a/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
@@ -14,5 +14,8 @@ kunit-clock-consumer {
 		compatible = "test,clk-consumer";
 		assigned-clocks = <&clk>;
 		assigned-clock-rates = <ASSIGNED_RATES_0_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso
index 389b4e2eb7f74f1770ff5f5c4be5b45dd344dc9c..3a717dab2d00b7fdaff580e30ed2cc520683ef95 100644
--- a/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso
@@ -12,5 +12,11 @@ clk: kunit-clock {
 				  <&clk 1>;
 		assigned-clock-rates-u64 = /bits/ 64 <ASSIGNED_RATES_0_RATE>,
 					   /bits/ 64 <ASSIGNED_RATES_1_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>,
+				      <ASSIGNED_SSCS_1_MODFREQ
+				       ASSIGNED_SSCS_1_SPREAD
+				       ASSIGNED_SSCS_1_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_multiple_consumer.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_multiple_consumer.dtso
index 3e117fd59b7da19cd8a603af77eff29175ce6900..cbee7cbad068f3336f0c8997a5b3e9af4db565c9 100644
--- a/drivers/clk/kunit_clk_assigned_rates_u64_multiple_consumer.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_u64_multiple_consumer.dtso
@@ -16,5 +16,11 @@ kunit-clock-consumer {
 				  <&clk 1>;
 		assigned-clock-rates-u64 = /bits/ 64 <ASSIGNED_RATES_0_RATE>,
 					   /bits/ 64 <ASSIGNED_RATES_1_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>,
+				      <ASSIGNED_SSCS_1_MODFREQ
+				       ASSIGNED_SSCS_1_SPREAD
+				       ASSIGNED_SSCS_1_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso
index 87041264e8f544dafddf2e905efc89dc1f917c54..9b04d6927f0830a5621af1cbea503a427b46bee0 100644
--- a/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso
@@ -10,5 +10,8 @@ clk: kunit-clock {
 		#clock-cells = <0>;
 		assigned-clocks = <&clk>;
 		assigned-clock-rates-u64 = /bits/ 64 <ASSIGNED_RATES_0_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_one_consumer.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_one_consumer.dtso
index 3259c003aec0be3269ab60a4a3a95df69f8de1f8..4784d40520f4193e4f08c8981386f0772a063452 100644
--- a/drivers/clk/kunit_clk_assigned_rates_u64_one_consumer.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_u64_one_consumer.dtso
@@ -14,5 +14,8 @@ kunit-clock-consumer {
 		compatible = "test,clk-consumer";
 		assigned-clocks = <&clk>;
 		assigned-clock-rates-u64 = /bits/ 64 <ASSIGNED_RATES_0_RATE>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
 	};
 };
diff --git a/drivers/clk/kunit_clk_assigned_sscs_null.dtso b/drivers/clk/kunit_clk_assigned_sscs_null.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..43b2068c845dea53ea1328bb63a2f58a4b8ef339
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_null.dtso
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_assigned_rates.h"
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+		assigned-clocks = <0>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
+	};
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_null_consumer.dtso b/drivers/clk/kunit_clk_assigned_sscs_null_consumer.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..bda008f5aaa35e53af97863e4f2e6d8a168cc053
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_null_consumer.dtso
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_assigned_rates.h"
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+	};
+
+	kunit-clock-consumer {
+		compatible = "test,clk-consumer";
+		assigned-clocks = <0>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
+	};
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_without.dtso b/drivers/clk/kunit_clk_assigned_sscs_without.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..08660846b55c12122bfb211c01c377a3a45223c9
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_without.dtso
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_assigned_rates.h"
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
+	};
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_without_consumer.dtso b/drivers/clk/kunit_clk_assigned_sscs_without_consumer.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..e1c089c6f0c0223f16f7ac9a396e7ac7b821c967
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_without_consumer.dtso
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_assigned_rates.h"
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+	};
+
+	kunit-clock-consumer {
+		compatible = "test,clk-consumer";
+		assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+				       ASSIGNED_SSCS_0_SPREAD
+				       ASSIGNED_SSCS_0_METHOD>;
+	};
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_zero.dtso b/drivers/clk/kunit_clk_assigned_sscs_zero.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..f39f4e754e532c9c1b1fdf034700e5af1f3f0779
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_zero.dtso
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+		assigned-clocks = <&clk>;
+		assigned-clock-sscs = <0 0 0>;
+	};
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_zero_consumer.dtso b/drivers/clk/kunit_clk_assigned_sscs_zero_consumer.dtso
new file mode 100644
index 0000000000000000000000000000000000000000..d6bd7dfada7e2f455cb23e483b9bd6ce24839e3a
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_zero_consumer.dtso
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	clk: kunit-clock {
+		compatible = "test,clk-assigned-rates";
+		#clock-cells = <0>;
+	};
+
+	kunit-clock-consumer {
+		compatible = "test,clk-consumer";
+		assigned-clocks = <&clk>;
+		assigned-clock-sscs = <0 0 0>;
+	};
+};

-- 
2.37.1

[PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Peng Fan]

The PLL clocks on NXP i.MX95 SoCs support Spread Spectrum (SS).
This patch introduces scmi_clk_imx_set_spread_spectrum to pass SS
configuration to the SCMI firmware, which handles the actual
implementation.

To ensure this feature is only enabled on NXP platforms,
scmi_clk_imx_extended_config_oem is added. Since SS is only applicable
to PLL clocks, config_oem_get is used to verify SS support for a given
clock.

i.MX95 SCMI firmware Spread Spectrum extConfigValue definition is as
below, no modulation method because firmware forces to use down spread.
	 extConfigValue[7:0]   - spread percentage (%)
	 extConfigValue[23:8]  - Modulation Frequency (KHz)
	 extConfigValue[24]    - Enable/Disable
	 extConfigValue[31:25] - Reserved

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk-scmi.c        | 64 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/scmi_protocol.h |  5 ++++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index d2408403283fc72f0cf902e65f4c08bcbc7b4b0b..bb5e20dab18e92932ab4b99192b496e0c4d96417 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/module.h>
 #include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
 #include <asm/div64.h>
 
 #define NOT_ATOMIC	false
@@ -23,6 +24,7 @@ enum scmi_clk_feats {
 	SCMI_CLK_RATE_CTRL_SUPPORTED,
 	SCMI_CLK_PARENT_CTRL_SUPPORTED,
 	SCMI_CLK_DUTY_CYCLE_SUPPORTED,
+	SCMI_CLK_IMX_SSC_SUPPORTED,
 	SCMI_CLK_FEATS_COUNT
 };
 
@@ -98,6 +100,35 @@ static int scmi_clk_set_parent(struct clk_hw *hw, u8 parent_index)
 	return scmi_proto_clk_ops->parent_set(clk->ph, clk->id, parent_index);
 }
 
+static int scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
+					    struct clk_spread_spectrum *clk_ss)
+{
+	struct scmi_clk *clk = to_scmi_clk(hw);
+	int ret;
+	u32 val;
+
+	/*
+	 * extConfigValue[7:0]   - spread percentage (%)
+	 * extConfigValue[23:8]  - Modulation Frequency
+	 * extConfigValue[24]    - Enable/Disable
+	 * extConfigValue[31:25] - Reserved
+	 */
+	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, clk_ss->spread_bp / 10000);
+	val |= FIELD_PREP(SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK, clk_ss->modfreq_hz);
+	if (clk_ss->method != CLK_SSC_NO_SPREAD)
+		val |= SCMI_CLOCK_IMX_SS_ENABLE_MASK;
+	ret = scmi_proto_clk_ops->config_oem_set(clk->ph, clk->id,
+						 SCMI_CLOCK_CFG_IMX_SSC,
+						 val, false);
+	if (ret)
+		dev_warn(clk->dev,
+			 "Failed to set spread spectrum(%u,%u,%u) for clock ID %d\n",
+			 clk_ss->modfreq_hz, clk_ss->spread_bp, clk_ss->method,
+			 clk->id);
+
+	return ret;
+}
+
 static u8 scmi_clk_get_parent(struct clk_hw *hw)
 {
 	struct scmi_clk *clk = to_scmi_clk(hw);
@@ -316,11 +347,33 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
 		ops->set_duty_cycle = scmi_clk_set_duty_cycle;
 	}
 
+	if (feats_key & BIT(SCMI_CLK_IMX_SSC_SUPPORTED))
+		ops->set_spread_spectrum = scmi_clk_imx_set_spread_spectrum;
+
 	return ops;
 }
 
+static void scmi_clk_imx_extended_config_oem(const struct scmi_handle *handle,
+					     struct scmi_clk *sclk,
+					     unsigned int *feats_key)
+{
+	int ret;
+	u32 val;
+
+	if (strcmp(handle->version->vendor_id, SCMI_IMX_VENDOR) ||
+	    strcmp(handle->version->sub_vendor_id, SCMI_IMX_SUBVENDOR))
+		return;
+
+	ret = scmi_proto_clk_ops->config_oem_get(sclk->ph, sclk->id,
+						 SCMI_CLOCK_CFG_IMX_SSC,
+						 &val, NULL, false);
+	if (!ret)
+		*feats_key |= BIT(SCMI_CLK_IMX_SSC_SUPPORTED);
+}
+
 /**
  * scmi_clk_ops_select() - Select a proper set of clock operations
+ * @handle: A reference to an SCMI entity
  * @sclk: A reference to an SCMI clock descriptor
  * @atomic_capable: A flag to indicate if atomic mode is supported by the
  *		    transport
@@ -345,8 +398,8 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
  *	   NULL otherwise.
  */
 static const struct clk_ops *
-scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
-		    unsigned int atomic_threshold_us,
+scmi_clk_ops_select(const struct scmi_handle *handle, struct scmi_clk *sclk,
+		    bool atomic_capable, unsigned int atomic_threshold_us,
 		    const struct clk_ops **clk_ops_db, size_t db_size)
 {
 	const struct scmi_clock_info *ci = sclk->info;
@@ -370,9 +423,12 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
 	if (!ci->parent_ctrl_forbidden)
 		feats_key |= BIT(SCMI_CLK_PARENT_CTRL_SUPPORTED);
 
-	if (ci->extended_config)
+	if (ci->extended_config) {
 		feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
 
+		scmi_clk_imx_extended_config_oem(handle, sclk, &feats_key);
+	}
+
 	if (WARN_ON(feats_key >= db_size))
 		return NULL;
 
@@ -459,7 +515,7 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
 		 * to avoid sharing the devm_ allocated clk_ops between multiple
 		 * SCMI clk driver instances.
 		 */
-		scmi_ops = scmi_clk_ops_select(sclk, transport_is_atomic,
+		scmi_ops = scmi_clk_ops_select(handle, sclk, transport_is_atomic,
 					       atomic_threshold_us,
 					       scmi_clk_ops_db,
 					       ARRAY_SIZE(scmi_clk_ops_db));
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index aafaac1496b06a6e4f0ca32eee58a9edf7d4a70f..37f422b4b1ef2af2b4231a1677161aa24e07d0e2 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -80,9 +80,14 @@ enum scmi_clock_oem_config {
 	SCMI_CLOCK_CFG_DUTY_CYCLE = 0x1,
 	SCMI_CLOCK_CFG_PHASE,
 	SCMI_CLOCK_CFG_OEM_START = 0x80,
+	SCMI_CLOCK_CFG_IMX_SSC = 0x80,
 	SCMI_CLOCK_CFG_OEM_END = 0xFF,
 };
 
+#define SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK	GENMASK(7, 0)
+#define SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK		GENMASK(23, 8)
+#define SCMI_CLOCK_IMX_SS_ENABLE_MASK		BIT(24)
+
 /**
  * struct scmi_clk_proto_ops - represents the various operations provided
  *	by SCMI Clock Protocol

-- 
2.37.1

Re: [PATCH v4 4/5] clk: Add KUnit tests for assigned-clock-sscs

[Brian Masney]

On Mon, Sep 15, 2025 at 04:29:38PM +0800, Peng Fan wrote:
> Spread spectrum configuration is part of clock frequency settings,
> and its behavior can be validated similarly to assigned clock rates.
> 
> Extend the existing KUnit tests for assigned-clock-rates to cover
> assigned-clock-sscs by reusing the test framework. Add new test
> device trees:
>   - kunit_clk_assigned_sscs_null.dtso
>   - kunit_clk_assigned_sscs_null_consumer.dtso
>   - kunit_clk_assigned_sscs_without.dtso
>   - kunit_clk_assigned_sscs_without_consumer.dtso
>   - kunit_clk_assigned_sscs_zero.dtso
>   - kunit_clk_assigned_sscs_zero_consumer.dtso
> 
> These tests cover various invalid configurations of assigned-clock-sscs,
> ensuring robustness and consistent error handling, similar to the coverage
> provided for assigned-clock-rates.
> 
> Co-developed-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

This looks good to me. I'd typically leave a Reviewed-by, but that's
probably not appropriate since I'm already listed with a
Co-developed-by.

Brian

Re: [PATCH v4 2/5] clk: Introduce clk_hw_set_spread_spectrum

[Stephen Boyd]

Quoting Peng Fan (2025-09-15 01:29:36)
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b821b2cdb155331c85fafbd2fac8ab3703a08e4d..06db8918a1b35e3280e565272bc4603a88295a92 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2802,6 +2802,32 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
>  }
>  EXPORT_SYMBOL_GPL(clk_set_max_rate);
>  
> +int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf)
> +{
> +       struct clk_core *core;
> +       int ret;
> +
> +       if (!hw)
> +               return 0;
> +
> +       core = hw->core;
> +
> +       clk_prepare_lock();
> +
> +       ret = clk_pm_runtime_get(core);
> +       if (ret)
> +               goto fail;
> +
> +       if (core->ops->set_spread_spectrum)
> +               ret = core->ops->set_spread_spectrum(hw, conf);
> +
> +       clk_pm_runtime_put(core);
> +
> +fail:
> +       clk_prepare_unlock();
> +       return ret;
> +}

Does it need to be exported? 

> +
>  /**
>   * clk_get_parent - return the parent of a clk
>   * @clk: the clk whose parent gets returned
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 630705a47129453c241f1b1755f2c2f2a7ed8f77..4f48a4df95a1c54638a0e91e0a449fcc8aa40b80 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -84,6 +84,19 @@ struct clk_duty {
>         unsigned int den;
>  };
>  
> +/**
> + * struct clk_spread_spectrum - Structure encoding spread spectrum of a clock
> + *
> + * @modfreq_hz:                Modulation frequency
> + * @spread_bp:         Modulation percent in permyriad
> + * @method:            Modulation method
> + */
> +struct clk_spread_spectrum {
> +       u32 modfreq_hz;
> +       u32 spread_bp;
> +       u32 method;

What are the possible values of 'method'? I'm guessing it's the defines
in the dt-bindings header? Please connect these two somehow, maybe
through an enum. Also, why are these u32? Shouldn't these be more common
types like unsigned long or unsigned long long instead being exactly 32
bits?

> +};
> +
>  /**
>   * struct clk_ops -  Callback operations for hardware clocks; these are to
>   * be provided by the clock implementation, and will be called by drivers
> @@ -178,6 +191,12 @@ struct clk_duty {
>   *             separately via calls to .set_parent and .set_rate.
>   *             Returns 0 on success, -EERROR otherwise.
>   *
> + * @set_spread_spectrum: Optional callback used to configure the spread
> + *             spectrum modulation frequency, percentage, and method
> + *             to reduce EMI by spreading the clock frequency over a
> + *             wider range.
> + *             Returns 0 on success, -EERROR otherwise.
> + *
>   * @recalc_accuracy: Recalculate the accuracy of this clock. The clock accuracy
>   *             is expressed in ppb (parts per billion). The parent accuracy is
>   *             an input parameter.
> @@ -255,6 +274,8 @@ struct clk_ops {
>         int             (*set_rate_and_parent)(struct clk_hw *hw,
>                                     unsigned long rate,
>                                     unsigned long parent_rate, u8 index);
> +       int             (*set_spread_spectrum)(struct clk_hw *hw,
> +                                              struct clk_spread_spectrum *clk_ss);

const clk_ss pointer? And is it actually 'conf' or 'ss_conf'?

>         unsigned long   (*recalc_accuracy)(struct clk_hw *hw,
>                                            unsigned long parent_accuracy);
>         int             (*get_phase)(struct clk_hw *hw);
> @@ -1430,6 +1451,7 @@ void clk_hw_get_rate_range(struct clk_hw *hw, unsigned long *min_rate,
>                            unsigned long *max_rate);
>  void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
>                            unsigned long max_rate);
> +int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf);

const conf? And 'ss_conf' again?

Re: [PATCH v4 2/5] clk: Introduce clk_hw_set_spread_spectrum

[Peng Fan]

Hi Stephen,

On Sun, Sep 21, 2025 at 01:53:34PM -0700, Stephen Boyd wrote:
>Quoting Peng Fan (2025-09-15 01:29:36)
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index b821b2cdb155331c85fafbd2fac8ab3703a08e4d..06db8918a1b35e3280e565272bc4603a88295a92 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -2802,6 +2802,32 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
>>  }
>>  EXPORT_SYMBOL_GPL(clk_set_max_rate);
>>  
>> +int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf)
>> +{
...
>> +       return ret;
>> +}
>
>Does it need to be exported? 

Yeah. it should be exported, I will add it in v5.

>
>> +
>>  /**
>>   * clk_get_parent - return the parent of a clk
>>   * @clk: the clk whose parent gets returned
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 630705a47129453c241f1b1755f2c2f2a7ed8f77..4f48a4df95a1c54638a0e91e0a449fcc8aa40b80 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -84,6 +84,19 @@ struct clk_duty {
>>         unsigned int den;
>>  };
>>  
>> +/**
>> + * struct clk_spread_spectrum - Structure encoding spread spectrum of a clock
>> + *
>> + * @modfreq_hz:                Modulation frequency
>> + * @spread_bp:         Modulation percent in permyriad
>> + * @method:            Modulation method
>> + */
>> +struct clk_spread_spectrum {
>> +       u32 modfreq_hz;
>> +       u32 spread_bp;
>> +       u32 method;
>
>What are the possible values of 'method'? I'm guessing it's the defines
>in the dt-bindings header?

Yes. The values are in dt-bindings header and aligned with dt-schema.

>Please connect these two somehow, maybe
>through an enum.

ok. I will add back enum in V5.

>Also, why are these u32? Shouldn't these be more common
>types like unsigned long or unsigned long long instead being exactly 32
>bits?

u32 is large enough for modfreq_hz and spread_bp.

modfreq_hz will not exceed a few MHz, normally [1-9] * 10KHz per my
understanding. 

>
>> +};
>> +
>>  /**
>>   * struct clk_ops -  Callback operations for hardware clocks; these are to
>>   * be provided by the clock implementation, and will be called by drivers
>> @@ -178,6 +191,12 @@ struct clk_duty {
>>   *             separately via calls to .set_parent and .set_rate.
>>   *             Returns 0 on success, -EERROR otherwise.
>>   *
>> + * @set_spread_spectrum: Optional callback used to configure the spread
>> + *             spectrum modulation frequency, percentage, and method
>> + *             to reduce EMI by spreading the clock frequency over a
>> + *             wider range.
>> + *             Returns 0 on success, -EERROR otherwise.
>> + *
>>   * @recalc_accuracy: Recalculate the accuracy of this clock. The clock accuracy
>>   *             is expressed in ppb (parts per billion). The parent accuracy is
>>   *             an input parameter.
>> @@ -255,6 +274,8 @@ struct clk_ops {
>>         int             (*set_rate_and_parent)(struct clk_hw *hw,
>>                                     unsigned long rate,
>>                                     unsigned long parent_rate, u8 index);
>> +       int             (*set_spread_spectrum)(struct clk_hw *hw,
>> +                                              struct clk_spread_spectrum *clk_ss);
>
>const clk_ss pointer? And is it actually 'conf' or 'ss_conf'?

I will use "const clk_spread_spectrum *ss_conf" in v5.

>
>>         unsigned long   (*recalc_accuracy)(struct clk_hw *hw,
>>                                            unsigned long parent_accuracy);
>>         int             (*get_phase)(struct clk_hw *hw);
>> @@ -1430,6 +1451,7 @@ void clk_hw_get_rate_range(struct clk_hw *hw, unsigned long *min_rate,
>>                            unsigned long *max_rate);
>>  void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
>>                            unsigned long max_rate);
>> +int clk_hw_set_spread_spectrum(struct clk_hw *hw, struct clk_spread_spectrum *conf);
>
>const conf? And 'ss_conf' again?

yeah. Fix in v5.

Thanks,
Peng

Re: [PATCH v4 1/5] dt-bindings: clock: Add spread spectrum definition

[Rob Herring (Arm)]

On Mon, 15 Sep 2025 16:29:35 +0800, Peng Fan wrote:
> Per dt-schema, the modulation methods are: down-spread(3), up-spread(2),
> center-spread(1), no-spread(0). So define them in dt-bindings to avoid
> write the magic number in device tree.
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  include/dt-bindings/clock/clock.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Sebin Francis]

Hi Peng

On 15/09/25 13:59, Peng Fan wrote:
> The PLL clocks on NXP i.MX95 SoCs support Spread Spectrum (SS).
> This patch introduces scmi_clk_imx_set_spread_spectrum to pass SS
> configuration to the SCMI firmware, which handles the actual
> implementation.
> 
> To ensure this feature is only enabled on NXP platforms,
> scmi_clk_imx_extended_config_oem is added. Since SS is only applicable
> to PLL clocks, config_oem_get is used to verify SS support for a given
> clock.
> 
> i.MX95 SCMI firmware Spread Spectrum extConfigValue definition is as
> below, no modulation method because firmware forces to use down spread.
> 	 extConfigValue[7:0]   - spread percentage (%)
> 	 extConfigValue[23:8]  - Modulation Frequency (KHz)
> 	 extConfigValue[24]    - Enable/Disable
> 	 extConfigValue[31:25] - Reserved
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

...

>   	if (WARN_ON(feats_key >= db_size))
>   		return NULL;
>   
> @@ -459,7 +515,7 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
>   		 * to avoid sharing the devm_ allocated clk_ops between multiple
>   		 * SCMI clk driver instances.
>   		 */
> -		scmi_ops = scmi_clk_ops_select(sclk, transport_is_atomic,
> +		scmi_ops = scmi_clk_ops_select(handle, sclk, transport_is_atomic,
>   					       atomic_threshold_us,
>   					       scmi_clk_ops_db,
>   					       ARRAY_SIZE(scmi_clk_ops_db));
> diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> index aafaac1496b06a6e4f0ca32eee58a9edf7d4a70f..37f422b4b1ef2af2b4231a1677161aa24e07d0e2 100644
> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -80,9 +80,14 @@ enum scmi_clock_oem_config {
>   	SCMI_CLOCK_CFG_DUTY_CYCLE = 0x1,
>   	SCMI_CLOCK_CFG_PHASE,
>   	SCMI_CLOCK_CFG_OEM_START = 0x80,
> +	SCMI_CLOCK_CFG_IMX_SSC = 0x80,

TI is also planning to implement the same in our upcoming platform. so 
can we use a generic ID instead of vender specfic message ID?

>   	SCMI_CLOCK_CFG_OEM_END = 0xFF,
>   };
>   
> +#define SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK	GENMASK(7, 0)
> +#define SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK		GENMASK(23, 8)
> +#define SCMI_CLOCK_IMX_SS_ENABLE_MASK		BIT(24)
> +
>   /**
>    * struct scmi_clk_proto_ops - represents the various operations provided
>    *	by SCMI Clock Protocol
> 

Thanks
Sebin

RE: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Peng Fan]

Hi Sebin,

> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> NXP i.MX95
> 
> > --- a/include/linux/scmi_protocol.h
> > +++ b/include/linux/scmi_protocol.h
> > @@ -80,9 +80,14 @@ enum scmi_clock_oem_config {
> >       SCMI_CLOCK_CFG_DUTY_CYCLE = 0x1,
> >       SCMI_CLOCK_CFG_PHASE,
> >       SCMI_CLOCK_CFG_OEM_START = 0x80,
> > +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> 
> TI is also planning to implement the same in our upcoming platform. so
> can we use a generic ID instead of vender specfic message ID?

I tried to push to new generic ID [1] in half a year ago, but in the end ARM
decided not to add generic ID for spread spectrum support.

To i.MX, it is too late to use a generic ID and waiting spec, i.MX firmware
has been public for quite some time and passed several external releases.
So I need to use what our firmware adds and spec allows: vendor
extension.

[1] https://lore.kernel.org/arm-scmi/Z8iKErarE0lHWxEy@pluto/

Regards,
Peng.

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Sebin Francis]

Hi Peng,

On 23/09/25 17:27, Peng Fan wrote:
> Hi Sebin,
> 
>> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
>> NXP i.MX95
>>
>>> --- a/include/linux/scmi_protocol.h
>>> +++ b/include/linux/scmi_protocol.h
>>> @@ -80,9 +80,14 @@ enum scmi_clock_oem_config {
>>>        SCMI_CLOCK_CFG_DUTY_CYCLE = 0x1,
>>>        SCMI_CLOCK_CFG_PHASE,
>>>        SCMI_CLOCK_CFG_OEM_START = 0x80,
>>> +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
>>
>> TI is also planning to implement the same in our upcoming platform. so
>> can we use a generic ID instead of vender specfic message ID?
> 
> I tried to push to new generic ID [1] in half a year ago, but in the end ARM
> decided not to add generic ID for spread spectrum support.
> 
> To i.MX, it is too late to use a generic ID and waiting spec, i.MX firmware
> has been public for quite some time and passed several external releases.
> So I need to use what our firmware adds and spec allows: vendor
> extension.

Thanks for the quick response,
Since this implementation is specific to i.MX, can you move this to a 
vendor specific file, so that it will not break i.MX's firmware and TI 
can implement SSC in TI specific file.

> 
> [1] https://lore.kernel.org/arm-scmi/Z8iKErarE0lHWxEy@pluto/
> 
> Regards,
> Peng.

Thanks
Sebin

RE: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Peng Fan]

> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> NXP i.MX95
...
> >>>        SCMI_CLOCK_CFG_OEM_START = 0x80,
> >>> +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> >>
> >> TI is also planning to implement the same in our upcoming platform.
> >> so can we use a generic ID instead of vender specfic message ID?
> >
> > I tried to push to new generic ID [1] in half a year ago, but in the
> > end ARM decided not to add generic ID for spread spectrum support.
> >
> > To i.MX, it is too late to use a generic ID and waiting spec, i.MX
> > firmware has been public for quite some time and passed several
> external releases.
> > So I need to use what our firmware adds and spec allows: vendor
> > extension.
> 
> Thanks for the quick response,
> Since this implementation is specific to i.MX, can you move this to a
> vendor specific file, so that it will not break i.MX's firmware and TI can
> implement SSC in TI specific file.

i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
both supports SCMI PINCTRL. Current linux scmi does not support
both drivers built in kernel image, because scmi devlink issue.

Sudeep said he would address the devlink issue in 6.19 cycle.

Given the current situation, I'm hesitant to introduce a new driver
saying clk-imx-scmi.c.

What I'm unclear about is whether moving to a vendor-specific file
implies creating a new driver (i.e., clk-imx-scmi.c), or if it could be
handled via a callback or another mechanism. Could you help
clarify the intended direction?

Sudeep, Cristian

Would you please jump in and say something?

Thanks,
Peng.

> 
> >
> > [1]
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> lore
> > .kernel.org%2Farm-
> scmi%2FZ8iKErarE0lHWxEy%40pluto%2F&data=05%7C02%7Cpe
> >
> ng.fan%40nxp.com%7C4bd4c6b93ff146fbfc4c08ddfb4c5ab5%7C686ea
> 1d3bc2b4c6f
> >
> a92cd99c5c301635%7C0%7C0%7C638943027585856649%7CUnknow
> n%7CTWFpbGZsb3d8
> >
> eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
> sIkFOIjoiTW
> >
> FpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=zEREeXuiaC5wWS
> C%2FUAIe6wfiTf
> > MG5AEY5vZ7D4OEIGI%3D&reserved=0
> >
> > Regards,
> > Peng.
> 
> Thanks
> Sebin

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Sebin Francis]

Hi Peng,

On 24/09/25 17:13, Peng Fan wrote:
>> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
>> NXP i.MX95
> ...
>>>>>         SCMI_CLOCK_CFG_OEM_START = 0x80,
>>>>> +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
>>>>
>>>> TI is also planning to implement the same in our upcoming platform.
>>>> so can we use a generic ID instead of vender specfic message ID?
>>>
>>> I tried to push to new generic ID [1] in half a year ago, but in the
>>> end ARM decided not to add generic ID for spread spectrum support.
>>>
>>> To i.MX, it is too late to use a generic ID and waiting spec, i.MX
>>> firmware has been public for quite some time and passed several
>> external releases.
>>> So I need to use what our firmware adds and spec allows: vendor
>>> extension.
>>
>> Thanks for the quick response,
>> Since this implementation is specific to i.MX, can you move this to a
>> vendor specific file, so that it will not break i.MX's firmware and TI can
>> implement SSC in TI specific file.
> 
> i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
> both supports SCMI PINCTRL. Current linux scmi does not support
> both drivers built in kernel image, because scmi devlink issue.
> 
> Sudeep said he would address the devlink issue in 6.19 cycle.
> 
> Given the current situation, I'm hesitant to introduce a new driver
> saying clk-imx-scmi.c.
> 
> What I'm unclear about is whether moving to a vendor-specific file
> implies creating a new driver (i.e., clk-imx-scmi.c), or if it could be
> handled via a callback or another mechanism. Could you help
> clarify the intended direction?

My intended was to handle it via callback or something similar, so that 
TI can its own callback for the TI's SSC implementation.

> 
> Sudeep, Cristian
> 
> Would you please jump in and say something?
> 
> Thanks,
> Peng.
> 
>>
>>>
>>> [1]
>>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>> lore
>>> .kernel.org%2Farm-
>> scmi%2FZ8iKErarE0lHWxEy%40pluto%2F&data=05%7C02%7Cpe
>>>
>> ng.fan%40nxp.com%7C4bd4c6b93ff146fbfc4c08ddfb4c5ab5%7C686ea
>> 1d3bc2b4c6f
>>>
>> a92cd99c5c301635%7C0%7C0%7C638943027585856649%7CUnknow
>> n%7CTWFpbGZsb3d8
>>>
>> eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
>> sIkFOIjoiTW
>>>
>> FpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=zEREeXuiaC5wWS
>> C%2FUAIe6wfiTf
>>> MG5AEY5vZ7D4OEIGI%3D&reserved=0
>>>
>>> Regards,
>>> Peng.
>>
>> Thanks
>> Sebin

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Cristian Marussi]

On Wed, Sep 24, 2025 at 05:45:32PM +0530, Sebin Francis wrote:
> Hi Peng,

Hi ,

> 
> On 24/09/25 17:13, Peng Fan wrote:
> > > Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> > > NXP i.MX95
> > ...
> > > > > >         SCMI_CLOCK_CFG_OEM_START = 0x80,
> > > > > > +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> > > > > 
> > > > > TI is also planning to implement the same in our upcoming platform.
> > > > > so can we use a generic ID instead of vender specfic message ID?
> > > > 
> > > > I tried to push to new generic ID [1] in half a year ago, but in the
> > > > end ARM decided not to add generic ID for spread spectrum support.
> > > > 
> > > > To i.MX, it is too late to use a generic ID and waiting spec, i.MX
> > > > firmware has been public for quite some time and passed several
> > > external releases.
> > > > So I need to use what our firmware adds and spec allows: vendor
> > > > extension.
> > > 
> > > Thanks for the quick response,
> > > Since this implementation is specific to i.MX, can you move this to a
> > > vendor specific file, so that it will not break i.MX's firmware and TI can
> > > implement SSC in TI specific file.
> > 
> > i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
> > both supports SCMI PINCTRL. Current linux scmi does not support
> > both drivers built in kernel image, because scmi devlink issue.
> >

Yes indeed, BUT the vendor protocol extensions mechanism was meant to
serve the development of vendor custom protocols and drivers, it was
NEVER meant really to allow multiple alternative drivers implementation
on top of the same standard protocols like it happened with pinctrl-imx-scmi...
 
> > Sudeep said he would address the devlink issue in 6.19 cycle.
> > 
> > Given the current situation, I'm hesitant to introduce a new driver
> > saying clk-imx-scmi.c.
> >

Even if the devlink issues will be solved, in THIS case the problem is
handling custom vendor extensions inside a standard protocol, as it is
allowed in this case...

> > What I'm unclear about is whether moving to a vendor-specific file
> > implies creating a new driver (i.e., clk-imx-scmi.c), or if it could be
> > handled via a callback or another mechanism. Could you help
> > clarify the intended direction?
> 
> My intended was to handle it via callback or something similar, so that TI
> can its own callback for the TI's SSC implementation.
> 

This is exactly what is needed, the ability to extend with vendor
extensions callback the behavior of a standard protocol where
allowed....this is NOT currently supported and sincerely that was the
reason months ago I proposed initially that maybe we could have standardized
a new common clock extension SSC instead of using the OEM extensions since:

1. it seemed a pretty generic operation
2. any per-vendor extension callback of std protocol was NOT ready :P

...then this proposal never went anywhere with ATG...AND now looking at
this thread I think that it is good at the end that we did NOT add a new
standard extended clock config instead of the IMX OEM, since now it
seems that TI wants its own non-compatible implementation...

So yes the ideal solutiomn would be to extend in a generic way the SCMI
framework so that you can add in these cases custom handling of vendor
extensions for standard protocols (and then generalize the current
clk-scmi IMX support and add the new TI one...)...but I have not thought
about this and I certainly dont have enough bandwidth now to work on
this...beside having already in the pipeline other stuff/fixes like
a proper fix for vendor drivers coex like Peng askes (rightly so a few
months ago)

Thanks,
Cristian

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Sudeep Holla]

Hi Cristian,

On Wed, Sep 24, 2025 at 01:40:56PM +0100, Cristian Marussi wrote:
> On Wed, Sep 24, 2025 at 05:45:32PM +0530, Sebin Francis wrote:
> > Hi Peng,
> 
> Hi ,
> 
> > 
> > On 24/09/25 17:13, Peng Fan wrote:
> > > > Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> > > > NXP i.MX95
> > > ...
> > > > > > >         SCMI_CLOCK_CFG_OEM_START = 0x80,
> > > > > > > +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> > > > > > 
> > > > > > TI is also planning to implement the same in our upcoming platform.
> > > > > > so can we use a generic ID instead of vender specfic message ID?
> > > > > 
> > > > > I tried to push to new generic ID [1] in half a year ago, but in the
> > > > > end ARM decided not to add generic ID for spread spectrum support.
> > > > > 
> > > > > To i.MX, it is too late to use a generic ID and waiting spec, i.MX
> > > > > firmware has been public for quite some time and passed several
> > > > external releases.
> > > > > So I need to use what our firmware adds and spec allows: vendor
> > > > > extension.
> > > > 
> > > > Thanks for the quick response,
> > > > Since this implementation is specific to i.MX, can you move this to a
> > > > vendor specific file, so that it will not break i.MX's firmware and TI can
> > > > implement SSC in TI specific file.
> > > 
> > > i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
> > > both supports SCMI PINCTRL. Current linux scmi does not support
> > > both drivers built in kernel image, because scmi devlink issue.
> > >
> 
> Yes indeed, BUT the vendor protocol extensions mechanism was meant to
> serve the development of vendor custom protocols and drivers, it was
> NEVER meant really to allow multiple alternative drivers implementation
> on top of the same standard protocols like it happened with pinctrl-imx-scmi...
>  
> > > Sudeep said he would address the devlink issue in 6.19 cycle.
> > > 
> > > Given the current situation, I'm hesitant to introduce a new driver
> > > saying clk-imx-scmi.c.
> > >
> 
> Even if the devlink issues will be solved, in THIS case the problem is
> handling custom vendor extensions inside a standard protocol, as it is
> allowed in this case...
> 
> > > What I'm unclear about is whether moving to a vendor-specific file
> > > implies creating a new driver (i.e., clk-imx-scmi.c), or if it could be
> > > handled via a callback or another mechanism. Could you help
> > > clarify the intended direction?
> > 
> > My intended was to handle it via callback or something similar, so that TI
> > can its own callback for the TI's SSC implementation.
> > 
> 
> This is exactly what is needed, the ability to extend with vendor
> extensions callback the behavior of a standard protocol where
> allowed....this is NOT currently supported and sincerely that was the
> reason months ago I proposed initially that maybe we could have standardized
> a new common clock extension SSC instead of using the OEM extensions since:
> 
> 1. it seemed a pretty generic operation
> 2. any per-vendor extension callback of std protocol was NOT ready :P
> 
> ...then this proposal never went anywhere with ATG...AND now looking at
> this thread I think that it is good at the end that we did NOT add a new
> standard extended clock config instead of the IMX OEM, since now it
> seems that TI wants its own non-compatible implementation...
> 
> So yes the ideal solutiomn would be to extend in a generic way the SCMI
> framework so that you can add in these cases custom handling of vendor
> extensions for standard protocols (and then generalize the current
> clk-scmi IMX support and add the new TI one...)...but I have not thought
> about this and I certainly dont have enough bandwidth now to work on
> this...beside having already in the pipeline other stuff/fixes like
> a proper fix for vendor drivers coex like Peng askes (rightly so a few
> months ago)
> 

Thanks for the detailed response as usual 😄. I don't have much to add
in case anyone is expecting different or more info from me.

-- 
Regards,
Sudeep

Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Sudeep Holla]

On Wed, Sep 24, 2025 at 11:43:56AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> > NXP i.MX95
> ...
> > >>>        SCMI_CLOCK_CFG_OEM_START = 0x80,
> > >>> +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> > >>
> > >> TI is also planning to implement the same in our upcoming platform.
> > >> so can we use a generic ID instead of vender specfic message ID?
> > >
> > > I tried to push to new generic ID [1] in half a year ago, but in the
> > > end ARM decided not to add generic ID for spread spectrum support.
> > >
> > > To i.MX, it is too late to use a generic ID and waiting spec, i.MX
> > > firmware has been public for quite some time and passed several
> > external releases.
> > > So I need to use what our firmware adds and spec allows: vendor
> > > extension.
> > 
> > Thanks for the quick response,
> > Since this implementation is specific to i.MX, can you move this to a
> > vendor specific file, so that it will not break i.MX's firmware and TI can
> > implement SSC in TI specific file.
> 
> i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
> both supports SCMI PINCTRL. Current linux scmi does not support
> both drivers built in kernel image, because scmi devlink issue.
> 
> Sudeep said he would address the devlink issue in 6.19 cycle.
>

Yes it is a different issue IMO and nothing related to this.

> Given the current situation, I'm hesitant to introduce a new driver
> saying clk-imx-scmi.c.
>

Yes please don't, and I don't see a strong reason for that(yet).

Unlike vendor protocol, there is no way we can no upfront how the vendors
can use this in their own colourful way. So I am not sure if we start
building something generic from the start or refactor as more vendors start
using it. Hard to decide 🙁. Lets see, need to think a bit.

If Peng or Sebin or others have some idea, please propose or start the
discussion so that we can evaluate the approach.

-- 
Regards,
Sudeep

RE: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Peng Fan]

Hi Cristian,

> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> NXP i.MX95
>
...
 
> 
> So yes the ideal solutiomn would be to extend in a generic way the
> SCMI framework so that you can add in these cases custom handling of
> vendor extensions for standard protocols (and then generalize the
> current clk-scmi IMX support and add the new TI one...)...but I have not
> thought about this and I certainly dont have enough bandwidth now to
> work on this

I see. So I get the point IMX SSC OEM is allowed, but need an elegant way
to extend SCMI framework or extend clk-scmi.c in an elegant way to make
it easier to support other platforms. 

...beside having already in the pipeline other stuff/fixes like
> a proper fix for vendor drivers coex like Peng askes (rightly so a few
> months ago)

Thanks for working on that.

I could continue give a look on this, if any suggestions, I would appreciate
that.

Thanks
Peng

> 
> Thanks,
> Cristian

RE: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for NXP i.MX95

[Peng Fan]

Hi Sudeep,

> Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> NXP i.MX95
> 
> On Wed, Sep 24, 2025 at 11:43:56AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH v4 5/5] clk: scmi: Support Spread Spectrum for
> > > NXP i.MX95
> > ...
> > > >>>        SCMI_CLOCK_CFG_OEM_START = 0x80,
> > > >>> +     SCMI_CLOCK_CFG_IMX_SSC = 0x80,
> > > >>
> > > >> TI is also planning to implement the same in our upcoming
> platform.
> > > >> so can we use a generic ID instead of vender specfic message ID?
> > > >
> > > > I tried to push to new generic ID [1] in half a year ago, but in
> > > > the end ARM decided not to add generic ID for spread spectrum
> support.
> > > >
> > > > To i.MX, it is too late to use a generic ID and waiting spec, i.MX
> > > > firmware has been public for quite some time and passed several
> > > external releases.
> > > > So I need to use what our firmware adds and spec allows: vendor
> > > > extension.
> > >
> > > Thanks for the quick response,
> > > Since this implementation is specific to i.MX, can you move this to
> > > a vendor specific file, so that it will not break i.MX's firmware
> > > and TI can implement SSC in TI specific file.
> >
> > i.MX has encountered issue with pinctrl-scmi.c and pinctrl-imx-scmi.c
> > both supports SCMI PINCTRL. Current linux scmi does not support
> both
> > drivers built in kernel image, because scmi devlink issue.
> >
> > Sudeep said he would address the devlink issue in 6.19 cycle.
> >
> 
> Yes it is a different issue IMO and nothing related to this.
> 
> > Given the current situation, I'm hesitant to introduce a new driver
> > saying clk-imx-scmi.c.
> >
> 
> Yes please don't, and I don't see a strong reason for that(yet).
> 
> Unlike vendor protocol, there is no way we can no upfront how the
> vendors can use this in their own colourful way. So I am not sure if we
> start building something generic from the start or refactor as more
> vendors start using it. Hard to decide 🙁. Lets see, need to think a bit.
> 
> If Peng or Sebin or others have some idea, please propose or start the
> discussion so that we can evaluate the approach.

I would give a look and hope there is an output. I will start
a new thread to discuss.

Just in my mind
Each standard protocol has its own OEM or vendor description, it might
be hard to use a generic way to support all standard protocols.

It might be a bit easier to use per protocol extension.

Anyway, let's discuss in the new thread with potential solutions.

Thanks
Peng. 

> 
> --
> Regards,
> Sudeep

RE: [PATCH v4 0/5] clk: Support spread spectrum and use it in clk-scmi

[Peng Fan]

Hi Stephen,

> Subject: [PATCH v4 0/5] clk: Support spread spectrum and use it in clk-
> scmi

Since clk-scmi.c for spread spectrum support needs some big changes,
we may need to change scmi framework to make OEM extension
in an elegant way. This will needs some time.

To patch 1-4, do you think is it ok to be in linux tree without patch 5?
If yes, I will post V5 soon with your comments in patch 2 addressed.
Otherwise I have to carry patch 1-4 in future clk-scmi patches.

Thanks,
Peng.

> 
> Since the assigned-clock-sscs property [1] has been accepted into the
> device tree schema, we can now support it in the Linux clock driver.
> Therefore, I’ve picked up the previously submitted work [2] titled “clk:
> Support spread spectrum and use it in clk-pll144x and clk-scmi.”
> As more than six months have passed since [2] was posted, I’m treating
> this patchset as a new submission rather than a v3.
> 
> - Introduce clk_set_spread_spectrum to set the parameters for enabling
>   spread spectrum of a clock.
> - Parse 'assigned-clock-sscs' and configure it by default before using the
>   clock. This property is parsed before parsing clock rate.
> - Enable this feature for clk-scmi on i.MX95.
> 
> Because SCMI spec will not include spread spectrum as a standard
> extension, we still need to use NXP i.MX OEM extension.
> 
> [1] https://github.com/devicetree-org/dt-schema/pull/154
> [2] https://lore.kernel.org/all/20250205-clk-ssc-v2-0-
> fa73083caa92@nxp.com/
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Changes in v4:
> - Add R-b for patch 1 from Brian
> - Drop unecessary change in patch 4 Per Brian
> - Link to v3: https://lore.kernel.org/r/20250912-clk-ssc-version1-v3-0-
> fd1e07476ba1@nxp.com
> 
> Changes in v3:
> - New patch 1 for dt-bindings per comment from Brian
>   https://lore.kernel.org/all/aLeEFzXkPog_dt2B@x1/
>   This might not be good to add a new dt-binding file in v3. But this is
>   quite a simple file that just has four macros to encode modulation
>   method. So hope this is fine for DT maintainers.
> - Add Brain's R-b for patch 2
> - New patch 3 to add Kunit test per Brain. Since Brain helped
>   draft part of the code, I added Co-developed-by tag from Brain.
> - Link to v2: https://lore.kernel.org/r/20250901-clk-ssc-version1-v2-0-
> 1d0a486dffe6@nxp.com
> 
> Changes in v2:
> - Simplify the code in patch 2 per Dan Carpenter and Brian Masney
> - Rebased to next-20250829
> - Link to v1: https://lore.kernel.org/r/20250812-clk-ssc-version1-v1-0-
> cef60f20d770@nxp.com
> 
> ---
> Peng Fan (5):
>       dt-bindings: clock: Add spread spectrum definition
>       clk: Introduce clk_hw_set_spread_spectrum
>       clk: conf: Support assigned-clock-sscs
>       clk: Add KUnit tests for assigned-clock-sscs
>       clk: scmi: Support Spread Spectrum for NXP i.MX95
> 
>  drivers/clk/Makefile                               |   6 +
>  drivers/clk/clk-conf.c                             |  69 ++++++++++++
>  drivers/clk/clk-scmi.c                             |  64 ++++++++++-
>  drivers/clk/clk.c                                  |  26 +++++
>  drivers/clk/clk_test.c                             | 121 ++++++++++++++++++++-
>  drivers/clk/kunit_clk_assigned_rates.h             |  10 ++
>  drivers/clk/kunit_clk_assigned_rates_multiple.dtso |   6 +
>  ...kunit_clk_assigned_rates_multiple_consumer.dtso |   6 +
>  drivers/clk/kunit_clk_assigned_rates_one.dtso      |   3 +
>  .../clk/kunit_clk_assigned_rates_one_consumer.dtso |   3 +
>  .../clk/kunit_clk_assigned_rates_u64_multiple.dtso |   6 +
>  ...t_clk_assigned_rates_u64_multiple_consumer.dtso |   6 +
>  drivers/clk/kunit_clk_assigned_rates_u64_one.dtso  |   3 +
>  .../kunit_clk_assigned_rates_u64_one_consumer.dtso |   3 +
>  drivers/clk/kunit_clk_assigned_sscs_null.dtso      |  16 +++
>  .../clk/kunit_clk_assigned_sscs_null_consumer.dtso |  20 ++++
>  drivers/clk/kunit_clk_assigned_sscs_without.dtso   |  15 +++
>  .../kunit_clk_assigned_sscs_without_consumer.dtso  |  19 ++++
>  drivers/clk/kunit_clk_assigned_sscs_zero.dtso      |  12 ++
>  .../clk/kunit_clk_assigned_sscs_zero_consumer.dtso |  16 +++
>  include/dt-bindings/clock/clock.h                  |  14 +++
>  include/linux/clk-provider.h                       |  22 ++++
>  include/linux/scmi_protocol.h                      |   5 +
>  23 files changed, 464 insertions(+), 7 deletions(-)
> ---
> base-commit: 8941e75c0f122fdd76dc54ed45c4ce917587e006
> change-id: 20250812-clk-ssc-version1-acf6f6efbd96
> 
> Best regards,
> --
> Peng Fan <peng.fan@nxp.com>

Re: [PATCH v4 0/5] clk: Support spread spectrum and use it in clk-scmi

[Brian Masney]

On Wed, Sep 24, 2025 at 02:44:21PM +0000, Peng Fan wrote:
> Hi Stephen,
> 
> > Subject: [PATCH v4 0/5] clk: Support spread spectrum and use it in clk-
> > scmi
> 
> Since clk-scmi.c for spread spectrum support needs some big changes,
> we may need to change scmi framework to make OEM extension
> in an elegant way. This will needs some time.
> 
> To patch 1-4, do you think is it ok to be in linux tree without patch 5?
> If yes, I will post V5 soon with your comments in patch 2 addressed.
> Otherwise I have to carry patch 1-4 in future clk-scmi patches.

This adds a new API, and there wouldn't be any users of it at this
point. Personally, I think it should probably wait and be merged as
one series. Unless there's another vendor that's ready to use this.

Brian

Re: [PATCH v4 3/5] clk: conf: Support assigned-clock-sscs

[Sebin Francis]

Hi Peng,

On 15/09/25 13:59, Peng Fan wrote:
> Parse the Spread Spectrum Configuration(SSC) from device tree and configure
> them before using the clock.
> 
> Each SSC is three u32 elements which means '<modfreq spreaddepth
> modmethod>', so assigned-clock-sscs is an array of multiple three u32
> elements.
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   drivers/clk/clk-conf.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 69 insertions(+)
> 
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a95655ce094a35b989c97ebc6fd8..dd6083597db3f8f27d86abf5640dfc3fb39a9b88 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -155,6 +155,71 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
>   	return 0;
>   }
>   
> +static int __set_clk_spread_spectrum(struct device_node *node, bool clk_supplier)
> +{
> +	struct clk_spread_spectrum *sscs __free(kfree) = NULL;
> +	u32 elem_size = sizeof(struct clk_spread_spectrum);
> +	struct of_phandle_args clkspec;
> +	int rc, count, index;
> +	struct clk *clk;
> +
> +	/* modfreq, spreadPercent, modmethod */
> +	count = of_property_count_elems_of_size(node, "assigned-clock-sscs", elem_size);
> +	if (count <= 0)
> +		return 0;
> +
> +	sscs = kcalloc(count, elem_size, GFP_KERNEL);
> +	if (!sscs)
> +		return -ENOMEM;
> +
> +	rc = of_property_read_u32_array(node, "assigned-clock-sscs", (u32 *)sscs,
> +					count * 3);
> +	if (rc)
> +		return rc;
> +
> +	for (index = 0; index < count; index++) {
> +		struct clk_spread_spectrum *conf = &sscs[index];
> +		struct clk_hw *hw;
> +
> +		if (!conf->modfreq_hz && !conf->spread_bp && !conf->method)
> +			continue;
> +
> +		rc = of_parse_phandle_with_args(node, "assigned-clocks", "#clock-cells",
> +						index, &clkspec);
> +		if (rc < 0) {
> +			/* skip empty (null) phandles */
> +			if (rc == -ENOENT)
> +				continue;
> +			else
> +				return rc;
> +		}
> +
> +		if (clkspec.np == node && !clk_supplier) {
> +			of_node_put(clkspec.np);
> +			return 0;
> +		}
> +
> +		clk = of_clk_get_from_provider(&clkspec);
> +		of_node_put(clkspec.np);
> +		if (IS_ERR(clk)) {
> +			if (PTR_ERR(clk) != -EPROBE_DEFER)
> +				pr_warn("clk: couldn't get clock %d for %pOF\n",
> +					index, node);
> +			return PTR_ERR(clk);
> +		}
> +
> +		hw = __clk_get_hw(clk);
> +		rc = clk_hw_set_spread_spectrum(hw, conf);
> +		if (rc < 0)
> +			pr_err("clk: couldn't set %s clk spread spectrum %u %u %u: %d\n",
> +			       __clk_get_name(clk), conf->modfreq_hz, conf->spread_bp,
> +			       conf->method, rc);
> +		clk_put(clk);
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * of_clk_set_defaults() - parse and set assigned clocks configuration
>    * @node: device node to apply clock settings for
> @@ -174,6 +239,10 @@ int of_clk_set_defaults(struct device_node *node, bool clk_supplier)
>   	if (!node)
>   		return 0;
>   
> +	rc = __set_clk_spread_spectrum(node, clk_supplier);
> +	if (rc < 0)
> +		return rc;
> +
>   	rc = __set_clk_parents(node, clk_supplier);
>   	if (rc < 0)
>   		return rc;
> 

Here you are setting the clock's ssc before the setting the parent and 
rate, is it possible to move it below setting of parent and rate? 
because the ssc is enabled after the parent and rate is set to a clock.

Thanks
Sebin

RE: [PATCH v4 3/5] clk: conf: Support assigned-clock-sscs

[Peng Fan]

> Subject: Re: [PATCH v4 3/5] clk: conf: Support assigned-clock-sscs
...
> >    * of_clk_set_defaults() - parse and set assigned clocks configuration
> >    * @node: device node to apply clock settings for @@ -174,6
> +239,10
> > @@ int of_clk_set_defaults(struct device_node *node, bool
> clk_supplier)
> >   	if (!node)
> >   		return 0;
> >
> > +	rc = __set_clk_spread_spectrum(node, clk_supplier);
> > +	if (rc < 0)
> > +		return rc;
> > +
> >   	rc = __set_clk_parents(node, clk_supplier);
> >   	if (rc < 0)
> >   		return rc;
> >
> 
> Here you are setting the clock's ssc before the setting the parent and
> rate, is it possible to move it below setting of parent and rate?
> because the ssc is enabled after the parent and rate is set to a clock.

The idea is setting ssc is to let driver/firmware latch the configuration,
and when set rate, the driver/firmware will configure the rate
with ssc settings. And this is what i.MX SCMI firmware implements.

Per my understanding, from hardware perspective, when setting rate,
ssc should be set together if need to enable ssc.

So TI's future platform may take NXP firmware's approach.

Thanks,
Peng. 

> 
> Thanks
> Sebin