Re: [PATCH v4] clk: spacemit: k3: add CPU PLL rate tables

["Troy Mitchell" <troy.mitchell@linux.spacemit.com>]

[-- Attachment #1: Type: text/plain, Size: 4294 bytes --]

Hi Aurelien,

On Fri Sep 4, 2026 at 1:27 AM +08, Aurelien Jarno wrote:
> Hi Troy,
>
> On 2026-09-03 11:06, Troy Mitchell wrote:
>> The K3 CPU PLL rate tables currently describe only one rate per PLL,
>> although the hardware supports a wider range.
>> 
>> PLL3 and PLL4 support rates from 1.05 to 2.4 GHz, while PLL5 and PLL8
>> support rates from 1.05 to 2 GHz. Populate the tables with every
>> supported rate in 50 MHz steps.
>> 
>> Cc: stable@vger.kernel.org # 7.0+
>> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>> ---
>> The corresponding U-Boot changes are available at:
>> 
>> - Add the CPU PLL rate tables:
>>   https://github.com/spacemit-com/uboot-2022.10/commit/787b5cf44c8dbc99db886d5522c0c9cd5a96ffd6
>> - Correct the 1.10 GHz and 2.15 GHz table entries:
>>   https://github.com/spacemit-com/uboot-2022.10/commit/16a775c20e9efb54ca55352215e76eb3f12e6cf0
>> ---
>> Changes in v4:
>> - Expand the update to every supported PLL3, PLL4, PLL5 and PLL8 rate.
>> - Drop Aurelien Jarno's Reviewed-by and Tested-by trailers because the
>>   patch scope has expanded.
>> - Add U-Boot references for the complete tables and corrected entries.
>> - Link to v3: https://patch.msgid.link/20260831-k3-pll5-pll8-1800mhz-v3-1-530ed0978615@linux.spacemit.com
>> 
>> Changes in v3:
>> - Mark the change for stable kernels starting with v7.0.
>> - Add Aurelien Jarno's Reviewed-by and Tested-by trailers.
>> - Rebase onto v7.3-rc1.
>> - Link to v2: https://patch.msgid.link/20260810-k3-pll5-pll8-1800mhz-v2-1-aabff510a797@linux.spacemit.com
>> 
>> Changes in v2:
>> - Drop "clk: spacemit: re-enable PLLs after init"; the PLL init issue
>>   will be handled in a separate series.
>> - Send the K3 PLL5 and PLL8 1.8 GHz rate-table update as a standalone
>>   patch.
>> - Clarify that 1.8 GHz is nominal and 2 GHz is an overclocking rate.
>> - Link to v1: https://patch.msgid.link/20260804-k3-pll5-pll8-1800mhz-v1-0-a3a75866ce32@linux.spacemit.com
>> 
>> To: Stephen Boyd <sboyd@kernel.org>
>> To: Brian Masney <bmasney+clk@redhat.com>
>> To: Jerome Brunet <jbrunet+clk@baylibre.com>
>> To: Yixun Lan <dlan@kernel.org>
>> To: Paul Walmsley <pjw@kernel.org>
>> To: Palmer Dabbelt <palmer@dabbelt.com>
>> To: Albert Ou <aou@eecs.berkeley.edu>
>> To: Alexandre Ghiti <alex@ghiti.fr>
>> Cc: linux-clk@vger.kernel.org
>> Cc: linux-riscv@lists.infradead.org
>> Cc: spacemit@lists.linux.dev
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>  drivers/clk/spacemit/ccu-k3.c | 96 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 96 insertions(+)
>
> Thanks for the patch, it makes sense to add all the values, to support 
> future firmware changes. The values match the ones in the vendor kernel.
I forgot that you had a similar patch as well..

>
>> diff --git a/drivers/clk/spacemit/ccu-k3.c b/drivers/clk/spacemit/ccu-k3.c
>> index 92b930d5ff30..950bf531c734 100644
>> --- a/drivers/clk/spacemit/ccu-k3.c
>> +++ b/drivers/clk/spacemit/ccu-k3.c
>> @@ -29,15 +29,91 @@ static const struct ccu_pll_rate_tbl pll2_rate_tbl[] = {
>>  	CCU_PLLA_RATE(3000000000UL, 0x0b3e2000, 0x00000000, 0xa0558c8c),
>>  };
>>  
>> +/* Keep the firmware default as the first entry. */
>>  static const struct ccu_pll_rate_tbl pll3_rate_tbl[] = {
>> +	CCU_PLLA_RATE(2000000000UL, 0x0b292aaa, 0x0000ab00, 0xa0558686),
>
> Is it really necessary to put this entry first? I guess it's the current 
> default frequency, but we have seen it can evolve. From my tests, the 
> driver seems to just keep the value configured by the firmware 
> independently from the table order, so I am not sure it's necessary.
I think keeping it first is necessary. If the current register settings do
not match any entry in the table, ccu_plla_init() falls back to the first
entry:


	ccu_plla_update_param(pll, &pll->config.rate_tbl[0]);

[...]

>>  };
>
> I have tested the patch, and I confirm it fixes the boot hang with newer 
> firmware, and will fix it if the default firmware values change again. 
> My comments are just minor ones, so:
>
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
> Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Thanks for the tags.

>
> Regards
> Aurelien 


-- 
Troy Mitchell

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 248 bytes --]