Re: [PATCH v4 07/12] clk: qcom: ipq-cmn-pll: Add PON reference clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk_pon_clk_determine_rate() incorrectly fails valid rate requests due to rounding at bound edges.
--

commit 9a9a4823a8ebca7e3c30590799eb2ac41ada876e
Author: Luo Jie <jie.luo@oss.qualcomm.com>

clk: qcom: ipq-cmn-pll: Add PON reference clock support

This commit adds support for the PON reference clock to the IPQ CMN PLL driver.
It configures the clock to operate in either UNIPHY mode with a fixed 31.25 MHz
rate, or PON mode using an 8-bit divider derived from the parent rate.

> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
> index 44d035870a89b..f12352a707f7a 100644
> --- a/drivers/clk/qcom/ipq-cmn-pll.c
> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
[ ... ]
> +static int clk_pon_clk_determine_rate(struct clk_hw *hw,
> +				      struct clk_rate_request *req)
> +{
> +	unsigned long div, pon_rate, uniphy_rate = 31250000UL;
> +	bool uniphy_rate_valid, pon_rate_valid;
> +
> +	if (!req->rate || !req->best_parent_rate)
> +		return -EINVAL;
> +
> +	div = DIV64_U64_ROUND_CLOSEST((u64)req->best_parent_rate, req->rate);
> +
> +	/* Clamp to valid range (1-255) */
> +	div = clamp_t(unsigned long, div, 1, 255);
> +
> +	pon_rate = DIV_ROUND_CLOSEST_ULL((u64)req->best_parent_rate, div);
> +
> +	uniphy_rate_valid = uniphy_rate >= req->min_rate &&
> +			    uniphy_rate <= req->max_rate;
> +	pon_rate_valid = pon_rate >= req->min_rate &&
> +			 pon_rate <= req->max_rate;
> +
> +	if (!uniphy_rate_valid && !pon_rate_valid)
> +		return -EINVAL;

[Severity: High]
Will this logic incorrectly reject valid rate requests when the closest
divider falls slightly outside the boundaries due to rounding?

If a client restricts the bounds (e.g., via clk_set_min_rate), the requested
rate is clamped to the bounds. If calculating the closest divider for that
rate yields a frequency that barely misses the min_rate or max_rate bound, 
pon_rate_valid becomes false and this returns -EINVAL.

Should this code check adjacent dividers (such as div - 1 or div + 1) to see
if they yield a rate strictly within the requested bounds before giving up?

> +
> +	/* Pick whichever mode gets closer to the requested rate */
> +	if (uniphy_rate_valid && pon_rate_valid) {
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-qcom_ipq5210_cmnpll-v4-0-aec5ed642f4f@oss.qualcomm.com?part=7