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 <[email protected]>
---
.../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
