[PATCH v2 0/3] clk: thead: Changes to TH1520 clock driver for disp

[PATCH v2 0/3] clk: thead: Changes to TH1520 clock driver for disp

[Icenowy Zheng]

This patchset is my changes to the TH1520 clock driver, mainly for
supporting the display controller.

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 patch that used to be in v1 is dropped, makes this patchset
purely display-related.

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 (3):
  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

 drivers/clk/thead/clk-th1520-ap.c | 146 +++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 32 deletions(-)

-- 
2.50.1

[PATCH v2 1/3] clk: thead: add support for enabling/disabling PLLs

[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>
---
No changes in v2.

 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..0b5458af8c550 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

Re: [PATCH v2 1/3] clk: thead: add support for enabling/disabling PLLs

[Drew Fustini]

On Wed, Aug 13, 2025 at 03:27:00PM +0800, Icenowy Zheng wrote:
> 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).

Do you think there is a way in the future to allow disabling PLLs for
run-time power management?  I think it is more important right now to
get hardware peripherals working upstream like you are doing, but I was
curious if there is the potential to do something more granular in the
future.

> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
> No changes in v2.
> 
>  drivers/clk/thead/clk-th1520-ap.c | 38 +++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 5 deletions(-)

Reviewed-by: Drew Fustini <fustini@kernel.org>

[PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate

[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>
---
Changes in v2:
- Dropped round_rate() because of deprecation.
- Changed the logic of determine_rate() to early return if the divider
  could be changed.

 drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 0b5458af8c550..b220a8ed22607 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,56 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
+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)
+		return divider_determine_rate(hw, req, 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_determine_rate(hw, req, NULL, cd->div.width,
+					 cd->div.flags, val);
+}
+
+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, &reg_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 +291,8 @@ 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,
+	.set_rate	= ccu_div_set_rate,
+	.determine_rate = ccu_div_determine_rate,
 };
 
 static void ccu_pll_disable(struct clk_hw *hw)
@@ -784,6 +836,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 +844,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 +853,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 +861,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 +945,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

Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate

[Drew Fustini]

On Wed, Aug 13, 2025 at 03:27:01PM +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>
> ---
> Changes in v2:
> - Dropped round_rate() because of deprecation.
> - Changed the logic of determine_rate() to early return if the divider
>   could be changed.
> 
>  drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> index 0b5458af8c550..b220a8ed22607 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,56 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw,
>  	return rate;
>  }
>  
> +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)
> +		return divider_determine_rate(hw, req, 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_determine_rate(hw, req, NULL, cd->div.width,
> +					 cd->div.flags, val);
> +}
> +
> +static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long parent_rate)

This should line up with open parenthesis. Other than that nit:

Reviewed-by: Drew Fustini <fustini@kernel.org>

If no other issues arise on this series, then I can just fix it up when
applying.

Thanks,
Drew

Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate

[Troy Mitchell]

On Wed, Aug 13, 2025 at 03:27:01PM +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>
> ---
> Changes in v2:
> - Dropped round_rate() because of deprecation.
> - Changed the logic of determine_rate() to early return if the divider
>   could be changed.
> 
>  drivers/clk/thead/clk-th1520-ap.c | 64 ++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> index 0b5458af8c550..b220a8ed22607 100644
> --- a/drivers/clk/thead/clk-th1520-ap.c
> +++ b/drivers/clk/thead/clk-th1520-ap.c
> @@ -55,6 +55,7 @@ struct ccu_gate {
>  
...
> +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, &reg_val);
> +	curr_val = reg_val;
uh? remove this line.

                - Troy

> +	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 +291,8 @@ 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,
> +	.set_rate	= ccu_div_set_rate,
> +	.determine_rate = ccu_div_determine_rate,
>  };
>  
>  static void ccu_pll_disable(struct clk_hw *hw)
> @@ -784,6 +836,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 +844,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 +853,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 +861,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 +945,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

Re: [PATCH v2 2/3] clk: thead: support changing DPU pixel clock rate

[Icenowy Zheng]

在 2025-08-15星期五的 10:23 +0800，Troy Mitchell写道：
> On Wed, Aug 13, 2025 at 03:27:01PM +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>
> > ---
> > Changes in v2:
> > - Dropped round_rate() because of deprecation.
> > - Changed the logic of determine_rate() to early return if the
> > divider
> >   could be changed.
> > 
> >  drivers/clk/thead/clk-th1520-ap.c | 64
> > ++++++++++++++++++++++++++++---
> >  1 file changed, 59 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/clk/thead/clk-th1520-ap.c
> > b/drivers/clk/thead/clk-th1520-ap.c
> > index 0b5458af8c550..b220a8ed22607 100644
> > --- a/drivers/clk/thead/clk-th1520-ap.c
> > +++ b/drivers/clk/thead/clk-th1520-ap.c
> > @@ -55,6 +55,7 @@ struct ccu_gate {
> >  
> ...
> > +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, &reg_val);
> > +       curr_val = reg_val;
> uh? remove this line.
> 
>                 - Troy
> 
> > +       curr_val = curr_val >> cd->div.shift;

Ooooops, I am silly enough...
Will change this to `curr_val = reg_val >> cd->div.shift;` instead in
the next revision.

> > +       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 +291,8 @@ 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,
> > +       .set_rate       = ccu_div_set_rate,
> > +       .determine_rate = ccu_div_determine_rate,
> >  };
> >  
> >  static void ccu_pll_disable(struct clk_hw *hw)
> > @@ -784,6 +836,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 +844,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 +853,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 +861,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 +945,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

[PATCH v2 3/3] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL

[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>
---
No changes in v2 except for rebasing error fixes (which I sent as FIXED
patches in v1).

 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 b220a8ed22607..0224f2dde0f63 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -566,7 +566,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),
 	},
 };
 
@@ -588,7 +588,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),
 	},
 };
 
@@ -737,7 +737,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),
 	},
 };
 
@@ -768,7 +768,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),
 	},
 };
 
@@ -793,7 +793,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),
 	},
 };
 
@@ -818,7 +818,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),
 	},
 };
 
@@ -879,23 +879,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);
@@ -939,11 +939,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",
@@ -975,9 +975,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,
@@ -987,11 +987,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

Re: [PATCH v2 3/3] clk: thead: th1520-ap: set all AXI clocks to CLK_IS_CRITICAL

[Drew Fustini]

On Wed, Aug 13, 2025 at 03:27:02PM +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>
> ---
> No changes in v2 except for rebasing error fixes (which I sent as FIXED
> patches in v1).
> 
>  drivers/clk/thead/clk-th1520-ap.c | 44 +++++++++++++++----------------
>  1 file changed, 22 insertions(+), 22 deletions(-)

Reviewed-by: Drew Fustini <fustini@kernel.org>