Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 9 Jan 2024 13:04:52 +0000 | From | Conor Dooley <> | Subject | Re: [PATCH v2 3/8] riscv: dts: thead: Add TH1520 pin control nodes |
| |
On Tue, Jan 09, 2024 at 04:02:01AM -0800, Emil Renner Berthing wrote: > Conor Dooley wrote: > > On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote: > > > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC. > > > > > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > > > --- > > > .../boot/dts/thead/th1520-beaglev-ahead.dts | 4 ++++ > > > .../dts/thead/th1520-lichee-module-4a.dtsi | 4 ++++ > > > arch/riscv/boot/dts/thead/th1520.dtsi | 24 +++++++++++++++++++ > > > 3 files changed, 32 insertions(+) > > > > > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts > > > index 70e8042c8304..6c56318a8705 100644 > > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts > > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts > > > @@ -44,6 +44,10 @@ &osc_32k { > > > clock-frequency = <32768>; > > > }; > > > > > > +&aonsys_clk { > > > + clock-frequency = <73728000>; > > > +}; > > > + > > > &apb_clk { > > > clock-frequency = <62500000>; > > > }; > > > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi > > > index a802ab110429..9865925be372 100644 > > > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi > > > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi > > > @@ -25,6 +25,10 @@ &osc_32k { > > > clock-frequency = <32768>; > > > }; > > > > > > +&aonsys_clk { > > > + clock-frequency = <73728000>; > > > +}; > > > + > > > &apb_clk { > > > clock-frequency = <62500000>; > > > }; > > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi > > > index ba4d2c673ac8..e65a306ff575 100644 > > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi > > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi > > > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator { > > > #clock-cells = <0>; > > > }; > > > > > > + aonsys_clk: aonsys-clk { > > > + compatible = "fixed-clock"; > > > + clock-output-names = "aonsys_clk"; > > > + #clock-cells = <0>; > > > + }; > > > > Did this stuff sneak into this commit accidentally? > > Not really by accident no. It turns out the clock tree has gates for the bus > clock of each pinctrl block and I think it's better to add this clock > dependency to the bindings and driver up front.
Maybe if I had looked a wee bit more deeply I would've noticed that it was used there, but it's always good to mention the rationale in the commit message so that it's more obvious why you're doin it.
> Since there is not yet any clock driver the initial device tree for the TH1520 > included the dummy apb_clk that two of the pinctrl blocks derive their clock > from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought > it was better to add this dummy clock with the only (so far) user of it, but if > you have a better idea, let me know.
No, that's fine. I was just wondering why there was an unmentioned set of clocks being added. If they're stubbed fixed clocks I dunno if it makes sense to add them to the board.dts/module.dtsi files though. Where do the initial values come from for the rates? Out of reset values or set by firmware that may vary from board to board?
Cheers, Conor. [unhandled content-type:application/pgp-signature] | ![\](/images/icornerr.gif) |