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.
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.1Hi Peng,
On Mon, Sep 01, 2025 at 11:51:46AM +0800, 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Stephen has also been asking for kunit tests to be added for new
functionality in the clk core. There's already one kunit test that
calls of_clk_set_defaults(). I attached a very rough draft of a patch
showing that it'd be possible to mock this up in a test with what's
already there. I set a log statement with the configuration from
device tree:
test_assigned_rate0: Spread Sprectrum Configuration: modfreq_hz=10000 spread_bp=3 method=1
You can run the kunit tests with:
./tools/testing/kunit/kunit.py run \
--kunitconfig drivers/clk/.kunitconfig \
--raw_output=all
Additionally, what do you think about making a dt-bindings include file
for CLK_SSC_CENTER_SPREAD + friends? Right now, the test illustrates
that we need to hardcode the number from the clk-provider.h file inside
the DTS.
Here's the patch and feel free to make it your own as you see fit.
Brian
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index a268d7b5d4cb..6cc3ad883b35 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 = {
@@ -3192,7 +3207,13 @@ static int clk_assigned_rates_test_init(struct kunit *test)
consumer = of_find_compatible_node(NULL, NULL, "test,clk-consumer"));
of_node_put_kunit(test, consumer);
+ // Here's an example of a test that shows where
+ // of_clk_set_defaults() is called for the consumer.
KUNIT_ASSERT_EQ(test, 0, of_clk_set_defaults(consumer, false));
+ pr_crit("%s: Spread Sprectrum Configuration: modfreq_hz=%u spread_bp=%u method=%u\n",
+ clk_hw_get_name(&ctx->clk0.hw), ctx->clk0.sscs.modfreq_hz,
+ ctx->clk0.sscs.spread_bp, ctx->clk0.sscs.method);
+
}
return 0;
diff --git a/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso b/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
index a41dca806318..a157a316a10d 100644
--- a/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
+++ b/drivers/clk/kunit_clk_assigned_rates_one_consumer.dtso
@@ -14,5 +14,6 @@ kunit-clock-consumer {
compatible = "test,clk-consumer";
assigned-clocks = <&clk>;
assigned-clock-rates = <ASSIGNED_RATES_0_RATE>;
+ assigned-clock-sscs = <10000 3 1>;
};
};
Hi Brian, On Tue, Sep 02, 2025 at 07:56:07PM -0400, Brian Masney wrote: >Hi Peng, > >On Mon, Sep 01, 2025 at 11:51:46AM +0800, 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. >> >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >Stephen has also been asking for kunit tests to be added for new >functionality in the clk core. There's already one kunit test that >calls of_clk_set_defaults(). I attached a very rough draft of a patch >showing that it'd be possible to mock this up in a test with what's >already there. I set a log statement with the configuration from >device tree: Appreciate your help on this. I was not aware of kunit tests is required. I will add one in V3. > >test_assigned_rate0: Spread Sprectrum Configuration: modfreq_hz=10000 spread_bp=3 method=1 > >You can run the kunit tests with: > >./tools/testing/kunit/kunit.py run \ > --kunitconfig drivers/clk/.kunitconfig \ > --raw_output=all > >Additionally, what do you think about making a dt-bindings include file >for CLK_SSC_CENTER_SPREAD + friends? Right now, the test illustrates >that we need to hardcode the number from the clk-provider.h file inside >the DTS. ok to add a generic dt-bindings file for the the SSC Spread method. I will add one in V3. > >Here's the patch and feel free to make it your own as you see fit. > Thanks a lot for sharing this. Thanks, Peng
On Mon, Sep 01, 2025 at 11:51:46AM +0800, 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.
>
> 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);
This chunk can be replaced with dev_warn_probe(). Sorry I missed that in
v1. Otherwise the rest looks good to me. With that fixed:
Reviewed-by: Brian Masney <bmasney@redhat.com>
Hi Brian,
On Tue, Sep 02, 2025 at 05:49:17PM -0400, Brian Masney wrote:
[...]
>> + 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);
>
>This chunk can be replaced with dev_warn_probe(). Sorry I missed that in
>v1. Otherwise the rest looks good to me. With that fixed:
dev_warn_probe() could not be used here. There is no 'device' pointer
here.
I take __set_clk_parents() as example here, so use pr_warn().
>
>Reviewed-by: Brian Masney <bmasney@redhat.com>
Could I still keep this tag with keeping pr_warn()?
Thanks,
Peng
>
On Tue, Sep 09, 2025 at 04:50:13PM +0800, Peng Fan wrote:
> Hi Brian,
> On Tue, Sep 02, 2025 at 05:49:17PM -0400, Brian Masney wrote:
> [...]
> >> + 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);
> >
> >This chunk can be replaced with dev_warn_probe(). Sorry I missed that in
> >v1. Otherwise the rest looks good to me. With that fixed:
>
> dev_warn_probe() could not be used here. There is no 'device' pointer
> here.
Ahh ok, gotcha.
> I take __set_clk_parents() as example here, so use pr_warn().
>
> >
> >Reviewed-by: Brian Masney <bmasney@redhat.com>
>
> Could I still keep this tag with keeping pr_warn()?
Yes. Thank you!
Brian
© 2016 - 2025 Red Hat, Inc.