Re: [PATCH v11 2/6] clk: Introduce clk_hw_set_spread_spectrum

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The clock's PM domain is unconditionally resumed before checking if the `set_spread_spectrum` callback is supported, causing an unnecessary hardware wakeup.
--

--- Patch [2]: [PATCH v11 2/6] clk: Introduce clk_hw_set_spread_spectrum ---
commit f5617081c24679334412209893c7d5c3a6321d1f
Author: Peng Fan <[email protected]>

clk: Introduce clk_hw_set_spread_spectrum

This commit introduces the clk_hw_set_spread_spectrum API to allow clock
drivers to configure spread spectrum features. It adds a set_spread_spectrum
callback to clk_ops for hardware-specific implementations.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 048adfa86a5d0..8c78621cde253 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c

[ ... ]

> @@ -2774,6 +2774,33 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
>  }
>  EXPORT_SYMBOL_GPL(clk_set_max_rate);
>  
> +int clk_hw_set_spread_spectrum(struct clk_hw *hw, const struct clk_spread_spectrum *ss_conf)
> +{
> +	struct clk_core *core;
> +	int ret;
> +
> +	if (!hw)
> +		return 0;
> +
> +	core = hw->core;
> +
> +	clk_prepare_lock();
> +
> +	ret = clk_pm_runtime_get(core);
> +	if (ret)
> +		goto fail;
> +
> +	if (core->ops->set_spread_spectrum)
> +		ret = core->ops->set_spread_spectrum(hw, ss_conf);

[Severity: Medium]
Is it necessary to wake up the device via PM runtime before verifying that
the set_spread_spectrum callback is supported? Waking up the device might
involve slow hardware communication or powering up a PM domain.

Also, if the callback is not populated, ret will remain 0 from the
successful clk_pm_runtime_get call. Does this mean the function will return
success when it actually failed to set the spread spectrum, instead of
returning an error like -EOPNOTSUPP?

> +
> +	clk_pm_runtime_put(core);
> +
> +fail:
> +	clk_prepare_unlock();
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(clk_hw_set_spread_spectrum);
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2