linux-kernel - Re: [PATCH v2 3/8] riscv: dts: thead: Add TH1520 pin control nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240109-boggle-frugality-03a77cab8308@spud>
Date: Tue, 9 Jan 2024 17:34:11 +0000
From: Conor Dooley <conor@...nel.org>
To: Emil Renner Berthing <emil.renner.berthing@...onical.com>
Cc: Conor Dooley <conor.dooley@...rochip.com>, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-riscv@...ts.infradead.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jisheng Zhang <jszhang@...nel.org>, Guo Ren <guoren@...nel.org>,
	Fu Wei <wefu@...hat.com>, Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Drew Fustini <dfustini@...libre.com>
Subject: Re: [PATCH v2 3/8] riscv: dts: thead: Add TH1520 pin control nodes

On Tue, Jan 09, 2024 at 06:28:19AM -0800, Emil Renner Berthing wrote:
> Conor Dooley wrote:
> > 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@...onical.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.
> 
> You absolutely right. I forgot to update the commit message.
> 
> > > 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?
> 
> The vendor u-boot sets the PLLs different from the reset values. For now I
> think it's the same code for every board using the Lichee Pi 4A module (and
> probably also for the BeagleV Ahead), but it might still make sense to move the
> freqency to the board instead of the module device tree.

Yeah, think so. Only temporarily though, do you have a clue if anyone is
working on the actual clock driver stuff? Seems pretty Deadge?
https://lore.kernel.org/linux-clk/?q=th1520

Cheers,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)