All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dario Binacchi" <dario.binacchi@amarulasolutions.com>
Cc: linux-clk@vger.kernel.org
Subject: Re: [PATCH v11 1/2] clk: validate spread spectrum configuration
Date: Tue, 01 Sep 2026 15:56:56 +0000	[thread overview]
Message-ID: <20260901155657.6A5981F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260901090912.585681-2-dario.binacchi@amarulasolutions.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer division truncates valid spread spectrum configurations to zero, silently disabling the feature.
--

commit f9da34cc1be223c5bb63d6e1c7898cf344a88cfe
Author: Dario Binacchi <dario.binacchi@amarulasolutions.com>
clk: validate spread spectrum configuration

This commit adds validation to the clk_hw_set_spread_spectrum function to
ensure that the modulation frequency is non-zero and the spread ratio does
not exceed 10000 basis points (100%). This prevents invalid device tree
configurations from passing incorrect values to clock providers.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> --- 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)

[Severity: High]
This isn't a bug introduced by this patch, but looking at
scmi_clk_imx_set_spread_spectrum() in drivers/clk/clk-scmi-oem.c:

scmi_clk_imx_set_spread_spectrum() {
    ...
    val = FIELD_PREP(SCMI_CLOCK_IMX_SS_PERCENTAGE_MASK, ss_conf->spread_bp / 10000);
    ...
}

Does this integer division incorrectly assume spread_bp is a direct percentage
multiplier rather than basis points? For standard device tree configurations,
it seems this would truncate valid permyriad percentages (e.g., 100 for 1%)
to 0. Would this result in the hardware being programmed with a 0% spread
instead of the configured value, effectively disabling the feature silently?

> +			return -EINVAL;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>  	core = hw->core;
>  
>  	clk_prepare_lock();

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

  reply	other threads:[~2026-09-01 15:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  9:05 [PATCH v11 0/2] Support spread spectrum clocking for i.MX8M PLLs Dario Binacchi
2026-09-01  9:05 ` [PATCH v11 1/2] clk: validate spread spectrum configuration Dario Binacchi
2026-09-01 15:56   ` sashiko-bot [this message]
2026-09-02  8:38   ` Peng Fan
2026-09-02 10:14     ` Dario Binacchi
2026-09-03  1:51       ` Peng Fan
2026-09-01  9:05 ` [PATCH v11 2/2] clk: imx: pll14xx: support spread spectrum clock generation Dario Binacchi
2026-09-01 16:01   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260901155657.6A5981F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dario.binacchi@amarulasolutions.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.