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

Chuan Liu via B4 Relay posted 4 patches 1 month, 1 week ago
drivers/clk/meson/clk-pll.c | 64 +++++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 25 deletions(-)
[PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Chuan Liu via B4 Relay 1 month, 1 week ago
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>
Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Martin Blumenstingl 1 month ago
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
Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Chuan Liu 1 month ago
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


Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Jerome Brunet 1 month, 1 week ago
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
Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Chuan Liu 1 month, 1 week ago
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

Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Jerome Brunet 1 month, 1 week ago
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.
Re: [PATCH v3 0/4] clk: amlogic: optimize the PLL driver
Posted by Chuan Liu 1 month, 1 week ago

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.