mbox series

[v2,00/12] Introduce the SC8180x devices

Message ID 20230325122444.249507-1-vkoul@kernel.org (mailing list archive)
Headers show
Series Introduce the SC8180x devices | expand

Message

Vinod Koul March 25, 2023, 12:24 p.m. UTC
This introduces Qualcomm SC8180x SoC which features in Lenovo Flex 5G
laptop. This also adds support for Primus platform as well as Lenovo Flex 5G
laptop.

I would be great if submaintainers can ack the binding patch so that
everything can go thru qcom tree

Changes in v2:
 - Fix the ufs pcie and phy bindings
 - Lots of error fixes for dtbs_check
 - Add few more missing compatiables

Bjorn Andersson (3):
  arm64: dts: qcom: Introduce the SC8180x platform
  arm64: dts: qcom: sc8180x: Introduce Primus
  arm64: dts: qcom: sc8180x: Introduce Lenovo Flex 5G

Vinod Koul (9):
  dt-bindings: firmware: document Qualcomm SC8180X SCM
  dt-bindings: PCI: qcom: Document sc8180x properties
  dt-bindings: phy: qcom,qmp-pcie: fix the sc8180x regs
  dt-bindings: usb: qcom,dwc3: Add SC8180x binding
  dt-bindings: interconnect: split SC8180x to own schema
  scsi: ufs: dt-bindings: Add SC8180x binding
  dt-bindings: phy: qcom,qmp-ufs: fix the sc8180x regs
  regulator: dt-bindings: qcom,rpmh: Add compatible for PMC8180
  dt-bindings: qcom,pdc: Add SC8180x compatible

 .../bindings/firmware/qcom,scm.yaml           |    1 +
 .../bindings/interconnect/qcom,rpmh.yaml      |   11 -
 .../interconnect/qcom,sc8180x-rpmh.yaml       |   76 +
 .../interrupt-controller/qcom,pdc.yaml        |    1 +
 .../devicetree/bindings/pci/qcom,pcie.yaml    |   49 +-
 .../phy/qcom,ipq8074-qmp-pcie-phy.yaml        |    2 +-
 .../phy/qcom,msm8996-qmp-ufs-phy.yaml         |   18 +-
 .../regulator/qcom,rpmh-regulator.yaml        |    4 +
 .../devicetree/bindings/ufs/qcom,ufs.yaml     |    2 +
 .../devicetree/bindings/usb/qcom,dwc3.yaml    |    2 +
 arch/arm64/boot/dts/qcom/Makefile             |    2 +
 .../boot/dts/qcom/sc8180x-lenovo-flex-5g.dts  |  590 +++
 arch/arm64/boot/dts/qcom/sc8180x-pmics.dtsi   |  326 ++
 arch/arm64/boot/dts/qcom/sc8180x-primus.dts   |  706 +++
 arch/arm64/boot/dts/qcom/sc8180x.dtsi         | 3950 +++++++++++++++++
 15 files changed, 5709 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,sc8180x-rpmh.yaml
 create mode 100644 arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc8180x-pmics.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sc8180x-primus.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc8180x.dtsi

Comments

Konrad Dybcio March 25, 2023, 12:34 p.m. UTC | #1
On 25.03.2023 13:24, Vinod Koul wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.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@linaro.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.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@85700000 {
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@100000 {
> +			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@8c0000 {
> +			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@880000 {
> +				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@1500000 {
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@1620000 {
> +			compatible = "qcom,sc8180x-system-noc";
> +			reg = <0 0x01620000 0 0x19400>;
> +			#interconnect-cells = <2>;
> +			qcom,bcm-voters = <&apps_bcm_voter>;
> +		};
> +
> +		aggre1_noc: interconnect@16e0000 {
> +			compatible = "qcom,sc8180x-aggre1-noc";
> +			reg = <0 0x016e0000 0 0xd080>;
> +			#interconnect-cells = <2>;
> +			qcom,bcm-voters = <&apps_bcm_voter>;
> +		};
> +
> +		aggre2_noc: interconnect@1700000 {
> +			compatible = "qcom,sc8180x-aggre2-noc";
> +			reg = <0 0x01700000 0 0x20000>;
> +			#interconnect-cells = <2>;
> +			qcom,bcm-voters = <&apps_bcm_voter>;
> +		};
> +
> +		compute_noc: interconnect@1720000 {
> +			compatible = "qcom,sc8180x-compute-noc";
> +			reg = <0 0x01720000 0 0x7000>;
> +			#interconnect-cells = <2>;
> +			qcom,bcm-voters = <&apps_bcm_voter>;
> +		};
> +
> +		mmss_noc: interconnect@1740000 {
> +			compatible = "qcom,sc8180x-mmss-noc";
> +			reg = <0 0x01740000 0 0x1c100>;
> +			#interconnect-cells = <2>;
> +			qcom,bcm-voters = <&apps_bcm_voter>;
> +		};
> +
[...]

> +		pcie0: pci@1c00000 {
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@1d84000 {
> +			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@2c00000 {
GPUSS and MDSS related nodes should also go to their separate,
respective patches.

[...]
> +
> +		remoteproc_mpss: remoteproc@4080000 {
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>;
> +	};
> +};
Vinod Koul March 27, 2023, 5:38 a.m. UTC | #2
On 25-03-23, 13:34, Konrad Dybcio wrote:
> 
> 
> On 25.03.2023 13:24, Vinod Koul wrote:
> > From: Bjorn Andersson <bjorn.andersson@linaro.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@linaro.org>
> > Signed-off-by: Vinod Koul <vkoul@kernel.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>;

You mean this or something else?

> > +			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.

Sure

> 
> > +			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?

Maybe :-) but we would like to land the dts now rather than wait more :)

> 
> [...]
> 
> > +	camnoc_virt: interconnect-0{
> Missing space before {

Will fix

> > +		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.

Are you referring to adding -0 for this?

> 
> > +		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@85700000 {
> the -region seems a bit unnecessary in all of these nodes

This is reserved for hyp, I think we should add it here so that we dont
touch this piece..?

> 
> > +			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

Sure

> 
> > +
> > +		gcc: clock-controller@100000 {
> > +			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

ok

> 
> 
> > +		};
> > +
> > +		qupv3_id_0: geniqup@8c0000 {
> > +			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@880000 {
> > +				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.

Will do

> 
> [...]
> 
> > +		config_noc: interconnect@1500000 {
> Interconnect could also realistically go to a separate patch.

Yeah already list is complaining, let me see how to split it up...

> 
> > +			compatible = "qcom,sc8180x-config-noc";
> > +			reg = <0 0x01500000 0 0x7400>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> > +		system_noc: interconnect@1620000 {
> > +			compatible = "qcom,sc8180x-system-noc";
> > +			reg = <0 0x01620000 0 0x19400>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> > +		aggre1_noc: interconnect@16e0000 {
> > +			compatible = "qcom,sc8180x-aggre1-noc";
> > +			reg = <0 0x016e0000 0 0xd080>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> > +		aggre2_noc: interconnect@1700000 {
> > +			compatible = "qcom,sc8180x-aggre2-noc";
> > +			reg = <0 0x01700000 0 0x20000>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> > +		compute_noc: interconnect@1720000 {
> > +			compatible = "qcom,sc8180x-compute-noc";
> > +			reg = <0 0x01720000 0 0x7000>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> > +		mmss_noc: interconnect@1740000 {
> > +			compatible = "qcom,sc8180x-mmss-noc";
> > +			reg = <0 0x01740000 0 0x1c100>;
> > +			#interconnect-cells = <2>;
> > +			qcom,bcm-voters = <&apps_bcm_voter>;
> > +		};
> > +
> [...]
> 
> > +		pcie0: pci@1c00000 {
> 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

ok

> 
> 
> [...]
> 
> > +
> > +		ufs_mem_hc: ufshc@1d84000 {
> > +			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@2c00000 {
> GPUSS and MDSS related nodes should also go to their separate,
> respective patches.

ok

> 
> [...]
> > +
> > +		remoteproc_mpss: remoteproc@4080000 {
> 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>;
> > +	};
> > +};
Krzysztof Kozlowski March 27, 2023, 7:46 a.m. UTC | #3
On 25/03/2023 13:24, Vinod Koul wrote:
> This introduces Qualcomm SC8180x SoC which features in Lenovo Flex 5G
> laptop. This also adds support for Primus platform as well as Lenovo Flex 5G
> laptop.
> 
> I would be great if submaintainers can ack the binding patch so that
> everything can go thru qcom tree

I think Bjorn recently was rejecting taking bindings patches, so what
changed?

Best regards,
Krzysztof
Konrad Dybcio March 27, 2023, 8:49 a.m. UTC | #4
On 27.03.2023 07:38, Vinod Koul wrote:
> On 25-03-23, 13:34, Konrad Dybcio wrote:
>>
>>
>> On 25.03.2023 13:24, Vinod Koul wrote:
>>> From: Bjorn Andersson <bjorn.andersson@linaro.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@linaro.org>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.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>;
> 
> You mean this or something else?
Check

https://lore.kernel.org/lkml/20221102065448.GA10650@thinkpad/T/

> 
>>> +			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.
> 
> Sure
> 
>>
>>> +			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?
> 
> Maybe :-) but we would like to land the dts now rather than wait more :)
> 
>>
>> [...]
>>
>>> +	camnoc_virt: interconnect-0{
>> Missing space before {
> 
> Will fix
> 
>>> +		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.
> 
> Are you referring to adding -0 for this?
I'm not sure which is preferred (-n vs -name), there's a mixed bag
upstream.. Krzysztof?

> 
>>
>>> +		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@85700000 {
>> the -region seems a bit unnecessary in all of these nodes
> 
> This is reserved for hyp, I think we should add it here so that we dont
> touch this piece..?
I meant the '-region' bit in the node names

(label: name@unit-address)

Konrad
> 
>>
>>> +			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
> 
> Sure
> 
>>
>>> +
>>> +		gcc: clock-controller@100000 {
>>> +			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
> 
> ok
> 
>>
>>
>>> +		};
>>> +
>>> +		qupv3_id_0: geniqup@8c0000 {
>>> +			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@880000 {
>>> +				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.
> 
> Will do
> 
>>
>> [...]
>>
>>> +		config_noc: interconnect@1500000 {
>> Interconnect could also realistically go to a separate patch.
> 
> Yeah already list is complaining, let me see how to split it up...
> 
>>
>>> +			compatible = "qcom,sc8180x-config-noc";
>>> +			reg = <0 0x01500000 0 0x7400>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>>> +		system_noc: interconnect@1620000 {
>>> +			compatible = "qcom,sc8180x-system-noc";
>>> +			reg = <0 0x01620000 0 0x19400>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>>> +		aggre1_noc: interconnect@16e0000 {
>>> +			compatible = "qcom,sc8180x-aggre1-noc";
>>> +			reg = <0 0x016e0000 0 0xd080>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>>> +		aggre2_noc: interconnect@1700000 {
>>> +			compatible = "qcom,sc8180x-aggre2-noc";
>>> +			reg = <0 0x01700000 0 0x20000>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>>> +		compute_noc: interconnect@1720000 {
>>> +			compatible = "qcom,sc8180x-compute-noc";
>>> +			reg = <0 0x01720000 0 0x7000>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>>> +		mmss_noc: interconnect@1740000 {
>>> +			compatible = "qcom,sc8180x-mmss-noc";
>>> +			reg = <0 0x01740000 0 0x1c100>;
>>> +			#interconnect-cells = <2>;
>>> +			qcom,bcm-voters = <&apps_bcm_voter>;
>>> +		};
>>> +
>> [...]
>>
>>> +		pcie0: pci@1c00000 {
>> 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
> 
> ok
> 
>>
>>
>> [...]
>>
>>> +
>>> +		ufs_mem_hc: ufshc@1d84000 {
>>> +			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@2c00000 {
>> GPUSS and MDSS related nodes should also go to their separate,
>> respective patches.
> 
> ok
> 
>>
>> [...]
>>> +
>>> +		remoteproc_mpss: remoteproc@4080000 {
>> 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>;
>>> +	};
>>> +};
>
Bjorn Andersson March 27, 2023, 2:12 p.m. UTC | #5
On Mon, Mar 27, 2023 at 09:46:31AM +0200, Krzysztof Kozlowski wrote:
> On 25/03/2023 13:24, Vinod Koul wrote:
> > This introduces Qualcomm SC8180x SoC which features in Lenovo Flex 5G
> > laptop. This also adds support for Primus platform as well as Lenovo Flex 5G
> > laptop.
> > 
> > I would be great if submaintainers can ack the binding patch so that
> > everything can go thru qcom tree
> 
> I think Bjorn recently was rejecting taking bindings patches, so what
> changed?
> 

Nothing changed. In the interest of reducing the risk for merge
conflicts I still think it's best if bindings goes via respective
maintainer trees; so patch 1 is for me...

Regards,
Bjorn
Vinod Koul March 28, 2023, 1:13 p.m. UTC | #6
On 27-03-23, 10:49, Konrad Dybcio wrote:
> On 27.03.2023 07:38, Vinod Koul wrote:
> > On 25-03-23, 13:34, Konrad Dybcio wrote:

> >>> +	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>;
> > 
> > You mean this or something else?
> Check
> 
> https://lore.kernel.org/lkml/20221102065448.GA10650@thinkpad/T/

Good point, thanks

> >>> +		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.
> > 
> > Are you referring to adding -0 for this?
> I'm not sure which is preferred (-n vs -name), there's a mixed bag
> upstream.. Krzysztof?

Either ways this should be consistent, so camnoc_virt:
interconnect-camnoc_virt makes sense rather than arbitrary -0

> >>> +		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@85700000 {
> >> the -region seems a bit unnecessary in all of these nodes
> > 
> > This is reserved for hyp, I think we should add it here so that we dont
> > touch this piece..?
> I meant the '-region' bit in the node names
> 
> (label: name@unit-address)

ack, thanks for clearing my misunderstanding
Konrad Dybcio March 28, 2023, 1:15 p.m. UTC | #7
On 28.03.2023 15:13, Vinod Koul wrote:
> On 27-03-23, 10:49, Konrad Dybcio wrote:
>> On 27.03.2023 07:38, Vinod Koul wrote:
>>> On 25-03-23, 13:34, Konrad Dybcio wrote:
> 
>>>>> +	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>;
>>>
>>> You mean this or something else?
>> Check
>>
>> https://lore.kernel.org/lkml/20221102065448.GA10650@thinkpad/T/
> 
> Good point, thanks
> 
>>>>> +		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.
>>>
>>> Are you referring to adding -0 for this?
>> I'm not sure which is preferred (-n vs -name), there's a mixed bag
>> upstream.. Krzysztof?
> 
> Either ways this should be consistent, so camnoc_virt:
> interconnect-camnoc_virt makes sense rather than arbitrary -0
Yeah, just remember that underscores are forbidden in node names,
use hyphen instead!

Konrad
> 
>>>>> +		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@85700000 {
>>>> the -region seems a bit unnecessary in all of these nodes
>>>
>>> This is reserved for hyp, I think we should add it here so that we dont
>>> touch this piece..?
>> I meant the '-region' bit in the node names
>>
>> (label: name@unit-address)
> 
> ack, thanks for clearing my misunderstanding
>
Mark Brown April 3, 2023, 3:07 p.m. UTC | #8
On Sat, 25 Mar 2023 17:54:32 +0530, Vinod Koul wrote:
> This introduces Qualcomm SC8180x SoC which features in Lenovo Flex 5G
> laptop. This also adds support for Primus platform as well as Lenovo Flex 5G
> laptop.
> 
> I would be great if submaintainers can ack the binding patch so that
> everything can go thru qcom tree
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[08/12] regulator: dt-bindings: qcom,rpmh: Add compatible for PMC8180
        commit: fc4fef625decc80cf3a72e884a4e37288bfa0f9b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Bjorn Andersson April 5, 2023, 4:09 a.m. UTC | #9
On Sat, 25 Mar 2023 17:54:32 +0530, Vinod Koul wrote:
> This introduces Qualcomm SC8180x SoC which features in Lenovo Flex 5G
> laptop. This also adds support for Primus platform as well as Lenovo Flex 5G
> laptop.
> 
> I would be great if submaintainers can ack the binding patch so that
> everything can go thru qcom tree
> 
> [...]

Applied, thanks!

[01/12] dt-bindings: firmware: document Qualcomm SC8180X SCM
        commit: c78ad8597ed961e822bf86ce7f1916dbfba255ef

Best regards,