linux-kernel - Re: [PATCH RFC 2/2] arm64: dts: qcom: sm8650: move board clocks to DTS files

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] [day] [month] [year] [list]

Message-ID: <4kopdkvbkrpcpzwteezm427ml5putqvzsnfkpmg76spsple7l5@mg7v3ihwxnit>
Date: Mon, 25 Nov 2024 15:23:22 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 2/2] arm64: dts: qcom: sm8650: move board clocks to
 DTS files

On Sat, Nov 23, 2024 at 06:02:06PM +0100, Krzysztof Kozlowski wrote:
> On 15/11/2024 07:59, Dmitry Baryshkov wrote:
> > SM8650 is one of the platforms where board-level clocks (XO, sleep)
> > definitions are split between the SoC dtsi file and the board file.
> > This is not correct, as these two clocks are not a part of the SoC. Also
> > such definitions don't fully follow the DT guidelines. Move these two
> > clocks to the board files completely.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 22 ++++++++++++++--------
> >  arch/arm64/boot/dts/qcom/sm8650-mtp.dts | 22 ++++++++++++++--------
> >  arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 22 ++++++++++++++--------
> >  arch/arm64/boot/dts/qcom/sm8650.dtsi    | 10 ----------
> >  4 files changed, 42 insertions(+), 34 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8650-hdk.dts b/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
> > index f00bdff4280af22f6f8b23e33238f53c602bc169..641bd817d75439bc6a050189565437e1c5ead3b5 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
> > @@ -30,6 +30,20 @@ chosen {
> >  		stdout-path = "serial0:115200n8";
> >  	};
> >  
> > +	clocks {
> > +		xo_board: xo-board {
> > +			compatible = "fixed-clock";
> > +			#clock-cells = <0>;
> > +			clock-frequency = <76800000>;
> > +		};
> > +
> > +		sleep_clk: sleep-clk {
> > +			compatible = "fixed-clock";
> > +			#clock-cells = <0>;
> > +			clock-frequency = <32000>;
> > +		};
> > +	};
> > +
> >  	hdmi-out {
> >  		compatible = "hdmi-connector";
> >  		type = "a";
> > @@ -1112,10 +1126,6 @@ &sdhc_2 {
> >  	status = "okay";
> >  };
> >  
> > -&sleep_clk {
> > -	clock-frequency = <32000>;
> > -};
> 
> No, code has correct style. To avoid duplication, the SoC DTSI keeps the
> skeleton of the clock, which also indicates that SoC expects it to be
> provided by board. Then the board provides the clock by setting the
> frequency.

So, what is the decision maker: a deduplication or board vs soc
placement? From the duplication point of view we can keep the complete
clock in SoC.dtsi (like we did for older platforms or I did for
sar2130p). Most if not all boards use the same clock for the SoC family.
>From the correctness point of view the clock should be completely in the
board DT, the clock is completely on the board and not on the SoC.
Having clock declaration in SoC.dtsi and patching the freq in board.dts
is a strange hack and mix of two solutions. For me as a reviewer and a
porter having such a split is a nightmare.

-- 
With best wishes
Dmitry