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

[Krzysztof Kozlowski]

On 20/10/2022 12:08, Krzysztof Kozlowski wrote:
> On 20/10/2022 09:54, Stephan Gerhold wrote:
>> 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?
> 
> Keeping it in DTSI also suggests you could have some different frequency.
> 
>>
>> 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.
> 
> I know, the same with most of other platforms. Qualcomm is not special
> here. Maybe the difference is that some other platforms have few
> external clocks and not all of them are required.
> 
>> 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".
> 
> It's not about whether you can change it or not. It's about describing
> hardware - SoC DTSI describes SoC. DTS describes the board (assuming
> there is no SoM or other DTSI files). This clock is not in DTSI.
> 
>> 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).
> 
> You can move entire clock to boards.
> 
>> 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. :)
> 
> No, it puts attention to the board designer that he needs to provide the
> clock in his design.
> 

Another solution which would satisfy me, would be to put these clocks to
a dedicated, re-usable DTSI and include that DTSI in each board DTS.

Best regards,
Krzysztof