Re: [PATCH v2 08/12] clk: qcom: ipq-cmn-pll: Add EPHY-RAW clock support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk_ephy_raw_determine_rate() fails to respect req->min_rate and req->max_rate boundaries.
--

--- Patch [8]: [PATCH v2 08/12] clk: qcom: ipq-cmn-pll: Add EPHY-RAW clock support ---
commit 49a391af3ca59aa96545f77e2d5dd595f1603325
Author: Luo Jie <[email protected]>

clk: qcom: ipq-cmn-pll: Add EPHY-RAW clock support

This commit introduces EPHY-RAW clock support for the IPQ5210 from the CMN 
PLL, providing the raw reference clock to the SoC-integrated Ethernet PHY. It 
adds clock operations and register definitions to select the output frequency 
between 125 MHz and 312.5 MHz based on the required link speed.

> diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
> index 356988c03a7ae..586f9c620e7e1 100644
> --- a/drivers/clk/qcom/ipq-cmn-pll.c
> +++ b/drivers/clk/qcom/ipq-cmn-pll.c

[ ... ]

> +static int clk_ephy_raw_determine_rate(struct clk_hw *hw,
> +				       struct clk_rate_request *req)
> +{
> +	unsigned long rate_125m = 125000000UL, rate_312p5m = 312500000UL;
> +
> +	/* Pick whichever of the two supported rates is closer to the request */
> +	req->rate = abs_diff(req->rate, rate_125m) < abs_diff(req->rate, rate_312p5m) ?
> +		    rate_125m : rate_312p5m;
> +
> +	return 0;
> +}

[Severity: Medium]
Does this rate selection logic need to check the req->min_rate and 
req->max_rate boundaries?

It appears that the target frequency is selected unconditionally based solely
on the absolute difference from req->rate, ignoring bounds applied via QoS or
OPP constraints. 

If a clock consumer requests a rate with a strict upper boundary (for example,
req->max_rate is 250 MHz and req->rate is clamped to 250 MHz), the absolute
difference to 312.5 MHz (62.5 MHz) is less than the difference to 125 MHz 
(125 MHz). 

Could this result in selecting a frequency outside the permissible bounds, 
violating the Common Clock Framework determine_rate API contract and causing
upstream rate-setting routines to fail?

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