Phone-Devel Archive on lore.kernel.org.
 help / color / Atom feed
* [PATCH] arm64: dts: add support for the Pixel 2 XL
@ 2021-03-05 21:35 Caleb Connolly
  2021-03-06  3:37 ` Bjorn Andersson
  2021-03-06 10:49 ` Konrad Dybcio
  0 siblings, 2 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-05 21:35 UTC (permalink / raw)
  To: caleb, Andy Gross, Bjorn Andersson, Rob Herring, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck
  Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
	devicetree, linux-kernel

Add a minimal devicetree capable of booting on the Pixel 2 XL MSM8998
device.

It's currently possible to boot the device into postmarketOS with USB
networking, however the display panel depends on Display Stream
Compression which is not yet supported in the kernel.

The bootloader also requires that the dtbo partition contains a device
tree overlay with a particular id which has to be overlayed onto the
existing dtb. It's possible to use a specially crafted dtbo partition to
workaround this, more information is available here:

    https://gitlab.com/calebccff/dtbo-google-wahoo-mainline

Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
It's possible to get wifi working by running Bjorns diag-router in the
background, without this the wifi firmware crashes every 10 seconds or
so. This is the same issue encountered on the OnePlus 5.

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/msm8998-google-taimen.dts   |  14 +
 .../boot/dts/qcom/msm8998-google-wahoo.dtsi   | 391 ++++++++++++++++++
 3 files changed, 406 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 5113fac80b7a..d942d3ec3928 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-msft-lumia-cityman.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-sony-xperia-kitakami-sumire.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-asus-novago-tp370ql.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-google-taimen.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
new file mode 100644
index 000000000000..ffaaafe14037
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Caleb Connolly <caleb@connolly.tech>
+ */
+
+/dts-v1/;
+
+#include "msm8998-google-wahoo.dtsi"
+
+/ {
+	model = "Google Pixel 2 XL";
+	compatible = "google,taimen", "google,wahoo", "qcom,msm8998", "qcom,msm8998-mtp";
+	qcom,msm-id = <0x124 0x20001>;
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
new file mode 100644
index 000000000000..0c221ead2df7
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
@@ -0,0 +1,391 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Caleb Connolly <caleb@connolly.tech> */
+
+#include "msm8998.dtsi"
+#include "pm8998.dtsi"
+#include "pmi8998.dtsi"
+#include "pm8005.dtsi"
+
+/delete-node/ &mpss_mem;
+/delete-node/ &venus_mem;
+/delete-node/ &mba_mem;
+/delete-node/ &slpi_mem;
+
+/ {
+	aliases {
+	};
+
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* Add "earlycon" intended to be used in combination with UART serial console */
+		bootargs = "clk_ignore_unused earlycon console=ttyGS0,115200";// loglevel=10 drm.debug=15 debug";
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+ 		#size-cells = <2>;
+ 		ranges;
+
+		mpss_mem: memory@8cc00000 {
+			reg = <0 0x8cc00000 0 0x7800000>;
+			no-map;
+		};
+
+		venus_mem: memory@94400000 {
+			reg = <0 0x94400000 0 0x500000>;
+			no-map;
+		};
+
+		mba_mem: memory@94100000 {
+			reg = <0 0x94900000 0 0x200000>;
+			no-map;
+		};
+
+		slpi_mem: memory@94b00000 {
+			reg = <0 0x94b00000 0 0x700000>;
+			no-map;
+		};
+
+		ramoops: ramoops@a1810000 {
+			compatible = "ramoops";
+			reg = <0 0xa1810000 0 0x200000>;
+			record-size = <0x20000>;
+			console-size = <0x100000>;
+			pmsg-size = <0x80000>;
+		};
+	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+};
+
+&blsp1_uart3 {
+	status = "disabled";
+
+	bluetooth {
+		compatible = "qcom,wcn3990-bt";
+
+		vddio-supply = <&vreg_s4a_1p8>;
+		vddxo-supply = <&vreg_l7a_1p8>;
+		vddrf-supply = <&vreg_l17a_1p3>;
+		vddch0-supply = <&vreg_l25a_3p3>;
+		max-speed = <3200000>;
+	};
+};
+
+&pcie0 {
+	status = "disabled";
+};
+
+&pm8005_lsid1 {
+	pm8005-regulators {
+		compatible = "qcom,pm8005-regulators";
+
+		vdd_s1-supply = <&vph_pwr>;
+
+		pm8005_s1: s1 { /* VDD_GFX supply */
+			regulator-min-microvolt = <524000>;
+			regulator-max-microvolt = <1100000>;
+			regulator-enable-ramp-delay = <500>;
+
+			/* hack until we rig up the gpu consumer */
+			regulator-always-on;
+		};
+	};
+};
+
+&qusb2phy {
+	status = "okay";
+
+	vdda-pll-supply = <&vreg_l12a_1p8>;
+	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
+};
+
+&remoteproc_adsp {
+	status = "okay";
+
+	firmware-name = "qcom/pixel2/adsp.mdt";
+};
+
+&remoteproc_mss {
+	firmware-name = "qcom/pixel2/mba.mbn",
+	                "qcom/pixel2/modem.mdt";
+};
+
+&remoteproc_slpi {
+	status = "okay";
+
+	firmware-name = "qcom/pixel2/slpi_v2.mdt";
+};
+
+&rpm_requests {
+	pm8998-regulators {
+		compatible = "qcom,rpm-pm8998-regulators";
+
+		vdd_s1-supply = <&vph_pwr>;
+		vdd_s2-supply = <&vph_pwr>;
+		vdd_s3-supply = <&vph_pwr>;
+		vdd_s4-supply = <&vph_pwr>;
+		vdd_s5-supply = <&vph_pwr>;
+		vdd_s6-supply = <&vph_pwr>;
+		vdd_s7-supply = <&vph_pwr>;
+		vdd_s8-supply = <&vph_pwr>;
+		vdd_s9-supply = <&vph_pwr>;
+		vdd_s10-supply = <&vph_pwr>;
+		vdd_s11-supply = <&vph_pwr>;
+		vdd_s12-supply = <&vph_pwr>;
+		vdd_s13-supply = <&vph_pwr>;
+		vdd_l1_l27-supply = <&vreg_s7a_1p025>;
+		vdd_l2_l8_l17-supply = <&vreg_s3a_1p35>;
+		vdd_l3_l11-supply = <&vreg_s7a_1p025>;
+		vdd_l4_l5-supply = <&vreg_s7a_1p025>;
+		vdd_l6-supply = <&vreg_s5a_2p04>;
+		vdd_l7_l12_l14_l15-supply = <&vreg_s5a_2p04>;
+		vdd_l9-supply = <&vreg_bob>;
+		vdd_l10_l23_l25-supply = <&vreg_bob>;
+		vdd_l13_l19_l21-supply = <&vreg_bob>;
+		vdd_l16_l28-supply = <&vreg_bob>;
+		vdd_l18_l22-supply = <&vreg_bob>;
+		vdd_l20_l24-supply = <&vreg_bob>;
+		vdd_l26-supply = <&vreg_s3a_1p35>;
+		vdd_lvs1_lvs2-supply = <&vreg_s4a_1p8>;
+
+		vreg_s3a_1p35: s3 {
+			regulator-min-microvolt = <1352000>;
+			regulator-max-microvolt = <1352000>;
+		};
+		vreg_s4a_1p8: s4 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
+		};
+		vreg_s5a_2p04: s5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+		vreg_s7a_1p025: s7 {
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1028000>;
+		};
+		vreg_l1a_0p875: l1 {
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+		};
+		vreg_l2a_1p2: l2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+		vreg_l3a_1p0: l3 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1000000>;
+		};
+		vreg_l5a_0p8: l5 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <800000>;
+		};
+		vreg_l6a_1p8: l6 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <1808000>;
+		};
+		vreg_l7a_1p8: l7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l8a_1p2: l8 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+		vreg_l9a_1p8: l9 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l10a_1p8: l10 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l11a_1p0: l11 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1000000>;
+		};
+		vreg_l12a_1p8: l12 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l13a_2p95: l13 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l14a_1p88: l14 {
+			regulator-min-microvolt = <1880000>;
+			regulator-max-microvolt = <1880000>;
+		};
+		vreg_15a_1p8: l15 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l16a_2p7: l16 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2704000>;
+		};
+		vreg_l17a_1p3: l17 {
+			regulator-min-microvolt = <1304000>;
+			regulator-max-microvolt = <1304000>;
+		};
+		vreg_l18a_2p7: l18 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2704000>;
+		};
+		vreg_l19a_3p0: l19 {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+		};
+		vreg_l20a_2p95: l20 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
+		};
+		vreg_l21a_2p95: l21 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
+			regulator-system-load = <800000>;
+		};
+		vreg_l22a_2p85: l22 {
+			regulator-min-microvolt = <2864000>;
+			regulator-max-microvolt = <2864000>;
+		};
+		vreg_l23a_3p3: l23 {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3312000>;
+		};
+		vreg_l24a_3p075: l24 {
+			regulator-min-microvolt = <3088000>;
+			regulator-max-microvolt = <3088000>;
+		};
+		vreg_l25a_3p3: l25 {
+			regulator-min-microvolt = <3104000>;
+			regulator-max-microvolt = <3312000>;
+		};
+		vreg_l26a_1p2: l26 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-allow-set-load;
+		};
+		vreg_l28_3p0: l28 {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+		};
+
+		vreg_lvs1a_1p8: lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_lvs2a_1p8: lvs2 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+	};
+
+	pmi8998-regulators {
+		compatible = "qcom,rpm-pmi8998-regulators";
+
+		vdd_bob-supply = <&vph_pwr>;
+
+		vreg_bob: bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+		};
+	};
+};
+
+&spmi_bus {
+	pmic@0 {
+		compatible = "qcom,pm8994", "qcom,spmi-pmic";
+		reg = <0x0 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pon@800 {
+			compatible = "qcom,pm8916-pon";
+
+			reg = <0x800>;
+			mode-bootloader = <0x2>;
+			mode-recovery = <0x1>;
+
+			pwrkey {
+				compatible = "qcom,pm8941-pwrkey";
+				interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
+				debounce = <15625>;
+				bias-pull-up;
+				linux,code = <KEY_POWER>;
+			};
+		};
+	};
+};
+
+&tlmm {
+	gpio-reserved-ranges = <0 4>, <81 4>;
+};
+
+/*
+ * The device does contain a USB3 capable type-c port,
+ * however it doesn't seem to work when superspeed mode is
+ * enabled.
+ */
+&usb3 {
+	status = "okay";
+
+	/* Operate "GCC_USB30_MASTER_CLK" in HS mode (>=60 MHz) */
+	assigned-clock-rates = <19200000>, <60000000>;
+
+	/* Disable USB3 pipe_clk requirement */
+	qcom,select-utmi-as-pipe-clk;
+};
+
+&usb3_dwc3 {
+	/* Drop USB 3 SuperSpeed PHY to bring up the "usb0" interface */
+	phys = <&qusb2phy>;
+	phy-names = "usb2-phy";
+
+	/* We can only operate at USB 2.0 speeds */
+	maximum-speed = "high-speed";
+
+	/* Force to peripheral until we have Type-C hooked up */
+	dr_mode = "peripheral";
+};
+
+&ufshc {
+	vcc-supply = <&vreg_l20a_2p95>;
+	vccq-supply = <&vreg_l26a_1p2>;
+	vccq2-supply = <&vreg_s4a_1p8>;
+	vcc-max-microamp = <750000>;
+	vccq-max-microamp = <560000>;
+	vccq2-max-microamp = <750000>;
+};
+
+&ufsphy {
+	vdda-phy-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l2a_1p2>;
+	vddp-ref-clk-supply = <&vreg_l26a_1p2>;
+	vdda-phy-max-microamp = <51400>;
+	vdda-pll-max-microamp = <14600>;
+	vddp-ref-clk-max-microamp = <100>;
+	vddp-ref-clk-always-on;
+};
+
+&wifi {
+	status = "okay";
+
+	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
+	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
+	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
+	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
+};
-- 
2.29.2



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

* Re: [PATCH] arm64: dts: add support for the Pixel 2 XL
  2021-03-05 21:35 [PATCH] arm64: dts: add support for the Pixel 2 XL Caleb Connolly
@ 2021-03-06  3:37 ` Bjorn Andersson
  2021-03-06 18:01   ` Caleb Connolly
  2021-03-06 10:49 ` Konrad Dybcio
  1 sibling, 1 reply; 5+ messages in thread
From: Bjorn Andersson @ 2021-03-06  3:37 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Andy Gross, Rob Herring, Kees Cook, Anton Vorontsov, Colin Cross,
	Tony Luck, ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
	devicetree, linux-kernel

On Fri 05 Mar 15:35 CST 2021, Caleb Connolly wrote:

Please add "qcom: " to $subject as well.

> Add a minimal devicetree capable of booting on the Pixel 2 XL MSM8998
> device.
> 
> It's currently possible to boot the device into postmarketOS with USB
> networking, however the display panel depends on Display Stream
> Compression which is not yet supported in the kernel.
> 
> The bootloader also requires that the dtbo partition contains a device
> tree overlay with a particular id which has to be overlayed onto the
> existing dtb. It's possible to use a specially crafted dtbo partition to
> workaround this, more information is available here:
> 
>     https://gitlab.com/calebccff/dtbo-google-wahoo-mainline
> 

So it's not possible to just erase the dto, like on most other devices?

> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
> It's possible to get wifi working by running Bjorns diag-router in the
> background, without this the wifi firmware crashes every 10 seconds or
> so. This is the same issue encountered on the OnePlus 5.
> 
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../boot/dts/qcom/msm8998-google-taimen.dts   |  14 +
>  .../boot/dts/qcom/msm8998-google-wahoo.dtsi   | 391 ++++++++++++++++++
>  3 files changed, 406 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 5113fac80b7a..d942d3ec3928 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-msft-lumia-cityman.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-sony-xperia-kitakami-sumire.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-asus-novago-tp370ql.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-google-taimen.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
> new file mode 100644
> index 000000000000..ffaaafe14037
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only

Would you be willing to release these as BSD-3-Clause instead?

> +/*
> + * Copyright (c) 2020, Caleb Connolly <caleb@connolly.tech>
> + */
> +
> +/dts-v1/;
> +
> +#include "msm8998-google-wahoo.dtsi"
> +
> +/ {
> +	model = "Google Pixel 2 XL";
> +	compatible = "google,taimen", "google,wahoo", "qcom,msm8998", "qcom,msm8998-mtp";
> +	qcom,msm-id = <0x124 0x20001>;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
> new file mode 100644
> index 000000000000..0c221ead2df7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
> @@ -0,0 +1,391 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 Caleb Connolly <caleb@connolly.tech> */
> +
> +#include "msm8998.dtsi"
> +#include "pm8998.dtsi"
> +#include "pmi8998.dtsi"
> +#include "pm8005.dtsi"
> +
> +/delete-node/ &mpss_mem;
> +/delete-node/ &venus_mem;
> +/delete-node/ &mba_mem;
> +/delete-node/ &slpi_mem;
> +
> +/ {
> +	aliases {
> +	};
> +
> +	chosen {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		/* Add "earlycon" intended to be used in combination with UART serial console */
> +		bootargs = "clk_ignore_unused earlycon console=ttyGS0,115200";// loglevel=10 drm.debug=15 debug";

Please drop earlycon from this list (user should be able to add it if
they care later?) and use stdout-path to set the console, like we do on
other devices.

> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> + 		#size-cells = <2>;
> + 		ranges;
> +
> +		mpss_mem: memory@8cc00000 {
> +			reg = <0 0x8cc00000 0 0x7800000>;
> +			no-map;
> +		};
> +
> +		venus_mem: memory@94400000 {
> +			reg = <0 0x94400000 0 0x500000>;
> +			no-map;
> +		};
> +
> +		mba_mem: memory@94100000 {
> +			reg = <0 0x94900000 0 0x200000>;
> +			no-map;
> +		};
> +
> +		slpi_mem: memory@94b00000 {
> +			reg = <0 0x94b00000 0 0x700000>;
> +			no-map;
> +		};
> +
> +		ramoops: ramoops@a1810000 {
> +			compatible = "ramoops";
> +			reg = <0 0xa1810000 0 0x200000>;
> +			record-size = <0x20000>;
> +			console-size = <0x100000>;
> +			pmsg-size = <0x80000>;
> +		};
> +	};
> +
> +	vph_pwr: vph-pwr-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vph_pwr";
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +};
> +
> +&blsp1_uart3 {
> +	status = "disabled";
> +
> +	bluetooth {
> +		compatible = "qcom,wcn3990-bt";
> +
> +		vddio-supply = <&vreg_s4a_1p8>;
> +		vddxo-supply = <&vreg_l7a_1p8>;
> +		vddrf-supply = <&vreg_l17a_1p3>;
> +		vddch0-supply = <&vreg_l25a_3p3>;
> +		max-speed = <3200000>;
> +	};
> +};
> +
> +&pcie0 {
> +	status = "disabled";

Isn't &pcie0 already disabled, from msm8998.dtsi?

> +};
> +
> +&pm8005_lsid1 {
> +	pm8005-regulators {
> +		compatible = "qcom,pm8005-regulators";
> +
> +		vdd_s1-supply = <&vph_pwr>;
> +
> +		pm8005_s1: s1 { /* VDD_GFX supply */
> +			regulator-min-microvolt = <524000>;
> +			regulator-max-microvolt = <1100000>;
> +			regulator-enable-ramp-delay = <500>;
> +
> +			/* hack until we rig up the gpu consumer */
> +			regulator-always-on;
> +		};
> +	};
> +};
> +
> +&qusb2phy {
> +	status = "okay";
> +
> +	vdda-pll-supply = <&vreg_l12a_1p8>;
> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> +};
> +
> +&remoteproc_adsp {
> +	status = "okay";
> +
> +	firmware-name = "qcom/pixel2/adsp.mdt";
> +};
> +
> +&remoteproc_mss {
> +	firmware-name = "qcom/pixel2/mba.mbn",
> +	                "qcom/pixel2/modem.mdt";
> +};
> +
> +&remoteproc_slpi {
> +	status = "okay";
> +
> +	firmware-name = "qcom/pixel2/slpi_v2.mdt";
> +};
> +
> +&rpm_requests {
> +	pm8998-regulators {
> +		compatible = "qcom,rpm-pm8998-regulators";
> +
> +		vdd_s1-supply = <&vph_pwr>;
> +		vdd_s2-supply = <&vph_pwr>;
> +		vdd_s3-supply = <&vph_pwr>;
> +		vdd_s4-supply = <&vph_pwr>;
> +		vdd_s5-supply = <&vph_pwr>;
> +		vdd_s6-supply = <&vph_pwr>;
> +		vdd_s7-supply = <&vph_pwr>;
> +		vdd_s8-supply = <&vph_pwr>;
> +		vdd_s9-supply = <&vph_pwr>;
> +		vdd_s10-supply = <&vph_pwr>;
> +		vdd_s11-supply = <&vph_pwr>;
> +		vdd_s12-supply = <&vph_pwr>;
> +		vdd_s13-supply = <&vph_pwr>;
> +		vdd_l1_l27-supply = <&vreg_s7a_1p025>;
> +		vdd_l2_l8_l17-supply = <&vreg_s3a_1p35>;
> +		vdd_l3_l11-supply = <&vreg_s7a_1p025>;
> +		vdd_l4_l5-supply = <&vreg_s7a_1p025>;
> +		vdd_l6-supply = <&vreg_s5a_2p04>;
> +		vdd_l7_l12_l14_l15-supply = <&vreg_s5a_2p04>;
> +		vdd_l9-supply = <&vreg_bob>;
> +		vdd_l10_l23_l25-supply = <&vreg_bob>;
> +		vdd_l13_l19_l21-supply = <&vreg_bob>;
> +		vdd_l16_l28-supply = <&vreg_bob>;
> +		vdd_l18_l22-supply = <&vreg_bob>;
> +		vdd_l20_l24-supply = <&vreg_bob>;
> +		vdd_l26-supply = <&vreg_s3a_1p35>;
> +		vdd_lvs1_lvs2-supply = <&vreg_s4a_1p8>;
> +
> +		vreg_s3a_1p35: s3 {
> +			regulator-min-microvolt = <1352000>;
> +			regulator-max-microvolt = <1352000>;
> +		};
> +		vreg_s4a_1p8: s4 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_s5a_2p04: s5 {
> +			regulator-min-microvolt = <1904000>;
> +			regulator-max-microvolt = <2040000>;
> +		};
> +		vreg_s7a_1p025: s7 {
> +			regulator-min-microvolt = <900000>;
> +			regulator-max-microvolt = <1028000>;
> +		};
> +		vreg_l1a_0p875: l1 {
> +			regulator-min-microvolt = <880000>;
> +			regulator-max-microvolt = <880000>;
> +		};
> +		vreg_l2a_1p2: l2 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +		};
> +		vreg_l3a_1p0: l3 {
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1000000>;
> +		};
> +		vreg_l5a_0p8: l5 {
> +			regulator-min-microvolt = <800000>;
> +			regulator-max-microvolt = <800000>;
> +		};
> +		vreg_l6a_1p8: l6 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <1808000>;
> +		};
> +		vreg_l7a_1p8: l7 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l8a_1p2: l8 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +		};
> +		vreg_l9a_1p8: l9 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l10a_1p8: l10 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l11a_1p0: l11 {
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1000000>;
> +		};
> +		vreg_l12a_1p8: l12 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l13a_2p95: l13 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l14a_1p88: l14 {
> +			regulator-min-microvolt = <1880000>;
> +			regulator-max-microvolt = <1880000>;
> +		};
> +		vreg_15a_1p8: l15 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l16a_2p7: l16 {
> +			regulator-min-microvolt = <2704000>;
> +			regulator-max-microvolt = <2704000>;
> +		};
> +		vreg_l17a_1p3: l17 {
> +			regulator-min-microvolt = <1304000>;
> +			regulator-max-microvolt = <1304000>;
> +		};
> +		vreg_l18a_2p7: l18 {
> +			regulator-min-microvolt = <2704000>;
> +			regulator-max-microvolt = <2704000>;
> +		};
> +		vreg_l19a_3p0: l19 {
> +			regulator-min-microvolt = <3008000>;
> +			regulator-max-microvolt = <3008000>;
> +		};
> +		vreg_l20a_2p95: l20 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2960000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_l21a_2p95: l21 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2960000>;
> +			regulator-allow-set-load;
> +			regulator-system-load = <800000>;
> +		};
> +		vreg_l22a_2p85: l22 {
> +			regulator-min-microvolt = <2864000>;
> +			regulator-max-microvolt = <2864000>;
> +		};
> +		vreg_l23a_3p3: l23 {
> +			regulator-min-microvolt = <3312000>;
> +			regulator-max-microvolt = <3312000>;
> +		};
> +		vreg_l24a_3p075: l24 {
> +			regulator-min-microvolt = <3088000>;
> +			regulator-max-microvolt = <3088000>;
> +		};
> +		vreg_l25a_3p3: l25 {
> +			regulator-min-microvolt = <3104000>;
> +			regulator-max-microvolt = <3312000>;
> +		};
> +		vreg_l26a_1p2: l26 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_l28_3p0: l28 {
> +			regulator-min-microvolt = <3008000>;
> +			regulator-max-microvolt = <3008000>;
> +		};
> +
> +		vreg_lvs1a_1p8: lvs1 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +
> +		vreg_lvs2a_1p8: lvs2 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +
> +	};
> +
> +	pmi8998-regulators {
> +		compatible = "qcom,rpm-pmi8998-regulators";
> +
> +		vdd_bob-supply = <&vph_pwr>;
> +
> +		vreg_bob: bob {
> +			regulator-min-microvolt = <3312000>;
> +			regulator-max-microvolt = <3600000>;
> +		};
> +	};
> +};
> +
> +&spmi_bus {
> +	pmic@0 {
> +		compatible = "qcom,pm8994", "qcom,spmi-pmic";

qcom,pm8994?

Per the include of pm8998.dtsi I think you already have pmic@0 defined,
with compatible of qcom,pm8998 here (which you override).

> +		reg = <0x0 SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pon@800 {

But pmic@0 already has pon@800 defined as &pm8998_pon, which seems to be
defined identically to yours.

So I think you should be able to drop this entire &spmi_bus node and its
children, but perhaps I'm just missing something obvious here?

> +			compatible = "qcom,pm8916-pon";
> +
> +			reg = <0x800>;
> +			mode-bootloader = <0x2>;
> +			mode-recovery = <0x1>;
> +
> +			pwrkey {
> +				compatible = "qcom,pm8941-pwrkey";
> +				interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> +				debounce = <15625>;
> +				bias-pull-up;
> +				linux,code = <KEY_POWER>;
> +			};
> +		};
> +	};
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <0 4>, <81 4>;
> +};
> +
> +/*
> + * The device does contain a USB3 capable type-c port,
> + * however it doesn't seem to work when superspeed mode is
> + * enabled.
> + */
> +&usb3 {
> +	status = "okay";
> +
> +	/* Operate "GCC_USB30_MASTER_CLK" in HS mode (>=60 MHz) */
> +	assigned-clock-rates = <19200000>, <60000000>;
> +
> +	/* Disable USB3 pipe_clk requirement */
> +	qcom,select-utmi-as-pipe-clk;
> +};
> +
> +&usb3_dwc3 {
> +	/* Drop USB 3 SuperSpeed PHY to bring up the "usb0" interface */
> +	phys = <&qusb2phy>;
> +	phy-names = "usb2-phy";
> +
> +	/* We can only operate at USB 2.0 speeds */
> +	maximum-speed = "high-speed";
> +
> +	/* Force to peripheral until we have Type-C hooked up */
> +	dr_mode = "peripheral";
> +};
> +
> +&ufshc {
> +	vcc-supply = <&vreg_l20a_2p95>;
> +	vccq-supply = <&vreg_l26a_1p2>;
> +	vccq2-supply = <&vreg_s4a_1p8>;
> +	vcc-max-microamp = <750000>;
> +	vccq-max-microamp = <560000>;
> +	vccq2-max-microamp = <750000>;
> +};
> +
> +&ufsphy {
> +	vdda-phy-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l2a_1p2>;
> +	vddp-ref-clk-supply = <&vreg_l26a_1p2>;
> +	vdda-phy-max-microamp = <51400>;
> +	vdda-pll-max-microamp = <14600>;
> +	vddp-ref-clk-max-microamp = <100>;
> +	vddp-ref-clk-always-on;
> +};
> +
> +&wifi {
> +	status = "okay";
> +

Can you please disable diag-router and try adding this here instead:

	clocks = <&rpmcc RPM_SMD_RF_CLK2_PIN>, <&rpmcc RPM_SMD_QDSS_CLK>;
	clock-names = "cxo_ref_clk_pin", "qdss";

What you describe in your comment above seems like what I saw on sm8150,
that launching diag-router would cause something on the modem side to
keep the debug subsystem clock on - which in turn caused WiFi not to
crash during loading.


I don't know why this was required, or if it's just working around
some other issue we're having.

Regards,
Bjorn

> +	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
> +	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
> +	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
> +	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
> +};
> -- 
> 2.29.2
> 
> 

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

* Re: [PATCH] arm64: dts: add support for the Pixel 2 XL
  2021-03-05 21:35 [PATCH] arm64: dts: add support for the Pixel 2 XL Caleb Connolly
  2021-03-06  3:37 ` Bjorn Andersson
@ 2021-03-06 10:49 ` Konrad Dybcio
  2021-03-06 18:31   ` Caleb Connolly
  1 sibling, 1 reply; 5+ messages in thread
From: Konrad Dybcio @ 2021-03-06 10:49 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Rob Herring,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck
  Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
	devicetree, linux-kernel


On 05.03.2021 22:35, Caleb Connolly wrote:
> Add a minimal devicetree capable of booting on the Pixel 2 XL MSM8998
> device.
>
> It's currently possible to boot the device into postmarketOS with USB
> networking, however the display panel depends on Display Stream
> Compression which is not yet supported in the kernel.
>
> The bootloader also requires that the dtbo partition contains a device
> tree overlay with a particular id which has to be overlayed onto the
> existing dtb. It's possible to use a specially crafted dtbo partition to
> workaround this, more information is available here:
>
>     https://gitlab.com/calebccff/dtbo-google-wahoo-mainline
>
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
> It's possible to get wifi working by running Bjorns diag-router in the
> background, without this the wifi firmware crashes every 10 seconds or
> so. This is the same issue encountered on the OnePlus 5.
>
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../boot/dts/qcom/msm8998-google-taimen.dts   |  14 +
>  .../boot/dts/qcom/msm8998-google-wahoo.dtsi   | 391 ++++++++++++++++++
>  3 files changed, 406 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 5113fac80b7a..d942d3ec3928 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-msft-lumia-cityman.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-sony-xperia-kitakami-sumire.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-asus-novago-tp370ql.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-google-taimen.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
> new file mode 100644
> index 000000000000..ffaaafe14037
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, Caleb Connolly <caleb@connolly.tech>
> + */
> +
> +/dts-v1/;
> +
> +#include "msm8998-google-wahoo.dtsi"
> +
> +/ {
> +	model = "Google Pixel 2 XL";
> +	compatible = "google,taimen", "google,wahoo", "qcom,msm8998", "qcom,msm8998-mtp";

Drop the mtp compatible. Also, afaict wahoo is a shared platform name for P2/2XL, so perhaps using the same naming scheme we used for Xperias/Lumias (soc-vendor-platform-board) would clear up some confusion. In this case, I'm not sure about the wahoo compatible, but I reckon it's fine for it to stay so that it's easier to introduce potential quirks that concern both devices.


> +	qcom,msm-id = <0x124 0x20001>;

Move it to the common dtsi, unless the other Pixel ships with a different SoC revision.


> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
> new file mode 100644
> index 000000000000..0c221ead2df7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
> @@ -0,0 +1,391 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 Caleb Connolly <caleb@connolly.tech> */
> +
> +#include "msm8998.dtsi"
> +#include "pm8998.dtsi"
> +#include "pmi8998.dtsi"
> +#include "pm8005.dtsi"
> +
> +/delete-node/ &mpss_mem;
> +/delete-node/ &venus_mem;
> +/delete-node/ &mba_mem;
> +/delete-node/ &slpi_mem;
> +
> +/ {
> +	aliases {
> +	};
> +
> +	chosen {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		/* Add "earlycon" intended to be used in combination with UART serial console */
> +		bootargs = "clk_ignore_unused earlycon console=ttyGS0,115200";// loglevel=10 drm.debug=15 debug";

clk_ignore_unused is a BIG hack!

You should trace which clocks are important for it to stay alive and fix it on the driver side.

What breaks if it's not there? Does it still happen with Angelo's clk patches that got in for the 5.12

window?

Aside from that, //loglevel... should also go.


> +
> +	vph_pwr: vph-pwr-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vph_pwr";
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +};

Don't you need to specify voltage here?


> +
> +&blsp1_uart3 {
> +	status = "disabled";
> +
> +	bluetooth {
> +		compatible = "qcom,wcn3990-bt";
> +
> +		vddio-supply = <&vreg_s4a_1p8>;
> +		vddxo-supply = <&vreg_l7a_1p8>;
> +		vddrf-supply = <&vreg_l17a_1p3>;
> +		vddch0-supply = <&vreg_l25a_3p3>;
> +		max-speed = <3200000>;
> +	};
> +};

Either enable the UART or rid the bluetooth for now.


> +
> +&pm8005_lsid1 {
> +	pm8005-regulators {
> +		compatible = "qcom,pm8005-regulators";
> +
> +		vdd_s1-supply = <&vph_pwr>;
> +
> +		pm8005_s1: s1 { /* VDD_GFX supply */

regulator-name = "vdd_gfx";


> +
> +&spmi_bus {
> +	pmic@0 {
> +		compatible = "qcom,pm8994", "qcom,spmi-pmic";
> +		reg = <0x0 SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pon@800 {
> +			compatible = "qcom,pm8916-pon";
> +
> +			reg = <0x800>;
> +			mode-bootloader = <0x2>;
> +			mode-recovery = <0x1>;
> +
> +			pwrkey {
> +				compatible = "qcom,pm8941-pwrkey";
> +				interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> +				debounce = <15625>;
> +				bias-pull-up;
> +				linux,code = <KEY_POWER>;
> +			};
> +		};
> +	};
> +};

That's a lot of indentation, it would be better to add a label: somewhere instead.. Moreover, the exact same pwrkey node is already present in pm8998.dtsi, so you should just drop this.


Konrad


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

* Re: [PATCH] arm64: dts: add support for the Pixel 2 XL
  2021-03-06  3:37 ` Bjorn Andersson
@ 2021-03-06 18:01   ` Caleb Connolly
  0 siblings, 0 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-06 18:01 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Rob Herring, Kees Cook, Anton Vorontsov, Colin Cross,
	Tony Luck, ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
	devicetree, linux-kernel

Hi Bjorn,

On 06/03/2021 3:37 am, Bjorn Andersson wrote:
> On Fri 05 Mar 15:35 CST 2021, Caleb Connolly wrote:
>
> Please add "qcom: " to $subject as well.
>
>> Add a minimal devicetree capable of booting on the Pixel 2 XL MSM8998
>> device.
>>
>> It's currently possible to boot the device into postmarketOS with USB
>> networking, however the display panel depends on Display Stream
>> Compression which is not yet supported in the kernel.
>>
>> The bootloader also requires that the dtbo partition contains a device
>> tree overlay with a particular id which has to be overlayed onto the
>> existing dtb. It's possible to use a specially crafted dtbo partition to
>> workaround this, more information is available here:
>>
>>      https://gitlab.com/calebccff/dtbo-google-wahoo-mainline
>>
> So it's not possible to just erase the dto, like on most other devices?

That's correct, I initially tried that but the bootloader just hung when 
I tried to boot anything.

It seems to require a DTO matching some revision-specific IDs, the 
bootloader is very unhappy when it can't

find them.

>
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>> ---
>> It's possible to get wifi working by running Bjorns diag-router in the
>> background, without this the wifi firmware crashes every 10 seconds or
>> so. This is the same issue encountered on the OnePlus 5.
>>
>>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>>   .../boot/dts/qcom/msm8998-google-taimen.dts   |  14 +
>>   .../boot/dts/qcom/msm8998-google-wahoo.dtsi   | 391 ++++++++++++++++++
>>   3 files changed, 406 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>>   create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 5113fac80b7a..d942d3ec3928 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-msft-lumia-cityman.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-sony-xperia-kitakami-sumire.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-asus-novago-tp370ql.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-google-taimen.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>> new file mode 100644
>> index 000000000000..ffaaafe14037
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>> @@ -0,0 +1,14 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
> Would you be willing to release these as BSD-3-Clause instead?
Yeah I'd be happy to, is this a new practice? As the device I submitted 
previously is also GPL-2.0.
>
>> +/*
>> + * Copyright (c) 2020, Caleb Connolly <caleb@connolly.tech>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8998-google-wahoo.dtsi"
>> +
>> +/ {
>> +	model = "Google Pixel 2 XL";
>> +	compatible = "google,taimen", "google,wahoo", "qcom,msm8998", "qcom,msm8998-mtp";
>> +	qcom,msm-id = <0x124 0x20001>;
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>> new file mode 100644
>> index 000000000000..0c221ead2df7
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>> @@ -0,0 +1,391 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2020 Caleb Connolly <caleb@connolly.tech> */
>> +
>> +#include "msm8998.dtsi"
>> +#include "pm8998.dtsi"
>> +#include "pmi8998.dtsi"
>> +#include "pm8005.dtsi"
>> +
>> +/delete-node/ &mpss_mem;
>> +/delete-node/ &venus_mem;
>> +/delete-node/ &mba_mem;
>> +/delete-node/ &slpi_mem;
>> +
>> +/ {
>> +	aliases {
>> +	};
>> +
>> +	chosen {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		/* Add "earlycon" intended to be used in combination with UART serial console */
>> +		bootargs = "clk_ignore_unused earlycon console=ttyGS0,115200";// loglevel=10 drm.debug=15 debug";
> Please drop earlycon from this list (user should be able to add it if
> they care later?) and use stdout-path to set the console, like we do on
> other devices.
>
>> +	};
>> +
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> + 		#size-cells = <2>;
>> + 		ranges;
>> +
>> +		mpss_mem: memory@8cc00000 {
>> +			reg = <0 0x8cc00000 0 0x7800000>;
>> +			no-map;
>> +		};
>> +
>> +		venus_mem: memory@94400000 {
>> +			reg = <0 0x94400000 0 0x500000>;
>> +			no-map;
>> +		};
>> +
>> +		mba_mem: memory@94100000 {
>> +			reg = <0 0x94900000 0 0x200000>;
>> +			no-map;
>> +		};
>> +
>> +		slpi_mem: memory@94b00000 {
>> +			reg = <0 0x94b00000 0 0x700000>;
>> +			no-map;
>> +		};
>> +
>> +		ramoops: ramoops@a1810000 {
>> +			compatible = "ramoops";
>> +			reg = <0 0xa1810000 0 0x200000>;
>> +			record-size = <0x20000>;
>> +			console-size = <0x100000>;
>> +			pmsg-size = <0x80000>;
>> +		};
>> +	};
>> +
>> +	vph_pwr: vph-pwr-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vph_pwr";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +	};
>> +};
>> +
>> +&blsp1_uart3 {
>> +	status = "disabled";
>> +
>> +	bluetooth {
>> +		compatible = "qcom,wcn3990-bt";
>> +
>> +		vddio-supply = <&vreg_s4a_1p8>;
>> +		vddxo-supply = <&vreg_l7a_1p8>;
>> +		vddrf-supply = <&vreg_l17a_1p3>;
>> +		vddch0-supply = <&vreg_l25a_3p3>;
>> +		max-speed = <3200000>;
>> +	};
>> +};
>> +
>> +&pcie0 {
>> +	status = "disabled";
> Isn't &pcie0 already disabled, from msm8998.dtsi?
>
>> +};
>> +
>> +&pm8005_lsid1 {
>> +	pm8005-regulators {
>> +		compatible = "qcom,pm8005-regulators";
>> +
>> +		vdd_s1-supply = <&vph_pwr>;
>> +
>> +		pm8005_s1: s1 { /* VDD_GFX supply */
>> +			regulator-min-microvolt = <524000>;
>> +			regulator-max-microvolt = <1100000>;
>> +			regulator-enable-ramp-delay = <500>;
>> +
>> +			/* hack until we rig up the gpu consumer */
>> +			regulator-always-on;
>> +		};
>> +	};
>> +};
>> +
>> +&qusb2phy {
>> +	status = "okay";
>> +
>> +	vdda-pll-supply = <&vreg_l12a_1p8>;
>> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
>> +};
>> +
>> +&remoteproc_adsp {
>> +	status = "okay";
>> +
>> +	firmware-name = "qcom/pixel2/adsp.mdt";
>> +};
>> +
>> +&remoteproc_mss {
>> +	firmware-name = "qcom/pixel2/mba.mbn",
>> +	                "qcom/pixel2/modem.mdt";
>> +};
>> +
>> +&remoteproc_slpi {
>> +	status = "okay";
>> +
>> +	firmware-name = "qcom/pixel2/slpi_v2.mdt";
>> +};
>> +
>> +&rpm_requests {
>> +	pm8998-regulators {
>> +		compatible = "qcom,rpm-pm8998-regulators";
>> +
>> +		vdd_s1-supply = <&vph_pwr>;
>> +		vdd_s2-supply = <&vph_pwr>;
>> +		vdd_s3-supply = <&vph_pwr>;
>> +		vdd_s4-supply = <&vph_pwr>;
>> +		vdd_s5-supply = <&vph_pwr>;
>> +		vdd_s6-supply = <&vph_pwr>;
>> +		vdd_s7-supply = <&vph_pwr>;
>> +		vdd_s8-supply = <&vph_pwr>;
>> +		vdd_s9-supply = <&vph_pwr>;
>> +		vdd_s10-supply = <&vph_pwr>;
>> +		vdd_s11-supply = <&vph_pwr>;
>> +		vdd_s12-supply = <&vph_pwr>;
>> +		vdd_s13-supply = <&vph_pwr>;
>> +		vdd_l1_l27-supply = <&vreg_s7a_1p025>;
>> +		vdd_l2_l8_l17-supply = <&vreg_s3a_1p35>;
>> +		vdd_l3_l11-supply = <&vreg_s7a_1p025>;
>> +		vdd_l4_l5-supply = <&vreg_s7a_1p025>;
>> +		vdd_l6-supply = <&vreg_s5a_2p04>;
>> +		vdd_l7_l12_l14_l15-supply = <&vreg_s5a_2p04>;
>> +		vdd_l9-supply = <&vreg_bob>;
>> +		vdd_l10_l23_l25-supply = <&vreg_bob>;
>> +		vdd_l13_l19_l21-supply = <&vreg_bob>;
>> +		vdd_l16_l28-supply = <&vreg_bob>;
>> +		vdd_l18_l22-supply = <&vreg_bob>;
>> +		vdd_l20_l24-supply = <&vreg_bob>;
>> +		vdd_l26-supply = <&vreg_s3a_1p35>;
>> +		vdd_lvs1_lvs2-supply = <&vreg_s4a_1p8>;
>> +
>> +		vreg_s3a_1p35: s3 {
>> +			regulator-min-microvolt = <1352000>;
>> +			regulator-max-microvolt = <1352000>;
>> +		};
>> +		vreg_s4a_1p8: s4 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +			regulator-allow-set-load;
>> +		};
>> +		vreg_s5a_2p04: s5 {
>> +			regulator-min-microvolt = <1904000>;
>> +			regulator-max-microvolt = <2040000>;
>> +		};
>> +		vreg_s7a_1p025: s7 {
>> +			regulator-min-microvolt = <900000>;
>> +			regulator-max-microvolt = <1028000>;
>> +		};
>> +		vreg_l1a_0p875: l1 {
>> +			regulator-min-microvolt = <880000>;
>> +			regulator-max-microvolt = <880000>;
>> +		};
>> +		vreg_l2a_1p2: l2 {
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +		};
>> +		vreg_l3a_1p0: l3 {
>> +			regulator-min-microvolt = <1000000>;
>> +			regulator-max-microvolt = <1000000>;
>> +		};
>> +		vreg_l5a_0p8: l5 {
>> +			regulator-min-microvolt = <800000>;
>> +			regulator-max-microvolt = <800000>;
>> +		};
>> +		vreg_l6a_1p8: l6 {
>> +			regulator-min-microvolt = <1808000>;
>> +			regulator-max-microvolt = <1808000>;
>> +		};
>> +		vreg_l7a_1p8: l7 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +		};
>> +		vreg_l8a_1p2: l8 {
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +		};
>> +		vreg_l9a_1p8: l9 {
>> +			regulator-min-microvolt = <1808000>;
>> +			regulator-max-microvolt = <2960000>;
>> +		};
>> +		vreg_l10a_1p8: l10 {
>> +			regulator-min-microvolt = <1808000>;
>> +			regulator-max-microvolt = <2960000>;
>> +		};
>> +		vreg_l11a_1p0: l11 {
>> +			regulator-min-microvolt = <1000000>;
>> +			regulator-max-microvolt = <1000000>;
>> +		};
>> +		vreg_l12a_1p8: l12 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +		};
>> +		vreg_l13a_2p95: l13 {
>> +			regulator-min-microvolt = <1808000>;
>> +			regulator-max-microvolt = <2960000>;
>> +		};
>> +		vreg_l14a_1p88: l14 {
>> +			regulator-min-microvolt = <1880000>;
>> +			regulator-max-microvolt = <1880000>;
>> +		};
>> +		vreg_15a_1p8: l15 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +		};
>> +		vreg_l16a_2p7: l16 {
>> +			regulator-min-microvolt = <2704000>;
>> +			regulator-max-microvolt = <2704000>;
>> +		};
>> +		vreg_l17a_1p3: l17 {
>> +			regulator-min-microvolt = <1304000>;
>> +			regulator-max-microvolt = <1304000>;
>> +		};
>> +		vreg_l18a_2p7: l18 {
>> +			regulator-min-microvolt = <2704000>;
>> +			regulator-max-microvolt = <2704000>;
>> +		};
>> +		vreg_l19a_3p0: l19 {
>> +			regulator-min-microvolt = <3008000>;
>> +			regulator-max-microvolt = <3008000>;
>> +		};
>> +		vreg_l20a_2p95: l20 {
>> +			regulator-min-microvolt = <2960000>;
>> +			regulator-max-microvolt = <2960000>;
>> +			regulator-allow-set-load;
>> +		};
>> +		vreg_l21a_2p95: l21 {
>> +			regulator-min-microvolt = <2960000>;
>> +			regulator-max-microvolt = <2960000>;
>> +			regulator-allow-set-load;
>> +			regulator-system-load = <800000>;
>> +		};
>> +		vreg_l22a_2p85: l22 {
>> +			regulator-min-microvolt = <2864000>;
>> +			regulator-max-microvolt = <2864000>;
>> +		};
>> +		vreg_l23a_3p3: l23 {
>> +			regulator-min-microvolt = <3312000>;
>> +			regulator-max-microvolt = <3312000>;
>> +		};
>> +		vreg_l24a_3p075: l24 {
>> +			regulator-min-microvolt = <3088000>;
>> +			regulator-max-microvolt = <3088000>;
>> +		};
>> +		vreg_l25a_3p3: l25 {
>> +			regulator-min-microvolt = <3104000>;
>> +			regulator-max-microvolt = <3312000>;
>> +		};
>> +		vreg_l26a_1p2: l26 {
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +			regulator-allow-set-load;
>> +		};
>> +		vreg_l28_3p0: l28 {
>> +			regulator-min-microvolt = <3008000>;
>> +			regulator-max-microvolt = <3008000>;
>> +		};
>> +
>> +		vreg_lvs1a_1p8: lvs1 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +		};
>> +
>> +		vreg_lvs2a_1p8: lvs2 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +		};
>> +
>> +	};
>> +
>> +	pmi8998-regulators {
>> +		compatible = "qcom,rpm-pmi8998-regulators";
>> +
>> +		vdd_bob-supply = <&vph_pwr>;
>> +
>> +		vreg_bob: bob {
>> +			regulator-min-microvolt = <3312000>;
>> +			regulator-max-microvolt = <3600000>;
>> +		};
>> +	};
>> +};
>> +
>> +&spmi_bus {
>> +	pmic@0 {
>> +		compatible = "qcom,pm8994", "qcom,spmi-pmic";
> qcom,pm8994?
>
> Per the include of pm8998.dtsi I think you already have pmic@0 defined,
> with compatible of qcom,pm8998 here (which you override).
>
>> +		reg = <0x0 SPMI_USID>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		pon@800 {
> But pmic@0 already has pon@800 defined as &pm8998_pon, which seems to be
> defined identically to yours.
>
> So I think you should be able to drop this entire &spmi_bus node and its
> children, but perhaps I'm just missing something obvious here?
>
>> +			compatible = "qcom,pm8916-pon";
>> +
>> +			reg = <0x800>;
>> +			mode-bootloader = <0x2>;
>> +			mode-recovery = <0x1>;
>> +
>> +			pwrkey {
>> +				compatible = "qcom,pm8941-pwrkey";
>> +				interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
>> +				debounce = <15625>;
>> +				bias-pull-up;
>> +				linux,code = <KEY_POWER>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&tlmm {
>> +	gpio-reserved-ranges = <0 4>, <81 4>;
>> +};
>> +
>> +/*
>> + * The device does contain a USB3 capable type-c port,
>> + * however it doesn't seem to work when superspeed mode is
>> + * enabled.
>> + */
>> +&usb3 {
>> +	status = "okay";
>> +
>> +	/* Operate "GCC_USB30_MASTER_CLK" in HS mode (>=60 MHz) */
>> +	assigned-clock-rates = <19200000>, <60000000>;
>> +
>> +	/* Disable USB3 pipe_clk requirement */
>> +	qcom,select-utmi-as-pipe-clk;
>> +};
>> +
>> +&usb3_dwc3 {
>> +	/* Drop USB 3 SuperSpeed PHY to bring up the "usb0" interface */
>> +	phys = <&qusb2phy>;
>> +	phy-names = "usb2-phy";
>> +
>> +	/* We can only operate at USB 2.0 speeds */
>> +	maximum-speed = "high-speed";
>> +
>> +	/* Force to peripheral until we have Type-C hooked up */
>> +	dr_mode = "peripheral";
>> +};
>> +
>> +&ufshc {
>> +	vcc-supply = <&vreg_l20a_2p95>;
>> +	vccq-supply = <&vreg_l26a_1p2>;
>> +	vccq2-supply = <&vreg_s4a_1p8>;
>> +	vcc-max-microamp = <750000>;
>> +	vccq-max-microamp = <560000>;
>> +	vccq2-max-microamp = <750000>;
>> +};
>> +
>> +&ufsphy {
>> +	vdda-phy-supply = <&vreg_l1a_0p875>;
>> +	vdda-pll-supply = <&vreg_l2a_1p2>;
>> +	vddp-ref-clk-supply = <&vreg_l26a_1p2>;
>> +	vdda-phy-max-microamp = <51400>;
>> +	vdda-pll-max-microamp = <14600>;
>> +	vddp-ref-clk-max-microamp = <100>;
>> +	vddp-ref-clk-always-on;
>> +};
>> +
>> +&wifi {
>> +	status = "okay";
>> +
Thanks.
> Can you please disable diag-router and try adding this here instead:
>
> 	clocks = <&rpmcc RPM_SMD_RF_CLK2_PIN>, <&rpmcc RPM_SMD_QDSS_CLK>;
> 	clock-names = "cxo_ref_clk_pin", "qdss";
>
> What you describe in your comment above seems like what I saw on sm8150,
> that launching diag-router would cause something on the modem side to
> keep the debug subsystem clock on - which in turn caused WiFi not to
> crash during loading.

This unfortunately didn't prevent the following crash:

[   15.553831] qcom-q6v5-mss 4080000.remoteproc: fatal error received: 
err_qdi.c:450:[]:EF:wlan_process:1:halphy_caldb.c:2822:Assertion 0 failed

Incidentally, I've been trying to bring up an SM8150 device and am 
encountering wlan firmware crashes even with diag-router, adding the 
above clocks there doesn't help either on my device:

[   44.367766] qcom_q6v5_pas 4080000.remoteproc: fatal error received: 
err_qdi.c:1024:EX:wlan_process:0x1:WLAN RT:0x1076:PC=0xb0008e20

Thanks for the review.

Regards,

Caleb

>
>
> I don't know why this was required, or if it's just working around
> some other issue we're having.
>
> Regards,
> Bjorn
>
>> +	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
>> +	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
>> +	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
>> +	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
>> +};
>> --
>> 2.29.2
>>
>>



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

* Re: [PATCH] arm64: dts: add support for the Pixel 2 XL
  2021-03-06 10:49 ` Konrad Dybcio
@ 2021-03-06 18:31   ` Caleb Connolly
  0 siblings, 0 replies; 5+ messages in thread
From: Caleb Connolly @ 2021-03-06 18:31 UTC (permalink / raw)
  To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Rob Herring,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck
  Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm,
	devicetree, linux-kernel

Hi Konrad,

On 06/03/2021 10:49 am, Konrad Dybcio wrote:
>
> On 05.03.2021 22:35, Caleb Connolly wrote:
>> Add a minimal devicetree capable of booting on the Pixel 2 XL MSM8998
>> device.
>>
>> It's currently possible to boot the device into postmarketOS with USB
>> networking, however the display panel depends on Display Stream
>> Compression which is not yet supported in the kernel.
>>
>> The bootloader also requires that the dtbo partition contains a device
>> tree overlay with a particular id which has to be overlayed onto the
>> existing dtb. It's possible to use a specially crafted dtbo partition to
>> workaround this, more information is available here:
>>
>>      https://gitlab.com/calebccff/dtbo-google-wahoo-mainline
>>
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>> ---
>> It's possible to get wifi working by running Bjorns diag-router in the
>> background, without this the wifi firmware crashes every 10 seconds or
>> so. This is the same issue encountered on the OnePlus 5.
>>
>>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>>   .../boot/dts/qcom/msm8998-google-taimen.dts   |  14 +
>>   .../boot/dts/qcom/msm8998-google-wahoo.dtsi   | 391 ++++++++++++++++++
>>   3 files changed, 406 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>>   create mode 100644 arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 5113fac80b7a..d942d3ec3928 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-msft-lumia-cityman.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-sony-xperia-kitakami-sumire.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-asus-novago-tp370ql.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-google-taimen.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>> new file mode 100644
>> index 000000000000..ffaaafe14037
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-taimen.dts
>> @@ -0,0 +1,14 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2020, Caleb Connolly <caleb@connolly.tech>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8998-google-wahoo.dtsi"
>> +
>> +/ {
>> +	model = "Google Pixel 2 XL";
>> +	compatible = "google,taimen", "google,wahoo", "qcom,msm8998", "qcom,msm8998-mtp";
> Drop the mtp compatible. Also, afaict wahoo is a shared platform name for P2/2XL, so perhaps using the same naming scheme we used for Xperias/Lumias (soc-vendor-platform-board) would clear up some confusion. In this case, I'm not sure about the wahoo compatible, but I reckon it's fine for it to stay so that it's easier to introduce potential quirks that concern both devices.
>
Yeah, I think it's useful to have a shared compatible for similar devices.
>> +	qcom,msm-id = <0x124 0x20001>;
> Move it to the common dtsi, unless the other Pixel ships with a different SoC revision.
>
>
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>> new file mode 100644
>> index 000000000000..0c221ead2df7
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8998-google-wahoo.dtsi
>> @@ -0,0 +1,391 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2020 Caleb Connolly <caleb@connolly.tech> */
>> +
>> +#include "msm8998.dtsi"
>> +#include "pm8998.dtsi"
>> +#include "pmi8998.dtsi"
>> +#include "pm8005.dtsi"
>> +
>> +/delete-node/ &mpss_mem;
>> +/delete-node/ &venus_mem;
>> +/delete-node/ &mba_mem;
>> +/delete-node/ &slpi_mem;
>> +
>> +/ {
>> +	aliases {
>> +	};
>> +
>> +	chosen {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		/* Add "earlycon" intended to be used in combination with UART serial console */
>> +		bootargs = "clk_ignore_unused earlycon console=ttyGS0,115200";// loglevel=10 drm.debug=15 debug";
> clk_ignore_unused is a BIG hack!
Ah, I haven't touched this device in a while, seems like we don't need 
this anymore on 5.11
>
> You should trace which clocks are important for it to stay alive and fix it on the driver side.
>
> What breaks if it's not there? Does it still happen with Angelo's clk patches that got in for the 5.12
>
> window?
>
> Aside from that, //loglevel... should also go.
>
>
>> +
>> +	vph_pwr: vph-pwr-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vph_pwr";
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +	};
>> +};
> Don't you need to specify voltage here?
This is based on the MTP board, which does not specify a voltage. Do you 
know what the correct voltage is here? I can find references to 
3300000-3500000 uv but I don't know if that's correct. It seems to be 
derived directly from the battery / power voltage, so this seems about 
right.
>
>> +
>> +&blsp1_uart3 {
>> +	status = "disabled";
>> +
>> +	bluetooth {
>> +		compatible = "qcom,wcn3990-bt";
>> +
>> +		vddio-supply = <&vreg_s4a_1p8>;
>> +		vddxo-supply = <&vreg_l7a_1p8>;
>> +		vddrf-supply = <&vreg_l17a_1p3>;
>> +		vddch0-supply = <&vreg_l25a_3p3>;
>> +		max-speed = <3200000>;
>> +	};
>> +};
> Either enable the UART or rid the bluetooth for now.
>
>
>> +
>> +&pm8005_lsid1 {
>> +	pm8005-regulators {
>> +		compatible = "qcom,pm8005-regulators";
>> +
>> +		vdd_s1-supply = <&vph_pwr>;
>> +
>> +		pm8005_s1: s1 { /* VDD_GFX supply */
> regulator-name = "vdd_gfx";
>
>
>> +
>> +&spmi_bus {
>> +	pmic@0 {
>> +		compatible = "qcom,pm8994", "qcom,spmi-pmic";
>> +		reg = <0x0 SPMI_USID>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		pon@800 {
>> +			compatible = "qcom,pm8916-pon";
>> +
>> +			reg = <0x800>;
>> +			mode-bootloader = <0x2>;
>> +			mode-recovery = <0x1>;
>> +
>> +			pwrkey {
>> +				compatible = "qcom,pm8941-pwrkey";
>> +				interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
>> +				debounce = <15625>;
>> +				bias-pull-up;
>> +				linux,code = <KEY_POWER>;
>> +			};
>> +		};
>> +	};
>> +};
> That's a lot of indentation, it would be better to add a label: somewhere instead.. Moreover, the exact same pwrkey node is already present in pm8998.dtsi, so you should just drop this.

Dropped.

Thanks for the review.

Regards,

Caleb

>
> Konrad
>



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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 21:35 [PATCH] arm64: dts: add support for the Pixel 2 XL Caleb Connolly
2021-03-06  3:37 ` Bjorn Andersson
2021-03-06 18:01   ` Caleb Connolly
2021-03-06 10:49 ` Konrad Dybcio
2021-03-06 18:31   ` Caleb Connolly

Phone-Devel Archive on lore.kernel.org.

Archives are clonable:
	git clone --mirror https://lore.kernel.org/phone-devel/0 phone-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 phone-devel phone-devel/ https://lore.kernel.org/phone-devel \
		phone-devel@vger.kernel.org
	public-inbox-index phone-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.phone-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git