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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJM55Z9HTLoE3ShMfyqN=-KeC+EY_kQ+JSvFpxpX1gumio1Brg@mail.gmail.com>
Date: Fri, 7 Feb 2025 04:21:53 -0800
From: Emil Renner Berthing <emil.renner.berthing@...onical.com>
To: E Shattow <e@...eshell.de>, 
	Emil Renner Berthing <emil.renner.berthing@...onical.com>, Conor Dooley <conor@...nel.org>, 
	Emil Renner Berthing <kernel@...il.dk>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2 1/5] riscv: dts: starfive: jh7110-common: replace
 syscrg clock assignments

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

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@starfivetech.com/
>
> 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