* [PATCH v10 1/6] dt-bindings: clock: Add spread spectrum definition
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 2/6] clk: Introduce clk_hw_set_spread_spectrum Peng Fan (OSS)
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
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>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Sebin Francis <sebin.francis@ti.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 0000000000000..155e2653a120b
--- /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.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v10 2/6] clk: Introduce clk_hw_set_spread_spectrum
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 1/6] dt-bindings: clock: Add spread spectrum definition Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:54 ` sashiko-bot
2026-06-12 8:46 ` [PATCH v10 3/6] clk: conf: Support assigned-clock-sscs Peng Fan (OSS)
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
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>
Reviewed-by: Sebin Francis <sebin.francis@ti.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/clk/clk.c | 27 +++++++++++++++++++++++++++
include/linux/clk-provider.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 048adfa86a5d0..8c78621cde253 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2774,6 +2774,33 @@ 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, const struct clk_spread_spectrum *ss_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, ss_conf);
+
+ clk_pm_runtime_put(core);
+
+fail:
+ clk_prepare_unlock();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(clk_hw_set_spread_spectrum);
+
/**
* 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 b01a38fef8cf2..7d3747378739c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -6,6 +6,7 @@
#ifndef __LINUX_CLK_PROVIDER_H
#define __LINUX_CLK_PROVIDER_H
+#include <dt-bindings/clock/clock.h>
#include <linux/of.h>
#include <linux/of_clk.h>
@@ -84,6 +85,26 @@ struct clk_duty {
unsigned int den;
};
+enum clk_ssc_method {
+ CLK_SPREAD_NO = CLK_SSC_NO_SPREAD,
+ CLK_SPREAD_CENTER = CLK_SSC_CENTER_SPREAD,
+ CLK_SPREAD_UP = CLK_SSC_UP_SPREAD,
+ CLK_SPREAD_DOWN = CLK_SSC_DOWN_SPREAD,
+};
+
+/**
+ * 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;
+ enum clk_ssc_method method;
+};
+
/**
* struct clk_ops - Callback operations for hardware clocks; these are to
* be provided by the clock implementation, and will be called by drivers
@@ -174,6 +195,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.
@@ -249,6 +276,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,
+ const struct clk_spread_spectrum *ss_conf);
unsigned long (*recalc_accuracy)(struct clk_hw *hw,
unsigned long parent_accuracy);
int (*get_phase)(struct clk_hw *hw);
@@ -1436,6 +1465,8 @@ 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,
+ const struct clk_spread_spectrum *ss_conf);
static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v10 2/6] clk: Introduce clk_hw_set_spread_spectrum
2026-06-12 8:46 ` [PATCH v10 2/6] clk: Introduce clk_hw_set_spread_spectrum Peng Fan (OSS)
@ 2026-06-12 8:54 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-12 8:54 UTC (permalink / raw)
Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clk_hw_set_spread_spectrum() modifies clock hardware state without checking if the clock rate is exclusively protected.
- [Medium] clk_hw_set_spread_spectrum() silently returns success (0) when the clock provider does not implement the set_spread_spectrum callback.
--
commit fbe42efd1852814fc95559f8dd8be280e7cdb66e
Author: Peng Fan <peng.fan@nxp.com>
clk: Introduce clk_hw_set_spread_spectrum
This commit adds clk_hw_set_spread_spectrum() to configure the spread spectrum
feature of a clock. It also introduces the set_spread_spectrum callback for
clock drivers to provide hardware-specific implementations.
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 048adfa86a5d0..8c78621cde253 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
[ ... ]
> @@ -2774,6 +2774,33 @@ 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, const struct clk_spread_spectrum *ss_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;
> +
[Severity: High]
Should this check if the clock rate is exclusively protected before modifying
the hardware state?
Other clock adjustment functions like clk_set_phase() or clk_set_rate() check
clk_core_rate_is_protected(core) and return -EBUSY to prevent interference
with consumers that require a strict, stable clock. Since enabling spread
spectrum dynamically modulates the frequency, it seems like it could disrupt
consumers relying on that protection.
> + if (core->ops->set_spread_spectrum)
> + ret = core->ops->set_spread_spectrum(hw, ss_conf);
> +
[Severity: Medium]
If the clock provider does not implement the set_spread_spectrum callback,
will this silently return 0 (success) because ret was set to 0 by a
successful clk_pm_runtime_get(core)?
Returning success when the operation is unsupported might mislead callers into
believing the spread spectrum modulation is active. Does it make sense to
return an error code like -EOPNOTSUPP instead?
> + clk_pm_runtime_put(core);
> +
> +fail:
> + clk_prepare_unlock();
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(clk_hw_set_spread_spectrum);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-clk-v10-v10-0-eb92484eda38@nxp.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v10 3/6] clk: conf: Support assigned-clock-sscs
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 1/6] dt-bindings: clock: Add spread spectrum definition Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 2/6] clk: Introduce clk_hw_set_spread_spectrum Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:54 ` sashiko-bot
2026-06-12 8:46 ` [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs Peng Fan (OSS)
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
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>
Reviewed-by: Sebin Francis <sebin.francis@ti.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/clk/clk-conf.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 303a0bb26e54a..550b8ae375a2c 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -155,6 +155,78 @@ 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)
+{
+ u32 elem_size = sizeof(struct clk_spread_spectrum);
+ struct clk_spread_spectrum *sscs;
+ 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)
+ goto free_sscs;
+
+ 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) {
+ rc = 0;
+ continue;
+ } else
+ goto free_sscs;
+ }
+
+ if (clkspec.np == node && !clk_supplier) {
+ of_node_put(clkspec.np);
+ goto free_sscs;
+ }
+
+ 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);
+ rc = PTR_ERR(clk);
+ goto free_sscs;
+ }
+
+ 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);
+ /* Do not fail */
+ rc = 0;
+ }
+ clk_put(clk);
+ }
+
+free_sscs:
+ kfree(sscs);
+ return rc;
+}
+
/**
* of_clk_set_defaults() - parse and set assigned clocks configuration
* @node: device node to apply clock settings for
@@ -174,6 +246,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.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v10 3/6] clk: conf: Support assigned-clock-sscs
2026-06-12 8:46 ` [PATCH v10 3/6] clk: conf: Support assigned-clock-sscs Peng Fan (OSS)
@ 2026-06-12 8:54 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-12 8:54 UTC (permalink / raw)
Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Spread Spectrum Configuration (SSC) is applied before clock parents and rates are set, which can result in the SSC configuration being overwritten or becoming mathematically invalid.
- [Medium] Parsing `assigned-clock-sscs` by casting an array of `struct clk_spread_spectrum` to a `u32 *` violates strict typing and is highly brittle.
--
commit dcd88e4f08b6ae89c38dd70d8a94011d30708d37
Author: Peng Fan <peng.fan@nxp.com>
clk: conf: Support assigned-clock-sscs
This commit adds support for parsing Spread Spectrum Configuration (SSC) from
the device tree and configuring clocks accordingly. It introduces a new
function to read the assigned-clock-sscs property and apply the settings.
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a..550b8ae375a2c 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -155,6 +155,78 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
[ ... ]
> +static int __set_clk_spread_spectrum(struct device_node *node, bool clk_supplier)
> +{
[ ... ]
> + 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);
[Severity: Medium]
Is it safe to cast an array of struct clk_spread_spectrum to u32 * here?
If any fields are added to the struct or the compiler adds padding, the OF
helper will read data in 12-byte chunks, which will silently misalign with the
struct array boundaries for all elements after the first.
Would it be better to read into a dynamically allocated temporary u32 array
and explicitly populate each struct field?
> + if (rc)
> + goto free_sscs;
> +
[ ... ]
> @@ -174,6 +246,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);
[Severity: High]
Does configuring spread spectrum before setting clock parents and rates cause
issues?
Spread spectrum modulates the base frequency, so its hardware parameters
usually depend on the final base rate and parent clock. By configuring SSC
first, the settings might be computed based on the old hardware state, or
subsequent rate and parent changes might wipe out the SSC settings.
Should SSC be configured after parents and rates are set?
> if (rc < 0)
> return rc;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-clk-v10-v10-0-eb92484eda38@nxp.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
` (2 preceding siblings ...)
2026-06-12 8:46 ` [PATCH v10 3/6] clk: conf: Support assigned-clock-sscs Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:55 ` sashiko-bot
2026-06-12 8:46 ` [PATCH v10 5/6] clk: scmi: Introduce common header for SCMI clock interface Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 6/6] clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver Peng Fan (OSS)
5 siblings, 1 reply; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Add KUnit test coverage for the assigned-clock-sscs DT property that
configures spread spectrum on clocks before they are used.
Extend the existing test infrastructure to support spread spectrum:
- Add struct clk_spread_spectrum field to clk_dummy_context and a
clk_dummy_set_spread_spectrum callback
- Wire set_spread_spectrum into all dummy clock ops
- Extend clk_assigned_rates_register_clk and test parameter struct
to propagate initial SSCS values
Add a new separate test suite clk_assigned_sscs with three categories:
1. clk_assigned_sscs_assigns_one — verifies that a single
assigned-clock-sscs entry correctly configures spread spectrum
on one clock, testing both provider and consumer paths
2. clk_assigned_sscs_assigns_multiple — verifies that multiple
assigned-clock-sscs entries configure spread spectrum on two
clocks, testing both provider and consumer paths
3. clk_assigned_sscs_skips — verifies that malformed DT properties
are correctly skipped without error: missing assigned-clocks,
zero-valued SSCS, and null phandles, tested for both provider
and consumer scenarios
New DT overlays are added for all test scenarios:
- kunit_clk_assigned_sscs_one{,consumer} — single valid entry
- kunit_clk_assigned_sscs_multiple{,consumer} — two valid entries
- kunit_clk_assigned_sscs_without{,consumer} — missing assigned-clocks
- kunit_clk_assigned_sscs_zero{,consumer} — all-zero SSCS values
- kunit_clk_assigned_sscs_null{,consumer} — null phandle
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 | 10 +
drivers/clk/clk_test.c | 203 ++++++++++++++++++++-
drivers/clk/kunit_clk_assigned_rates.h | 10 +
.../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_multiple.dtso | 20 ++
.../kunit_clk_assigned_sscs_multiple_consumer.dtso | 24 +++
drivers/clk/kunit_clk_assigned_sscs_null.dtso | 16 ++
.../clk/kunit_clk_assigned_sscs_null_consumer.dtso | 20 ++
drivers/clk/kunit_clk_assigned_sscs_one.dtso | 16 ++
.../clk/kunit_clk_assigned_sscs_one_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, 416 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index cc108a75a9008..6a726331b6c9e 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -18,6 +18,16 @@ 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_one.dtbo.o \
+ kunit_clk_assigned_sscs_one_consumer.dtbo.o \
+ kunit_clk_assigned_sscs_multiple.dtbo.o \
+ kunit_clk_assigned_sscs_multiple_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 b1961daac5e22..824adc95e0b2f 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,
+ const struct clk_spread_spectrum *ss_conf)
+{
+ struct clk_dummy_context *ctx =
+ container_of(hw, struct clk_dummy_context, hw);
+
+ ctx->sscs = *ss_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));
@@ -3403,6 +3423,182 @@ static struct kunit_suite clk_assigned_rates_suite = {
.init = clk_assigned_rates_test_init,
};
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_one);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_one_consumer);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_multiple);
+OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_multiple_consumer);
+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);
+
+static void clk_assigned_sscs_assigns_one(struct kunit *test)
+{
+ struct clk_assigned_rates_context *ctx = test->priv;
+
+ 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);
+}
+
+/* Test cases that assign sscs for one clk */
+static const struct clk_assigned_rates_test_param clk_assigned_sscs_assigns_one_test_params[] = {
+ {
+ /*
+ * Test that a single cell assigned-clock-sscs property
+ * assigns the sscs when the property is in the provider.
+ */
+ .desc = "provider assigns",
+ TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_one),
+ },
+ {
+ /*
+ * Test that a single cell assigned-clock-sscs property
+ * assigns the sscs when the property is in the consumer.
+ */
+ .desc = "consumer assigns",
+ TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_one_consumer),
+ .consumer_test = true,
+ },
+};
+KUNIT_ARRAY_PARAM_DESC(clk_assigned_sscs_assigns_one,
+ clk_assigned_sscs_assigns_one_test_params, desc)
+
+static void clk_assigned_sscs_assigns_multiple(struct kunit *test)
+{
+ struct clk_assigned_rates_context *ctx = test->priv;
+
+ 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.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);
+}
+
+/* Test cases that assign sscs for multiple clks */
+static const
+struct clk_assigned_rates_test_param clk_assigned_sscs_assigns_multiple_test_params[] = {
+ {
+ /*
+ * Test that a multiple cell assigned-clock-sscs property
+ * assigns the sscs when the property is in the provider.
+ */
+ .desc = "provider assigns",
+ TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_multiple),
+ },
+ {
+ /*
+ * Test that a multiple cell assigned-clock-sscs property
+ * assigns the sscs when the property is in the consumer.
+ */
+ .desc = "consumer assigns",
+ TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_multiple_consumer),
+ .consumer_test = true,
+ },
+};
+KUNIT_ARRAY_PARAM_DESC(clk_assigned_sscs_assigns_multiple,
+ clk_assigned_sscs_assigns_multiple_test_params,
+ desc)
+
+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);
+}
+
+/* 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-sscs 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-sscs property of zero doesn't
+ * set sscs 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-sscs property of zero doesn't
+ * set sscs 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 sscs 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 sscs when the property is in the consumer.
+ */
+ .desc = "consumer 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_sscs_test_cases[] = {
+ KUNIT_CASE_PARAM(clk_assigned_sscs_assigns_one,
+ clk_assigned_sscs_assigns_one_gen_params),
+ KUNIT_CASE_PARAM(clk_assigned_sscs_assigns_multiple,
+ clk_assigned_sscs_assigns_multiple_gen_params),
+ KUNIT_CASE_PARAM(clk_assigned_sscs_skips,
+ clk_assigned_sscs_skips_gen_params),
+ {}
+};
+
+/*
+ * Test suite for assigned-clock-sscs DT property.
+ */
+static struct kunit_suite clk_assigned_sscs_suite = {
+ .name = "clk_assigned_sscs",
+ .test_cases = clk_assigned_sscs_test_cases,
+ .init = clk_assigned_rates_test_init,
+};
+
static const struct clk_init_data clk_hw_get_dev_of_node_init_data = {
.name = "clk_hw_get_dev_of_node",
.ops = &empty_clk_ops,
@@ -3544,6 +3740,7 @@ static struct kunit_suite clk_hw_get_dev_of_node_test_suite = {
kunit_test_suites(
&clk_assigned_rates_suite,
+ &clk_assigned_sscs_suite,
&clk_hw_get_dev_of_node_test_suite,
&clk_leaf_mux_set_rate_parent_test_suite,
&clk_test_suite,
diff --git a/drivers/clk/kunit_clk_assigned_rates.h b/drivers/clk/kunit_clk_assigned_rates.h
index df2d84dcaa935..d7ae5ec2d25be 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_u64_multiple.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso
index 389b4e2eb7f74..3a717dab2d00b 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 3e117fd59b7da..cbee7cbad068f 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 87041264e8f54..9b04d6927f083 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 3259c003aec0b..4784d40520f41 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_multiple.dtso b/drivers/clk/kunit_clk_assigned_sscs_multiple.dtso
new file mode 100644
index 0000000000000..e3472f95987c3
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_multiple.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 = <1>;
+ assigned-clocks = <&clk 0>,
+ <&clk 1>;
+ 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_sscs_multiple_consumer.dtso b/drivers/clk/kunit_clk_assigned_sscs_multiple_consumer.dtso
new file mode 100644
index 0000000000000..6e8971bd272ab
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_multiple_consumer.dtso
@@ -0,0 +1,24 @@
+// 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 = <1>;
+ };
+
+ kunit-clock-consumer {
+ compatible = "test,clk-consumer";
+ assigned-clocks = <&clk 0>,
+ <&clk 1>;
+ 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_sscs_null.dtso b/drivers/clk/kunit_clk_assigned_sscs_null.dtso
new file mode 100644
index 0000000000000..43b2068c845de
--- /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 0000000000000..bda008f5aaa35
--- /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_one.dtso b/drivers/clk/kunit_clk_assigned_sscs_one.dtso
new file mode 100644
index 0000000000000..91f585b5d8c9b
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_one.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 = <&clk>;
+ assigned-clock-sscs = <ASSIGNED_SSCS_0_MODFREQ
+ ASSIGNED_SSCS_0_SPREAD
+ ASSIGNED_SSCS_0_METHOD>;
+ };
+};
diff --git a/drivers/clk/kunit_clk_assigned_sscs_one_consumer.dtso b/drivers/clk/kunit_clk_assigned_sscs_one_consumer.dtso
new file mode 100644
index 0000000000000..0bc8a03c20412
--- /dev/null
+++ b/drivers/clk/kunit_clk_assigned_sscs_one_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 = <&clk>;
+ 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 0000000000000..08660846b55c1
--- /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 0000000000000..e1c089c6f0c02
--- /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 0000000000000..f39f4e754e532
--- /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 0000000000000..d6bd7dfada7e2
--- /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.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs
2026-06-12 8:46 ` [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs Peng Fan (OSS)
@ 2026-06-12 8:55 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-12 8:55 UTC (permalink / raw)
Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Extraneous `assigned-clock-sscs` properties injected into `u64` rate test overlays without accompanying test assertions.
--
commit 210e00ec864807cd43e4b4e5dca8755417d9ee5a
Author: Peng Fan <peng.fan@nxp.com>
Subject: clk: Add KUnit tests for assigned-clock-sscs
This commit adds KUnit test coverage for the assigned-clock-sscs device tree
property. It extends the dummy clock framework to support spread spectrum
configuration and introduces a new test suite with several scenarios,
including single assignments, multiple assignments, and malformed DT
properties. It also adds new DT overlays for these test scenarios.
> diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso
> index 389b4e2eb7f74..3a717dab2d00b 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>;
[Severity: Low]
Looking at the kunit-clock node in
drivers/clk/kunit_clk_assigned_rates_u64_multiple.dtso, are these
assigned-clock-sscs properties necessary in the u64 rate test overlays?
The test cases utilizing these overlays only verify rate assignments and do
not assert anything about the parsed SSCS state. Since dedicated SSCS test
overlays were added, it seems these additions might be extraneous.
> };
> };
[ ... ]
> diff --git a/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso b/drivers/clk/kunit_clk_assigned_rates_u64_one.dtso
> index 87041264e8f54..9b04d6927f083 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>;
[Severity: Low]
Similar to the multiple overlay, is this property needed in the kunit-clock
node of drivers/clk/kunit_clk_assigned_rates_u64_one.dtso for the u64 tests?
> };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-clk-v10-v10-0-eb92484eda38@nxp.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v10 5/6] clk: scmi: Introduce common header for SCMI clock interface
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
` (3 preceding siblings ...)
2026-06-12 8:46 ` [PATCH v10 4/6] clk: Add KUnit tests for assigned-clock-sscs Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:46 ` [PATCH v10 6/6] clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver Peng Fan (OSS)
5 siblings, 0 replies; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Added a new header file 'clk-scmi.h' to define common structures and
interfaces for the SCMI clock driver. This header will also be used by
OEM-specific extensions to ensure consistency and reusability.
Moved relevant structure definitions from the driver implementation to
'clk-scmi.h' to facilitate shared usage.
Reviewed-by: Sebin Francis <sebin.francis@ti.com>
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/clk/clk-scmi.c | 27 +--------------------------
drivers/clk/clk-scmi.h | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index 7c562559ad8bb..d88e78cc9a12e 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -13,32 +13,7 @@
#include <linux/module.h>
#include <linux/scmi_protocol.h>
-#define NOT_ATOMIC false
-#define ATOMIC true
-
-enum scmi_clk_feats {
- SCMI_CLK_ATOMIC_SUPPORTED,
- SCMI_CLK_STATE_CTRL_SUPPORTED,
- SCMI_CLK_RATE_CTRL_SUPPORTED,
- SCMI_CLK_PARENT_CTRL_SUPPORTED,
- SCMI_CLK_DUTY_CYCLE_SUPPORTED,
- SCMI_CLK_FEATS_COUNT
-};
-
-#define SCMI_MAX_CLK_OPS BIT(SCMI_CLK_FEATS_COUNT)
-
-static const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
-
-struct scmi_clk {
- u32 id;
- struct device *dev;
- struct clk_hw hw;
- const struct scmi_clock_info *info;
- const struct scmi_protocol_handle *ph;
- struct clk_parent_data *parent_data;
-};
-
-#define to_scmi_clk(clk) container_of(clk, struct scmi_clk, hw)
+const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
static unsigned long scmi_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
diff --git a/drivers/clk/clk-scmi.h b/drivers/clk/clk-scmi.h
new file mode 100644
index 0000000000000..6ef6adc77c836
--- /dev/null
+++ b/drivers/clk/clk-scmi.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef __SCMI_CLK_H
+#define __SCMI_CLK_H
+
+#include <linux/bits.h>
+#include <linux/scmi_protocol.h>
+#include <linux/types.h>
+
+#define NOT_ATOMIC false
+#define ATOMIC true
+
+enum scmi_clk_feats {
+ SCMI_CLK_ATOMIC_SUPPORTED,
+ SCMI_CLK_STATE_CTRL_SUPPORTED,
+ SCMI_CLK_RATE_CTRL_SUPPORTED,
+ SCMI_CLK_PARENT_CTRL_SUPPORTED,
+ SCMI_CLK_DUTY_CYCLE_SUPPORTED,
+ SCMI_CLK_FEATS_COUNT
+};
+
+#define SCMI_MAX_CLK_OPS BIT(SCMI_CLK_FEATS_COUNT)
+
+struct scmi_clk {
+ u32 id;
+ struct device *dev;
+ struct clk_hw hw;
+ const struct scmi_clock_info *info;
+ const struct scmi_protocol_handle *ph;
+ struct clk_parent_data *parent_data;
+};
+
+#define to_scmi_clk(clk) container_of(clk, struct scmi_clk, hw)
+
+extern const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
+
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v10 6/6] clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver
2026-06-12 8:46 [PATCH v10 0/6] clk: Support spread spectrum and use it in clk-scmi Peng Fan (OSS)
` (4 preceding siblings ...)
2026-06-12 8:46 ` [PATCH v10 5/6] clk: scmi: Introduce common header for SCMI clock interface Peng Fan (OSS)
@ 2026-06-12 8:46 ` Peng Fan (OSS)
2026-06-12 8:58 ` sashiko-bot
5 siblings, 1 reply; 11+ messages in thread
From: Peng Fan (OSS) @ 2026-06-12 8:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sudeep Holla, Cristian Marussi,
Sebin Francis
Cc: linux-kernel, linux-clk, devicetree, arm-scmi, linux-arm-kernel,
Peng Fan
From: Peng Fan <peng.fan@nxp.com>
- Introduce 'clk-scmi-oem.c' to support vendor-specific OEM extensions
for the SCMI clock driver, allows clean integration of vendor-specific
features without impacting the core SCMI clock driver logic.
- Extend 'clk-scmi.h' with 'scmi_clk_oem' structure and related
declarations.
- Initialize OEM extensions via 'scmi_clk_oem_init()'.
- Support querying OEM-specific features and setting spread spectrum.
- Pass 'scmi_device' to 'scmi_clk_ops_select()' for OEM data access.
Reviewed-by: Sebin Francis <sebin.francis@ti.com>
Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/clk/Makefile | 2 +-
drivers/clk/clk-scmi-oem.c | 108 +++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/clk-scmi.c | 19 ++++++--
drivers/clk/clk-scmi.h | 11 +++++
4 files changed, 136 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 6a726331b6c9e..c2ae700ec0f2a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -98,7 +98,7 @@ obj-$(CONFIG_COMMON_CLK_RP1) += clk-rp1.o
obj-$(CONFIG_COMMON_CLK_RPMI) += clk-rpmi.o
obj-$(CONFIG_COMMON_CLK_HI655X) += clk-hi655x.o
obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
-obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o
+obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o clk-scmi-oem.o
obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o
obj-$(CONFIG_COMMON_CLK_SI5341) += clk-si5341.o
obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o
diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
new file mode 100644
index 0000000000000..be11d359b4ec3
--- /dev/null
+++ b/drivers/clk/clk-scmi-oem.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * The Vendor OEM extension for System Control and Power Interface (SCMI)
+ * Protocol based clock driver
+ *
+ * Copyright 2025 NXP
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/scmi_imx_protocol.h>
+#include <linux/scmi_protocol.h>
+
+#include "clk-scmi.h"
+
+#define SCMI_CLOCK_CFG_IMX_SSC 0x80
+#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)
+
+/*
+ * Selection is based on SCMI vendor_id/sub_vendor_id and optional machine
+ * compatible string, without involving impl_ver. impl_ver‑specific behavior
+ * should be considered a bug and handled via SCMI Quirk framework.
+ */
+struct scmi_clk_oem_info {
+ char *vendor_id;
+ char *sub_vendor_id;
+ char *compatible;
+ const void *data;
+};
+
+static int
+scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
+ const struct clk_spread_spectrum *ss_conf)
+{
+ 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, ss_conf->spread_bp / 10000);
+ val |= FIELD_PREP(SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK, ss_conf->modfreq_hz);
+ if (ss_conf->method != CLK_SPREAD_NO)
+ 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",
+ ss_conf->modfreq_hz, ss_conf->spread_bp, ss_conf->method,
+ clk->id);
+
+ return ret;
+}
+
+static int
+scmi_clk_imx_query_oem_feats(const struct scmi_protocol_handle *ph, u32 id,
+ unsigned int *feats_key)
+{
+ int ret;
+ u32 val;
+
+ ret = scmi_proto_clk_ops->config_oem_get(ph, id,
+ SCMI_CLOCK_CFG_IMX_SSC,
+ &val, NULL, false);
+ if (!ret)
+ *feats_key |= BIT(SCMI_CLK_EXT_OEM_SSC_SUPPORTED);
+
+ return 0;
+}
+
+static const struct scmi_clk_oem scmi_clk_oem_imx = {
+ .query_ext_oem_feats = scmi_clk_imx_query_oem_feats,
+ .set_spread_spectrum = scmi_clk_imx_set_spread_spectrum,
+};
+
+static const struct scmi_clk_oem_info info[] = {
+ { SCMI_IMX_VENDOR, SCMI_IMX_SUBVENDOR, NULL, &scmi_clk_oem_imx },
+};
+
+int scmi_clk_oem_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ int i, size = ARRAY_SIZE(info);
+
+ for (i = 0; i < size; i++) {
+ if (strcmp(handle->version->vendor_id, info[i].vendor_id) ||
+ strcmp(handle->version->sub_vendor_id, info[i].sub_vendor_id))
+ continue;
+ if (info[i].compatible &&
+ !of_machine_is_compatible(info[i].compatible))
+ continue;
+
+ break;
+ }
+
+ if (i < size)
+ dev_set_drvdata(&sdev->dev, (void *)info[i].data);
+
+ return 0;
+}
diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index d88e78cc9a12e..2dd50c5b4ea8f 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -13,6 +13,8 @@
#include <linux/module.h>
#include <linux/scmi_protocol.h>
+#include "clk-scmi.h"
+
const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
static unsigned long scmi_clk_recalc_rate(struct clk_hw *hw,
@@ -210,6 +212,7 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk,
static const struct clk_ops *
scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
{
+ struct scmi_clk_oem *oem_data = dev_get_drvdata(dev);
struct clk_ops *ops;
ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
@@ -256,11 +259,15 @@ scmi_clk_ops_alloc(struct device *dev, unsigned long feats_key)
ops->set_duty_cycle = scmi_clk_set_duty_cycle;
}
+ if (oem_data && (feats_key & BIT(SCMI_CLK_EXT_OEM_SSC_SUPPORTED)))
+ ops->set_spread_spectrum = oem_data->set_spread_spectrum;
+
return ops;
}
/**
* scmi_clk_ops_select() - Select a proper set of clock operations
+ * @sdev: pointer to the SCMI device
* @sclk: A reference to an SCMI clock descriptor
* @atomic_capable: A flag to indicate if atomic mode is supported by the
* transport
@@ -285,8 +292,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(struct scmi_device *sdev, struct scmi_clk *sclk,
+ bool atomic_capable, unsigned int atomic_threshold_us,
const struct clk_ops **clk_ops_db, size_t db_size)
{
int ret;
@@ -294,6 +301,7 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
const struct scmi_clock_info *ci = sclk->info;
unsigned int feats_key = 0;
const struct clk_ops *ops;
+ struct scmi_clk_oem *oem_data = dev_get_drvdata(&sdev->dev);
/*
* Note that when transport is atomic but SCMI protocol did not
@@ -318,6 +326,9 @@ scmi_clk_ops_select(struct scmi_clk *sclk, bool atomic_capable,
&val, NULL, false);
if (!ret)
feats_key |= BIT(SCMI_CLK_DUTY_CYCLE_SUPPORTED);
+
+ if (oem_data && oem_data->query_ext_oem_feats)
+ oem_data->query_ext_oem_feats(sclk->ph, sclk->id, &feats_key);
}
if (WARN_ON(feats_key >= db_size))
@@ -375,6 +386,8 @@ static int scmi_clocks_probe(struct scmi_device *sdev)
clk_data->num = count;
hws = clk_data->hws;
+ scmi_clk_oem_init(sdev);
+
transport_is_atomic = handle->is_transport_atomic(handle,
&atomic_threshold_us);
@@ -406,7 +419,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(sdev, sclk, transport_is_atomic,
atomic_threshold_us,
scmi_clk_ops_db,
ARRAY_SIZE(scmi_clk_ops_db));
diff --git a/drivers/clk/clk-scmi.h b/drivers/clk/clk-scmi.h
index 6ef6adc77c836..d7f63f36c56d1 100644
--- a/drivers/clk/clk-scmi.h
+++ b/drivers/clk/clk-scmi.h
@@ -7,6 +7,7 @@
#define __SCMI_CLK_H
#include <linux/bits.h>
+#include <linux/clk-provider.h>
#include <linux/scmi_protocol.h>
#include <linux/types.h>
@@ -19,6 +20,7 @@ enum scmi_clk_feats {
SCMI_CLK_RATE_CTRL_SUPPORTED,
SCMI_CLK_PARENT_CTRL_SUPPORTED,
SCMI_CLK_DUTY_CYCLE_SUPPORTED,
+ SCMI_CLK_EXT_OEM_SSC_SUPPORTED,
SCMI_CLK_FEATS_COUNT
};
@@ -37,4 +39,13 @@ struct scmi_clk {
extern const struct scmi_clk_proto_ops *scmi_proto_clk_ops;
+struct scmi_clk_oem {
+ int (*query_ext_oem_feats)(const struct scmi_protocol_handle *ph,
+ u32 id, unsigned int *feats_key);
+ int (*set_spread_spectrum)(struct clk_hw *hw,
+ const struct clk_spread_spectrum *ss_conf);
+};
+
+int scmi_clk_oem_init(struct scmi_device *dev);
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v10 6/6] clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver
2026-06-12 8:46 ` [PATCH v10 6/6] clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver Peng Fan (OSS)
@ 2026-06-12 8:58 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-12 8:58 UTC (permalink / raw)
Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The driver fails to link when compiled as a module (`CONFIG_COMMON_CLK_SCMI=m`) because `clk-scmi.c` and `clk-scmi-oem.c` are built as separate modules without exporting their shared symbols.
- [High] Incorrect unit conversion for spread spectrum percentage causes valid spread values to be truncated to 0.
Pre-existing issues:
- [High] Missing bounds check on SCMI firmware-provided parent clock index leads to heap out-of-bounds read and wild pointer dereference.
--
commit 02412e4d55042034939e6eece844d1bd4d62d5e9
Author: Peng Fan <peng.fan@nxp.com>
clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver
Introduces clk-scmi-oem.c to support vendor-specific OEM extensions for the
SCMI clock driver. This enables querying OEM-specific features and setting
spread spectrum configurations.
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 6a726331b6c9e..c2ae700ec0f2a 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -98,7 +98,7 @@ obj-$(CONFIG_COMMON_CLK_RP1) += clk-rp1.o
> obj-$(CONFIG_COMMON_CLK_RPMI) += clk-rpmi.o
> obj-$(CONFIG_COMMON_CLK_HI655X) += clk-hi655x.o
> obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
> -obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o
> +obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o clk-scmi-oem.o
[Severity: High]
Will this fail to link when compiled as a module (CONFIG_COMMON_CLK_SCMI=m)?
It looks like Kbuild will treat clk-scmi.o and clk-scmi-oem.o as separate
kernel modules rather than linking them into a single composite module.
Since neither file exports their shared symbols (like scmi_proto_clk_ops
or scmi_clk_oem_init), won't this result in undefined reference errors?
Should this use a composite module definition instead?
scmi-clk-y := clk-scmi.o clk-scmi-oem.o
obj-$(CONFIG_COMMON_CLK_SCMI) += scmi-clk.o
[ ... ]
> diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
> new file mode 100644
> index 0000000000000..be11d359b4ec3
> --- /dev/null
> +++ b/drivers/clk/clk-scmi-oem.c
[ ... ]
> +static int
> +scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
> + const struct clk_spread_spectrum *ss_conf)
> +{
> + 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, ss_conf->spread_bp / 10000);
[Severity: High]
Does this integer division by 10000 inadvertently truncate valid spread
spectrum values to 0?
Since ss_conf->spread_bp is in basis points (where 100 basis points equals
1%), and standard configurations usually range from 50 to 500 basis points
(0.5% to 5%), dividing by 10000 will always result in zero. Could this
divide by 100 instead to correctly convert basis points to a percentage?
[ ... ]
> diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
> index d88e78cc9a12e..2dd50c5b4ea8f 100644
> --- a/drivers/clk/clk-scmi.c
> +++ b/drivers/clk/clk-scmi.c
[ ... ]
> @@ -406,7 +419,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(sdev, sclk, transport_is_atomic,
> atomic_threshold_us,
> scmi_clk_ops_db,
> ARRAY_SIZE(scmi_clk_ops_db));
[Severity: High]
This is a pre-existing issue, but is it possible for the SCMI firmware to
trigger an out-of-bounds read right after this ops setup?
Looking at scmi_clocks_probe(), it uses the parent index provided by the
SCMI firmware to directly access the dynamically allocated hws array:
drivers/clk/clk-scmi.c:scmi_clocks_probe() {
...
for (int i = 0; i < sclk->info->num_parents; i++) {
sclk->parent_data[i].index = sclk->info->parents[i];
sclk->parent_data[i].hw = hws[sclk->info->parents[i]];
...
}
If the firmware provides an index greater than or equal to the total clock
count, won't this result in an out-of-bounds read on the heap, potentially
dereferencing a wild pointer? Should this index be verified against count
first?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-clk-v10-v10-0-eb92484eda38@nxp.com?part=6
^ permalink raw reply [flat|nested] 11+ messages in thread