Re: [PATCH v3 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 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential divide-by-zero when computing SDM fraction if bootloader leaves specific PLL registers uninitialized or misconfigured.
- [High] FRL_8G4L_RATE (3.2GHz) cascade mode is not handled correctly in clock rate recalculation, causing it to return 0.
--

commit 7c989d5f5cf68e3aea19b9dd58be7c8b907619aa
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

phy: rockchip: samsung-hdptx: Fix rate recalculation for high bpc

This patch updates the rate recalculation logic to read the bpc value directly
from hardware registers instead of relying on the uninitialized HDMI PHY
configuration. This resolves incorrect rate calculations when bpc is higher
than 8.

> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> index 2d973bc37f076..710603afff866 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> @@ -2168,7 +2168,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
>  	struct lcpll_config lcpll_hw;
>  	struct ropll_config ropll_hw;
>  	u64 fout, sdm;
> -	u32 mode, val;
> +	u32 mode, bpc, val;
>  	int ret, i;
>  
>  	ret = regmap_read(hdptx->regmap, CMN_REG(0008), &mode);

[Severity: High]
This is a pre-existing issue, but does 
rk_hdptx_phy_clk_calc_rate_from_pll_cfg() fail to handle cascade mode for the
3.2GHz FRL_8G4L_RATE?

When the hardware is configured in cascade mode using both LCPLL and ROPLL,
CMN_REG(0008) has LCPLL_LCVCO_MODE_EN_MASK set.

The function sees this bit and assumes the rate must be determined solely
from the rk_hdptx_frl_lcpll_cfg array:

drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_clk_calc_rate_from_pll_cfg() {
    ...
    if (mode & LCPLL_LCVCO_MODE_EN_MASK) {
        ...
        for (i = 0; i < ARRAY_SIZE(rk_hdptx_frl_lcpll_cfg); i++) {
            ...
                return cfg->rate;
        }

        dev_dbg(hdptx->dev, "%s no FRL match found\n", __func__);
        return 0;
    }
    ...
}

For the 3.2GHz config, it searches the array for an entry with pms_mdiv of
107, finds no match, and returns 0 instead of falling through to the ROPLL
calculation.

Could this bypass the ROPLL calculation and cause the clock framework to
register a 0 Hz rate for the 3.2GHz HDMI mode?

[ ... ]

> @@ -2266,6 +2266,7 @@ static u64 rk_hdptx_phy_clk_calc_rate_from_pll_cfg(struct rk_hdptx_phy *hdptx)
>  	if (ret)
>  		return 0;
>  	ropll_hw.pms_sdiv = ((val & PLL_PCG_POSTDIV_SEL_MASK) >> 4) + 1;
> +	bpc = (FIELD_GET(PLL_PCG_CLK_SEL_MASK, val) << 1) + 8;
>  
>  	fout = PLL_REF_CLK * ropll_hw.pms_mdiv;
>  	if (ropll_hw.sdm_en) {

[Severity: High]
This is a pre-existing issue, but could this division trigger a
divide-by-zero panic if the bootloader leaves the PLL registers
uninitialized or misconfigured?

The calculation uses div_u64() with a denominator read directly from hardware:

drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_clk_calc_rate_from_pll_cfg() {
    ...
    if (ropll_hw.sdm_en) {
        sdm = div_u64(PLL_REF_CLK * ropll_hw.sdc_deno *
                      ropll_hw.pms_mdiv * ropll_hw.sdm_num,
                      16 * ropll_hw.sdm_deno *
                      (ropll_hw.sdc_deno * ropll_hw.sdc_n - ropll_hw.sdc_num));
    ...
}

The values ropll_hw.sdm_deno, ropll_hw.sdc_deno, ropll_hw.sdc_n, and
ropll_hw.sdc_num are populated from CMN_REG(0060), CMN_REG(0070), etc.,
without any zero-checks.

If the PLL is enabled but these registers evaluate to 0, it appears this
would cause a panic during driver probe or when the clock framework queries
the rate.

Is it safe to directly trust the hardware register state for this denominator
without sanitization?

> @@ -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_ROUND_CLOSEST_ULL(fout * 2 * 8, ropll_hw.pms_sdiv * 10 * bpc);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-hdptx-clk-fixes-v3-0-67b1b0c00e16@collabora.com?part=1

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