linux-kernel - Re: [PATCH v2 2/3] arm64: dts: qcom: Add base QDU1000/QRU1000 DTSIs

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]

Date:   Thu, 20 Oct 2022 15:54:46 +0200
From:   Stephan Gerhold <stephan@...hold.net>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Melody Olvera <quic_molvera@...cinc.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] arm64: dts: qcom: Add base QDU1000/QRU1000 DTSIs

On Thu, Oct 20, 2022 at 08:41:15AM -0400, Krzysztof Kozlowski wrote:
> On 19/10/2022 16:21, Melody Olvera wrote:
> > On 10/15/2022 6:28 AM, Krzysztof Kozlowski wrote:
> >> On 14/10/2022 18:11, Melody Olvera wrote:
> >> [...]
> >>> +	clocks {
> >>> +		xo_board: xo-board {
> >>> +			compatible = "fixed-clock";
> >>> +			clock-frequency = <19200000>;
> >> Both clocks are not a property of a SoC. They are provided by the board,
> >> so they should either be defined by board DTS or at least their
> >> frequency must be provided by the board.
> > That doesn't seem in keeping with precedent.... the sm8* series all have the clocks in
> > the dtsi. These are common to the boards anyways.
> 
> Because people do not pay attention what is part of SoC, what is part of
> board. DTSI is for the SoC and these are inputs to the SoC.
> 

(Just chiming in because I had this thought already a few times when you
 suggested moving the XO "clock-frequency" to the board DTS:)

I understand your reasoning for moving components of the board to the
board DTS, but IMHO adding just the clock-frequency to the board DTS is
even more misleading: It suggests that there are functional board
designs where you would use a XO clock with a different clock-frequency.
Is that really realistic though?

There are assumptions about the XO clock frequency in a lot of places:
You would need to fully rewrite the gcc-<SoC>.c driver because it has
fixed multipliers/dividers for one specific XO frequency. All firmware
binaries would likely need changes. And does the hardware even support a
different XO clock frequency? The APQ8016E datasheet for example
strictly documents a XO clock input of 19.2 MHz and a sleep clock of
32.768 kHz.

IMHO the only realistic variation of the XO clock setup would be to have
a physical "fixed-clock" with a higher frequency, followed by a
"fixed-factor-clock" that brings it back to the expected frequency. To
model that properly it is not enough to have just the "clock-frequency"
in the board DTS. In this case you need two clock nodes, and the
xo_board would be the "fixed-factor-clock".

Therefore it should be all or nothing IMO: Either we move the full
xo-board node to the board DTS (which allows alternatively defining the
"fixed-factor-clock" or whatever). Or we assume that there will be
always an input clock signal with the fixed frequency and keep it fully
in the SoC .dtsi.

Having just the "clock-frequency" in the board DTS puts the attention on
the wrong detail, IMO. :)

Thanks,
Stephan