Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d5e176b3-75e2-67bc-3fc6-b54fe685c9a0@linaro.org>
Date:   Thu, 20 Oct 2022 12:26:48 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH v2 2/3] arm64: dts: qcom: Add base QDU1000/QRU1000 DTSIs
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To:     Stephan Gerhold <stephan@gerhold.net>
Cc:     Melody Olvera <quic_molvera@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20221014221138.7552-1-quic_molvera@quicinc.com>
 <20221014221138.7552-3-quic_molvera@quicinc.com>
 <56af2a04-1b21-000d-e3f9-86b6ac74aaf2@linaro.org>
 <a0032338-482f-0de7-5952-c3c8b8423df6@quicinc.com>
 <50372a15-56ce-6ad6-f622-00624b909db8@linaro.org>
 <Y1FTJgloEi5ag2/j@gerhold.net>
 <9fbe1bf5-a84d-c56b-1c0e-6848ee3d30fe@linaro.org>
In-Reply-To: <9fbe1bf5-a84d-c56b-1c0e-6848ee3d30fe@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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