[PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Chuan Liu via B4 Relay]

This patch series consists of four topics involving the amlogic PLL
driver:
- Fix out-of-range PLL frequency setting.
- Improve the issue of PLL lock failures.
- Add handling for PLL lock failure.
- Optimize PLL enable timing.

For easier review and management, these are submitted as a single
patch series.

The PLL timing optimization changes were merged into our internal
repository quite some time ago and have been verified on a large
number of SoCs:
- Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
- Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.

Based on the upstream code base, I have performed functional testing
on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.

Additionally, stress testing using scripts was conducted on A5 and
A1, with over 40,000 and 50,000 iterations respectively, and no
abnormalities were observed. Below is a portion of the stress test
log (CLOCK_ALLOW_WRITE_DEBUGFS has been manually enabled):

- For A5:
  # echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
  # cnt=0
  # while true; do
  >     echo "------------ cnt=$cnt -----------"
  >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
  >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
  >     if [ "$en" != "1" ]; then
  >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
  >         break
  >     fi
  > 
  >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
  >     cnt=$((cnt + 1))
  >     echo -e "sleep time: 1 s."
  >     sleep 1
  > done &
  # ------------ cnt=0 -----------
  sleep time: 1 s.
  ------------ cnt=1 -----------
  sleep time: 1 s.
  ------------ cnt=2 -----------
  sleep time: 1 s.
  ...
  ------------ cnt=42076 -----------
  sleep time: 1 s.

- For A1:
  # echo 983040000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
  # cnt=0
  # while true; do
  >     echo "------------ cnt=$cnt -----------"
  >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
  >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
  >     if [ "$en" != "1" ]; then
  >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
  >         break
  >     fi
  > 
  >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
  >     cnt=$((cnt + 1))
  >     echo -e "sleep time: 1 s."
  >     sleep 1
  > done &
  # ------------ cnt=0 -----------
  sleep time: 1 s.
  ------------ cnt=1 -----------
  sleep time: 1 s.
  ------------ cnt=2 -----------
  sleep time: 1 s.
  ...
  ------------ cnt=55051 -----------
  sleep time: 1 s.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
Changes in v3:
- Fix some formatting issues.
- Move the 20 us delay after reset into the corresponding if
condition (no delay is needed if there is no reset).
- Move the code that releases rst back to execute before current_en.
- Remove the patch that changes the active level of l_detect.
- Link to v2: https://lore.kernel.org/r/20251030-optimize_pll_driver-v2-0-37273f5b25ab@amlogic.com

Changes in v2:
- Modify the judgment condition of 'm' out of range.
- Split the PLL timing optimization patch to make it easier to review.
- Link to v1: https://lore.kernel.org/r/20251022-optimize_pll_driver-v1-0-a275722fb6f4@amlogic.com

---
Chuan Liu (4):
      clk: amlogic: Fix out-of-range PLL frequency setting
      clk: amlogic: Improve the issue of PLL lock failures
      clk: amlogic: Add handling for PLL lock failure
      clk: amlogic: Optimize PLL enable timing

 drivers/clk/meson/clk-pll.c | 64 +++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 25 deletions(-)
---
base-commit: 01f3a6d1d59b8e25a6de243b0d73075cf0415eaf
change-id: 20251020-optimize_pll_driver-7bef91876c41

Best regards,
-- 
Chuan Liu <chuan.liu@amlogic.com>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 1/4] clk: amlogic: Fix out-of-range PLL frequency setting

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

If the calculated 'm' falls into the range:

    pll->range->max < m < (1 << pll->m.width)

Here an incorrect 'm' value could be obtained, so an additional
condition is added to ensure that the calculated 'm' stays within a
valid range.

Fixes: 8eed1db1adec6 ("clk: meson: pll: update driver for the g12a")
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 1ea6579a760f..629f6af18ea1 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -191,7 +191,7 @@ static int meson_clk_get_pll_range_index(unsigned long rate,
 	*m = meson_clk_get_pll_range_m(rate, parent_rate, *n, pll);
 
 	/* the pre-divider gives a multiplier too big - stop */
-	if (*m >= (1 << pll->m.width))
+	if (*m > pll->range->max || *m >= (1 << pll->m.width))
 		return -EINVAL;
 
 	return 0;

-- 
2.42.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 2/4] clk: amlogic: Improve the issue of PLL lock failures

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

Due to factors such as temperature and process variations, the
internal circuits of the PLL may require a longer time to reach a
steady state, which can result in occasional lock failures on some
SoCs under low-temperature conditions.

After enabling the PLL and releasing its reset, a 20 us delay is
added at each step to provide enough time for the internal PLL
circuit to stabilize, thus reducing the probability of PLL lock
failure.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 629f6af18ea1..70c8c7078046 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -369,10 +369,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
 	/* Enable the pll */
 	meson_parm_write(clk->map, &pll->en, 1);
 
+	/* Wait for Bandgap and LDO to power up and stabilize */
+	udelay(20);
+
 	/* Take the pll out reset */
-	if (MESON_PARM_APPLICABLE(&pll->rst))
+	if (MESON_PARM_APPLICABLE(&pll->rst)) {
 		meson_parm_write(clk->map, &pll->rst, 0);
 
+		/* Wait for PLL loop stabilization */
+		udelay(20);
+	}
+
 	/*
 	 * Compared with the previous SoCs, self-adaption current module
 	 * is newly added for A1, keep the new power-on sequence to enable the

-- 
2.42.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 2/4] clk: amlogic: Improve the issue of PLL lock failures

[Martin Blumenstingl]

On Fri, Oct 31, 2025 at 9:10 AM Chuan Liu via B4 Relay
<devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> Due to factors such as temperature and process variations, the
> internal circuits of the PLL may require a longer time to reach a
> steady state, which can result in occasional lock failures on some
> SoCs under low-temperature conditions.
>
> After enabling the PLL and releasing its reset, a 20 us delay is
> added at each step to provide enough time for the internal PLL
> circuit to stabilize, thus reducing the probability of PLL lock
> failure.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> # Odroid-C1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 3/4] clk: amlogic: Add handling for PLL lock failure

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

If the PLL fails to lock, it should be disabled, This makes the logic
more complete, and also helps save unnecessary power consumption when
the PLL is malfunctioning.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 70c8c7078046..cdb39a723bd0 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -353,6 +353,23 @@ static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
 	return -EIO;
 }
 
+static void meson_clk_pll_disable(struct clk_hw *hw)
+{
+	struct clk_regmap *clk = to_clk_regmap(hw);
+	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
+
+	/* Put the pll is in reset */
+	if (MESON_PARM_APPLICABLE(&pll->rst))
+		meson_parm_write(clk->map, &pll->rst, 1);
+
+	/* Disable the pll */
+	meson_parm_write(clk->map, &pll->en, 0);
+
+	/* Disable PLL internal self-adaption current module */
+	if (MESON_PARM_APPLICABLE(&pll->current_en))
+		meson_parm_write(clk->map, &pll->current_en, 0);
+}
+
 static int meson_clk_pll_enable(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
@@ -399,29 +416,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
 		meson_parm_write(clk->map, &pll->l_detect, 0);
 	}
 
-	if (meson_clk_pll_wait_lock(hw))
+	if (meson_clk_pll_wait_lock(hw)) {
+		/* disable PLL when PLL lock failed. */
+		meson_clk_pll_disable(hw);
+		pr_warn("%s: PLL lock failed!\n", clk_hw_get_name(hw));
+
 		return -EIO;
+	}
 
 	return 0;
 }
 
-static void meson_clk_pll_disable(struct clk_hw *hw)
-{
-	struct clk_regmap *clk = to_clk_regmap(hw);
-	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
-
-	/* Put the pll is in reset */
-	if (MESON_PARM_APPLICABLE(&pll->rst))
-		meson_parm_write(clk->map, &pll->rst, 1);
-
-	/* Disable the pll */
-	meson_parm_write(clk->map, &pll->en, 0);
-
-	/* Disable PLL internal self-adaption current module */
-	if (MESON_PARM_APPLICABLE(&pll->current_en))
-		meson_parm_write(clk->map, &pll->current_en, 0);
-}
-
 static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 				  unsigned long parent_rate)
 {

-- 
2.42.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 4/4] clk: amlogic: Optimize PLL enable timing

[Chuan Liu via B4 Relay]

From: Chuan Liu <chuan.liu@amlogic.com>

l_detect controls the enablement of the PLL lock detection module.
It should remain disabled while the internal PLL circuits are
reaching a steady state; otherwise, the lock signal may be falsely
triggered high.

A 20 us delay has been added before enabling the PLL, so there’s no
need for an additional 10 us delay before enabling current_en.

Currently, only A1 supports both l_detect and current_en, so this
patch will only affect A1.

Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index cdb39a723bd0..c9b941adf688 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -383,6 +383,10 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
 	if (MESON_PARM_APPLICABLE(&pll->rst))
 		meson_parm_write(clk->map, &pll->rst, 1);
 
+	/* Disable the PLL lock-detect module */
+	if (MESON_PARM_APPLICABLE(&pll->l_detect))
+		meson_parm_write(clk->map, &pll->l_detect, 1);
+
 	/* Enable the pll */
 	meson_parm_write(clk->map, &pll->en, 1);
 
@@ -401,20 +405,18 @@ 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 20us
 	 * 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);
 	}
 
-	if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
-		meson_parm_write(clk->map, &pll->l_detect, 1);
+	/* Enable the lock-detect module */
+	if (MESON_PARM_APPLICABLE(&pll->l_detect))
 		meson_parm_write(clk->map, &pll->l_detect, 0);
-	}
 
 	if (meson_clk_pll_wait_lock(hw)) {
 		/* disable PLL when PLL lock failed. */

-- 
2.42.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Jerome Brunet]

On Fri 31 Oct 2025 at 16:10, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:

> This patch series consists of four topics involving the amlogic PLL
> driver:
> - Fix out-of-range PLL frequency setting.
> - Improve the issue of PLL lock failures.
> - Add handling for PLL lock failure.
> - Optimize PLL enable timing.
>
> For easier review and management, these are submitted as a single
> patch series.
>
> The PLL timing optimization changes were merged into our internal
> repository quite some time ago and have been verified on a large
> number of SoCs:
> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>
> Based on the upstream code base, I have performed functional testing
> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
>
> Additionally, stress testing using scripts was conducted on A5 and
> A1, with over 40,000 and 50,000 iterations respectively, and no
> abnormalities were observed. Below is a portion of the stress test
> log (CLOCK_ALLOW_WRITE_DEBUGFS has been manually enabled):

Okay, this little game has been going on long enough.

You've posted v2 24h hours ago
You've got feedback within hours
There was still a 1 question pending
The rest of community had no chance to review.

and yet, here the v3 already ! still with bearing pr_warn().

Chuan, the community is not dedicated to reviewing your submission.
Time and time again you ignore the feedback provided in reviews and the
documentation. I've had enough of your sloppy submission.

I will not review or apply anything from you in this cycle.

You have been warned multiple times. Every time you ignore a review,
you'll get ignored in return. This is how it will be from now on.

>
> - For A5:
>   # echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
>   # cnt=0
>   # while true; do
>   >     echo "------------ cnt=$cnt -----------"
>   >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>   >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
>   >     if [ "$en" != "1" ]; then
>   >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
>   >         break
>   >     fi
>   > 
>   >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>   >     cnt=$((cnt + 1))
>   >     echo -e "sleep time: 1 s."
>   >     sleep 1
>   > done &
>   # ------------ cnt=0 -----------
>   sleep time: 1 s.
>   ------------ cnt=1 -----------
>   sleep time: 1 s.
>   ------------ cnt=2 -----------
>   sleep time: 1 s.
>   ...
>   ------------ cnt=42076 -----------
>   sleep time: 1 s.
>
> - For A1:
>   # echo 983040000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
>   # cnt=0
>   # while true; do
>   >     echo "------------ cnt=$cnt -----------"
>   >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>   >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
>   >     if [ "$en" != "1" ]; then
>   >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
>   >         break
>   >     fi
>   > 
>   >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>   >     cnt=$((cnt + 1))
>   >     echo -e "sleep time: 1 s."
>   >     sleep 1
>   > done &
>   # ------------ cnt=0 -----------
>   sleep time: 1 s.
>   ------------ cnt=1 -----------
>   sleep time: 1 s.
>   ------------ cnt=2 -----------
>   sleep time: 1 s.
>   ...
>   ------------ cnt=55051 -----------
>   sleep time: 1 s.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
> Changes in v3:
> - Fix some formatting issues.
> - Move the 20 us delay after reset into the corresponding if
> condition (no delay is needed if there is no reset).
> - Move the code that releases rst back to execute before current_en.
> - Remove the patch that changes the active level of l_detect.
> - Link to v2: https://lore.kernel.org/r/20251030-optimize_pll_driver-v2-0-37273f5b25ab@amlogic.com
>
> Changes in v2:
> - Modify the judgment condition of 'm' out of range.
> - Split the PLL timing optimization patch to make it easier to review.
> - Link to v1: https://lore.kernel.org/r/20251022-optimize_pll_driver-v1-0-a275722fb6f4@amlogic.com
>
> ---
> Chuan Liu (4):
>       clk: amlogic: Fix out-of-range PLL frequency setting
>       clk: amlogic: Improve the issue of PLL lock failures
>       clk: amlogic: Add handling for PLL lock failure
>       clk: amlogic: Optimize PLL enable timing
>
>  drivers/clk/meson/clk-pll.c | 64 +++++++++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 25 deletions(-)
> ---
> base-commit: 01f3a6d1d59b8e25a6de243b0d73075cf0415eaf
> change-id: 20251020-optimize_pll_driver-7bef91876c41
>
> Best regards,

-- 
Jerome

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Chuan Liu]

Hi Jerome,

On 10/31/2025 5:04 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> On Fri 31 Oct 2025 at 16:10, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
> 
>> This patch series consists of four topics involving the amlogic PLL
>> driver:
>> - Fix out-of-range PLL frequency setting.
>> - Improve the issue of PLL lock failures.
>> - Add handling for PLL lock failure.
>> - Optimize PLL enable timing.
>>
>> For easier review and management, these are submitted as a single
>> patch series.
>>
>> The PLL timing optimization changes were merged into our internal
>> repository quite some time ago and have been verified on a large
>> number of SoCs:
>> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
>> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>>
>> Based on the upstream code base, I have performed functional testing
>> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
>>
>> Additionally, stress testing using scripts was conducted on A5 and
>> A1, with over 40,000 and 50,000 iterations respectively, and no
>> abnormalities were observed. Below is a portion of the stress test
>> log (CLOCK_ALLOW_WRITE_DEBUGFS has been manually enabled):
> 
> Okay, this little game has been going on long enough.
> 
> You've posted v2 24h hours ago
> You've got feedback within hours
> There was still a 1 question pending
> The rest of community had no chance to review.
> 

There might be a serious misunderstanding here.

In recent years, we've mainly been maintaining our code in our
internal repository, which has led to some differences between our
internal codebase and the upstream version. The patches that account
for these differences are already queued for submission, and several
SoCs are also waiting in line to be submitted. As a result, quite a
few patches have piled up, waiting to go upstream.

Previously, I had been waiting for your clock driver restructuring
patches to be ready (which have recently been merged), so for almost
a year, we haven't made much progress on clock driver–related
upstreaming.

Other drivers that depend on the clock driver have also been blocked.
This situation has been very stressful for me. I've been doing my
best to update versions as soon as possible in response to review
feedback.

> and yet, here the v3 already ! still with bearing pr_warn().
> 

Regarding pr_warn(), I explained in a previous email why I didn't
choose to use pr_warn_once(). The quick iteration of versions wasn't
an intentional disregard of your suggestions.

It's just that the upstream submission process has been severely
jammed. I admit I've been anxious, because at the previous pace, our
chips have already been in mass production for quite some time while
our clock driver still hasn't been submitted upstream, which puts us
in a difficult position, so please understand.

> Chuan, the community is not dedicated to reviewing your submission.
> Time and time again you ignore the feedback provided in reviews and the
> documentation. I've had enough of your sloppy submission.
> 
> I will not review or apply anything from you in this cycle.
> 
> You have been warned multiple times. Every time you ignore a review,
> you'll get ignored in return. This is how it will be from now on.
> 

Seeing your message was really disheartening. I want to clarify that
I've always been very grateful to and respectful of you, as well as
Neil, Martin, and others. Throughout the upstreaming process, your
feedback has been extremely helpful.

I've always taken your reviews seriously and communicate frendly. I
truly never meant to ignore your comments, please don't misunderstand.

>>
>> - For A5:
>>    # echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
>>    # cnt=0
>>    # while true; do
>>    >     echo "------------ cnt=$cnt -----------"
>>    >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>>    >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
>>    >     if [ "$en" != "1" ]; then
>>    >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
>>    >         break
>>    >     fi
>>    >
>>    >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>>    >     cnt=$((cnt + 1))
>>    >     echo -e "sleep time: 1 s."
>>    >     sleep 1
>>    > done &
>>    # ------------ cnt=0 -----------
>>    sleep time: 1 s.
>>    ------------ cnt=1 -----------
>>    sleep time: 1 s.
>>    ------------ cnt=2 -----------
>>    sleep time: 1 s.
>>    ...
>>    ------------ cnt=42076 -----------
>>    sleep time: 1 s.
>>
>> - For A1:
>>    # echo 983040000 > /sys/kernel/debug/clk/hifi_pll/clk_rate
>>    # cnt=0
>>    # while true; do
>>    >     echo "------------ cnt=$cnt -----------"
>>    >     echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>>    >     en=$(cat /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable 2>/dev/null)
>>    >     if [ "$en" != "1" ]; then
>>    >         echo "[ERROR] PLL enable test failed! (clk_prepare_enable=$en)"
>>    >         break
>>    >     fi
>>    >
>>    >     echo 0 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable
>>    >     cnt=$((cnt + 1))
>>    >     echo -e "sleep time: 1 s."
>>    >     sleep 1
>>    > done &
>>    # ------------ cnt=0 -----------
>>    sleep time: 1 s.
>>    ------------ cnt=1 -----------
>>    sleep time: 1 s.
>>    ------------ cnt=2 -----------
>>    sleep time: 1 s.
>>    ...
>>    ------------ cnt=55051 -----------
>>    sleep time: 1 s.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>> Changes in v3:
>> - Fix some formatting issues.
>> - Move the 20 us delay after reset into the corresponding if
>> condition (no delay is needed if there is no reset).
>> - Move the code that releases rst back to execute before current_en.
>> - Remove the patch that changes the active level of l_detect.
>> - Link to v2: https://lore.kernel.org/r/20251030-optimize_pll_driver-v2-0-37273f5b25ab@amlogic.com
>>
>> Changes in v2:
>> - Modify the judgment condition of 'm' out of range.
>> - Split the PLL timing optimization patch to make it easier to review.
>> - Link to v1: https://lore.kernel.org/r/20251022-optimize_pll_driver-v1-0-a275722fb6f4@amlogic.com
>>
>> ---
>> Chuan Liu (4):
>>        clk: amlogic: Fix out-of-range PLL frequency setting
>>        clk: amlogic: Improve the issue of PLL lock failures
>>        clk: amlogic: Add handling for PLL lock failure
>>        clk: amlogic: Optimize PLL enable timing
>>
>>   drivers/clk/meson/clk-pll.c | 64 +++++++++++++++++++++++++++------------------
>>   1 file changed, 39 insertions(+), 25 deletions(-)
>> ---
>> base-commit: 01f3a6d1d59b8e25a6de243b0d73075cf0415eaf
>> change-id: 20251020-optimize_pll_driver-7bef91876c41
>>
>> Best regards,
> 
> --
> Jerome


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Jerome Brunet]

On Fri 31 Oct 2025 at 23:09, Chuan Liu <chuan.liu@amlogic.com> wrote:

> Hi Jerome,
>
> On 10/31/2025 5:04 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Fri 31 Oct 2025 at 16:10, Chuan Liu via B4 Relay
>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>> 
>>> This patch series consists of four topics involving the amlogic PLL
>>> driver:
>>> - Fix out-of-range PLL frequency setting.
>>> - Improve the issue of PLL lock failures.
>>> - Add handling for PLL lock failure.
>>> - Optimize PLL enable timing.
>>>
>>> For easier review and management, these are submitted as a single
>>> patch series.
>>>
>>> The PLL timing optimization changes were merged into our internal
>>> repository quite some time ago and have been verified on a large
>>> number of SoCs:
>>> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
>>> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>>>
>>> Based on the upstream code base, I have performed functional testing
>>> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
>>>
>>> Additionally, stress testing using scripts was conducted on A5 and
>>> A1, with over 40,000 and 50,000 iterations respectively, and no
>>> abnormalities were observed. Below is a portion of the stress test
>>> log (CLOCK_ALLOW_WRITE_DEBUGFS has been manually enabled):
>> Okay, this little game has been going on long enough.
>> You've posted v2 24h hours ago
>> You've got feedback within hours
>> There was still a 1 question pending
>> The rest of community had no chance to review.
>> 
>
> There might be a serious misunderstanding here.
>
> In recent years, we've mainly been maintaining our code in our
> internal repository, which has led to some differences between our
> internal codebase and the upstream version. The patches that account
> for these differences are already queued for submission, and several
> SoCs are also waiting in line to be submitted. As a result, quite a
> few patches have piled up, waiting to go upstream.
>
> Previously, I had been waiting for your clock driver restructuring
> patches to be ready (which have recently been merged), so for almost
> a year, we haven't made much progress on clock driver–related
> upstreaming.

Ohoh now you are just teasing me !

That work was made necessary because of all the copy/paste Amlogic was
submitting. Despite many requests, this was never addressed so I had
to step in.

If you want things to go faster, then *really* pay attention to the review
you are getting, do not ask question to ignore the answers and stop
making people repeat themselves over and over.


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Chuan Liu]

On 11/1/2025 12:27 AM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> On Fri 31 Oct 2025 at 23:09, Chuan Liu <chuan.liu@amlogic.com> wrote:
> 
>> Hi Jerome,
>>
>> On 10/31/2025 5:04 PM, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Fri 31 Oct 2025 at 16:10, Chuan Liu via B4 Relay
>>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>>
>>>> This patch series consists of four topics involving the amlogic PLL
>>>> driver:
>>>> - Fix out-of-range PLL frequency setting.
>>>> - Improve the issue of PLL lock failures.
>>>> - Add handling for PLL lock failure.
>>>> - Optimize PLL enable timing.
>>>>
>>>> For easier review and management, these are submitted as a single
>>>> patch series.
>>>>
>>>> The PLL timing optimization changes were merged into our internal
>>>> repository quite some time ago and have been verified on a large
>>>> number of SoCs:
>>>> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
>>>> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>>>>
>>>> Based on the upstream code base, I have performed functional testing
>>>> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
>>>>
>>>> Additionally, stress testing using scripts was conducted on A5 and
>>>> A1, with over 40,000 and 50,000 iterations respectively, and no
>>>> abnormalities were observed. Below is a portion of the stress test
>>>> log (CLOCK_ALLOW_WRITE_DEBUGFS has been manually enabled):
>>> Okay, this little game has been going on long enough.
>>> You've posted v2 24h hours ago
>>> You've got feedback within hours
>>> There was still a 1 question pending
>>> The rest of community had no chance to review.
>>>
>>
>> There might be a serious misunderstanding here.
>>
>> In recent years, we've mainly been maintaining our code in our
>> internal repository, which has led to some differences between our
>> internal codebase and the upstream version. The patches that account
>> for these differences are already queued for submission, and several
>> SoCs are also waiting in line to be submitted. As a result, quite a
>> few patches have piled up, waiting to go upstream.
>>
>> Previously, I had been waiting for your clock driver restructuring
>> patches to be ready (which have recently been merged), so for almost
>> a year, we haven't made much progress on clock driver–related
>> upstreaming.
> 
> Ohoh now you are just teasing me !

I must clarify that there seems to have been a misunderstanding. We
are increasingly focusing on the development of upstream ecosystem
and actively working to make it more comprehensive.

> 
> That work was made necessary because of all the copy/paste Amlogic was
> submitting. Despite many requests, this was never addressed so I had
> to step in.

I fully understand your perspective, especially given the challenging
process of upstreaming our clock driver due to historical reasons. I
have always maintained a friendly attitude and kept communication
open with you. My intention here is solely to provide context and
avoid any further misunderstandings.

First, I acknowledge your approach: these optimizations are
beneficial.

Over the years, we have also recognized these issues and have
implemented a series of improvements in our internal repository, such
as:
   - Reducing driver code volume and improving reusability,
   - Standardizing naming conventions (e.g., clock names, clock IDs)
   - Optimizing image size
   - Enhancing driver execution efficiency
   - Ensuring compatibility between ARM and ARM64 architectures

To address the challenges encountered, we have also contributed to
optimizing SoC hardware design. Improvements have already been
observed in newer SoCs, and we plan to gradually submit the related
drivers to the community in the future.

> 
> If you want things to go faster, then *really* pay attention to the review
> you are getting, do not ask question to ignore the answers and stop
> making people repeat themselves over and over.
> 

Thank you and the other reviewers for taking the time to review my
patch.

There may have been instances where I misunderstood your comments,
but I never intentionally ignored your feedback. Moving forward, I
will actively submit our company's drivers to the upstream, and I
certainly hope not to be perceived as uncooperative from the start.



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Martin Blumenstingl]

On Fri, Oct 31, 2025 at 9:10 AM Chuan Liu via B4 Relay
<devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
> This patch series consists of four topics involving the amlogic PLL
> driver:
> - Fix out-of-range PLL frequency setting.
> - Improve the issue of PLL lock failures.
> - Add handling for PLL lock failure.
> - Optimize PLL enable timing.
>
> For easier review and management, these are submitted as a single
> patch series.
>
> The PLL timing optimization changes were merged into our internal
> repository quite some time ago and have been verified on a large
> number of SoCs:
> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>
> Based on the upstream code base, I have performed functional testing
> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
In the past I had most problems with Meson8/8b/8m2 CPU clock scaling
(via sys_pll).
So I tested this series locally using the following shell script on an
Odroid-C1 (Meson8b):
#!/bin/bash

CPUFREQ="/sys/bus/cpu/devices/cpu0/cpufreq"

echo "userspace" > "${CPUFREQ}/scaling_governor"

while read -ra LINE
do
    for (( i=0; i<${#LINE[*]}; i++ ))
    do
        for (( j=0; j<${#LINE[*]}; j++ ))
        do
            if [ $i != $j ]
            then
                echo "${LINE[i]} -> ${LINE[j]}"
                echo "${LINE[i]}" > "${CPUFREQ}/scaling_setspeed"
                sleep 1s
                echo "${LINE[j]}" > "${CPUFREQ}/scaling_setspeed"
                sleep 1s
            fi
        done
    done
done < "${CPUFREQ}/scaling_available_frequencies"

This has been running in a loop for two hours (at an ambient
temperature of ~13°C) and I haven't observed any problem.
Since most patches are a no-op for my case I'll separately reply to
patch #2 with my Tested-by (as that's what I've been effectively
testing).


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver

[Chuan Liu]

Hi Martin,

Thank you for providing the test results. I previously found a
Meson8b board and wanted to try a stress test, but it threw errors
when running in the kernel (maybe because my bootloader version is
too old)... Anyway, thank you!

On 11/9/2025 5:04 AM, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> On Fri, Oct 31, 2025 at 9:10 AM Chuan Liu via B4 Relay
> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>
>> This patch series consists of four topics involving the amlogic PLL
>> driver:
>> - Fix out-of-range PLL frequency setting.
>> - Improve the issue of PLL lock failures.
>> - Add handling for PLL lock failure.
>> - Optimize PLL enable timing.
>>
>> For easier review and management, these are submitted as a single
>> patch series.
>>
>> The PLL timing optimization changes were merged into our internal
>> repository quite some time ago and have been verified on a large
>> number of SoCs:
>> - Already supported upstream: G12A, G12B, SM1, S4, A1, C3.
>> - Planned for upstream support: T7, A5, A4, S7, S7D, S6, etc.
>>
>> Based on the upstream code base, I have performed functional testing
>> on G12A, A1, A5, A4, T7, S7, S7D, and S6, all of which passed.
> In the past I had most problems with Meson8/8b/8m2 CPU clock scaling
> (via sys_pll).
> So I tested this series locally using the following shell script on an
> Odroid-C1 (Meson8b):
> #!/bin/bash
> 
> CPUFREQ="/sys/bus/cpu/devices/cpu0/cpufreq"
> 
> echo "userspace" > "${CPUFREQ}/scaling_governor"
> 
> while read -ra LINE
> do
>      for (( i=0; i<${#LINE[*]}; i++ ))
>      do
>          for (( j=0; j<${#LINE[*]}; j++ ))
>          do
>              if [ $i != $j ]
>              then
>                  echo "${LINE[i]} -> ${LINE[j]}"
>                  echo "${LINE[i]}" > "${CPUFREQ}/scaling_setspeed"
>                  sleep 1s
>                  echo "${LINE[j]}" > "${CPUFREQ}/scaling_setspeed"
>                  sleep 1s
>              fi
>          done
>      done
> done < "${CPUFREQ}/scaling_available_frequencies"
> 
> This has been running in a loop for two hours (at an ambient
> temperature of ~13°C) and I haven't observed any problem.
> Since most patches are a no-op for my case I'll separately reply to
> patch #2 with my Tested-by (as that's what I've been effectively
> testing).
> 
> 
> Best regards,
> Martin



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic