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] [day] [month] [year] [list]
Message-ID:
 <ZQ2PR01MB1307DB02B26454DE62F30B24E6C4A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn>
Date: Thu, 20 Feb 2025 06:38:11 +0000
From: Hal Feng <hal.feng@...rfivetech.com>
To: E Shattow <e@...eshell.de>, Hal Feng <hal.feng@...ux.starfivetech.com>,
	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" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>
Subject: RE: [PATCH v2 1/5] riscv: dts: starfive: jh7110-common: replace
 syscrg clock assignments

> On 10.02.25 07:34, E Shattow wrote:
> On 2/7/25 00:31, Hal Feng wrote:
> > On 2/5/2025 8:52 PM, 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.
> >>
> >> 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@starfive
> >> tech.com/
> >>
> >> What is the correct thing to do here?
> >>
> >> -E
> >>
> >> From mboxrd@z Thu Jan  1 00:00:00 1970
> >> Return-Path:
> >> <linux-riscv-bounces+linux-riscv=archiver.kernel.org@...ts.infradead.
> >> org>
> >> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> >> 	aws-us-west-2-korg-lkml-1.web.codeaurora.org
> >> Received: from bombadil.infradead.org (bombadil.infradead.org
> [198.137.202.133])
> >> 	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384
> (256/256 bits))
> >> 	(No client certificate requested)
> >> 	by smtp.lore.kernel.org (Postfix) with ESMTPS id B9A24C02192
> >> 	for <linux-riscv@...hiver.kernel.org>; Wed,  5 Feb 2025 13:10:59
> >> +0000 (UTC)
> >> DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
> >> 	d=lists.infradead.org; s=bombadil.20210309; h=Sender:
> >> 	Content-Transfer-Encoding:Content-Type:List-Subscribe:List-
> Help:List-Post:
> >> 	List-Archive:List-Unsubscribe:List-Id:In-Reply-
> To:From:References:Cc:To:
> >> 	Subject:MIME-Version:Date:Message-ID:Reply-To:Content-
> ID:Content-Description:
> >> 	Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-
> Cc:Resent-Message-ID:
> >> 	List-Owner;
> bh=GY86gaXkDRjEBAUNvogHkHuyO230wjLSabDM8v7zKQQ=;
> b=Un7uhhDTAT8/9N
> >>
> 	FxyCZTIeuEf9Tz2EWguoSASPTIRzVsA8OD+zansoq7n0Em+ejnESLoVic
> WRdNflaSCojelA6mxlZr
> >>
> 	79fy10oRgiIKMOAb1fwJcsq+rGF8jSdXwi0a2zKjGYb4u4ZNy/uLBiIynsS
> H/VCYysTKQK6p7wAiC
> >>
> 	7RYsK3WfvbZKMTBmH2vKxA7ERtfZGfNAJqRjHzBM06+ZfEDf9V2UQ3p
> GUdGPoTZYkQoS8smFEx47Z
> >>
> 	U3KclAiQD6NRzOmPD/VXwUGXQEpLonSaLk7kbAdo3cWww6Wyou3
> w4XqxHQpym6FyLsKAWWSk7d4vx
> >> 	ZbYQckPNKc65NmLst1TA==;
> >> Received: from localhost ([::1] helo=bombadil.infradead.org)
> >> 	by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux))
> >> 	id 1tffB9-00000003Lk1-2ly8;
> >> 	Wed, 05 Feb 2025 13:10:55 +0000
> >> Received: from freeshell.de ([2a01:4f8:231:482b::2])
> >> 	by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux))
> >> 	id 1tfetw-00000003I2R-42Et
> >> 	for linux-riscv@...ts.infradead.org;
> >> 	Wed, 05 Feb 2025 12:53:10 +0000
> >> Received: from [192.168.2.35] (unknown [98.97.25.24])
> >> 	(Authenticated sender: e)
> >> 	by freeshell.de (Postfix) with ESMTPSA id 7ADA8B4C01E1;
> >> 	Wed,  5 Feb 2025 13:53:01 +0100 (CET)
> >> Message-ID: <981a3f30-c646-423a-a2dd-e19fef5c69e5@...eshell.de>
> >> Date: Wed, 5 Feb 2025 04:52:59 -0800
> >> MIME-Version: 1.0
> >> User-Agent: Mozilla Thunderbird
> >> Subject: Re: [PATCH v2 1/5] riscv: dts: starfive: jh7110-common:
> >> replace  syscrg clock assignments
> >> To: 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
> >> References: <20250203013730.269558-1-e@...eshell.de>
> >>  <20250203013730.269558-2-e@...eshell.de>
> >>  <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
> Q@...l.gmail.com>
> >> Content-Language: en-US
> >> From: E Shattow <e@...eshell.de>
> >> In-Reply-To:
> >> <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
> Q@...l.gmail.com>
> >> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) )
> >> MR-646709E3
> >> X-CRM114-CacheID: sfid-20250205_045309_156119_468548BF
> >> X-CRM114-Status: GOOD (  16.14  )
> >> X-BeenThere: linux-riscv@...ts.infradead.org
> >> X-Mailman-Version: 2.1.34
> >> Precedence: list
> >> List-Id: <linux-riscv.lists.infradead.org>
> >> List-Unsubscribe:
> >> <http://lists.infradead.org/mailman/options/linux-riscv>,
> >>  <mailto:linux-riscv-request@...ts.infradead.org?subject=unsubscribe>
> >> List-Archive: <http://lists.infradead.org/pipermail/linux-riscv/>
> >> List-Post: <mailto:linux-riscv@...ts.infradead.org>
> >> List-Help:
> >> <mailto:linux-riscv-request@...ts.infradead.org?subject=help>
> >> List-Subscribe:
> >> <http://lists.infradead.org/mailman/listinfo/linux-riscv>,
> >>  <mailto:linux-riscv-request@...ts.infradead.org?subject=subscribe>
> >> Content-Type: text/plain; charset="us-ascii"
> >> Content-Transfer-Encoding: 7bit
> >> Sender: "linux-riscv" <linux-riscv-bounces@...ts.infradead.org>
> >> Errors-To:
> >> linux-riscv-bounces+linux-riscv=archiver.kernel.org@...ts.infradead.o
> >> rg
> >>
> >>
> >>
> >> 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.
> >>
> >> 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@starfive
> >> tech.com/
> >>
> >> What is the correct thing to do here?
> >
> > Please see my reply in
> >
> https://lore.kernel.org/all/ZQ2PR01MB130736F5C893337606FD6937E6F1A@
> ZQ2
> > PR01MB1307.CHNPR01.prod.partner.outlook.cn/
> >
> > Thanks.
> >
> > Best regards,
> > Hal
> >
> 
> What is the explanation to remove the (hardware default) CPU power supply
> 0.9V operation, and remove hardware default CPU speed?
> 
> I would think you will want jh7110.dtsi similar as follows:
> 
>         cpu_opp: opp-table-0 {
>                         compatible = "operating-points-v2";
>                         opp-shared;
>                         opp-250000000 {
>                                         opp-hz = /bits/ 64 <250000000>;
>                                         opp-microvolt = <800000>;
>                         };
>                         opp-333000000 {
>                                         opp-hz = /bits/ 64 <333000000>;
>                                         opp-microvolt = <800000>;
>                         };
>                         opp-375000000 {
>                                         opp-hz = /bits/ 64 <375000000>;
>                                         opp-microvolt = <800000>;
>                         };
>                         opp-500000000 {
>                                         opp-hz = /bits/ 64 <500000000>;
>                                         opp-microvolt = <800000>;
>                         };
>                         opp-750000000 {
>                                         opp-hz = /bits/ 64 <750000000>;
>                                         opp-microvolt = <800000>;
>                         };
>                         opp-1000000000 {
>                                         opp-hz = /bits/ 64 <1000000000>;
>                                         opp-microvolt = <900000>;
>                         };
>                         opp-1500000000 {
>                                         opp-hz = /bits/ 64 <1500000000>;
>                                         opp-microvolt = <1040000>;
>                         };
>         };

This method doesn't make sense, because the division factor is an integer
and changing clock pll0 frequently may cause errors.

> 
> I guess at the voltages here, you will have to provide a correction about what
> is realistic.
> 
> Board makers are designing their board layout with opp-1000000000 default
> voltage with default clock frequency and not opp-1500000000 on an
> increased voltage and maximum clock speed yes?

No, default CPU supply voltage or frequency were never designed for the board
and JH7110 SoC. The default voltage I said in [1] means the default output voltage
of PMIC AXP15060. When the board is powered up, the CPU supply which comes
from PMIC is 0.9V, so the CPU can just run at 1.0GHz or lower. After that, the board
can run at other frequency if the CPU supply voltage, pll0 clock frequency, division
factor are set correctly. 1.5GHz is a stable frequency to run. We had done a lot of test
at this frequency. Actually, JH7110 can even run at 1.75GHz, but that is not stable.

[1] https://lore.kernel.org/all/ZQ2PR01MB130736F5C893337606FD6937E6F1A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/

> 
> We should not exceed the hardware default CPU voltage (and clock
> frequency) for all boards without some very good reason, and I do not accept
> "because more voltage and higher frequency is faster" to be a reason to do
> this.
> 
> Hal, you have not explained why it is *necessary* to have this non-default
> voltage and clock frequency table for all boards. I know that using a JH7110
> board that allows 1.5GHz operation results in thermal over-temperature and
> shutdown, where it then does no work at all and is not useful to me. I do

Really? Could you please give more detail to us? Thanks for your feedback.
We had tested 1.5GHz under large loads many times and running at 1.5GHz is
the requirement from some developers, so we upstreamed it before.

> accept that you want 1.5GHz operation, for a specific board that is no problem
> with me because I do not have that board to test if it is any problem; I have to
> trust that you have done this testing, then.
> 
> Summary of my thoughts are:
> 
> 1. Yes to support for lower and higher core voltage and clock frequency
> 
> 2. No to imposing non-default hardware profiles on all boards, unless there is
> some actual reason why it is strictly necessary (thermal performance testing,
> operational stability, power savings, ...)

If some boards can't run at 1.5GHz, we can move node opp-table-0 to the
board device tree from jh7110.dtsi. Then each board can set its own CPU
frequency.

Best regards,
Hal

> 
> Question follow-up:
> 
> 1. Can we restore 1GHz operation ?  I ask if there is a property or strategy
> here to force 1GHz maximum operation unless the user decides for exceeding
> the hardware default 0.9V ?
> 
> -E

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ