| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1jy0r64fcr.fsf@starbuckisacylon.baylibre.com>
Date: Tue, 26 Aug 2025 11:11:32 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Chuan Liu <chuan.liu@...ogic.com>
Cc: Da Xue <da@...re.computer>, Neil Armstrong <neil.armstrong@...aro.org>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Kevin Hilman <khilman@...libre.com>, Martin
Blumenstingl <martin.blumenstingl@...glemail.com>, Jian Hu
<jian.hu@...ogic.com>, linux-amlogic@...ts.infradead.org,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] clk: meson-g12a: fix bit range for fixed sys and
hifi pll
On Tue 26 Aug 2025 at 15:59, Chuan Liu <chuan.liu@...ogic.com> wrote:
> Hi Da:
>
> Thanks for your feedback. but this patch is wrong.
>
>
> On 8/22/2025 8:22 AM, Da Xue wrote:
>> [ 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 ?
>
>>
>> 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 ?
>
>
>>
>> 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 <da@...re.computer>
>> ---
>> 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@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
--
Jerome
Powered by blists - more mailing lists