[PATCH v2 12/15] phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers
Jonas Karlman
jonas at kwiboo.se
Tue Sep 1 11:14:23 PDT 2026
Hi Michal,
On 8/28/2026 3:47 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.
RK3528 and RK3538/RK3539 also seems to use the generic Innosilicon ones,
so we should hopefully also be able to use the helpers for those SoCs.
>
> The RK3228 pre-PLL sits at different register addresses, so it keeps its
> own register level code and only shares the table lookup.
>
> The register writes, their order and the values written are unchanged.
> No functional change intended.
>
> Signed-off-by: Michal Wilczynski <m.wilczynski at samsung.com>
> ---
> drivers/phy/rockchip/Kconfig | 1 +
> drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 127 +++++---------------------
> 2 files changed, 25 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index 14698571b607590dc0e7a29f3b39eb9a5d4e3d25..9732c5be87717dc3ddbea1c60080952336a07af9 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -34,6 +34,7 @@ config PHY_ROCKCHIP_INNO_HDMI
> depends on COMMON_CLK
> depends on HAS_IOMEM
> select GENERIC_PHY
> + select PHY_INNO_HDMI
> help
> Enable this to support the Rockchip Innosilicon HDMI PHY.
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> index 1483907413faa5ce1dc8614db05e6e929410404d..fae766607b0ea120b3adb36001387062f39a5c00 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> @@ -17,6 +17,7 @@
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/phy/inno-hdmi-phy.h>
> #include <linux/phy/phy.h>
> #include <linux/slab.h>
>
> @@ -240,6 +241,8 @@ struct inno_hdmi_phy {
> const struct inno_hdmi_phy_drv_data *plat_data;
> int chip_version;
>
> + struct inno_hdmi_phy_pre_pll pre_pll;
> +
> /* clk provider */
> struct clk_hw hw;
> struct clk *phyclk;
> @@ -247,21 +250,6 @@ struct inno_hdmi_phy {
> unsigned long tmdsclock;
> };
>
> -struct pre_pll_config {
> - unsigned long pixclock;
> - unsigned long tmdsclock;
> - u8 prediv;
> - u16 fbdiv;
> - u8 tmds_div_a;
> - u8 tmds_div_b;
> - u8 tmds_div_c;
> - u8 pclk_div_a;
> - u8 pclk_div_b;
> - u8 pclk_div_c;
> - u8 pclk_div_d;
> - u8 vco_div_5_en;
> - u32 fracdiv;
> -};
>
> struct post_pll_config {
> unsigned long tmdsclock;
> @@ -290,7 +278,7 @@ struct inno_hdmi_phy_drv_data {
> const struct phy_config *phy_cfg_table;
> };
>
> -static const struct pre_pll_config pre_pll_cfg_table[] = {
> +static const struct inno_hdmi_phy_pre_pll_config pre_pll_cfg_table[] = {
> { 25175000, 25175000, 3, 125, 3, 1, 1, 1, 3, 3, 4, 0, 0xe00000},
> { 25175000, 31468750, 1, 41, 0, 3, 3, 1, 3, 3, 4, 0, 0xf5554f},
> { 27000000, 27000000, 1, 36, 0, 3, 3, 1, 2, 3, 4, 0, 0x0},
> @@ -673,20 +661,12 @@ static const struct phy_ops inno_hdmi_phy_ops = {
> };
>
> static const
> -struct pre_pll_config *inno_hdmi_phy_get_pre_pll_cfg(struct inno_hdmi_phy *inno,
> - unsigned long rate)
> +struct inno_hdmi_phy_pre_pll_config *inno_hdmi_phy_get_pre_pll_cfg(struct inno_hdmi_phy *inno,
> + unsigned long rate)
> {
> - const struct pre_pll_config *cfg = pre_pll_cfg_table;
> unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
>
> - for (; cfg->pixclock != 0; cfg++)
> - if (cfg->pixclock == rate && cfg->tmdsclock == tmdsclock)
> - break;
> -
> - if (cfg->pixclock == 0)
> - return ERR_PTR(-EINVAL);
> -
> - return cfg;
> + return inno_hdmi_phy_pre_pll_lookup(&inno->pre_pll, rate, tmdsclock);
Please fully remove the local inno_hdmi_phy_get_pre_pll_cfg() helper and
call inno_hdmi_phy_pre_pll_lookup() in its place.
> }
>
> static int inno_hdmi_phy_rk3228_clk_is_prepared(struct clk_hw *hw)
> @@ -752,7 +732,7 @@ unsigned long inno_hdmi_phy_rk3228_clk_recalc_rate(struct clk_hw *hw,
> static int inno_hdmi_phy_rk3228_clk_determine_rate(struct clk_hw *hw,
> struct clk_rate_request *req)
> {
> - const struct pre_pll_config *cfg = pre_pll_cfg_table;
> + const struct inno_hdmi_phy_pre_pll_config *cfg = pre_pll_cfg_table;
>
> req->rate = (req->rate / 1000) * 1000;
>
> @@ -773,7 +753,7 @@ static int inno_hdmi_phy_rk3228_clk_set_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> {
> struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
> - const struct pre_pll_config *cfg;
> + const struct inno_hdmi_phy_pre_pll_config *cfg;
> unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
> u32 v;
> int ret;
> @@ -870,39 +850,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 = inno_hdmi_phy_pre_pll_recalc_rate(&inno->pre_pll,
> + parent_rate);
>
> - inno->pixclock = DIV_ROUND_CLOSEST((unsigned long)vco, 1000) * 1000;
> -
> - 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);
VCO should be between 1.4-3.2 GHz, we now loose this debug information.
This is a change that probably should be noted in the commit message.
>
> return inno->pixclock;
> }
> @@ -910,20 +862,9 @@ unsigned long inno_hdmi_phy_rk3328_clk_recalc_rate(struct clk_hw *hw,
> static int inno_hdmi_phy_rk3328_clk_determine_rate(struct clk_hw *hw,
> struct clk_rate_request *req)
> {
> - const struct pre_pll_config *cfg = pre_pll_cfg_table;
> -
> - req->rate = (req->rate / 1000) * 1000;
> -
> - for (; cfg->pixclock != 0; cfg++)
> - if (cfg->pixclock == req->rate)
> - break;
> -
> - if (cfg->pixclock == 0)
> - return -EINVAL;
> -
> - req->rate = cfg->pixclock;
> + struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
>
> - return 0;
> + return inno_hdmi_phy_pre_pll_determine_rate(&inno->pre_pll, req);
> }
>
> static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
> @@ -931,9 +872,8 @@ static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> {
> struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
> - const struct pre_pll_config *cfg;
> + const struct inno_hdmi_phy_pre_pll_config *cfg;
> unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
> - u32 val;
> int ret;
>
> dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
> @@ -946,35 +886,11 @@ static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
> if (IS_ERR(cfg))
> return PTR_ERR(cfg);
>
> - inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN,
> - RK3328_PRE_PLL_POWER_DOWN);
> -
> - /* Configure pre-pll */
> - inno_update_bits(inno, 0xa0, RK3328_PCLK_VCO_DIV_5_MASK,
> - RK3328_PCLK_VCO_DIV_5(cfg->vco_div_5_en));
> - inno_write(inno, 0xa1, RK3328_PRE_PLL_PRE_DIV(cfg->prediv));
> -
> - val = RK3328_SPREAD_SPECTRUM_MOD_DISABLE;
> - if (!cfg->fracdiv)
> - val |= RK3328_PRE_PLL_FRAC_DIV_DISABLE;
> - inno_write(inno, 0xa2, RK3328_PRE_PLL_FB_DIV_11_8(cfg->fbdiv) | val);
> - inno_write(inno, 0xa3, RK3328_PRE_PLL_FB_DIV_7_0(cfg->fbdiv));
> - inno_write(inno, 0xa5, RK3328_PRE_PLL_PCLK_DIV_A(cfg->pclk_div_a) |
> - RK3328_PRE_PLL_PCLK_DIV_B(cfg->pclk_div_b));
> - inno_write(inno, 0xa6, RK3328_PRE_PLL_PCLK_DIV_C(cfg->pclk_div_c) |
> - RK3328_PRE_PLL_PCLK_DIV_D(cfg->pclk_div_d));
> - inno_write(inno, 0xa4, RK3328_PRE_PLL_TMDSCLK_DIV_C(cfg->tmds_div_c) |
> - RK3328_PRE_PLL_TMDSCLK_DIV_A(cfg->tmds_div_a) |
> - RK3328_PRE_PLL_TMDSCLK_DIV_B(cfg->tmds_div_b));
> - inno_write(inno, 0xd3, RK3328_PRE_PLL_FRAC_DIV_7_0(cfg->fracdiv));
> - inno_write(inno, 0xd2, RK3328_PRE_PLL_FRAC_DIV_15_8(cfg->fracdiv));
> - inno_write(inno, 0xd1, RK3328_PRE_PLL_FRAC_DIV_23_16(cfg->fracdiv));
> + inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, true);
> + inno_hdmi_phy_pre_pll_configure(&inno->pre_pll, cfg);
> + inno_hdmi_phy_pre_pll_power_down(&inno->pre_pll, false);
>
> - inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN, 0);
This should likely also remove all now unused RK3328 pre_pll related
macros.
Regards,
Jonas
> -
> - /* Wait for Pre-PLL lock */
> - ret = inno_poll(inno, 0xa9, val, val & RK3328_PRE_PLL_LOCK_STATUS,
> - 1000, 10000);
> + ret = inno_hdmi_phy_pre_pll_wait_locked(&inno->pre_pll, 10000);
> if (ret) {
> dev_err(inno->dev, "Pre-PLL locking failed\n");
> return ret;
> @@ -1373,6 +1289,11 @@ static int inno_hdmi_phy_probe(struct platform_device *pdev)
> if (IS_ERR(inno->regmap))
> return PTR_ERR(inno->regmap);
>
> + /* The PHY block starts the register space on these SoCs. */
> + inno->pre_pll.regmap = inno->regmap;
> + inno->pre_pll.offset = 0;
> + inno->pre_pll.table = pre_pll_cfg_table;
> +
> /* only the newer rk3328 hdmiphy has an interrupt */
> inno->irq = platform_get_irq(pdev, 0);
> if (inno->irq > 0) {
>
More information about the linux-phy
mailing list