[PATCH] clk: meson: Fix PLL enable timing

[PATCH] clk: meson: Fix PLL enable timing

[Jian Hu via B4 Relay]

From: Jian Hu <jian.hu@amlogic.com>

The hardware requires a 50us delay after setting the PLL enable
bit to meet its timing requirements.

Without this delay, GP0 and HIFI PLLs on Amlogic T7, SM1 and C3
may fail to lock under stress testing.

For example, enabling the GP0 PLL at 1512MHz may lose lock and
result in an output I/O error:

echo 1512000000 > /sys/kernel/debug/clk/gp0_pll/clk_rate
echo 1 > /sys/kernel/debug/clk/gp0_pll/clk_prepare_enable
sh: write error: I/O error

Add the required 50us delay after enabling the PLL.

Testing on GXL shows that the PLL works correctly without the
additional delay and continues to lock successfully after the
50us delay is added.

On A1, remove the redundant 10us delay before enabling the
self-adaption current module, as the new PLL enable delay already
covers this timing requirement.

Tested on: T7, C3, S4, AXG, GXL, SM1 and A1.

Fixes: e40c7e3cda07 ("clk: meson: clk-pll: add enable bit")
Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 1ea6579a760f..77ea2960120f 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -368,6 +368,7 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
 
 	/* Enable the pll */
 	meson_parm_write(clk->map, &pll->en, 1);
+	udelay(50);
 
 	/* Take the pll out reset */
 	if (MESON_PARM_APPLICABLE(&pll->rst))
@@ -377,12 +378,11 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
 	 * Compared with the previous SoCs, self-adaption current module
 	 * is newly added for A1, keep the new power-on sequence to enable the
 	 * PLL. The sequence is:
-	 * 1. enable the pll, delay for 10us
+	 * 1. enable the pll, delay for 50us
 	 * 2. enable the pll self-adaption current module, delay for 40us
 	 * 3. enable the lock detect module
 	 */
 	if (MESON_PARM_APPLICABLE(&pll->current_en)) {
-		udelay(10);
 		meson_parm_write(clk->map, &pll->current_en, 1);
 		udelay(40);
 	}

---
base-commit: 363f97a1ce635b86be7fd263d56a040fbd87bdbb
change-id: 20261027-fix_enable_timing-842bfa84d4d4

Best regards,
-- 
Jian Hu <jian.hu@amlogic.com>

Re: [PATCH] clk: meson: Fix PLL enable timing

[Jerome Brunet]

> The hardware requires a 50us delay after setting the PLL enable
> bit to meet its timing requirements.
> 
> Without this delay, GP0 and HIFI PLLs on Amlogic T7, SM1 and C3
> may fail to lock under stress testing.
> 
> For example, enabling the GP0 PLL at 1512MHz may lose lock and
> result in an output I/O error:
> 
> echo 1512000000 > /sys/kernel/debug/clk/gp0_pll/clk_rate
> echo 1 > /sys/kernel/debug/clk/gp0_pll/clk_prepare_enable
> sh: write error: I/O error

This is something for the cover letter or bellow the --- for a single change series.

> 
> Add the required 50us delay after enabling the PLL.
> 
> Testing on GXL shows that the PLL works correctly without the
> additional delay and continues to lock successfully after the
> 50us delay is added.
> 
> On A1, remove the redundant 10us delay before enabling the
> self-adaption current module, as the new PLL enable delay already
> covers this timing requirement.
> 
> Tested on: T7, C3, S4, AXG, GXL, SM1 and A1.
> 
> Fixes: e40c7e3cda07 ("clk: meson: clk-pll: add enable bit")

This is the commit which introduced the enable bit and it is not really
what you are fixing here

A more likely candidate is b6ec400aa153 which introduced the delay
sequence you are modifying here.

I've got this other patch addressing PLL delays in my queue:

https://lore.kernel.org/all/20251031-optimize_pll_driver-v3-2-92f3b2f36a83@amlogic.com/

Which one is it ? both ?

Please consolidate and resubmit.

-- 
Jerome

Re: [PATCH] clk: meson: Fix PLL enable timing

[Jian Hu]

Hi Jerome,


Thanks for your review.

On 10/30/2026 5:04 PM, Jerome Brunet wrote:
>
>> The hardware requires a 50us delay after setting the PLL enable
>> bit to meet its timing requirements.
>>
>> Without this delay, GP0 and HIFI PLLs on Amlogic T7, SM1 and C3
>> may fail to lock under stress testing.
>>
>> For example, enabling the GP0 PLL at 1512MHz may lose lock and
>> result in an output I/O error:
>>
>> echo 1512000000 > /sys/kernel/debug/clk/gp0_pll/clk_rate
>> echo 1 > /sys/kernel/debug/clk/gp0_pll/clk_prepare_enable
>> sh: write error: I/O error
> This is something for the cover letter or bellow the --- for a single change series.


Ok, I will move the example bellow the ---.

>> Add the required 50us delay after enabling the PLL.
>>
>> Testing on GXL shows that the PLL works correctly without the
>> additional delay and continues to lock successfully after the
>> 50us delay is added.
>>
>> On A1, remove the redundant 10us delay before enabling the
>> self-adaption current module, as the new PLL enable delay already
>> covers this timing requirement.
>>
>> Tested on: T7, C3, S4, AXG, GXL, SM1 and A1.
>>
>> Fixes: e40c7e3cda07 ("clk: meson: clk-pll: add enable bit")
> This is the commit which introduced the enable bit and it is not really
> what you are fixing here
>
> A more likely candidate is b6ec400aa153 which introduced the delay
> sequence you are modifying here.


Ok, I will change the fix tag to b6ec400aa153.

> I've got this other patch addressing PLL delays in my queue:
>
> https://lore.kernel.org/all/20251031-optimize_pll_driver-v3-2-92f3b2f36a83@amlogic.com/
>
> Which one is it ? both ?


Yes, the are both addressing the PLL delays.

>
> Please consolidate and resubmit.


I will combine these two commits into one in v2.

>
> --
> Jerome

--

Jian