[PATCH v13 0/4] Support spread spectrum clocking for i.MX8M PLLs

[PATCH v13 0/4] Support spread spectrum clocking for i.MX8M PLLs

[Dario Binacchi]

The series adds support for spread spectrum clocking for i.MX8M{M,N,P}
PLLs (audio, video and DRAM). It has been tested for the video PLL on
boards using i.MX8MN and i.MX8MP.

While at it, it validates the spread spectrum configuration in the clk
core before it reaches the providers, and fixes the i.MX SCMI OEM
extension, which converted the spread to the wrong unit.

Changes in v13:
 - Add patch "clk: scmi: reject SSC configuration out of the OEM field
   range".
 - Rename patch "clk: scmi: fix SSC permyriad conversion" to "clk: scmi:
   fix SSC spread conversion".

Changes in v12:
 - Add patch "clk: scmi: fix SSC permyriad conversion" to fix the
   permyriad to percent conversion in clk-scmi-oem.c, reported by
   Sashiko AI review.
 - Fix issues claimed by Sashiko for patch "clk: imx: pll14xx: support
   spread spectrum clock". See the details in the patch changelog.

Changes in v11:
 - Add patch "clk: validate spread spectrum configuration" to reject
   invalid parameters in the clk core before they reach the providers.
 - Fix issues claimed by Sashiko for patch "clk: imx: pll14xx: support
   spread spectrum clock". See the details in the patch changelog.
 
Changes in v10:
- Drop patches 1-22, superseded by the generic spread spectrum clocking
  support from Peng Fan, merged in 7.3, so the i.MX8M specific infrastructure
  is no longer needed. This patch is now based on the following commits:
  - ae794aa6e88d ("dt-bindings: clock: Add spread spectrum definition")
  - c86814e70390 ("clk: Introduce clk_hw_set_spread_spectrum")
  - a7f16660af39 ("clk: conf: Support assigned-clock-sscs")
  - 7d98de6ffdab ("clk: Add KUnit tests for assigned-clock-sscs")
  https://lore.kernel.org/all/20260826165611.236321-1-sboyd@kernel.org/

Changes in v9:
- Add 'Reviewed-by' tag of Peng Fan for imx8mn platform patches
- Fix building warning raised by the kernel test robot for patch
  v8, 11/18 clk: imx: add support for i.MX8MN anatop clock driver
- Add patches for imx8m{m,p} platforms:
  - 23/23 clk: imx8mm: support spread spectrum clock generation
  - 22/23 clk: imx: add support for i.MX8MM anatop clock driver
  - 21/23 clk: imx8mp: support spread spectrum clock generation
  - 20/23 clk: imx8mp: rename ccm_base to base
  - 19/23 clk: imx: add support for i.MX8MP anatop clock driver

Changes in v8:
- Drop the patches added in version 7:
  - 10/23 dt-bindings: clock: imx8m-clock: add phandle to the anatop
  - 11/23 arm64: dts: imx8mm: add phandle to anatop within CCM
  - 12/23 arm64: dts: imx8mn: add phandle to anatop within CCM
  - 13/23 arm64: dts: imx8mp: add phandle to anatop within CCM
  - 14/23 arm64: dts: imx8mq: add phandle to anatop within CCM

Changes in v7:
- Add and manage fsl,anatop property as phandle to the anatop node with
  the new patches:
  - 10/23 dt-bindings: clock: imx8m-clock: add phandle to the anatop
  - 11/23 arm64: dts: imx8mm: add phandle to anatop within CCM
  - 12/23 arm64: dts: imx8mn: add phandle to anatop within CCM
  - 13/23 arm64: dts: imx8mp: add phandle to anatop within CCM
  - 14/23 arm64: dts: imx8mq: add phandle to anatop within CCM

Changes in v6:
- Merge patches:
  10/20 dt-bindings: clock: imx8mm: add binding definitions for anatop
  11/20 dt-bindings: clock: imx8mn: add binding definitions for anatop
  12/20 dt-bindings: clock: imx8mp: add binding definitions for anatop
  to
  05/20 dt-bindings: clock: imx8m-anatop: define clocks/clock-names
  now renamed
  05/18 dt-bindings: clock: imx8m-anatop: add oscillators and PLLs
- Split the patch
  15/20 dt-bindings-clock-imx8m-clock-support-spread-spectru.patch
  into
  12/18 dt-bindings: clock: imx8m-clock: add PLLs
  16/18 dt-bindings: clock: imx8m-clock: support spread spectrum clocking

Changes in v5:
- Fix compilation errors.
- Separate driver code from dt-bindings

Changes in v4:
- Add dt-bindings for anatop
- Add anatop driver
- Drop fsl,ssc-clocks from spread spectrum dt-bindings

Changes in v3:
- Patches 1/8 has been added in version 3. The dt-bindings have
  been moved from fsl,imx8m-anatop.yaml to imx8m-clock.yaml. The
  anatop device (fsl,imx8m-anatop.yaml) is indeed more or less a
  syscon, so it represents a memory area accessible by ccm
  (imx8m-clock.yaml) to setup the PLLs.
- Patches {3,5}/8 have been added in version 3.
- Patches {4,6,8}/8 use ccm device node instead of the anatop one.

Changes in v2:
- Add "allOf:" and place it after "required:" block, like in the
  example schema.
- Move the properties definition to the top-level.
- Drop unit types as requested by the "make dt_binding_check" command.

Dario Binacchi (4):
  clk: scmi: fix SSC spread conversion
  clk: scmi: reject SSC configuration out of the OEM field range
  clk: validate spread spectrum configuration
  clk: imx: pll14xx: support spread spectrum clock generation

 drivers/clk/clk-scmi-oem.c             | 17 ++++-
 drivers/clk/clk.c                      | 14 ++++
 drivers/clk/clk_test.c                 | 12 ++--
 drivers/clk/imx/clk-pll14xx.c          | 99 ++++++++++++++++++++++++++
 drivers/clk/kunit_clk_assigned_rates.h |  4 +-
 5 files changed, 136 insertions(+), 10 deletions(-)

---
base-commit: 940de590b839f71d6dc846160534bf202401b8b7
branch: imx-pll14xx-spread-spectrum

-- 
2.43.0

[PATCH v13 1/4] clk: scmi: fix SSC spread conversion

[Dario Binacchi]

The spread_bp field of struct clk_spread_spectrum holds the modulation
depth in permyriad (1/100 of a percent), as documented in
clk-provider.h and in the assigned-clock-sscs binding, while the i.MX
SCMI OEM extension expects a value in tenths of a percent, as pointed
out by Peng Fan.

The conversion divides spread_bp by 10000 instead of 10, as reported by
Sashiko, so any valid configuration is truncated to 0 and the hardware
is silently programmed with no spread at all.

Divide by 10 and fix the comment accordingly.

Fixes: 77369b1e6a37 ("clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260901155657.6A5981F00A3A@smtp.kernel.org
Reported-by: Peng Fan <peng.fan@nxp.com>
Closes: https://lore.kernel.org/r/ZR6PR04MB375776A27E98F140534E8F7FE488B62@ZR6PR04MB375776.eurprd04.prod.outlook.com
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

Changes in v13:
 - Divide spread_bp by 10 instead of 100 as requested by Peng Fan.
 - Rename the patch title and reword the description accordingly.
 - Add the Reported-by/Closes tags for Sashiko and Peng Fan.

 drivers/clk/clk-scmi-oem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
index be11d359b4ec..c1ebbdc6bbc5 100644
--- a/drivers/clk/clk-scmi-oem.c
+++ b/drivers/clk/clk-scmi-oem.c
@@ -39,12 +39,12 @@ scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
 	u32 val;
 
 	/*
-	 * extConfigValue[7:0]   - spread percentage (%)
+	 * extConfigValue[7:0]   - spread percentage in tenths of a percent
 	 * 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_PERCENTAGE_MASK, ss_conf->spread_bp / 10);
 	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;
-- 
2.43.0

Re: [PATCH v13 1/4] clk: scmi: fix SSC spread conversion

[Peng Fan]

On Thu, Sep 03, 2026 at 05:32:13PM +0200, Dario Binacchi wrote:
>The spread_bp field of struct clk_spread_spectrum holds the modulation
>depth in permyriad (1/100 of a percent), as documented in
>clk-provider.h and in the assigned-clock-sscs binding, while the i.MX
>SCMI OEM extension expects a value in tenths of a percent, as pointed
>out by Peng Fan.
>
>The conversion divides spread_bp by 10000 instead of 10, as reported by
>Sashiko, so any valid configuration is truncated to 0 and the hardware
>is silently programmed with no spread at all.
>
>Divide by 10 and fix the comment accordingly.
>
>Fixes: 77369b1e6a37 ("clk: scmi: Add i.MX95 OEM extension support for SCMI clock driver")
>Reported-by: Sashiko <sashiko-bot@kernel.org>
>Closes: https://lore.kernel.org/r/20260901155657.6A5981F00A3A@smtp.kernel.org
>Reported-by: Peng Fan <peng.fan@nxp.com>
>Closes: https://lore.kernel.org/r/ZR6PR04MB375776A27E98F140534E8F7FE488B62@ZR6PR04MB375776.eurprd04.prod.outlook.com
>Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH v13 2/4] clk: scmi: reject SSC configuration out of the OEM field range

[Dario Binacchi]

The i.MX SCMI OEM extension stores the spread in an 8-bit field, in
tenths of a percent, and the modulation frequency in a 16-bit field.

FIELD_PREP() silently truncates values that do not fit in the target
field. Moreover, the conversion from permyriad to tenths of a percent
turns values below 10 permyriad into zero, which is then passed to the
firmware as no spread at all.

Reject these cases with a warning instead of silently programming a
configuration different from the requested one.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

Changes in v13:
 - New patch

 drivers/clk/clk-scmi-oem.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
index c1ebbdc6bbc5..4dac608edd4c 100644
--- a/drivers/clk/clk-scmi-oem.c
+++ b/drivers/clk/clk-scmi-oem.c
@@ -35,6 +35,7 @@ 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);
+	u32 spread_pm = ss_conf->spread_bp / 10;
 	int ret;
 	u32 val;
 
@@ -44,7 +45,19 @@ scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
 	 * extConfigValue[24]    - Enable/Disable
 	 * extConfigValue[31:25] - Reserved
 	 */
-	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, ss_conf->spread_bp / 10);
+	if (!spread_pm || spread_pm > FIELD_MAX(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK)) {
+		dev_warn(clk->dev, "%s: spread (%u permyriad) out of range\n",
+			 clk_hw_get_name(hw), ss_conf->spread_bp);
+		return -EINVAL;
+	}
+
+	if (ss_conf->modfreq_hz > FIELD_MAX(SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK)) {
+		dev_warn(clk->dev, "%s: modulation frequency (%u Hz) out of range\n",
+			 clk_hw_get_name(hw), ss_conf->modfreq_hz);
+		return -EINVAL;
+	}
+
+	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, spread_pm);
 	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;
-- 
2.43.0

Re: [PATCH v13 2/4] clk: scmi: reject SSC configuration out of the OEM field range

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional rejection of `spread_pm == 0` breaks the ability to disable Spread Spectrum Clocking (SSC).
--

commit bf8cfe80822f02414072ae479a6270e985c74e54
Author: Dario Binacchi <dario.binacchi@amarulasolutions.com>

clk: scmi: reject SSC configuration out of the OEM field range

This commit adds validation to reject spread spectrum configurations that
exceed the 8-bit OEM field range or the 16-bit modulation frequency field.
It aims to prevent silent truncation and incorrect programming of the
firmware.

> diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
> index c1ebbdc6bbc5f..4dac608edd4ce 100644
> --- a/drivers/clk/clk-scmi-oem.c
> +++ b/drivers/clk/clk-scmi-oem.c
> @@ -35,6 +35,7 @@ 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);
> +	u32 spread_pm = ss_conf->spread_bp / 10;
>  	int ret;
>  	u32 val;
>  
> @@ -44,7 +45,19 @@ scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
>  	 * extConfigValue[24]    - Enable/Disable
>  	 * extConfigValue[31:25] - Reserved
>  	 */
> -	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, ss_conf->spread_bp / 10);
> +	if (!spread_pm || spread_pm > FIELD_MAX(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK)) {
> +		dev_warn(clk->dev, "%s: spread (%u permyriad) out of range\n",
> +			 clk_hw_get_name(hw), ss_conf->spread_bp);
> +		return -EINVAL;
> +	}

[Severity: High]
Does this unconditional rejection of spread_pm == 0 break the ability to
disable Spread Spectrum Clocking (SSC)?

When a caller (or device tree via assigned-clock-sscs) requests to disable
SSC, it generally sets method = CLK_SPREAD_NO and spread_bp = 0. Because
spread_pm is calculated as ss_conf->spread_bp / 10, a 0 spread results in
spread_pm == 0, triggering this -EINVAL return.

Since the driver contains logic to handle method == CLK_SPREAD_NO by omitting
the SCMI_CLOCK_IMX_SS_ENABLE_MASK bit, it appears designed to support disabling
SSC. This new check prevents that valid path from executing and could abort
device probing if triggered during initialization.

Should the !spread_pm check be restricted to cases where
ss_conf->method != CLK_SPREAD_NO?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903153836.373267-1-dario.binacchi@amarulasolutions.com?part=2

Re: [PATCH v13 2/4] clk: scmi: reject SSC configuration out of the OEM field range

[Peng Fan]

On Thu, Sep 03, 2026 at 05:32:14PM +0200, Dario Binacchi wrote:
>The i.MX SCMI OEM extension stores the spread in an 8-bit field, in
>tenths of a percent, and the modulation frequency in a 16-bit field.
>
>FIELD_PREP() silently truncates values that do not fit in the target
>field. Moreover, the conversion from permyriad to tenths of a percent
>turns values below 10 permyriad into zero, which is then passed to the
>firmware as no spread at all.
>
>Reject these cases with a warning instead of silently programming a
>configuration different from the requested one.
>
>Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>
>---
>
>Changes in v13:
> - New patch
>
> drivers/clk/clk-scmi-oem.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
>index c1ebbdc6bbc5..4dac608edd4c 100644
>--- a/drivers/clk/clk-scmi-oem.c
>+++ b/drivers/clk/clk-scmi-oem.c
>@@ -35,6 +35,7 @@ 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);
>+	u32 spread_pm = ss_conf->spread_bp / 10;
> 	int ret;
> 	u32 val;
> 
>@@ -44,7 +45,19 @@ scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
> 	 * extConfigValue[24]    - Enable/Disable
> 	 * extConfigValue[31:25] - Reserved
> 	 */
>-	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, ss_conf->spread_bp / 10);
>+	if (!spread_pm || spread_pm > FIELD_MAX(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK)) {
>+		dev_warn(clk->dev, "%s: spread (%u permyriad) out of range\n",
>+			 clk_hw_get_name(hw), ss_conf->spread_bp);
>+		return -EINVAL;

sashiko's comments is valid.

And patch 1 & 3 have fixes tag. Patch 2 and 4 are new patches.
If you would like patch 1 & 3 to be accepted in this 7.13, better post them
as separate patchset. Then CLK maintainer may pick them up for this release.

Regards
Peng

>+	}
>+
>+	if (ss_conf->modfreq_hz > FIELD_MAX(SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK)) {
>+		dev_warn(clk->dev, "%s: modulation frequency (%u Hz) out of range\n",
>+			 clk_hw_get_name(hw), ss_conf->modfreq_hz);
>+		return -EINVAL;
>+	}
>+
>+	val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, spread_pm);
> 	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;
>-- 
>2.43.0
>
>

Re: [PATCH v13 2/4] clk: scmi: reject SSC configuration out of the OEM field range

[Dario Binacchi]

Hi Peng,

On Fri, Sep 4, 2026 at 6:34 AM Peng Fan <peng.fan@oss.nxp.com> wrote:
>
> On Thu, Sep 03, 2026 at 05:32:14PM +0200, Dario Binacchi wrote:
> >The i.MX SCMI OEM extension stores the spread in an 8-bit field, in
> >tenths of a percent, and the modulation frequency in a 16-bit field.
> >
> >FIELD_PREP() silently truncates values that do not fit in the target
> >field. Moreover, the conversion from permyriad to tenths of a percent
> >turns values below 10 permyriad into zero, which is then passed to the
> >firmware as no spread at all.
> >
> >Reject these cases with a warning instead of silently programming a
> >configuration different from the requested one.
> >
> >Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >
> >---
> >
> >Changes in v13:
> > - New patch
> >
> > drivers/clk/clk-scmi-oem.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/clk/clk-scmi-oem.c b/drivers/clk/clk-scmi-oem.c
> >index c1ebbdc6bbc5..4dac608edd4c 100644
> >--- a/drivers/clk/clk-scmi-oem.c
> >+++ b/drivers/clk/clk-scmi-oem.c
> >@@ -35,6 +35,7 @@ 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);
> >+      u32 spread_pm = ss_conf->spread_bp / 10;
> >       int ret;
> >       u32 val;
> >
> >@@ -44,7 +45,19 @@ scmi_clk_imx_set_spread_spectrum(struct clk_hw *hw,
> >        * extConfigValue[24]    - Enable/Disable
> >        * extConfigValue[31:25] - Reserved
> >        */
> >-      val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, ss_conf->spread_bp / 10);
> >+      if (!spread_pm || spread_pm > FIELD_MAX(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK)) {
> >+              dev_warn(clk->dev, "%s: spread (%u permyriad) out of range\n",
> >+                       clk_hw_get_name(hw), ss_conf->spread_bp);
> >+              return -EINVAL;
>
> sashiko's comments is valid.
>
> And patch 1 & 3 have fixes tag. Patch 2 and 4 are new patches.
> If you would like patch 1 & 3 to be accepted in this 7.13, better post them
> as separate patchset. Then CLK maintainer may pick them up for this release.

I'd rather keep a single series, since patch 2 depends on patch 1.
In v14 the two fixes come first so they can be picked up separately.

Sashiko's comment is addressed in v14.

Thanks and regards,
Dario

>
> Regards
> Peng
>
> >+      }
> >+
> >+      if (ss_conf->modfreq_hz > FIELD_MAX(SCMI_CLOCK_IMX_SS_MOD_FREQ_MASK)) {
> >+              dev_warn(clk->dev, "%s: modulation frequency (%u Hz) out of range\n",
> >+                       clk_hw_get_name(hw), ss_conf->modfreq_hz);
> >+              return -EINVAL;
> >+      }
> >+
> >+      val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, spread_pm);
> >       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;
> >--
> >2.43.0
> >
> >



-- 
Dario Binacchi
Senior Embedded Software Engineer
M. +39 328 0625246
dario.binacchi@amarulasolutions.com
―――――――――――――――
Amarula Solutions SRL
 Via Felice Cavallotti 25D, 41012 Carpi, MO, IT
info@amarulasolutions.com
www.amarulasolutions.com

[PATCH v13 3/4] clk: validate spread spectrum configuration

[Dario Binacchi]

The spread spectrum configuration is passed to the provider's
set_spread_spectrum() callback without any validation, as clk-conf.c
only skips all-zero triplets from "assigned-clock-sscs". An invalid
device tree can hand providers a zero modulation frequency or a spread
ratio above 100%, and each provider would have to add the same checks
to protect e.g. divisions in its rate computations.

The KUnit test data for assigned-clock-sscs uses spread values of 30000
and 40000 permyriad (300% and 400%), which the new check rejects, as
reported by Sashiko, so fix them to 300 and 400 (3% and 4%). Also use
a realistic 6% value for the initial settings of the skip tests, for
consistency.

Fixes: c86814e70390 ("clk: Introduce clk_hw_set_spread_spectrum")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260901155657.6A5981F00A3A@smtp.kernel.org
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

Changes in v13:
 - Fix the KUnit test data to realistic spread values.
 - Add the Reported-by/Closes tag for Sashiko.

 drivers/clk/clk.c                      | 14 ++++++++++++++
 drivers/clk/clk_test.c                 | 12 ++++++------
 drivers/clk/kunit_clk_assigned_rates.h |  4 ++--
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fef87167a60b..208caf60eeb5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2851,6 +2851,20 @@ int clk_hw_set_spread_spectrum(struct clk_hw *hw, const struct clk_spread_spectr
 	if (!hw)
 		return 0;
 
+	switch (ss_conf->method) {
+	case CLK_SPREAD_NO:
+		break;
+	case CLK_SPREAD_CENTER:
+	case CLK_SPREAD_UP:
+	case CLK_SPREAD_DOWN:
+		if (!ss_conf->modfreq_hz || !ss_conf->spread_bp ||
+		    ss_conf->spread_bp > 10000)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	core = hw->core;
 
 	clk_prepare_lock();
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 1c5c8b7c1f3c..21e62d68f87f 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -3526,7 +3526,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "provider missing assigned-clocks",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 	},
 	{
 		/*
@@ -3535,7 +3535,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "consumer missing assigned-clocks",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without_consumer),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 		.consumer_test = true,
 	},
 	{
@@ -3545,7 +3545,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "provider assigned-clock-sscs of zero",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 	},
 	{
 		/*
@@ -3554,7 +3554,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "consumer assigned-clock-sscs of zero",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero_consumer),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 		.consumer_test = true,
 	},
 	{
@@ -3564,7 +3564,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "provider assigned-clocks null phandle",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 	},
 	{
 		/*
@@ -3573,7 +3573,7 @@ static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_p
 		 */
 		.desc = "consumer assigned-clocks null phandle",
 		TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null_consumer),
-		.sscs = {50000, 60000, 3},
+		.sscs = {50000, 600, 3},
 		.consumer_test = true,
 	},
 };
diff --git a/drivers/clk/kunit_clk_assigned_rates.h b/drivers/clk/kunit_clk_assigned_rates.h
index d7ae5ec2d25b..c5d9f004ee81 100644
--- a/drivers/clk/kunit_clk_assigned_rates.h
+++ b/drivers/clk/kunit_clk_assigned_rates.h
@@ -9,10 +9,10 @@
 #define ASSIGNED_RATES_1_RATE		9700000
 
 #define ASSIGNED_SSCS_0_MODFREQ		10000
-#define ASSIGNED_SSCS_0_SPREAD		30000
+#define ASSIGNED_SSCS_0_SPREAD		300
 #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_SPREAD		400
 #define ASSIGNED_SSCS_1_METHOD		CLK_SSC_UP_SPREAD
 
 #endif
-- 
2.43.0

Re: [PATCH v13 3/4] clk: validate spread spectrum configuration

[Peng Fan]

On Thu, Sep 03, 2026 at 05:32:15PM +0200, Dario Binacchi wrote:
>The spread spectrum configuration is passed to the provider's
>set_spread_spectrum() callback without any validation, as clk-conf.c
>only skips all-zero triplets from "assigned-clock-sscs". An invalid
>device tree can hand providers a zero modulation frequency or a spread
>ratio above 100%, and each provider would have to add the same checks
>to protect e.g. divisions in its rate computations.
>
>The KUnit test data for assigned-clock-sscs uses spread values of 30000
>and 40000 permyriad (300% and 400%), which the new check rejects, as
>reported by Sashiko, so fix them to 300 and 400 (3% and 4%). Also use
>a realistic 6% value for the initial settings of the skip tests, for
>consistency.
>
>Fixes: c86814e70390 ("clk: Introduce clk_hw_set_spread_spectrum")
>Reported-by: Sashiko <sashiko-bot@kernel.org>
>Closes: https://lore.kernel.org/r/20260901155657.6A5981F00A3A@smtp.kernel.org
>Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH v13 4/4] clk: imx: pll14xx: support spread spectrum clock generation

[Dario Binacchi]

Add support for spread spectrum clock (SSC) generation to the pll14xx
driver.

Tested on the video PLL of i.MX8MN and i.MX8MP based boards.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

(no changes since v12)

Changes in v12:
 - Apply the SSC settings also from the set_spread_spectrum() callback
   to handle the case the clock framework skips set_rate() if the
   requested rate is unchanged.

Changes in v11:
 - Drop the dt-bindings/clock/clock.h include and use the enum
   clk_ssc_method values from clk-provider.h, as clk-scmi-oem.c does.
 - Disable SSC and clear the modulation fields in SSCG_CTRL when no
   spread method is configured, so that a setup left enabled by the
   bootloader is not kept active.
 - Skip the SSC setup instead of dividing by zero when modfreq_hz is
   zero or mfr truncates to zero.
 - Reject mfr and mrr values that do not fit the MFREQ_CTL and MRAT_CTL
   register fields instead of letting FIELD_PREP silently truncate
   them. This also prevents the 10000 * mfr multiplication from
   overflowing.
 - Compute the mfr divisor in 64-bit arithmetic, as the 32-bit product
   could wrap with out of range modfreq_hz values.

Changes in v10:
- Drop 'Reviewed-by' tag of Peng Fan.
- Adapt the driver to the new infrastructure. Implement the
  set_spread_spectrum() and get the modulation parameters from
  struct clk_spread_spectrum.

Changes in v9:
- Add 'Reviewed-by' tag of Peng Fan.

 drivers/clk/imx/clk-pll14xx.c | 99 +++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b6f1cc9f5700..5b187101c558 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -13,6 +13,7 @@
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/math64.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/jiffies.h>
@@ -22,6 +23,8 @@
 #define GNRL_CTL	0x0
 #define DIV_CTL0	0x4
 #define DIV_CTL1	0x8
+#define SSCG_CTRL	0xc
+
 #define LOCK_STATUS	BIT(31)
 #define LOCK_SEL_MASK	BIT(29)
 #define CLKE_MASK	BIT(11)
@@ -33,6 +36,13 @@
 #define KDIV_MASK	GENMASK(15, 0)
 #define KDIV_MIN	SHRT_MIN
 #define KDIV_MAX	SHRT_MAX
+#define SSCG_ENABLE	BIT(31)
+#define MFREQ_CTL_MASK	GENMASK(19, 12)
+#define MRAT_CTL_MASK	GENMASK(9, 4)
+#define SEL_PF_DOWN_SPREAD	0
+#define SEL_PF_UP_SPREAD	1
+#define SEL_PF_CENTER_SPREAD	2
+#define SEL_PF_MASK	GENMASK(1, 0)
 
 #define LOCK_TIMEOUT_US		10000
 
@@ -44,6 +54,7 @@ struct clk_pll14xx {
 	int rate_count;
 	s16 delta_k;
 	spinlock_t lock;
+	struct clk_spread_spectrum ss_conf;
 };
 
 #define to_clk_pll14xx(_hw) container_of(_hw, struct clk_pll14xx, hw)
@@ -366,6 +377,58 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 	return 0;
 }
 
+static void __clk_pll1443x_set_spread_spectrum(struct clk_hw *hw,
+					       unsigned long parent_rate,
+					       unsigned int pdiv,
+					       unsigned int mdiv)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
+	struct clk_spread_spectrum *conf = &pll->ss_conf;
+	u32 sscg_ctrl, mfr, mrr, sel_pf;
+
+	sscg_ctrl = readl_relaxed(pll->base + SSCG_CTRL);
+	sscg_ctrl &= ~(SSCG_ENABLE | MFREQ_CTL_MASK | MRAT_CTL_MASK | SEL_PF_MASK);
+
+	switch (conf->method) {
+	case CLK_SPREAD_CENTER:
+		sel_pf = SEL_PF_CENTER_SPREAD;
+		break;
+	case CLK_SPREAD_UP:
+		sel_pf = SEL_PF_UP_SPREAD;
+		break;
+	case CLK_SPREAD_DOWN:
+		sel_pf = SEL_PF_DOWN_SPREAD;
+		break;
+	default:
+		/* No spread: disable modulation and clear any stale state */
+		goto out;
+	}
+
+	if (!conf->modfreq_hz || !parent_rate || !pdiv)
+		goto out;
+
+	mfr = div64_u64(parent_rate, (u64)conf->modfreq_hz * pdiv * BIT(5));
+	if (!mfr || mfr > FIELD_MAX(MFREQ_CTL_MASK)) {
+		pr_warn("%s: SSC disabled, modulation frequency (%u Hz) out of range\n",
+			clk_hw_get_name(hw), conf->modfreq_hz);
+		goto out;
+	}
+
+	mrr = (conf->spread_bp * mdiv * BIT(6)) / (10000 * mfr);
+	if (!mrr || mrr > FIELD_MAX(MRAT_CTL_MASK)) {
+		pr_warn("%s: SSC disabled, spread (%u permyriad) out of range\n",
+			clk_hw_get_name(hw), conf->spread_bp);
+		goto out;
+	}
+
+	sscg_ctrl |= SSCG_ENABLE | FIELD_PREP(MFREQ_CTL_MASK, mfr) |
+		FIELD_PREP(MRAT_CTL_MASK, mrr) |
+		FIELD_PREP(SEL_PF_MASK, sel_pf);
+
+out:
+	writel_relaxed(sscg_ctrl, pll->base + SSCG_CTRL);
+}
+
 static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 				 unsigned long prate)
 {
@@ -390,6 +453,9 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
 			       pll->base + DIV_CTL1);
 
+		__clk_pll1443x_set_spread_spectrum(hw, prate, rate.pdiv,
+						   rate.mdiv);
+
 		spin_unlock_irqrestore(&pll->lock, flags);
 
 		return 0;
@@ -411,6 +477,8 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base + DIV_CTL1);
 
+	__clk_pll1443x_set_spread_spectrum(hw, prate, rate.pdiv, rate.mdiv);
+
 	spin_unlock_irqrestore(&pll->lock, flags);
 
 	/*
@@ -437,6 +505,36 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	return 0;
 }
 
+static int clk_pll1443x_set_spread_spectrum(struct clk_hw *hw,
+					    const struct clk_spread_spectrum *ss_conf)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
+	struct clk_hw *parent = clk_hw_get_parent(hw);
+	unsigned long parent_rate = parent ? clk_hw_get_rate(parent) : 0;
+	unsigned long flags;
+	u32 div_ctl0;
+
+	spin_lock_irqsave(&pll->lock, flags);
+
+	pll->ss_conf = *ss_conf;
+
+	/*
+	 * Apply the configuration to the hardware right away, using the
+	 * current PLL dividers: the clock framework does not call set_rate()
+	 * if the requested rate is unchanged, so relying on it would leave
+	 * the SSC settings unapplied when the PLL is already at the target
+	 * rate (e.g. configured by the bootloader).
+	 */
+	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
+	__clk_pll1443x_set_spread_spectrum(hw, parent_rate,
+					   FIELD_GET(PDIV_MASK, div_ctl0),
+					   FIELD_GET(MDIV_MASK, div_ctl0));
+
+	spin_unlock_irqrestore(&pll->lock, flags);
+
+	return 0;
+}
+
 static int clk_pll14xx_prepare(struct clk_hw *hw)
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
@@ -509,6 +607,7 @@ static const struct clk_ops clk_pll1443x_ops = {
 	.recalc_rate	= clk_pll14xx_recalc_rate,
 	.determine_rate = clk_pll1443x_determine_rate,
 	.set_rate	= clk_pll1443x_set_rate,
+	.set_spread_spectrum = clk_pll1443x_set_spread_spectrum,
 };
 
 struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
-- 
2.43.0

Re: [PATCH v13 4/4] clk: imx: pll14xx: support spread spectrum clock generation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Potential 32-bit integer overflow in the numerator of the mrr calculation escapes bounds checking.
- [Medium] Invalid SSC configurations generate repeated warnings on every clock rate change.
--

commit 769a49cba9b51c72357919447fa12a82a6548ced
Author: Dario Binacchi <dario.binacchi@amarulasolutions.com>

    clk: imx: pll14xx: support spread spectrum clock generation
    
    Add support for spread spectrum clock (SSC) generation to the pll14xx
    driver.

> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index b6f1cc9f57005..5b187101c558c 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c

[ ... ]

> @@ -366,6 +377,58 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  	return 0;
>  }
>  
> +static void __clk_pll1443x_set_spread_spectrum(struct clk_hw *hw,
> +					       unsigned long parent_rate,
> +					       unsigned int pdiv,
> +					       unsigned int mdiv)
> +{
> +	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> +	struct clk_spread_spectrum *conf = &pll->ss_conf;
> +	u32 sscg_ctrl, mfr, mrr, sel_pf;

[ ... ]

> +	if (!mfr || mfr > FIELD_MAX(MFREQ_CTL_MASK)) {
> +		pr_warn("%s: SSC disabled, modulation frequency (%u Hz) out of range\n",
> +			clk_hw_get_name(hw), conf->modfreq_hz);
> +		goto out;
> +	}
> +
> +	mrr = (conf->spread_bp * mdiv * BIT(6)) / (10000 * mfr);

[Severity: Medium]
Could this calculation overflow 32-bit arithmetic?

If conf->spread_bp from the device tree is a large invalid value, the
left-to-right evaluated multiplication conf->spread_bp * mdiv occurs
in 32-bit arithmetic before being promoted.

Furthermore, on 32-bit architectures where BIT(6) is a 32-bit type,
the entire numerator conf->spread_bp * mdiv * BIT(6) is evaluated in 32 bits,
which can overflow for spread_bp values as low as 65600. This wrapped value
could then inadvertently pass the mrr > FIELD_MAX(...) bounds check that
follows.

While typical values are well within bounds, the commit message mentions
a goal of using 64-bit arithmetic to prevent 32-bit wrapping from
out-of-range DT values. It appears this code misses that protection
for spread_bp.

> +	if (!mrr || mrr > FIELD_MAX(MRAT_CTL_MASK)) {
> +		pr_warn("%s: SSC disabled, spread (%u permyriad) out of range\n",
> +			clk_hw_get_name(hw), conf->spread_bp);
> +		goto out;
> +	}
> +
> +	sscg_ctrl |= SSCG_ENABLE | FIELD_PREP(MFREQ_CTL_MASK, mfr) |
> +		FIELD_PREP(MRAT_CTL_MASK, mrr) |
> +		FIELD_PREP(SEL_PF_MASK, sel_pf);
> +
> +out:
> +	writel_relaxed(sscg_ctrl, pll->base + SSCG_CTRL);
> +}

[ ... ]

> +static int clk_pll1443x_set_spread_spectrum(struct clk_hw *hw,
> +					    const struct clk_spread_spectrum *ss_conf)
> +{
> +	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> +	struct clk_hw *parent = clk_hw_get_parent(hw);
> +	unsigned long parent_rate = parent ? clk_hw_get_rate(parent) : 0;
> +	unsigned long flags;
> +	u32 div_ctl0;
> +
> +	spin_lock_irqsave(&pll->lock, flags);
> +
> +	pll->ss_conf = *ss_conf;

[Severity: Medium]
Will caching an invalid configuration here cause excessive log spam?

If an out-of-bounds spread spectrum configuration is provided, it is
unconditionally cached to pll->ss_conf and 0 is returned instead of an error.

Subsequent runtime calls to clk_pll1443x_set_rate() will re-evaluate this
cached invalid configuration via __clk_pll1443x_set_spread_spectrum(), which
will repeatedly trigger the un-ratelimited pr_warn() calls on every clock
rate change.

> +
> +	/*
> +	 * Apply the configuration to the hardware right away, using the
> +	 * current PLL dividers: the clock framework does not call set_rate()
> +	 * if the requested rate is unchanged, so relying on it would leave
> +	 * the SSC settings unapplied when the PLL is already at the target
> +	 * rate (e.g. configured by the bootloader).
> +	 */
> +	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> +	__clk_pll1443x_set_spread_spectrum(hw, parent_rate,
> +					   FIELD_GET(PDIV_MASK, div_ctl0),
> +					   FIELD_GET(MDIV_MASK, div_ctl0));
> +
> +	spin_unlock_irqrestore(&pll->lock, flags);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903153836.373267-1-dario.binacchi@amarulasolutions.com?part=4

Re: [PATCH v13 4/4] clk: imx: pll14xx: support spread spectrum clock generation

[Peng Fan]

On Thu, Sep 03, 2026 at 05:32:16PM +0200, Dario Binacchi wrote:
>Add support for spread spectrum clock (SSC) generation to the pll14xx
>driver.
>
>Tested on the video PLL of i.MX8MN and i.MX8MP based boards.
>
>Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>