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

[PATCH v1 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 JH7110 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.

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      | 29 +++++++++++++++----
 1 file changed, 24 insertions(+), 5 deletions(-)


base-commit: 708d55db3edbe2ccf88d94b5f2e2b404bc0ba37c
-- 
2.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[E Shattow]

Replace syscrg assignments of clocks, clock parents, and rates, 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 | 12 +++++++++---
 1 file changed, 9 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..55c6743100a7 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -359,9 +359,15 @@ 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>;
+	assigned-clock-rates = <0>, <0>, <0>, <0>;
 };
 
 &sysgpio {
-- 
2.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 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>
---
 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 55c6743100a7..651f9a602226 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.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 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 651f9a602226..bf2f0c34ad4e 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -636,6 +636,7 @@ GPOEN_DISABLE,
 };
 
 &uart0 {
+	clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
-- 
2.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 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>
---
 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 bf2f0c34ad4e..ad5cb85ebc59 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: eeprom@50 {
+		compatible = "atmel,24c04";
+		reg = <0x50>;
+		pagesize = <16>;
+	};
 };
 
 &i2c6 {
-- 
2.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v1 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 ad5cb85ebc59..99de0750d049 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: 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>;
@@ -406,6 +409,7 @@ GPOEN_SYS_I2C2_DATA,
 	};
 
 	i2c5_pins: i2c5-0 {
+		bootph-pre-ram;
 		i2c-pins {
 			pinmux = <GPIOMUX(19, GPOUT_LOW,
 					      GPOEN_SYS_I2C5_CLK,
@@ -414,6 +418,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;
 		};
@@ -642,6 +647,7 @@ GPOEN_DISABLE,
 };
 
 &uart0 {
+	bootph-pre-ram;
 	clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins>;
-- 
2.45.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1440 bytes --]

On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
> Replace syscrg assignments of clocks, clock parents, and rates, 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 | 12 +++++++++---
>  1 file changed, 9 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..55c6743100a7 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> @@ -359,9 +359,15 @@ 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>;
> +	assigned-clock-rates = <0>, <0>, <0>, <0>;

Why is assigned rates here 0s, rather than the property just removed?

>  };
>  
>  &sysgpio {
> -- 
> 2.45.2
> 

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[E Shattow]

Hi, Conor  (added CC: Minda Chen, Hal Feng)

On 1/4/25 10:33, Conor Dooley wrote:
> On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
>> Replace syscrg assignments of clocks, clock parents, and rates, 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 | 12 +++++++++---
>>   1 file changed, 9 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..55c6743100a7 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>> @@ -359,9 +359,15 @@ 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>;
>> +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> 
> Why is assigned rates here 0s, rather than the property just removed?
> 
>>   };
>>   
>>   &sysgpio {
>> -- 
>> 2.45.2
>>

Assigned rates all zeroes is how it is in U-Boot. Removing the 
assigned-clock-rates property as suggested does work in U-Boot and Linux 
both.

For context, U-Boot fails when replacing assigned-clocks to 
JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT (1500MHz) 
from Linux. So I tried to merge all properties together and in testing 
then U-Boot failed (or I did it wrong). However replacing the Linux 
properties with the U-Boot configuration (above) on Linux does work for 
both.

I do not know if this is correct but I can test any suggestions and 
report if they are working.

Do these changes make sense? Are there other variations I should test?

Thanks,

-E

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2615 bytes --]

On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote:
> Hi, Conor  (added CC: Minda Chen, Hal Feng)
> 
> On 1/4/25 10:33, Conor Dooley wrote:
> > On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
> > > Replace syscrg assignments of clocks, clock parents, and rates, 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 | 12 +++++++++---
> > >   1 file changed, 9 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..55c6743100a7 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > @@ -359,9 +359,15 @@ 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>;
> > > +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> > 
> > Why is assigned rates here 0s, rather than the property just removed?
> > 
> > >   };
> > >   &sysgpio {
> > > -- 
> > > 2.45.2
> > > 
> 
> Assigned rates all zeroes is how it is in U-Boot. Removing the
> assigned-clock-rates property as suggested does work in U-Boot and Linux
> both.
> 
> For context, U-Boot fails when replacing assigned-clocks to
> JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT (1500MHz) from
> Linux. So I tried to merge all properties together and in testing then
> U-Boot failed (or I did it wrong). However replacing the Linux properties
> with the U-Boot configuration (above) on Linux does work for both.
> 
> I do not know if this is correct but I can test any suggestions and report
> if they are working.
> 
> Do these changes make sense? Are there other variations I should test?

I'd like the commit message to at least explain why these clocks need to
be set to zero (I assume that means disabled?). Maybe the StarFive folks
know why it is required?

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Hal Feng]

On 1/3/2025 3:45 AM, 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.
> 
> Signed-off-by: E Shattow <e@freeshell.de>
> ---
>  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 55c6743100a7..651f9a602226 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>;

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Hal Feng]

On 1/3/2025 3:45 AM, 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>
> ---
>  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 bf2f0c34ad4e..ad5cb85ebc59 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: eeprom@50 {

The label "eeprom:" can be dropped if no one use it.

> +		compatible = "atmel,24c04";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
>  };
>  
>  &i2c6 {

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Hal Feng]

> On 07.01.25 04:08, Conor Dooley wrote: 
> On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote:
> > Hi, Conor  (added CC: Minda Chen, Hal Feng)
> >
> > On 1/4/25 10:33, Conor Dooley wrote:
> > > On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
> > > > Replace syscrg assignments of clocks, clock parents, and rates,
> > > > 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 | 12 +++++++++---
> > > >   1 file changed, 9 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..55c6743100a7 100644
> > > > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > > @@ -359,9 +359,15 @@ 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>;
> > > > +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> > >
> > > Why is assigned rates here 0s, rather than the property just removed?
> > >
> > > >   };
> > > >   &sysgpio {
> > > > --
> > > > 2.45.2
> > > >
> >
> > Assigned rates all zeroes is how it is in U-Boot. Removing the
> > assigned-clock-rates property as suggested does work in U-Boot and
> > Linux both.
> >
> > For context, U-Boot fails when replacing assigned-clocks to
> > JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT
> (1500MHz)
> > from Linux. So I tried to merge all properties together and in testing
> > then U-Boot failed (or I did it wrong). However replacing the Linux
> > properties with the U-Boot configuration (above) on Linux does work for
> both.
> >
> > I do not know if this is correct but I can test any suggestions and
> > report if they are working.
> >
> > Do these changes make sense? Are there other variations I should test?
> 
> I'd like the commit message to at least explain why these clocks need to be
> set to zero (I assume that means disabled?). Maybe the StarFive folks know
> why it is required?

Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates.
You can refer to
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml/

Linux here setting JH7110_SYSCLK_CPU_CORE to 500MHz and JH7110_PLLCLK_PLL0_OUT
to 1500MHz are for increasing the CPU frequency to 1500MHz.

VF2 u-boot is still running at 1000MHz now. You failed to set JH7110_PLLCLK_PLL0_OUT
to 1500MHz, because CPU power supply voltage needs to be increased before running at
1500MHz.

I think a better choice now is changing Linux device tree as follows:

&syscrg {
	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
			  <&syscrg JH7110_SYSCLK_QSPI_REF>,
			  <&syscrg JH7110_SYSCLK_CPU_CORE>,
			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1500000000>;
};

For u-boot, if there is no requirement to run u-boot at 1500MHz, just keep
&syscrg {
	assigned-clock-rates = <0>, <0>, <0>, <0>;
};
in u-boot device tree. If we need running 1500MHz in u-boot, I will send a patch
to implement it and then &syscrg{...} in u-boot device tree can be dropped.

Best regards,
Hal

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4871 bytes --]

On Wed, Jan 15, 2025 at 06:33:08AM +0000, Hal Feng wrote:
> > On 07.01.25 04:08, Conor Dooley wrote: 
> > On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote:
> > > Hi, Conor  (added CC: Minda Chen, Hal Feng)
> > >
> > > On 1/4/25 10:33, Conor Dooley wrote:
> > > > On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
> > > > > Replace syscrg assignments of clocks, clock parents, and rates,
> > > > > 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 | 12 +++++++++---
> > > > >   1 file changed, 9 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..55c6743100a7 100644
> > > > > --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > > > +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> > > > > @@ -359,9 +359,15 @@ 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>;
> > > > > +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> > > >
> > > > Why is assigned rates here 0s, rather than the property just removed?
> > > >
> > > > >   };
> > > > >   &sysgpio {
> > > > > --
> > > > > 2.45.2
> > > > >
> > >
> > > Assigned rates all zeroes is how it is in U-Boot. Removing the
> > > assigned-clock-rates property as suggested does work in U-Boot and
> > > Linux both.
> > >
> > > For context, U-Boot fails when replacing assigned-clocks to
> > > JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT
> > (1500MHz)
> > > from Linux. So I tried to merge all properties together and in testing
> > > then U-Boot failed (or I did it wrong). However replacing the Linux
> > > properties with the U-Boot configuration (above) on Linux does work for
> > both.
> > >
> > > I do not know if this is correct but I can test any suggestions and
> > > report if they are working.
> > >
> > > Do these changes make sense? Are there other variations I should test?
> > 
> > I'd like the commit message to at least explain why these clocks need to be
> > set to zero (I assume that means disabled?). Maybe the StarFive folks know
> > why it is required?
> 
> Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates.
> You can refer to
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml/

If you check the wording there, it says:
  To skip setting parent or rate of a clock its corresponding entry should be
  set to 0, or can be omitted if it is not followed by any non-zero entry.
Since all clocks are being set to 0 above, we should be in the "can be
omitted" case for the entire property, no? That would...

> Linux here setting JH7110_SYSCLK_CPU_CORE to 500MHz and JH7110_PLLCLK_PLL0_OUT
> to 1500MHz are for increasing the CPU frequency to 1500MHz.
> 
> VF2 u-boot is still running at 1000MHz now. You failed to set JH7110_PLLCLK_PLL0_OUT
> to 1500MHz, because CPU power supply voltage needs to be increased before running at
> 1500MHz.
> 
> I think a better choice now is changing Linux device tree as follows:
> 
> &syscrg {
> 	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
> 			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
> 			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
> 			  <&syscrg JH7110_SYSCLK_QSPI_REF>,
> 			  <&syscrg JH7110_SYSCLK_CPU_CORE>,
> 			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
> 	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
> 				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
> 	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1500000000>;
> };

...make this one a reasonable change...

> For u-boot, if there is no requirement to run u-boot at 1500MHz, just keep
> &syscrg {
> 	assigned-clock-rates = <0>, <0>, <0>, <0>;
> };

...but not this one.

> in u-boot device tree. If we need running 1500MHz in u-boot, I will send a patch
> to implement it and then &syscrg{...} in u-boot device tree can be dropped.
> 
> Best regards,
> Hal

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[E Shattow]

On 1/15/25 01:35, Conor Dooley wrote:
> On Wed, Jan 15, 2025 at 06:33:08AM +0000, Hal Feng wrote:
>>> On 07.01.25 04:08, Conor Dooley wrote:
>>> On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote:
>>>> Hi, Conor  (added CC: Minda Chen, Hal Feng)
>>>>
>>>> On 1/4/25 10:33, Conor Dooley wrote:
>>>>> On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
>>>>>> Replace syscrg assignments of clocks, clock parents, and rates,
>>>>>> 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 | 12 +++++++++---
>>>>>>    1 file changed, 9 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..55c6743100a7 100644
>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
>>>>>> @@ -359,9 +359,15 @@ 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>;
>>>>>> +	assigned-clock-rates = <0>, <0>, <0>, <0>;
>>>>>
>>>>> Why is assigned rates here 0s, rather than the property just removed?
>>>>>
>>>>>>    };
>>>>>>    &sysgpio {
>>>>>> --
>>>>>> 2.45.2
>>>>>>
>>>>
>>>> Assigned rates all zeroes is how it is in U-Boot. Removing the
>>>> assigned-clock-rates property as suggested does work in U-Boot and
>>>> Linux both.
>>>>
>>>> For context, U-Boot fails when replacing assigned-clocks to
>>>> JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT
>>> (1500MHz)
>>>> from Linux. So I tried to merge all properties together and in testing
>>>> then U-Boot failed (or I did it wrong). However replacing the Linux
>>>> properties with the U-Boot configuration (above) on Linux does work for
>>> both.
>>>>
>>>> I do not know if this is correct but I can test any suggestions and
>>>> report if they are working.
>>>>
>>>> Do these changes make sense? Are there other variations I should test?
>>>
>>> I'd like the commit message to at least explain why these clocks need to be
>>> set to zero (I assume that means disabled?). Maybe the StarFive folks know
>>> why it is required?
>>
>> Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates.
>> You can refer to
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml/
> 
> If you check the wording there, it says:
>    To skip setting parent or rate of a clock its corresponding entry should be
>    set to 0, or can be omitted if it is not followed by any non-zero entry.
> Since all clocks are being set to 0 above, we should be in the "can be
> omitted" case for the entire property, no? That would...
> 
>> Linux here setting JH7110_SYSCLK_CPU_CORE to 500MHz and JH7110_PLLCLK_PLL0_OUT
>> to 1500MHz are for increasing the CPU frequency to 1500MHz.
>>
>> VF2 u-boot is still running at 1000MHz now. You failed to set JH7110_PLLCLK_PLL0_OUT
>> to 1500MHz, because CPU power supply voltage needs to be increased before running at
>> 1500MHz.

Conor / Hal,  how does this work:

Are these the minimum and maximum values for CPU frequency, CPU CORE and 
PLL0 OUT ?

>>
>> I think a better choice now is changing Linux device tree as follows:
>>
>> &syscrg {
>> 	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
>> 			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
>> 			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
>> 			  <&syscrg JH7110_SYSCLK_QSPI_REF>,
>> 			  <&syscrg JH7110_SYSCLK_CPU_CORE>,
>> 			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
>> 	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
>> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
>> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
>> 				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
>> 	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1500000000>;
>> };
> 
> ...make this one a reasonable change...
> 
>> For u-boot, if there is no requirement to run u-boot at 1500MHz, just keep
>> &syscrg {
>> 	assigned-clock-rates = <0>, <0>, <0>, <0>;
>> };
> 
> ...but not this one.
> 
>> in u-boot device tree. If we need running 1500MHz in u-boot, I will send a patch
>> to implement it and then &syscrg{...} in u-boot device tree can be dropped.
>>
>> Best regards,
>> Hal

Hal,

What voltage (and speed) has less heat output?

I think it must be configured to be *reliable first* as it is pictured 
being a developer board on a person's desk (with no extra accessory - no 
fans, heatsink, etc.) and for example doing work as of a Linux kernel 
compile with using all cores.

We should not force a configuration that will fail for the common 
situation. What voltage and clock speed is the best for continuous 
reliable operation of a JH7110 CPU with no added heatsink or fan and at 
ambient temperature?

Does CPU frequency range need to be given in devicetree? Can instead 
this be done with userspace tool or Linux subsystem at runtime?

The thermal dissipation of JH7110 CPU will be depend on the board layout 
and physical application. So, my opinion here:  I think any minimum and 
maximum CPU frequency limit that is given by manufacturer recommendation 
of safe continuous operation is okay for dts include. If some part of 
this frequency range is not reliable (from undervolt, or overheat) then 
it is a "no" from me in Linux and in U-Boot. The fact of failing to boot 
(in U-Boot) because JH7110 CPU default voltage is not high enough for 
1500MHz operation tells me this is a wrong idea or there is missing code 
in U-Boot to select a default sane clock speed and voltage.

-E

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Hal Feng]

> On 24.01.25 18:46, E Shattow wrote:
> On 1/15/25 01:35, Conor Dooley wrote:
> > On Wed, Jan 15, 2025 at 06:33:08AM +0000, Hal Feng wrote:
> >>> On 07.01.25 04:08, Conor Dooley wrote:
> >>> On Sat, Jan 04, 2025 at 01:04:30PM -0800, E Shattow wrote:
> >>>> Hi, Conor  (added CC: Minda Chen, Hal Feng)
> >>>>
> >>>> On 1/4/25 10:33, Conor Dooley wrote:
> >>>>> On Thu, Jan 02, 2025 at 11:45:07AM -0800, E Shattow wrote:
> >>>>>> Replace syscrg assignments of clocks, clock parents, and rates,
> >>>>>> 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 | 12 +++++++++--
> -
> >>>>>>    1 file changed, 9 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..55c6743100a7 100644
> >>>>>> --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
> >>>>>> @@ -359,9 +359,15 @@ 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>;
> >>>>>> +	assigned-clock-rates = <0>, <0>, <0>, <0>;
> >>>>>
> >>>>> Why is assigned rates here 0s, rather than the property just removed?
> >>>>>
> >>>>>>    };
> >>>>>>    &sysgpio {
> >>>>>> --
> >>>>>> 2.45.2
> >>>>>>
> >>>>
> >>>> Assigned rates all zeroes is how it is in U-Boot. Removing the
> >>>> assigned-clock-rates property as suggested does work in U-Boot and
> >>>> Linux both.
> >>>>
> >>>> For context, U-Boot fails when replacing assigned-clocks to
> >>>> JH7110_SYSCLK_CPU_CORE (500MHz) and JH7110_PLLCLK_PLL0_OUT
> >>> (1500MHz)
> >>>> from Linux. So I tried to merge all properties together and in
> >>>> testing then U-Boot failed (or I did it wrong). However replacing
> >>>> the Linux properties with the U-Boot configuration (above) on Linux
> >>>> does work for
> >>> both.
> >>>>
> >>>> I do not know if this is correct but I can test any suggestions and
> >>>> report if they are working.
> >>>>
> >>>> Do these changes make sense? Are there other variations I should test?
> >>>
> >>> I'd like the commit message to at least explain why these clocks
> >>> need to be set to zero (I assume that means disabled?). Maybe the
> >>> StarFive folks know why it is required?
> >>
> >> Here "assigned-clock-rates = <0>, ..." means skipping setting clock rates.
> >> You can refer to
> >> https://github.com/devicetree-org/dt-
> schema/blob/main/dtschema/schema
> >> s/clock/clock.yaml/
> >
> > If you check the wording there, it says:
> >    To skip setting parent or rate of a clock its corresponding entry should be
> >    set to 0, or can be omitted if it is not followed by any non-zero entry.
> > Since all clocks are being set to 0 above, we should be in the "can be
> > omitted" case for the entire property, no? That would...
> >
> >> Linux here setting JH7110_SYSCLK_CPU_CORE to 500MHz and
> >> JH7110_PLLCLK_PLL0_OUT to 1500MHz are for increasing the CPU
> frequency to 1500MHz.
> >>
> >> VF2 u-boot is still running at 1000MHz now. You failed to set
> >> JH7110_PLLCLK_PLL0_OUT to 1500MHz, because CPU power supply
> voltage
> >> needs to be increased before running at 1500MHz.
> 
> Conor / Hal,  how does this work:
> 
> Are these the minimum and maximum values for CPU frequency, CPU CORE
> and
> PLL0 OUT ?
> 
> >>
> >> I think a better choice now is changing Linux device tree as follows:
> >>
> >> &syscrg {
> >> 	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
> >> 			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
> >> 			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
> >> 			  <&syscrg JH7110_SYSCLK_QSPI_REF>,
> >> 			  <&syscrg JH7110_SYSCLK_CPU_CORE>,
> >> 			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
> >> 	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
> >> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
> >> 				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
> >> 				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
> >> 	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>,
> >> <1500000000>; };
> >
> > ...make this one a reasonable change...
> >
> >> For u-boot, if there is no requirement to run u-boot at 1500MHz, just
> >> keep &syscrg {
> >> 	assigned-clock-rates = <0>, <0>, <0>, <0>; };
> >
> > ...but not this one.
> >
> >> in u-boot device tree. If we need running 1500MHz in u-boot, I will
> >> send a patch to implement it and then &syscrg{...} in u-boot device tree
> can be dropped.
> >>
> >> Best regards,
> >> Hal
> 
> Hal,
> 
> What voltage (and speed) has less heat output?
> 
> I think it must be configured to be *reliable first* as it is pictured being a
> developer board on a person's desk (with no extra accessory - no fans,
> heatsink, etc.) and for example doing work as of a Linux kernel compile with
> using all cores.
> 
> We should not force a configuration that will fail for the common situation.
> What voltage and clock speed is the best for continuous reliable operation of a
> JH7110 CPU with no added heatsink or fan and at ambient temperature?
> 
> Does CPU frequency range need to be given in devicetree? Can instead this be
> done with userspace tool or Linux subsystem at runtime?
> 
> The thermal dissipation of JH7110 CPU will be depend on the board layout and
> physical application. So, my opinion here:  I think any minimum and maximum
> CPU frequency limit that is given by manufacturer recommendation of safe
> continuous operation is okay for dts include. If some part of this frequency
> range is not reliable (from undervolt, or overheat) then it is a "no" from me in
> Linux and in U-Boot. The fact of failing to boot (in U-Boot) because JH7110
> CPU default voltage is not high enough for 1500MHz operation tells me this is
> a wrong idea or there is missing code in U-Boot to select a default sane clock
> speed and voltage.

CPU_FREQ is enabled in Linux, so the CPU frequency and power supply can be adjusted
automatically. JH7110 CPUs frequency can adjusted to 375 / 500 / 750 / 1500 MHz, and
the CPU power supply should be adjusted to 0.8 / 0.8 / 0.8 / 1.04 V respectively.
Please see node opp-table-0 in arch/riscv/boot/dts/starfive/jh7110.dtsi.

Please don't drop clock rate assignments, it will cause CPUs can not run at
375 / 500 / 750 / 1500MHz but 250/333/500/1000MHz. You can modify as I suggested above.

For U-Boot, CPU_FREQ is not enabled and CPUs are running at 1000MHz. CPU power
supply is 0.9V which is the default value. If we need to run U-Boot at 1500MHz, the
CPU power supply should be changed to 1.04V first.

So I suggest

Linux:
&syscrg {
	assigned-clocks = <&syscrg JH7110_SYSCLK_CPU_ROOT>,
			  <&syscrg JH7110_SYSCLK_BUS_ROOT>,
			  <&syscrg JH7110_SYSCLK_PERH_ROOT>,
			  <&syscrg JH7110_SYSCLK_QSPI_REF>,
			  <&syscrg JH7110_SYSCLK_CPU_CORE>,
			  <&pllclk JH7110_PLLCLK_PLL0_OUT>;
	assigned-clock-parents = <&pllclk JH7110_PLLCLK_PLL0_OUT>,
				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
				 <&pllclk JH7110_PLLCLK_PLL2_OUT>,
				 <&syscrg JH7110_SYSCLK_QSPI_REF_SRC>;
	assigned-clock-rates = <0>, <0>, <0>, <0>, <500000000>, <1500000000>;
};

U-Boot:
&syscrg {
	assigned-clock-rates = <0>, <0>, <0>, <0>;
};

Best regards,
Hal
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv