[PATCH v5 03/10] phy: rockchip: samsung-hdptx: Guard against clk rate integer underflow

[Cristian Ciocaltea]

On 7/23/26 10:57 PM, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] rk_hdptx_phy_clk_determine_rate() incorrectly returns a clock rate directly instead of 0 on success, violating the CCF API.
> --
> 
> --- Patch [3]: [PATCH v5 03/10] phy: rockchip: samsung-hdptx: Guard against clk rate integer underflow ---
> commit cf68603e6625cb39a98418e9f792a4e2e373dfcd
> Author: Cristian Ciocaltea <cristian.ciocaltea at collabora.com>
> 
> phy: rockchip: samsung-hdptx: Guard against clk rate integer underflow
> 
> This commit adds a check to avoid an integer underflow in the ROPLL
> hardware state calculation. By validating that the computed sigma-delta
> modulator offset does not exceed the base frequency, it prevents
> returning excessively high clock rates to the Common Clock Framework.
> 
> Closes: https://lore.kernel.org/all/20260611235702.0E9691F000E9@smtp.kernel.org/
> 
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index 44e99343f2499..fcf04e9814cc6 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -2280,10 +2280,16 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
>>  		sdm = div_u64(PLL_REF_CLK * ropll_hw.sdc_deno *
>>  			      ropll_hw.pms_mdiv * ropll_hw.sdm_num, val);
>>  
>> -		if (ropll_hw.sdm_num_sign)
>> +		if (ropll_hw.sdm_num_sign) {
>> +			if (sdm > fout) {
>> +				dev_dbg(hdptx->dev, "Invalid ROPLL hw state: sdm > fout\n");
>> +				return 0;
>> +			}
>> +
>>  			fout = fout - sdm;
>> -		else
>> +		} else {
>>  			fout = fout + sdm;
>> +		}
>>  	}
>>  
>>  	return DIV_ROUND_CLOSEST_ULL(fout * 2 * 8, ropll_hw.pms_sdiv * 10 * bpc);
>>  }
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential problem in
> rk_hdptx_phy_clk_determine_rate() located nearby in the same file:
> 
> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_clk_determine_rate() {
> 	struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
> 
> 	if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
> 		return hdptx->hdmi_cfg.rate;
> 	...
> }
> 
> Does this violate the Common Clock Framework API contract for
> .determine_rate callbacks?
> 
> The .determine_rate callback is expected to update req->rate and return 0
> on success. By returning the rate directly, it appears this fails to update
> the request and instead returns a massive integer.
> 
> For FRL_8G4L_RATE (3,200,000,000), casting it to a 32-bit signed int would
> wrap around to a negative value (-1,094,967,296).
> 
> When the clock rate is negotiated or set for 8Gbps FRL mode, CCF could
> receive this wrapped negative value, interpret it as an error code, and
> cause clk_set_rate() to fail entirely.
> 
> Even for smaller rates that do not wrap, leaving req->rate unmodified might
> bypass rounding and cause downstream failures in HDMI configuration.
> 
> Should this instead update req->rate and return 0?

This is implicitly fixed by [PATCH v5 05/10] phy: rockchip: samsung-hdptx:
Handle uncommitted PHY config changes. If a new revision is required, I can
provide a dedicated patch.