[PATCH v2 0/5] riscv: dts: starfive: jh7110-common: Sync downstream U-Boot changes

[PATCH v2 0/5] riscv: dts: starfive: jh7110-common: Sync downstream U-Boot changes

[E Shattow]

U-Boot boot loader has adopted using the Linux dt-rebasing tree for dts
with StarFive VisionFive2 board target (and related JH7110 common boards).
Sync the minimum changes from jh7110-common.dtsi needed for boot so these
can be dropped from U-Boot.

Changes since v1:
  Do not set unused eeprom label.
  Do not set no-op zero-value assigned-clock-rates, adjust commit message.

E Shattow (5):
  riscv: dts: starfive: jh7110-common: replace syscrg clock assignments
  riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2
    cycles max 100MHz
  riscv: dts: starfive: jh7110-common: assign 24MHz clock-frequency to
    uart0
  riscv: dts: starfive: jh7110-common: add eeprom node to i2c5
  riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by
    boot  loader

 .../boot/dts/starfive/jh7110-common.dtsi      | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)


base-commit: 708d55db3edbe2ccf88d94b5f2e2b404bc0ba37c
-- 
2.47.2

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

[E Shattow]

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>;
 };
 
 &sysgpio {
-- 
2.47.2

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

[Emil Renner Berthing]

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

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

[E Shattow]

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

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

[Hal Feng]

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@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
> 
> From mboxrd@z Thu Jan  1 00:00:00 1970
> Return-Path: <linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.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@archiver.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+ejnESLoVicWRdNflaSCojelA6mxlZr
> 	79fy10oRgiIKMOAb1fwJcsq+rGF8jSdXwi0a2zKjGYb4u4ZNy/uLBiIynsSH/VCYysTKQK6p7wAiC
> 	7RYsK3WfvbZKMTBmH2vKxA7ERtfZGfNAJqRjHzBM06+ZfEDf9V2UQ3pGUdGPoTZYkQoS8smFEx47Z
> 	U3KclAiQD6NRzOmPD/VXwUGXQEpLonSaLk7kbAdo3cWww6Wyou3w4XqxHQpym6FyLsKAWWSk7d4vx
> 	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@lists.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@freeshell.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@canonical.com>,
>  Conor Dooley <conor@kernel.org>, Emil Renner Berthing <kernel@esmil.dk>,
>  Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,
>  Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt
>  <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
>  linux-riscv@lists.infradead.org
> References: <20250203013730.269558-1-e@freeshell.de>
>  <20250203013730.269558-2-e@freeshell.de>
>  <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-Q@mail.gmail.com>
> Content-Language: en-US
> From: E Shattow <e@freeshell.de>
> In-Reply-To: <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-Q@mail.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@lists.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@lists.infradead.org?subject=unsubscribe>
> List-Archive: <http://lists.infradead.org/pipermail/linux-riscv/>
> List-Post: <mailto:linux-riscv@lists.infradead.org>
> List-Help: <mailto:linux-riscv-request@lists.infradead.org?subject=help>
> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-riscv>,
>  <mailto:linux-riscv-request@lists.infradead.org?subject=subscribe>
> Content-Type: text/plain; charset="us-ascii"
> Content-Transfer-Encoding: 7bit
> Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org>
> Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org
> 
> 
> 
> 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?

Please see my reply in
https://lore.kernel.org/all/ZQ2PR01MB130736F5C893337606FD6937E6F1A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/

Thanks.

Best regards,
Hal

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

[E Shattow]

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@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
>>
>> From mboxrd@z Thu Jan  1 00:00:00 1970
>> Return-Path: <linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.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@archiver.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+ejnESLoVicWRdNflaSCojelA6mxlZr
>> 	79fy10oRgiIKMOAb1fwJcsq+rGF8jSdXwi0a2zKjGYb4u4ZNy/uLBiIynsSH/VCYysTKQK6p7wAiC
>> 	7RYsK3WfvbZKMTBmH2vKxA7ERtfZGfNAJqRjHzBM06+ZfEDf9V2UQ3pGUdGPoTZYkQoS8smFEx47Z
>> 	U3KclAiQD6NRzOmPD/VXwUGXQEpLonSaLk7kbAdo3cWww6Wyou3w4XqxHQpym6FyLsKAWWSk7d4vx
>> 	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@lists.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@freeshell.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@canonical.com>,
>>  Conor Dooley <conor@kernel.org>, Emil Renner Berthing <kernel@esmil.dk>,
>>  Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,
>>  Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt
>>  <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>
>> Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
>>  linux-riscv@lists.infradead.org
>> References: <20250203013730.269558-1-e@freeshell.de>
>>  <20250203013730.269558-2-e@freeshell.de>
>>  <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-Q@mail.gmail.com>
>> Content-Language: en-US
>> From: E Shattow <e@freeshell.de>
>> In-Reply-To: <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-Q@mail.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@lists.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@lists.infradead.org?subject=unsubscribe>
>> List-Archive: <http://lists.infradead.org/pipermail/linux-riscv/>
>> List-Post: <mailto:linux-riscv@lists.infradead.org>
>> List-Help: <mailto:linux-riscv-request@lists.infradead.org?subject=help>
>> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-riscv>,
>>  <mailto:linux-riscv-request@lists.infradead.org?subject=subscribe>
>> Content-Type: text/plain; charset="us-ascii"
>> Content-Transfer-Encoding: 7bit
>> Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org>
>> Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org
>>
>>
>>
>> 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?
> 
> Please see my reply in
> https://lore.kernel.org/all/ZQ2PR01MB130736F5C893337606FD6937E6F1A@ZQ2PR01MB1307.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>;
                        };
        };

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?

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

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

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

[Hal Feng]

> 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@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@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@lists.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@archiver.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@lists.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@freeshell.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@canonical.com>,
> >>  Conor Dooley <conor@kernel.org>, Emil Renner Berthing
> >> <kernel@esmil.dk>,  Rob Herring <robh@kernel.org>, Krzysztof
> >> Kozlowski <krzk+dt@kernel.org>,  Paul Walmsley
> >> <paul.walmsley@sifive.com>, Palmer Dabbelt  <palmer@dabbelt.com>,
> >> Albert Ou <aou@eecs.berkeley.edu>
> >> Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
> >> linux-riscv@lists.infradead.org
> >> References: <20250203013730.269558-1-e@freeshell.de>
> >>  <20250203013730.269558-2-e@freeshell.de>
> >>  <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
> Q@mail.gmail.com>
> >> Content-Language: en-US
> >> From: E Shattow <e@freeshell.de>
> >> In-Reply-To:
> >> <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
> Q@mail.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@lists.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@lists.infradead.org?subject=unsubscribe>
> >> List-Archive: <http://lists.infradead.org/pipermail/linux-riscv/>
> >> List-Post: <mailto:linux-riscv@lists.infradead.org>
> >> List-Help:
> >> <mailto:linux-riscv-request@lists.infradead.org?subject=help>
> >> List-Subscribe:
> >> <http://lists.infradead.org/mailman/listinfo/linux-riscv>,
> >>  <mailto:linux-riscv-request@lists.infradead.org?subject=subscribe>
> >> Content-Type: text/plain; charset="us-ascii"
> >> Content-Transfer-Encoding: 7bit
> >> Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org>
> >> Errors-To:
> >> linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.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@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@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

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

[E Shattow]

On 2/19/25 22:38, Hal Feng wrote:
>> 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@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@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@lists.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@archiver.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@lists.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@freeshell.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@canonical.com>,
>>>>  Conor Dooley <conor@kernel.org>, Emil Renner Berthing
>>>> <kernel@esmil.dk>,  Rob Herring <robh@kernel.org>, Krzysztof
>>>> Kozlowski <krzk+dt@kernel.org>,  Paul Walmsley
>>>> <paul.walmsley@sifive.com>, Palmer Dabbelt  <palmer@dabbelt.com>,
>>>> Albert Ou <aou@eecs.berkeley.edu>
>>>> Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
>>>> linux-riscv@lists.infradead.org
>>>> References: <20250203013730.269558-1-e@freeshell.de>
>>>>  <20250203013730.269558-2-e@freeshell.de>
>>>>  <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
>> Q@mail.gmail.com>
>>>> Content-Language: en-US
>>>> From: E Shattow <e@freeshell.de>
>>>> In-Reply-To:
>>>> <CAJM55Z-M9MsJAtPi-t=_tNV3oG_kdDiS6H=H3koJwTEwB8GJ-
>> Q@mail.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@lists.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@lists.infradead.org?subject=unsubscribe>
>>>> List-Archive: <http://lists.infradead.org/pipermail/linux-riscv/>
>>>> List-Post: <mailto:linux-riscv@lists.infradead.org>
>>>> List-Help:
>>>> <mailto:linux-riscv-request@lists.infradead.org?subject=help>
>>>> List-Subscribe:
>>>> <http://lists.infradead.org/mailman/listinfo/linux-riscv>,
>>>>  <mailto:linux-riscv-request@lists.infradead.org?subject=subscribe>
>>>> Content-Type: text/plain; charset="us-ascii"
>>>> Content-Transfer-Encoding: 7bit
>>>> Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org>
>>>> Errors-To:
>>>> linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.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@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@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

I apologize Hal, this topic is more in-depth than my basic understanding
about hardware design. Can we make this so we have six runtime
frequencies around the manufacturer StarFive suggestion of 1.5GHz?:

250MHz
500MHz
750MHz
1GHz
1.25GHz
1.5GHz
1.75GHz (seven? you say this doesn't work well though.)

Is 250MHz stable?

I hear you say that it is an integer division, okay. How about a
multiplier up from 250MHz though? That is a bad suggestion?

If you can be kind and explain to me - (I am dumb about this topic but
I do want to learn) - and explain what is the limitation why this is an
integer division and not a multiplier?

I've been asking other people in my peer group and none have this
knowledge, so I do need help with this because your explanation is clear
but lacks the basic theory which I did not learn yet (and no one I know
has this knowledge either).

-E

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

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

[PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2 cycles max 100MHz

[E Shattow]

Sync qspi flash setting to read-delay=2 and spi-max-frequency 100MHz for
better compatibility with operating system and downstream boot loader SPL
secondary program loader.

Signed-off-by: E Shattow <e@freeshell.de>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index a5661b677687..8a59c3001339 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -317,8 +317,8 @@ &qspi {
 	nor_flash: flash@0 {
 		compatible = "jedec,spi-nor";
 		reg = <0>;
-		cdns,read-delay = <5>;
-		spi-max-frequency = <12000000>;
+		cdns,read-delay = <2>;
+		spi-max-frequency = <100000000>;
 		cdns,tshsl-ns = <1>;
 		cdns,tsd2d-ns = <1>;
 		cdns,tchsh-ns = <1>;
-- 
2.47.2

Re: [PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2 cycles max 100MHz

[Emil Renner Berthing]

E Shattow wrote:
> Sync qspi flash setting to read-delay=2 and spi-max-frequency 100MHz for
> better compatibility with operating system and downstream boot loader SPL
> secondary program loader.

Here you should be explaining why these values better describe the hardware. To
me this just reads as "u-boot does this, so let's do the same" whith doesn't
really explain anything.

/Emil

>
> Signed-off-by: E Shattow <e@freeshell.de>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index a5661b677687..8a59c3001339 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -317,8 +317,8 @@ &qspi {
>  	nor_flash: flash@0 {
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
> -		cdns,read-delay = <5>;
> -		spi-max-frequency = <12000000>;
> +		cdns,read-delay = <2>;
> +		spi-max-frequency = <100000000>;
>  		cdns,tshsl-ns = <1>;
>  		cdns,tsd2d-ns = <1>;
>  		cdns,tchsh-ns = <1>;
> --
> 2.47.2
>

Re: [PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2 cycles max 100MHz

[E Shattow]

Hi Emil,

On 2/5/25 02:18, Emil Renner Berthing wrote:
> E Shattow wrote:
>> Sync qspi flash setting to read-delay=2 and spi-max-frequency 100MHz for
>> better compatibility with operating system and downstream boot loader SPL
>> secondary program loader.
> 
> Here you should be explaining why these values better describe the hardware. To
> me this just reads as "u-boot does this, so let's do the same" whith doesn't
> really explain anything.
> 
> /Emil
> 
>>
>> Signed-off-by: E Shattow <e@freeshell.de>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index a5661b677687..8a59c3001339 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -317,8 +317,8 @@ &qspi {
>>  	nor_flash: flash@0 {
>>  		compatible = "jedec,spi-nor";
>>  		reg = <0>;
>> -		cdns,read-delay = <5>;
>> -		spi-max-frequency = <12000000>;
>> +		cdns,read-delay = <2>;
>> +		spi-max-frequency = <100000000>;
>>  		cdns,tshsl-ns = <1>;
>>  		cdns,tsd2d-ns = <1>;
>>  		cdns,tchsh-ns = <1>;
>> --
>> 2.47.2
>>

That is true, there's not much to explain. It works at delay 2 and
100MHz max. I tried many delay values with the pre-existing 12MHz max,
they did not work. I don't have more information than that except what I
wrote about on the U-Boot mailing list [1]

[1]
https://lore.kernel.org/u-boot/ZQ2PR01MB13072E932737DD9D65E3A250E637A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/

The parameter training code in U-Boot when given max 100MHz has this at
somewhere between delay 2 and delay 3, where delay 3 almost appears to
work but there's data corruption, and delay 1 is totally non-functional.
With all testing so far the delay 2 and max frequency 100MHz is reliable
and also is a common occurrence in the Linux code base for similar qspi
access.

My testing methodology was pretty extensive and I wrote about the result
on the U-Boot mailing list, however here we're taking the success of
that in U-Boot and literally just applying it to Linux where it also
happens to align with some common similar assignments for other boards
in the devicetree code base. It works and there's no deeper insight here.

Welcome to hear from anyone that might actually understand this hardware?

-E

Re: [PATCH v2 2/5] riscv: dts: starfive: jh7110-common: qspi flash setting read-delay 2 cycles max 100MHz

[Emil Renner Berthing]

E Shattow wrote:
> Hi Emil,
>
> On 2/5/25 02:18, Emil Renner Berthing wrote:
> > E Shattow wrote:
> >> Sync qspi flash setting to read-delay=2 and spi-max-frequency 100MHz for
> >> better compatibility with operating system and downstream boot loader SPL
> >> secondary program loader.
> >
> > Here you should be explaining why these values better describe the hardware. To
> > me this just reads as "u-boot does this, so let's do the same" whith doesn't
> > really explain anything.
> >
> > /Emil
> >
> >>
> >> Signed-off-by: E Shattow <e@freeshell.de>
> >> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> >> ---
> >>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> index a5661b677687..8a59c3001339 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >> @@ -317,8 +317,8 @@ &qspi {
> >>  	nor_flash: flash@0 {
> >>  		compatible = "jedec,spi-nor";
> >>  		reg = <0>;
> >> -		cdns,read-delay = <5>;
> >> -		spi-max-frequency = <12000000>;
> >> +		cdns,read-delay = <2>;
> >> +		spi-max-frequency = <100000000>;
> >>  		cdns,tshsl-ns = <1>;
> >>  		cdns,tsd2d-ns = <1>;
> >>  		cdns,tchsh-ns = <1>;
> >> --
> >> 2.47.2
> >>
>
> That is true, there's not much to explain. It works at delay 2 and
> 100MHz max. I tried many delay values with the pre-existing 12MHz max,
> they did not work. I don't have more information than that except what I
> wrote about on the U-Boot mailing list [1]
>
> [1]
> https://lore.kernel.org/u-boot/ZQ2PR01MB13072E932737DD9D65E3A250E637A@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/
>
> The parameter training code in U-Boot when given max 100MHz has this at
> somewhere between delay 2 and delay 3, where delay 3 almost appears to
> work but there's data corruption, and delay 1 is totally non-functional.
> With all testing so far the delay 2 and max frequency 100MHz is reliable
> and also is a common occurrence in the Linux code base for similar qspi
> access.
>
> My testing methodology was pretty extensive and I wrote about the result
> on the U-Boot mailing list, however here we're taking the success of
> that in U-Boot and literally just applying it to Linux where it also
> happens to align with some common similar assignments for other boards
> in the devicetree code base. It works and there's no deeper insight here.

This explanation is fine, but it should be in the commit message so we can go
back and check why those numbers were chosen.

/Emil

[PATCH v2 3/5] riscv: dts: starfive: jh7110-common: assign 24MHz clock-frequency to uart0

[E Shattow]

Set uart0 clock-frequency for better compatibility with operating system
and downstream boot loader SPL secondary program loader.

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 8a59c3001339..6bb13af82147 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -635,6 +635,7 @@ GPOEN_DISABLE,
 };
 
 &uart0 {
+	clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
-- 
2.47.2

Re: [PATCH v2 3/5] riscv: dts: starfive: jh7110-common: assign 24MHz clock-frequency to uart0

[Hal Feng]

On 2/3/2025 9:37 AM, E Shattow wrote:
> Set uart0 clock-frequency for better compatibility with operating system
> and downstream boot loader SPL secondary program loader.
> 
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 8a59c3001339..6bb13af82147 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -635,6 +635,7 @@ GPOEN_DISABLE,
>  };
>  
>  &uart0 {
> +	clock-frequency = <24000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";

Reviewed-by: Hal Feng <hal.feng@starfivetech.com>

Best regards,
Hal

Re: [PATCH v2 3/5] riscv: dts: starfive: jh7110-common: assign 24MHz clock-frequency to uart0

[Emil Renner Berthing]

E Shattow wrote:
> Set uart0 clock-frequency for better compatibility with operating system
> and downstream boot loader SPL secondary program loader.
>
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 8a59c3001339..6bb13af82147 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -635,6 +635,7 @@ GPOEN_DISABLE,
>  };
>
>  &uart0 {
> +	clock-frequency = <24000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pins>;
>  	status = "okay";

The uart0 node already has a reference to the uart0_core clock, so it shouldn't
need this property.

/Emil

Re: [PATCH v2 3/5] riscv: dts: starfive: jh7110-common: assign 24MHz clock-frequency to uart0

[E Shattow]

On 2/5/25 02:29, Emil Renner Berthing wrote:
> E Shattow wrote:
>> Set uart0 clock-frequency for better compatibility with operating system
>> and downstream boot loader SPL secondary program loader.
>>
>> Signed-off-by: E Shattow <e@freeshell.de>
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index 8a59c3001339..6bb13af82147 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -635,6 +635,7 @@ GPOEN_DISABLE,
>>  };
>>
>>  &uart0 {
>> +	clock-frequency = <24000000>;
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&uart0_pins>;
>>  	status = "okay";
> 
> The uart0 node already has a reference to the uart0_core clock, so it shouldn't
> need this property.
> 
> /Emil

Okay. I'll drop this patch from next version of the series, and asking
on U-Boot mailing list what to do about it there [1]. Thanks for reviewing!

-E

1:
https://lore.kernel.org/u-boot/c0023d23-4614-40c5-b612-9c0cb5b4d8b1@freeshell.de/

[PATCH v2 4/5] riscv: dts: starfive: jh7110-common: add eeprom node to i2c5

[E Shattow]

StarFive VisionFive2 and similar JH7110 boards have an eeprom compatible
with Atmel 24c04. Add the node so this may be used with the at24 driver.

Signed-off-by: E Shattow <e@freeshell.de>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 6bb13af82147..30c5f3487c8b 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -245,6 +245,12 @@ emmc_vdd: aldo4 {
 			};
 		};
 	};
+
+	eeprom@50 {
+		compatible = "atmel,24c04";
+		reg = <0x50>;
+		pagesize = <16>;
+	};
 };
 
 &i2c6 {
-- 
2.47.2

Re: [PATCH v2 4/5] riscv: dts: starfive: jh7110-common: add eeprom node to i2c5

[Emil Renner Berthing]

E Shattow wrote:
> StarFive VisionFive2 and similar JH7110 boards have an eeprom compatible
> with Atmel 24c04. Add the node so this may be used with the at24 driver.
>
> Signed-off-by: E Shattow <e@freeshell.de>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 6bb13af82147..30c5f3487c8b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -245,6 +245,12 @@ emmc_vdd: aldo4 {
>  			};
>  		};
>  	};
> +
> +	eeprom@50 {
> +		compatible = "atmel,24c04";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};

This looks right to me, thanks!

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

[PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[E Shattow]

Add bootph-pre-ram hinting to jh7110-common.dtsi:
  - i2c5_pins and i2c-pins subnode for connection to eeprom
  - eeprom node
  - qspi flash configuration subnode
  - memory node
  - uart0 for serial console

  With this the U-Boot SPL secondary program loader may drop such
  overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 30c5f3487c8b..c9e7ae59ee7c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -28,6 +28,7 @@ chosen {
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0x1 0x0>;
+		bootph-pre-ram;
 	};
 
 	gpio-restart {
@@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
 	};
 
 	eeprom@50 {
+		bootph-pre-ram;
 		compatible = "atmel,24c04";
 		reg = <0x50>;
 		pagesize = <16>;
@@ -323,6 +325,7 @@ &qspi {
 	nor_flash: flash@0 {
 		compatible = "jedec,spi-nor";
 		reg = <0>;
+		bootph-pre-ram;
 		cdns,read-delay = <2>;
 		spi-max-frequency = <100000000>;
 		cdns,tshsl-ns = <1>;
@@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
 	};
 
 	i2c5_pins: i2c5-0 {
+		bootph-pre-ram;
 		i2c-pins {
 			pinmux = <GPIOMUX(19, GPOUT_LOW,
 					      GPOEN_SYS_I2C5_CLK,
@@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
 					      GPOEN_SYS_I2C5_DATA,
 					      GPI_SYS_I2C5_DATA)>;
 			bias-disable; /* external pull-up */
+			bootph-pre-ram;
 			input-enable;
 			input-schmitt-enable;
 		};
@@ -641,6 +646,7 @@ GPOEN_DISABLE,
 };
 
 &uart0 {
+	bootph-pre-ram;
 	clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins>;
-- 
2.47.2

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Hal Feng]

On 2/3/2025 9:37 AM, E Shattow wrote:
> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>   - i2c5_pins and i2c-pins subnode for connection to eeprom
>   - eeprom node
>   - qspi flash configuration subnode
>   - memory node
>   - uart0 for serial console
> 
>   With this the U-Boot SPL secondary program loader may drop such
>   overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
> 
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 30c5f3487c8b..c9e7ae59ee7c 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -28,6 +28,7 @@ chosen {
>  	memory@40000000 {
>  		device_type = "memory";
>  		reg = <0x0 0x40000000 0x1 0x0>;
> +		bootph-pre-ram;
>  	};
>  
>  	gpio-restart {
> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>  	};
>  
>  	eeprom@50 {
> +		bootph-pre-ram;
>  		compatible = "atmel,24c04";
>  		reg = <0x50>;
>  		pagesize = <16>;
> @@ -323,6 +325,7 @@ &qspi {
>  	nor_flash: flash@0 {
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
> +		bootph-pre-ram;
>  		cdns,read-delay = <2>;
>  		spi-max-frequency = <100000000>;
>  		cdns,tshsl-ns = <1>;
> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>  	};
>  
>  	i2c5_pins: i2c5-0 {
> +		bootph-pre-ram;
>  		i2c-pins {
>  			pinmux = <GPIOMUX(19, GPOUT_LOW,
>  					      GPOEN_SYS_I2C5_CLK,
> @@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
>  					      GPOEN_SYS_I2C5_DATA,
>  					      GPI_SYS_I2C5_DATA)>;
>  			bias-disable; /* external pull-up */
> +			bootph-pre-ram;
>  			input-enable;
>  			input-schmitt-enable;
>  		};
> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>  };
>  
>  &uart0 {
> +	bootph-pre-ram;
>  	clock-frequency = <24000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pins>;

What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins, &mmc1_pins, &i2c5?
Why not add "bootph-pre-ram;" for them?

Best regards,
Hal

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Heinrich Schuchardt]

On 2/5/25 08:57, Hal Feng wrote:
> On 2/3/2025 9:37 AM, E Shattow wrote:
>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>    - i2c5_pins and i2c-pins subnode for connection to eeprom
>>    - eeprom node
>>    - qspi flash configuration subnode
>>    - memory node
>>    - uart0 for serial console
>>
>>    With this the U-Boot SPL secondary program loader may drop such
>>    overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
>>
>> Signed-off-by: E Shattow <e@freeshell.de>
>> ---
>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -28,6 +28,7 @@ chosen {
>>   	memory@40000000 {
>>   		device_type = "memory";
>>   		reg = <0x0 0x40000000 0x1 0x0>;
>> +		bootph-pre-ram;
>>   	};
>>   
>>   	gpio-restart {
>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>   	};
>>   
>>   	eeprom@50 {
>> +		bootph-pre-ram;
>>   		compatible = "atmel,24c04";
>>   		reg = <0x50>;
>>   		pagesize = <16>;
>> @@ -323,6 +325,7 @@ &qspi {
>>   	nor_flash: flash@0 {
>>   		compatible = "jedec,spi-nor";
>>   		reg = <0>;
>> +		bootph-pre-ram;
>>   		cdns,read-delay = <2>;
>>   		spi-max-frequency = <100000000>;
>>   		cdns,tshsl-ns = <1>;
>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>   	};
>>   
>>   	i2c5_pins: i2c5-0 {
>> +		bootph-pre-ram;
>>   		i2c-pins {
>>   			pinmux = <GPIOMUX(19, GPOUT_LOW,
>>   					      GPOEN_SYS_I2C5_CLK,
>> @@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
>>   					      GPOEN_SYS_I2C5_DATA,
>>   					      GPI_SYS_I2C5_DATA)>;
>>   			bias-disable; /* external pull-up */
>> +			bootph-pre-ram;
>>   			input-enable;
>>   			input-schmitt-enable;
>>   		};
>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>   };
>>   
>>   &uart0 {
>> +	bootph-pre-ram;
>>   	clock-frequency = <24000000>;
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&uart0_pins>;
> 
> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins, &mmc1_pins, &i2c5?
> Why not add "bootph-pre-ram;" for them?

Would they be needed before relocation of U-Boot to DRAM?

Best regards

Heinrich

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Hal Feng]

On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
> On 2/5/25 08:57, Hal Feng wrote:
>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>    - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>    - eeprom node
>>>    - qspi flash configuration subnode
>>>    - memory node
>>>    - uart0 for serial console
>>>
>>>    With this the U-Boot SPL secondary program loader may drop such
>>>    overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
>>>
>>> Signed-off-by: E Shattow <e@freeshell.de>
>>> ---
>>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>> @@ -28,6 +28,7 @@ chosen {
>>>       memory@40000000 {
>>>           device_type = "memory";
>>>           reg = <0x0 0x40000000 0x1 0x0>;
>>> +        bootph-pre-ram;
>>>       };
>>>         gpio-restart {
>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>       };
>>>         eeprom@50 {
>>> +        bootph-pre-ram;
>>>           compatible = "atmel,24c04";
>>>           reg = <0x50>;
>>>           pagesize = <16>;
>>> @@ -323,6 +325,7 @@ &qspi {
>>>       nor_flash: flash@0 {
>>>           compatible = "jedec,spi-nor";
>>>           reg = <0>;
>>> +        bootph-pre-ram;
>>>           cdns,read-delay = <2>;
>>>           spi-max-frequency = <100000000>;
>>>           cdns,tshsl-ns = <1>;
>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>       };
>>>         i2c5_pins: i2c5-0 {
>>> +        bootph-pre-ram;
>>>           i2c-pins {
>>>               pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>                             GPOEN_SYS_I2C5_CLK,
>>> @@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
>>>                             GPOEN_SYS_I2C5_DATA,
>>>                             GPI_SYS_I2C5_DATA)>;
>>>               bias-disable; /* external pull-up */
>>> +            bootph-pre-ram;
>>>               input-enable;
>>>               input-schmitt-enable;
>>>           };
>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>   };
>>>     &uart0 {
>>> +    bootph-pre-ram;
>>>       clock-frequency = <24000000>;
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&uart0_pins>;
>>
>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins, &mmc1_pins, &i2c5?
>> Why not add "bootph-pre-ram;" for them?
> 
> Would they be needed before relocation of U-Boot to DRAM?

Yeah, they are needed by SPL and they are set in
U-Boot arch/riscv/dts/jh7110-common-u-boot.dtsi.

Best regards,
Hal

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[E Shattow]

On 2/5/25 18:59, Hal Feng wrote:
> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>> On 2/5/25 08:57, Hal Feng wrote:
>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>    - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>    - eeprom node
>>>>    - qspi flash configuration subnode
>>>>    - memory node
>>>>    - uart0 for serial console
>>>>
>>>>    With this the U-Boot SPL secondary program loader may drop such
>>>>    overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
>>>>
>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>> ---
>>>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>> @@ -28,6 +28,7 @@ chosen {
>>>>       memory@40000000 {
>>>>           device_type = "memory";
>>>>           reg = <0x0 0x40000000 0x1 0x0>;
>>>> +        bootph-pre-ram;
>>>>       };
>>>>         gpio-restart {
>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>       };
>>>>         eeprom@50 {
>>>> +        bootph-pre-ram;
>>>>           compatible = "atmel,24c04";
>>>>           reg = <0x50>;
>>>>           pagesize = <16>;
>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>       nor_flash: flash@0 {
>>>>           compatible = "jedec,spi-nor";
>>>>           reg = <0>;
>>>> +        bootph-pre-ram;
>>>>           cdns,read-delay = <2>;
>>>>           spi-max-frequency = <100000000>;
>>>>           cdns,tshsl-ns = <1>;
>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>       };
>>>>         i2c5_pins: i2c5-0 {
>>>> +        bootph-pre-ram;
>>>>           i2c-pins {
>>>>               pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>                             GPOEN_SYS_I2C5_CLK,
>>>> @@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
>>>>                             GPOEN_SYS_I2C5_DATA,
>>>>                             GPI_SYS_I2C5_DATA)>;
>>>>               bias-disable; /* external pull-up */
>>>> +            bootph-pre-ram;
>>>>               input-enable;
>>>>               input-schmitt-enable;
>>>>           };
>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>   };
>>>>     &uart0 {
>>>> +    bootph-pre-ram;
>>>>       clock-frequency = <24000000>;
>>>>       pinctrl-names = "default";
>>>>       pinctrl-0 = <&uart0_pins>;
>>>
>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins, &mmc1_pins, &i2c5?
>>> Why not add "bootph-pre-ram;" for them?
>>
>> Would they be needed before relocation of U-Boot to DRAM?
> 
> Yeah, they are needed by SPL and they are set in
> U-Boot arch/riscv/dts/jh7110-common-u-boot.dtsi.
> 
> Best regards,
> Hal
> 

When I tested on Star64 there was none of those needed to boot. We can
add more bootph-pre-ram as needed but I want to know how to test
(because I did not see any need for these).

How do you test that these are needed?

-E

RE: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Hal Feng]

> On 06.02.25 19:17, E Shattow wrote:
> On 2/5/25 18:59, Hal Feng wrote:
> > On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
> >> On 2/5/25 08:57, Hal Feng wrote:
> >>> On 2/3/2025 9:37 AM, E Shattow wrote:
> >>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
> >>>>    - i2c5_pins and i2c-pins subnode for connection to eeprom
> >>>>    - eeprom node
> >>>>    - qspi flash configuration subnode
> >>>>    - memory node
> >>>>    - uart0 for serial console
> >>>>
> >>>>    With this the U-Boot SPL secondary program loader may drop such
> >>>>    overrides when using dt-rebasing with JH7110 OF_UPSTREAM board
> targets.
> >>>>
> >>>> Signed-off-by: E Shattow <e@freeshell.de>
> >>>> ---
> >>>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
> >>>>   1 file changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
> >>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>> @@ -28,6 +28,7 @@ chosen {
> >>>>       memory@40000000 {
> >>>>           device_type = "memory";
> >>>>           reg = <0x0 0x40000000 0x1 0x0>;
> >>>> +        bootph-pre-ram;
> >>>>       };
> >>>>         gpio-restart {
> >>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
> >>>>       };
> >>>>         eeprom@50 {
> >>>> +        bootph-pre-ram;
> >>>>           compatible = "atmel,24c04";
> >>>>           reg = <0x50>;
> >>>>           pagesize = <16>;
> >>>> @@ -323,6 +325,7 @@ &qspi {
> >>>>       nor_flash: flash@0 {
> >>>>           compatible = "jedec,spi-nor";
> >>>>           reg = <0>;
> >>>> +        bootph-pre-ram;
> >>>>           cdns,read-delay = <2>;
> >>>>           spi-max-frequency = <100000000>;
> >>>>           cdns,tshsl-ns = <1>;
> >>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
> >>>>       };
> >>>>         i2c5_pins: i2c5-0 {
> >>>> +        bootph-pre-ram;
> >>>>           i2c-pins {
> >>>>               pinmux = <GPIOMUX(19, GPOUT_LOW,
> >>>>                             GPOEN_SYS_I2C5_CLK, @@ -413,6 +417,7 @@
> >>>> GPI_SYS_I2C5_CLK)>,
> >>>>                             GPOEN_SYS_I2C5_DATA,
> >>>>                             GPI_SYS_I2C5_DATA)>;
> >>>>               bias-disable; /* external pull-up */
> >>>> +            bootph-pre-ram;
> >>>>               input-enable;
> >>>>               input-schmitt-enable;
> >>>>           };
> >>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
> >>>>   };
> >>>>     &uart0 {
> >>>> +    bootph-pre-ram;
> >>>>       clock-frequency = <24000000>;
> >>>>       pinctrl-names = "default";
> >>>>       pinctrl-0 = <&uart0_pins>;
> >>>
> >>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
> &mmc1_pins, &i2c5?
> >>> Why not add "bootph-pre-ram;" for them?
> >>
> >> Would they be needed before relocation of U-Boot to DRAM?
> >
> > Yeah, they are needed by SPL and they are set in U-Boot
> > arch/riscv/dts/jh7110-common-u-boot.dtsi.
> >
> > Best regards,
> > Hal
> >
> 
> When I tested on Star64 there was none of those needed to boot. We can add
> more bootph-pre-ram as needed but I want to know how to test (because I
> did not see any need for these).
> 
> How do you test that these are needed?

In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard (mmc1) or
QSPI flash (qspi). Also it need to choose the correct DTB by reading EEPROM
mounted on i2c5. To run mmc / i2c drivers, the pin configurations (sysgpio, mmc0/1_pins)
are also needed.

Best regards,
Hal

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[E Shattow]

On 2/6/25 19:01, Hal Feng wrote:
>> On 06.02.25 19:17, E Shattow wrote:
>> On 2/5/25 18:59, Hal Feng wrote:
>>> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>>>> On 2/5/25 08:57, Hal Feng wrote:
>>>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>>>    - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>>>    - eeprom node
>>>>>>    - qspi flash configuration subnode
>>>>>>    - memory node
>>>>>>    - uart0 for serial console
>>>>>>
>>>>>>    With this the U-Boot SPL secondary program loader may drop such
>>>>>>    overrides when using dt-rebasing with JH7110 OF_UPSTREAM board
>> targets.
>>>>>>
>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>> ---
>>>>>>   arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>>>   1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> @@ -28,6 +28,7 @@ chosen {
>>>>>>       memory@40000000 {
>>>>>>           device_type = "memory";
>>>>>>           reg = <0x0 0x40000000 0x1 0x0>;
>>>>>> +        bootph-pre-ram;
>>>>>>       };
>>>>>>         gpio-restart {
>>>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>>>       };
>>>>>>         eeprom@50 {
>>>>>> +        bootph-pre-ram;
>>>>>>           compatible = "atmel,24c04";
>>>>>>           reg = <0x50>;
>>>>>>           pagesize = <16>;
>>>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>>>       nor_flash: flash@0 {
>>>>>>           compatible = "jedec,spi-nor";
>>>>>>           reg = <0>;
>>>>>> +        bootph-pre-ram;
>>>>>>           cdns,read-delay = <2>;
>>>>>>           spi-max-frequency = <100000000>;
>>>>>>           cdns,tshsl-ns = <1>;
>>>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>>>       };
>>>>>>         i2c5_pins: i2c5-0 {
>>>>>> +        bootph-pre-ram;
>>>>>>           i2c-pins {
>>>>>>               pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>>>                             GPOEN_SYS_I2C5_CLK, @@ -413,6 +417,7 @@
>>>>>> GPI_SYS_I2C5_CLK)>,
>>>>>>                             GPOEN_SYS_I2C5_DATA,
>>>>>>                             GPI_SYS_I2C5_DATA)>;
>>>>>>               bias-disable; /* external pull-up */
>>>>>> +            bootph-pre-ram;
>>>>>>               input-enable;
>>>>>>               input-schmitt-enable;
>>>>>>           };
>>>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>>>   };
>>>>>>     &uart0 {
>>>>>> +    bootph-pre-ram;
>>>>>>       clock-frequency = <24000000>;
>>>>>>       pinctrl-names = "default";
>>>>>>       pinctrl-0 = <&uart0_pins>;
>>>>>
>>>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
>> &mmc1_pins, &i2c5?
>>>>> Why not add "bootph-pre-ram;" for them?
>>>>
>>>> Would they be needed before relocation of U-Boot to DRAM?
>>>
>>> Yeah, they are needed by SPL and they are set in U-Boot
>>> arch/riscv/dts/jh7110-common-u-boot.dtsi.
>>>
>>> Best regards,
>>> Hal
>>>
>>
>> When I tested on Star64 there was none of those needed to boot. We can add
>> more bootph-pre-ram as needed but I want to know how to test (because I
>> did not see any need for these).
>>
>> How do you test that these are needed?
> 
> In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard (mmc1) or
> QSPI flash (qspi). Also it need to choose the correct DTB by reading EEPROM
> mounted on i2c5. To run mmc / i2c drivers, the pin configurations (sysgpio, mmc0/1_pins)
> are also needed.
> 
> Best regards,
> Hal

EMMC or SDcard are not possible to boot (via JH7110 boot ROM, distinct
from QSPI boot of U-Boot and later EMMC or SDcard capability) in this
way on some of the boards where transistor logic pairs GPIO0 and GPIO1
both-off or both-on. SDcard boot is officially recommended against as
"not supported" by the StarFive reference documentation, and I suppose
what remains is EMMC boot as valid though I have not heard of any users
for this.

What is the test procedure for EMMC boot, can you explain and I will try
it on the Star64?

-E

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Maud Spierings]

On 2/28/25 8:53 AM, E Shattow wrote:
>
> On 2/6/25 19:01, Hal Feng wrote:
>>> On 06.02.25 19:17, E Shattow wrote:
>>> On 2/5/25 18:59, Hal Feng wrote:
>>>> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>>>>> On 2/5/25 08:57, Hal Feng wrote:
>>>>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>>>>     - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>>>>     - eeprom node
>>>>>>>     - qspi flash configuration subnode
>>>>>>>     - memory node
>>>>>>>     - uart0 for serial console
>>>>>>>
>>>>>>>     With this the U-Boot SPL secondary program loader may drop such
>>>>>>>     overrides when using dt-rebasing with JH7110 OF_UPSTREAM board
>>> targets.
>>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>>> ---
>>>>>>>    arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>>>>    1 file changed, 6 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>> @@ -28,6 +28,7 @@ chosen {
>>>>>>>        memory@40000000 {
>>>>>>>            device_type = "memory";
>>>>>>>            reg = <0x0 0x40000000 0x1 0x0>;
>>>>>>> +        bootph-pre-ram;
>>>>>>>        };
>>>>>>>          gpio-restart {
>>>>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>>>>        };
>>>>>>>          eeprom@50 {
>>>>>>> +        bootph-pre-ram;
>>>>>>>            compatible = "atmel,24c04";
>>>>>>>            reg = <0x50>;
>>>>>>>            pagesize = <16>;
>>>>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>>>>        nor_flash: flash@0 {
>>>>>>>            compatible = "jedec,spi-nor";
>>>>>>>            reg = <0>;
>>>>>>> +        bootph-pre-ram;
>>>>>>>            cdns,read-delay = <2>;
>>>>>>>            spi-max-frequency = <100000000>;
>>>>>>>            cdns,tshsl-ns = <1>;
>>>>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>>>>        };
>>>>>>>          i2c5_pins: i2c5-0 {
>>>>>>> +        bootph-pre-ram;
>>>>>>>            i2c-pins {
>>>>>>>                pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>>>>                              GPOEN_SYS_I2C5_CLK, @@ -413,6 +417,7 @@
>>>>>>> GPI_SYS_I2C5_CLK)>,
>>>>>>>                              GPOEN_SYS_I2C5_DATA,
>>>>>>>                              GPI_SYS_I2C5_DATA)>;
>>>>>>>                bias-disable; /* external pull-up */
>>>>>>> +            bootph-pre-ram;
>>>>>>>                input-enable;
>>>>>>>                input-schmitt-enable;
>>>>>>>            };
>>>>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>>>>    };
>>>>>>>      &uart0 {
>>>>>>> +    bootph-pre-ram;
>>>>>>>        clock-frequency = <24000000>;
>>>>>>>        pinctrl-names = "default";
>>>>>>>        pinctrl-0 = <&uart0_pins>;
>>>>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
>>> &mmc1_pins, &i2c5?
>>>>>> Why not add "bootph-pre-ram;" for them?
>>>>> Would they be needed before relocation of U-Boot to DRAM?
>>>> Yeah, they are needed by SPL and they are set in U-Boot
>>>> arch/riscv/dts/jh7110-common-u-boot.dtsi.
>>>>
>>>> Best regards,
>>>> Hal
>>>>
>>> When I tested on Star64 there was none of those needed to boot. We can add
>>> more bootph-pre-ram as needed but I want to know how to test (because I
>>> did not see any need for these).
>>>
>>> How do you test that these are needed?
>> In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard (mmc1) or
>> QSPI flash (qspi). Also it need to choose the correct DTB by reading EEPROM
>> mounted on i2c5. To run mmc / i2c drivers, the pin configurations (sysgpio, mmc0/1_pins)
>> are also needed.
>>
>> Best regards,
>> Hal
> EMMC or SDcard are not possible to boot (via JH7110 boot ROM, distinct
> from QSPI boot of U-Boot and later EMMC or SDcard capability) in this
> way on some of the boards where transistor logic pairs GPIO0 and GPIO1
> both-off or both-on. SDcard boot is officially recommended against as
> "not supported" by the StarFive reference documentation, and I suppose
> what remains is EMMC boot as valid though I have not heard of any users
> for this.


I tend to use SDcard boot on my deepcomputing fml13v01 board to test

before I flash the firmware to spi flash. I've not encountered issues. I
have not tried eMMC but I assume it is the same.

> What is the test procedure for EMMC boot, can you explain and I will try
> it on the Star64?


I believe this page contains enough info to use it:

https://docs.u-boot.org/en/latest/board/starfive/visionfive2.html

> -E

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[E Shattow]

On 2/28/25 01:54, Maud Spierings wrote:
> 
> On 2/28/25 8:53 AM, E Shattow wrote:
>>
>> On 2/6/25 19:01, Hal Feng wrote:
>>>> On 06.02.25 19:17, E Shattow wrote:
>>>> On 2/5/25 18:59, Hal Feng wrote:
>>>>> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>>>>>> On 2/5/25 08:57, Hal Feng wrote:
>>>>>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>>>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>>>>>     - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>>>>>     - eeprom node
>>>>>>>>     - qspi flash configuration subnode
>>>>>>>>     - memory node
>>>>>>>>     - uart0 for serial console
>>>>>>>>
>>>>>>>>     With this the U-Boot SPL secondary program loader may drop such
>>>>>>>>     overrides when using dt-rebasing with JH7110 OF_UPSTREAM board
>>>> targets.
>>>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>>>> ---
>>>>>>>>    arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>>>>>    1 file changed, 6 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>> @@ -28,6 +28,7 @@ chosen {
>>>>>>>>        memory@40000000 {
>>>>>>>>            device_type = "memory";
>>>>>>>>            reg = <0x0 0x40000000 0x1 0x0>;
>>>>>>>> +        bootph-pre-ram;
>>>>>>>>        };
>>>>>>>>          gpio-restart {
>>>>>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>>>>>        };
>>>>>>>>          eeprom@50 {
>>>>>>>> +        bootph-pre-ram;
>>>>>>>>            compatible = "atmel,24c04";
>>>>>>>>            reg = <0x50>;
>>>>>>>>            pagesize = <16>;
>>>>>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>>>>>        nor_flash: flash@0 {
>>>>>>>>            compatible = "jedec,spi-nor";
>>>>>>>>            reg = <0>;
>>>>>>>> +        bootph-pre-ram;
>>>>>>>>            cdns,read-delay = <2>;
>>>>>>>>            spi-max-frequency = <100000000>;
>>>>>>>>            cdns,tshsl-ns = <1>;
>>>>>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>>>>>        };
>>>>>>>>          i2c5_pins: i2c5-0 {
>>>>>>>> +        bootph-pre-ram;
>>>>>>>>            i2c-pins {
>>>>>>>>                pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>>>>>                              GPOEN_SYS_I2C5_CLK, @@ -413,6
>>>>>>>> +417,7 @@
>>>>>>>> GPI_SYS_I2C5_CLK)>,
>>>>>>>>                              GPOEN_SYS_I2C5_DATA,
>>>>>>>>                              GPI_SYS_I2C5_DATA)>;
>>>>>>>>                bias-disable; /* external pull-up */
>>>>>>>> +            bootph-pre-ram;
>>>>>>>>                input-enable;
>>>>>>>>                input-schmitt-enable;
>>>>>>>>            };
>>>>>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>>>>>    };
>>>>>>>>      &uart0 {
>>>>>>>> +    bootph-pre-ram;
>>>>>>>>        clock-frequency = <24000000>;
>>>>>>>>        pinctrl-names = "default";
>>>>>>>>        pinctrl-0 = <&uart0_pins>;
>>>>>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
>>>> &mmc1_pins, &i2c5?
>>>>>>> Why not add "bootph-pre-ram;" for them?
>>>>>> Would they be needed before relocation of U-Boot to DRAM?
>>>>> Yeah, they are needed by SPL and they are set in U-Boot
>>>>> arch/riscv/dts/jh7110-common-u-boot.dtsi.
>>>>>
>>>>> Best regards,
>>>>> Hal
>>>>>
>>>> When I tested on Star64 there was none of those needed to boot. We
>>>> can add
>>>> more bootph-pre-ram as needed but I want to know how to test (because I
>>>> did not see any need for these).
>>>>
>>>> How do you test that these are needed?
>>> In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard
>>> (mmc1) or
>>> QSPI flash (qspi). Also it need to choose the correct DTB by reading
>>> EEPROM
>>> mounted on i2c5. To run mmc / i2c drivers, the pin configurations
>>> (sysgpio, mmc0/1_pins)
>>> are also needed.
>>>
>>> Best regards,
>>> Hal
>> EMMC or SDcard are not possible to boot (via JH7110 boot ROM, distinct
>> from QSPI boot of U-Boot and later EMMC or SDcard capability) in this
>> way on some of the boards where transistor logic pairs GPIO0 and GPIO1
>> both-off or both-on. SDcard boot is officially recommended against as
>> "not supported" by the StarFive reference documentation, and I suppose
>> what remains is EMMC boot as valid though I have not heard of any users
>> for this.
> 
> 
> I tend to use SDcard boot on my deepcomputing fml13v01 board to test
> 
> before I flash the firmware to spi flash. I've not encountered issues. I
> have not tried eMMC but I assume it is the same.
> 
>> What is the test procedure for EMMC boot, can you explain and I will try
>> it on the Star64?
> 
> 
> I believe this page contains enough info to use it:
> 
> https://docs.u-boot.org/en/latest/board/starfive/visionfive2.html
> 
>> -E

I followed that info to get JH7110 ZBL SD Card boot mode to load U-Boot
by adding only bootph-pre-ram to the &mmc1 node. However, no success for
U-Boot with JH7110 ZBL eMMC boot mode. I additionally tried the build of
U-Boot as the Star64 arrives from factory, and U-Boot release 2025.01,
both which are also working for JH7110 ZBL SD card boot mode and not
JH7110 ZBL eMMC boot mode. Obvious to me is that page does not contain
enough info to test for eMMC, and/or I am doing this wrong.

When no valid eMMC device is present:

dwmci_s: Response Timeout.
BOOT fail,Error is 0xffffffff
dwmci_s: Response Timeout.
BOOT fail,Error is 0xffffffff

With the prepared eMMC storage connected there is not any output.

What is the preparation and test procedure for JH7110 ZBL eMMC boot mode?

-E

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Heinrich Schuchardt]

On 24.04.25 07:15, E Shattow wrote:
> 
> 
> On 2/28/25 01:54, Maud Spierings wrote:
>>
>> On 2/28/25 8:53 AM, E Shattow wrote:
>>>
>>> On 2/6/25 19:01, Hal Feng wrote:
>>>>> On 06.02.25 19:17, E Shattow wrote:
>>>>> On 2/5/25 18:59, Hal Feng wrote:
>>>>>> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>>>>>>> On 2/5/25 08:57, Hal Feng wrote:
>>>>>>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>>>>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>>>>>>      - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>>>>>>      - eeprom node
>>>>>>>>>      - qspi flash configuration subnode
>>>>>>>>>      - memory node
>>>>>>>>>      - uart0 for serial console
>>>>>>>>>
>>>>>>>>>      With this the U-Boot SPL secondary program loader may drop such
>>>>>>>>>      overrides when using dt-rebasing with JH7110 OF_UPSTREAM board
>>>>> targets.
>>>>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>>>>> ---
>>>>>>>>>     arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>>>>>>     1 file changed, 6 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>> @@ -28,6 +28,7 @@ chosen {
>>>>>>>>>         memory@40000000 {
>>>>>>>>>             device_type = "memory";
>>>>>>>>>             reg = <0x0 0x40000000 0x1 0x0>;
>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>         };
>>>>>>>>>           gpio-restart {
>>>>>>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>>>>>>         };
>>>>>>>>>           eeprom@50 {
>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>             compatible = "atmel,24c04";
>>>>>>>>>             reg = <0x50>;
>>>>>>>>>             pagesize = <16>;
>>>>>>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>>>>>>         nor_flash: flash@0 {
>>>>>>>>>             compatible = "jedec,spi-nor";
>>>>>>>>>             reg = <0>;
>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>             cdns,read-delay = <2>;
>>>>>>>>>             spi-max-frequency = <100000000>;
>>>>>>>>>             cdns,tshsl-ns = <1>;
>>>>>>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>>>>>>         };
>>>>>>>>>           i2c5_pins: i2c5-0 {
>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>             i2c-pins {
>>>>>>>>>                 pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>>>>>>                               GPOEN_SYS_I2C5_CLK, @@ -413,6
>>>>>>>>> +417,7 @@
>>>>>>>>> GPI_SYS_I2C5_CLK)>,
>>>>>>>>>                               GPOEN_SYS_I2C5_DATA,
>>>>>>>>>                               GPI_SYS_I2C5_DATA)>;
>>>>>>>>>                 bias-disable; /* external pull-up */
>>>>>>>>> +            bootph-pre-ram;
>>>>>>>>>                 input-enable;
>>>>>>>>>                 input-schmitt-enable;
>>>>>>>>>             };
>>>>>>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>>>>>>     };
>>>>>>>>>       &uart0 {
>>>>>>>>> +    bootph-pre-ram;
>>>>>>>>>         clock-frequency = <24000000>;
>>>>>>>>>         pinctrl-names = "default";
>>>>>>>>>         pinctrl-0 = <&uart0_pins>;
>>>>>>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
>>>>> &mmc1_pins, &i2c5?
>>>>>>>> Why not add "bootph-pre-ram;" for them?
>>>>>>> Would they be needed before relocation of U-Boot to DRAM?
>>>>>> Yeah, they are needed by SPL and they are set in U-Boot
>>>>>> arch/riscv/dts/jh7110-common-u-boot.dtsi.
>>>>>>
>>>>>> Best regards,
>>>>>> Hal
>>>>>>
>>>>> When I tested on Star64 there was none of those needed to boot. We
>>>>> can add
>>>>> more bootph-pre-ram as needed but I want to know how to test (because I
>>>>> did not see any need for these).
>>>>>
>>>>> How do you test that these are needed?
>>>> In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard
>>>> (mmc1) or
>>>> QSPI flash (qspi). Also it need to choose the correct DTB by reading
>>>> EEPROM
>>>> mounted on i2c5. To run mmc / i2c drivers, the pin configurations
>>>> (sysgpio, mmc0/1_pins)
>>>> are also needed.
>>>>
>>>> Best regards,
>>>> Hal
>>> EMMC or SDcard are not possible to boot (via JH7110 boot ROM, distinct
>>> from QSPI boot of U-Boot and later EMMC or SDcard capability) in this
>>> way on some of the boards where transistor logic pairs GPIO0 and GPIO1
>>> both-off or both-on. SDcard boot is officially recommended against as
>>> "not supported" by the StarFive reference documentation, and I suppose
>>> what remains is EMMC boot as valid though I have not heard of any users
>>> for this.
>>
>>
>> I tend to use SDcard boot on my deepcomputing fml13v01 board to test
>>
>> before I flash the firmware to spi flash. I've not encountered issues. I
>> have not tried eMMC but I assume it is the same.
>>
>>> What is the test procedure for EMMC boot, can you explain and I will try
>>> it on the Star64?
>>
>>
>> I believe this page contains enough info to use it:
>>
>> https://docs.u-boot.org/en/latest/board/starfive/visionfive2.html
>>
>>> -E
> 
> I followed that info to get JH7110 ZBL SD Card boot mode to load U-Boot
> by adding only bootph-pre-ram to the &mmc1 node. However, no success for
> U-Boot with JH7110 ZBL eMMC boot mode. I additionally tried the build of
> U-Boot as the Star64 arrives from factory, and U-Boot release 2025.01,
> both which are also working for JH7110 ZBL SD card boot mode and not
> JH7110 ZBL eMMC boot mode. Obvious to me is that page does not contain
> enough info to test for eMMC, and/or I am doing this wrong.
> 
> When no valid eMMC device is present:
> 
> dwmci_s: Response Timeout.
> BOOT fail,Error is 0xffffffff
> dwmci_s: Response Timeout.
> BOOT fail,Error is 0xffffffff
> 
> With the prepared eMMC storage connected there is not any output.
> 
> What is the preparation and test procedure for JH7110 ZBL eMMC boot mode?
> 
> -E

Hello E,

I have tested on the StarFive VisionFive 2.

You can write U-Boot SPL to sector 0 of the eMMC and it will be booted. 
But with our code it will not find main U-Boot. (There seems to be a bug 
in our SPL code as it does not fall back to Y-modem.)

To allow adding a GPT table the vendor image therefore uses the 
fall-back to the backup SPL as follows.

With u-boot-spl.bin.normal.out starting at sector 4096 (byte 0x200000) 
add the following bytes to the GPT table:

Position 0x0000: 40 02 00 00  00 00 20 00
Position 0x0290: 40 02 00 00  00 00 20 00

The value 0x00000240 is the offset to the SPL header.
The value 0x00200000 is the position of the backup SPL.

Likewise with u-boot-spl.bin.normal.out at sector 2048 (byte 0x100000) use:

Position 0x0000: 40 02 00 00  00 00 10 00
Position 0x0290: 40 02 00 00  00 00 10 00

Best regards

Heinrich

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[E Shattow]

On 4/24/25 03:54, Heinrich Schuchardt wrote:
> On 24.04.25 07:15, E Shattow wrote:
>>
>>
>> On 2/28/25 01:54, Maud Spierings wrote:
>>>
>>> On 2/28/25 8:53 AM, E Shattow wrote:
>>>>
>>>> On 2/6/25 19:01, Hal Feng wrote:
>>>>>> On 06.02.25 19:17, E Shattow wrote:
>>>>>> On 2/5/25 18:59, Hal Feng wrote:
>>>>>>> On 2/5/2025 6:01 PM, Heinrich Schuchardt wrote:
>>>>>>>> On 2/5/25 08:57, Hal Feng wrote:
>>>>>>>>> On 2/3/2025 9:37 AM, E Shattow wrote:
>>>>>>>>>> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>>>>>>>>>>      - i2c5_pins and i2c-pins subnode for connection to eeprom
>>>>>>>>>>      - eeprom node
>>>>>>>>>>      - qspi flash configuration subnode
>>>>>>>>>>      - memory node
>>>>>>>>>>      - uart0 for serial console
>>>>>>>>>>
>>>>>>>>>>      With this the U-Boot SPL secondary program loader may
>>>>>>>>>> drop such
>>>>>>>>>>      overrides when using dt-rebasing with JH7110 OF_UPSTREAM
>>>>>>>>>> board
>>>>>> targets.
>>>>>>>>>> Signed-off-by: E Shattow <e@freeshell.de>
>>>>>>>>>> ---
>>>>>>>>>>     arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>>>>>>>>>>     1 file changed, 6 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>>> b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>>> index 30c5f3487c8b..c9e7ae59ee7c 100644
>>>>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>>>>>> @@ -28,6 +28,7 @@ chosen {
>>>>>>>>>>         memory@40000000 {
>>>>>>>>>>             device_type = "memory";
>>>>>>>>>>             reg = <0x0 0x40000000 0x1 0x0>;
>>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>>         };
>>>>>>>>>>           gpio-restart {
>>>>>>>>>> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>>>>>>>>>>         };
>>>>>>>>>>           eeprom@50 {
>>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>>             compatible = "atmel,24c04";
>>>>>>>>>>             reg = <0x50>;
>>>>>>>>>>             pagesize = <16>;
>>>>>>>>>> @@ -323,6 +325,7 @@ &qspi {
>>>>>>>>>>         nor_flash: flash@0 {
>>>>>>>>>>             compatible = "jedec,spi-nor";
>>>>>>>>>>             reg = <0>;
>>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>>             cdns,read-delay = <2>;
>>>>>>>>>>             spi-max-frequency = <100000000>;
>>>>>>>>>>             cdns,tshsl-ns = <1>;
>>>>>>>>>> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>>>>>>>>>>         };
>>>>>>>>>>           i2c5_pins: i2c5-0 {
>>>>>>>>>> +        bootph-pre-ram;
>>>>>>>>>>             i2c-pins {
>>>>>>>>>>                 pinmux = <GPIOMUX(19, GPOUT_LOW,
>>>>>>>>>>                               GPOEN_SYS_I2C5_CLK, @@ -413,6
>>>>>>>>>> +417,7 @@
>>>>>>>>>> GPI_SYS_I2C5_CLK)>,
>>>>>>>>>>                               GPOEN_SYS_I2C5_DATA,
>>>>>>>>>>                               GPI_SYS_I2C5_DATA)>;
>>>>>>>>>>                 bias-disable; /* external pull-up */
>>>>>>>>>> +            bootph-pre-ram;
>>>>>>>>>>                 input-enable;
>>>>>>>>>>                 input-schmitt-enable;
>>>>>>>>>>             };
>>>>>>>>>> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>>>>>>>>>>     };
>>>>>>>>>>       &uart0 {
>>>>>>>>>> +    bootph-pre-ram;
>>>>>>>>>>         clock-frequency = <24000000>;
>>>>>>>>>>         pinctrl-names = "default";
>>>>>>>>>>         pinctrl-0 = <&uart0_pins>;
>>>>>>>>> What about &mmc0, &mmc1, &qspi, &sysgpio, &mmc0_pins,
>>>>>> &mmc1_pins, &i2c5?
>>>>>>>>> Why not add "bootph-pre-ram;" for them?
>>>>>>>> Would they be needed before relocation of U-Boot to DRAM?
>>>>>>> Yeah, they are needed by SPL and they are set in U-Boot
>>>>>>> arch/riscv/dts/jh7110-common-u-boot.dtsi.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Hal
>>>>>>>
>>>>>> When I tested on Star64 there was none of those needed to boot. We
>>>>>> can add
>>>>>> more bootph-pre-ram as needed but I want to know how to test
>>>>>> (because I
>>>>>> did not see any need for these).
>>>>>>
>>>>>> How do you test that these are needed?
>>>>> In my opinion, SPL need to read U-Boot from EMMC (mmc0) or SDcard
>>>>> (mmc1) or
>>>>> QSPI flash (qspi). Also it need to choose the correct DTB by reading
>>>>> EEPROM
>>>>> mounted on i2c5. To run mmc / i2c drivers, the pin configurations
>>>>> (sysgpio, mmc0/1_pins)
>>>>> are also needed.
>>>>>
>>>>> Best regards,
>>>>> Hal
>>>> EMMC or SDcard are not possible to boot (via JH7110 boot ROM, distinct
>>>> from QSPI boot of U-Boot and later EMMC or SDcard capability) in this
>>>> way on some of the boards where transistor logic pairs GPIO0 and GPIO1
>>>> both-off or both-on. SDcard boot is officially recommended against as
>>>> "not supported" by the StarFive reference documentation, and I suppose
>>>> what remains is EMMC boot as valid though I have not heard of any users
>>>> for this.
>>>
>>>
>>> I tend to use SDcard boot on my deepcomputing fml13v01 board to test
>>>
>>> before I flash the firmware to spi flash. I've not encountered issues. I
>>> have not tried eMMC but I assume it is the same.
>>>
>>>> What is the test procedure for EMMC boot, can you explain and I will
>>>> try
>>>> it on the Star64?
>>>
>>>
>>> I believe this page contains enough info to use it:
>>>
>>> https://docs.u-boot.org/en/latest/board/starfive/visionfive2.html
>>>
>>>> -E
>>
>> I followed that info to get JH7110 ZBL SD Card boot mode to load U-Boot
>> by adding only bootph-pre-ram to the &mmc1 node. However, no success for
>> U-Boot with JH7110 ZBL eMMC boot mode. I additionally tried the build of
>> U-Boot as the Star64 arrives from factory, and U-Boot release 2025.01,
>> both which are also working for JH7110 ZBL SD card boot mode and not
>> JH7110 ZBL eMMC boot mode. Obvious to me is that page does not contain
>> enough info to test for eMMC, and/or I am doing this wrong.
>>
>> When no valid eMMC device is present:
>>
>> dwmci_s: Response Timeout.
>> BOOT fail,Error is 0xffffffff
>> dwmci_s: Response Timeout.
>> BOOT fail,Error is 0xffffffff
>>
>> With the prepared eMMC storage connected there is not any output.
>>
>> What is the preparation and test procedure for JH7110 ZBL eMMC boot mode?
>>
>> -E
> 
> Hello E,
> 
> I have tested on the StarFive VisionFive 2.
> 
> You can write U-Boot SPL to sector 0 of the eMMC and it will be booted.
> But with our code it will not find main U-Boot. (There seems to be a bug
> in our SPL code as it does not fall back to Y-modem.)
> 
> To allow adding a GPT table the vendor image therefore uses the fall-
> back to the backup SPL as follows.
> 
> With u-boot-spl.bin.normal.out starting at sector 4096 (byte 0x200000)
> add the following bytes to the GPT table:
> 

> Position 0x0000: 40 02 00 00  00 00 20 00

Position 0x0000 is within the executable code area of standard (or
protective) MBR, and so GPT is not affected.

What about SD-card, does JH7110 loader code in MaskROM follow or ignore
this treatment?

> Position 0x0290: 40 02 00 00  00 00 20 00

Position 0x0290 is within the "must be zeroes for the rest of the block"
area of GPT header when sector size is 512 bytes. For eMMC we're working
with 512byte sectors? If eMMC was written with standard MBR then what is
this, just assumed to be somewhere inside the first partition? How does
StarFive create distro images i.e.
starfive-jh7110-202302-eMMC-minimal-desktop.img [1] for eMMC with these
prepared values, is this documented officially or in the StarFive git
repo history?

> 
> The value 0x00000240 is the offset to the SPL header.

> The value 0x00200000 is the position of the backup SPL.
> 
> Likewise with u-boot-spl.bin.normal.out at sector 2048 (byte 0x100000) use:
> 
> Position 0x0000: 40 02 00 00  00 00 10 00
> Position 0x0290: 40 02 00 00  00 00 10 00
> 
> Best regards
> 
> Heinrich

Thanks, Heinrich, helpful to know I was missing information. It does not
answer for me the question of what the logic is in the JH7110 boot code
for forming a correct test methodology. Anyway I learn a lot from this
analysis and it is much appreciated!

Hal - I would like (and/or from anyone representing StarFive) some code
listing of the JH7110 CPU MaskROM on what the SD-card and eMMC boot code
 is actually doing ... if you want this supported or you should describe
the logic flow and magic numbers, offset locations, etc. expected so I
can properly test for it. Aside, there is also a promise from 2023
January to release the supporting code for jh7110-recovery.bin SPL
utility for writing SPI NOR flash and eMMC contents [2] that should be
considered... where is the code and documentation to do what you want done?

-E

1: https://rvspace.org/en/project/VisionFive2_Debian_Wiki_202302_Release
2: https://github.com/starfive-tech/Tools/issues/2

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Emil Renner Berthing]

E Shattow wrote:
> Add bootph-pre-ram hinting to jh7110-common.dtsi:
>   - i2c5_pins and i2c-pins subnode for connection to eeprom
>   - eeprom node
>   - qspi flash configuration subnode
>   - memory node
>   - uart0 for serial console
>
>   With this the U-Boot SPL secondary program loader may drop such
>   overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
>
> Signed-off-by: E Shattow <e@freeshell.de>

I can't find the booph-pre-ram property in any of the schemas in
Documentation/devicetree/bindings. Does make ARCH=riscv <..> dtbs_check not
complain about this?

/Emil

> ---
>  arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> index 30c5f3487c8b..c9e7ae59ee7c 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -28,6 +28,7 @@ chosen {
>  	memory@40000000 {
>  		device_type = "memory";
>  		reg = <0x0 0x40000000 0x1 0x0>;
> +		bootph-pre-ram;
>  	};
>
>  	gpio-restart {
> @@ -247,6 +248,7 @@ emmc_vdd: aldo4 {
>  	};
>
>  	eeprom@50 {
> +		bootph-pre-ram;
>  		compatible = "atmel,24c04";
>  		reg = <0x50>;
>  		pagesize = <16>;
> @@ -323,6 +325,7 @@ &qspi {
>  	nor_flash: flash@0 {
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
> +		bootph-pre-ram;
>  		cdns,read-delay = <2>;
>  		spi-max-frequency = <100000000>;
>  		cdns,tshsl-ns = <1>;
> @@ -405,6 +408,7 @@ GPOEN_SYS_I2C2_DATA,
>  	};
>
>  	i2c5_pins: i2c5-0 {
> +		bootph-pre-ram;
>  		i2c-pins {
>  			pinmux = <GPIOMUX(19, GPOUT_LOW,
>  					      GPOEN_SYS_I2C5_CLK,
> @@ -413,6 +417,7 @@ GPI_SYS_I2C5_CLK)>,
>  					      GPOEN_SYS_I2C5_DATA,
>  					      GPI_SYS_I2C5_DATA)>;
>  			bias-disable; /* external pull-up */
> +			bootph-pre-ram;
>  			input-enable;
>  			input-schmitt-enable;
>  		};
> @@ -641,6 +646,7 @@ GPOEN_DISABLE,
>  };
>
>  &uart0 {
> +	bootph-pre-ram;
>  	clock-frequency = <24000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_pins>;
> --
> 2.47.2
>

Re: [PATCH v2 5/5] riscv: dts: starfive: jh7110-common: bootph-pre-ram hinting needed by boot loader

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

On Wed, Feb 05, 2025 at 02:35:42AM -0800, Emil Renner Berthing wrote:
> E Shattow wrote:
> > Add bootph-pre-ram hinting to jh7110-common.dtsi:
> >   - i2c5_pins and i2c-pins subnode for connection to eeprom
> >   - eeprom node
> >   - qspi flash configuration subnode
> >   - memory node
> >   - uart0 for serial console
> >
> >   With this the U-Boot SPL secondary program loader may drop such
> >   overrides when using dt-rebasing with JH7110 OF_UPSTREAM board targets.
> >
> > Signed-off-by: E Shattow <e@freeshell.de>
> 
> I can't find the booph-pre-ram property in any of the schemas in
> Documentation/devicetree/bindings. Does make ARCH=riscv <..> dtbs_check not
> complain about this?

They come directly from dt-schema:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/bootph.yaml

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]