[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bfc64e55-3c06-e36b-70cc-33a0303681be@linaro.org>
Date: Sat, 25 Mar 2023 13:34:41 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Vinod Koul <vkoul@...nel.org>,
Bjorn Andersson <andersson@...nel.org>
Cc: linux-arm-msm@...r.kernel.org,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 10/12] arm64: dts: qcom: Introduce the SC8180x platform
On 25.03.2023 13:24, Vinod Koul wrote:
> From: Bjorn Andersson <bjorn.andersson@...aro.org>
>
> Introduce a base dtsi for the Qualcomm SC8180x platform, with CPUs,
> global clock controller, SMMU, rpmh clocks, rpmh power-domains, CPUfreq,
> QUP blocks, UFS, USB, ADSP, CDSP and MPSS and WiFi.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> Signed-off-by: Vinod Koul <vkoul@...nel.org>
> ---
> arch/arm64/boot/dts/qcom/sc8180x.dtsi | 3950 +++++++++++++++++++++++++
> 1 file changed, 3950 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sc8180x.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8180x.dtsi b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
> new file mode 100644
> index 000000000000..4d4ee6bc91e5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc8180x.dtsi
> @@ -0,0 +1,3950 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2020-2023, Linaro Limited
> + */
> +
> +#include <dt-bindings/clock/qcom,dispcc-sm8250.h>
> +#include <dt-bindings/clock/qcom,gcc-sc8180x.h>
> +#include <dt-bindings/clock/qcom,gpucc-sm8150.h>
> +#include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> +#include <dt-bindings/interconnect/qcom,sc8180x.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.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_clk: xo-board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <38400000>;
> + };
> +
> + sleep_clk: sleep-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32764>;
> + clock-output-names = "sleep_clk";
> + };
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x0>;
Please add clocks = <&cpufreq_hw n>;
> + enable-method = "psci";
> + capacity-dmips-mhz = <602>;
> + next-level-cache = <&L2_0>;
> + qcom,freq-domain = <&cpufreq_hw 0>;
> + operating-points-v2 = <&cpu0_opp_table>;
> + interconnects = <&gem_noc MASTER_AMPSS_M0 3 &mc_virt SLAVE_EBI_CH0 3>,
> + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
> + power-domains = <&CPU_PD0>;
> + power-domain-names = "psci";
> + #cooling-cells = <2>;
Add a newline before subnodes, please.
> + L2_0: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + L3_0: l3-cache {
> + compatible = "cache";
> + };
> + };
> + };
> +
[...]
> +
> + cpu0_opp_table: opp-table-cpu0 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-peak-kBps = <800000 9600000>;
Maybe adding bwmon from the get-go would be better than statically
scaling DDR freq?
[...]
> + camnoc_virt: interconnect-0{
Missing space before {
> + compatible = "qcom,sc8180x-camnoc-virt";
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + mc_virt: interconnect-mc-virt {
Please be consistent with your naming.
> + compatible = "qcom,sc8180x-mc-virt";
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + qup_virt: interconnect-qup-virt {
> + compatible = "qcom,sc8180x-qup-virt";
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
[...]
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + hyp_mem: hyp-region@...00000 {
the -region seems a bit unnecessary in all of these nodes
> + reg = <0x0 0x85700000 0x0 0x600000>;
> + no-map;
> + };
> +
[...]
> +
> + soc: soc@0 {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0 0 0 0 0x10 0>;
> + dma-ranges = <0 0 0 0 0x10 0>;
> + compatible = "simple-bus";
compat
addr-cells
size-cella
ranges
dma-ranges
please
> +
> + gcc: clock-controller@...000 {
> + compatible = "qcom,gcc-sc8180x";
> + reg = <0x0 0x00100000 0x0 0x1f0000>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + clock-names = "bi_tcxo",
> + "bi_tcxo_ao",
> + "sleep_clk";
> + clocks = <&rpmhcc RPMH_CXO_CLK>,
> + <&rpmhcc RPMH_CXO_CLK_A>,
> + <&sleep_clk>;
property before property-names
> + };
> +
> + qupv3_id_0: geniqup@...000 {
> + compatible = "qcom,geni-se-qup";
> + reg = <0 0x008c0000 0 0x6000>;
> + clock-names = "m-ahb", "s-ahb";
> + clocks = <&gcc GCC_QUPV3_WRAP_0_M_AHB_CLK>,
> + <&gcc GCC_QUPV3_WRAP_0_S_AHB_CLK>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + iommus = <&apps_smmu 0x4c3 0>;
> + status = "disabled";
> +
> + i2c0: i2c@...000 {
> + compatible = "qcom,geni-i2c";
> + reg = <0 0x00880000 0 0x4000>;
> + clock-names = "se";
> + clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
property before property-names
Please split QUPs into a separate patch, this one is way
too big.
[...]
> + config_noc: interconnect@...0000 {
Interconnect could also realistically go to a separate patch.
> + compatible = "qcom,sc8180x-config-noc";
> + reg = <0 0x01500000 0 0x7400>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + system_noc: interconnect@...0000 {
> + compatible = "qcom,sc8180x-system-noc";
> + reg = <0 0x01620000 0 0x19400>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + aggre1_noc: interconnect@...0000 {
> + compatible = "qcom,sc8180x-aggre1-noc";
> + reg = <0 0x016e0000 0 0xd080>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + aggre2_noc: interconnect@...0000 {
> + compatible = "qcom,sc8180x-aggre2-noc";
> + reg = <0 0x01700000 0 0x20000>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + compute_noc: interconnect@...0000 {
> + compatible = "qcom,sc8180x-compute-noc";
> + reg = <0 0x01720000 0 0x7000>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
> + mmss_noc: interconnect@...0000 {
> + compatible = "qcom,sc8180x-mmss-noc";
> + reg = <0 0x01740000 0 0x1c100>;
> + #interconnect-cells = <2>;
> + qcom,bcm-voters = <&apps_bcm_voter>;
> + };
> +
[...]
> + pcie0: pci@...0000 {
And PCIe
> + compatible = "qcom,pcie-sc8180x";
> + reg = <0 0x01c00000 0 0x3000>,
> + <0 0x60000000 0 0xf1d>,
> + <0 0x60000f20 0 0xa8>,
> + <0 0x60001000 0 0x1000>,
> + <0 0x60100000 0 0x100000>;
> + reg-names = "parf", "dbi", "elbi", "atu", "config";
One per line, please
[...]
> +
> + ufs_mem_hc: ufshc@...4000 {
> + compatible = "qcom,sc8180x-ufshc", "qcom,ufshc",
> + "jedec,ufs-2.0";
> + reg = <0 0x01d84000 0 0x2500>;
> + interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&ufs_mem_phy_lanes>;
> + phy-names = "ufsphy";
> + lanes-per-direction = <2>;
> + #reset-cells = <1>;
> + resets = <&gcc GCC_UFS_PHY_BCR>;
> + reset-names = "rst";
> +
> + iommus = <&apps_smmu 0x300 0>;
> +
> + clock-names =
No need for this weird newline split.
Also, property before property-names.
[...]
> +
> + gpu: gpu@...0000 {
GPUSS and MDSS related nodes should also go to their separate,
respective patches.
[...]
> +
> + remoteproc_mpss: remoteproc@...0000 {
And remote procs as well
> + compatible = "qcom,sc8180x-mpss-pas";
> + reg = <0x0 0x04080000 0x0 0x4040>;
> +
[...]
> + thermal-zones {
And thermal-zones as well.
I'll go more in-depth after you split it up, this is pretty hard
to review as-is.
Konrad
> + cpu0-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 1>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu1-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 2>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu2-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 3>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu3-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 4>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu4-top-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 7>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu5-top-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 8>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu6-top-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 9>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu7-top-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 10>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu4-bottom-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 11>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu5-bottom-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 12>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu6-bottom-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 13>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu7-bottom-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 14>;
> +
> + trips {
> + cpu-crit {
> + temperature = <110000>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + aoss0-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 0>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + cluster0-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 5>;
> +
> + trips {
> + cluster-crit {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cluster1-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 6>;
> +
> + trips {
> + cluster-crit {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + gpu-thermal-top {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens0 15>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + aoss1-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 0>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + wlan-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 1>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + video-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 2>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + mem-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 3>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + q6-hvx-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 4>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + camera-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 5>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + compute-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 6>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + mdm-dsp-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 7>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + npu-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 8>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> +
> + gpu-thermal-bottom {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens1 11>;
> +
> + trips {
> + trip-point0 {
> + temperature = <90000>;
> + hysteresis = <2000>;
> + type = "hot";
> + };
> + };
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
Powered by blists - more mailing lists