Skip to contentSkip to conversation

Add DT files for sc7280 IDP2 board

PATCH Superseded Rajendra Nayak 3 participants 7 messages Aug 2, 2021 – Aug 3, 2021

Subsystems (4)
Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support Arm/Qualcomm chromebook support

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rajendra Nayak

author
#0 lore raw

cover Add DT files for sc7280 IDP2 board

The IDP2 board based off sc7280 SoC is derived from the existing IDP platform, but has some differences like
* on board Embedded controller and H1 secure microprocessor
* PMIC changes (some pmic functionality handled by EC)
* MDP/Display power grid changes
* USB configuration changes

To handle the differences, all common functionality is now moved to sc7280-idp.dtsi, which is included in both sc7280-idp and sc7280-idp2 dts files.

Additional functionality differences in IDP2 as listed above will be added by subsequent patches, this series just makes sure we have the split done to handle the commonalities and differences.

Rajendra Nayak (2):
dt-bindings: arm: qcom: Document qcom,sc7280-idp2 board
arm64: dts: qcom: sc7280-idp: Add device tree files for IDP2

 Documentation/devicetree/bindings/arm/qcom.yaml    |   1 +
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            | 352 +--------------------
 .../dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi}   |  48 +--
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts           |  24 ++
 5 files changed, 30 insertions(+), 396 deletions(-)
 copy arch/arm64/boot/dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi} (87%)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-idp2.dts
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#1 lore raw

PATCH 1/2 dt-bindings: arm: qcom: Document qcom,sc7280-idp2 board

Document the qcom,sc7280-idp2 board based off sc7280 SoC

Signed-off-by Rajendra Nayak <[email protected]>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/arm/qcom.yaml +1 -0
index 7163e5f..6264082 100644
@@ -187,6 +187,7 @@ properties:
       - items:
           - enum:
               - qcom,sc7280-idp
+              - qcom,sc7280-idp2
               - google,senor
           - const: qcom,sc7280
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#3 lore raw

PATCH 2/2 arm64: dts: qcom: sc7280-idp: Add device tree files for IDP2

In reply to Rajendra Nayak

Move all the common device tree bits for both sc7280 IDPs into a sc7280-idp.dtsi and create 2 different dts files (sc7280-idp.dts and sc7280-idp2.dts) in order to manage differences across the IDP SKU1 and SKU2 Boards.
PMR735A is present on IDP board only and is not present on IDP2.

Signed-off-by Rajendra Nayak <[email protected]>
---
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            | 352 +--------------------
 .../dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi}   |  48 +--
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts           |  24 ++
 4 files changed, 29 insertions(+), 396 deletions(-)
 copy arch/arm64/boot/dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi} (87%)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-idp2.dts
arch/arm64/boot/dts/qcom/Makefile +1 -0
index a571bfdd..50cf2cf 100644
@@ -61,6 +61,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pompom-r3-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
arch/arm64/boot/dts/qcom/sc7280-idp.dts +3 -349

371 lines not on this page · raw message

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -47

72 lines not on this page · raw message

arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +24 -0
new file mode 100644
index 0000000..c842da6
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * sc7280 IDP2 board device tree source
+ *
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sc7280.dtsi"
+#include "sc7280-idp.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";
+	compatible = "qcom,sc7280-idp2", "qcom,sc7280";
+
+	aliases {
+		serial0 = &uart5;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#4 lore raw

Re: [PATCH 2/2] arm64: dts: qcom: sc7280-idp: Add device tree files for IDP2

Hi!

Quoted 10 lines

Move all the common device tree bits for both sc7280 IDPs into a sc7280-idp.dtsi and create 2 different dts files (sc7280-idp.dts and sc7280-idp2.dts) in order to manage differences across the IDP SKU1 and SKU2 Boards.
PMR735A is present on IDP board only and is not present on IDP2.

Signed-off-by Rajendra Nayak <[email protected]>
---
 arch/arm64/boot/dts/qcom/Makefile                  |   1 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            | 352 +--------------------

At this point, wouldn't it be more clear to rename it to sc7280-idp-sku1.dts and the other one to sc7280-idp-sku2.dts?

Quoted 23 lines
 .../dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi}   |  48 +--
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts           |  24 ++
 4 files changed, 29 insertions(+), 396 deletions(-)
 copy arch/arm64/boot/dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi} (87%)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-idp2.dts
arch/arm64/boot/dts/qcom/Makefile +1 -0
index a571bfdd..50cf2cf 100644
@@ -61,6 +61,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pompom-r3-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -0
index 73225e3..f014cda 100644

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 12

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -3
@@ -7,19 +7,12 @@
 
 /dts-v1/;
 
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>

Don't you want pmr735a include to stay here in SKU1 and not be included in the common one, as the hw is not present on sku2?

Quoted 224 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +3 -203
-#include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include "sc7280.dtsi"
-#include "pm7325.dtsi"
+#include "sc7280-idp.dtsi"
 #include "pmr735a.dtsi"
-#include "pm8350c.dtsi"
-#include "pmk8350.dtsi"
 
 / {
-	model = "Qualcomm Technologies, Inc. sc7280 IDP platform";
+	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform";
 	compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
 
 	aliases {

@@ -31,187 +24,7 @@
 	};
 };
 
-&apps_rsc {
-	pm7325-regulators {
-		compatible = "qcom,pm7325-rpmh-regulators";
-		qcom,pmic-id = "b";
-
-		vreg_s1b_1p8: smps1 {
-			regulator-min-microvolt = <1856000>;
-			regulator-max-microvolt = <2040000>;
-		};
-
-		vreg_s7b_0p9: smps7 {
-			regulator-min-microvolt = <535000>;
-			regulator-max-microvolt = <1120000>;
-		};
-
-		vreg_s8b_1p2: smps8 {
-			regulator-min-microvolt = <1256000>;
-			regulator-max-microvolt = <1500000>;
-		};
-
-		vreg_l1b_0p8: ldo1 {
-			regulator-min-microvolt = <825000>;
-			regulator-max-microvolt = <925000>;
-		};
-
-		vreg_l2b_3p0: ldo2 {
-			regulator-min-microvolt = <2700000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l6b_1p2: ldo6 {
-			regulator-min-microvolt = <1140000>;
-			regulator-max-microvolt = <1260000>;
-		};
-
-		vreg_l7b_2p9: ldo7 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-		};
-
-		vreg_l8b_0p9: ldo8 {
-			regulator-min-microvolt = <870000>;
-			regulator-max-microvolt = <970000>;
-		};
-
-		vreg_l9b_1p2: ldo9 {
-			regulator-min-microvolt = <1080000>;
-			regulator-max-microvolt = <1304000>;
-		};
-
-		vreg_l11b_1p7: ldo11 {
-			regulator-min-microvolt = <1504000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l12b_0p8: ldo12 {
-			regulator-min-microvolt = <751000>;
-			regulator-max-microvolt = <824000>;
-		};
-
-		vreg_l13b_0p8: ldo13 {
-			regulator-min-microvolt = <530000>;
-			regulator-max-microvolt = <824000>;
-		};
-
-		vreg_l14b_1p2: ldo14 {
-			regulator-min-microvolt = <1080000>;
-			regulator-max-microvolt = <1304000>;
-		};
-
-		vreg_l15b_0p8: ldo15 {
-			regulator-min-microvolt = <765000>;
-			regulator-max-microvolt = <1020000>;
-		};
-
-		vreg_l16b_1p2: ldo16 {
-			regulator-min-microvolt = <1100000>;
-			regulator-max-microvolt = <1300000>;
-		};
-
-		vreg_l17b_1p8: ldo17 {
-			regulator-min-microvolt = <1700000>;
-			regulator-max-microvolt = <1900000>;
-		};
-
-		vreg_l18b_1p8: ldo18 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l19b_1p8: ldo19 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-		};
-	};
-
-	pm8350c-regulators {
-		compatible = "qcom,pm8350c-rpmh-regulators";
-		qcom,pmic-id = "c";
-
-		vreg_s1c_2p2: smps1 {
-			regulator-min-microvolt = <2190000>;
-			regulator-max-microvolt = <2210000>;
-		};
-
-		vreg_s9c_1p0: smps9 {
-			regulator-min-microvolt = <1010000>;
-			regulator-max-microvolt = <1170000>;
-		};
-
-		vreg_l1c_1p8: ldo1 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1980000>;
-		};
-
-		vreg_l2c_1p8: ldo2 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <1980000>;
-		};
-
-		vreg_l3c_3p0: ldo3 {
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <3540000>;
-		};
-
-		vreg_l4c_1p8: ldo4 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <3300000>;
-		};
-
-		vreg_l5c_1p8: ldo5 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <3300000>;
-		};
-
-		vreg_l6c_2p9: ldo6 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2950000>;
-		};
-
-		vreg_l7c_3p0: ldo7 {
-			regulator-min-microvolt = <3000000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l8c_1p8: ldo8 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l9c_2p9: ldo9 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-		};
-
-		vreg_l10c_0p8: ldo10 {
-			regulator-min-microvolt = <720000>;
-			regulator-max-microvolt = <1050000>;
-		};
-
-		vreg_l11c_2p8: ldo11 {
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l12c_1p8: ldo12 {
-			regulator-min-microvolt = <1650000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l13c_3p0: ldo13 {
-			regulator-min-microvolt = <2700000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_bob: bob {
-			regulator-min-microvolt = <3008000>;
-			regulator-max-microvolt = <3960000>;
-		};
-	};
-
+&apps_rsc{
 	pmr735a-regulators {
 		compatible = "qcom,pmr735a-rpmh-regulators";
 		qcom,pmic-id = "e";

@@ -242,162 +55,3 @@
 		};
 	};
 };
-
-&pmk8350_vadc {
-		pm8350_die_temp {
-			reg = <PM8350_ADC7_DIE_TEMP>;
-			label = "pm8350_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
-		pmk8350_die_temp {
-			reg = <PMK8350_ADC7_DIE_TEMP>;
-			label = "pmk8350_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
-		pmr735a_die_temp {
-			reg = <PMR735A_ADC7_DIE_TEMP>;

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 270

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -10
-			label = "pmr735a_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
-		pmr735b_die_temp {
-			reg = <PMR735B_ADC7_DIE_TEMP>;
-			label = "pmr735b_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-};

It may be Thunderbird messing with me again, but it looks like there's a tab too many in this block.

Quoted 164 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -133
-
-&qupv3_id_0 {
-	status = "okay";
-};
-
-&sdhc_1 {
-	status = "okay";
-
-	pinctrl-names = "default", "sleep";
-	pinctrl-0 = <&sdc1_on>;
-	pinctrl-1 = <&sdc1_off>;
-
-	non-removable;
-	no-sd;
-	no-sdio;
-
-	vmmc-supply = <&vreg_l7b_2p9>;
-	vqmmc-supply = <&vreg_l19b_1p8>;
-};
-
-&sdhc_2 {
-	status = "okay";
-
-	pinctrl-names = "default", "sleep";
-	pinctrl-0 = <&sdc2_on>;
-	pinctrl-1 = <&sdc2_off>;
-
-	vmmc-supply = <&vreg_l9c_2p9>;
-	vqmmc-supply = <&vreg_l6c_2p9>;
-
-	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
-};
-
-&uart5 {
-	status = "okay";
-};
-
-&usb_1 {
-	status = "okay";
-};
-
-&usb_1_dwc3 {
-	dr_mode = "host";
-};
-
-&usb_1_hsphy {
-	status = "okay";
-
-	vdda-pll-supply = <&vreg_l10c_0p8>;
-	vdda33-supply = <&vreg_l2b_3p0>;
-	vdda18-supply = <&vreg_l1c_1p8>;
-};
-
-&usb_1_qmpphy {
-	status = "okay";
-
-	vdda-phy-supply = <&vreg_l6b_1p2>;
-	vdda-pll-supply = <&vreg_l1b_0p8>;
-};
-
-&usb_2 {
-	status = "okay";
-};
-
-&usb_2_dwc3 {
-	dr_mode = "peripheral";
-};
-
-&usb_2_hsphy {
-	status = "okay";
-
-	vdda-pll-supply = <&vreg_l10c_0p8>;
-	vdda33-supply = <&vreg_l2b_3p0>;
-	vdda18-supply = <&vreg_l1c_1p8>;
-};
-
-/* PINCTRL - additions to nodes defined in sc7280.dtsi */
-
-&qup_uart5_default {
-	tx {
-		pins = "gpio46";
-		drive-strength = <2>;
-		bias-disable;
-	};
-
-	rx {
-		pins = "gpio47";
-		drive-strength = <2>;
-		bias-pull-up;
-	};
-};
-
-&sdc1_on {
-	clk {
-		bias-disable;
-		drive-strength = <16>;
-	};
-
-	cmd {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	data {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	rclk {
-		bias-pull-down;
-	};
-};
-
-&sdc2_on {
-	clk {
-		bias-disable;
-		drive-strength = <16>;
-	};
-
-	cmd {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	data {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	sd-cd {
-		bias-pull-up;
-	};
-};
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -6
similarity index 87%
copy from arch/arm64/boot/dts/qcom/sc7280-idp.dts
copy to arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 73225e3..295d88a 100644
@@ -1,12 +1,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 /*
- * sc7280 IDP board device tree source
+ * sc7280 IDP board device tree source (common between SKU1 and SKU2)
  *
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
  */
 
-/dts-v1/;
-
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>

@@ -14,23 +12,9 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
-#include "pmr735a.dtsi"
 #include "pm8350c.dtsi"
 #include "pmk8350.dtsi"
 
-/ {
-	model = "Qualcomm Technologies, Inc. sc7280 IDP platform";

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi line 32

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -10
-	compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
-
-	aliases {
-		serial0 = &uart5;
-	};
-
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-};

Shouldn't these be common if they were there before on SKU1 and now are there on SKU2?

Quoted 47 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -31
-
 &apps_rsc {
 	pm7325-regulators {
 		compatible = "qcom,pm7325-rpmh-regulators";

@@ -211,36 +195,6 @@
 			regulator-max-microvolt = <3960000>;
 		};
 	};
-
-	pmr735a-regulators {
-		compatible = "qcom,pmr735a-rpmh-regulators";
-		qcom,pmic-id = "e";
-
-		vreg_l2e_1p2: ldo2 {
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-		};
-
-		vreg_l3e_0p9: ldo3 {
-			regulator-min-microvolt = <912000>;
-			regulator-max-microvolt = <1020000>;
-		};
-
-		vreg_l4e_1p7: ldo4 {
-			regulator-min-microvolt = <1776000>;
-			regulator-max-microvolt = <1890000>;
-		};
-
-		vreg_l5e_0p8: ldo5 {
-			regulator-min-microvolt = <800000>;
-			regulator-max-microvolt = <800000>;
-		};
-
-		vreg_l6e_0p8: ldo6 {
-			regulator-min-microvolt = <480000>;
-			regulator-max-microvolt = <904000>;
-		};
-	};
 };
 
 &pmk8350_vadc {
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +0 -0
new file mode 100644
index 0000000..c842da6

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 10

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +10 -0
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * sc7280 IDP2 board device tree source
+ *
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sc7280.dtsi"

sc7280.dtsi is already included in the sc7280-idp.dtsi file.

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 15

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +5 -0
+#include "sc7280-idp.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";
+	compatible = "qcom,sc7280-idp2", "qcom,sc7280";

No "google,something" compatible on SKU2?

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 24

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +9 -0
+
+	aliases {
+		serial0 = &uart5;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};

Konrad

author
#5 lore raw

Re: [PATCH 2/2] arm64: dts: qcom: sc7280-idp: Add device tree files for IDP2

Quoted 8 lines · Konrad Dybcio

Hi!

Quoted 6 lines

Move all the common device tree bits for both sc7280 IDPs into a sc7280-idp.dtsi and create 2 different dts files (sc7280-idp.dts and sc7280-idp2.dts) in order to manage differences across the IDP SKU1 and SKU2 Boards.
PMR735A is present on IDP board only and is not present on IDP2.

Quoted 6 lines
Quoted 4 lines
Signed-off-by Rajendra Nayak <[email protected]>
---
  arch/arm64/boot/dts/qcom/Makefile                  |   1 +
  arch/arm64/boot/dts/qcom/sc7280-idp.dts            | 352 +--------------------

At this point, wouldn't it be more clear to rename it to sc7280-idp-sku1.dts and the other one to sc7280-idp-sku2.dts?

I wasn't really sure if its a good idea to change an existing board dts name and if that's going to break something someplace with build scripts etc, but if none of that's a concern, then yes, perhaps I could name them idp1 and idp2 instead of the current idp and idp2 which I agree is a little confusing.

Quoted 24 lines
Quoted 24 lines
  .../dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi}   |  48 +--
  arch/arm64/boot/dts/qcom/sc7280-idp2.dts           |  24 ++
  4 files changed, 29 insertions(+), 396 deletions(-)

copy arch/arm64/boot/dts/qcom/{sc7280-idp.dts => sc7280-idp.dtsi} (87%) create mode 100644 arch/arm64/boot/dts/qcom/sc7280-idp2.dts

arch/arm64/boot/dts/qcom/Makefile +1 -0
index a571bfdd..50cf2cf 100644
@@ -61,6 +61,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pompom-r3-lte.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -0
index 73225e3..f014cda 100644
@@ -7,19 +7,12 @@
Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 12

Quoted 5 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -3
  /dts-v1/;
 
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>

Don't you want pmr735a include to stay here in SKU1 and not be included in the common one, as the hw is not present on sku2?

[]..

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 12

Quoted 1 line
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -0
  #include "pmr735a.dtsi"

Right, that's why this ^^^ include stays here and is not part of the common dtsi.

Quoted 222 lines
Quoted 222 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +2 -204
-#include "pm8350c.dtsi"
-#include "pmk8350.dtsi"
 
  / {
-	model = "Qualcomm Technologies, Inc. sc7280 IDP platform";
+	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform";
  	compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
 
  	aliases {

@@ -31,187 +24,7 @@
  	};
  };
 
-&apps_rsc {
-	pm7325-regulators {
-		compatible = "qcom,pm7325-rpmh-regulators";
-		qcom,pmic-id = "b";
-
-		vreg_s1b_1p8: smps1 {
-			regulator-min-microvolt = <1856000>;
-			regulator-max-microvolt = <2040000>;
-		};
-
-		vreg_s7b_0p9: smps7 {
-			regulator-min-microvolt = <535000>;
-			regulator-max-microvolt = <1120000>;
-		};
-
-		vreg_s8b_1p2: smps8 {
-			regulator-min-microvolt = <1256000>;
-			regulator-max-microvolt = <1500000>;
-		};
-
-		vreg_l1b_0p8: ldo1 {
-			regulator-min-microvolt = <825000>;
-			regulator-max-microvolt = <925000>;
-		};
-
-		vreg_l2b_3p0: ldo2 {
-			regulator-min-microvolt = <2700000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l6b_1p2: ldo6 {
-			regulator-min-microvolt = <1140000>;
-			regulator-max-microvolt = <1260000>;
-		};
-
-		vreg_l7b_2p9: ldo7 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-		};
-
-		vreg_l8b_0p9: ldo8 {
-			regulator-min-microvolt = <870000>;
-			regulator-max-microvolt = <970000>;
-		};
-
-		vreg_l9b_1p2: ldo9 {
-			regulator-min-microvolt = <1080000>;
-			regulator-max-microvolt = <1304000>;
-		};
-
-		vreg_l11b_1p7: ldo11 {
-			regulator-min-microvolt = <1504000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l12b_0p8: ldo12 {
-			regulator-min-microvolt = <751000>;
-			regulator-max-microvolt = <824000>;
-		};
-
-		vreg_l13b_0p8: ldo13 {
-			regulator-min-microvolt = <530000>;
-			regulator-max-microvolt = <824000>;
-		};
-
-		vreg_l14b_1p2: ldo14 {
-			regulator-min-microvolt = <1080000>;
-			regulator-max-microvolt = <1304000>;
-		};
-
-		vreg_l15b_0p8: ldo15 {
-			regulator-min-microvolt = <765000>;
-			regulator-max-microvolt = <1020000>;
-		};
-
-		vreg_l16b_1p2: ldo16 {
-			regulator-min-microvolt = <1100000>;
-			regulator-max-microvolt = <1300000>;
-		};
-
-		vreg_l17b_1p8: ldo17 {
-			regulator-min-microvolt = <1700000>;
-			regulator-max-microvolt = <1900000>;
-		};
-
-		vreg_l18b_1p8: ldo18 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l19b_1p8: ldo19 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1800000>;
-		};
-	};
-
-	pm8350c-regulators {
-		compatible = "qcom,pm8350c-rpmh-regulators";
-		qcom,pmic-id = "c";
-
-		vreg_s1c_2p2: smps1 {
-			regulator-min-microvolt = <2190000>;
-			regulator-max-microvolt = <2210000>;
-		};
-
-		vreg_s9c_1p0: smps9 {
-			regulator-min-microvolt = <1010000>;
-			regulator-max-microvolt = <1170000>;
-		};
-
-		vreg_l1c_1p8: ldo1 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <1980000>;
-		};
-
-		vreg_l2c_1p8: ldo2 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <1980000>;
-		};
-
-		vreg_l3c_3p0: ldo3 {
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <3540000>;
-		};
-
-		vreg_l4c_1p8: ldo4 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <3300000>;
-		};
-
-		vreg_l5c_1p8: ldo5 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <3300000>;
-		};
-
-		vreg_l6c_2p9: ldo6 {
-			regulator-min-microvolt = <1800000>;
-			regulator-max-microvolt = <2950000>;
-		};
-
-		vreg_l7c_3p0: ldo7 {
-			regulator-min-microvolt = <3000000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l8c_1p8: ldo8 {
-			regulator-min-microvolt = <1620000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l9c_2p9: ldo9 {
-			regulator-min-microvolt = <2960000>;
-			regulator-max-microvolt = <2960000>;
-		};
-
-		vreg_l10c_0p8: ldo10 {
-			regulator-min-microvolt = <720000>;
-			regulator-max-microvolt = <1050000>;
-		};
-
-		vreg_l11c_2p8: ldo11 {
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_l12c_1p8: ldo12 {
-			regulator-min-microvolt = <1650000>;
-			regulator-max-microvolt = <2000000>;
-		};
-
-		vreg_l13c_3p0: ldo13 {
-			regulator-min-microvolt = <2700000>;
-			regulator-max-microvolt = <3544000>;
-		};
-
-		vreg_bob: bob {
-			regulator-min-microvolt = <3008000>;
-			regulator-max-microvolt = <3960000>;
-		};
-	};
-
+&apps_rsc{
  	pmr735a-regulators {
  		compatible = "qcom,pmr735a-rpmh-regulators";
  		qcom,pmic-id = "e";

@@ -242,162 +55,3 @@
  		};
  	};
  };
-
-&pmk8350_vadc {
-		pm8350_die_temp {
-			reg = <PM8350_ADC7_DIE_TEMP>;
-			label = "pm8350_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
-		pmk8350_die_temp {
-			reg = <PMK8350_ADC7_DIE_TEMP>;
-			label = "pmk8350_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
-		pmr735a_die_temp {
-			reg = <PMR735A_ADC7_DIE_TEMP>;
-			label = "pmr735a_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-
Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 270

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -6
-		pmr735b_die_temp {
-			reg = <PMR735B_ADC7_DIE_TEMP>;
-			label = "pmr735b_die_temp";
-			qcom,pre-scaling = <1 1>;
-		};
-};

It may be Thunderbird messing with me again, but it looks like there's a tab too many in this block.

Its not your mailer, the existing file seems to have the extra tab.

Quoted 162 lines
Quoted 162 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -133
-
-&qupv3_id_0 {
-	status = "okay";
-};
-
-&sdhc_1 {
-	status = "okay";
-
-	pinctrl-names = "default", "sleep";
-	pinctrl-0 = <&sdc1_on>;
-	pinctrl-1 = <&sdc1_off>;
-
-	non-removable;
-	no-sd;
-	no-sdio;
-
-	vmmc-supply = <&vreg_l7b_2p9>;
-	vqmmc-supply = <&vreg_l19b_1p8>;
-};
-
-&sdhc_2 {
-	status = "okay";
-
-	pinctrl-names = "default", "sleep";
-	pinctrl-0 = <&sdc2_on>;
-	pinctrl-1 = <&sdc2_off>;
-
-	vmmc-supply = <&vreg_l9c_2p9>;
-	vqmmc-supply = <&vreg_l6c_2p9>;
-
-	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
-};
-
-&uart5 {
-	status = "okay";
-};
-
-&usb_1 {
-	status = "okay";
-};
-
-&usb_1_dwc3 {
-	dr_mode = "host";
-};
-
-&usb_1_hsphy {
-	status = "okay";
-
-	vdda-pll-supply = <&vreg_l10c_0p8>;
-	vdda33-supply = <&vreg_l2b_3p0>;
-	vdda18-supply = <&vreg_l1c_1p8>;
-};
-
-&usb_1_qmpphy {
-	status = "okay";
-
-	vdda-phy-supply = <&vreg_l6b_1p2>;
-	vdda-pll-supply = <&vreg_l1b_0p8>;
-};
-
-&usb_2 {
-	status = "okay";
-};
-
-&usb_2_dwc3 {
-	dr_mode = "peripheral";
-};
-
-&usb_2_hsphy {
-	status = "okay";
-
-	vdda-pll-supply = <&vreg_l10c_0p8>;
-	vdda33-supply = <&vreg_l2b_3p0>;
-	vdda18-supply = <&vreg_l1c_1p8>;
-};
-
-/* PINCTRL - additions to nodes defined in sc7280.dtsi */
-
-&qup_uart5_default {
-	tx {
-		pins = "gpio46";
-		drive-strength = <2>;
-		bias-disable;
-	};
-
-	rx {
-		pins = "gpio47";
-		drive-strength = <2>;
-		bias-pull-up;
-	};
-};
-
-&sdc1_on {
-	clk {
-		bias-disable;
-		drive-strength = <16>;
-	};
-
-	cmd {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	data {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	rclk {
-		bias-pull-down;
-	};
-};
-
-&sdc2_on {
-	clk {
-		bias-disable;
-		drive-strength = <16>;
-	};
-
-	cmd {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	data {
-		bias-pull-up;
-		drive-strength = <10>;
-	};
-
-	sd-cd {
-		bias-pull-up;
-	};
-};
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -4
similarity index 87%
copy from arch/arm64/boot/dts/qcom/sc7280-idp.dts
copy to arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 73225e3..295d88a 100644
@@ -1,12 +1,10 @@
  // SPDX-License-Identifier: BSD-3-Clause
  /*
- * sc7280 IDP board device tree source
+ * sc7280 IDP board device tree source (common between SKU1 and SKU2)
   *
   * Copyright (c) 2021, The Linux Foundation. All rights reserved.
   */
 
-/dts-v1/;
-
  #include <dt-bindings/gpio/gpio.h>
  #include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
  #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>

@@ -14,23 +12,9 @@
  #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
  #include "sc7280.dtsi"
  #include "pm7325.dtsi"
-#include "pmr735a.dtsi"
  #include "pm8350c.dtsi"
  #include "pmk8350.dtsi"
Quoted 14 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dtsi line 32

Quoted 12 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -12
-/ {
-	model = "Qualcomm Technologies, Inc. sc7280 IDP platform";
-	compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
-
-	aliases {
-		serial0 = &uart5;
-	};
-
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-};

Shouldn't these be common if they were there before on SKU1 and now are there on SKU2?

I was expecting other things inside of these nodes to be different as more patches get added on top so left these in the resp. dts files.

Quoted 51 lines
Quoted 51 lines
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi renamed from arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -31
-
  &apps_rsc {
  	pm7325-regulators {
  		compatible = "qcom,pm7325-rpmh-regulators";

@@ -211,36 +195,6 @@
  			regulator-max-microvolt = <3960000>;
  		};
  	};
-
-	pmr735a-regulators {
-		compatible = "qcom,pmr735a-rpmh-regulators";
-		qcom,pmic-id = "e";
-
-		vreg_l2e_1p2: ldo2 {
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-		};
-
-		vreg_l3e_0p9: ldo3 {
-			regulator-min-microvolt = <912000>;
-			regulator-max-microvolt = <1020000>;
-		};
-
-		vreg_l4e_1p7: ldo4 {
-			regulator-min-microvolt = <1776000>;
-			regulator-max-microvolt = <1890000>;
-		};
-
-		vreg_l5e_0p8: ldo5 {
-			regulator-min-microvolt = <800000>;
-			regulator-max-microvolt = <800000>;
-		};
-
-		vreg_l6e_0p8: ldo6 {
-			regulator-min-microvolt = <480000>;
-			regulator-max-microvolt = <904000>;
-		};
-	};
  };
 
  &pmk8350_vadc {
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +4 -0
new file mode 100644
index 0000000..c842da6
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * sc7280 IDP2 board device tree source
+ *
Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 10

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +6 -0
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sc7280.dtsi"

sc7280.dtsi is already included in the sc7280-idp.dtsi file.

ah yes, I will drop this.

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 15

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +5 -0
+#include "sc7280-idp.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";
+	compatible = "qcom,sc7280-idp2", "qcom,sc7280";

No "google,something" compatible on SKU2?

Thats a good point, I need to go back and check with how this would be handled by the bootloader, thanks for catching it.

Appreciate you taking time out to review.

Quoted 14 lines

arch/arm64/boot/dts/qcom/sc7280-idp2.dts line 24

Quoted 11 lines
arch/arm64/boot/dts/qcom/sc7280-idp2.dts new file +9 -0
+
+	aliases {
+		serial0 = &uart5;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};

Konrad

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#6 lore raw

Re: [PATCH 2/2] arm64: dts: qcom: sc7280-idp: Add device tree files for IDP2

Quoted 8 lines · Rajendra Nayak
Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 11

Quoted 8 lines

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts index 73225e3..f014cda 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -7,19 +7,12 @@
  /dts-v1/;

arch/arm64/boot/dts/qcom/sc7280-idp.dts +0 -2
-#include <dt-bindings/gpio/gpio.h>
-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>

I re-read your comment and I guess you meant this ^^^ header include should stay too, and that's right, I seem to have wrongly moved it to the common dtsi, will fix when I respost. thanks.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

-#include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>

Don't you want pmr735a include to stay here in SKU1 and not be included in the common one, as the hw is not present on sku2?

[]..

Quoted 1 line
Quoted 1 line

  #include "pmr735a.dtsi"

Right, that's why this ^^^ include stays here and is not part of the common dtsi.

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation