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

[E Shattow <e@freeshell.de>]

On 2/5/25 02:16, Emil Renner Berthing wrote:
> E Shattow wrote:
>> Replace syscrg assignments of clocks, clock parents, and rates with
>> default settings 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 | 11 ++++++++---
>>  1 file changed, 8 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..a5661b677687 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -359,9 +359,14 @@ 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>;
> 
> I think Conor asked about this too, but you still don't write why it's ok to
> drop the 500MHz and 1,5GHz assignments to the cpu-core and pll0 clocks
> respectively. You should add this to the commit message itself.
> 
> /Emil

Is this a remedy for a bug in the JH7110 CPU? I'm not clear why tweaking
the frequencies and increasing core voltage was ever needed.

This goes back to series "clk: starfive: jh7110-sys: Fix lower rate of
CPUfreq by setting PLL0 rate to 1.5GHz" [1].

Since [1] I have had problems with several passively cooled Milk-V Mars
CM Lite systems powering off due to thermal limits. My experience then
is that the specialized 1.5GHz operation is not appropriate for all
JH7110 CPU board layouts and applications.

Hal says I failed to get these assignments in Linux to work in U-Boot
because U-Boot doesn't have driver support to increase CPU voltage, and
Hal offering to add this to a driver in U-Boot... but that's the wrong
way around in my opinion, unless there's some defect in the JH7110 CPU
that it won't run reliably with hardware defaults.

1:
https://lore.kernel.org/all/20240603020607.25122-1-xingyu.wu@starfivetech.com/

What is the correct thing to do here?

-E