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

[sashiko-bot@kernel.org]

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