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 | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 303a0bb26e54a95655ce094a35b989c97ebc6fd8..81a2c1f8ca4c44df2c54c1e51f800f533c9453b3 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -155,6 +155,72 @@ 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 *sscs __free(kfree) = NULL;
+ u32 elem_size = sizeof(u32) * 3;
+ 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) {
+ sscs = kcalloc(count, elem_size, GFP_KERNEL);
+ if (!sscs)
+ return -ENOMEM;
+ rc = of_property_read_u32_array(node,
+ "assigned-clock-sscs",
+ sscs, count * 3);
+ } else {
+ return 0;
+ }
+
+ if (rc)
+ return rc;
+
+ for (index = 0; index < count; index++) {
+ u32 modfreq_hz = sscs[index * 3], spread_bp = sscs[index * 3 + 1];
+ u32 method = sscs[index * 3 + 2];
+ struct clk_hw *hw;
+
+ if (modfreq_hz || spread_bp || method) {
+ 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, modfreq_hz, spread_bp, method);
+ if (rc < 0)
+ pr_err("clk: couldn't set %s clk spread spectrum %u %u %u: %d\n",
+ __clk_get_name(clk), modfreq_hz, spread_bp, 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 +240,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.1On Tue, Aug 12, 2025 at 08:17:06PM +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 | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a95655ce094a35b989c97ebc6fd8..81a2c1f8ca4c44df2c54c1e51f800f533c9453b3 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -155,6 +155,72 @@ 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 *sscs __free(kfree) = NULL;
> + u32 elem_size = sizeof(u32) * 3;
> + 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) {
> + sscs = kcalloc(count, elem_size, GFP_KERNEL);
> + if (!sscs)
> + return -ENOMEM;
> + rc = of_property_read_u32_array(node,
> + "assigned-clock-sscs",
> + sscs, count * 3);
> + } else {
> + return 0;
> + }
> +
> + if (rc)
> + return rc;
Nit pick: Please, flip these conditions around.
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", sscs,
count * 3);
if (rc)
return rc;
> +
> + for (index = 0; index < count; index++) {
> + u32 modfreq_hz = sscs[index * 3], spread_bp = sscs[index * 3 + 1];
> + u32 method = sscs[index * 3 + 2];
This math would be nicer if you created a struct:
struct spread_config {
u32 modfreq_hz;
u32 spread_depth;
u32 method;
};
Then you could use that instead of sscs.
for (i = 0; i < count; i++) {
struct spread_config *conf = &configs[i];
struct clk_hw *hw;
if (!conf->modfreq_hz && !conf->spread_depth && !conf->method)
continue;
> + struct clk_hw *hw;
> +
> + if (modfreq_hz || spread_bp || method) {
> + 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) {
Could you add a comment for this condition? It's strange to me that we
don't iterate through the whole array.
regards,
dan carpenter
> + 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, modfreq_hz, spread_bp, method);
> + if (rc < 0)
> + pr_err("clk: couldn't set %s clk spread spectrum %u %u %u: %d\n",
> + __clk_get_name(clk), modfreq_hz, spread_bp, method, rc);
> + clk_put(clk);
> + }
> + }
> +
> + return 0;
> +}
Hi Dan,
On Wed, Aug 13, 2025 at 08:48:15AM +0300, Dan Carpenter wrote:
>On Tue, Aug 12, 2025 at 08:17:06PM +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 | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 70 insertions(+)
>>
>> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
>> index 303a0bb26e54a95655ce094a35b989c97ebc6fd8..81a2c1f8ca4c44df2c54c1e51f800f533c9453b3 100644
>> --- a/drivers/clk/clk-conf.c
>> +++ b/drivers/clk/clk-conf.c
>> @@ -155,6 +155,72 @@ 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 *sscs __free(kfree) = NULL;
>> + u32 elem_size = sizeof(u32) * 3;
>> + 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) {
>> + sscs = kcalloc(count, elem_size, GFP_KERNEL);
>> + if (!sscs)
>> + return -ENOMEM;
>> + rc = of_property_read_u32_array(node,
>> + "assigned-clock-sscs",
>> + sscs, count * 3);
>> + } else {
>> + return 0;
>> + }
>> +
>> + if (rc)
>> + return rc;
>
>Nit pick: Please, flip these conditions around.
ok. Fix in next version.
>
> 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", sscs,
> count * 3);
> if (rc)
> return rc;
>
>> +
>> + for (index = 0; index < count; index++) {
>> + u32 modfreq_hz = sscs[index * 3], spread_bp = sscs[index * 3 + 1];
>> + u32 method = sscs[index * 3 + 2];
>
>This math would be nicer if you created a struct:
>
>struct spread_config {
> u32 modfreq_hz;
> u32 spread_depth;
> u32 method;
>};
>
>Then you could use that instead of sscs.
>
> for (i = 0; i < count; i++) {
> struct spread_config *conf = &configs[i];
> struct clk_hw *hw;
>
> if (!conf->modfreq_hz && !conf->spread_depth && !conf->method)
> continue;
Thanks for the tips. Update in next version.
>
>> + struct clk_hw *hw;
>> +
>> + if (modfreq_hz || spread_bp || method) {
>> + 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) {
>
>Could you add a comment for this condition? It's strange to me that we
>don't iterate through the whole array.
I just follow the logic in __set_clk_parents and __set_clk_rates, nothing
special here.
It is just like to phase out cases as below:
node-x {
/* node-x is not a clk provider, but assigned-clocks uses node-x phandle */
assigned-clocks = <&node-x XYZ>;
}
Thanks for reviewing the patch. I will wait to collect more comments before
posting next version.
Regards,
Peng
>
>regards,
>dan carpenter
>
>> + 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, modfreq_hz, spread_bp, method);
>> + if (rc < 0)
>> + pr_err("clk: couldn't set %s clk spread spectrum %u %u %u: %d\n",
>> + __clk_get_name(clk), modfreq_hz, spread_bp, method, rc);
>> + clk_put(clk);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>
On Fri, Aug 15, 2025 at 04:50:42PM +0800, Peng Fan wrote:
> >> + if (modfreq_hz || spread_bp || method) {
> >> + 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) {
> >
> >Could you add a comment for this condition? It's strange to me that we
> >don't iterate through the whole array.
>
> I just follow the logic in __set_clk_parents and __set_clk_rates, nothing
> special here.
>
> It is just like to phase out cases as below:
> node-x {
> /* node-x is not a clk provider, but assigned-clocks uses node-x phandle */
> assigned-clocks = <&node-x XYZ>;
> }
>
Ah. Great. Thanks.
regards,
dan carpenter
© 2016 - 2025 Red Hat, Inc.