Re: [PATCH v2 1/6] phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc

On 5/13/26 1:51 AM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Clock rate calculation drops a rounding step, replacing DIV_ROUND_CLOSEST_ULL() with truncation via div_u64(), which can cause a 1 Hz clock rate mismatch.
> --
> 
> commit f013b4b7bc1d4ae235d28b10cad3143d009d131c
> Author: Cristian Ciocaltea <[email protected]>
> 
> phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc
> 
> This commit fixes the clock rate recalculation in the HDMI PHY probe path
> by directly reading the bpc value from hardware registers instead of
> relying on uninitialized configuration struct members.
> 
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index 2d973bc37f076..7fb1c22318bbf 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -2280,7 +2281,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
>>  			fout = fout + sdm;
>>  	}
>>  
>> -	return div_u64(fout * 2, ropll_hw.pms_sdiv * 10);
>> +	return div_u64(fout * 2 * 8, ropll_hw.pms_sdiv * 10 * bpc);
>>  }
>>  
>>  static unsigned long rk_hdptx_phy_clk_recalc_rate(struct clk_hw *hw,
>> @@ -2288,19 +2289,13 @@ static unsigned long rk_hdptx_phy_clk_recalc_rate(struct clk_hw *hw,
>>  {
>>  	struct rk_hdptx_phy *hdptx = to_rk_hdptx_phy(hw);
>>  	u32 status;
>> -	u64 rate;
>>  	int ret;
>>  
>>  	ret = regmap_read(hdptx->grf, GRF_HDPTX_CON0, &status);
>>  	if (ret || !(status & HDPTX_I_PLL_EN))
>>  		return 0;
>>  
>> -	rate = rk_hdptx_phy_clk_calc_rate_from_pll_cfg(hdptx);
>> -
>> -	if (hdptx->hdmi_cfg.mode == PHY_HDMI_MODE_FRL)
>> -		return rate;
>> -
>> -	return DIV_ROUND_CLOSEST_ULL(rate * 8, hdptx->hdmi_cfg.bpc);
>> +	return rk_hdptx_phy_clk_calc_rate_from_pll_cfg(hdptx);
> 
> Does this inadvertently change the rounding behavior?
> 
> By moving the * 8 and / bpc math into rk_hdptx_phy_clk_calc_rate_from_pll_cfg()
> and using div_u64(), the result is now truncated instead of rounded.
> 
> For example, if the exact calculated value ends in .6 or .8, the truncation
> will yield a value 1 Hz lower than the rounded value.
> 
> Since rk_hdptx_phy_clk_determine_rate() still calculates the expected rate
> using DIV_ROUND_CLOSEST_ULL(), could this truncation lead to a 1 Hz mismatch
> in the Common Clock Framework?

I couldn't notice any change in the functional behavior, but for consistency we
can stick with DIV_ROUND_CLOSEST_ULL().  I'll make this change in the next
revision, if any required, otherwise it will be submitted as part of the FRL
TxFFE level control series:

https://lore.kernel.org/all/[email protected]/