From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.forwardemail.net (smtp.forwardemail.net [121.127.44.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD209499F19 for ; Tue, 1 Sep 2026 18:16:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=121.127.44.66 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788286572; cv=none; b=P9MyfvsroDqFyOpa+9sU0glunmUL1RL7/Qj2qJvS6cIWAJo27KDdQtcsCunq2GXF0nSXqLNjiwZ169SOWi7wvImsdmmwQOIjC8uMt7v4yrE2hxIabobISXo2UD0gtvMneDwZ5iALnScA9j4e8mLSxb/Kae+IsuCYPHPJ5lL8Z3Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788286572; c=relaxed/simple; bh=f+zjjjmZX+MuA/T8DxWJycZ4Nhs0VBrPXLYhaC7HQl4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=t1v7WNHc5zu1fXPFRy6y/vExTElfmDNJug/HMzJOBOqu39SIQPGoSOW5V4aTc9osWsCACTArADDQ/5vVtQVh1u4vy7mF1v9mSEHj3AeU0AhWlipYvLZLAnGgpbOfHyJ+tIuTdlxoHj7c0HSAm4vgmmdpOC9z4d0+/96y4Xv/KhU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=kwiboo.se; spf=pass smtp.mailfrom=fe-bounces.kwiboo.se; dkim=pass (2048-bit key) header.d=kwiboo.se header.i=@kwiboo.se header.b=ZdBbNANn; arc=none smtp.client-ip=121.127.44.66 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=kwiboo.se Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fe-bounces.kwiboo.se Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kwiboo.se header.i=@kwiboo.se header.b="ZdBbNANn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kwiboo.se; h=Content-Transfer-Encoding: Content-Type: In-Reply-To: From: References: Cc: To: Subject: MIME-Version: Date: Message-ID; q=dns/txt; s=fe-e1b5cab7be; t=1788286561; bh=e7plJxIygwISfSRDt4IRuRj0Ea3yUgy5TIf9N8SYgTA=; b=ZdBbNANnNQyfCHJZYifgIONxE3fddmhwaifUSkaSCh6z+3LW9j3KnLabdDDotWTOWg7zSwIma jgOG8dTQXwLZHr9S2G08ebqZxl1InQJVbLeUqWh2XORl1+hEo6UVBN1c5Kb+vJwqi9s1f/6PQF0 b/63lvVPevK3eavwS14/whaXFUM67kwDsYloE42lDjQI7TzcjupeSOYoGNtcE8XUJHz6JOI6C0j FIz6g5T/kWb6JDywSBp04PM/LMHUp3nAaSgMwjRTpTEus2dF2KyNinVsXTGdPN45fwqwbAP5QOt im6aRie0xMQOkkaexzMwMlCI1Rrv78KNRupu3R1Hlm6A== X-Forward-Email-ID: 6a971609a9d0b3c06ff430dc X-Forward-Email-Sender: rfc822; jonas@kwiboo.se, smtp.forwardemail.net, 121.127.44.66 X-Forward-Email-Version: 2.14.0 X-Forward-Email-Website: https://forwardemail.net X-Complaints-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Report-Abuse-To: abuse@forwardemail.net Message-ID: Date: Tue, 1 Sep 2026 20:14:23 +0200 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 12/15] phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers To: Michal Wilczynski Cc: Heiko Stuebner , Vinod Koul , Neil Armstrong , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Robert Foss , Laurent Pinchart , Jernej Skrabec , Luca Ceresoli , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Lee Jones , Andy Yan , Philipp Zabel , Emil Renner Berthing , Hal Feng , Michael Turquette , Stephen Boyd , Brian Masney , Conor Dooley , Paul Walmsley , Palmer Dabbelt , Albert Ou , Alexandre Ghiti , Dominique Belhachemi , linux-phy@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, mfd@lists.linux.dev, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-riscv@lists.infradead.org, Icenowy Zheng , Andy Yan , Marek Szyprowski , Maud Spierings , Graham Markall References: <20260828-jh7110-clean-send-v2-0-331680c8b9d1@samsung.com> <20260828-jh7110-clean-send-v2-12-331680c8b9d1@samsung.com> Content-Language: en-US From: Jonas Karlman In-Reply-To: <20260828-jh7110-clean-send-v2-12-331680c8b9d1@samsung.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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) { >