* Re: [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors @ 2026-08-17 9:16 pcb 0 siblings, 0 replies; 3+ messages in thread From: pcb @ 2026-08-17 9:16 UTC (permalink / raw) To: u-boot@lists.u-boot-project.org Hi Jonas, Quentin, We are an embedded design house building products on the RK3576, and this series lines up with several clock-related issues we have hit during our own U-Boot bring-up. Thanks for putting it together ― and for the honest note that some of these fixes are only compile-tested. That is exactly the gap we can help close. On the RK3576 side specifically, the fixes in patch 11 that matter most to us in production are the VPLL mode register correction (we rely on VPLL for display clocks) and the PPLL base fix from PMU domain to PHP domain (PPLL feeds the DRAM controller, so a wrong register window here is the kind of thing that only shows up as intermittent instability rather than a clean failure). Happy to share more concrete observations if useful. On Quentin's point about RK3576_MODE_CON0 being incorrect for PPLL but masked by ROCKCHIP_PLL_FIXED_MODE: I agree a new macro is the right long-term direction, and I would add one more angle ― the same latent hazard exists in the RK3588 PPLL entry, so whatever we settle on should be applied to both drivers in the same series. In the meantime, a compile-time assertion in the PLL() macro (e.g. BUILD_BUG_ON when FIXED_MODE is set but a mode register is still required) would prevent future contributors from silently reintroducing the wrong address. If useful, we can draft that as a follow-up patch. Finally, if a v2 is not already in flight: we would be happy to test patches 4, 5, and 11 on our RK3576 reference hardware (DDR init, VPLL/display bring-up, and I2C parent selection are all exercised during our regular boot testing) and provide Tested-by tags plus any rate mismatches we observe. Best regards, Owen Boardcon Embedded Design Limited https://www.boardcon.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes @ 2026-07-30 14:12 Jonas Karlman 2026-07-30 14:13 ` [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors Jonas Karlman 0 siblings, 1 reply; 3+ messages in thread From: Jonas Karlman @ 2026-07-30 14:12 UTC (permalink / raw) To: Quentin Schulz, Kever Yang, Tom Rini, Ilias Apalodimas Cc: Simon Glass, Lukasz Majewski, u-boot, Jonas Karlman This series fixes a few trivial copy-paste mistakes and other misc clock issues for RK35xx SoCs. Patch 1-3 fixes minor and very unlikely issues for plls Patch 4 fixes PPLL mode handling on RK3588/RK3576 Patch 5 disable 'special pll handling' for RK3576 plls Patch 6-7,11-13 fixes trivial clock configuration errors Patch 8-9 fixes less trivial clock configuration errors Patch 14-15 remove unused GRF syscon lookups Some of these fixes has only been compile tested as it involves clocks or rates that is rarely used in U-Boot. The 'special pll handling' should ideally be moved into clk_rk3588 driver, something that can be done in a future follow-up series. Jonas Karlman (15): clk: rockchip: pll: Fix double use of postdiv1 clk: rockchip: pll: Always write the dsmpd flag clk: rockchip: pll: Always write the k param clk: rockchip: pll: Use PLL_FIXED_MODE flag on rk3588/rk3576 plls clk: rockchip: pll: Limit special rk3588_pll handling to RK3588 clk: rockchip: rk3568: Fix trivial clock configuration errors clk: rockchip: rk3588: Fix trivial clock configuration errors clk: rockchip: rk3588: Fix possible divide by zero clk: rockchip: rk3588: Fix ACLK_BUS_ROOT rate set during probe clk: rockchip: rk3588: Use SPLL_HZ constant clk: rockchip: rk3576: Fix trivial clock configuration errors clk: rockchip: rk3528: Fix trivial clock configuration errors clk: rockchip: rk3506: Fix trivial clock configuration errors clk: rockchip: rk3568: Drop unused GRF syscon lookup clk: rockchip: rk3588: Drop unused GRF syscon lookup .../include/asm/arch-rockchip/cru_rk3568.h | 1 - .../include/asm/arch-rockchip/cru_rk3576.h | 5 +- .../include/asm/arch-rockchip/cru_rk3588.h | 6 +- drivers/clk/rockchip/clk_pll.c | 56 ++++++++++--------- drivers/clk/rockchip/clk_rk3506.c | 2 + drivers/clk/rockchip/clk_rk3528.c | 7 ++- drivers/clk/rockchip/clk_rk3568.c | 12 ++-- drivers/clk/rockchip/clk_rk3576.c | 26 ++++----- drivers/clk/rockchip/clk_rk3588.c | 39 ++++++------- 9 files changed, 72 insertions(+), 82 deletions(-) -- 2.54.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors 2026-07-30 14:12 [PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes Jonas Karlman @ 2026-07-30 14:13 ` Jonas Karlman 2026-08-07 16:04 ` Quentin Schulz via U-Boot 0 siblings, 1 reply; 3+ messages in thread From: Jonas Karlman @ 2026-07-30 14:13 UTC (permalink / raw) To: Quentin Schulz, Kever Yang, Tom Rini, Ilias Apalodimas, Simon Glass, Lukasz Majewski Cc: u-boot, Jonas Karlman The RK3576 clock driver has a few trivial copy-paste mistakes in its clock handling. Fix the trivial clock configuration errors: - use correct VPLL mode reg - rename and use PHP_PLL_CON macro - set correct parent for ACLK_TOP clocks - avoid overriding the selected I2C parent clock - stop CLK_I2C8 from falling through into CLK_I2C9 - use correct SARADC and TSADC clksel regs - use correct parent pll rate for UART clocks - align BPLL configuration to match other PLLs - remove unused BPLL_CON macro Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- .../include/asm/arch-rockchip/cru_rk3576.h | 5 ++-- drivers/clk/rockchip/clk_rk3576.c | 23 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h index fb77fbd7307a..41e225245843 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h @@ -127,24 +127,23 @@ struct pll_rate_table { #define RK3576_SDMMC_CON0 0xC30 #define RK3576_SDMMC_CON1 0xC34 +#define RK3576_PHP_PLL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE) #define RK3576_PHP_CLKSEL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x300) #define RK3576_PHP_CLKGATE_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x800) #define RK3576_PHP_SOFTRST_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0xa00) -#define RK3576_PMU_PLL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE) #define RK3576_PMU_CLKSEL_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x300) #define RK3576_PMU_CLKGATE_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x800) #define RK3576_PMU_SOFTRST_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0xa00) +#define RK3576_LPLL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE) #define RK3576_CCI_CLKSEL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x300) #define RK3576_CCI_CLKGATE_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x800) #define RK3576_CCI_SOFTRST_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0xa00) -#define RK3576_BPLL_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE) #define RK3576_BIGCORE_CLKSEL_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x300) #define RK3576_BIGCORE_CLKGATE_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x800) #define RK3576_BIGCORE_SOFTRST_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0xa00) -#define RK3576_LPLL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE) #define RK3576_LITCORE_CLKSEL_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x300) #define RK3576_LITCORE_CLKGATE_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x800) #define RK3576_LITCORE_SOFTRST_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0xa00) diff --git a/drivers/clk/rockchip/clk_rk3576.c b/drivers/clk/rockchip/clk_rk3576.c index 92bde425b0ee..75b705ffba2f 100644 --- a/drivers/clk/rockchip/clk_rk3576.c +++ b/drivers/clk/rockchip/clk_rk3576.c @@ -44,19 +44,18 @@ static struct rockchip_pll_rate_table rk3576_24m_pll_rates[] = { static struct rockchip_pll_clock rk3576_pll_clks[] = { [BPLL] = PLL(pll_rk3588, PLL_BPLL, RK3576_PLL_CON(0), - RK3576_BPLL_MODE_CON0, 0, 15, 0, - rk3576_24m_pll_rates), + RK3576_BPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates), [LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3576_LPLL_CON(16), RK3576_LPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates), [VPLL] = PLL(pll_rk3588, PLL_VPLL, RK3576_PLL_CON(88), - RK3576_LPLL_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates), + RK3576_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates), [AUPLL] = PLL(pll_rk3588, PLL_AUPLL, RK3576_PLL_CON(96), RK3576_MODE_CON0, 6, 15, 0, rk3576_24m_pll_rates), [CPLL] = PLL(pll_rk3588, PLL_CPLL, RK3576_PLL_CON(104), RK3576_MODE_CON0, 8, 15, 0, rk3576_24m_pll_rates), [GPLL] = PLL(pll_rk3588, PLL_GPLL, RK3576_PLL_CON(112), RK3576_MODE_CON0, 2, 15, 0, rk3576_24m_pll_rates), - [PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PMU_PLL_CON(128), + [PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PHP_PLL_CON(128), RK3576_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE, rk3576_24m_pll_rates), }; @@ -320,8 +319,7 @@ static ulong rk3576_top_set_clk(struct rk3576_clk_priv *priv, rk_clrsetreg(&cru->clksel_con[10], ACLK_TOP_MID_DIV_MASK | ACLK_TOP_MID_SEL_MASK, - (ACLK_TOP_MID_SEL_GPLL << - ACLK_TOP_MID_SEL_SHIFT) | + (src_clk << ACLK_TOP_MID_SEL_SHIFT) | (src_clk_div - 1) << ACLK_TOP_MID_DIV_SHIFT); break; case PCLK_TOP_ROOT: @@ -429,7 +427,7 @@ static ulong rk3576_i2c_set_clk(struct rk3576_clk_priv *priv, ulong clk_id, src_clk = CLK_I2C_SEL_200M; else if (rate >= 99 * MHz) src_clk = CLK_I2C_SEL_100M; - if (rate >= 50 * MHz) + else if (rate >= 50 * MHz) src_clk = CLK_I2C_SEL_50M; else src_clk = CLK_I2C_SEL_OSC; @@ -470,6 +468,7 @@ static ulong rk3576_i2c_set_clk(struct rk3576_clk_priv *priv, ulong clk_id, case CLK_I2C8: rk_clrsetreg(&cru->clksel_con[57], CLK_I2C8_SEL_MASK, src_clk << CLK_I2C8_SEL_SHIFT); + break; case CLK_I2C9: rk_clrsetreg(&cru->clksel_con[58], CLK_I2C9_SEL_MASK, src_clk << CLK_I2C9_SEL_SHIFT); @@ -691,7 +690,7 @@ static ulong rk3576_adc_set_clk(struct rk3576_clk_priv *priv, } else { src_clk_div = DIV_ROUND_UP(priv->gpll_hz, rate); assert(src_clk_div - 1 <= 255); - rk_clrsetreg(&cru->clksel_con[59], + rk_clrsetreg(&cru->clksel_con[58], CLK_SARADC_SEL_MASK | CLK_SARADC_DIV_MASK, (CLK_SARADC_SEL_GPLL << @@ -703,7 +702,7 @@ static ulong rk3576_adc_set_clk(struct rk3576_clk_priv *priv, case CLK_TSADC: src_clk_div = DIV_ROUND_UP(OSC_HZ, rate); assert(src_clk_div - 1 <= 255); - rk_clrsetreg(&cru->clksel_con[58], + rk_clrsetreg(&cru->clksel_con[59], CLK_TSADC_DIV_MASK, (src_clk_div - 1) << CLK_TSADC_DIV_SHIFT); @@ -1715,7 +1714,7 @@ static ulong rk3576_uart_frac_set_rate(struct rk3576_clk_priv *priv, p_rate = OSC_HZ; } else { clk_src = CLK_UART_SRC_SEL_GPLL; - p_rate = priv->cpll_hz; + p_rate = priv->gpll_hz; } rational_best_approximation(rate, p_rate, GENMASK(16 - 1, 0), @@ -1843,7 +1842,7 @@ static ulong rk3576_uart_set_rate(struct rk3576_clk_priv *priv, div = DIV_ROUND_UP(priv->gpll_hz, rate); } else if (!(priv->cpll_hz % rate)) { clk_src = CLK_UART_SEL_CPLL; - div = DIV_ROUND_UP(priv->gpll_hz, rate); + div = DIV_ROUND_UP(priv->cpll_hz, rate); } else if (!(rk3576_uart_frac_get_rate(priv, CLK_UART_FRAC_0) % rate)) { clk_src = CLK_UART_SEL_FRAC0; div = DIV_ROUND_UP(rk3576_uart_frac_get_rate(priv, CLK_UART_FRAC_0), rate); @@ -2458,7 +2457,7 @@ static int rk3576_clk_probe(struct udevice *dev) RK3576_SCRU_BASE + RK3576_MODE_CON0); /* fix ppll\aupll\cpll */ writel(BITS_WITH_WMASK(2, 0x7U, 6), - RK3576_CRU_BASE + RK3576_PMU_PLL_CON(129)); + RK3576_CRU_BASE + RK3576_PHP_PLL_CON(129)); writel(BITS_WITH_WMASK(2, 0x7U, 6), RK3576_CRU_BASE + RK3576_PLL_CON(97)); writel(BITS_WITH_WMASK(2, 0x7U, 6), -- 2.54.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors 2026-07-30 14:13 ` [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors Jonas Karlman @ 2026-08-07 16:04 ` Quentin Schulz via U-Boot 0 siblings, 0 replies; 3+ messages in thread From: Quentin Schulz via U-Boot @ 2026-08-07 16:04 UTC (permalink / raw) To: Jonas Karlman, Kever Yang, Tom Rini, Ilias Apalodimas, Simon Glass, Lukasz Majewski Cc: u-boot Hi Jonas, On 7/30/26 4:13 PM, Jonas Karlman wrote: > The RK3576 clock driver has a few trivial copy-paste mistakes in its > clock handling. > > Fix the trivial clock configuration errors: > - use correct VPLL mode reg > - rename and use PHP_PLL_CON macro > - set correct parent for ACLK_TOP clocks > - avoid overriding the selected I2C parent clock > - stop CLK_I2C8 from falling through into CLK_I2C9 > - use correct SARADC and TSADC clksel regs > - use correct parent pll rate for UART clocks > - align BPLL configuration to match other PLLs > - remove unused BPLL_CON macro > Please split those into separate commits. All changes are fine individually. See small remark below for a change I believe would help with reading the code more easily. > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > .../include/asm/arch-rockchip/cru_rk3576.h | 5 ++-- > drivers/clk/rockchip/clk_rk3576.c | 23 +++++++++---------- > 2 files changed, 13 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h > index fb77fbd7307a..41e225245843 100644 > --- a/arch/arm/include/asm/arch-rockchip/cru_rk3576.h > +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3576.h > @@ -127,24 +127,23 @@ struct pll_rate_table { > #define RK3576_SDMMC_CON0 0xC30 > #define RK3576_SDMMC_CON1 0xC34 > > +#define RK3576_PHP_PLL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE) Same remark as for the RK3588 patch, please add 0x200 so we can use RK3576_PHP_PLL_CON(0) when we want to interact with PHPTOPCRU_PPLL_CON0. > #define RK3576_PHP_CLKSEL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x300) > #define RK3576_PHP_CLKGATE_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0x800) > #define RK3576_PHP_SOFTRST_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE + 0xa00) > > -#define RK3576_PMU_PLL_CON(x) ((x) * 0x4 + RK3576_PHP_CRU_BASE) > #define RK3576_PMU_CLKSEL_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x300) > #define RK3576_PMU_CLKGATE_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0x800) > #define RK3576_PMU_SOFTRST_CON(x) ((x) * 0x4 + RK3576_PMU_CRU_BASE + 0xa00) > > +#define RK3576_LPLL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE) Please add 0x40 so we can do RK3576_LPLL_CON(0) to interact with CCICRU_LPLL_CON0. > #define RK3576_CCI_CLKSEL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x300) > #define RK3576_CCI_CLKGATE_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0x800) > #define RK3576_CCI_SOFTRST_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE + 0xa00) > > -#define RK3576_BPLL_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE) > #define RK3576_BIGCORE_CLKSEL_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x300) > #define RK3576_BIGCORE_CLKGATE_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0x800) > #define RK3576_BIGCORE_SOFTRST_CON(x) ((x) * 0x4 + RK3576_BIGCORE_CRU_BASE + 0xa00) > -#define RK3576_LPLL_CON(x) ((x) * 0x4 + RK3576_CCI_CRU_BASE) > #define RK3576_LITCORE_CLKSEL_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x300) > #define RK3576_LITCORE_CLKGATE_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0x800) > #define RK3576_LITCORE_SOFTRST_CON(x) ((x) * 0x4 + RK3576_LITCORE_CRU_BASE + 0xa00) > diff --git a/drivers/clk/rockchip/clk_rk3576.c b/drivers/clk/rockchip/clk_rk3576.c > index 92bde425b0ee..75b705ffba2f 100644 > --- a/drivers/clk/rockchip/clk_rk3576.c > +++ b/drivers/clk/rockchip/clk_rk3576.c > @@ -44,19 +44,18 @@ static struct rockchip_pll_rate_table rk3576_24m_pll_rates[] = { > > static struct rockchip_pll_clock rk3576_pll_clks[] = { > [BPLL] = PLL(pll_rk3588, PLL_BPLL, RK3576_PLL_CON(0), > - RK3576_BPLL_MODE_CON0, 0, 15, 0, > - rk3576_24m_pll_rates), > + RK3576_BPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates), > [LPLL] = PLL(pll_rk3588, PLL_LPLL, RK3576_LPLL_CON(16), > RK3576_LPLL_MODE_CON0, 0, 15, 0, rk3576_24m_pll_rates), > [VPLL] = PLL(pll_rk3588, PLL_VPLL, RK3576_PLL_CON(88), > - RK3576_LPLL_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates), > + RK3576_MODE_CON0, 4, 15, 0, rk3576_24m_pll_rates), > [AUPLL] = PLL(pll_rk3588, PLL_AUPLL, RK3576_PLL_CON(96), > RK3576_MODE_CON0, 6, 15, 0, rk3576_24m_pll_rates), > [CPLL] = PLL(pll_rk3588, PLL_CPLL, RK3576_PLL_CON(104), > RK3576_MODE_CON0, 8, 15, 0, rk3576_24m_pll_rates), > [GPLL] = PLL(pll_rk3588, PLL_GPLL, RK3576_PLL_CON(112), > RK3576_MODE_CON0, 2, 15, 0, rk3576_24m_pll_rates), > - [PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PMU_PLL_CON(128), > + [PPLL] = PLL(pll_rk3588, PLL_PPLL, RK3576_PHP_PLL_CON(128), > RK3576_MODE_CON0, 10, 15, ROCKCHIP_PLL_FIXED_MODE, RK3576_MODE_CON0 is incorrect here, but since ROCKCHIP_PLL_FIXED_MODE is set, this won't be used as far as I could tell. I'm wondering whether we should have a new macros that wouldn't force us to define something necessarily incorrect. Something for later though. Cheers, Quentin ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-17 13:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-17 9:16 [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors pcb -- strict thread matches above, loose matches on Subject: below -- 2026-07-30 14:12 [PATCH 00/15] rockchip: Miscellaneous RK35xx clock fixes Jonas Karlman 2026-07-30 14:13 ` [PATCH 11/15] clk: rockchip: rk3576: Fix trivial clock configuration errors Jonas Karlman 2026-08-07 16:04 ` Quentin Schulz via U-Boot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.