From d59c58dd90bad70a7e36b5023948046428141e3c Mon Sep 17 00:00:00 2001 From: Nick Hollinghurst Date: Tue, 11 Feb 2025 13:47:27 +0000 Subject: [PATCH 1/4] clk: rp1: Allow audio out to use PLL_AUDIO_SEC; workaround rounding error Connect PLL_AUDIO_SEC to CLK_AUDIO_OUT, which had been commented out to avoid interference with I2S: we expect them never to be enabled at the same time. Work around a rounding error that occurs when the desired rate is exactly the max but not exactly achievable by the PLL. Signed-off-by: Nick Hollinghurst --- drivers/clk/clk-rp1.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c index fd144755b87986..9197854b5140e5 100644 --- a/drivers/clk/clk-rp1.c +++ b/drivers/clk/clk-rp1.c @@ -1083,9 +1083,11 @@ static void rp1_clock_choose_div_and_prate(struct clk_hw *hw, /* * Prevent overclocks - if all parent choices result in * a downstream clock in excess of the maximum, then the - * call to set the clock will fail. + * call to set the clock will fail. But due to round-to- + * nearest in the PLL core (which has 24 fractional bits), + * it's expedient to tolerate a tiny error (1Hz/33MHz). */ - if (tmp > data->max_freq) + if (tmp > data->max_freq + (data->max_freq >> 25)) *calc_rate = 0; else *calc_rate = tmp; @@ -1738,7 +1740,7 @@ static struct rp1_clk_desc clk_audio_in_desc = REGISTER_CLK( static const struct clk_parent_data clk_audio_out_parents[] = { { .index = -1 }, - { .index = -1 }, + { .hw = &pll_audio_sec_desc.hw }, { .hw = &pll_video_sec_desc.hw }, { .index = 0 }, }; From 7eba47a81c5d248a4a02a77668e8a308db411351 Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Thu, 18 Dec 2025 10:44:58 +0000 Subject: [PATCH 2/4] clk: rp1: Correct declarations of divider parents PLL dividers are registered using the clk_hw in the clk_divider member of rp1_clk_desc, rather than the direct clk_hw member. In order for parent location to work, parent declarations must link to &.div.hw, not &.hw. Signed-off-by: Phil Elwell --- drivers/clk/clk-rp1.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c index 9197854b5140e5..9e148d3545532c 100644 --- a/drivers/clk/clk-rp1.c +++ b/drivers/clk/clk-rp1.c @@ -1429,7 +1429,7 @@ static struct rp1_clk_desc pll_video_sec_desc = REGISTER_PLL_DIV( static const struct clk_parent_data clk_eth_tsu_parents[] = { { .index = 0 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = -1 }, { .index = -1 }, { .index = -1 }, @@ -1460,7 +1460,7 @@ static struct rp1_clk_desc clk_eth_tsu_desc = REGISTER_CLK( static const struct clk_parent_data clk_eth_parents[] = { { .hw = &pll_sys_sec_desc.div.hw }, { .hw = &pll_sys_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, }; static struct rp1_clk_desc clk_eth_desc = REGISTER_CLK( @@ -1661,7 +1661,7 @@ static struct rp1_clk_desc clk_uart_desc = REGISTER_CLK( static const struct clk_parent_data clk_pwm0_parents[] = { { .index = -1 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = 0 }, }; @@ -1687,7 +1687,7 @@ static struct rp1_clk_desc clk_pwm0_desc = REGISTER_CLK( static const struct clk_parent_data clk_pwm1_parents[] = { { .index = -1 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = 0 }, }; @@ -1715,7 +1715,7 @@ static const struct clk_parent_data clk_audio_in_parents[] = { { .index = -1 }, { .index = -1 }, { .index = -1 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = 0 }, }; @@ -1740,8 +1740,8 @@ static struct rp1_clk_desc clk_audio_in_desc = REGISTER_CLK( static const struct clk_parent_data clk_audio_out_parents[] = { { .index = -1 }, - { .hw = &pll_audio_sec_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_audio_sec_desc.div.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = 0 }, }; @@ -1767,7 +1767,7 @@ static struct rp1_clk_desc clk_audio_out_desc = REGISTER_CLK( static const struct clk_parent_data clk_i2s_parents[] = { { .index = 0 }, { .hw = &pll_audio_desc.hw }, - { .hw = &pll_audio_sec_desc.hw }, + { .hw = &pll_audio_sec_desc.div.hw }, }; static struct rp1_clk_desc clk_i2s_desc = REGISTER_CLK( @@ -1889,7 +1889,7 @@ static struct rp1_clk_desc clk_sdio_alt_src_desc = REGISTER_CLK( static const struct clk_parent_data clk_dpi_parents[] = { { .hw = &pll_sys_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .hw = &pll_video_desc.hw }, { .index = -1 }, { .index = -1 }, @@ -2025,7 +2025,7 @@ static struct rp1_clk_desc clksrc_mipi1_dsi_byteclk_desc = REGISTER_CLK( static const struct clk_parent_data clk_mipi0_dpi_parents[] = { { .hw = &pll_sys_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .hw = &pll_video_desc.hw }, { .hw = &clksrc_mipi0_dsi_byteclk_desc.hw }, { .index = -1 }, @@ -2056,7 +2056,7 @@ static struct rp1_clk_desc clk_mipi0_dpi_desc = REGISTER_CLK( static const struct clk_parent_data clk_mipi1_dpi_parents[] = { { .hw = &pll_sys_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .hw = &pll_video_desc.hw }, { .hw = &clksrc_mipi1_dsi_byteclk_desc.hw }, { .index = -1 }, @@ -2092,7 +2092,7 @@ static const struct clk_parent_data clk_gp2_parents[] = { { .index = -1 }, { .index = -1 }, { .index = -1 }, - { .hw = &pll_sys_sec_desc.hw }, + { .hw = &pll_sys_sec_desc.div.hw }, { .index = -1 }, { .hw = &pll_video_desc.hw }, { .hw = &clk_audio_in_desc.hw }, @@ -2173,7 +2173,7 @@ static const struct clk_parent_data clk_gp4_parents[] = { { .index = -1 }, { .index = -1 }, { .index = -1 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .index = -1 }, { .index = -1 }, { .index = -1 }, @@ -2207,7 +2207,7 @@ static struct rp1_clk_desc clk_gp4_desc = REGISTER_CLK( static const struct clk_parent_data clk_vec_parents[] = { { .hw = &pll_sys_pri_ph_desc.hw }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .hw = &pll_video_desc.hw }, { .index = -1 }, { .index = -1 }, @@ -2243,7 +2243,7 @@ static const struct clk_parent_data clk_gp5_parents[] = { { .index = -1 }, { .index = -1 }, { .index = -1 }, - { .hw = &pll_video_sec_desc.hw }, + { .hw = &pll_video_sec_desc.div.hw }, { .hw = &clk_eth_tsu_desc.hw }, { .index = -1 }, { .hw = &clk_vec_desc.hw }, From aa85f2e891685301b8655b0f4538340a3d63f1c9 Mon Sep 17 00:00:00 2001 From: Nick Hollinghurst Date: Wed, 17 Dec 2025 18:54:09 +0000 Subject: [PATCH 3/4] clk: rp1: Bug fix! Set correct value for PLL_CS_REFDIV_MASK In fact the register field has 6 bits, but we only ever set it to unity. Due to a typo we were setting it to BIT(1) == 2, causing PLLs to run at half the desired rate. Signed-off-by: Nick Hollinghurst --- drivers/clk/clk-rp1.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c index 9e148d3545532c..88b6c87413e523 100644 --- a/drivers/clk/clk-rp1.c +++ b/drivers/clk/clk-rp1.c @@ -234,7 +234,8 @@ #define PLL_SEC_DIV_MASK GENMASK(12, 8) #define PLL_CS_LOCK BIT(31) -#define PLL_CS_REFDIV_MASK BIT(1) +#define PLL_CS_REFDIV_MASK GENMASK(5, 0) +#define PLL_CS_REFDIV_UNITY FIELD_PREP(PLL_CS_REFDIV_MASK, 1) #define PLL_PWR_PD BIT(0) #define PLL_PWR_DACPD BIT(1) @@ -413,7 +414,7 @@ static int rp1_pll_core_on(struct clk_hw *hw) clockman_write(clockman, data->pwr_reg, PLL_PWR_MASK); clockman_write(clockman, data->fbdiv_int_reg, 20); clockman_write(clockman, data->fbdiv_frac_reg, 0); - clockman_write(clockman, data->cs_reg, PLL_CS_REFDIV_MASK); + clockman_write(clockman, data->cs_reg, PLL_CS_REFDIV_UNITY); } /* Come out of reset. */ @@ -505,7 +506,7 @@ static int rp1_pll_core_set_rate(struct clk_hw *hw, /* Don't need to divide ref unless parent_rate > (output freq / 16) */ clockman_write(clockman, data->cs_reg, clockman_read(clockman, data->cs_reg) | - PLL_CS_REFDIV_MASK); + PLL_CS_REFDIV_UNITY); spin_unlock(&clockman->regs_lock); return 0; From 560b8d785c9036ed266999b0f3649c686f38a825 Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Thu, 18 Dec 2025 16:46:37 +0000 Subject: [PATCH 4/4] clk: rp1: Fix rp1_pll_divider_determine_rate The determine_rate member of clk_ops returns the rate to the caller by modifying the pass-by-reference req structure. Its actual return value is a status code. Signed-off-by: Phil Elwell --- drivers/clk/clk-rp1.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c index 88b6c87413e523..5ac4aeadedafaf 100644 --- a/drivers/clk/clk-rp1.c +++ b/drivers/clk/clk-rp1.c @@ -771,9 +771,7 @@ static unsigned long rp1_pll_divider_recalc_rate(struct clk_hw *hw, static int rp1_pll_divider_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { - req->rate = clk_divider_ops.determine_rate(hw, req); - - return 0; + return clk_divider_ops.determine_rate(hw, req); } static int rp1_clock_is_on(struct clk_hw *hw)