Re: [PATCH v11 1/2] clk: validate spread spectrum configuration

[Dario Binacchi]

Hi Peng,

On Wed, Sep 2, 2026 at 10:33 AM Peng Fan <peng.fan@xxxxxxxxxxx> wrote:
>
> On Tue, Sep 01, 2026 at 11:05:17AM +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.
> >
> >Fixes: c86814e70390 ("clk: Introduce clk_hw_set_spread_spectrum")
> >Signed-off-by: Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx>
> >---
> >
> >(no changes since v1)
> >
> > drivers/clk/clk.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> >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;
>
> I think sashiko comment is wrong.
> In [1], we reached an agreement that spread "in permyriad, i.e. 0.01%".
> So the input value must be the real value * 10000, saying 0.01% * 10000

The DT cell is a u32 holding the integer permyriad value, i.e. the
ratio * 10000 as agreed in [1]: 1% is written as 100, 100% as 10000.
extConfigValue[7:0] takes an integer percentage, so the conversion
is a division by 100, not by 10000:

  depth   DT value   / 10000   / 100
  0.1%        10         0        0
  1%         100         0        1
  3%         300         0        3
  100%     10000         1      100

With integer arithmetic the current code programs 0% for any depth
below 100%. I am sending the fix as the first patch of v12.

[1] https://github.com/devicetree-org/dt-schema/pull/154

Thanks and regards,
Dario
>
> [1] https://github.com/devicetree-org/dt-schema/pull/154
>
> Regards
> Peng
>
> >+              break;
> >+      default:
> >+              return -EINVAL;
> >+      }
> >+
> >       core = hw->core;
> >
> >       clk_prepare_lock();
> >--
> >2.43.0
> >
> >



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