On Tue 26 Aug 2025 at 15:59, Chuan Liu [off-list ref] wrote:

Hi Da:

    Thanks for your feedback. but this patch is wrong.


On 8/22/2025 8:22 AM, Da Xue wrote:

quoted

[ EXTERNAL EMAIL ]

The bit range 17:0 does not match the datasheet for A311D / S905D3.
Change the bit range to 18:0 for FIX and HIFI PLLs to match datasheet.

The upper 2 bits (bit18, bit17) of the frac were deprecated long ago.

deprecated ? that is really confusing

That seems to imply that it does have an effect but you are choosing not
to use it. Please clarify.

The actual effective bit field for frac is bit[16:0]. However, the
corresponding datasheet has not been updated. I will provide feedback
and update the datasheet accordingly.

What about bit 17 and 18 then ? does it have any effect at all ?

quoted

The frac field is missing for sys pll so add that as well.

PLLs with frac support are used in scenarios requiring a wide range
of output frequencies (e.g., audio/video applications).

Since sys_pll is dedicated to clocking the CPU and does not require
such frequency versatility, it does not support fractional frequency
multiplication.

You are mixing "HW support" and "usage choice" here.

What I read is :
* Da says the SYS PLL does have HW support for the frac parameter
* Amlogic does not see the point of using it since the CPU does not
  require fine tuning of the rate.

Is that correct ? or is the HW just no present ?

quoted

Patched:

+ sudo cat /sys/devices/system/cpu/cpufreq/policy0/cpuinfo_cur_freq \
/sys/devices/system/cpu/cpufreq/policy2/cpuinfo_cur_freq
996999
500000
+ sudo cat /sys/kernel/debug/meson-clk-msr/measure_summary
+ grep -i '\(sys_\|hifi_\|fixed_\)pll'
  hifi_pll                      0    +/-1562Hz
  sys_pll_div16                 0    +/-1562Hz
  sys_pll_cpub_div16            0    +/-1562Hz
+ sudo cat /sys/kernel/debug/clk/clk_summary
+ grep -i '\(sys_\|hifi_\|fixed_\)pll'
     hifi_pll_dco                     0       0        0        0
        hifi_pll                      0       0        0        0
     sys_pll_dco                      1       1        0        3999999985
        sys_pll                       0       0        0        499999999
           sys_pll_div16_en           0       0        0        499999999
              sys_pll_div16           0       0        0        31249999
     fixed_pll_dco                    1       1        1        3987999985
        fixed_pll                     3       3        1        1993999993

Unpatch:

+ sudo cat /sys/devices/system/cpu/cpufreq/policy0/cpuinfo_cur_freq \
/sys/devices/system/cpu/cpufreq/policy2/cpuinfo_cur_freq
1000000
500000
+ sudo cat /sys/kernel/debug/meson-clk-msr/measure_summary
+ grep -i '\(sys_\|hifi_\|fixed_\)pll'
  hifi_pll                      0    +/-1562Hz
  sys_pll_div16                 0    +/-1562Hz
  sys_pll_cpub_div16            0    +/-1562Hz
+ sudo cat /sys/kernel/debug/clk/clk_summary
+ grep -i '\(sys_\|hifi_\|fixed_\)pll'
     hifi_pll_dco                     0       0        0        0
        hifi_pll                      0       0        0        0
     sys_pll_dco                      1       1        0        4800000000
        sys_pll                       0       0        0        1200000000
           sys_pll_div16_en           0       0        0        1200000000
              sys_pll_div16           0       0        0        75000000
     fixed_pll_dco                    1       1        1        3999999939
        fixed_pll                     3       3        1        1999999970

Fixes: 085a4ea93d54 ("clk: meson: g12a: add peripheral clock controller")
Signed-off-by: Da Xue <redacted>
---
  drivers/clk/meson/g12a.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 66f0e817e416..f78cca619ca5 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c

@@ -157,7 +157,7 @@ static struct clk_regmap g12a_fixed_pll_dco = {
                 .frac = {
                         .reg_off = HHI_FIX_PLL_CNTL1,
                         .shift   = 0,
-                       .width   = 17,
+                       .width   = 19,
                 },
                 .l = {
                         .reg_off = HHI_FIX_PLL_CNTL0,

@@ -223,6 +223,11 @@ static struct clk_regmap g12a_sys_pll_dco = {
                         .shift   = 10,
                         .width   = 5,
                 },
+               .frac = {
+                       .reg_off = HHI_SYS_PLL_CNTL1,
+                       .shift   = 0,
+                       .width   = 19,
+               },
                 .l = {
                         .reg_off = HHI_SYS_PLL_CNTL0,
                         .shift   = 31,

@@ -1901,7 +1906,7 @@ static struct clk_regmap g12a_hifi_pll_dco = {
                 .frac = {
                         .reg_off = HHI_HIFI_PLL_CNTL1,
                         .shift   = 0,
-                       .width   = 17,
+                       .width   = 19,
                 },
                 .l = {
                         .reg_off = HHI_HIFI_PLL_CNTL0,
--

2.47.2


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

-- 
Jerome