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

[Emil Renner Berthing]

E Shattow wrote:
>
>
> 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@xxxxxxxxxxxx>
> >> ---
> >>  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.

I think it's quite common for SoCs come out of reset in a safe slow clock
configuration and then expect software to set clock speeds to what the chip is
really capable of.

> 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.

If that's the case then maybe only lower the clock speed on the Mars CM Lite
boards.

> 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@xxxxxxxxxxxxxxxx/
>
> What is the correct thing to do here?

I really don't think slowing down all the boards because of missing code in
U-Boot is the right solution, but Hal also answered in the old thread.

/Emil