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 <[email protected]>
> ---
> drivers/phy/rockchip/Kconfig | 1 +
> drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 165
> ++++----------------------
> 2 files changed, 26 insertions(+), 140 deletions(-)
>
[...]
>
> @@ -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 <[email protected]>
> return inno->pixclock;
> }
--
Best,
Chaoyi
