All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1
@ 2022-01-25 22:44 Douglas Anderson
  2022-01-25 22:44 ` [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address Douglas Anderson
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Akhil P Oommen, Andy Gross,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

This series adds support for herobrine-rev1. Note that it's likely
that with the introduction of -rev1 we can drop -rev0 support, but
we'll keep it for now (though we won't try to "fit it in" and share
code with it).

This series is confirmed to boot herobrine-rev1 atop mainline, commit
0280e3c58f92 ("Merge tag 'nfs-for-5.17-1' of
git://git.linux-nfs.org/projects/anna/linux-nfs"), though it requires
a hack to work around a misconfigured DMA for i2c14
(https://crrev.com/c/3378660)

Changes in v2:
- memory-region syntax change as per Stephen.
- ("Factor gpio.h include to sc7280.dtsi") new for v2
- Herobrine compatible on one line, not two
- Wording change in comments for components enabled per-board
- Always sort "bias" above "drive-strength" in pinctrl.
- Properly sort "hub_en" pinctrl.
- Two comments moved from multiline to single line.
- Space after "/delete-property/"

Douglas Anderson (5):
  arm64: dts: qcom: sc7280: Fix gmu unit address
  arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts
  arm64: dts: qcom: sc7280: Factor out Chrome common fragment
  arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi
  arm64: dts: qcom: sc7280: Add herobrine-r1

 arch/arm64/boot/dts/qcom/Makefile             |    3 +-
 .../boot/dts/qcom/sc7280-chrome-common.dtsi   |   97 ++
 .../qcom/sc7280-herobrine-herobrine-r0.dts    | 1350 +++++++++++++++++
 .../qcom/sc7280-herobrine-herobrine-r1.dts    |  313 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dts |   14 -
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 1056 +++----------
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      |   76 +-
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    |  553 +++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |    3 +-
 9 files changed, 2530 insertions(+), 935 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
 delete mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi

-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
@ 2022-01-25 22:44 ` Douglas Anderson
  2022-02-01  7:27   ` Akhil P Oommen
  2022-01-25 22:44 ` [PATCH v2 2/5] arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts Douglas Anderson
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Akhil P Oommen, Andy Gross,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

When processing sc7280 device trees, I can see:

  Warning (simple_bus_reg): /soc@0/gmu@3d69000:
    simple-bus unit address format error, expected "3d6a000"

There's a clear typo in the node name. Fix it.

Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---

(no changes since v1)

 arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 937c2e0e93eb..eab7a8505053 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1790,7 +1790,7 @@ opp-550000000 {
 			};
 		};
 
-		gmu: gmu@3d69000 {
+		gmu: gmu@3d6a000 {
 			compatible="qcom,adreno-gmu-635.0", "qcom,adreno-gmu";
 			reg = <0 0x03d6a000 0 0x34000>,
 				<0 0x3de0000 0 0x10000>,
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/5] arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
  2022-01-25 22:44 ` [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address Douglas Anderson
@ 2022-01-25 22:44 ` Douglas Anderson
  2022-01-25 22:44 ` [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment Douglas Anderson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Andy Gross, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

The upcoming herobrine-r1 board is really not very similar to
herobrine-r0. Let's get rid of the "herobrine.dtsi" file and stick all
the content in the -r0 dts file directly. We'll also rename the dts so
it's obvious that it's just for -r0.

While renaming, let's actually name the file so it's obvious that
"herobrine" is both the name of the board and the name of the
"baseboard". In other words "herobrine" is an actual board but also
often used as the name of a whole class of similar boards that forked
from a design. While "herobrine-herobrine" is a bit of mouthful it
makes it more obvious which things are part of an actual board rather
than the baseboard.

NOTE: herobrine-rev0's days are likely doomed and this device tree is
likely to be deleted in the future.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---

(no changes since v1)

 arch/arm64/boot/dts/qcom/Makefile                  |  2 +-
 ...rine.dtsi => sc7280-herobrine-herobrine-r0.dts} |  6 ++++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dts      | 14 --------------
 3 files changed, 7 insertions(+), 15 deletions(-)
 rename arch/arm64/boot/dts/qcom/{sc7280-herobrine.dtsi => sc7280-herobrine-herobrine-r0.dts} (99%)
 delete mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index f7232052d286..9db743826391 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -82,7 +82,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-pompom-r3.dtb
 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-herobrine.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-herobrine-herobrine-r0.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-crd.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
similarity index 99%
rename from arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
rename to arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index 4619fa9fcacd..8676c93590b5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -22,6 +22,12 @@
 #include "pm8350c.dtsi"
 #include "pmk8350.dtsi"
 
+/ {
+	model = "Google Herobrine (rev0)";
+	compatible = "google,herobrine",
+		     "qcom,sc7280";
+};
+
 /*
  * Reserved memory changes
  *
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dts
deleted file mode 100644
index 7a92679a688b..000000000000
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dts
+++ /dev/null
@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
-/*
- * Google Herobrine board device tree source
- *
- * Copyright 2021 Google LLC.
- */
-
-#include "sc7280-herobrine.dtsi"
-
-/ {
-	model = "Google Herobrine";
-	compatible = "google,herobrine",
-		     "qcom,sc7280";
-};
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
  2022-01-25 22:44 ` [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address Douglas Anderson
  2022-01-25 22:44 ` [PATCH v2 2/5] arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts Douglas Anderson
@ 2022-01-25 22:44 ` Douglas Anderson
  2022-02-01  5:19   ` (subset) " Bjorn Andersson
  2022-01-25 22:44 ` [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi Douglas Anderson
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Andy Gross, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

This factors out a device tree fragment from some sc7280 device
trees. It represents the device tree bits that should be included for
"Chrome" based sc7280 boards. On these boards the bootloader (Coreboot
+ Depthcharge) configures things slightly different than the
bootloader that Qualcomm provides. The modem firmware on these boards
also works differently than on other Qulacomm products and thus the
reserved memory map needs to be adjusted.

NOTES:
- This is _not_ quite a no-op change. The "herobrine" and "idp"
  fragments here were different and it looks like someone simply
  forgot to update the herobrine version. This updates a few numbers
  to match IDP. This will also cause the `pmk8350_pon` to be disabled
  on idp/crd, which I belive is a correct change.
- At the moment this assumes LTE skus. Once it's clearer how WiFi SKUs
  will work (how much of the memory map they can reclaim) we may add
  an extra fragment that will rejigger one way or the other.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---

Changes in v2:
- memory-region syntax change as per Stephen.

 .../boot/dts/qcom/sc7280-chrome-common.dtsi   | 97 +++++++++++++++++++
 .../qcom/sc7280-herobrine-herobrine-r0.dts    | 70 +------------
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      | 75 +-------------
 3 files changed, 101 insertions(+), 141 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
new file mode 100644
index 000000000000..9f4a9c263c35
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * sc7280 fragment for devices with Chrome bootloader
+ *
+ * This file mainly tries to abstract out the memory protections put into
+ * place by the Chrome bootloader which are different than what's put into
+ * place by Qualcomm's typical bootloader. It also has a smattering of other
+ * things that will hold true for any conceivable Chrome design
+ *
+ * Copyright 2022 Google LLC.
+ */
+
+/*
+ * Reserved memory changes
+ *
+ * Delete all unused memory nodes and define the peripheral memory regions
+ * required by the setup for Chrome boards.
+ */
+
+/delete-node/ &hyp_mem;
+/delete-node/ &xbl_mem;
+/delete-node/ &reserved_xbl_uefi_log;
+/delete-node/ &sec_apps_mem;
+
+/ {
+	reserved-memory {
+		adsp_mem: memory@86700000 {
+			reg = <0x0 0x86700000 0x0 0x2800000>;
+			no-map;
+		};
+
+		camera_mem: memory@8ad00000 {
+			reg = <0x0 0x8ad00000 0x0 0x500000>;
+			no-map;
+		};
+
+		venus_mem: memory@8b200000 {
+			reg = <0x0 0x8b200000 0x0 0x500000>;
+			no-map;
+		};
+
+		mpss_mem: memory@8b800000 {
+			reg = <0x0 0x8b800000 0x0 0xf600000>;
+			no-map;
+		};
+
+		wpss_mem: memory@9ae00000 {
+			reg = <0x0 0x9ae00000 0x0 0x1900000>;
+			no-map;
+		};
+
+		mba_mem: memory@9c700000 {
+			reg = <0x0 0x9c700000 0x0 0x200000>;
+			no-map;
+		};
+	};
+};
+
+/* The PMIC PON code isn't compatible w/ how Chrome EC/BIOS handle things. */
+&pmk8350_pon {
+	status = "disabled";
+};
+
+/*
+ * Chrome designs always boot from SPI flash hooked up to the qspi.
+ *
+ * It's expected that all boards will support "dual SPI" at 37.5 MHz.
+ * If some boards need a different speed or have a package that allows
+ * Quad SPI together with WP then those boards can easily override.
+ */
+&qspi {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
+
+	spi_flash: flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+
+		spi-max-frequency = <37500000>;
+		spi-tx-bus-width = <2>;
+		spi-rx-bus-width = <2>;
+	};
+};
+
+/* Modem setup is different on Chrome setups than typical Qualcomm setup */
+&remoteproc_mpss {
+	status = "okay";
+	compatible = "qcom,sc7280-mss-pil";
+	iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
+	memory-region = <&mba_mem>, <&mpss_mem>;
+};
+
+/* Increase the size from 2.5MB to 8MB */
+&rmtfs_mem {
+	reg = <0x0 0x9c900000 0x0 0x800000>;
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index 8676c93590b5..67680a13c234 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -22,62 +22,15 @@
 #include "pm8350c.dtsi"
 #include "pmk8350.dtsi"
 
+#include "sc7280-chrome-common.dtsi"
+
 / {
 	model = "Google Herobrine (rev0)";
 	compatible = "google,herobrine",
 		     "qcom,sc7280";
 };
 
-/*
- * Reserved memory changes
- *
- * Delete all unused memory nodes and define the peripheral memory regions
- * required by the board dts.
- *
- */
-
-/delete-node/ &hyp_mem;
-/delete-node/ &xbl_mem;
-/delete-node/ &sec_apps_mem;
-
-/* Increase the size from 2MB to 8MB */
-&rmtfs_mem {
-	reg = <0x0 0x83600000 0x0 0x800000>;
-};
-
 / {
-	reserved-memory {
-		adsp_mem: memory@86700000 {
-			reg = <0x0 0x86700000 0x0 0x2800000>;
-			no-map;
-		};
-
-		camera_mem: memory@8ad00000 {
-			reg = <0x0 0x8ad00000 0x0 0x500000>;
-			no-map;
-		};
-
-		venus_mem: memory@8b200000 {
-			reg = <0x0 0x8b200000 0x0 0x500000>;
-			no-map;
-		};
-
-		mpss_mem: memory@8b800000 {
-			reg = <0x0 0x8b800000 0x0 0xf600000>;
-			no-map;
-		};
-
-		wpss_mem: memory@9ae00000 {
-			reg = <0x0 0x9ae00000 0x0 0x1900000>;
-			no-map;
-		};
-
-		mba_mem: memory@9c700000 {
-			reg = <0x0 0x9c700000 0x0 0x200000>;
-			no-map;
-		};
-	};
-
 	aliases {
 		serial0 = &uart5;
 		serial1 = &uart7;
@@ -691,10 +644,6 @@ &pmk8350_gpios {
 	status = "disabled"; /* No GPIOs are connected */
 };
 
-&pmk8350_pon {
-	status = "disabled";
-};
-
 &pmk8350_rtc {
 	status = "disabled";
 };
@@ -717,21 +666,6 @@ &qfprom {
 	vcc-supply = <&vdd_qfprom>;
 };
 
-&qspi {
-	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
-
-	flash@0 {
-		compatible = "jedec,spi-nor";
-		reg = <0>;
-
-		spi-max-frequency = <37500000>;
-		spi-tx-bus-width = <2>;
-		spi-rx-bus-width = <2>;
-	};
-};
-
 &qupv3_id_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index d623d71d8bd4..98c8f39ce459 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -13,6 +13,8 @@
 #include "pm8350c.dtsi"
 #include "pmk8350.dtsi"
 
+#include "sc7280-chrome-common.dtsi"
+
 / {
 	gpio-keys {
 		compatible = "gpio-keys";
@@ -45,58 +47,6 @@ nvme_3v3_regulator: nvme-3v3-regulator {
 	};
 };
 
-/*
- * Reserved memory changes
- *
- * Delete all unused memory nodes and define the peripheral memory regions
- * required by the board dts.
- *
- */
-
-/delete-node/ &hyp_mem;
-/delete-node/ &xbl_mem;
-/delete-node/ &reserved_xbl_uefi_log;
-/delete-node/ &sec_apps_mem;
-
-/* Increase the size from 2.5MB to 8MB */
-&rmtfs_mem {
-	reg = <0x0 0x9c900000 0x0 0x800000>;
-};
-
-/ {
-	reserved-memory {
-		adsp_mem: memory@86700000 {
-			reg = <0x0 0x86700000 0x0 0x2800000>;
-			no-map;
-		};
-
-		camera_mem: memory@8ad00000 {
-			reg = <0x0 0x8ad00000 0x0 0x500000>;
-			no-map;
-		};
-
-		venus_mem: memory@8b200000 {
-			reg = <0x0 0x8b200000 0x0 0x500000>;
-			no-map;
-		};
-
-		mpss_mem: memory@8b800000 {
-			reg = <0x0 0x8b800000 0x0 0xf600000>;
-			no-map;
-		};
-
-		wpss_mem: memory@9ae00000 {
-			reg = <0x0 0x9ae00000 0x0 0x1900000>;
-			no-map;
-		};
-
-		mba_mem: memory@9c700000 {
-			reg = <0x0 0x9c700000 0x0 0x200000>;
-			no-map;
-		};
-	};
-};
-
 &apps_rsc {
 	pm7325-regulators {
 		compatible = "qcom,pm7325-rpmh-regulators";
@@ -313,20 +263,6 @@ &qfprom {
 	vcc-supply = <&vreg_l1c_1p8>;
 };
 
-&qspi {
-	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
-
-	flash@0 {
-		compatible = "jedec,spi-nor";
-		reg = <0>;
-		spi-max-frequency = <37500000>;
-		spi-tx-bus-width = <2>;
-		spi-rx-bus-width = <2>;
-	};
-};
-
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -335,13 +271,6 @@ &qupv3_id_1 {
 	status = "okay";
 };
 
-&remoteproc_mpss {
-	status = "okay";
-	compatible = "qcom,sc7280-mss-pil";
-	iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
-	memory-region = <&mba_mem &mpss_mem>;
-};
-
 &sdhc_1 {
 	status = "okay";
 
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
                   ` (2 preceding siblings ...)
  2022-01-25 22:44 ` [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment Douglas Anderson
@ 2022-01-25 22:44 ` Douglas Anderson
  2022-01-25 22:54   ` Stephen Boyd
  2022-01-25 22:44 ` [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1 Douglas Anderson
  2022-01-31 18:24 ` (subset) [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Bjorn Andersson
  5 siblings, 1 reply; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Andy Gross, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

Though sc7280 itself doesn't need any of the defines in gpio.h, it's
highly likely that the actual boards will use them. Let's add the
include to the sc7280.dtsi file so that boards don't need to do it.

Suggested-by: Konrad Dybcio <konrad.dybcio@somainline.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- ("Factor gpio.h include to sc7280.dtsi") new for v2

 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts | 1 -
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi                   | 1 -
 arch/arm64/boot/dts/qcom/sc7280.dtsi                       | 1 +
 3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index 67680a13c234..ad4fe288b53c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -7,7 +7,6 @@
 
 /dts-v1/;
 
-#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
 #include <dt-bindings/input/gpio-keys.h>
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 98c8f39ce459..ec9836f4019e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -5,7 +5,6 @@
  * Copyright (c) 2021, The Linux Foundation. All rights reserved.
  */
 
-#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include <dt-bindings/input/linux-event-codes.h>
 #include "sc7280.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index eab7a8505053..02aff23d025e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/clock/qcom,gpucc-sc7280.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/clock/qcom,videocc-sc7280.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interconnect/qcom,sc7280.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
                   ` (3 preceding siblings ...)
  2022-01-25 22:44 ` [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi Douglas Anderson
@ 2022-01-25 22:44 ` Douglas Anderson
  2022-01-25 22:58   ` Stephen Boyd
  2022-01-31 18:24 ` (subset) [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Bjorn Andersson
  5 siblings, 1 reply; 21+ messages in thread
From: Douglas Anderson @ 2022-01-25 22:44 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Douglas Anderson, Andy Gross, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

Add the new herobrine-r1. Note that this is pretty much a re-design
compared to herobrine-r0 so we don't attempt any dtsi to share stuff
between them.

This patch attempts to define things at 3 levels:

1. The Qcard level. Herobrine includes a Qcard PCB and the Qcard PCB
   is supposed to be the same (modulo stuffing options) across
   multiple boards, so trying to define what's there hopefully makes
   sense. NOTE that newer "CRD" boards from Qualcomm also use
   Qcard. When support for CRD3 is added hopefully it can use the
   Qcard include (and perhaps we should even evaluate it using
   herobrine.dtsi?)
2. The herobrine "baseboard" level. Right now most stuff is here with
   the exception of things that we _know_ will be different per
   board. We know that not all boards will have the same set of eMMC,
   nvme, and SD. We also know that the exact pin names are likely to
   be different.
3. The actual "board" level, AKA herobrine-rev1.

NOTES:
- This boots to command prompt, but no eDP yet since eDP hasn't
  been added to sc7280.dtsi yet.
- This assumes LTE for now. Once it's clear how WiFi-only SKUs will
  work we expect some small changes.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---

Changes in v2:
- Herobrine compatible on one line, not two
- Wording change in comments for components enabled per-board
- Always sort "bias" above "drive-strength" in pinctrl.
- Properly sort "hub_en" pinctrl.
- Two comments moved from multiline to single line.
- Space after "/delete-property/"

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../qcom/sc7280-herobrine-herobrine-r0.dts    |   3 +-
 .../qcom/sc7280-herobrine-herobrine-r1.dts    | 313 +++++++
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 778 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    | 553 +++++++++++++
 5 files changed, 1646 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 9db743826391..54998e108092 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -83,6 +83,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-herobrine-herobrine-r0.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-herobrine-herobrine-r1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-crd.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
index ad4fe288b53c..e19eea1cb69a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts
@@ -25,8 +25,7 @@
 
 / {
 	model = "Google Herobrine (rev0)";
-	compatible = "google,herobrine",
-		     "qcom,sc7280";
+	compatible = "google,herobrine-rev0", "qcom,sc7280";
 };
 
 / {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
new file mode 100644
index 000000000000..f95273052da0
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Herobrine board device tree source
+ *
+ * Copyright 2022 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7280-herobrine.dtsi"
+
+/ {
+	model = "Google Herobrine (rev1+)";
+	compatible = "google,herobrine", "qcom,sc7280";
+};
+
+/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
+
+&ap_spi_fp {
+	status = "okay";
+};
+
+/*
+ * Although the trackpad is really part of the herobrine baseboard, we'll
+ * put the actual definition in the board device tree since different boards
+ * might hook up different trackpads (or no i2c trackpad at all in the case
+ * of tablets / detachables).
+ */
+ap_tp_i2c: &i2c0 {
+	status = "okay";
+	clock-frequency = <400000>;
+
+	trackpad: trackpad@15 {
+		compatible = "elan,ekth3000";
+		reg = <0x15>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&tp_int_odl>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+
+		vcc-supply = <&pp3300_z1>;
+
+		wakeup-source;
+	};
+};
+
+/*
+ * The touchscreen connector might come off the Qcard, at least in the case of
+ * eDP. Like the trackpad, we'll put it in the board device tree file since
+ * different boards have different touchscreens.
+ */
+ts_i2c: &i2c13 {
+	status = "okay";
+	clock-frequency = <400000>;
+
+	ap_ts: touchscreen@5c {
+		compatible = "hid-over-i2c";
+		reg = <0x5c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ts_int_conn>, <&ts_rst_conn>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <55 IRQ_TYPE_LEVEL_LOW>;
+
+		post-power-on-delay-ms = <500>;
+		hid-descr-addr = <0x0000>;
+
+		vdd-supply = <&ts_avdd>;
+	};
+};
+
+/* For nvme */
+&pcie1 {
+	status = "okay";
+};
+
+/* For nvme */
+&pcie1_phy {
+	status = "okay";
+};
+
+/* For eMMC */
+&sdhc_1 {
+	status = "okay";
+};
+
+/* For SD Card */
+&sdhc_2 {
+	status = "okay";
+};
+
+/* PINCTRL - BOARD-SPECIFIC */
+
+/*
+ * Methodology for gpio-line-names:
+ * - If a pin goes to herobrine board and is named it gets that name.
+ * - If a pin goes to herobrine board and is not named, it gets no name.
+ * - If a pin is totally internal to Qcard then it gets Qcard name.
+ * - If a pin is not hooked up on Qcard, it gets no name.
+ */
+
+&pm8350c_gpios {
+	gpio-line-names = "FLASH_STROBE_1",		/* 1 */
+			  "AP_SUSPEND",
+			  "PM8008_1_RST_N",
+			  "",
+			  "",
+			  "",
+			  "PMIC_EDP_BL_EN",
+			  "PMIC_EDP_BL_PWM",
+			  "";
+};
+
+&tlmm {
+	gpio-line-names = "AP_TP_I2C_SDA",		/* 0 */
+			  "AP_TP_I2C_SCL",
+			  "SSD_RST_L",
+			  "PE_WAKE_ODL",
+			  "AP_SAR_SDA",
+			  "AP_SAR_SCL",
+			  "PRB_SC_GPIO_6",
+			  "TP_INT_ODL",
+			  "HP_I2C_SDA",
+			  "HP_I2C_SCL",
+
+			  "GNSS_L1_EN",			/* 10 */
+			  "GNSS_L5_EN",
+			  "SPI_AP_MOSI",
+			  "SPI_AP_MISO",
+			  "SPI_AP_CLK",
+			  "SPI_AP_CS0_L",
+			  /*
+			   * AP_FLASH_WP is crossystem ABI. Schematics
+			   * call it BIOS_FLASH_WP_OD.
+			   */
+			  "AP_FLASH_WP",
+			  "",
+			  "AP_EC_INT_L",
+			  "",
+
+			  "UF_CAM_RST_L",		/* 20 */
+			  "WF_CAM_RST_L",
+			  "UART_AP_TX_DBG_RX",
+			  "UART_DBG_TX_AP_RX",
+			  "",
+			  "PM8008_IRQ_1",
+			  "HOST2WLAN_SOL",
+			  "WLAN2HOST_SOL",
+			  "MOS_BT_UART_CTS",
+			  "MOS_BT_UART_RFR",
+
+			  "MOS_BT_UART_TX",		/* 30 */
+			  "MOS_BT_UART_RX",
+			  "PRB_SC_GPIO_32",
+			  "HUB_RST_L",
+			  "",
+			  "",
+			  "AP_SPI_FP_MISO",
+			  "AP_SPI_FP_MOSI",
+			  "AP_SPI_FP_CLK",
+			  "AP_SPI_FP_CS_L",
+
+			  "AP_EC_SPI_MISO",		/* 40 */
+			  "AP_EC_SPI_MOSI",
+			  "AP_EC_SPI_CLK",
+			  "AP_EC_SPI_CS_L",
+			  "LCM_RST_L",
+			  "EARLY_EUD_N",
+			  "",
+			  "DP_HOT_PLUG_DET",
+			  "IO_BRD_MLB_ID0",
+			  "IO_BRD_MLB_ID1",
+
+			  "IO_BRD_MLB_ID2",		/* 50 */
+			  "SSD_EN",
+			  "TS_I2C_SDA_CONN",
+			  "TS_I2C_CLK_CONN",
+			  "TS_RST_CONN",
+			  "TS_INT_CONN",
+			  "AP_I2C_TPM_SDA",
+			  "AP_I2C_TPM_SCL",
+			  "PRB_SC_GPIO_58",
+			  "PRB_SC_GPIO_59",
+
+			  "EDP_HOT_PLUG_DET_N",		/* 60 */
+			  "FP_TO_AP_IRQ_L",
+			  "",
+			  "AMP_EN",
+			  "CAM0_MCLK_GPIO_64",
+			  "CAM1_MCLK_GPIO_65",
+			  "WF_CAM_MCLK",
+			  "PRB_SC_GPIO_67",
+			  "FPMCU_BOOT0",
+			  "UF_CAM_SDA",
+
+			  "UF_CAM_SCL",			/* 70 */
+			  "",
+			  "",
+			  "WF_CAM_SDA",
+			  "WF_CAM_SCL",
+			  "",
+			  "",
+			  "EN_FP_RAILS",
+			  "FP_RST_L",
+			  "PCIE1_CLKREQ_ODL",
+
+			  "EN_PP3300_DX_EDP",		/* 80 */
+			  "SC_GPIO_81",
+			  "FORCED_USB_BOOT",
+			  "WCD_RESET_N",
+			  "MOS_WLAN_EN",
+			  "MOS_BT_EN",
+			  "MOS_SW_CTRL",
+			  "MOS_PCIE0_RST",
+			  "MOS_PCIE0_CLKREQ_N",
+			  "MOS_PCIE0_WAKE_N",
+
+			  "MOS_LAA_AS_EN",		/* 90 */
+			  "SD_CD_ODL",
+			  "",
+			  "",
+			  "MOS_BT_WLAN_SLIMBUS_CLK",
+			  "MOS_BT_WLAN_SLIMBUS_DAT0",
+			  "HP_MCLK",
+			  "HP_BCLK",
+			  "HP_DOUT",
+			  "HP_DIN",
+
+			  "HP_LRCLK",			/* 100 */
+			  "HP_IRQ",
+			  "",
+			  "",
+			  "GSC_AP_INT_ODL",
+			  "EN_PP3300_CODEC",
+			  "AMP_BCLK",
+			  "AMP_DIN",
+			  "AMP_LRCLK",
+			  "UIM1_DATA_GPIO_109",
+
+			  "UIM1_CLK_GPIO_110",		/* 110 */
+			  "UIM1_RESET_GPIO_111",
+			  "PRB_SC_GPIO_112",
+			  "UIM0_DATA",
+			  "UIM0_CLK",
+			  "UIM0_RST",
+			  "UIM0_PRESENT_ODL",
+			  "SDM_RFFE0_CLK",
+			  "SDM_RFFE0_DATA",
+			  "WF_CAM_EN",
+
+			  "FASTBOOT_SEL_0",		/* 120 */
+			  "SC_GPIO_121",
+			  "FASTBOOT_SEL_1",
+			  "SC_GPIO_123",
+			  "FASTBOOT_SEL_2",
+			  "SM_RFFE4_CLK_GRFC_8",
+			  "SM_RFFE4_DATA_GRFC_9",
+			  "WLAN_COEX_UART1_RX",
+			  "WLAN_COEX_UART1_TX",
+			  "PRB_SC_GPIO_129",
+
+			  "LCM_ID0",			/* 130 */
+			  "LCM_ID1",
+			  "",
+			  "SDR_QLINK_REQ",
+			  "SDR_QLINK_EN",
+			  "QLINK0_WMSS_RESET_N",
+			  "SMR526_QLINK1_REQ",
+			  "SMR526_QLINK1_EN",
+			  "SMR526_QLINK1_WMSS_RESET_N",
+			  "PRB_SC_GPIO_139",
+
+			  "SAR1_IRQ_ODL",		/* 140 */
+			  "SAR0_IRQ_ODL",
+			  "PRB_SC_GPIO_142",
+			  "",
+			  "WCD_SWR_TX_CLK",
+			  "WCD_SWR_TX_DATA0",
+			  "WCD_SWR_TX_DATA1",
+			  "WCD_SWR_RX_CLK",
+			  "WCD_SWR_RX_DATA0",
+			  "WCD_SWR_RX_DATA1",
+
+			  "DMIC01_CLK",			/* 150 */
+			  "DMIC01_DATA",
+			  "DMIC23_CLK",
+			  "DMIC23_DATA",
+			  "",
+			  "",
+			  "EC_IN_RW_ODL",
+			  "HUB_EN",
+			  "WCD_SWR_TX_DATA2",
+			  "",
+
+			  "",				/* 160 */
+			  "",
+			  "",
+			  "",
+			  "",
+			  "",
+			  "",
+			  "",
+			  "",
+			  "",
+
+			  "",				/* 170 */
+			  "MOS_BLE_UART_TX",
+			  "MOS_BLE_UART_RX",
+			  "",
+			  "",
+			  "";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
new file mode 100644
index 000000000000..24c34ddebd18
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -0,0 +1,778 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Herobrine baseboard device tree source
+ *
+ * The set of things in this file is a bit loosely defined. It's roughly
+ * defined as the set of things that the child boards happen to have in
+ * common. Since all of the child boards started from the same original
+ * design this is hopefully a large set of things but as more derivatives
+ * appear things may "bubble down" out of this file. For things that are
+ * part of the reference design but might not exist on child nodes we will
+ * follow the lead of the SoC dtsi files and leave their status as "disabled".
+ *
+ * Copyright 2022 Google LLC.
+ */
+
+#include <dt-bindings/input/gpio-keys.h>
+#include <dt-bindings/input/input.h>
+
+#include "sc7280-qcard.dtsi"
+#include "sc7280-chrome-common.dtsi"
+
+/ {
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	/*
+	 * FIXED REGULATORS
+	 *
+	 * Sort order:
+	 * 1. parents above children.
+	 * 2. higher voltage above lower voltage.
+	 * 3. alphabetically by node name.
+	 */
+
+	/* This is the top level supply and variable voltage */
+	ppvar_sys: ppvar-sys-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "ppvar_sys";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
+	/* This divides ppvar_sys by 2, so voltage is variable */
+	src_vph_pwr: src-vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "src_vph_pwr";
+
+		/* EC turns on with switchcap_on; always on for AP */
+		regulator-always-on;
+		regulator-boot-on;
+
+		vin-supply = <&ppvar_sys>;
+	};
+
+	pp5000_s5: pp5000-s5-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp5000_s5";
+
+		/* EC turns on with en_pp5000_s5; always on for AP */
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+
+		vin-supply = <&ppvar_sys>;
+	};
+
+	pp3300_z1: pp3300-z1-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_z1";
+
+		/* EC turns on with en_pp3300_z1; always on for AP */
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		vin-supply = <&ppvar_sys>;
+	};
+
+	pp3300_codec: pp3300-codec-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_codec";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 105 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&en_pp3300_codec>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp3300_left_in_mlb: pp3300-left-in-mlb {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_left_in_mlb";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&en_pp3300_dx_edp>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp3300_mcu_fp:
+	pp3300_fp_ls:
+	pp3300_fp_mcu: pp3300-fp-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_fp";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		regulator-boot-on;
+		regulator-always-on;
+
+		/*
+		 * WARNING: it is intentional that GPIO 77 isn't listed here.
+		 * The userspace script for updating the fingerprint firmware
+		 * needs to control the FP regulators during a FW update,
+		 * hence the signal can't be owned by the kernel regulator.
+		 */
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&en_fp_rails>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp3300_hub: pp3300-hub-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_hub";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		regulator-boot-on;
+		regulator-always-on;
+
+		gpio = <&tlmm 157 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hub_en>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp3300_tp: pp3300-tp-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_tp";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		/* AP turns on with PP1800_L18B_S0; always on for AP */
+		regulator-always-on;
+		regulator-boot-on;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp3300_ssd: pp3300-ssd {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_ssd";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ssd_en>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp2850_vcm_wf_cam: pp2850-vcm-wf-cam {
+		compatible = "regulator-fixed";
+		regulator-name = "pp2850_vcm_wf_cam";
+
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2850000>;
+
+		gpio = <&tlmm 119 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-names = "default";
+		pinctrl-0 = <&wf_cam_en>;
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp2850_wf_cam: pp2850-wf-cam {
+		compatible = "regulator-fixed";
+		regulator-name = "pp2850_wf_cam";
+
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <2850000>;
+
+		gpio = <&tlmm 119 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		/*
+		 * The pinconf can only be referenced once so we put it on the
+		 * first regulator and comment it out here.
+		 *
+		 * pinctrl-names = "default";
+		 * pinctrl-0 = <&wf_cam_en>;
+		 */
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	pp1800_fp: pp1800-fp-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1800_fp";
+
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+
+		regulator-boot-on;
+		regulator-always-on;
+
+		/*
+		 * WARNING: it is intentional that GPIO 77 isn't listed here.
+		 * The userspace script for updating the fingerprint firmware
+		 * needs to control the FP regulators during a FW update,
+		 * hence the signal can't be owned by the kernel regulator.
+		 */
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&en_fp_rails>;
+
+		vin-supply = <&pp1800_l18b_s0>;
+		status = "disabled";
+	};
+
+	pp1800_wf_cam: pp1800-wf-cam {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1800_wf_cam";
+
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+
+		gpio = <&tlmm 119 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		/*
+		 * The pinconf can only be referenced once so we put it on the
+		 * first regulator and comment it out here.
+		 *
+		 * pinctrl-names = "default";
+		 * pinctrl-0 = <&wf_cam_en>;
+		 */
+
+		vin-supply = <&vreg_l19b_s0>;
+	};
+
+	pp1200_wf_cam: pp1200-wf-cam {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1200_wf_cam";
+
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+
+		gpio = <&tlmm 119 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		/*
+		 * The pinconf can only be referenced once so we put it on the
+		 * first regulator and comment it out here.
+		 *
+		 * pinctrl-names = "default";
+		 * pinctrl-0 = <&wf_cam_en>;
+		 */
+
+		vin-supply = <&pp3300_z1>;
+	};
+
+	/* BOARD-SPECIFIC TOP LEVEL NODES */
+
+	pwmleds {
+		compatible = "pwm-leds";
+		status = "disabled";
+		keyboard_backlight: keyboard-backlight {
+			status = "disabled";
+			label = "cros_ec::kbd_backlight";
+			pwms = <&cros_ec_pwm 0>;
+			max-brightness = <1023>;
+		};
+	};
+};
+
+/*
+ * BOARD-LOCAL NAMES FOR REGULATORS THAT CONNECT TO QCARD
+ *
+ * Names are only listed here if regulators go somewhere other than a
+ * testpoint.
+ */
+
+/* From Qcard to our board; ordered by PMIC-ID / rail number */
+
+pp1256_s8b: &vreg_s8b_1p256 {};
+
+pp1800_l18b_s0: &vreg_l18b_1p8 {};
+pp1800_l18b:    &vreg_l18b_1p8 {};
+
+vreg_l19b_s0: &vreg_l19b_1p8 {};
+
+pp1800_alc5682: &vreg_l2c_1p8 {};
+pp1800_l2c:     &vreg_l2c_1p8 {};
+
+vreg_l4c: &vreg_l4c_1p8_3p0 {};
+
+ppvar_l6c: &vreg_l6c_2p96 {};
+
+pp3000_l7c: &vreg_l7c_3p0 {};
+
+pp1800_prox: &vreg_l8c_1p8 {};
+pp1800_l8c:  &vreg_l8c_1p8 {};
+
+pp2950_l9c: &vreg_l9c_2p96 {};
+
+pp1800_lcm:  &vreg_l12c_1p8 {};
+pp1800_mipi: &vreg_l12c_1p8 {};
+pp1800_l12c: &vreg_l12c_1p8 {};
+
+pp3300_lcm:  &vreg_l13c_3p0 {};
+pp3300_mipi: &vreg_l13c_3p0 {};
+pp3300_l13c: &vreg_l13c_3p0 {};
+
+/* From our board to Qcard; ordered same as node definition above */
+
+vreg_edp_bl: &ppvar_sys {};
+
+ts_avdd:      &pp3300_left_in_mlb {};
+vreg_edp_3p3: &pp3300_left_in_mlb {};
+
+/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
+
+ap_i2c_tpm: &i2c14 {
+	status = "okay";
+	clock-frequency = <400000>;
+
+	tpm@50 {
+		compatible = "google,cr50";
+		reg = <0x50>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&gsc_ap_int_odl>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <104 IRQ_TYPE_EDGE_RISING>;
+	};
+};
+
+/* NVMe drive, enabled on a per-board basis */
+&pcie1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie1_clkreq_n>, <&ssd_rst_l>, <&pe_wake_odl>;
+
+	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
+	vddpe-3v3-supply = <&pp3300_ssd>;
+};
+
+&pmk8350_rtc {
+	status = "disabled";
+};
+
+&qupv3_id_0 {
+	status = "okay";
+};
+
+&qupv3_id_1 {
+	status = "okay";
+};
+
+/* SD Card, enabled on a per-board basis */
+&sdhc_2 {
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_on>;
+	pinctrl-1 = <&sdc2_off>;
+
+	vmmc-supply = <&pp2950_l9c>;
+	vqmmc-supply = <&ppvar_l6c>;
+
+	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
+};
+
+/* Fingerprint, enabled on a per-board basis */
+ap_spi_fp: &spi9 {
+	pinctrl-0 = <&qup_spi9_data_clk>, <&qup_spi9_cs_gpio_init_high>, <&qup_spi9_cs_gpio>;
+
+	cs-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
+
+	cros_ec_fp: ec@0 {
+		compatible = "google,cros-ec-spi";
+		reg = <0>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <61 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&fp_to_ap_irq_l>, <&fp_rst_l>, <&fpmcu_boot0>;
+		spi-max-frequency = <3000000>;
+	};
+};
+
+ap_ec_spi: &spi10 {
+	status = "okay";
+	pinctrl-0 = <&qup_spi10_data_clk>, <&qup_spi10_cs_gpio_init_high>, <&qup_spi10_cs_gpio>;
+
+	cs-gpios = <&tlmm 43 GPIO_ACTIVE_LOW>;
+
+	cros_ec: ec@0 {
+		compatible = "google,cros-ec-spi";
+		reg = <0>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ap_ec_int_l>;
+		spi-max-frequency = <3000000>;
+
+		cros_ec_pwm: ec-pwm {
+			compatible = "google,cros-ec-pwm";
+			#pwm-cells = <1>;
+		};
+
+		i2c_tunnel: i2c-tunnel {
+			compatible = "google,cros-ec-i2c-tunnel";
+			google,remote-bus = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		typec {
+			compatible = "google,cros-ec-typec";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			usb_c0: connector@0 {
+				compatible = "usb-c-connector";
+				reg = <0>;
+				label = "left";
+				power-role = "dual";
+				data-role = "host";
+				try-power-role = "source";
+			};
+
+			usb_c1: connector@1 {
+				compatible = "usb-c-connector";
+				reg = <1>;
+				label = "right";
+				power-role = "dual";
+				data-role = "host";
+				try-power-role = "source";
+			};
+		};
+	};
+};
+
+#include <arm/cros-ec-keyboard.dtsi>
+#include <arm/cros-ec-sbs.dtsi>
+
+&keyboard_controller {
+	function-row-physmap = <
+		MATRIX_KEY(0x00, 0x02, 0)	/* T1 */
+		MATRIX_KEY(0x03, 0x02, 0)	/* T2 */
+		MATRIX_KEY(0x02, 0x02, 0)	/* T3 */
+		MATRIX_KEY(0x01, 0x02, 0)	/* T4 */
+		MATRIX_KEY(0x03, 0x04, 0)	/* T5 */
+		MATRIX_KEY(0x02, 0x04, 0)	/* T6 */
+		MATRIX_KEY(0x01, 0x04, 0)	/* T7 */
+		MATRIX_KEY(0x02, 0x09, 0)	/* T8 */
+		MATRIX_KEY(0x01, 0x09, 0)	/* T9 */
+		MATRIX_KEY(0x00, 0x04, 0)	/* T10 */
+	>;
+	linux,keymap = <
+		MATRIX_KEY(0x00, 0x02, KEY_BACK)
+		MATRIX_KEY(0x03, 0x02, KEY_REFRESH)
+		MATRIX_KEY(0x02, 0x02, KEY_ZOOM)
+		MATRIX_KEY(0x01, 0x02, KEY_SCALE)
+		MATRIX_KEY(0x03, 0x04, KEY_SYSRQ)
+		MATRIX_KEY(0x02, 0x04, KEY_BRIGHTNESSDOWN)
+		MATRIX_KEY(0x01, 0x04, KEY_BRIGHTNESSUP)
+		MATRIX_KEY(0x02, 0x09, KEY_MUTE)
+		MATRIX_KEY(0x01, 0x09, KEY_VOLUMEDOWN)
+		MATRIX_KEY(0x00, 0x04, KEY_VOLUMEUP)
+
+		CROS_STD_MAIN_KEYMAP
+	>;
+};
+
+&usb_1 {
+	status = "okay";
+};
+
+&usb_1_dwc3 {
+	dr_mode = "host";
+};
+
+&usb_1_hsphy {
+	status = "okay";
+};
+
+&usb_1_qmpphy {
+	status = "okay";
+};
+
+&usb_2 {
+	status = "okay";
+};
+
+&usb_2_dwc3 {
+	dr_mode = "host";
+};
+
+&usb_2_hsphy {
+	status = "okay";
+};
+
+/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
+
+&qspi_cs0 {
+	bias-disable;
+	drive-strength = <8>;
+};
+
+&qspi_clk {
+	bias-disable;
+	drive-strength = <8>;
+};
+
+&qspi_data01 {
+	/* High-Z when no transfers; nice to park the lines */
+	bias-pull-up;
+	drive-strength = <8>;
+};
+
+/* For ap_tp_i2c */
+&qup_i2c0_data_clk {
+	/* Has external pull */
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For ap_i2c_tpm */
+&qup_i2c14_data_clk {
+	/* Has external pull */
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For ap_spi_fp */
+&qup_spi9_data_clk {
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For ap_spi_fp */
+&qup_spi9_cs_gpio {
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For ap_ec_spi */
+&qup_spi10_data_clk {
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For ap_ec_spi */
+&qup_spi10_cs_gpio {
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For uart_dbg */
+&qup_uart5_rx {
+	bias-pull-up;
+};
+
+/* For uart_dbg */
+&qup_uart5_tx {
+	bias-disable;
+	drive-strength = <2>;
+};
+
+&sdc2_on {
+	clk {
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	cmd {
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	data {
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sd-cd {
+		pins = "gpio91";
+		bias-pull-up;
+	};
+};
+
+/* PINCTRL - board-specific pinctrl */
+
+&pm7325_gpios {
+	/*
+	 * On a quick glance it might look like KYPD_VOL_UP_N is used, but
+	 * that only passes through to a debug connector and not to the actual
+	 * volume up key.
+	 */
+	status = "disabled"; /* No GPIOs are connected */
+};
+
+&pmk8350_gpios {
+	status = "disabled"; /* No GPIOs are connected */
+};
+
+&tlmm {
+	/* pinctrl settings for pins that have no real owners. */
+	pinctrl-names = "default";
+	pinctrl-0 = <&bios_flash_wp_od>;
+
+	amp_en: amp-en {
+		pins = "gpio63";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	ap_ec_int_l: ap-ec-int-l {
+		pins = "gpio18";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	bios_flash_wp_od: bios-flash-wp-od {
+		pins = "gpio16";
+		function = "gpio";
+		/* Has external pull */
+		bias-disable;
+	};
+
+	en_fp_rails: en-fp-rails {
+		pins = "gpio77";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+		output-high;
+	};
+
+	en_pp3300_codec: en-pp3300-codec {
+		pins = "gpio105";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	en_pp3300_dx_edp: en-pp3300-dx-edp {
+		pins = "gpio80";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	fp_rst_l: fp-rst-l {
+		pins = "gpio78";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+		output-high;
+	};
+
+	fp_to_ap_irq_l: fp-to-ap-irq-l {
+		pins = "gpio61";
+		function = "gpio";
+		/* Has external pullup */
+		bias-disable;
+	};
+
+	fpmcu_boot0: fpmcu-boot0 {
+		pins = "gpio68";
+		function = "gpio";
+		bias-disable;
+		output-low;
+	};
+
+	gsc_ap_int_odl: gsc-ap-int-odl {
+		pins = "gpio104";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	hp_irq: hp-irq {
+		pins = "gpio101";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	hub_en: hub-en {
+		pins = "gpio157";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	pe_wake_odl: pe-wake-odl {
+		pins = "gpio3";
+		function = "gpio";
+		/* Has external pull */
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	/* For ap_spi_fp */
+	qup_spi9_cs_gpio_init_high: qup-spi9-cs-gpio-init-high {
+		pins = "gpio39";
+		function = "gpio";
+		output-high;
+	};
+
+	/* For ap_ec_spi */
+	qup_spi10_cs_gpio_init_high: qup-spi10-cs-gpio-init-high {
+		pins = "gpio43";
+		function = "gpio";
+		output-high;
+	};
+
+	sar0_irq_odl: sar0-irq-odl {
+		pins = "gpio141";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	sar1_irq_odl: sar0-irq-odl {
+		pins = "gpio140";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	ssd_en: ssd-en {
+		pins = "gpio51";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
+	ssd_rst_l: ssd-rst-l {
+		pins = "gpio2";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+		output-low;
+	};
+
+	tp_int_odl: tp-int-odl {
+		pins = "gpio7";
+		function = "gpio";
+		/* Has external pullup */
+		bias-disable;
+	};
+
+	wf_cam_en: wf-cam-en {
+		pins = "gpio119";
+		function = "gpio";
+		/* Has external pulldown */
+		bias-disable;
+		drive-strength = <2>;
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
new file mode 100644
index 000000000000..a9cd746c12c6
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
@@ -0,0 +1,553 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * sc7280 Qcard device tree source
+ *
+ * Qcard PCB has the processor, RAM, eMMC (if stuffed), and eDP connector (if
+ * stuffed) on it. This device tree tries to encapsulate all the things that
+ * all boards using Qcard will have in common. Given that there are stuffing
+ * options, some things may be left with status "disabled" and enabled in
+ * the actual board device tree files.
+ *
+ * Copyright 2022 Google LLC.
+ */
+
+#include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/iio/qcom,spmi-adc7-pmr735a.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+#include "sc7280.dtsi"
+
+/* PMICs depend on spmi_bus label and so must come after SoC */
+#include "pm7325.dtsi"
+#include "pm8350c.dtsi"
+#include "pmk8350.dtsi"
+
+/ {
+	aliases {
+		bluetooth0 = &bluetooth;
+		serial0 = &uart5;
+		serial1 = &uart7;
+	};
+};
+
+&apps_rsc {
+	/*
+	 * Regulators are given labels corresponding to the various names
+	 * they are referred to on schematics. They are also given labels
+	 * corresponding to named voltage inputs on the SoC or components
+	 * bundled with the SoC (like radio companion chips). We totally
+	 * ignore it when one regulator is the input to another regulator.
+	 * That's handled automatically by the initial config given to
+	 * RPMH by the firmware.
+	 *
+	 * Regulators that the HLOS (High Level OS) doesn't touch at all
+	 * are left out of here since they are managed elsewhere.
+	 */
+
+	pm7325-regulators {
+		compatible = "qcom,pm7325-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vdd19_pmu_pcie_i:
+		vdd19_pmu_rfa_i:
+		vreg_s1b_1p856: smps1 {
+			regulator-min-microvolt = <1856000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		vdd_pmu_aon_i:
+		vdd09_pmu_rfa_i:
+		vdd095_mx_pmu:
+		vdd095_pmu:
+		vreg_s7b_0p952: smps7 {
+			regulator-min-microvolt = <535000>;
+			regulator-max-microvolt = <1120000>;
+		};
+
+		vdd13_pmu_rfa_i:
+		vdd13_pmu_pcie_i:
+		vreg_s8b_1p256: smps8 {
+			regulator-min-microvolt = <1256000>;
+			regulator-max-microvolt = <1500000>;
+		};
+
+		vdd_a_usbssdp_0_core:
+		vreg_l1b_0p912: ldo1 {
+			regulator-min-microvolt = <825000>;
+			regulator-max-microvolt = <925000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_a_usbhs_3p1:
+		vreg_l2b_3p072: ldo2 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_a_csi_0_1_1p2:
+		vdd_a_csi_2_3_1p2:
+		vdd_a_csi_4_1p2:
+		vdd_a_dsi_0_1p2:
+		vdd_a_edp_0_1p2:
+		vdd_a_qlink_0_1p2:
+		vdd_a_qlink_1_1p2:
+		vdd_a_pcie_0_1p2:
+		vdd_a_pcie_1_1p2:
+		vdd_a_ufs_0_1p2:
+		vdd_a_usbssdp_0_1p2:
+		vreg_l6b_1p2: ldo6 {
+			regulator-min-microvolt = <1140000>;
+			regulator-max-microvolt = <1260000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		/*
+		 * Despite the fact that this is named to be 2.5V on the
+		 * schematic, it powers eMMC which doesn't accept 2.5V
+		 */
+		vreg_l7b_2p5: ldo7 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_px_wcd9385:
+		vdd_txrx:
+		vddpx_0:
+		vddpx_3:
+		vddpx_7:
+		vreg_l18b_1p8: ldo18 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_1p8:
+		vdd_px_sdr735:
+		vdd_pxm:
+		vdd18_io:
+		vddio_px_1:
+		vddio_px_2:
+		vddio_px_3:
+		vddpx_ts:
+		vddpx_wl4otp:
+		vreg_l19b_1p8: ldo19 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	pm8350c-regulators {
+		compatible = "qcom,pm8350c-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vdd22_wlbtpa_ch0:
+		vdd22_wlbtpa_ch1:
+		vdd22_wlbtppa_ch0:
+		vdd22_wlbtppa_ch1:
+		vdd22_wlpa5g_ch0:
+		vdd22_wlpa5g_ch1:
+		vdd22_wlppa5g_ch0:
+		vdd22_wlppa5g_ch1:
+		vreg_s1c_2p2: smps1 {
+			regulator-min-microvolt = <2190000>;
+			regulator-max-microvolt = <2210000>;
+		};
+
+		lp4_vdd2_1p052:
+		vreg_s9c_0p676: smps9 {
+			regulator-min-microvolt = <1010000>;
+			regulator-max-microvolt = <1170000>;
+		};
+
+		vdda_apc_cs_1p8:
+		vdda_gfx_cs_1p8:
+		vdda_turing_q6_cs_1p8:
+		vdd_a_cxo_1p8:
+		vdd_a_qrefs_1p8:
+		vdd_a_usbhs_1p8:
+		vdd_qfprom:
+		vreg_l1c_1p8: ldo1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1980000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2c_1p8: ldo2 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <1980000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3c_3p0: ldo3 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3540000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vddpx_5:
+		vreg_l4c_1p8_3p0: ldo4 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vddpx_6:
+		vreg_l5c_1p8_3p0: ldo5 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vddpx_2:
+		vreg_l6c_2p96: ldo6 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2950000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7c_3p0: ldo7 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l8c_1p8: ldo8 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l9c_2p96: ldo9 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_a_csi_0_1_0p9:
+		vdd_a_csi_2_3_0p9:
+		vdd_a_csi_4_0p9:
+		vdd_a_dsi_0_0p9:
+		vdd_a_dsi_0_pll_0p9:
+		vdd_a_edp_0_0p9:
+		vdd_a_gnss_0p9:
+		vdd_a_pcie_0_core:
+		vdd_a_pcie_1_core:
+		vdd_a_qlink_0_0p9:
+		vdd_a_qlink_0_0p9_ck:
+		vdd_a_qlink_1_0p9:
+		vdd_a_qlink_1_0p9_ck:
+		vdd_a_qrefs_0p875_0:
+		vdd_a_qrefs_0p875_1:
+		vdd_a_qrefs_0p875_2:
+		vdd_a_qrefs_0p875_3:
+		vdd_a_qrefs_0p875_4_5:
+		vdd_a_qrefs_0p875_6:
+		vdd_a_qrefs_0p875_7:
+		vdd_a_qrefs_0p875_8:
+		vdd_a_qrefs_0p875_9:
+		vdd_a_ufs_0_core:
+		vdd_a_usbhs_core:
+		vreg_l10c_0p88: ldo10 {
+			regulator-min-microvolt = <720000>;
+			regulator-max-microvolt = <1050000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l11c_2p8: ldo11 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l12c_1p8: ldo12 {
+			regulator-min-microvolt = <1650000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13c_3p0: ldo13 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdd_flash:
+		vdd_iris_rgb:
+		vdd_mic_bias:
+		vreg_bob: bob {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+	};
+};
+
+/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
+
+&ipa {
+	status = "okay";
+	modem-init;
+};
+
+&pcie1_phy {
+	vdda-phy-supply = <&vreg_l10c_0p88>;
+	vdda-pll-supply = <&vreg_l6b_1p2>;
+};
+
+&pmk8350_vadc {
+	pmk8350-die-temp@3 {
+		reg = <PMK8350_ADC7_DIE_TEMP>;
+		label = "pmk8350_die_temp";
+		qcom,pre-scaling = <1 1>;
+	};
+
+	pmr735a-die-temp@403 {
+		reg = <PMR735A_ADC7_DIE_TEMP>;
+		label = "pmr735a_die_temp";
+		qcom,pre-scaling = <1 1>;
+	};
+};
+
+&qfprom {
+	vcc-supply = <&vdd_qfprom>;
+};
+
+/* For eMMC. NOTE: not all Qcards have eMMC stuffed */
+&sdhc_1 {
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc1_on>;
+	pinctrl-1 = <&sdc1_off>;
+
+	vmmc-supply = <&vreg_l7b_2p5>;
+	vqmmc-supply = <&vreg_l19b_1p8>;
+
+	non-removable;
+	no-sd;
+	no-sdio;
+};
+
+uart_dbg: &uart5 {
+	compatible = "qcom,geni-debug-uart";
+	status = "okay";
+};
+
+mos_bt_uart: &uart7 {
+	status = "okay";
+
+	/delete-property/ interrupts;
+	interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
+				<&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>, <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
+
+	bluetooth: bluetooth {
+		compatible = "qcom,wcn6750-bt";
+		pinctrl-names = "default";
+		pinctrl-0 = <&mos_bt_en>;
+		enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
+		swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
+		vddaon-supply = <&vreg_s7b_0p952>;
+		vddbtcxmx-supply = <&vreg_s7b_0p952>;
+		vddrfacmn-supply = <&vreg_s7b_0p952>;
+		vddrfa0p8-supply = <&vreg_s7b_0p952>;
+		vddrfa1p7-supply = <&vdd19_pmu_rfa_i>;
+		vddrfa1p2-supply = <&vdd13_pmu_rfa_i>;
+		vddrfa2p2-supply = <&vreg_s1c_2p2>;
+		vddasd-supply = <&vreg_l11c_2p8>;
+		vddio-supply = <&vreg_l18b_1p8>;
+		max-speed = <3200000>;
+	};
+};
+
+&usb_1_hsphy {
+	vdda-pll-supply = <&vdd_a_usbhs_core>;
+	vdda33-supply = <&vdd_a_usbhs_3p1>;
+	vdda18-supply = <&vdd_a_usbhs_1p8>;
+};
+
+&usb_1_qmpphy {
+	vdda-phy-supply = <&vdd_a_usbssdp_0_1p2>;
+	vdda-pll-supply = <&vdd_a_usbssdp_0_core>;
+};
+
+&usb_2_hsphy {
+	vdda-pll-supply = <&vdd_a_usbhs_core>;
+	vdda33-supply = <&vdd_a_usbhs_3p1>;
+	vdda18-supply = <&vdd_a_usbhs_1p8>;
+};
+
+/*
+ * PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES
+ *
+ * NOTE: In general if pins leave the Qcard then the pinctrl goes in the
+ * baseboard or board device tree, not here.
+ */
+
+/*
+ * For ts_i2c
+ *
+ * Technically this i2c bus actually leaves the Qcard, but it leaves directly
+ * via the eDP connector (it doesn't hit the baseboard). The external pulls
+ * are on Qcard.
+ */
+&qup_i2c13_data_clk {
+	/* Has external pull */
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For mos_bt_uart */
+&qup_uart7_cts {
+	/* Configure a pull-down on CTS to match the pull of the Bluetooth module. */
+	bias-pull-down;
+};
+
+/* For mos_bt_uart */
+&qup_uart7_rts {
+	/* We'll drive RTS, so no pull */
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For mos_bt_uart */
+&qup_uart7_tx {
+	/* We'll drive TX, so no pull */
+	bias-disable;
+	drive-strength = <2>;
+};
+
+/* For mos_bt_uart */
+&qup_uart7_rx {
+	/*
+	 * Configure a pull-up on RX. This is needed to avoid
+	 * garbage data when the TX pin of the Bluetooth module is
+	 * in tri-state (module powered off or not driving the
+	 * signal yet).
+	 */
+	bias-pull-up;
+};
+
+/* eMMC, if stuffed, is straight on the Qcard */
+&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;
+	};
+};
+
+/*
+ * PINCTRL - QCARD
+ *
+ * This has entries that are defined by Qcard even if they go to the main
+ * board. In cases where the pulls may be board dependent we defer those
+ * settings to the board device tree. Drive strengths tend to be assinged here
+ * but could conceivably be overwridden by board device trees.
+ */
+
+&pm8350c_gpios {
+	pmic_edp_bl_en: pmic-edp-bl-en {
+		pins = "gpio7";
+		function = "normal";
+		bias-disable;
+		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+
+		/* Force backlight to be disabled to match state at boot. */
+		output-low;
+	};
+
+	pmic_edp_bl_pwm: pmic-edp-bl-pwm {
+		pins = "gpio8";
+		function = "func1";
+		bias-disable;
+		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+		output-low;
+		power-source = <0>;
+	};
+};
+
+&tlmm {
+	mos_bt_en: mos-bt-en {
+		pins = "gpio85";
+		function = "gpio";
+		drive-strength = <2>;
+		output-low;
+	};
+
+	/* For mos_bt_uart */
+	qup_uart7_sleep_cts: qup-uart7-sleep-cts {
+		pins = "gpio28";
+		function = "gpio";
+		/*
+		 * Configure a pull-down on CTS to match the pull of
+		 * the Bluetooth module.
+		 */
+		bias-pull-down;
+	};
+
+	/* For mos_bt_uart */
+	qup_uart7_sleep_rts: qup-uart7-sleep-rts {
+		pins = "gpio29";
+		function = "gpio";
+		/*
+		 * Configure pull-down on RTS. As RTS is active low
+		 * signal, pull it low to indicate the BT SoC that it
+		 * can wakeup the system anytime from suspend state by
+		 * pulling RX low (by sending wakeup bytes).
+		 */
+		bias-pull-down;
+	};
+
+	/* For mos_bt_uart */
+	qup_uart7_sleep_rx: qup-uart7-sleep-rx {
+		pins = "gpio31";
+		function = "gpio";
+		/*
+		 * Configure a pull-up on RX. This is needed to avoid
+		 * garbage data when the TX pin of the Bluetooth module
+		 * is floating which may cause spurious wakeups.
+		 */
+		bias-pull-up;
+	};
+
+	/* For mos_bt_uart */
+	qup_uart7_sleep_tx: qup-uart7-sleep-tx {
+		pins = "gpio30";
+		function = "gpio";
+		/*
+		 * Configure pull-up on TX when it isn't actively driven
+		 * to prevent BT SoC from receiving garbage during sleep.
+		 */
+		bias-pull-up;
+	};
+
+	ts_int_conn: ts-int-conn {
+		pins = "gpio55";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	ts_rst_conn: ts-rst-conn {
+		pins = "gpio54";
+		function = "gpio";
+		bias-pull-up;
+		drive-strength = <2>;
+	};
+};
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi
  2022-01-25 22:44 ` [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi Douglas Anderson
@ 2022-01-25 22:54   ` Stephen Boyd
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Boyd @ 2022-01-25 22:54 UTC (permalink / raw)
  To: Bjorn Andersson, Douglas Anderson
  Cc: konrad.dybcio, kgodara, mka, sibis, pmaliset, quic_rjendra,
	Andy Gross, Rob Herring, devicetree, linux-arm-msm, linux-kernel

Quoting Douglas Anderson (2022-01-25 14:44:21)
> Though sc7280 itself doesn't need any of the defines in gpio.h, it's
> highly likely that the actual boards will use them. Let's add the
> include to the sc7280.dtsi file so that boards don't need to do it.
>
> Suggested-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-25 22:44 ` [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1 Douglas Anderson
@ 2022-01-25 22:58   ` Stephen Boyd
  2022-01-25 23:46     ` Doug Anderson
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Boyd @ 2022-01-25 22:58 UTC (permalink / raw)
  To: Bjorn Andersson, Douglas Anderson
  Cc: konrad.dybcio, kgodara, mka, sibis, pmaliset, quic_rjendra,
	Andy Gross, Rob Herring, devicetree, linux-arm-msm, linux-kernel

Quoting Douglas Anderson (2022-01-25 14:44:22)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> new file mode 100644
> index 000000000000..f95273052da0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> @@ -0,0 +1,313 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Herobrine board device tree source
> + *
> + * Copyright 2022 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7280-herobrine.dtsi"
> +
> +/ {
> +       model = "Google Herobrine (rev1+)";
> +       compatible = "google,herobrine", "qcom,sc7280";

Can we stop adding "qcom,sc7280" to the board compatible string? It
looks out of place. It's the compatible for the SoC and should really be
the compatible for the /soc node.

> +};
> +
> +/* ADDITIONS TO NODES DEFINED IN PARENT DEVICE TREE FILES */
> +
> +&ap_spi_fp {
> +       status = "okay";
> +};
[...]
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> new file mode 100644
> index 000000000000..24c34ddebd18
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -0,0 +1,778 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Herobrine baseboard device tree source
> + *
[...]
> +
> +               vin-supply = <&ppvar_sys>;
> +       };
> +
> +       pp3300_codec: pp3300-codec-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp3300_codec";
> +
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +
> +               gpio = <&tlmm 105 GPIO_ACTIVE_HIGH>;
> +               enable-active-high;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&en_pp3300_codec>;
> +
> +               vin-supply = <&pp3300_z1>;
> +       };
> +
> +       pp3300_left_in_mlb: pp3300-left-in-mlb {

Sometimes '-regulator' is left out. Is that intentional? I suppose it
would be better if every node had regulator postfix, but it may be too
long of a node name?

> +               compatible = "regulator-fixed";
> +               regulator-name = "pp3300_left_in_mlb";
> +
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +
> +               gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-25 22:58   ` Stephen Boyd
@ 2022-01-25 23:46     ` Doug Anderson
  2022-01-26  3:01       ` Bjorn Andersson
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Anderson @ 2022-01-25 23:46 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Bjorn Andersson, Konrad Dybcio, kgodara, Matthias Kaehlcke,
	Sibi Sankar, Prasad Malisetty, quic_rjendra, Andy Gross,
	Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Douglas Anderson (2022-01-25 14:44:22)
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > new file mode 100644
> > index 000000000000..f95273052da0
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > @@ -0,0 +1,313 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Google Herobrine board device tree source
> > + *
> > + * Copyright 2022 Google LLC.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sc7280-herobrine.dtsi"
> > +
> > +/ {
> > +       model = "Google Herobrine (rev1+)";
> > +       compatible = "google,herobrine", "qcom,sc7280";
>
> Can we stop adding "qcom,sc7280" to the board compatible string? It
> looks out of place. It's the compatible for the SoC and should really be
> the compatible for the /soc node.

I don't have any objections, but I feel like this is the type of thing
I'd like Bjorn to have the final say on. What say you, Bjorn?


> > +       pp3300_left_in_mlb: pp3300-left-in-mlb {
>
> Sometimes '-regulator' is left out. Is that intentional? I suppose it
> would be better if every node had regulator postfix, but it may be too
> long of a node name?

Huh, you're right. No, it's not intentional. It looks like it was that
way on herobrine-rev0 too... It also looks like it's missing on
"pp3300-hub" on trogdor.

Happy to add "-regulator" in the herobrine / trogdor nodes that need
it. I'll probably do it as a follow-on patch if that works OK? Then I
can just clean them all up at once...

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-25 23:46     ` Doug Anderson
@ 2022-01-26  3:01       ` Bjorn Andersson
  2022-01-27 21:16         ` Stephen Boyd
  0 siblings, 1 reply; 21+ messages in thread
From: Bjorn Andersson @ 2022-01-26  3:01 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Konrad Dybcio, kgodara, Matthias Kaehlcke,
	Sibi Sankar, Prasad Malisetty, quic_rjendra, Andy Gross,
	Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

On Tue 25 Jan 15:46 PST 2022, Doug Anderson wrote:

> Hi,
> 
> On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Douglas Anderson (2022-01-25 14:44:22)
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > new file mode 100644
> > > index 000000000000..f95273052da0
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > @@ -0,0 +1,313 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +/*
> > > + * Google Herobrine board device tree source
> > > + *
> > > + * Copyright 2022 Google LLC.
> > > + */
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "sc7280-herobrine.dtsi"
> > > +
> > > +/ {
> > > +       model = "Google Herobrine (rev1+)";
> > > +       compatible = "google,herobrine", "qcom,sc7280";
> >
> > Can we stop adding "qcom,sc7280" to the board compatible string? It
> > looks out of place. It's the compatible for the SoC and should really be
> > the compatible for the /soc node.
> 
> I don't have any objections, but I feel like this is the type of thing
> I'd like Bjorn to have the final say on. What say you, Bjorn?
> 

One practical case I can think of right away, where this matters is in
cpufreq-dt-plat.c where we blocklist qcom,sc7280.

I don't know if we rely on this in any other places, but I'm not keen on
seeing a bunch of board-specific compatibles sprinkled throughout the
implementation - it's annoying enough having to add each platform to
these drivers.

Regards,
Bjorn

> 
> > > +       pp3300_left_in_mlb: pp3300-left-in-mlb {
> >
> > Sometimes '-regulator' is left out. Is that intentional? I suppose it
> > would be better if every node had regulator postfix, but it may be too
> > long of a node name?
> 
> Huh, you're right. No, it's not intentional. It looks like it was that
> way on herobrine-rev0 too... It also looks like it's missing on
> "pp3300-hub" on trogdor.
> 
> Happy to add "-regulator" in the herobrine / trogdor nodes that need
> it. I'll probably do it as a follow-on patch if that works OK? Then I
> can just clean them all up at once...
> 
> -Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-26  3:01       ` Bjorn Andersson
@ 2022-01-27 21:16         ` Stephen Boyd
  2022-01-31 16:41           ` Bjorn Andersson
  2022-01-31 16:44           ` Doug Anderson
  0 siblings, 2 replies; 21+ messages in thread
From: Stephen Boyd @ 2022-01-27 21:16 UTC (permalink / raw)
  To: Bjorn Andersson, Doug Anderson, Viresh Kumar
  Cc: Konrad Dybcio, kgodara, Matthias Kaehlcke, Sibi Sankar,
	Prasad Malisetty, quic_rjendra, Andy Gross, Rob Herring,
	devicetree, linux-arm-msm, LKML

Quoting Bjorn Andersson (2022-01-25 19:01:31)
> On Tue 25 Jan 15:46 PST 2022, Doug Anderson wrote:
>
> > Hi,
> >
> > On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Quoting Douglas Anderson (2022-01-25 14:44:22)
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > new file mode 100644
> > > > index 000000000000..f95273052da0
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > @@ -0,0 +1,313 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > +/*
> > > > + * Google Herobrine board device tree source
> > > > + *
> > > > + * Copyright 2022 Google LLC.
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +
> > > > +#include "sc7280-herobrine.dtsi"
> > > > +
> > > > +/ {
> > > > +       model = "Google Herobrine (rev1+)";
> > > > +       compatible = "google,herobrine", "qcom,sc7280";
> > >
> > > Can we stop adding "qcom,sc7280" to the board compatible string? It
> > > looks out of place. It's the compatible for the SoC and should really be
> > > the compatible for the /soc node.
> >
> > I don't have any objections, but I feel like this is the type of thing
> > I'd like Bjorn to have the final say on. What say you, Bjorn?
> >
>
> One practical case I can think of right away, where this matters is in
> cpufreq-dt-plat.c where we blocklist qcom,sc7280.
>
> I don't know if we rely on this in any other places, but I'm not keen on
> seeing a bunch of board-specific compatibles sprinkled throughout the
> implementation - it's annoying enough having to add each platform to
> these drivers.

Looking at sc7180, grep only shows cpufreq-dt-plat.c

 $ git grep qcom,sc7180\" -- drivers
 drivers/cpufreq/cpufreq-dt-platdev.c:   { .compatible = "qcom,sc7180", },

Simplest solution would be to look at / and /soc for a compatible
string.

 $ git grep -W 'soc[^:]*{' -- arch/arm*/boot/dts/ | grep compatible |
grep -v "simple-bus"

doesn't show many hits. The first hit is "ti,omap-infra" which is
actually inside an soc node, but even then I don't see anything that
matches the cpufreq-dt-plat.c lists.

----8<-----
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
b/drivers/cpufreq/cpufreq-dt-platdev.c
index ca1d103ec449..32bfe453f8b4 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -179,25 +179,29 @@ static bool __init cpu0_node_has_opp_v2_prop(void)
 static int __init cpufreq_dt_platdev_init(void)
 {
 	struct device_node *np = of_find_node_by_path("/");
+	struct device_node *soc_np = of_find_node_by_path("/soc");
 	const struct of_device_id *match;
 	const void *data = NULL;

-	if (!np)
+	if (!np && !soc_np)
 		return -ENODEV;

 	match = of_match_node(allowlist, np);
-	if (match) {
+	if (match || (match = of_match_node(allowlist, soc_np))) {
 		data = match->data;
 		goto create_pdev;
 	}

-	if (cpu0_node_has_opp_v2_prop() && !of_match_node(blocklist, np))
+	if (cpu0_node_has_opp_v2_prop() && !of_match_node(blocklist, np) &&
+	    !of_match_node(blocklist, soc_np))
 		goto create_pdev;

+	of_node_put(soc_np);
 	of_node_put(np);
 	return -ENODEV;

 create_pdev:
+	of_node_put(soc_np);
 	of_node_put(np);
 	return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq-dt",
 			       -1, data,

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-27 21:16         ` Stephen Boyd
@ 2022-01-31 16:41           ` Bjorn Andersson
  2022-01-31 16:50             ` Doug Anderson
  2022-02-03 21:55             ` Stephen Boyd
  2022-01-31 16:44           ` Doug Anderson
  1 sibling, 2 replies; 21+ messages in thread
From: Bjorn Andersson @ 2022-01-31 16:41 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Doug Anderson, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring, devicetree, linux-arm-msm, LKML

On Thu 27 Jan 15:16 CST 2022, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2022-01-25 19:01:31)
> > On Tue 25 Jan 15:46 PST 2022, Doug Anderson wrote:
> >
> > > Hi,
> > >
> > > On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > >
> > > > Quoting Douglas Anderson (2022-01-25 14:44:22)
> > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > new file mode 100644
> > > > > index 000000000000..f95273052da0
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > @@ -0,0 +1,313 @@
> > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > > +/*
> > > > > + * Google Herobrine board device tree source
> > > > > + *
> > > > > + * Copyright 2022 Google LLC.
> > > > > + */
> > > > > +
> > > > > +/dts-v1/;
> > > > > +
> > > > > +#include "sc7280-herobrine.dtsi"
> > > > > +
> > > > > +/ {
> > > > > +       model = "Google Herobrine (rev1+)";
> > > > > +       compatible = "google,herobrine", "qcom,sc7280";
> > > >
> > > > Can we stop adding "qcom,sc7280" to the board compatible string? It
> > > > looks out of place. It's the compatible for the SoC and should really be
> > > > the compatible for the /soc node.
> > >
> > > I don't have any objections, but I feel like this is the type of thing
> > > I'd like Bjorn to have the final say on. What say you, Bjorn?
> > >
> >
> > One practical case I can think of right away, where this matters is in
> > cpufreq-dt-plat.c where we blocklist qcom,sc7280.
> >
> > I don't know if we rely on this in any other places, but I'm not keen on
> > seeing a bunch of board-specific compatibles sprinkled throughout the
> > implementation - it's annoying enough having to add each platform to
> > these drivers.
> 
> Looking at sc7180, grep only shows cpufreq-dt-plat.c
> 

Good, then we handle all other platform specifics in drivers using
platform-specific compatibles.

>  $ git grep qcom,sc7180\" -- drivers
>  drivers/cpufreq/cpufreq-dt-platdev.c:   { .compatible = "qcom,sc7180", },
> 
> Simplest solution would be to look at / and /soc for a compatible
> string.
> 

You mean that / would contain the device's compatible and /soc the soc's
compatible? I'm afraid I don't see how this would help you - you still
need the compatible in the dts, just now in two places.


Either we leave it as is - which follows my interpretation of what the DT
spec says - or we (and the DT maitainers) agree that it shouldn't be
there (because this dtb won't run on any random qcom,sc7180 anyways) at
all.

Regards,
Bjorn

>  $ git grep -W 'soc[^:]*{' -- arch/arm*/boot/dts/ | grep compatible |
> grep -v "simple-bus"
> 
> doesn't show many hits. The first hit is "ti,omap-infra" which is
> actually inside an soc node, but even then I don't see anything that
> matches the cpufreq-dt-plat.c lists.
> 
> ----8<-----
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
> b/drivers/cpufreq/cpufreq-dt-platdev.c
> index ca1d103ec449..32bfe453f8b4 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -179,25 +179,29 @@ static bool __init cpu0_node_has_opp_v2_prop(void)
>  static int __init cpufreq_dt_platdev_init(void)
>  {
>  	struct device_node *np = of_find_node_by_path("/");
> +	struct device_node *soc_np = of_find_node_by_path("/soc");
>  	const struct of_device_id *match;
>  	const void *data = NULL;
> 
> -	if (!np)
> +	if (!np && !soc_np)
>  		return -ENODEV;
> 
>  	match = of_match_node(allowlist, np);
> -	if (match) {
> +	if (match || (match = of_match_node(allowlist, soc_np))) {
>  		data = match->data;
>  		goto create_pdev;
>  	}
> 
> -	if (cpu0_node_has_opp_v2_prop() && !of_match_node(blocklist, np))
> +	if (cpu0_node_has_opp_v2_prop() && !of_match_node(blocklist, np) &&
> +	    !of_match_node(blocklist, soc_np))
>  		goto create_pdev;
> 
> +	of_node_put(soc_np);
>  	of_node_put(np);
>  	return -ENODEV;
> 
>  create_pdev:
> +	of_node_put(soc_np);
>  	of_node_put(np);
>  	return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq-dt",
>  			       -1, data,

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-27 21:16         ` Stephen Boyd
  2022-01-31 16:41           ` Bjorn Andersson
@ 2022-01-31 16:44           ` Doug Anderson
  2022-02-03 21:50             ` Stephen Boyd
  1 sibling, 1 reply; 21+ messages in thread
From: Doug Anderson @ 2022-01-31 16:44 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Bjorn Andersson, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Thu, Jan 27, 2022 at 1:16 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> ----8<-----
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
> b/drivers/cpufreq/cpufreq-dt-platdev.c
> index ca1d103ec449..32bfe453f8b4 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -179,25 +179,29 @@ static bool __init cpu0_node_has_opp_v2_prop(void)
>  static int __init cpufreq_dt_platdev_init(void)
>  {
>         struct device_node *np = of_find_node_by_path("/");
> +       struct device_node *soc_np = of_find_node_by_path("/soc");

Seems that some device trees have "/soc" and others "/soc@0". For at
least a little context, see commit a1875bf98290 ("arm64: dts: qcom:
sdm845: Add unit name to soc node"). Since (presumably) this would
only be for newer SoCs then I guess you should search for "/soc@0"?
...and then if we ever have a SoC that's not @0 then we would have to
iterate through all SoCs

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-31 16:41           ` Bjorn Andersson
@ 2022-01-31 16:50             ` Doug Anderson
  2022-02-01  1:01               ` Doug Anderson
  2022-02-03 21:55             ` Stephen Boyd
  1 sibling, 1 reply; 21+ messages in thread
From: Doug Anderson @ 2022-01-31 16:50 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephen Boyd, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Mon, Jan 31, 2022 at 8:41 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 27 Jan 15:16 CST 2022, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2022-01-25 19:01:31)
> > > On Tue 25 Jan 15:46 PST 2022, Doug Anderson wrote:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > > >
> > > > > Quoting Douglas Anderson (2022-01-25 14:44:22)
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..f95273052da0
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > > @@ -0,0 +1,313 @@
> > > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > > > +/*
> > > > > > + * Google Herobrine board device tree source
> > > > > > + *
> > > > > > + * Copyright 2022 Google LLC.
> > > > > > + */
> > > > > > +
> > > > > > +/dts-v1/;
> > > > > > +
> > > > > > +#include "sc7280-herobrine.dtsi"
> > > > > > +
> > > > > > +/ {
> > > > > > +       model = "Google Herobrine (rev1+)";
> > > > > > +       compatible = "google,herobrine", "qcom,sc7280";
> > > > >
> > > > > Can we stop adding "qcom,sc7280" to the board compatible string? It
> > > > > looks out of place. It's the compatible for the SoC and should really be
> > > > > the compatible for the /soc node.
> > > >
> > > > I don't have any objections, but I feel like this is the type of thing
> > > > I'd like Bjorn to have the final say on. What say you, Bjorn?
> > > >
> > >
> > > One practical case I can think of right away, where this matters is in
> > > cpufreq-dt-plat.c where we blocklist qcom,sc7280.
> > >
> > > I don't know if we rely on this in any other places, but I'm not keen on
> > > seeing a bunch of board-specific compatibles sprinkled throughout the
> > > implementation - it's annoying enough having to add each platform to
> > > these drivers.
> >
> > Looking at sc7180, grep only shows cpufreq-dt-plat.c
> >
>
> Good, then we handle all other platform specifics in drivers using
> platform-specific compatibles.
>
> >  $ git grep qcom,sc7180\" -- drivers
> >  drivers/cpufreq/cpufreq-dt-platdev.c:   { .compatible = "qcom,sc7180", },
> >
> > Simplest solution would be to look at / and /soc for a compatible
> > string.
> >
>
> You mean that / would contain the device's compatible and /soc the soc's
> compatible? I'm afraid I don't see how this would help you - you still
> need the compatible in the dts, just now in two places.
>
>
> Either we leave it as is - which follows my interpretation of what the DT
> spec says - or we (and the DT maitainers) agree that it shouldn't be
> there (because this dtb won't run on any random qcom,sc7180 anyways) at
> all.

I'm curious what part of the DT spec says that we should have the SoC
in there? I know I've always done it, but it's always just been
following the examples of what was done before. When talking about the
root node, I see this in the `devicetree-specification-v0.4-rc1` spec:

---

Specifies a list of platform architectures with which this platform is
compatible. This property can be used by operating systems in
selecting platform specific code. The recommended form of the property
value is: "manufacturer,model"

For example:
compatible = "fsl,mpc8572ds"

---

That doesn't say anything about putting the SoC there.


I would also note that I'd be at least moderately inclined to land
things as-is and deal with this in a follow-up patch, though I'm happy
to spin if that's what people agree upon too. This is not a new
problem and so it doesn't seem like it makes sense to glom dealing
with it into this patch series...

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: (subset) [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1
  2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
                   ` (4 preceding siblings ...)
  2022-01-25 22:44 ` [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1 Douglas Anderson
@ 2022-01-31 18:24 ` Bjorn Andersson
  5 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2022-01-31 18:24 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: mka, devicetree, Rob Herring, linux-arm-msm, sibis, quic_rjendra,
	pmaliset, Akhil P Oommen, konrad.dybcio, swboyd, Andy Gross,
	kgodara, linux-kernel

On Tue, 25 Jan 2022 14:44:17 -0800, Douglas Anderson wrote:
> This series adds support for herobrine-rev1. Note that it's likely
> that with the introduction of -rev1 we can drop -rev0 support, but
> we'll keep it for now (though we won't try to "fit it in" and share
> code with it).
> 
> This series is confirmed to boot herobrine-rev1 atop mainline, commit
> 0280e3c58f92 ("Merge tag 'nfs-for-5.17-1' of
> git://git.linux-nfs.org/projects/anna/linux-nfs"), though it requires
> a hack to work around a misconfigured DMA for i2c14
> (https://crrev.com/c/3378660)
> 
> [...]

Applied, thanks!

[1/5] arm64: dts: qcom: sc7280: Fix gmu unit address
      commit: 142a4d995c6adb6bf5b22166f51b525e83c96661
[2/5] arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts
      commit: 61a6262f95e0c400baee59ced0721f49ffca604c
[3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment
      commit: 56eead37681511d3bd5c5869cf2878865942ba75
[4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi
      commit: 40ab97eb383dfd9de37d049883a8707397a478b1

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-31 16:50             ` Doug Anderson
@ 2022-02-01  1:01               ` Doug Anderson
  2022-02-02 21:32                 ` Doug Anderson
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Anderson @ 2022-02-01  1:01 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephen Boyd, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Mon, Jan 31, 2022 at 8:50 AM Doug Anderson <dianders@chromium.org> wrote:
>
> > Either we leave it as is - which follows my interpretation of what the DT
> > spec says - or we (and the DT maitainers) agree that it shouldn't be
> > there (because this dtb won't run on any random qcom,sc7180 anyways) at
> > all.
>
> I'm curious what part of the DT spec says that we should have the SoC
> in there? I know I've always done it, but it's always just been
> following the examples of what was done before. When talking about the
> root node, I see this in the `devicetree-specification-v0.4-rc1` spec:
>
> ---
>
> Specifies a list of platform architectures with which this platform is
> compatible. This property can be used by operating systems in
> selecting platform specific code. The recommended form of the property
> value is: "manufacturer,model"
>
> For example:
> compatible = "fsl,mpc8572ds"
>
> ---
>
> That doesn't say anything about putting the SoC there.
>
>
> I would also note that I'd be at least moderately inclined to land
> things as-is and deal with this in a follow-up patch, though I'm happy
> to spin if that's what people agree upon too. This is not a new
> problem and so it doesn't seem like it makes sense to glom dealing
> with it into this patch series...

I noticed that you applied the first 4 patches in the series (thanks!)
but not this one. Are we waiting to get agreement on this before
landing? As per above, I think it'd be OK to land as-is and then I'm
happy to do a follow-up patch to clean this up since this isn't a new
issue. Having this patch outstanding makes it a little confusing with
the other cleanup patches that I'm posting... ;-)

Thanks!

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: (subset) [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment
  2022-01-25 22:44 ` [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment Douglas Anderson
@ 2022-02-01  5:19   ` Bjorn Andersson
  0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Andersson @ 2022-02-01  5:19 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: quic_rjendra, swboyd, mka, Rob Herring, sibis, devicetree,
	konrad.dybcio, kgodara, pmaliset, linux-arm-msm, Andy Gross,
	linux-kernel

On Tue, 25 Jan 2022 14:44:20 -0800, Douglas Anderson wrote:
> This factors out a device tree fragment from some sc7280 device
> trees. It represents the device tree bits that should be included for
> "Chrome" based sc7280 boards. On these boards the bootloader (Coreboot
> + Depthcharge) configures things slightly different than the
> bootloader that Qualcomm provides. The modem firmware on these boards
> also works differently than on other Qulacomm products and thus the
> reserved memory map needs to be adjusted.
> 
> [...]

Applied, thanks!

[3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment
      commit: 90c856602e0346ce9ff234062e86a198d71fa723

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address
  2022-01-25 22:44 ` [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address Douglas Anderson
@ 2022-02-01  7:27   ` Akhil P Oommen
  0 siblings, 0 replies; 21+ messages in thread
From: Akhil P Oommen @ 2022-02-01  7:27 UTC (permalink / raw)
  To: Douglas Anderson, Bjorn Andersson
  Cc: konrad.dybcio, swboyd, kgodara, mka, sibis, pmaliset,
	quic_rjendra, Andy Gross, Rob Herring, devicetree, linux-arm-msm,
	linux-kernel

On 1/26/2022 4:14 AM, Douglas Anderson wrote:
> When processing sc7280 device trees, I can see:
>
>    Warning (simple_bus_reg): /soc@0/gmu@3d69000:
>      simple-bus unit address format error, expected "3d6a000"
>
> There's a clear typo in the node name. Fix it.
>
> Fixes: 96c471970b7b ("arm64: dts: qcom: sc7280: Add gpu support")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
> (no changes since v1)
>
>   arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 937c2e0e93eb..eab7a8505053 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1790,7 +1790,7 @@ opp-550000000 {
>   			};
>   		};
>   
> -		gmu: gmu@3d69000 {
> +		gmu: gmu@3d6a000 {
>   			compatible="qcom,adreno-gmu-635.0", "qcom,adreno-gmu";
>   			reg = <0 0x03d6a000 0 0x34000>,
>   				<0 0x3de0000 0 0x10000>,

My bad! Thanks for the fix.

fwiw, Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com>

-Akhil.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-02-01  1:01               ` Doug Anderson
@ 2022-02-02 21:32                 ` Doug Anderson
  0 siblings, 0 replies; 21+ messages in thread
From: Doug Anderson @ 2022-02-02 21:32 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephen Boyd, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Mon, Jan 31, 2022 at 5:01 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jan 31, 2022 at 8:50 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > > Either we leave it as is - which follows my interpretation of what the DT
> > > spec says - or we (and the DT maitainers) agree that it shouldn't be
> > > there (because this dtb won't run on any random qcom,sc7180 anyways) at
> > > all.
> >
> > I'm curious what part of the DT spec says that we should have the SoC
> > in there? I know I've always done it, but it's always just been
> > following the examples of what was done before. When talking about the
> > root node, I see this in the `devicetree-specification-v0.4-rc1` spec:
> >
> > ---
> >
> > Specifies a list of platform architectures with which this platform is
> > compatible. This property can be used by operating systems in
> > selecting platform specific code. The recommended form of the property
> > value is: "manufacturer,model"
> >
> > For example:
> > compatible = "fsl,mpc8572ds"
> >
> > ---
> >
> > That doesn't say anything about putting the SoC there.
> >
> >
> > I would also note that I'd be at least moderately inclined to land
> > things as-is and deal with this in a follow-up patch, though I'm happy
> > to spin if that's what people agree upon too. This is not a new
> > problem and so it doesn't seem like it makes sense to glom dealing
> > with it into this patch series...
>
> I noticed that you applied the first 4 patches in the series (thanks!)
> but not this one. Are we waiting to get agreement on this before
> landing? As per above, I think it'd be OK to land as-is and then I'm
> happy to do a follow-up patch to clean this up since this isn't a new
> issue. Having this patch outstanding makes it a little confusing with
> the other cleanup patches that I'm posting... ;-)

I didn't hear anything and I was sending a new version of the cleanup
patch series, so I:

* Added this last patch to the end of the cleanup series.

* Addressed the "-regulator" suffix issue that Stephen pointed out.

* Didn't remove the SoC compatible from the top-level node in this
patch, but added follow-on patches in the series that do it.

Hopefully that looks good to everyone. I removed both of Stephen's and
Matthias's review tags from the v3 version.

https://lore.kernel.org/r/20220202132301.v3.12.I5604b7af908e8bbe709ac037a6a8a6ba8a2bfa94@changeid

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-31 16:44           ` Doug Anderson
@ 2022-02-03 21:50             ` Stephen Boyd
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Boyd @ 2022-02-03 21:50 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Bjorn Andersson, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring, devicetree, linux-arm-msm, LKML

Quoting Doug Anderson (2022-01-31 08:44:35)
> Hi,
>
> On Thu, Jan 27, 2022 at 1:16 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > ----8<-----
> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
> > b/drivers/cpufreq/cpufreq-dt-platdev.c
> > index ca1d103ec449..32bfe453f8b4 100644
> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> > @@ -179,25 +179,29 @@ static bool __init cpu0_node_has_opp_v2_prop(void)
> >  static int __init cpufreq_dt_platdev_init(void)
> >  {
> >         struct device_node *np = of_find_node_by_path("/");
> > +       struct device_node *soc_np = of_find_node_by_path("/soc");
>
> Seems that some device trees have "/soc" and others "/soc@0". For at
> least a little context, see commit a1875bf98290 ("arm64: dts: qcom:
> sdm845: Add unit name to soc node"). Since (presumably) this would
> only be for newer SoCs then I guess you should search for "/soc@0"?
> ...and then if we ever have a SoC that's not @0 then we would have to
> iterate through all SoCs
>

Yes. We can probably just use of_find_node_by_name(NULL, "soc") instead
though and then if there are multiple soc nodes in the future we can
iterate over all the soc nodes with for_each_of_allnodes_from() and make
this logic more complicated. I'd rather not implement any of that
complicated logic until we need to though.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1
  2022-01-31 16:41           ` Bjorn Andersson
  2022-01-31 16:50             ` Doug Anderson
@ 2022-02-03 21:55             ` Stephen Boyd
  1 sibling, 0 replies; 21+ messages in thread
From: Stephen Boyd @ 2022-02-03 21:55 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Doug Anderson, Viresh Kumar, Konrad Dybcio, kgodara,
	Matthias Kaehlcke, Sibi Sankar, Prasad Malisetty, quic_rjendra,
	Andy Gross, Rob Herring, devicetree, linux-arm-msm, LKML

Quoting Bjorn Andersson (2022-01-31 08:41:47)
> On Thu 27 Jan 15:16 CST 2022, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2022-01-25 19:01:31)
> > > On Tue 25 Jan 15:46 PST 2022, Doug Anderson wrote:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Jan 25, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > > > >
> > > > > Quoting Douglas Anderson (2022-01-25 14:44:22)
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..f95273052da0
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
> > > > > > @@ -0,0 +1,313 @@
> > > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > > > +/*
> > > > > > + * Google Herobrine board device tree source
> > > > > > + *
> > > > > > + * Copyright 2022 Google LLC.
> > > > > > + */
> > > > > > +
> > > > > > +/dts-v1/;
> > > > > > +
> > > > > > +#include "sc7280-herobrine.dtsi"
> > > > > > +
> > > > > > +/ {
> > > > > > +       model = "Google Herobrine (rev1+)";
> > > > > > +       compatible = "google,herobrine", "qcom,sc7280";
> > > > >
> > > > > Can we stop adding "qcom,sc7280" to the board compatible string? It
> > > > > looks out of place. It's the compatible for the SoC and should really be
> > > > > the compatible for the /soc node.
> > > >
> > > > I don't have any objections, but I feel like this is the type of thing
> > > > I'd like Bjorn to have the final say on. What say you, Bjorn?
> > > >
> > >
> > > One practical case I can think of right away, where this matters is in
> > > cpufreq-dt-plat.c where we blocklist qcom,sc7280.
> > >
> > > I don't know if we rely on this in any other places, but I'm not keen on
> > > seeing a bunch of board-specific compatibles sprinkled throughout the
> > > implementation - it's annoying enough having to add each platform to
> > > these drivers.
> >
> > Looking at sc7180, grep only shows cpufreq-dt-plat.c
> >
>
> Good, then we handle all other platform specifics in drivers using
> platform-specific compatibles.
>
> >  $ git grep qcom,sc7180\" -- drivers
> >  drivers/cpufreq/cpufreq-dt-platdev.c:   { .compatible = "qcom,sc7180", },
> >
> > Simplest solution would be to look at / and /soc for a compatible
> > string.
> >
>
> You mean that / would contain the device's compatible and /soc the soc's
> compatible? I'm afraid I don't see how this would help you - you still
> need the compatible in the dts, just now in two places.

I'd like / to contain the board compatible string and /soc to contain
the SoC's compatible string. The two strings are different. In this case
the board compatible at the root would be "google,trogdor-lazor" and the
soc node compatible would be "qcom,sc7180".

>
>
> Either we leave it as is - which follows my interpretation of what the DT
> spec says - or we (and the DT maitainers) agree that it shouldn't be
> there (because this dtb won't run on any random qcom,sc7180 anyways) at
> all.

Sure. Hopefully DT maintainers can chime in here.

As you say, this dtb won't run on any random board that has a
qcom,sc7180 SoC placed on the PCB so having it in the root node
compatible is redundant at the least.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-02-03 21:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 22:44 [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Douglas Anderson
2022-01-25 22:44 ` [PATCH v2 1/5] arm64: dts: qcom: sc7280: Fix gmu unit address Douglas Anderson
2022-02-01  7:27   ` Akhil P Oommen
2022-01-25 22:44 ` [PATCH v2 2/5] arm64: dts: qcom: sc7280: Move herobrine-r0 to its own dts Douglas Anderson
2022-01-25 22:44 ` [PATCH v2 3/5] arm64: dts: qcom: sc7280: Factor out Chrome common fragment Douglas Anderson
2022-02-01  5:19   ` (subset) " Bjorn Andersson
2022-01-25 22:44 ` [PATCH v2 4/5] arm64: dts: qcom: sc7280: Factor gpio.h include to sc7280.dtsi Douglas Anderson
2022-01-25 22:54   ` Stephen Boyd
2022-01-25 22:44 ` [PATCH v2 5/5] arm64: dts: qcom: sc7280: Add herobrine-r1 Douglas Anderson
2022-01-25 22:58   ` Stephen Boyd
2022-01-25 23:46     ` Doug Anderson
2022-01-26  3:01       ` Bjorn Andersson
2022-01-27 21:16         ` Stephen Boyd
2022-01-31 16:41           ` Bjorn Andersson
2022-01-31 16:50             ` Doug Anderson
2022-02-01  1:01               ` Doug Anderson
2022-02-02 21:32                 ` Doug Anderson
2022-02-03 21:55             ` Stephen Boyd
2022-01-31 16:44           ` Doug Anderson
2022-02-03 21:50             ` Stephen Boyd
2022-01-31 18:24 ` (subset) [PATCH v2 0/5] arm64: dts: qcom: sc7280: Introduce herobrine-rev1 Bjorn Andersson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.