Re: [PATCH 4/5] arm64: dts: qcom: Add base sm8750 dtsi and mtp and qrd dts

From: Melody Olvera
Date: Wed Oct 23 2024 - 17:59:07 EST



On 10/22/2024 8:12 PM, Bjorn Andersson wrote:
On Mon, Oct 21, 2024 at 04:21:13PM GMT, Melody Olvera wrote:

2 "and", 2 "dts" and a "dtsi" in the subject. I'd prefer a more
succinct:

"arm64: dts: qcom: Add SM8750 platform, mtp and qrd"

Ack.


Add base dtsi for the sm8750 SoC describing the CPUs, GCC and
RPMHCC clock controllers, geni UART, interrupt controller, TLMM,
reserved memory, interconnects, regulator, and SMMU nodes. Also add
MTP and QRD board dts files for sm8750.

Nice.

Co-developed-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
Co-developed-by: Jishnu Prakash <quic_jprakash@xxxxxxxxxxx>
Signed-off-by: Jishnu Prakash <quic_jprakash@xxxxxxxxxxx>
Co-developed-by: Raviteja Laggyshetty <quic_rlaggysh@xxxxxxxxxxx>
Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@xxxxxxxxxxx>
Signed-off-by: Melody Olvera <quic_molvera@xxxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/Makefile | 2 +
arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 968 ++++++++
arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 965 ++++++++
arch/arm64/boot/dts/qcom/sm8750.dtsi | 2903 +++++++++++++++++++++++
4 files changed, 4838 insertions(+)
Most reviewers will prefer the platform and individual devices to be
added in separate patches, to not hit 5kloc. I don't mind it...

I wasn't sure whether or not to split; some recent efforts had combined platform
and devices into one patch. Either works for me.


create mode 100644 arch/arm64/boot/dts/qcom/sm8750-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sm8750-qrd.dts
create mode 100644 arch/arm64/boot/dts/qcom/sm8750.dtsi

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 065bb19481c1..3bedfa6b37f2 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -269,6 +269,8 @@ dtb-$(CONFIG_ARCH_QCOM) += sm8650-hdk-display-card.dtb
dtb-$(CONFIG_ARCH_QCOM) += sm8650-hdk.dtb
dtb-$(CONFIG_ARCH_QCOM) += sm8650-mtp.dtb
dtb-$(CONFIG_ARCH_QCOM) += sm8650-qrd.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sm8750-mtp.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sm8750-qrd.dtb
dtb-$(CONFIG_ARCH_QCOM) += x1e78100-lenovo-thinkpad-t14s.dtb
dtb-$(CONFIG_ARCH_QCOM) += x1e80100-asus-vivobook-s15.dtb
dtb-$(CONFIG_ARCH_QCOM) += x1e80100-crd.dtb
diff --git a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
new file mode 100644
index 000000000000..e1a94dc76e2f
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
@@ -0,0 +1,968 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sm8750.dtsi"
+#include "pm8010.dtsi"
+#include "pm8550.dtsi"
+#define PMK8550VE_SID 8
+#include "pm8550ve.dtsi"
+#include "pmd8028.dtsi"
+#include "pmih0108.dtsi"
+#include "pmk8550.dtsi"
+#include "pmr735d_a.dtsi"
+
+/ {
+ model = "Qualcomm Technologies, Inc. SM8750 MTP";
+ compatible = "qcom,sm8750-mtp", "qcom,sm8750";
Can we please here have a:

chassis-type = "handset";

Ack.


+
[..]
diff --git a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
new file mode 100644
index 000000000000..af0174d95e3a
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
@@ -0,0 +1,965 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sm8750.dtsi"
+#include "pm8010.dtsi"
+#include "pm8550.dtsi"
+#define PMK8550VE_SID 8
+#include "pm8550ve.dtsi"
+#include "pmd8028.dtsi"
+#include "pmih0108.dtsi"
+#include "pmk8550.dtsi"
+#include "pmr735d_a.dtsi"
+
+/ {
+ model = "Qualcomm Technologies, Inc. SM8750 QRD";
+ compatible = "qcom,sm8750-qrd", "qcom,sm8750";
chassis-type please.

Ack.


+
+ aliases {
+ serial0 = &uart7;
+ };
[..]
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
new file mode 100644
index 000000000000..98ab82caa007
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -0,0 +1,2903 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <dt-bindings/clock/qcom,rpmh.h>
+#include <dt-bindings/clock/qcom,sm8750-gcc.h>
+#include <dt-bindings/clock/qcom,sm8750-tcsr.h>
+#include <dt-bindings/dma/qcom-gpi.h>
+#include <dt-bindings/interconnect/qcom,icc.h>
+#include <dt-bindings/interconnect/qcom,sm8750-rpmh.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/power/qcom,rpmhpd.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
+#include <dt-bindings/soc/qcom,rpmh-rsc.h>
+
+/ {
+ interrupt-parent = <&intc>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ chosen { };
You should be able to omit this...

Ack.


+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
I just merged a cleanup series from Krzysztof, making all labels lower
case. So, I'd unfortunately like you to do the same...

Understood.


+ device_type = "cpu";
+ compatible = "qcom,oryon";
+ reg = <0x0 0x0>;
+ enable-method = "psci";
+ next-level-cache = <&L2_0>;
+ power-domains = <&CPU_PD0>;
+ power-domain-names = "psci";
+ cpu-idle-states = <&CLUSTER0_C4>;
+
+ L2_0: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ cache-unified;
+ };
+ };
[..]
+ memory@a0000000 {
+ device_type = "memory";
+ /* We expect the bootloader to fill in the size */
+ reg = <0x0 0xa0000000 0x0 0x0>;
gunyah_hyp_mem below is at 0x80000000, so I presume it's not just size
that's being updated?

Yes; I believe it adds some more regions as well.


+ };
+
[..]
+ soc: soc@0 {
[..]
+ };
+
+
Extra newline, didn't checkpatch --strict complain about that/

Didn't realize I needed to run w --strict; will remove extra line.


+ cnoc_main: interconnect@1500000 {
+ compatible = "qcom,sm8750-cnoc-main";
+ reg = <0x0 0x1500000 0x0 0x16080>;
Until here you padded the base address to 8 digits, making it easy for
humans to keep nodes sorted (by address). Please correct the remaining
7-digit nodes as well, to make it easier for future contributors to get
their ordering right.

Will do.


+ qcom,bcm-voters = <&apps_bcm_voter>;
+ #interconnect-cells = <2>;
+ };
+
+ config_noc: interconnect@1600000 {
+ compatible = "qcom,sm8750-cnoc-cfg";
+ reg = <0x0 0x1600000 0x0 0x6200>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ #interconnect-cells = <2>;
+ };
[..]
+ apps_rsc: rsc@16500000 {
+ compatible = "qcom,rpmh-rsc";
+ reg = <0x0 0x16500000 0x0 0x10000>,
+ <0x0 0x16510000 0x0 0x10000>,
+ <0x0 0x16520000 0x0 0x10000>;
+ reg-names = "drv-0",
+ "drv-1",
+ "drv-2";
+ qcom,drv-count = <3>;
+
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+ qcom,tcs-offset = <0xd00>;
+ qcom,drv-id = <2>;
+ qcom,tcs-config = <ACTIVE_TCS 3>, <SLEEP_TCS 2>,
+ <WAKE_TCS 2>, <CONTROL_TCS 0>;
Is 0 CONTROL_TCSs correct?

I believe so; I have no reason to think otherwise.

Thanks,
Melody

+
+ label = "apps_rsc";
+