Re: [PATCH 2/4] arm64: dts: qcom: add SC8280XP platform

[Bjorn Andersson]

On Mon 20 Jun 23:53 PDT 2022, Krzysztof Kozlowski wrote:

> 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@xxxxxxxxxx>
> >>> ---
> >>>  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.
> 

I find this to be a rather strict interpretation of "board specific",
but I'm okay with it.

Regards,
Bjorn