[PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit

[PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

Gate the H616 PLLs with the output enable bit (27) instead of the enable
bit (31), which powers the PLL down, as the manual (3.3.3.6) recommends
and the A100, D1 and A523 drivers do. Power them up once at probe.

On an Orange Pi Zero2, powering pll-video0 down at 453 MHz crashes the
CPU doing the write.

Fixes: 88dde5e23da1 ("clk: sunxi-ng: Add support for the Allwinner H616 CCU")
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
No Cc: stable: the crash needs out-of-tree H616 HDMI. Tested on an
Orange Pi Zero2 (7.3.4 with HDMI patches): the failing mode set 40
times without an oops. Idle power not measured.

Developed with Claude (Anthropic); I reviewed it.
---
 drivers/clk/sunxi-ng/ccu-sun50i-h616.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
index 955c614830fa..da4156b39c2c 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
@@ -36,7 +36,7 @@
  */
 #define SUN50I_H616_PLL_CPUX_REG	0x000
 static struct ccu_mult pll_cpux_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.mult		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.common		= {
@@ -50,7 +50,7 @@ static struct ccu_mult pll_cpux_clk = {
 /* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
 #define SUN50I_H616_PLL_DDR0_REG	0x010
 static struct ccu_nkmp pll_ddr0_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -65,7 +65,7 @@ static struct ccu_nkmp pll_ddr0_clk = {
 
 #define SUN50I_H616_PLL_DDR1_REG	0x018
 static struct ccu_nkmp pll_ddr1_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -80,7 +80,7 @@ static struct ccu_nkmp pll_ddr1_clk = {
 
 #define SUN50I_H616_PLL_PERIPH0_REG	0x020
 static struct ccu_nkmp pll_periph0_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -97,7 +97,7 @@ static struct ccu_nkmp pll_periph0_clk = {
 
 #define SUN50I_H616_PLL_PERIPH1_REG	0x028
 static struct ccu_nkmp pll_periph1_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -114,7 +114,7 @@ static struct ccu_nkmp pll_periph1_clk = {
 
 #define SUN50I_H616_PLL_GPU_REG		0x030
 static struct ccu_nkmp pll_gpu_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -133,7 +133,7 @@ static struct ccu_nkmp pll_gpu_clk = {
  */
 #define SUN50I_H616_PLL_VIDEO0_REG	0x040
 static struct ccu_nm pll_video0_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -151,7 +151,7 @@ static struct ccu_nm pll_video0_clk = {
 
 #define SUN50I_H616_PLL_VIDEO1_REG	0x048
 static struct ccu_nm pll_video1_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -169,7 +169,7 @@ static struct ccu_nm pll_video1_clk = {
 
 #define SUN50I_H616_PLL_VIDEO2_REG	0x050
 static struct ccu_nm pll_video2_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -187,7 +187,7 @@ static struct ccu_nm pll_video2_clk = {
 
 #define SUN50I_H616_PLL_VE_REG		0x058
 static struct ccu_nkmp pll_ve_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -202,7 +202,7 @@ static struct ccu_nkmp pll_ve_clk = {
 
 #define SUN50I_H616_PLL_DE_REG		0x060
 static struct ccu_nkmp pll_de_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(1, 1), /* input divider */
@@ -229,7 +229,7 @@ static struct ccu_sdm_setting pll_audio_sdm_table[] = {
 
 #define SUN50I_H616_PLL_AUDIO_REG	0x078
 static struct ccu_nm pll_audio_hs_clk = {
-	.enable		= BIT(31),
+	.enable		= BIT(27),
 	.lock		= BIT(28),
 	.n		= _SUNXI_CCU_MULT_MIN(8, 8, 12),
 	.m		= _SUNXI_CCU_DIV(16, 6),
@@ -1214,6 +1214,13 @@ static int sun50i_h616_ccu_probe(struct platform_device *pdev)
 	val |= BIT(0);
 	writel(val, reg + SUN50I_H616_PLL_AUDIO_REG);
 
+	/* Keep the PLLs powered and gate them with bit 27 (manual 3.3.3.6). */
+	for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
+		val = readl(reg + pll_regs[i]);
+		val |= BIT(31);
+		writel(val, reg + pll_regs[i]);
+	}
+
 	/*
 	 * Set the input-divider for the gpu1 clock to 3, to reach a safe 400 MHz.
 	 */

---
base-commit: 280af24c860569000b375edda6436ba8e3ee4b3a
change-id: 20261114-h616-pll-output-gate-c9d3d4a409e9

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

Re: [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] clk: sunxi-ng: h616: sun50i_h616_ccu_probe() leaves BIT(27)
  forced on in loop
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] clk: sunxi-ng: h616: wrong GPU notifier registration order in
  sun50i_h616_ccu_probe() [1]
--

commit 733f137b19620733cea59e88a8f5b18ba7301d4a
Author: Arun Iyengar <aiyengar1990@gmail.com>

clk: sunxi-ng: h616: Gate the PLLs with their output enable bit

This commit changes the H616 PLL clocks to gate using their output enable
bit (BIT(27)) instead of the PLL enable bit (BIT(31)), and powers up the
PLLs in sun50i_h616_ccu_probe().

> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> index 955c614830fae..da4156b39c2cc 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c

[ ... ]

> @@ -1214,6 +1214,13 @@ static int sun50i_h616_ccu_probe(struct platform_device *pdev)
>  	val |= BIT(0);
>  	writel(val, reg + SUN50I_H616_PLL_AUDIO_REG);
>  
> +	/* Keep the PLLs powered and gate them with bit 27 (manual 3.3.3.6). */
> +	for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
> +		val = readl(reg + pll_regs[i]);
> +		val |= BIT(31);
> +		writel(val, reg + pll_regs[i]);
> +	}
> +
>  	/*
>  	 * Set the input-divider for the gpu1 clock to 3, to reach a safe 400 MHz.
>  	 */

[Severity: Medium]
Should the existing pll_regs loop earlier in sun50i_h616_ccu_probe() be
updated to set BIT(31) | BIT(29) instead of adding this second loop?

drivers/clk/sunxi-ng/ccu-sun50i-h616.c:sun50i_h616_ccu_probe() {
    ...
	/* Enable the lock bits and the output enable bits on all PLLs */
	for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
		val = readl(reg + pll_regs[i]);
		val |= BIT(29) | BIT(27);
		writel(val, reg + pll_regs[i]);
	}
    ...
}

Leaving that first loop setting BIT(27) unconditionally ungates the output
of all 12 PLLs before the video and audio PLL dividers are configured and
before disabled PLLs are powered up via BIT(31), and causes the clock
framework to see every PLL as already enabled at registration time.

[Severity: High]
This problem wasn't introduced by this patch, but later in
sun50i_h616_ccu_probe() the GPU clock mux notifier is registered before
the GPU PLL notifier [1]:

drivers/clk/sunxi-ng/ccu-sun50i-h616.c:sun50i_h616_ccu_probe() {
    ...
	/* Reparent GPU during GPU PLL rate changes */
	ccu_mux_notifier_register(pll_gpu_clk.common.hw.clk,
				  &sun50i_h616_gpu_nb);

	/* Re-lock the GPU PLL after any rate changes */
	ccu_pll_notifier_register(&sun50i_h616_pll_gpu_nb);
    ...
}

Because both notifiers use priority 0, they run in registration order.
On POST_RATE_CHANGE during GPU frequency scaling, ccu_mux_notifier_cb()
reparents gpu0_clk back to pll_gpu_clk before ccu_pll_notifier_cb()
re-enables and waits for the PLL to lock, driving the GPU from an
unlocked PLL.

[1] https://sashiko.dev/#/bug/linux-b48c9987-4bbf-4231-b1fc-24d8ccdf62d9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-h616-pll-output-gate-v1-1-4cb3fc4d6c48@gmail.com?part=1

Re: [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit

[Chen-Yu Tsai]

On Sun, Nov 15, 2026 at 4:45 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] clk: sunxi-ng: h616: sun50i_h616_ccu_probe() leaves BIT(27)
>   forced on in loop
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [High] clk: sunxi-ng: h616: wrong GPU notifier registration order in
>   sun50i_h616_ccu_probe() [1]
> --
>
> commit 733f137b19620733cea59e88a8f5b18ba7301d4a
> Author: Arun Iyengar <aiyengar1990@gmail.com>
>
> clk: sunxi-ng: h616: Gate the PLLs with their output enable bit
>
> This commit changes the H616 PLL clocks to gate using their output enable
> bit (BIT(27)) instead of the PLL enable bit (BIT(31)), and powers up the
> PLLs in sun50i_h616_ccu_probe().
>
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > index 955c614830fae..da4156b39c2cc 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h616.c
>
> [ ... ]
>
> > @@ -1214,6 +1214,13 @@ static int sun50i_h616_ccu_probe(struct platform_device *pdev)
> >       val |= BIT(0);
> >       writel(val, reg + SUN50I_H616_PLL_AUDIO_REG);
> >
> > +     /* Keep the PLLs powered and gate them with bit 27 (manual 3.3.3.6). */
> > +     for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
> > +             val = readl(reg + pll_regs[i]);
> > +             val |= BIT(31);
> > +             writel(val, reg + pll_regs[i]);
> > +     }
> > +
> >       /*
> >        * Set the input-divider for the gpu1 clock to 3, to reach a safe 400 MHz.
> >        */
>
> [Severity: Medium]
> Should the existing pll_regs loop earlier in sun50i_h616_ccu_probe() be
> updated to set BIT(31) | BIT(29) instead of adding this second loop?

I agree with Sashiko's assessment. Please tweak the existing loop instead
of adding a new one.


ChenYu


> drivers/clk/sunxi-ng/ccu-sun50i-h616.c:sun50i_h616_ccu_probe() {
>     ...
>         /* Enable the lock bits and the output enable bits on all PLLs */
>         for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
>                 val = readl(reg + pll_regs[i]);
>                 val |= BIT(29) | BIT(27);
>                 writel(val, reg + pll_regs[i]);
>         }
>     ...
> }
>
> Leaving that first loop setting BIT(27) unconditionally ungates the output
> of all 12 PLLs before the video and audio PLL dividers are configured and
> before disabled PLLs are powered up via BIT(31), and causes the clock
> framework to see every PLL as already enabled at registration time.
>
> [Severity: High]
> This problem wasn't introduced by this patch, but later in
> sun50i_h616_ccu_probe() the GPU clock mux notifier is registered before
> the GPU PLL notifier [1]:
>
> drivers/clk/sunxi-ng/ccu-sun50i-h616.c:sun50i_h616_ccu_probe() {
>     ...
>         /* Reparent GPU during GPU PLL rate changes */
>         ccu_mux_notifier_register(pll_gpu_clk.common.hw.clk,
>                                   &sun50i_h616_gpu_nb);
>
>         /* Re-lock the GPU PLL after any rate changes */
>         ccu_pll_notifier_register(&sun50i_h616_pll_gpu_nb);
>     ...
> }
>
> Because both notifiers use priority 0, they run in registration order.
> On POST_RATE_CHANGE during GPU frequency scaling, ccu_mux_notifier_cb()
> reparents gpu0_clk back to pll_gpu_clk before ccu_pll_notifier_cb()
> re-enables and waits for the PLL to lock, driving the GPU from an
> unlocked PLL.
>
> [1] https://sashiko.dev/#/bug/linux-b48c9987-4bbf-4231-b1fc-24d8ccdf62d9
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261115-h616-pll-output-gate-v1-1-4cb3fc4d6c48@gmail.com?part=1
>