| Message ID | 20231130-bobbing-valid-b97f26fe8edc@spud (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] riscv: dts: starfive: move timebase-frequency to .dtsi | expand |
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Friday, December 1, 2023 12:11 AM > To: linux-riscv@lists.infradead.org > Cc: conor@kernel.org; Conor Dooley <conor.dooley@microchip.com>; Emil Renner Berthing <kernel@esmil.dk>; Rob Herring > <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Paul Walmsley <paul.walmsley@sifive.com>; > Palmer Dabbelt <palmer@dabbelt.com>; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Walker Chen > <walker.chen@starfivetech.com>; JeeHeng Sia <jeeheng.sia@starfivetech.com>; Leyfoon Tan <leyfoon.tan@starfivetech.com> > Subject: [PATCH v1] riscv: dts: starfive: move timebase-frequency to .dtsi > > From: Conor Dooley <conor.dooley@microchip.com> > > 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 <conor.dooley@microchip.com> > --- > 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 <kernel@esmil.dk> > CC: Conor Dooley <conor@kernel.org> > CC: Rob Herring <robh+dt@kernel.org> > CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > CC: Paul Walmsley <paul.walmsley@sifive.com> > CC: Palmer Dabbelt <palmer@dabbelt.com> > CC: linux-riscv@lists.infradead.org > CC: devicetree@vger.kernel.org > CC: linux-kernel@vger.kernel.org > CC: Walker Chen <walker.chen@starfivetech.com> > CC: JeeHeng Sia <jeeheng.sia@starfivetech.com> > CC: Leyfoon Tan <leyfoon.tan@starfivetech.com> > --- > 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
Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > 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 <conor.dooley@microchip.com> > --- > 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 <kernel@esmil.dk> > CC: Conor Dooley <conor@kernel.org> > CC: Rob Herring <robh+dt@kernel.org> > CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > CC: Paul Walmsley <paul.walmsley@sifive.com> > CC: Palmer Dabbelt <palmer@dabbelt.com> > CC: linux-riscv@lists.infradead.org > CC: devicetree@vger.kernel.org > CC: linux-kernel@vger.kernel.org > CC: Walker Chen <walker.chen@starfivetech.com> > CC: JeeHeng Sia <jeeheng.sia@starfivetech.com> > CC: Leyfoon Tan <leyfoon.tan@starfivetech.com> > --- > 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 >
On Fri, Dec 01, 2023 at 02:44:58PM +0100, Emil Renner Berthing wrote: > Conor Dooley wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > > > 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.
Conor Dooley wrote: > On Fri, Dec 01, 2023 at 02:44:58PM +0100, Emil Renner Berthing wrote: > > Conor Dooley wrote: > > > From: Conor Dooley <conor.dooley@microchip.com> > > > > > > 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
On 2023/12/1 0:11, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > 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 <conor.dooley@microchip.com> > --- > 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 <kernel@esmil.dk> > CC: Conor Dooley <conor@kernel.org> > CC: Rob Herring <robh+dt@kernel.org> > CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > CC: Paul Walmsley <paul.walmsley@sifive.com> > CC: Palmer Dabbelt <palmer@dabbelt.com> > CC: linux-riscv@lists.infradead.org > CC: devicetree@vger.kernel.org > CC: linux-kernel@vger.kernel.org > CC: Walker Chen <walker.chen@starfivetech.com> > CC: JeeHeng Sia <jeeheng.sia@starfivetech.com> > CC: Leyfoon Tan <leyfoon.tan@starfivetech.com> > --- > 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
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";