Re: [PATCH v1 1/5] riscv: dts: starfive: jh7110-common: replace syscrg clock assignments

[E Shattow <e@freeshell.de>]

Hi, Conor  (added CC: Minda Chen, Hal Feng)

On 1/4/25 10:33, Conor Dooley wrote:
> On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
>> Replace syscrg assignments of clocks, clock parents, and rates, for
>> compatibility with downstream boot loader SPL secondary program
>> loader.
>>
>> Signed-off-by: E Shattow <e@freeshell.de>
>> ---
>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index 48fb5091b817..55c6743100a7 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -359,9 +359,15 @@ spi_dev0: spi@0 {
>>   };
>>   
>>   &syscrg {
>> -	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_CORE>,
>> -			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
>> -	assigned-clock-rates = <500000000>, <1500000000>;
>> +	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
>> +			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
>> +			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
>> +			  <&syscrg JH7110_SYSCLK_QSPI_REF>;
>> +	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
>> +				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
>> +				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
>> +				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
>> +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> 
> Why is assigned rates here 0s, rather than the property just removed?
> 
>>   };
>>   
>>   &sysgpio {
>> -- 
>> 2.45.2
>>

Assigned rates all zeroes is how it is in U-Boot. Removing the 
assigned-clock-rates property as suggested does work in U-Boot and Linux 
both.

For context, U-Boot fails when replacing assigned-clocks to 
JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT (1500MHz) 
from Linux. So I tried to merge all properties together and in testing 
then U-Boot failed (or I did it wrong). However replacing the Linux 
properties with the U-Boot configuration (above) on Linux does work for 
both.

I do not know if this is correct but I can test any suggestions and 
report if they are working.

Do these changes make sense? Are there other variations I should test?

Thanks,

-E