* [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit
@ 2026-11-15 8:31 Arun Iyengar via B4 Relay
2026-11-15 8:45 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Arun Iyengar via B4 Relay @ 2026-11-15 8:31 UTC (permalink / raw)
To: Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Stephen Boyd,
Brian Masney, Jerome Brunet
Cc: Andre Przywara, linux-clk, linux-sunxi, linux-arm-kernel,
linux-kernel, Arun Iyengar
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>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit
2026-11-15 8:31 [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit Arun Iyengar via B4 Relay
@ 2026-11-15 8:45 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-15 8:45 UTC (permalink / raw)
To: Arun Iyengar; +Cc: linux-clk, linux-sunxi
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-15 8:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-15 8:31 [PATCH] clk: sunxi-ng: h616: Gate the PLLs with their output enable bit Arun Iyengar via B4 Relay
2026-11-15 8:45 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox