* [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver
@ 2025-08-12 5:42 Icenowy Zheng
2025-08-12 5:42 ` [PATCH 1/4] clk: thead: add support for enabling/disabling PLLs Icenowy Zheng
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Icenowy Zheng @ 2025-08-12 5:42 UTC (permalink / raw)
To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd,
Michal Wilczynski
Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng
This patchset is my changes to the TH1520 clock driver, mainly for
supporting the display controller (the first 3).
The first two are functionality additions, with the first one adding
support for enabling/disabling PLLs (for DPU PLL) and the second one
adding support for changing DPU dividers.
The 3rd one is to address hang issues met when testing the DPU driver
w/o clk_ignore_unused command line option.
The 4th one has no relationship to display, and only exists for my need
to change an arbitrary GPIO (well, GPIO3_3, the one controlling the fan
on Lichee Pi 4A) with gpioset.
This patchset has a dependency (a 0th one) [1].
[1] https://lore.kernel.org/linux-riscv/20250809-fix_clocks_thead_aug_9-v1-1-299c33d7a593@samsung.com/
Icenowy Zheng (4):
clk: thead: add support for enabling/disabling PLLs
clk: thead: support changing DPU pixel clock rate
clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL
clk: thead: th1520-ap: fix parent of padctrl0 clock
drivers/clk/thead/clk-th1520-ap.c | 174 ++++++++++++++++++++++++------
1 file changed, 143 insertions(+), 31 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/4] clk: thead: add support for enabling/disabling PLLs 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng @ 2025-08-12 5:42 ` Icenowy Zheng 2025-08-12 5:42 ` [PATCH 2/4] clk: thead: support changing DPU pixel clock rate Icenowy Zheng ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 5:42 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The 2nd control word of T-Head TH1520 PLLs contains a bit to put the VCO into reset state, which means disabling the PLL. Some PLLs are put to disabled state by the bootloader, and the clock driver should be able to enable them. Add support for enabling/disabling PLLs. PLLs other than DPU ones are set CLK_IS_CRITICAL to prevent killing the system -- they're meant to drive CPU or system buses (even the GMAC/Video ones are driving arbitrary buses). Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- drivers/clk/thead/clk-th1520-ap.c | 38 +++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index dd6359441f7e8..2f87c7c2c3baf 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -18,6 +18,7 @@ #define TH1520_PLL_FBDIV GENMASK(19, 8) #define TH1520_PLL_REFDIV GENMASK(5, 0) #define TH1520_PLL_BYPASS BIT(30) +#define TH1520_PLL_VCO_RST BIT(29) #define TH1520_PLL_DSMPD BIT(24) #define TH1520_PLL_FRAC GENMASK(23, 0) #define TH1520_PLL_FRAC_BITS 24 @@ -242,6 +243,30 @@ static const struct clk_ops ccu_div_ops = { .determine_rate = clk_hw_determine_rate_no_reparent, }; +static void ccu_pll_disable(struct clk_hw *hw) +{ + struct ccu_pll *pll = hw_to_ccu_pll(hw); + + regmap_set_bits(pll->common.map, pll->common.cfg1, + TH1520_PLL_VCO_RST); +} + +static int ccu_pll_enable(struct clk_hw *hw) +{ + struct ccu_pll *pll = hw_to_ccu_pll(hw); + + return regmap_clear_bits(pll->common.map, pll->common.cfg1, + TH1520_PLL_VCO_RST); +} + +static int ccu_pll_is_enabled(struct clk_hw *hw) +{ + struct ccu_pll *pll = hw_to_ccu_pll(hw); + + return !regmap_test_bits(pll->common.map, pll->common.cfg1, + TH1520_PLL_VCO_RST); +} + static unsigned long th1520_pll_vco_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -299,6 +324,9 @@ static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw, } static const struct clk_ops clk_pll_ops = { + .disable = ccu_pll_disable, + .enable = ccu_pll_enable, + .is_enabled = ccu_pll_is_enabled, .recalc_rate = ccu_pll_recalc_rate, }; @@ -314,7 +342,7 @@ static struct ccu_pll cpu_pll0_clk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("cpu-pll0", osc_24m_clk, &clk_pll_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -326,7 +354,7 @@ static struct ccu_pll cpu_pll1_clk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("cpu-pll1", osc_24m_clk, &clk_pll_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -338,7 +366,7 @@ static struct ccu_pll gmac_pll_clk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("gmac-pll", osc_24m_clk, &clk_pll_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -358,7 +386,7 @@ static struct ccu_pll video_pll_clk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("video-pll", osc_24m_clk, &clk_pll_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -410,7 +438,7 @@ static struct ccu_pll tee_pll_clk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("tee-pll", osc_24m_clk, &clk_pll_ops, - 0), + CLK_IS_CRITICAL), }, }; -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] clk: thead: support changing DPU pixel clock rate 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 2025-08-12 5:42 ` [PATCH 1/4] clk: thead: add support for enabling/disabling PLLs Icenowy Zheng @ 2025-08-12 5:42 ` Icenowy Zheng 2025-08-13 2:04 ` Troy Mitchell 2025-08-21 18:29 ` Brian Masney 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng ` (2 subsequent siblings) 4 siblings, 2 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 5:42 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The DPU pixel clock rate corresponds to the required dot clock of the display mode, so it needs to be tweakable. Add support to change it, by adding generic divider setting code, arming the code to the dpu0/dpu1 clocks, and setting the pixel clock connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate it to the dividers. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- drivers/clk/thead/clk-th1520-ap.c | 87 +++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 2f87c7c2c3baf..3e81f3051cd6c 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -55,6 +55,7 @@ struct ccu_gate { struct ccu_div { u32 enable; + u32 div_en; struct ccu_div_internal div; struct ccu_internal mux; struct ccu_common common; @@ -198,6 +199,78 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, return rate; } +static long ccu_div_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + unsigned int val; + + if (!cd->div_en) { + regmap_read(cd->common.map, cd->common.cfg0, &val); + val = val >> cd->div.shift; + val &= GENMASK(cd->div.width - 1, 0); + return divider_ro_round_rate(hw, rate, parent_rate, + NULL, cd->div.width, cd->div.flags, + val); + } else { + return divider_round_rate(hw, rate, parent_rate, + NULL, cd->div.width, cd->div.flags); + } +} + +static int ccu_div_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + unsigned int val; + + if (!cd->div_en) { + regmap_read(cd->common.map, cd->common.cfg0, &val); + val = val >> cd->div.shift; + val &= GENMASK(cd->div.width - 1, 0); + return divider_ro_determine_rate(hw, req, NULL, + cd->div.width, + cd->div.flags, + val); + } else { + return divider_determine_rate(hw, req, NULL, + cd->div.width, cd->div.flags); + } +} + +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct ccu_div *cd = hw_to_ccu_div(hw); + int val = divider_get_val(rate, parent_rate, NULL, + cd->div.width, cd->div.flags); + unsigned int curr_val, reg_val; + + if (val < 0) + return val; + + regmap_read(cd->common.map, cd->common.cfg0, ®_val); + curr_val = reg_val; + curr_val = curr_val >> cd->div.shift; + curr_val &= GENMASK(cd->div.width - 1, 0); + + if (!cd->div_en && curr_val != val) + return -EINVAL; + + reg_val &= ~cd->div_en; + regmap_write(cd->common.map, cd->common.cfg0, reg_val); + udelay(1); + + reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift); + reg_val |= val << cd->div.shift; + regmap_write(cd->common.map, cd->common.cfg0, reg_val); + + reg_val |= cd->div_en; + regmap_write(cd->common.map, cd->common.cfg0, reg_val); + + return 0; +} + static u8 ccu_div_get_parent(struct clk_hw *hw) { struct ccu_div *cd = hw_to_ccu_div(hw); @@ -240,7 +313,9 @@ static const struct clk_ops ccu_div_ops = { .get_parent = ccu_div_get_parent, .set_parent = ccu_div_set_parent, .recalc_rate = ccu_div_recalc_rate, - .determine_rate = clk_hw_determine_rate_no_reparent, + .round_rate = ccu_div_round_rate, + .set_rate = ccu_div_set_rate, + .determine_rate = ccu_div_determine_rate, }; static void ccu_pll_disable(struct clk_hw *hw) @@ -784,6 +859,7 @@ static struct ccu_div venc_clk = { }; static struct ccu_div dpu0_clk = { + .div_en = BIT(8), .div = TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED), .common = { .clkid = CLK_DPU0, @@ -791,7 +867,7 @@ static struct ccu_div dpu0_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("dpu0", dpu0_pll_clk_parent, &ccu_div_ops, - 0), + CLK_SET_RATE_UNGATE), }, }; @@ -800,6 +876,7 @@ static const struct clk_parent_data dpu0_clk_pd[] = { }; static struct ccu_div dpu1_clk = { + .div_en = BIT(8), .div = TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED), .common = { .clkid = CLK_DPU1, @@ -807,7 +884,7 @@ static struct ccu_div dpu1_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("dpu1", dpu1_pll_clk_parent, &ccu_div_ops, - 0), + CLK_SET_RATE_UNGATE), }, }; @@ -891,9 +968,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd, static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", video_pll_clk_pd, 0x0, BIT(4), 0); static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", - dpu0_clk_pd, 0x0, BIT(5), 0); + dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", - dpu1_clk_pd, 0x0, BIT(6), 0); + dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT); static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", video_pll_clk_pd, 0x0, BIT(7), 0); static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", video_pll_clk_pd, 0x0, -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] clk: thead: support changing DPU pixel clock rate 2025-08-12 5:42 ` [PATCH 2/4] clk: thead: support changing DPU pixel clock rate Icenowy Zheng @ 2025-08-13 2:04 ` Troy Mitchell 2025-08-13 5:54 ` Icenowy Zheng 2025-08-21 18:29 ` Brian Masney 1 sibling, 1 reply; 17+ messages in thread From: Troy Mitchell @ 2025-08-13 2:04 UTC (permalink / raw) To: Icenowy Zheng, Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Troy Mitchell On Tue, Aug 12, 2025 at 01:42:56PM +0800, Icenowy Zheng wrote: > The DPU pixel clock rate corresponds to the required dot clock of the > display mode, so it needs to be tweakable. > > Add support to change it, by adding generic divider setting code, > arming the code to the dpu0/dpu1 clocks, and setting the pixel clock > connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate > it to the dividers. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > drivers/clk/thead/clk-th1520-ap.c | 87 +++++++++++++++++++++++++++++-- > 1 file changed, 82 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c > index 2f87c7c2c3baf..3e81f3051cd6c 100644 > --- a/drivers/clk/thead/clk-th1520-ap.c > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -55,6 +55,7 @@ struct ccu_gate { > > struct ccu_div { > u32 enable; > + u32 div_en; > struct ccu_div_internal div; > struct ccu_internal mux; > struct ccu_common common; > @@ -198,6 +199,78 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, > return rate; > } > > +static long ccu_div_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + unsigned int val; > + > + if (!cd->div_en) { > + regmap_read(cd->common.map, cd->common.cfg0, &val); > + val = val >> cd->div.shift; > + val &= GENMASK(cd->div.width - 1, 0); > + return divider_ro_round_rate(hw, rate, parent_rate, > + NULL, cd->div.width, cd->div.flags, > + val); > + } else { > + return divider_round_rate(hw, rate, parent_rate, > + NULL, cd->div.width, cd->div.flags); > + } How about this: ``` if (cd->div_en) return divider_round_rate(hw, rate, parent_rate, NULL, cd->div.width, cd->div.flags); regmap_read(cd->common.map, cd->common.cfg0, &val); val = val >> cd->div.shift; val &= GENMASK(cd->div.width - 1, 0); return divider_ro_round_rate(hw, rate, parent_rate, NULL, cd->div.width, cd->div.flags, val); ``` > +} > + > +static int ccu_div_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + unsigned int val; > + > + if (!cd->div_en) { > + regmap_read(cd->common.map, cd->common.cfg0, &val); > + val = val >> cd->div.shift; > + val &= GENMASK(cd->div.width - 1, 0); > + return divider_ro_determine_rate(hw, req, NULL, > + cd->div.width, > + cd->div.flags, > + val); > + } else { > + return divider_determine_rate(hw, req, NULL, > + cd->div.width, cd->div.flags); ditto. > + } > +} > + > +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + int val = divider_get_val(rate, parent_rate, NULL, > + cd->div.width, cd->div.flags); > + unsigned int curr_val, reg_val; > + > + if (val < 0) > + return val; > + > + regmap_read(cd->common.map, cd->common.cfg0, ®_val); > + curr_val = reg_val; > + curr_val = curr_val >> cd->div.shift; > + curr_val &= GENMASK(cd->div.width - 1, 0); > + > + if (!cd->div_en && curr_val != val) > + return -EINVAL; > + > + reg_val &= ~cd->div_en; > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > + udelay(1); > + > + reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift); > + reg_val |= val << cd->div.shift; > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > + > + reg_val |= cd->div_en; > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > + > + return 0; > +} > + > static u8 ccu_div_get_parent(struct clk_hw *hw) > { > struct ccu_div *cd = hw_to_ccu_div(hw); > @@ -240,7 +313,9 @@ static const struct clk_ops ccu_div_ops = { > .get_parent = ccu_div_get_parent, > .set_parent = ccu_div_set_parent, > .recalc_rate = ccu_div_recalc_rate, > - .determine_rate = clk_hw_determine_rate_no_reparent, > + .round_rate = ccu_div_round_rate, > + .set_rate = ccu_div_set_rate, > + .determine_rate = ccu_div_determine_rate, > }; > > static void ccu_pll_disable(struct clk_hw *hw) > @@ -784,6 +859,7 @@ static struct ccu_div venc_clk = { > }; > > static struct ccu_div dpu0_clk = { > + .div_en = BIT(8), > .div = TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED), > .common = { > .clkid = CLK_DPU0, > @@ -791,7 +867,7 @@ static struct ccu_div dpu0_clk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("dpu0", > dpu0_pll_clk_parent, > &ccu_div_ops, > - 0), > + CLK_SET_RATE_UNGATE), > }, > }; > > @@ -800,6 +876,7 @@ static const struct clk_parent_data dpu0_clk_pd[] = { > }; > > static struct ccu_div dpu1_clk = { > + .div_en = BIT(8), > .div = TH_CCU_DIV_FLAGS(0, 8, CLK_DIVIDER_ONE_BASED), > .common = { > .clkid = CLK_DPU1, > @@ -807,7 +884,7 @@ static struct ccu_div dpu1_clk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("dpu1", > dpu1_pll_clk_parent, > &ccu_div_ops, > - 0), > + CLK_SET_RATE_UNGATE), > }, > }; > > @@ -891,9 +968,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd, > static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", > video_pll_clk_pd, 0x0, BIT(4), 0); > static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", > - dpu0_clk_pd, 0x0, BIT(5), 0); > + dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); > static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", > - dpu1_clk_pd, 0x0, BIT(6), 0); > + dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT); > static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", video_pll_clk_pd, 0x0, > BIT(7), 0); > static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", video_pll_clk_pd, 0x0, > -- > 2.50.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] clk: thead: support changing DPU pixel clock rate 2025-08-13 2:04 ` Troy Mitchell @ 2025-08-13 5:54 ` Icenowy Zheng 0 siblings, 0 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-13 5:54 UTC (permalink / raw) To: Troy Mitchell, Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel 在 2025-08-13星期三的 10:04 +0800，Troy Mitchell写道： > On Tue, Aug 12, 2025 at 01:42:56PM +0800, Icenowy Zheng wrote: > > The DPU pixel clock rate corresponds to the required dot clock of > > the > > display mode, so it needs to be tweakable. > > > > Add support to change it, by adding generic divider setting code, > > arming the code to the dpu0/dpu1 clocks, and setting the pixel > > clock > > connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to > > propagate > > it to the dividers. > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > --- > > drivers/clk/thead/clk-th1520-ap.c | 87 > > +++++++++++++++++++++++++++++-- > > 1 file changed, 82 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clk/thead/clk-th1520-ap.c > > b/drivers/clk/thead/clk-th1520-ap.c > > index 2f87c7c2c3baf..3e81f3051cd6c 100644 > > --- a/drivers/clk/thead/clk-th1520-ap.c > > +++ b/drivers/clk/thead/clk-th1520-ap.c > > @@ -55,6 +55,7 @@ struct ccu_gate { > > > > struct ccu_div { > > u32 enable; > > + u32 div_en; > > struct ccu_div_internal div; > > struct ccu_internal mux; > > struct ccu_common common; > > @@ -198,6 +199,78 @@ static unsigned long > > ccu_div_recalc_rate(struct clk_hw *hw, > > return rate; > > } > > > > +static long ccu_div_round_rate(struct clk_hw *hw, unsigned long > > rate, > > + unsigned long *parent_rate) > > +{ > > + struct ccu_div *cd = hw_to_ccu_div(hw); > > + unsigned int val; > > + > > + if (!cd->div_en) { > > + regmap_read(cd->common.map, cd->common.cfg0, &val); > > + val = val >> cd->div.shift; > > + val &= GENMASK(cd->div.width - 1, 0); > > + return divider_ro_round_rate(hw, rate, parent_rate, > > + NULL, cd->div.width, > > cd->div.flags, > > + val); > > + } else { > > + return divider_round_rate(hw, rate, parent_rate, > > + NULL, cd->div.width, cd- > > >div.flags); > > + } > How about this: > ``` > if (cd->div_en) > return divider_round_rate(hw, rate, parent_rate, > NULL, cd->div.width, cd->div.flags); > > > regmap_read(cd->common.map, cd->common.cfg0, &val); > val = val >> cd->div.shift; > val &= GENMASK(cd->div.width - 1, 0); > return divider_ro_round_rate(hw, rate, parent_rate, > NULL, cd->div.width, cd->div.flags, > val); > ``` Sounds quite reasonable. Although this function is going to hell and next revision of this patchset will only contain the function below (I will do the change on that function). > > > +} > > + > > +static int ccu_div_determine_rate(struct clk_hw *hw, > > + struct clk_rate_request *req) > > +{ > > + struct ccu_div *cd = hw_to_ccu_div(hw); > > + unsigned int val; > > + > > + if (!cd->div_en) { > > + regmap_read(cd->common.map, cd->common.cfg0, &val); > > + val = val >> cd->div.shift; > > + val &= GENMASK(cd->div.width - 1, 0); > > + return divider_ro_determine_rate(hw, req, NULL, > > + cd->div.width, > > + cd->div.flags, > > + val); > > + } else { > > + return divider_determine_rate(hw, req, NULL, > > + cd->div.width, cd- > > >div.flags); > ditto. > > > + } > > +} > > + > > +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long parent_rate) > > +{ > > + struct ccu_div *cd = hw_to_ccu_div(hw); > > + int val = divider_get_val(rate, parent_rate, NULL, > > + cd->div.width, cd->div.flags); > > + unsigned int curr_val, reg_val; > > + > > + if (val < 0) > > + return val; > > + > > + regmap_read(cd->common.map, cd->common.cfg0, ®_val); > > + curr_val = reg_val; > > + curr_val = curr_val >> cd->div.shift; > > + curr_val &= GENMASK(cd->div.width - 1, 0); > > + > > + if (!cd->div_en && curr_val != val) > > + return -EINVAL; > > + > > + reg_val &= ~cd->div_en; > > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > > + udelay(1); > > + > > + reg_val &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd- > > >div.shift); > > + reg_val |= val << cd->div.shift; > > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > > + > > + reg_val |= cd->div_en; > > + regmap_write(cd->common.map, cd->common.cfg0, reg_val); > > + > > + return 0; > > +} > > + > > static u8 ccu_div_get_parent(struct clk_hw *hw) > > { > > struct ccu_div *cd = hw_to_ccu_div(hw); > > @@ -240,7 +313,9 @@ static const struct clk_ops ccu_div_ops = { > > .get_parent = ccu_div_get_parent, > > .set_parent = ccu_div_set_parent, > > .recalc_rate = ccu_div_recalc_rate, > > - .determine_rate = clk_hw_determine_rate_no_reparent, > > + .round_rate = ccu_div_round_rate, > > + .set_rate = ccu_div_set_rate, > > + .determine_rate = ccu_div_determine_rate, > > }; > > > > static void ccu_pll_disable(struct clk_hw *hw) > > @@ -784,6 +859,7 @@ static struct ccu_div venc_clk = { > > }; > > > > static struct ccu_div dpu0_clk = { > > + .div_en = BIT(8), > > .div = TH_CCU_DIV_FLAGS(0, 8, > > CLK_DIVIDER_ONE_BASED), > > .common = { > > .clkid = CLK_DPU0, > > @@ -791,7 +867,7 @@ static struct ccu_div dpu0_clk = { > > .hw.init = CLK_HW_INIT_PARENTS_HW("dpu0", > > dpu0_pll_clk_parent, > > &ccu_div_ops, > > - 0), > > + CLK_SET_RATE_UNGATE), > > }, > > }; > > > > @@ -800,6 +876,7 @@ static const struct clk_parent_data > > dpu0_clk_pd[] = { > > }; > > > > static struct ccu_div dpu1_clk = { > > + .div_en = BIT(8), > > .div = TH_CCU_DIV_FLAGS(0, 8, > > CLK_DIVIDER_ONE_BASED), > > .common = { > > .clkid = CLK_DPU1, > > @@ -807,7 +884,7 @@ static struct ccu_div dpu1_clk = { > > .hw.init = CLK_HW_INIT_PARENTS_HW("dpu1", > > dpu1_pll_clk_parent, > > &ccu_div_ops, > > - 0), > > + CLK_SET_RATE_UNGATE), > > }, > > }; > > > > @@ -891,9 +968,9 @@ static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, > > "gpu-core-clk", video_pll_clk_pd, > > static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", > > video_pll_clk_pd, 0x0, BIT(4), 0); > > static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", > > - dpu0_clk_pd, 0x0, BIT(5), 0); > > + dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); > > static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", > > - dpu1_clk_pd, 0x0, BIT(6), 0); > > + dpu1_clk_pd, 0x0, BIT(6), CLK_SET_RATE_PARENT); > > static CCU_GATE(CLK_DPU_HCLK, dpu_hclk, "dpu-hclk", > > video_pll_clk_pd, 0x0, > > BIT(7), 0); > > static CCU_GATE(CLK_DPU_ACLK, dpu_aclk, "dpu-aclk", > > video_pll_clk_pd, 0x0, > > -- > > 2.50.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] clk: thead: support changing DPU pixel clock rate 2025-08-12 5:42 ` [PATCH 2/4] clk: thead: support changing DPU pixel clock rate Icenowy Zheng 2025-08-13 2:04 ` Troy Mitchell @ 2025-08-21 18:29 ` Brian Masney 1 sibling, 0 replies; 17+ messages in thread From: Brian Masney @ 2025-08-21 18:29 UTC (permalink / raw) To: Icenowy Zheng Cc: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski, linux-riscv, linux-clk, linux-kernel On Tue, Aug 12, 2025 at 01:42:56PM +0800, Icenowy Zheng wrote: > The DPU pixel clock rate corresponds to the required dot clock of the > display mode, so it needs to be tweakable. > > Add support to change it, by adding generic divider setting code, > arming the code to the dpu0/dpu1 clocks, and setting the pixel clock > connected to the DPU (after a gate) to CLK_SET_RATE_PARENT to propagate > it to the dividers. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > drivers/clk/thead/clk-th1520-ap.c | 87 +++++++++++++++++++++++++++++-- > 1 file changed, 82 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c > index 2f87c7c2c3baf..3e81f3051cd6c 100644 > --- a/drivers/clk/thead/clk-th1520-ap.c > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -55,6 +55,7 @@ struct ccu_gate { > > struct ccu_div { > u32 enable; > + u32 div_en; > struct ccu_div_internal div; > struct ccu_internal mux; > struct ccu_common common; > @@ -198,6 +199,78 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, > return rate; > } > > +static long ccu_div_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct ccu_div *cd = hw_to_ccu_div(hw); > + unsigned int val; > + > + if (!cd->div_en) { > + regmap_read(cd->common.map, cd->common.cfg0, &val); > + val = val >> cd->div.shift; > + val &= GENMASK(cd->div.width - 1, 0); > + return divider_ro_round_rate(hw, rate, parent_rate, > + NULL, cd->div.width, cd->div.flags, > + val); > + } else { > + return divider_round_rate(hw, rate, parent_rate, > + NULL, cd->div.width, cd->div.flags); > + } > +} The round_rate clk op is deprecated. Please convert this over to use determine_rate. Brian ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 2025-08-12 5:42 ` [PATCH 1/4] clk: thead: add support for enabling/disabling PLLs Icenowy Zheng 2025-08-12 5:42 ` [PATCH 2/4] clk: thead: support changing DPU pixel clock rate Icenowy Zheng @ 2025-08-12 5:42 ` Icenowy Zheng 2025-08-12 5:56 ` Icenowy Zheng ` (2 more replies) 2025-08-12 5:42 ` [PATCH 4/4] clk: thead: th1520-ap: fix parent of padctrl0 clock Icenowy Zheng 2025-08-12 13:42 ` [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 4 siblings, 3 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 5:42 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The AXI crossbar of TH1520 has no proper timeout handling, which means gating AXI clocks can easily lead to bus timeout and thus system hang. Set all AXI clock gates to CLK_IS_CRITICAL. All these clock gates are ungated by default on system reset. In addition, convert all current CLK_IGNORE_UNUSED usage to CLK_IS_CRITICAL to prevent unwanted clock gating. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- drivers/clk/thead/clk-th1520-ap.c | 42 ++++++++++++++++--------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 3e81f3051cd6c..0117e5ea1bf58 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -589,7 +589,7 @@ static struct ccu_div axi4_cpusys2_aclk = { .hw.init = CLK_HW_INIT_PARENTS_HW("axi4-cpusys2-aclk", gmac_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -611,7 +611,7 @@ static struct ccu_div axi_aclk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("axi-aclk", axi_parents, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -760,7 +760,7 @@ static struct ccu_div apb_pclk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("apb-pclk", apb_parents, &ccu_div_ops, - CLK_IGNORE_UNUSED), + CLK_IS_CRITICAL), }, }; @@ -791,7 +791,7 @@ static struct ccu_div vi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vi", video_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -816,7 +816,7 @@ static struct ccu_div vo_axi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vo-axi", video_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -841,7 +841,7 @@ static struct ccu_div vp_axi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vp-axi", video_pll_clk_parent, &ccu_div_ops, - CLK_IGNORE_UNUSED), + CLK_IS_CRITICAL), }, }; @@ -902,23 +902,25 @@ static const struct clk_parent_data emmc_sdio_ref_clk_pd[] = { static CCU_GATE(CLK_BROM, brom_clk, "brom", ahb2_cpusys_hclk_pd, 0x100, BIT(4), 0); static CCU_GATE(CLK_BMU, bmu_clk, "bmu", axi4_cpusys2_aclk_pd, 0x100, BIT(5), 0); static CCU_GATE(CLK_AON2CPU_A2X, aon2cpu_a2x_clk, "aon2cpu-a2x", axi4_cpusys2_aclk_pd, - 0x134, BIT(8), 0); + 0x134, BIT(8), CLK_IS_CRITICAL); static CCU_GATE(CLK_X2X_CPUSYS, x2x_cpusys_clk, "x2x-cpusys", axi4_cpusys2_aclk_pd, - 0x134, BIT(7), 0); + 0x134, BIT(7), CLK_IS_CRITICAL); static CCU_GATE(CLK_CPU2AON_X2H, cpu2aon_x2h_clk, "cpu2aon-x2h", axi_aclk_pd, - 0x138, BIT(8), CLK_IGNORE_UNUSED); + 0x138, BIT(8), CLK_IS_CRITICAL); static CCU_GATE(CLK_CPU2PERI_X2H, cpu2peri_x2h_clk, "cpu2peri-x2h", axi4_cpusys2_aclk_pd, - 0x140, BIT(9), CLK_IGNORE_UNUSED); + 0x140, BIT(9), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB1_HCLK, perisys_apb1_hclk, "perisys-apb1-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(9), CLK_IGNORE_UNUSED); + 0x150, BIT(9), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB2_HCLK, perisys_apb2_hclk, "perisys-apb2-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(10), CLK_IGNORE_UNUSED); + 0x150, BIT(10), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB3_HCLK, perisys_apb3_hclk, "perisys-apb3-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(11), CLK_IGNORE_UNUSED); + 0x150, BIT(11), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys-apb4-hclk", perisys_ahb_hclk_pd, 0x150, BIT(12), 0); static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), 0); static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), 0); +static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), CLK_IS_CRITICAL); +static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), CLK_IS_CRITICAL); static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", emmc_sdio_ref_clk_pd, 0x204, BIT(30), 0); static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, 0x204, BIT(26), 0); static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", perisys_apb_pclk_pd, 0x204, BIT(24), 0); @@ -962,11 +964,11 @@ static CCU_GATE(CLK_SRAM2, sram2_clk, "sram2", axi_aclk_pd, 0x20c, BIT(2), 0); static CCU_GATE(CLK_SRAM3, sram3_clk, "sram3", axi_aclk_pd, 0x20c, BIT(1), 0); static CCU_GATE(CLK_AXI4_VO_ACLK, axi4_vo_aclk, "axi4-vo-aclk", - video_pll_clk_pd, 0x0, BIT(0), 0); + video_pll_clk_pd, 0x0, BIT(0), CLK_IS_CRITICAL); static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd, 0x0, BIT(3), 0); static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", - video_pll_clk_pd, 0x0, BIT(4), 0); + video_pll_clk_pd, 0x0, BIT(4), CLK_IS_CRITICAL); static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", @@ -998,9 +1000,9 @@ static CCU_GATE(CLK_MIPI_DSI1_REFCLK, mipi_dsi1_refclk, "mipi-dsi1-refclk", static CCU_GATE(CLK_HDMI_I2S, hdmi_i2s_clk, "hdmi-i2s-clk", video_pll_clk_pd, 0x0, BIT(19), 0); static CCU_GATE(CLK_X2H_DPU1_ACLK, x2h_dpu1_aclk, "x2h-dpu1-aclk", - video_pll_clk_pd, 0x0, BIT(20), 0); + video_pll_clk_pd, 0x0, BIT(20), CLK_IS_CRITICAL); static CCU_GATE(CLK_X2H_DPU_ACLK, x2h_dpu_aclk, "x2h-dpu-aclk", - video_pll_clk_pd, 0x0, BIT(21), 0); + video_pll_clk_pd, 0x0, BIT(21), CLK_IS_CRITICAL); static CCU_GATE(CLK_AXI4_VO_PCLK, axi4_vo_pclk, "axi4-vo-pclk", video_pll_clk_pd, 0x0, BIT(22), 0); static CCU_GATE(CLK_IOPMP_VOSYS_DPU_PCLK, iopmp_vosys_dpu_pclk, @@ -1010,11 +1012,11 @@ static CCU_GATE(CLK_IOPMP_VOSYS_DPU1_PCLK, iopmp_vosys_dpu1_pclk, static CCU_GATE(CLK_IOPMP_VOSYS_GPU_PCLK, iopmp_vosys_gpu_pclk, "iopmp-vosys-gpu-pclk", video_pll_clk_pd, 0x0, BIT(25), 0); static CCU_GATE(CLK_IOPMP_DPU1_ACLK, iopmp_dpu1_aclk, "iopmp-dpu1-aclk", - video_pll_clk_pd, 0x0, BIT(27), 0); + video_pll_clk_pd, 0x0, BIT(27), CLK_IS_CRITICAL); static CCU_GATE(CLK_IOPMP_DPU_ACLK, iopmp_dpu_aclk, "iopmp-dpu-aclk", - video_pll_clk_pd, 0x0, BIT(28), 0); + video_pll_clk_pd, 0x0, BIT(28), CLK_IS_CRITICAL); static CCU_GATE(CLK_IOPMP_GPU_ACLK, iopmp_gpu_aclk, "iopmp-gpu-aclk", - video_pll_clk_pd, 0x0, BIT(29), 0); + video_pll_clk_pd, 0x0, BIT(29), CLK_IS_CRITICAL); static CCU_GATE(CLK_MIPIDSI0_PIXCLK, mipi_dsi0_pixclk, "mipi-dsi0-pixclk", video_pll_clk_pd, 0x0, BIT(30), 0); static CCU_GATE(CLK_MIPIDSI1_PIXCLK, mipi_dsi1_pixclk, "mipi-dsi1-pixclk", -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng @ 2025-08-12 5:56 ` Icenowy Zheng 2025-08-12 6:04 ` [PATCH 3/4 FIXED] " Icenowy Zheng 2025-08-12 6:23 ` [PATCH 3/4] " Drew Fustini 2 siblings, 0 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 5:56 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel 在 2025-08-12星期二的 13:42 +0800，Icenowy Zheng写道： > The AXI crossbar of TH1520 has no proper timeout handling, which > means > gating AXI clocks can easily lead to bus timeout and thus system > hang. > > Set all AXI clock gates to CLK_IS_CRITICAL. All these clock gates are > ungated by default on system reset. > > In addition, convert all current CLK_IGNORE_UNUSED usage to > CLK_IS_CRITICAL to prevent unwanted clock gating. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > drivers/clk/thead/clk-th1520-ap.c | 42 ++++++++++++++++------------- > -- > 1 file changed, 22 insertions(+), 20 deletions(-) > > diff --git a/drivers/clk/thead/clk-th1520-ap.c > b/drivers/clk/thead/clk-th1520-ap.c > index 3e81f3051cd6c..0117e5ea1bf58 100644 > --- a/drivers/clk/thead/clk-th1520-ap.c > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -589,7 +589,7 @@ static struct ccu_div axi4_cpusys2_aclk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("axi4- > cpusys2-aclk", > gmac_pll_clk_parent, > &ccu_div_ops, > - 0), > + CLK_IS_CRITICAL), > }, > }; > > @@ -611,7 +611,7 @@ static struct ccu_div axi_aclk = { > .hw.init = CLK_HW_INIT_PARENTS_DATA("axi- > aclk", > axi_parents, > &ccu_div_ops, > - 0), > + > CLK_IS_CRITICAL), > }, > }; > > @@ -760,7 +760,7 @@ static struct ccu_div apb_pclk = { > .hw.init = CLK_HW_INIT_PARENTS_DATA("apb- > pclk", > apb_parents, > &ccu_div_ops, > - > CLK_IGNORE_UNUSED), > + > CLK_IS_CRITICAL), > }, > }; > > @@ -791,7 +791,7 @@ static struct ccu_div vi_clk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("vi", > video_pll_clk_parent, > &ccu_div_ops, > - 0), > + CLK_IS_CRITICAL), > }, > }; > > @@ -816,7 +816,7 @@ static struct ccu_div vo_axi_clk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("vo-axi", > video_pll_clk_parent, > &ccu_div_ops, > - 0), > + CLK_IS_CRITICAL), > }, > }; > > @@ -841,7 +841,7 @@ static struct ccu_div vp_axi_clk = { > .hw.init = CLK_HW_INIT_PARENTS_HW("vp-axi", > video_pll_clk_parent, > &ccu_div_ops, > - CLK_IGNORE_UNUSED), > + CLK_IS_CRITICAL), > }, > }; > > @@ -902,23 +902,25 @@ static const struct clk_parent_data > emmc_sdio_ref_clk_pd[] = { > static CCU_GATE(CLK_BROM, brom_clk, "brom", ahb2_cpusys_hclk_pd, > 0x100, BIT(4), 0); > static CCU_GATE(CLK_BMU, bmu_clk, "bmu", axi4_cpusys2_aclk_pd, > 0x100, BIT(5), 0); > static CCU_GATE(CLK_AON2CPU_A2X, aon2cpu_a2x_clk, "aon2cpu-a2x", > axi4_cpusys2_aclk_pd, > - 0x134, BIT(8), 0); > + 0x134, BIT(8), CLK_IS_CRITICAL); > static CCU_GATE(CLK_X2X_CPUSYS, x2x_cpusys_clk, "x2x-cpusys", > axi4_cpusys2_aclk_pd, > - 0x134, BIT(7), 0); > + 0x134, BIT(7), CLK_IS_CRITICAL); > static CCU_GATE(CLK_CPU2AON_X2H, cpu2aon_x2h_clk, "cpu2aon-x2h", > axi_aclk_pd, > - 0x138, BIT(8), CLK_IGNORE_UNUSED); > + 0x138, BIT(8), CLK_IS_CRITICAL); > static CCU_GATE(CLK_CPU2PERI_X2H, cpu2peri_x2h_clk, "cpu2peri-x2h", > axi4_cpusys2_aclk_pd, > - 0x140, BIT(9), CLK_IGNORE_UNUSED); > + 0x140, BIT(9), CLK_IS_CRITICAL); > static CCU_GATE(CLK_PERISYS_APB1_HCLK, perisys_apb1_hclk, "perisys- > apb1-hclk", perisys_ahb_hclk_pd, > - 0x150, BIT(9), CLK_IGNORE_UNUSED); > + 0x150, BIT(9), CLK_IS_CRITICAL); > static CCU_GATE(CLK_PERISYS_APB2_HCLK, perisys_apb2_hclk, "perisys- > apb2-hclk", perisys_ahb_hclk_pd, > - 0x150, BIT(10), CLK_IGNORE_UNUSED); > + 0x150, BIT(10), CLK_IS_CRITICAL); > static CCU_GATE(CLK_PERISYS_APB3_HCLK, perisys_apb3_hclk, "perisys- > apb3-hclk", perisys_ahb_hclk_pd, > - 0x150, BIT(11), CLK_IGNORE_UNUSED); > + 0x150, BIT(11), CLK_IS_CRITICAL); > static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys- > apb4-hclk", perisys_ahb_hclk_pd, > 0x150, BIT(12), 0); > static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, > 0x1c8, BIT(5), 0); > static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, > 0x1e0, BIT(13), 0); > +static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, > 0x1c8, BIT(5), CLK_IS_CRITICAL); > +static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, > 0x1e0, BIT(13), CLK_IS_CRITICAL); Oops... sorry, my rebase operation broke this patch. Previously this patch was the 4th (in the sequence of being written), but I rebased it to be the 3rd, and this code being in the same chunk with perisys-apb4-hclk breaked it. > static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", > emmc_sdio_ref_clk_pd, 0x204, BIT(30), 0); > static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, > 0x204, BIT(26), 0); > static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", > perisys_apb_pclk_pd, 0x204, BIT(24), 0); > @@ -962,11 +964,11 @@ static CCU_GATE(CLK_SRAM2, sram2_clk, "sram2", > axi_aclk_pd, 0x20c, BIT(2), 0); > static CCU_GATE(CLK_SRAM3, sram3_clk, "sram3", axi_aclk_pd, 0x20c, > BIT(1), 0); > > static CCU_GATE(CLK_AXI4_VO_ACLK, axi4_vo_aclk, "axi4-vo-aclk", > - video_pll_clk_pd, 0x0, BIT(0), 0); > + video_pll_clk_pd, 0x0, BIT(0), CLK_IS_CRITICAL); > static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", > video_pll_clk_pd, > 0x0, BIT(3), 0); > static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", > - video_pll_clk_pd, 0x0, BIT(4), 0); > + video_pll_clk_pd, 0x0, BIT(4), CLK_IS_CRITICAL); > static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", > dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); > static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", > @@ -998,9 +1000,9 @@ static CCU_GATE(CLK_MIPI_DSI1_REFCLK, > mipi_dsi1_refclk, "mipi-dsi1-refclk", > static CCU_GATE(CLK_HDMI_I2S, hdmi_i2s_clk, "hdmi-i2s-clk", > video_pll_clk_pd, > 0x0, BIT(19), 0); > static CCU_GATE(CLK_X2H_DPU1_ACLK, x2h_dpu1_aclk, "x2h-dpu1-aclk", > - video_pll_clk_pd, 0x0, BIT(20), 0); > + video_pll_clk_pd, 0x0, BIT(20), CLK_IS_CRITICAL); > static CCU_GATE(CLK_X2H_DPU_ACLK, x2h_dpu_aclk, "x2h-dpu-aclk", > - video_pll_clk_pd, 0x0, BIT(21), 0); > + video_pll_clk_pd, 0x0, BIT(21), CLK_IS_CRITICAL); > static CCU_GATE(CLK_AXI4_VO_PCLK, axi4_vo_pclk, "axi4-vo-pclk", > video_pll_clk_pd, 0x0, BIT(22), 0); > static CCU_GATE(CLK_IOPMP_VOSYS_DPU_PCLK, iopmp_vosys_dpu_pclk, > @@ -1010,11 +1012,11 @@ static CCU_GATE(CLK_IOPMP_VOSYS_DPU1_PCLK, > iopmp_vosys_dpu1_pclk, > static CCU_GATE(CLK_IOPMP_VOSYS_GPU_PCLK, iopmp_vosys_gpu_pclk, > "iopmp-vosys-gpu-pclk", video_pll_clk_pd, 0x0, > BIT(25), 0); > static CCU_GATE(CLK_IOPMP_DPU1_ACLK, iopmp_dpu1_aclk, "iopmp-dpu1- > aclk", > - video_pll_clk_pd, 0x0, BIT(27), 0); > + video_pll_clk_pd, 0x0, BIT(27), CLK_IS_CRITICAL); > static CCU_GATE(CLK_IOPMP_DPU_ACLK, iopmp_dpu_aclk, "iopmp-dpu- > aclk", > - video_pll_clk_pd, 0x0, BIT(28), 0); > + video_pll_clk_pd, 0x0, BIT(28), CLK_IS_CRITICAL); > static CCU_GATE(CLK_IOPMP_GPU_ACLK, iopmp_gpu_aclk, "iopmp-gpu- > aclk", > - video_pll_clk_pd, 0x0, BIT(29), 0); > + video_pll_clk_pd, 0x0, BIT(29), CLK_IS_CRITICAL); > static CCU_GATE(CLK_MIPIDSI0_PIXCLK, mipi_dsi0_pixclk, "mipi-dsi0- > pixclk", > video_pll_clk_pd, 0x0, BIT(30), 0); > static CCU_GATE(CLK_MIPIDSI1_PIXCLK, mipi_dsi1_pixclk, "mipi-dsi1- > pixclk", ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4 FIXED] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng 2025-08-12 5:56 ` Icenowy Zheng @ 2025-08-12 6:04 ` Icenowy Zheng 2025-08-12 6:26 ` Drew Fustini 2025-08-12 6:23 ` [PATCH 3/4] " Drew Fustini 2 siblings, 1 reply; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 6:04 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The AXI crossbar of TH1520 has no proper timeout handling, which means gating AXI clocks can easily lead to bus timeout and thus system hang. Set all AXI clock gates to CLK_IS_CRITICAL. All these clock gates are ungated by default on system reset. In addition, convert all current CLK_IGNORE_UNUSED usage to CLK_IS_CRITICAL to prevent unwanted clock gating. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- This is for fixing my unfortunate rebasing error when tweaking the sequence of the patchset. Please ignore the original 3/4, which created a build failure because of forgetting to remove extra definition of npu-axi and cpu2vp. drivers/clk/thead/clk-th1520-ap.c | 44 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 2544e35a7bfa2..75ea1705cc08f 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -589,7 +589,7 @@ static struct ccu_div axi4_cpusys2_aclk = { .hw.init = CLK_HW_INIT_PARENTS_HW("axi4-cpusys2-aclk", gmac_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -611,7 +611,7 @@ static struct ccu_div axi_aclk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("axi-aclk", axi_parents, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -760,7 +760,7 @@ static struct ccu_div apb_pclk = { .hw.init = CLK_HW_INIT_PARENTS_DATA("apb-pclk", apb_parents, &ccu_div_ops, - CLK_IGNORE_UNUSED), + CLK_IS_CRITICAL), }, }; @@ -791,7 +791,7 @@ static struct ccu_div vi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vi", video_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -816,7 +816,7 @@ static struct ccu_div vo_axi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vo-axi", video_pll_clk_parent, &ccu_div_ops, - 0), + CLK_IS_CRITICAL), }, }; @@ -841,7 +841,7 @@ static struct ccu_div vp_axi_clk = { .hw.init = CLK_HW_INIT_PARENTS_HW("vp-axi", video_pll_clk_parent, &ccu_div_ops, - CLK_IGNORE_UNUSED), + CLK_IS_CRITICAL), }, }; @@ -902,23 +902,23 @@ static const struct clk_parent_data emmc_sdio_ref_clk_pd[] = { static CCU_GATE(CLK_BROM, brom_clk, "brom", ahb2_cpusys_hclk_pd, 0x100, BIT(4), 0); static CCU_GATE(CLK_BMU, bmu_clk, "bmu", axi4_cpusys2_aclk_pd, 0x100, BIT(5), 0); static CCU_GATE(CLK_AON2CPU_A2X, aon2cpu_a2x_clk, "aon2cpu-a2x", axi4_cpusys2_aclk_pd, - 0x134, BIT(8), 0); + 0x134, BIT(8), CLK_IS_CRITICAL); static CCU_GATE(CLK_X2X_CPUSYS, x2x_cpusys_clk, "x2x-cpusys", axi4_cpusys2_aclk_pd, - 0x134, BIT(7), 0); + 0x134, BIT(7), CLK_IS_CRITICAL); static CCU_GATE(CLK_CPU2AON_X2H, cpu2aon_x2h_clk, "cpu2aon-x2h", axi_aclk_pd, - 0x138, BIT(8), CLK_IGNORE_UNUSED); + 0x138, BIT(8), CLK_IS_CRITICAL); static CCU_GATE(CLK_CPU2PERI_X2H, cpu2peri_x2h_clk, "cpu2peri-x2h", axi4_cpusys2_aclk_pd, - 0x140, BIT(9), CLK_IGNORE_UNUSED); + 0x140, BIT(9), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB1_HCLK, perisys_apb1_hclk, "perisys-apb1-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(9), CLK_IGNORE_UNUSED); + 0x150, BIT(9), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB2_HCLK, perisys_apb2_hclk, "perisys-apb2-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(10), CLK_IGNORE_UNUSED); + 0x150, BIT(10), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB3_HCLK, perisys_apb3_hclk, "perisys-apb3-hclk", perisys_ahb_hclk_pd, - 0x150, BIT(11), CLK_IGNORE_UNUSED); + 0x150, BIT(11), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys-apb4-hclk", perisys_ahb_hclk_pd, 0x150, BIT(12), 0); -static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), 0); -static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), 0); +static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), CLK_IS_CRITICAL); +static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), CLK_IS_CRITICAL); static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", emmc_sdio_ref_clk_pd, 0x204, BIT(30), 0); static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, 0x204, BIT(26), 0); static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", perisys_apb_pclk_pd, 0x204, BIT(24), 0); @@ -962,11 +962,11 @@ static CCU_GATE(CLK_SRAM2, sram2_clk, "sram2", axi_aclk_pd, 0x20c, BIT(2), 0); static CCU_GATE(CLK_SRAM3, sram3_clk, "sram3", axi_aclk_pd, 0x20c, BIT(1), 0); static CCU_GATE(CLK_AXI4_VO_ACLK, axi4_vo_aclk, "axi4-vo-aclk", - video_pll_clk_pd, 0x0, BIT(0), 0); + video_pll_clk_pd, 0x0, BIT(0), CLK_IS_CRITICAL); static CCU_GATE(CLK_GPU_CORE, gpu_core_clk, "gpu-core-clk", video_pll_clk_pd, 0x0, BIT(3), 0); static CCU_GATE(CLK_GPU_CFG_ACLK, gpu_cfg_aclk, "gpu-cfg-aclk", - video_pll_clk_pd, 0x0, BIT(4), 0); + video_pll_clk_pd, 0x0, BIT(4), CLK_IS_CRITICAL); static CCU_GATE(CLK_DPU_PIXELCLK0, dpu0_pixelclk, "dpu0-pixelclk", dpu0_clk_pd, 0x0, BIT(5), CLK_SET_RATE_PARENT); static CCU_GATE(CLK_DPU_PIXELCLK1, dpu1_pixelclk, "dpu1-pixelclk", @@ -998,9 +998,9 @@ static CCU_GATE(CLK_MIPI_DSI1_REFCLK, mipi_dsi1_refclk, "mipi-dsi1-refclk", static CCU_GATE(CLK_HDMI_I2S, hdmi_i2s_clk, "hdmi-i2s-clk", video_pll_clk_pd, 0x0, BIT(19), 0); static CCU_GATE(CLK_X2H_DPU1_ACLK, x2h_dpu1_aclk, "x2h-dpu1-aclk", - video_pll_clk_pd, 0x0, BIT(20), 0); + video_pll_clk_pd, 0x0, BIT(20), CLK_IS_CRITICAL); static CCU_GATE(CLK_X2H_DPU_ACLK, x2h_dpu_aclk, "x2h-dpu-aclk", - video_pll_clk_pd, 0x0, BIT(21), 0); + video_pll_clk_pd, 0x0, BIT(21), CLK_IS_CRITICAL); static CCU_GATE(CLK_AXI4_VO_PCLK, axi4_vo_pclk, "axi4-vo-pclk", video_pll_clk_pd, 0x0, BIT(22), 0); static CCU_GATE(CLK_IOPMP_VOSYS_DPU_PCLK, iopmp_vosys_dpu_pclk, @@ -1010,11 +1010,11 @@ static CCU_GATE(CLK_IOPMP_VOSYS_DPU1_PCLK, iopmp_vosys_dpu1_pclk, static CCU_GATE(CLK_IOPMP_VOSYS_GPU_PCLK, iopmp_vosys_gpu_pclk, "iopmp-vosys-gpu-pclk", video_pll_clk_pd, 0x0, BIT(25), 0); static CCU_GATE(CLK_IOPMP_DPU1_ACLK, iopmp_dpu1_aclk, "iopmp-dpu1-aclk", - video_pll_clk_pd, 0x0, BIT(27), 0); + video_pll_clk_pd, 0x0, BIT(27), CLK_IS_CRITICAL); static CCU_GATE(CLK_IOPMP_DPU_ACLK, iopmp_dpu_aclk, "iopmp-dpu-aclk", - video_pll_clk_pd, 0x0, BIT(28), 0); + video_pll_clk_pd, 0x0, BIT(28), CLK_IS_CRITICAL); static CCU_GATE(CLK_IOPMP_GPU_ACLK, iopmp_gpu_aclk, "iopmp-gpu-aclk", - video_pll_clk_pd, 0x0, BIT(29), 0); + video_pll_clk_pd, 0x0, BIT(29), CLK_IS_CRITICAL); static CCU_GATE(CLK_MIPIDSI0_PIXCLK, mipi_dsi0_pixclk, "mipi-dsi0-pixclk", video_pll_clk_pd, 0x0, BIT(30), 0); static CCU_GATE(CLK_MIPIDSI1_PIXCLK, mipi_dsi1_pixclk, "mipi-dsi1-pixclk", -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4 FIXED] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL 2025-08-12 6:04 ` [PATCH 3/4 FIXED] " Icenowy Zheng @ 2025-08-12 6:26 ` Drew Fustini 0 siblings, 0 replies; 17+ messages in thread From: Drew Fustini @ 2025-08-12 6:26 UTC (permalink / raw) To: Icenowy Zheng Cc: Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski, linux-riscv, linux-clk, linux-kernel On Tue, Aug 12, 2025 at 02:04:08PM +0800, Icenowy Zheng wrote: > The AXI crossbar of TH1520 has no proper timeout handling, which means > gating AXI clocks can easily lead to bus timeout and thus system hang. > > Set all AXI clock gates to CLK_IS_CRITICAL. All these clock gates are > ungated by default on system reset. > > In addition, convert all current CLK_IGNORE_UNUSED usage to > CLK_IS_CRITICAL to prevent unwanted clock gating. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > This is for fixing my unfortunate rebasing error when tweaking the > sequence of the patchset. > > Please ignore the original 3/4, which created a build failure because of > forgetting to remove extra definition of npu-axi and cpu2vp. Thanks for fixing. I'll review the rest. -Drew ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng 2025-08-12 5:56 ` Icenowy Zheng 2025-08-12 6:04 ` [PATCH 3/4 FIXED] " Icenowy Zheng @ 2025-08-12 6:23 ` Drew Fustini 2 siblings, 0 replies; 17+ messages in thread From: Drew Fustini @ 2025-08-12 6:23 UTC (permalink / raw) To: Icenowy Zheng Cc: Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski, linux-riscv, linux-clk, linux-kernel On Tue, Aug 12, 2025 at 01:42:57PM +0800, Icenowy Zheng wrote: > The AXI crossbar of TH1520 has no proper timeout handling, which means > gating AXI clocks can easily lead to bus timeout and thus system hang. > > Set all AXI clock gates to CLK_IS_CRITICAL. All these clock gates are > ungated by default on system reset. > > In addition, convert all current CLK_IGNORE_UNUSED usage to > CLK_IS_CRITICAL to prevent unwanted clock gating. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > drivers/clk/thead/clk-th1520-ap.c | 42 ++++++++++++++++--------------- > 1 file changed, 22 insertions(+), 20 deletions(-) Thanks for working on the display controller. I'll review the series but I wanted to point out a simple error first: > static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), 0); > static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), 0); > +static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), CLK_IS_CRITICAL); > +static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), CLK_IS_CRITICAL); The compiler complains about redefinition of npu_axi_clk and cpu2vp_clk. I've fixed it up by removing the old lines with '0' flags, but I'm noting in case there is another revision. Thanks, Drew ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] clk: thead: th1520-ap: fix parent of padctrl0 clock 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng ` (2 preceding siblings ...) 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng @ 2025-08-12 5:42 ` Icenowy Zheng 2025-08-12 6:04 ` [PATCH 4/4 FIXED] " Icenowy Zheng 2025-08-12 13:42 ` [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 4 siblings, 1 reply; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 5:42 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The padctrl0 clock seems to be a child of the perisys_apb4_hclk clock, gating the later makes padctrl0 registers stuck. Fix this relationship. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- drivers/clk/thead/clk-th1520-ap.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 0117e5ea1bf58..19f6b0285390d 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -917,6 +917,11 @@ static CCU_GATE(CLK_PERISYS_APB3_HCLK, perisys_apb3_hclk, "perisys-apb3-hclk", p 0x150, BIT(11), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys-apb4-hclk", perisys_ahb_hclk_pd, 0x150, BIT(12), 0); + +static const struct clk_parent_data perisys_apb4_hclk_pd[] = { + { .hw = &perisys_apb4_hclk.common.hw }, +}; + static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), 0); static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), 0); static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), CLK_IS_CRITICAL); @@ -925,7 +930,7 @@ static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", emmc_sdio_ref_clk_pd, static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, 0x204, BIT(26), 0); static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", perisys_apb_pclk_pd, 0x204, BIT(24), 0); static CCU_GATE(CLK_DSMART, dsmart_clk, "dsmart", perisys_apb_pclk_pd, 0x204, BIT(23), 0); -static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", perisys_apb_pclk_pd, 0x204, BIT(22), 0); +static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", perisys_apb4_hclk_pd, 0x204, BIT(22), 0); static CCU_GATE(CLK_GMAC_AXI, gmac_axi_clk, "gmac-axi", axi4_cpusys2_aclk_pd, 0x204, BIT(21), 0); static CCU_GATE(CLK_GPIO3, gpio3_clk, "gpio3-clk", peri2sys_apb_pclk_pd, 0x204, BIT(20), 0); static CCU_GATE(CLK_GMAC0, gmac0_clk, "gmac0", gmac_pll_clk_pd, 0x204, BIT(19), 0); -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4 FIXED] clk: thead: th1520-ap: fix parent of padctrl0 clock 2025-08-12 5:42 ` [PATCH 4/4] clk: thead: th1520-ap: fix parent of padctrl0 clock Icenowy Zheng @ 2025-08-12 6:04 ` Icenowy Zheng 2025-08-12 9:17 ` Icenowy Zheng 0 siblings, 1 reply; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 6:04 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel, Icenowy Zheng The padctrl0 clock seems to be a child of the perisys_apb4_hclk clock, gating the later makes padctrl0 registers stuck. Fix this relationship. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- This is the consequence of fixing 3/4, because these two patches are at nearly the same position, and unfortunately get dependent by the algorithm of diff. Ignore the original 4/4 and look at this too. drivers/clk/thead/clk-th1520-ap.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c index 75ea1705cc08f..0ecedac50d6cb 100644 --- a/drivers/clk/thead/clk-th1520-ap.c +++ b/drivers/clk/thead/clk-th1520-ap.c @@ -917,13 +917,18 @@ static CCU_GATE(CLK_PERISYS_APB3_HCLK, perisys_apb3_hclk, "perisys-apb3-hclk", p 0x150, BIT(11), CLK_IS_CRITICAL); static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys-apb4-hclk", perisys_ahb_hclk_pd, 0x150, BIT(12), 0); + +static const struct clk_parent_data perisys_apb4_hclk_pd[] = { + { .hw = &perisys_apb4_hclk.common.hw }, +}; + static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, 0x1c8, BIT(5), CLK_IS_CRITICAL); static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, 0x1e0, BIT(13), CLK_IS_CRITICAL); static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", emmc_sdio_ref_clk_pd, 0x204, BIT(30), 0); static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, 0x204, BIT(26), 0); static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", perisys_apb_pclk_pd, 0x204, BIT(24), 0); static CCU_GATE(CLK_DSMART, dsmart_clk, "dsmart", perisys_apb_pclk_pd, 0x204, BIT(23), 0); -static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", perisys_apb_pclk_pd, 0x204, BIT(22), 0); +static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", perisys_apb4_hclk_pd, 0x204, BIT(22), 0); static CCU_GATE(CLK_GMAC_AXI, gmac_axi_clk, "gmac-axi", axi4_cpusys2_aclk_pd, 0x204, BIT(21), 0); static CCU_GATE(CLK_GPIO3, gpio3_clk, "gpio3-clk", peri2sys_apb_pclk_pd, 0x204, BIT(20), 0); static CCU_GATE(CLK_GMAC0, gmac0_clk, "gmac0", gmac_pll_clk_pd, 0x204, BIT(19), 0); -- 2.50.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4 FIXED] clk: thead: th1520-ap: fix parent of padctrl0 clock 2025-08-12 6:04 ` [PATCH 4/4 FIXED] " Icenowy Zheng @ 2025-08-12 9:17 ` Icenowy Zheng 0 siblings, 0 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 9:17 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel 在 2025-08-12星期二的 14:04 +0800，Icenowy Zheng写道： > The padctrl0 clock seems to be a child of the perisys_apb4_hclk > clock, > gating the later makes padctrl0 registers stuck. > > Fix this relationship. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > This is the consequence of fixing 3/4, because these two patches are > at > nearly the same position, and unfortunately get dependent by the > algorithm of diff. > > Ignore the original 4/4 and look at this too. > > drivers/clk/thead/clk-th1520-ap.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/thead/clk-th1520-ap.c > b/drivers/clk/thead/clk-th1520-ap.c > index 75ea1705cc08f..0ecedac50d6cb 100644 > --- a/drivers/clk/thead/clk-th1520-ap.c > +++ b/drivers/clk/thead/clk-th1520-ap.c > @@ -917,13 +917,18 @@ static CCU_GATE(CLK_PERISYS_APB3_HCLK, > perisys_apb3_hclk, "perisys-apb3-hclk", p > 0x150, BIT(11), CLK_IS_CRITICAL); > static CCU_GATE(CLK_PERISYS_APB4_HCLK, perisys_apb4_hclk, "perisys- > apb4-hclk", perisys_ahb_hclk_pd, > 0x150, BIT(12), 0); > + > +static const struct clk_parent_data perisys_apb4_hclk_pd[] = { > + { .hw = &perisys_apb4_hclk.common.hw }, > +}; > + > static CCU_GATE(CLK_NPU_AXI, npu_axi_clk, "npu-axi", axi_aclk_pd, > 0x1c8, BIT(5), CLK_IS_CRITICAL); > static CCU_GATE(CLK_CPU2VP, cpu2vp_clk, "cpu2vp", axi_aclk_pd, > 0x1e0, BIT(13), CLK_IS_CRITICAL); > static CCU_GATE(CLK_EMMC_SDIO, emmc_sdio_clk, "emmc-sdio", > emmc_sdio_ref_clk_pd, 0x204, BIT(30), 0); > static CCU_GATE(CLK_GMAC1, gmac1_clk, "gmac1", gmac_pll_clk_pd, > 0x204, BIT(26), 0); > static CCU_GATE(CLK_PADCTRL1, padctrl1_clk, "padctrl1", > perisys_apb_pclk_pd, 0x204, BIT(24), 0); > static CCU_GATE(CLK_DSMART, dsmart_clk, "dsmart", > perisys_apb_pclk_pd, 0x204, BIT(23), 0); > -static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", > perisys_apb_pclk_pd, 0x204, BIT(22), 0); > +static CCU_GATE(CLK_PADCTRL0, padctrl0_clk, "padctrl0", > perisys_apb4_hclk_pd, 0x204, BIT(22), 0); Oops looks like this does not work and orphans the clock... > static CCU_GATE(CLK_GMAC_AXI, gmac_axi_clk, "gmac-axi", > axi4_cpusys2_aclk_pd, 0x204, BIT(21), 0); > static CCU_GATE(CLK_GPIO3, gpio3_clk, "gpio3-clk", > peri2sys_apb_pclk_pd, 0x204, BIT(20), 0); > static CCU_GATE(CLK_GMAC0, gmac0_clk, "gmac0", gmac_pll_clk_pd, > 0x204, BIT(19), 0); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng ` (3 preceding siblings ...) 2025-08-12 5:42 ` [PATCH 4/4] clk: thead: th1520-ap: fix parent of padctrl0 clock Icenowy Zheng @ 2025-08-12 13:42 ` Icenowy Zheng 2025-08-12 16:24 ` Drew Fustini 4 siblings, 1 reply; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 13:42 UTC (permalink / raw) To: Drew Fustini, Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski Cc: linux-riscv, linux-clk, linux-kernel 在 2025-08-12星期二的 13:42 +0800，Icenowy Zheng写道： > This patchset is my changes to the TH1520 clock driver, mainly for > supporting the display controller (the first 3). > > The first two are functionality additions, with the first one adding > support for enabling/disabling PLLs (for DPU PLL) and the second one > adding support for changing DPU dividers. > > The 3rd one is to address hang issues met when testing the DPU driver > w/o clk_ignore_unused command line option. > > The 4th one has no relationship to display, and only exists for my > need > to change an arbitrary GPIO (well, GPIO3_3, the one controlling the > fan > on Lichee Pi 4A) with gpioset. > > This patchset has a dependency (a 0th one) [1]. > > [1] > https://lore.kernel.org/linux-riscv/20250809-fix_clocks_thead_aug_9-v1-1-299c33d7a593@samsung.com/ > Oops, looks like this patchset deserves a new revision now... PATCH 2/4 has a round_rate() implementation, which is considered out- of-date and determine_rate() will replace it. PATCH 3/4 was broken during rebasing. PATCH 4/4 is found to be not working (and even makes padctrl0 an orphan clock). Yao Zi told me that I need to first do some changes to ccu_gate code. > Icenowy Zheng (4): > clk: thead: add support for enabling/disabling PLLs > clk: thead: support changing DPU pixel clock rate > clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL > clk: thead: th1520-ap: fix parent of padctrl0 clock > > drivers/clk/thead/clk-th1520-ap.c | 174 ++++++++++++++++++++++++---- > -- > 1 file changed, 143 insertions(+), 31 deletions(-) > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver 2025-08-12 13:42 ` [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng @ 2025-08-12 16:24 ` Drew Fustini 2025-08-12 19:50 ` Icenowy Zheng 0 siblings, 1 reply; 17+ messages in thread From: Drew Fustini @ 2025-08-12 16:24 UTC (permalink / raw) To: Icenowy Zheng Cc: Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski, linux-riscv, linux-clk, linux-kernel On Tue, Aug 12, 2025 at 09:42:23PM +0800, Icenowy Zheng wrote: > 在 2025-08-12星期二的 13:42 +0800，Icenowy Zheng写道： > > This patchset is my changes to the TH1520 clock driver, mainly for > > supporting the display controller (the first 3). > > > > The first two are functionality additions, with the first one adding > > support for enabling/disabling PLLs (for DPU PLL) and the second one > > adding support for changing DPU dividers. > > > > The 3rd one is to address hang issues met when testing the DPU driver > > w/o clk_ignore_unused command line option. > > > > The 4th one has no relationship to display, and only exists for my > > need > > to change an arbitrary GPIO (well, GPIO3_3, the one controlling the > > fan > > on Lichee Pi 4A) with gpioset. > > > > This patchset has a dependency (a 0th one) [1]. > > > > [1] > > https://lore.kernel.org/linux-riscv/20250809-fix_clocks_thead_aug_9-v1-1-299c33d7a593@samsung.com/ > > > > Oops, looks like this patchset deserves a new revision now... Thanks, that does make it easier to apply with 'b4 shazam'. > PATCH 2/4 has a round_rate() implementation, which is considered out- > of-date and determine_rate() will replace it. I saw your reply over in the big patchset from Brian Masney. That's good that we will be able to skip adding round_rate() to the driver. > PATCH 3/4 was broken during rebasing. > > PATCH 4/4 is found to be not working (and even makes padctrl0 an orphan > clock). Yao Zi told me that I need to first do some changes to ccu_gate > code. Is there a way to test the functionality without additional patches from your branch? I look forward to v2. Thanks for working on the display. -Drew ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver 2025-08-12 16:24 ` Drew Fustini @ 2025-08-12 19:50 ` Icenowy Zheng 0 siblings, 0 replies; 17+ messages in thread From: Icenowy Zheng @ 2025-08-12 19:50 UTC (permalink / raw) To: Drew Fustini Cc: Guo Ren, Fu Wei, Michael Turquette, Stephen Boyd, Michal Wilczynski, linux-riscv, linux-clk, linux-kernel 在 2025-08-12星期二的 09:24 -0700，Drew Fustini写道： > On Tue, Aug 12, 2025 at 09:42:23PM +0800, Icenowy Zheng wrote: > > 在 2025-08-12星期二的 13:42 +0800，Icenowy Zheng写道： > > > This patchset is my changes to the TH1520 clock driver, mainly > > > for > > > supporting the display controller (the first 3). > > > > > > The first two are functionality additions, with the first one > > > adding > > > support for enabling/disabling PLLs (for DPU PLL) and the second > > > one > > > adding support for changing DPU dividers. > > > > > > The 3rd one is to address hang issues met when testing the DPU > > > driver > > > w/o clk_ignore_unused command line option. > > > > > > The 4th one has no relationship to display, and only exists for > > > my > > > need > > > to change an arbitrary GPIO (well, GPIO3_3, the one controlling > > > the > > > fan > > > on Lichee Pi 4A) with gpioset. > > > > > > This patchset has a dependency (a 0th one) [1]. > > > > > > [1] > > > https://lore.kernel.org/linux-riscv/20250809-fix_clocks_thead_aug_9-v1-1-299c33d7a593@samsung.com/ > > > > > > > Oops, looks like this patchset deserves a new revision now... > > Thanks, that does make it easier to apply with 'b4 shazam'. > > > PATCH 2/4 has a round_rate() implementation, which is considered > > out- > > of-date and determine_rate() will replace it. > > I saw your reply over in the big patchset from Brian Masney. That's > good > that we will be able to skip adding round_rate() to the driver. > > > PATCH 3/4 was broken during rebasing. > > > > PATCH 4/4 is found to be not working (and even makes padctrl0 an > > orphan > > clock). Yao Zi told me that I need to first do some changes to > > ccu_gate > > code. > > Is there a way to test the functionality without additional patches > from > your branch? To be honest I know about little... The 4th patch is the only one easy to test, but it's broken... (A refactor must come with it to make it work). For patch 1~3, if you are brave enough, in file drivers/clk/clk.c, change the line `#undef CLOCK_ALLOW_WRITE_DEBUGFS` 's `#undef` to `#define` may give you some testing facility in debugfs -- writing some clock rate (e.g. 148500000) to /sys/kernel/debug/clk/dpu1- pixelclk/clk_rate and then 1 to /sys/kernel/debug/clk/dpu1- pixelclk/clk_prepare_enable in this situation could mimic what the DC driver does -- I believe in /sys/kernel/debug/clk/clk_summary the effect can be shown (dpu1-pll hardware enable becomes Y and dpu1- pixelclk shows the expected rate. WARNING: THE PARAGRAPH ABOVE IS ONLY BASED ON MY READING OF CODE, NOT VERIFIED, CAN GET A DRAGON TO BURN YOUR HOUSE. > > I look forward to v2. Thanks for working on the display. > > -Drew ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-08-21 18:30 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-12 5:42 [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 2025-08-12 5:42 ` [PATCH 1/4] clk: thead: add support for enabling/disabling PLLs Icenowy Zheng 2025-08-12 5:42 ` [PATCH 2/4] clk: thead: support changing DPU pixel clock rate Icenowy Zheng 2025-08-13 2:04 ` Troy Mitchell 2025-08-13 5:54 ` Icenowy Zheng 2025-08-21 18:29 ` Brian Masney 2025-08-12 5:42 ` [PATCH 3/4] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL Icenowy Zheng 2025-08-12 5:56 ` Icenowy Zheng 2025-08-12 6:04 ` [PATCH 3/4 FIXED] " Icenowy Zheng 2025-08-12 6:26 ` Drew Fustini 2025-08-12 6:23 ` [PATCH 3/4] " Drew Fustini 2025-08-12 5:42 ` [PATCH 4/4] clk: thead: th1520-ap: fix parent of padctrl0 clock Icenowy Zheng 2025-08-12 6:04 ` [PATCH 4/4 FIXED] " Icenowy Zheng 2025-08-12 9:17 ` Icenowy Zheng 2025-08-12 13:42 ` [PATCH 0/4] clk: thead: Misc changes to TH1520 clock driver Icenowy Zheng 2025-08-12 16:24 ` Drew Fustini 2025-08-12 19:50 ` Icenowy Zheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).