linux-kernel - Re: [PATCH v5 5/5] riscv: dts: spacemit: Add clock tree for Spacemit K1

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8qSAKXQEfnyVef6@ketchup>
Date: Fri, 7 Mar 2025 06:28:16 +0000
From: Haylen Chu <heylenay@....org>
To: Inochi Amaoto <inochiama@...il.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Haylen Chu <heylenay@...look.com>, Yixun Lan <dlan@...too.org>
Cc: linux-riscv@...ts.infradead.org, linux-clk@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	spacemit@...ts.linux.dev, Inochi Amaoto <inochiama@...look.com>,
	Chen Wang <unicornxdotw@...mail.com>,
	Jisheng Zhang <jszhang@...nel.org>,
	Meng Zhang <zhangmeng.kevin@...ux.spacemit.com>
Subject: Re: [PATCH v5 5/5] riscv: dts: spacemit: Add clock tree for Spacemit
 K1

On Fri, Mar 07, 2025 at 09:55:15AM +0800, Inochi Amaoto wrote:
> On Thu, Mar 06, 2025 at 05:57:51PM +0000, Haylen Chu wrote:
> > Describe the PLL and system controllers that're capable of generating
> > clock signals in the devicetree.
> > 
> > Signed-off-by: Haylen Chu <heylenay@....org>
> > ---
> >  arch/riscv/boot/dts/spacemit/k1.dtsi | 79 ++++++++++++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index c670ebf8fa12..09a9100986b1 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > @@ -3,6 +3,8 @@
> >   * Copyright (C) 2024 Yangyu Chen <cyy@...self.name>
> >   */
> >  
> > +#include <dt-bindings/clock/spacemit,k1-ccu.h>
> > +
> >  /dts-v1/;
> >  / {
> >  	#address-cells = <2>;
> > @@ -306,6 +308,40 @@ cluster1_l2_cache: l2-cache1 {
> >  		};
> >  	};
> >  
> > +	clocks {
> 
> > +		#address-cells = <0x2>;
> > +		#size-cells = <0x2>;
> > +		ranges;
> 
> why setting this?

I just noticed these unnecessary properties after sending the series,
will drop them, thanks.

> > +
> > +		vctcxo_1m: clock-1m {
> > +			compatible = "fixed-clock";
> 
> > +			clock-frequency = <1000000>;
> 
> Should the frequency this move to the board file?
> I do not think these clock are in the soc.
> This applys to all clock below.

This should be the special case described in the dts-coding-style
documentation,

> A partial exception is a common external reference SoC input clock,
> which could be coded as a fixed-clock in the SoC DTSI with its
> frequency provided by each board DTS.[1]

These four external clocks are essential for K1 SoC to operate as shown
in the clock tree diagram[2]. I think they fit the exception.

Furthermore, the SoC expects these four clocks in fixed rates, thus I
don't see a reason not to include the clock-frequency property here.
Please correct me if I get something wrong.

> > +			clock-output-names = "vctcxo_1m";
> > +			#clock-cells = <0>;
> > +		};
> > +
> > +		vctcxo_24m: clock-24m {
> > +			compatible = "fixed-clock";
> > +			clock-frequency = <24000000>;
> > +			clock-output-names = "vctcxo_24m";
> > +			#clock-cells = <0>;
> > +		};
> > +
> > +		vctcxo_3m: clock-3m {
> > +			compatible = "fixed-clock";
> > +			clock-frequency = <3000000>;
> > +			clock-output-names = "vctcxo_3m";
> > +			#clock-cells = <0>;
> > +		};
> > +
> > +		osc_32k: clock-32k {
> > +			compatible = "fixed-clock";
> > +			clock-frequency = <32000>;
> > +			clock-output-names = "osc_32k";
> > +			#clock-cells = <0>;
> > +		};
> > +	};
> > +
> >  	soc {
> >  		compatible = "simple-bus";
> >  		interrupt-parent = <&plic>;

Thanks,
Haylen Chu

[1]: https://elixir.bootlin.com/linux/v6.11-rc1/source/Documentation/devicetree/bindings/dts-coding-style.rst#L196
[2]: https://developer.spacemit.com/resource/file/images?fileName=E9VPb63n8o1heox2KXHcgWiVnWh.png