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: <9a31a488-f4fe-33f5-84e7-fe4509769d7c@linaro.org>
Date:   Tue, 21 Jun 2022 08:53:39 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH 2/4] arm64: dts: qcom: add SC8280XP platform
Content-Language: en-US
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        Johan Hovold <johan+linaro@kernel.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220607214113.4057684-1-bjorn.andersson@linaro.org>
 <20220607214113.4057684-3-bjorn.andersson@linaro.org>
 <79443fb4-5c09-a33f-594d-71ac93cc0317@linaro.org>
 <YrE84tLOpJtzrNW4@builder.lan>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <YrE84tLOpJtzrNW4@builder.lan>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/06/2022 05:37, Bjorn Andersson wrote:
> On Wed 08 Jun 03:18 CDT 2022, Krzysztof Kozlowski wrote:
> 
>> On 07/06/2022 23:41, Bjorn Andersson wrote:
>>> Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx
>>> Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster
>>> idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks,
>>> interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and
>>> tsens.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2195 ++++++++++++++++++++++++
>>>  1 file changed, 2195 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> new file mode 100644
>>> index 000000000000..4143813643ad
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
>>> @@ -0,0 +1,2195 @@
>>> +// SPDX-License-Identifier: BSD-3-Clause
>>> +/*
>>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2022, Linaro Limited
>>> + */
>>> +
>>> +#include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
>>> +#include <dt-bindings/clock/qcom,rpmh.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +#include <dt-bindings/interconnect/qcom,sc8280xp.h>
>>> +#include <dt-bindings/mailbox/qcom-ipcc.h>
>>> +#include <dt-bindings/power/qcom-rpmpd.h>
>>> +#include <dt-bindings/soc/qcom,rpmh-rsc.h>
>>> +#include <dt-bindings/thermal/thermal.h>
>>> +
>>> +/ {
>>> +	interrupt-parent = <&intc>;
>>> +
>>> +	#address-cells = <2>;
>>> +	#size-cells = <2>;
>>> +
>>> +	clocks {
>>> +		xo_board: xo-board {
>>
>> xo-board-clk
>>
>>> +			compatible = "fixed-clock";
>>> +			#clock-cells = <0>;
>>> +			clock-frequency = <38400000>;
>>
>> The clock is probably on the board, so the frequency should be rather
>> defined in DTS.
>>
> 
> It's an interesting question, but I don't think it's possible to change
> the rate of this clock from one board to another.
> 
> So I think it's best to keep this in the .dtsi, to avoid unnecessary
> duplication.

It does not matter whether the frequency can be changed or not. This is
the same on almost every SoC and the same comments appear every time -
the clock is a property of the board, not of the SoC, so it should be in
the board DTSI. To avoid the duplication you can indeed keep here most
of the clock properties, but the frequency must be in board DTS.

Best regards,
Krzysztof