Re: [PATCH v3 16/19] phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers
From: Chaoyi Chen <hidden>
Date: 2026-09-07 03:26:03
Also in:
dri-devel, linux-arm-kernel, linux-clk, linux-phy, linux-riscv, linux-rockchip, lkml
Hello Michal, On 9/4/2026 9:27 PM, Michal Wilczynski wrote:
The RK3328 pre-PLL programming, its table lookup and its recalc_rate and determine_rate implementations are the generic Innosilicon ones, so drop the local copies and use the shared helpers instead. The RK3228 pre-PLL sits at different register addresses, so it keeps its own register level code and only shares the table lookup. The now unused RK3328 pre-PLL register macros go with it, as does the local pre-PLL config lookup wrapper; both call sites already have the TMDS clock to hand and call inno_hdmi_phy_pre_pll_lookup() directly. The RK3328 recalc_rate used to log the pre-PLL output as "vco". The shared helper logs it too, and reports the actual VCO frequency alongside the rate, which is the value with the 1.4-3.2 GHz constraint. The register writes, their order and the values written are unchanged. No functional change intended. Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/phy/rockchip/Kconfig | 1 + drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 165 ++++---------------------- 2 files changed, 26 insertions(+), 140 deletions(-)
[...]
quoted hunk ↗ jump to hunk
@@ -915,39 +858,11 @@ unsigned long inno_hdmi_phy_rk3328_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw); - unsigned long frac; - u8 nd, no_a, no_b, no_d; - u64 vco; - u16 nf; - - nd = inno_read(inno, 0xa1) & RK3328_PRE_PLL_PRE_DIV_MASK; - nf = ((inno_read(inno, 0xa2) & RK3328_PRE_PLL_FB_DIV_11_8_MASK) << 8); - nf |= inno_read(inno, 0xa3); - vco = parent_rate * nf; - - if (!(inno_read(inno, 0xa2) & RK3328_PRE_PLL_FRAC_DIV_DISABLE)) { - frac = inno_read(inno, 0xd3) | - (inno_read(inno, 0xd2) << 8) | - (inno_read(inno, 0xd1) << 16); - vco += DIV_ROUND_CLOSEST(parent_rate * frac, (1 << 24)); - } - - if (inno_read(inno, 0xa0) & RK3328_PCLK_VCO_DIV_5_MASK) { - do_div(vco, nd * 5); - } else { - no_a = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_A_MASK; - no_b = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_B_MASK; - no_b >>= RK3328_PRE_PLL_PCLK_DIV_B_SHIFT; - no_b += 2; - no_d = inno_read(inno, 0xa6) & RK3328_PRE_PLL_PCLK_DIV_D_MASK; - - do_div(vco, (nd * (no_a == 1 ? no_b : no_a) * no_d * 2)); - } - inno->pixclock = DIV_ROUND_CLOSEST((unsigned long)vco, 1000) * 1000; + inno->pixclock = inno_hdmi_phy_pre_pll_recalc_rate(&inno->pre_pll, + parent_rate); - dev_dbg(inno->dev, "%s rate %lu vco %llu\n", - __func__, inno->pixclock, vco); + dev_dbg(inno->dev, "%s rate %lu\n", __func__, inno->pixclock);
The printing here is redundant, because it has already been handled in inno_hdmi_phy_pre_pll_recalc_rate(). Reviewed-by: Chaoyi Chen <redacted>
return inno->pixclock; }
-- Best, Chaoyi