[PATCH v2 1/6] phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc
Cristian Ciocaltea
cristian.ciocaltea at collabora.com
Wed May 13 09:12:23 PDT 2026
On 5/13/26 1:51 AM, sashiko-bot at kernel.org 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 <cristian.ciocaltea at 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?
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/20260328-hdptx-ffe-v1-0-53ebd5dea20a@collabora.com/
More information about the linux-phy
mailing list