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

[Stephan Gerhold]

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