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

[sashiko-bot@kernel.org]

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 <cristian.ciocaltea@collabora.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-hdptx-clk-fixes-v2-0-664e41379cab@collabora.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy