2023-11-30 16:12:19

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi

From: Conor Dooley <[email protected]>

Properties fixed by the SoC should be defined in the $soc.dtsi, and the
timebase-frequency is not sourced directly from an off-chip oscillator.

Signed-off-by: Conor Dooley <[email protected]>
---
I actually have no idea whether this is true or not, I asked on the
jh8100 series but only got an answer for that SoC and not the existing
ones. I'm hoping that a patch envokes more of a reaction!

CC: Emil Renner Berthing <[email protected]>
CC: Conor Dooley <[email protected]>
CC: Rob Herring <[email protected]>
CC: Krzysztof Kozlowski <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: Walker Chen <[email protected]>
CC: JeeHeng Sia <[email protected]>
CC: Leyfoon Tan <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100-common.dtsi | 4 ----
arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
.../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ----
arch/riscv/boot/dts/starfive/jh7110.dtsi | 1 +
4 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index b93ce351a90f..214f27083d7b 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -19,10 +19,6 @@ chosen {
stdout-path = "serial0:115200n8";
};

- cpus {
- timebase-frequency = <6250000>;
- };
-
memory@80000000 {
device_type = "memory";
reg = <0x0 0x80000000 0x2 0x0>;
diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index e68cafe7545f..c50b32424721 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -16,6 +16,7 @@ / {
cpus {
#address-cells = <1>;
#size-cells = <0>;
+ timebase-frequency = <6250000>;

U74_0: cpu@0 {
compatible = "sifive,u74-mc", "riscv";
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index b89e9791efa7..7873c7ffde4d 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -26,10 +26,6 @@ chosen {
stdout-path = "serial0:115200n8";
};

- cpus {
- timebase-frequency = <4000000>;
- };
-
memory@40000000 {
device_type = "memory";
reg = <0x0 0x40000000 0x1 0x0>;
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 45213cdf50dc..ee7d4bb1f537 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -18,6 +18,7 @@ / {
cpus {
#address-cells = <1>;
#size-cells = <0>;
+ timebase-frequency = <4000000>;

S7_0: cpu@0 {
compatible = "sifive,s7", "riscv";
--
2.39.2


2023-12-01 13:33:50

by Sia Jee Heng

[permalink] [raw]
Subject: RE: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi



> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Friday, December 1, 2023 12:11 AM
> To: [email protected]
> Cc: [email protected]; Conor Dooley <[email protected]>; Emil Renner Berthing <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski <[email protected]>; Paul Walmsley <[email protected]>;
> Palmer Dabbelt <[email protected]>; [email protected]; [email protected]; Walker Chen
> <[email protected]>; JeeHeng Sia <[email protected]>; Leyfoon Tan <[email protected]>
> Subject: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi
>
> From: Conor Dooley <[email protected]>
>
> Properties fixed by the SoC should be defined in the $soc.dtsi, and the
> timebase-frequency is not sourced directly from an off-chip oscillator.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> I actually have no idea whether this is true or not, I asked on the
> jh8100 series but only got an answer for that SoC and not the existing
> ones. I'm hoping that a patch envokes more of a reaction!
I believe it is controlled by the internal clock, but I will let Walker
have the final say.
>
> CC: Emil Renner Berthing <[email protected]>
> CC: Conor Dooley <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> CC: Paul Walmsley <[email protected]>
> CC: Palmer Dabbelt <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: Walker Chen <[email protected]>
> CC: JeeHeng Sia <[email protected]>
> CC: Leyfoon Tan <[email protected]>
> ---
> arch/riscv/boot/dts/starfive/jh7100-common.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
> .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 1 +
> 4 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index b93ce351a90f..214f27083d7b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -19,10 +19,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <6250000>;
> - };
> -
> memory@80000000 {
> device_type = "memory";
> reg = <0x0 0x80000000 0x2 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index e68cafe7545f..c50b32424721 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -16,6 +16,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <6250000>;
>
> U74_0: cpu@0 {
> compatible = "sifive,u74-mc", "riscv";
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-
> 2.dtsi
> index b89e9791efa7..7873c7ffde4d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -26,10 +26,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <4000000>;
> - };
> -
> memory@40000000 {
> device_type = "memory";
> reg = <0x0 0x40000000 0x1 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 45213cdf50dc..ee7d4bb1f537 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -18,6 +18,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <4000000>;
>
> S7_0: cpu@0 {
> compatible = "sifive,s7", "riscv";
> --
> 2.39.2

2023-12-01 13:46:27

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi

Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> Properties fixed by the SoC should be defined in the $soc.dtsi, and the
> timebase-frequency is not sourced directly from an off-chip oscillator.

Yes, according to the JH7100 docs[1] the mtime register is sourced from the
osc_sys external oscillator through u74rtc_toggle. However I haven't yet found
a place in the docs that describe where that clock is divided by 4 to get
6.25MHz from the 25MHz.

I expect the JH7110 mtime is set up in a similar way, but haven't yet dug into
the available documentation.

[1]: https://github.com/starfive-tech/JH7100_Docs/blob/main/vic_u7_manual_with_creativecommons.pdf

/Emil

>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> I actually have no idea whether this is true or not, I asked on the
> jh8100 series but only got an answer for that SoC and not the existing
> ones. I'm hoping that a patch envokes more of a reaction!
>
> CC: Emil Renner Berthing <[email protected]>
> CC: Conor Dooley <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> CC: Paul Walmsley <[email protected]>
> CC: Palmer Dabbelt <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: Walker Chen <[email protected]>
> CC: JeeHeng Sia <[email protected]>
> CC: Leyfoon Tan <[email protected]>
> ---
> arch/riscv/boot/dts/starfive/jh7100-common.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
> .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 1 +
> 4 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index b93ce351a90f..214f27083d7b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -19,10 +19,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <6250000>;
> - };
> -
> memory@80000000 {
> device_type = "memory";
> reg = <0x0 0x80000000 0x2 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index e68cafe7545f..c50b32424721 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -16,6 +16,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <6250000>;
>
> U74_0: cpu@0 {
> compatible = "sifive,u74-mc", "riscv";
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index b89e9791efa7..7873c7ffde4d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -26,10 +26,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <4000000>;
> - };
> -
> memory@40000000 {
> device_type = "memory";
> reg = <0x0 0x40000000 0x1 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 45213cdf50dc..ee7d4bb1f537 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -18,6 +18,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <4000000>;
>
> S7_0: cpu@0 {
> compatible = "sifive,s7", "riscv";
> --
> 2.39.2
>

2023-12-03 11:04:10

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi

On Fri, Dec 01, 2023 at 02:44:58PM +0100, Emil Renner Berthing wrote:
> Conor Dooley wrote:
> > From: Conor Dooley <[email protected]>
> >
> > Properties fixed by the SoC should be defined in the $soc.dtsi, and the
> > timebase-frequency is not sourced directly from an off-chip oscillator.
>
> Yes, according to the JH7100 docs[1] the mtime register is sourced from the
> osc_sys external oscillator through u74rtc_toggle. However I haven't yet found
> a place in the docs that describe where that clock is divided by 4 to get
> 6.25MHz from the 25MHz.
>
> I expect the JH7110 mtime is set up in a similar way, but haven't yet dug into
> the available documentation.

Your other reply suggests that this is a fixed division for the jh7110,
in which case it makes sense to leave it as-is. mpfs is different in
that it is fixed to 1 MHz regardless of which of the permitted external
oscillator frequencies you use.


Attachments:
(No filename) (944.00 B)
signature.asc (235.00 B)
Download all attachments

2023-12-03 16:09:17

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi

Conor Dooley wrote:
> On Fri, Dec 01, 2023 at 02:44:58PM +0100, Emil Renner Berthing wrote:
> > Conor Dooley wrote:
> > > From: Conor Dooley <[email protected]>
> > >
> > > Properties fixed by the SoC should be defined in the $soc.dtsi, and the
> > > timebase-frequency is not sourced directly from an off-chip oscillator.
> >
> > Yes, according to the JH7100 docs[1] the mtime register is sourced from the
> > osc_sys external oscillator through u74rtc_toggle. However I haven't yet found
> > a place in the docs that describe where that clock is divided by 4 to get
> > 6.25MHz from the 25MHz.
> >
> > I expect the JH7110 mtime is set up in a similar way, but haven't yet dug into
> > the available documentation.
>
> Your other reply suggests that this is a fixed division for the jh7110,
> in which case it makes sense to leave it as-is. mpfs is different in
> that it is fixed to 1 MHz regardless of which of the permitted external
> oscillator frequencies you use.

This is what I've found for the JH7100:

osc_sys (25MHz) -> u74rtc_toggle (gate) -> ? (div 4) -> mtime

The divide by 4 is not in the regular clock tree, so if it is configurable it
must be some bits hidden in the syscon area or something. The only restriction
I've found in the docs is that it must be strictly less than half the rate of
the core clock.

For the JH7110 it goes:

osc (24MHz) -> rtc_toggle (div N) -> mtime

..where N defaults to 6 and this is also the maximum N.

/Emil

2023-12-04 11:37:40

by Xingyu Wu

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi

On 2023/12/1 0:11, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> Properties fixed by the SoC should be defined in the $soc.dtsi, and the
> timebase-frequency is not sourced directly from an off-chip oscillator.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> I actually have no idea whether this is true or not, I asked on the
> jh8100 series but only got an answer for that SoC and not the existing
> ones. I'm hoping that a patch envokes more of a reaction!
>
> CC: Emil Renner Berthing <[email protected]>
> CC: Conor Dooley <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Krzysztof Kozlowski <[email protected]>
> CC: Paul Walmsley <[email protected]>
> CC: Palmer Dabbelt <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: Walker Chen <[email protected]>
> CC: JeeHeng Sia <[email protected]>
> CC: Leyfoon Tan <[email protected]>
> ---
> arch/riscv/boot/dts/starfive/jh7100-common.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
> .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ----
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 1 +
> 4 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index b93ce351a90f..214f27083d7b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -19,10 +19,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <6250000>;
> - };
> -
> memory@80000000 {
> device_type = "memory";
> reg = <0x0 0x80000000 0x2 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index e68cafe7545f..c50b32424721 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -16,6 +16,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <6250000>;
>
> U74_0: cpu@0 {
> compatible = "sifive,u74-mc", "riscv";
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index b89e9791efa7..7873c7ffde4d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -26,10 +26,6 @@ chosen {
> stdout-path = "serial0:115200n8";
> };
>
> - cpus {
> - timebase-frequency = <4000000>;
> - };
> -
> memory@40000000 {
> device_type = "memory";
> reg = <0x0 0x40000000 0x1 0x0>;
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 45213cdf50dc..ee7d4bb1f537 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -18,6 +18,7 @@ / {
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + timebase-frequency = <4000000>;
>
> S7_0: cpu@0 {
> compatible = "sifive,s7", "riscv";


Hi Conor and Emil,

I found some information that I hope will be useful to you.
What Emil said is right:
osc (24MHz) -> rtc_toggle (div N) -> mtime

I found the N is depend on this clock register in drivers/clk/starfive/clk-starfive-jh7110-sys.c:

83 JH71X0__DIV(JH7110_SYSCLK_RTC_TOGGLE, "rtc_toggle", 6, JH7110_SYSCLK_OSC),

and the description of the register is that the divider defaults to and is fixed to 6. So the timebase-frequency is 4MHz on the JH7110.

Best regards,
Xingyu Wu