LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RESEND PATCH 0/2] arm64: dts: sm8350: add support for Microsoft Surface Duo 2
@ 2021-11-22 19:05 Katherine Perez
  2021-11-22 19:05 ` [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2 Katherine Perez
  2021-11-22 19:05 ` [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address Katherine Perez
  0 siblings, 2 replies; 12+ messages in thread
From: Katherine Perez @ 2021-11-22 19:05 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Vinod Koul
  Cc: linux-arm-msm, devicetree, linux-kernel

Add initial support for the Microsoft Surface Duo 2 based on the
sm8350-mtp DT.

Katherine Perez (2):
  arm64: dts: add minimal DTS for Microsoft Surface Duo2
  arm64: dts: sm8350: fix tlmm base address

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../qcom/sm8350-microsoft-surface-duo2.dts    | 363 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi          |   4 +-
 3 files changed, 366 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts

--
2.31.1


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

* [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2
  2021-11-22 19:05 [RESEND PATCH 0/2] arm64: dts: sm8350: add support for Microsoft Surface Duo 2 Katherine Perez
@ 2021-11-22 19:05 ` Katherine Perez
  2021-11-23  4:00   ` Vinod Koul
  2021-11-22 19:05 ` [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address Katherine Perez
  1 sibling, 1 reply; 12+ messages in thread
From: Katherine Perez @ 2021-11-22 19:05 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Vinod Koul
  Cc: linux-arm-msm, devicetree, linux-kernel

This is a minimal devicetree for Microsoft Surface Duo 2 with SM8350
Chipset

Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
---

Changes since v1:
- Change remoteprocs firmware-naming scheme to qcom/sm8350/microsft/*
- Add chassis-type

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../qcom/sm8350-microsoft-surface-duo2.dts    | 369 ++++++++++++++++++
 2 files changed, 370 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6b816eb33309..a8cc6bd3c423 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -106,4 +106,5 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx203.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx206.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-hdk.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-microsoft-surface-duo2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-mtp.dtb
diff --git a/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
new file mode 100644
index 000000000000..d4963c9015cb
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (C) 2021, Microsoft Corporation
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sm8350.dtsi"
+#include "pm8350.dtsi"
+#include "pm8350b.dtsi"
+#include "pm8350c.dtsi"
+#include "pmk8350.dtsi"
+#include "pmr735a.dtsi"
+#include "pmr735b.dtsi"
+
+/ {
+	model = "Microsoft Surface Duo 2";
+	compatible = "microsoft,surface-duo2", "qcom,sm8350";
+	chassis-type = "handset";
+
+	aliases {
+		serial0 = &uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-min-microvolt = <3700000>;
+		regulator-max-microvolt = <3700000>;
+
+		regulator-always-on;
+		regulator-boot-on;
+	};
+};
+
+&adsp {
+	status = "okay";
+	firmware-name = "qcom/sm8350/microsoft/adsp.mbn";
+};
+
+&apps_rsc {
+	pm8350-rpmh-regulators {
+		compatible = "qcom,pm8350-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		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-l1-l4-supply = <&vreg_s11b_0p95>;
+		vdd-l2-l7-supply = <&vreg_bob>;
+		vdd-l3-l5-supply = <&vreg_bob>;
+		vdd-l6-l9-l10-supply = <&vreg_s11b_0p95>;
+		vdd-l8-supply = <&vreg_s2c_0p8>;
+
+		vreg_s10b_1p8: smps10 {
+			regulator-name = "vreg_s10b_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_s11b_0p95: smps11 {
+			regulator-name = "vreg_s11b_0p95";
+			regulator-min-microvolt = <752000>;
+			regulator-max-microvolt = <1000000>;
+		};
+
+		vreg_s12b_1p25: smps12 {
+			regulator-name = "vreg_s12b_1p25";
+			regulator-min-microvolt = <1224000>;
+			regulator-max-microvolt = <1360000>;
+		};
+
+		vreg_l1b_0p88: ldo1 {
+			regulator-name = "vreg_l1b_0p88";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <920000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2b_3p07: ldo2 {
+			regulator-name = "vreg_l2b_3p07";
+			regulator-min-microvolt = <3072000>;
+			regulator-max-microvolt = <3072000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3b_0p9: ldo3 {
+			regulator-name = "vreg_l3b_0p9";
+			regulator-min-microvolt = <904000>;
+			regulator-max-microvolt = <904000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5b_0p88: ldo5 {
+			regulator-name = "vreg_l3b_0p9";
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <888000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6b_1p2: ldo6 {
+			regulator-name = "vreg_l6b_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1208000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7b_2p96: ldo7 {
+			regulator-name = "vreg_l7b_2p96";
+			regulator-min-microvolt = <2400000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l9b_1p2: ldo9 {
+			regulator-name = "vreg_l9b_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+
+	pm8350c-rpmh-regulators {
+		compatible = "qcom,pm8350c-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		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-l1-l12-supply = <&vreg_s1c_1p86>;
+		vdd-l2-l8-supply = <&vreg_s1c_1p86>;
+		vdd-l3-l4-l5-l7-l13-supply = <&vreg_bob>;
+		vdd-l6-l9-l11-supply = <&vreg_bob>;
+		vdd-l10-supply = <&vreg_s12b_1p25>;
+
+		vdd-bob-supply = <&vph_pwr>;
+
+		vreg_s1c_1p86: smps1 {
+			regulator-name = "vreg_s1c_1p86";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1952000>;
+		};
+
+		vreg_s2c_0p8: smps2 {
+			regulator-name = "vreg_s2c_0p8";
+			regulator-min-microvolt = <640000>;
+			regulator-max-microvolt = <1000000>;
+		};
+
+		vreg_s10c_1p05: smps10 {
+			regulator-name = "vreg_s10c_1p05";
+			regulator-min-microvolt = <1048000>;
+			regulator-max-microvolt = <1128000>;
+		};
+
+		vreg_bob: bob {
+			regulator-name = "vreg_bob";
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+
+		vreg_l1c_1p8: ldo1 {
+			regulator-name = "vreg_l1c_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l2c_1p8: ldo2 {
+			regulator-name = "vreg_l2c_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l3c_3p0: ldo3 {
+			regulator-name = "vreg_l3c_3p0";
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l4c_uim1: ldo4 {
+			regulator-name = "vreg_l4c_uim1";
+			regulator-min-microvolt = <1704000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l5c_uim2: ldo5 {
+			regulator-name = "vreg_l5c_uim2";
+			regulator-min-microvolt = <1704000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l6c_1p8: ldo6 {
+			regulator-name = "vreg_l6c_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l7c_3p0: ldo7 {
+			regulator-name = "vreg_l7c_3p0";
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l8c_1p8: ldo8 {
+			regulator-name = "vreg_l8c_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l9c_2p96: ldo9 {
+			regulator-name = "vreg_l9c_2p96";
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l10c_1p2: ldo10 {
+			regulator-name = "vreg_l10c_1p2";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l11c_2p96: ldo11 {
+			regulator-name = "vreg_l11c_2p96";
+			regulator-min-microvolt = <2400000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l12c_1p8: ldo12 {
+			regulator-name = "vreg_l12c_1p8";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13c_3p0: ldo13 {
+			regulator-name = "vreg_l13c_3p0";
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+	};
+};
+
+&cdsp {
+	status = "okay";
+	firmware-name = "qcom/sm8350/microsoft/cdsp.mbn";
+};
+
+&ipa {
+	status = "okay";
+
+	memory-region = <&pil_ipa_fw_mem>;
+};
+
+&mpss {
+	status = "okay";
+	firmware-name = "qcom/sm8350/microsoft/modem.mbn";
+};
+
+&qupv3_id_0 {
+	status = "okay";
+};
+
+&slpi {
+	status = "okay";
+	firmware-name = "qcom/sm8350/microsoft/slpi.mbn";
+};
+
+&tlmm {
+	gpio-reserved-ranges = <9 8>;
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&ufs_mem_hc {
+	status = "okay";
+
+	reset-gpios = <&tlmm 203 GPIO_ACTIVE_LOW>;
+
+	vcc-supply = <&vreg_l7b_2p96>;
+	vcc-max-microamp = <800000>;
+	vccq-supply = <&vreg_l9b_1p2>;
+	vccq-max-microamp = <900000>;
+};
+
+&ufs_mem_phy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l5b_0p88>;
+	vdda-max-microamp = <91600>;
+	vdda-pll-supply = <&vreg_l6b_1p2>;
+	vdda-pll-max-microamp = <19000>;
+};
+
+&usb_1 {
+	dr_mode = "peripheral";
+};
+
+&usb_1_hsphy {
+	status = "okay";
+
+	vdda-pll-supply = <&vreg_l5b_0p88>;
+	vdda18-supply = <&vreg_l1c_1p8>;
+	vdda33-supply = <&vreg_l2b_3p07>;
+};
+
+&usb_1_qmpphy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l6b_1p2>;
+	vdda-pll-supply = <&vreg_l1b_0p88>;
+};
+
+&usb_2 {
+	status = "okay";
+};
+
+&usb_2_hsphy {
+	status = "okay";
+
+	vdda-pll-supply = <&vreg_l5b_0p88>;
+	vdda18-supply = <&vreg_l1c_1p8>;
+	vdda33-supply = <&vreg_l2b_3p07>;
+};
+
+&usb_2_qmpphy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l6b_1p2>;
+	vdda-pll-supply = <&vreg_l5b_0p88>;
+};
--
2.31.1


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

* [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-11-22 19:05 [RESEND PATCH 0/2] arm64: dts: sm8350: add support for Microsoft Surface Duo 2 Katherine Perez
  2021-11-22 19:05 ` [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2 Katherine Perez
@ 2021-11-22 19:05 ` Katherine Perez
  2021-11-23  4:03   ` Vinod Koul
  1 sibling, 1 reply; 12+ messages in thread
From: Katherine Perez @ 2021-11-22 19:05 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Rob Herring, Vinod Koul
  Cc: linux-arm-msm, devicetree, linux-kernel

TLMM controller base address is incorrect and will hang on some platforms.
Fix by giving the correct address.

Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index d134280e2939..624d294612d8 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -960,9 +960,9 @@ spmi_bus: spmi@c440000 {
 			#interrupt-cells = <4>;
 		};
 
-		tlmm: pinctrl@f100000 {
+		tlmm: pinctrl@f000000 {
 			compatible = "qcom,sm8350-tlmm";
-			reg = <0 0x0f100000 0 0x300000>;
+			reg = <0 0x0f000000 0 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
-- 
2.31.1


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

* Re: [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2
  2021-11-22 19:05 ` [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2 Katherine Perez
@ 2021-11-23  4:00   ` Vinod Koul
  2021-11-30 17:44     ` Katherine Perez
  0 siblings, 1 reply; 12+ messages in thread
From: Vinod Koul @ 2021-11-23  4:00 UTC (permalink / raw)
  To: Katherine Perez, Rob Herring
  Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, devicetree, linux-kernel

On 22-11-21, 11:05, Katherine Perez wrote:
> This is a minimal devicetree for Microsoft Surface Duo 2 with SM8350
> Chipset
> 
> Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
> ---
> 
> Changes since v1:
> - Change remoteprocs firmware-naming scheme to qcom/sm8350/microsft/*
> - Add chassis-type
> 
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../qcom/sm8350-microsoft-surface-duo2.dts    | 369 ++++++++++++++++++
>  2 files changed, 370 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 6b816eb33309..a8cc6bd3c423 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -106,4 +106,5 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx203.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx206.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-hdk.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-microsoft-surface-duo2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-mtp.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> new file mode 100644
> index 000000000000..d4963c9015cb
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> @@ -0,0 +1,369 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (C) 2021, Microsoft Corporation
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include "sm8350.dtsi"
> +#include "pm8350.dtsi"
> +#include "pm8350b.dtsi"
> +#include "pm8350c.dtsi"
> +#include "pmk8350.dtsi"
> +#include "pmr735a.dtsi"
> +#include "pmr735b.dtsi"
> +
> +/ {
> +	model = "Microsoft Surface Duo 2";
> +	compatible = "microsoft,surface-duo2", "qcom,sm8350";
> +	chassis-type = "handset";

This is interesting, I see it used at lot of place, unfortunately, it
does not seem to be documented :(

-- 
~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-11-22 19:05 ` [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address Katherine Perez
@ 2021-11-23  4:03   ` Vinod Koul
  2021-12-01  3:15     ` Bjorn Andersson
  0 siblings, 1 reply; 12+ messages in thread
From: Vinod Koul @ 2021-11-23  4:03 UTC (permalink / raw)
  To: Katherine Perez
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	devicetree, linux-kernel

On 22-11-21, 11:05, Katherine Perez wrote:
> TLMM controller base address is incorrect and will hang on some platforms.
> Fix by giving the correct address.

Thanks, recheck the spec this looks correct. We should have tlmm reg
space here and not tlmm base which also contains xpu region (thus hang)

Reviewed-by: Vinod Koul <vkoul@kernel.org>
Fixes: b7e8f433a673 ("arm64: dts: qcom: Add basic devicetree support for SM8350 SoC")

> 
> Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index d134280e2939..624d294612d8 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -960,9 +960,9 @@ spmi_bus: spmi@c440000 {
>  			#interrupt-cells = <4>;
>  		};
>  
> -		tlmm: pinctrl@f100000 {
> +		tlmm: pinctrl@f000000 {
>  			compatible = "qcom,sm8350-tlmm";
> -			reg = <0 0x0f100000 0 0x300000>;
> +			reg = <0 0x0f000000 0 0x300000>;
>  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
> -- 
> 2.31.1

-- 
~Vinod

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

* Re: [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2
  2021-11-23  4:00   ` Vinod Koul
@ 2021-11-30 17:44     ` Katherine Perez
  0 siblings, 0 replies; 12+ messages in thread
From: Katherine Perez @ 2021-11-30 17:44 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Herring, Andy Gross, Bjorn Andersson, linux-arm-msm,
	devicetree, linux-kernel

On Tue, Nov 23, 2021 at 09:30:01AM +0530, Vinod Koul wrote:
> On 22-11-21, 11:05, Katherine Perez wrote:
> > This is a minimal devicetree for Microsoft Surface Duo 2 with SM8350
> > Chipset
> > 
> > Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
> > ---
> > 
> > Changes since v1:
> > - Change remoteprocs firmware-naming scheme to qcom/sm8350/microsft/*
> > - Add chassis-type
> > 
> >  arch/arm64/boot/dts/qcom/Makefile             |   1 +
> >  .../qcom/sm8350-microsoft-surface-duo2.dts    | 369 ++++++++++++++++++
> >  2 files changed, 370 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 6b816eb33309..a8cc6bd3c423 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -106,4 +106,5 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-mtp.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx203.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= sm8250-sony-xperia-edo-pdx206.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-hdk.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-microsoft-surface-duo2.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= sm8350-mtp.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> > new file mode 100644
> > index 000000000000..d4963c9015cb
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dts
> > @@ -0,0 +1,369 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (C) 2021, Microsoft Corporation
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > +#include "sm8350.dtsi"
> > +#include "pm8350.dtsi"
> > +#include "pm8350b.dtsi"
> > +#include "pm8350c.dtsi"
> > +#include "pmk8350.dtsi"
> > +#include "pmr735a.dtsi"
> > +#include "pmr735b.dtsi"
> > +
> > +/ {
> > +	model = "Microsoft Surface Duo 2";
> > +	compatible = "microsoft,surface-duo2", "qcom,sm8350";
> > +	chassis-type = "handset";
> 
> This is interesting, I see it used at lot of place, unfortunately, it
> does not seem to be documented :(
> 
> -- 
> ~Vinod

Hi Vinod,

Looks like "chassis-type" is documented in the Devicetree Specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter3-devicenodes.html.

-Katherine

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-11-23  4:03   ` Vinod Koul
@ 2021-12-01  3:15     ` Bjorn Andersson
  2021-12-07  1:16       ` Katherine Perez
  2021-12-07 11:46       ` Vinod Koul
  0 siblings, 2 replies; 12+ messages in thread
From: Bjorn Andersson @ 2021-12-01  3:15 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Katherine Perez, Andy Gross, Rob Herring, linux-arm-msm,
	devicetree, linux-kernel

On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:

> On 22-11-21, 11:05, Katherine Perez wrote:
> > TLMM controller base address is incorrect and will hang on some platforms.
> > Fix by giving the correct address.
> 
> Thanks, recheck the spec this looks correct. We should have tlmm reg
> space here and not tlmm base which also contains xpu region (thus hang)
> 

Aren't you reading the patch backwards?

Afaict downstream the driver carries an offset of 0x100000, which we
dropped as we upstreamed the driver. As such changing reg to 0x0f000000
should cause most gpio register accesses to fall outside the actual
register window.

Or perhaps I'm missing something here?

Regards,
Bjorn

> Reviewed-by: Vinod Koul <vkoul@kernel.org>
> Fixes: b7e8f433a673 ("arm64: dts: qcom: Add basic devicetree support for SM8350 SoC")
> 
> > 
> > Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8350.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > index d134280e2939..624d294612d8 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > @@ -960,9 +960,9 @@ spmi_bus: spmi@c440000 {
> >  			#interrupt-cells = <4>;
> >  		};
> >  
> > -		tlmm: pinctrl@f100000 {
> > +		tlmm: pinctrl@f000000 {
> >  			compatible = "qcom,sm8350-tlmm";
> > -			reg = <0 0x0f100000 0 0x300000>;
> > +			reg = <0 0x0f000000 0 0x300000>;
> >  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> >  			gpio-controller;
> >  			#gpio-cells = <2>;
> > -- 
> > 2.31.1
> 
> -- 
> ~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-12-01  3:15     ` Bjorn Andersson
@ 2021-12-07  1:16       ` Katherine Perez
  2021-12-07 11:46       ` Vinod Koul
  1 sibling, 0 replies; 12+ messages in thread
From: Katherine Perez @ 2021-12-07  1:16 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Bjorn Andersson, Andy Gross, Rob Herring, linux-arm-msm,
	devicetree, linux-kernel

On Tue, Nov 30, 2021 at 09:15:29PM -0600, Bjorn Andersson wrote:
> On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:
> 
> > On 22-11-21, 11:05, Katherine Perez wrote:
> > > TLMM controller base address is incorrect and will hang on some platforms.
> > > Fix by giving the correct address.
> > 
> > Thanks, recheck the spec this looks correct. We should have tlmm reg
> > space here and not tlmm base which also contains xpu region (thus hang)
> > 
> 
> Aren't you reading the patch backwards?
> 
> Afaict downstream the driver carries an offset of 0x100000, which we
> dropped as we upstreamed the driver. As such changing reg to 0x0f000000
> should cause most gpio register accesses to fall outside the actual
> register window.
> 
> Or perhaps I'm missing something here?
> 
> Regards,
> Bjorn

Hi Vinod,

Gentle reminder about the response above. Without the change to the TLMM
base address, my platform hangs. I have ensured that the pinctrl driver
contains the patch that Bjorn has previously submitted here:
https://lore.kernel.org/all/20211104170835.1993686-1-bjorn.andersson@linaro.org/

Best,
Katherine

> 
> > Reviewed-by: Vinod Koul <vkoul@kernel.org>
> > Fixes: b7e8f433a673 ("arm64: dts: qcom: Add basic devicetree support for SM8350 SoC")
> > 
> > > 
> > > Signed-off-by: Katherine Perez <kaperez@linux.microsoft.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm8350.dtsi | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > > index d134280e2939..624d294612d8 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> > > @@ -960,9 +960,9 @@ spmi_bus: spmi@c440000 {
> > >  			#interrupt-cells = <4>;
> > >  		};
> > >  
> > > -		tlmm: pinctrl@f100000 {
> > > +		tlmm: pinctrl@f000000 {
> > >  			compatible = "qcom,sm8350-tlmm";
> > > -			reg = <0 0x0f100000 0 0x300000>;
> > > +			reg = <0 0x0f000000 0 0x300000>;
> > >  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> > >  			gpio-controller;
> > >  			#gpio-cells = <2>;
> > > -- 
> > > 2.31.1
> > 
> > -- 
> > ~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-12-01  3:15     ` Bjorn Andersson
  2021-12-07  1:16       ` Katherine Perez
@ 2021-12-07 11:46       ` Vinod Koul
  2021-12-08  2:21         ` Katherine Perez
  1 sibling, 1 reply; 12+ messages in thread
From: Vinod Koul @ 2021-12-07 11:46 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Katherine Perez, Andy Gross, Rob Herring, linux-arm-msm,
	devicetree, linux-kernel

On 30-11-21, 21:15, Bjorn Andersson wrote:
> On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:
> 
> > On 22-11-21, 11:05, Katherine Perez wrote:
> > > TLMM controller base address is incorrect and will hang on some platforms.
> > > Fix by giving the correct address.
> > 
> > Thanks, recheck the spec this looks correct. We should have tlmm reg
> > space here and not tlmm base which also contains xpu region (thus hang)
> > 
> 
> Aren't you reading the patch backwards?

I guess :(

> Afaict downstream the driver carries an offset of 0x100000, which we
> dropped as we upstreamed the driver. As such changing reg to 0x0f000000
> should cause most gpio register accesses to fall outside the actual
> register window.
> 
> Or perhaps I'm missing something here?

I relooked and XPU is at 0xF000000 and Reg at 0xF100000
So this patch should be dropped as such. The size mentioned in
documentation is also correct

Katherine, can you elaborate more on the hang you have observed? Any
specific pins you use which causes this?


-- 
~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-12-07 11:46       ` Vinod Koul
@ 2021-12-08  2:21         ` Katherine Perez
  2021-12-08  2:35           ` Bjorn Andersson
  0 siblings, 1 reply; 12+ messages in thread
From: Katherine Perez @ 2021-12-08  2:21 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Gross, Bjorn Andersson, Rob Herring, Felipe Balbi,
	linux-arm-msm, devicetree, linux-kernel

On Tue, Dec 07, 2021 at 05:16:14PM +0530, Vinod Koul wrote:
> On 30-11-21, 21:15, Bjorn Andersson wrote:
> > On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:
> > 
> > > On 22-11-21, 11:05, Katherine Perez wrote:
> > > > TLMM controller base address is incorrect and will hang on some platforms.
> > > > Fix by giving the correct address.
> > > 
> > > Thanks, recheck the spec this looks correct. We should have tlmm reg
> > > space here and not tlmm base which also contains xpu region (thus hang)
> > > 
> > 
> > Aren't you reading the patch backwards?
> 
> I guess :(
> 
> > Afaict downstream the driver carries an offset of 0x100000, which we
> > dropped as we upstreamed the driver. As such changing reg to 0x0f000000
> > should cause most gpio register accesses to fall outside the actual
> > register window.
> > 
> > Or perhaps I'm missing something here?
> 
> I relooked and XPU is at 0xF000000 and Reg at 0xF100000
> So this patch should be dropped as such. The size mentioned in
> documentation is also correct
> 
> Katherine, can you elaborate more on the hang you have observed? Any
> specific pins you use which causes this?

Hi Vinod,

Yes, it seems to hang in msm_pinctrl_probe. Specifically, line 734 in
gpiolib.c: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c#n734.
On i=4, it hangs on assign_bit and the system goes into a reboot loop.
When I set the TLMM address to f000000, I don't see this issue at all.

> 
> 
> -- 
> ~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-12-08  2:21         ` Katherine Perez
@ 2021-12-08  2:35           ` Bjorn Andersson
  2021-12-08 23:18             ` Katherine Perez
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2021-12-08  2:35 UTC (permalink / raw)
  To: Katherine Perez
  Cc: Vinod Koul, Andy Gross, Rob Herring, Felipe Balbi, linux-arm-msm,
	devicetree, linux-kernel

On Tue 07 Dec 18:21 PST 2021, Katherine Perez wrote:

> On Tue, Dec 07, 2021 at 05:16:14PM +0530, Vinod Koul wrote:
> > On 30-11-21, 21:15, Bjorn Andersson wrote:
> > > On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:
> > > 
> > > > On 22-11-21, 11:05, Katherine Perez wrote:
> > > > > TLMM controller base address is incorrect and will hang on some platforms.
> > > > > Fix by giving the correct address.
> > > > 
> > > > Thanks, recheck the spec this looks correct. We should have tlmm reg
> > > > space here and not tlmm base which also contains xpu region (thus hang)
> > > > 
> > > 
> > > Aren't you reading the patch backwards?
> > 
> > I guess :(
> > 
> > > Afaict downstream the driver carries an offset of 0x100000, which we
> > > dropped as we upstreamed the driver. As such changing reg to 0x0f000000
> > > should cause most gpio register accesses to fall outside the actual
> > > register window.
> > > 
> > > Or perhaps I'm missing something here?
> > 
> > I relooked and XPU is at 0xF000000 and Reg at 0xF100000
> > So this patch should be dropped as such. The size mentioned in
> > documentation is also correct
> > 
> > Katherine, can you elaborate more on the hang you have observed? Any
> > specific pins you use which causes this?
> 
> Hi Vinod,
> 
> Yes, it seems to hang in msm_pinctrl_probe. Specifically, line 734 in
> gpiolib.c: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c#n734.
> On i=4, it hangs on assign_bit and the system goes into a reboot loop.
> When I set the TLMM address to f000000, I don't see this issue at all.
> 

The cause for that is quite likely that gc->get_direction() will read
the configuration from gpio<i>'s registers and gpio4 in your system is
reserved for use by some trusted application.

When you change the TLMM address you avoid this problem by just reading
random registers outside the region that contains protected registers.


Adjust the gpio-reserved-ranges in your device's tlmm node to mark gpio4
(probably 4 pins long) as "invalid", gpiolib will then not touch them.

Regards,
Bjorn

> > 
> > 
> > -- 
> > ~Vinod

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

* Re: [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address
  2021-12-08  2:35           ` Bjorn Andersson
@ 2021-12-08 23:18             ` Katherine Perez
  0 siblings, 0 replies; 12+ messages in thread
From: Katherine Perez @ 2021-12-08 23:18 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Vinod Koul, Andy Gross, Rob Herring, linux-arm-msm, devicetree,
	linux-kernel, Felipe Balbi

On Tue, Dec 07, 2021 at 06:35:41PM -0800, Bjorn Andersson wrote:
> On Tue 07 Dec 18:21 PST 2021, Katherine Perez wrote:
> 
> > On Tue, Dec 07, 2021 at 05:16:14PM +0530, Vinod Koul wrote:
> > > On 30-11-21, 21:15, Bjorn Andersson wrote:
> > > > On Mon 22 Nov 22:03 CST 2021, Vinod Koul wrote:
> > > > 
> > > > > On 22-11-21, 11:05, Katherine Perez wrote:
> > > > > > TLMM controller base address is incorrect and will hang on some platforms.
> > > > > > Fix by giving the correct address.
> > > > > 
> > > > > Thanks, recheck the spec this looks correct. We should have tlmm reg
> > > > > space here and not tlmm base which also contains xpu region (thus hang)
> > > > > 
> > > > 
> > > > Aren't you reading the patch backwards?
> > > 
> > > I guess :(
> > > 
> > > > Afaict downstream the driver carries an offset of 0x100000, which we
> > > > dropped as we upstreamed the driver. As such changing reg to 0x0f000000
> > > > should cause most gpio register accesses to fall outside the actual
> > > > register window.
> > > > 
> > > > Or perhaps I'm missing something here?
> > > 
> > > I relooked and XPU is at 0xF000000 and Reg at 0xF100000
> > > So this patch should be dropped as such. The size mentioned in
> > > documentation is also correct
> > > 
> > > Katherine, can you elaborate more on the hang you have observed? Any
> > > specific pins you use which causes this?
> > 
> > Hi Vinod,
> > 
> > Yes, it seems to hang in msm_pinctrl_probe. Specifically, line 734 in
> > gpiolib.c: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c#n734.
> > On i=4, it hangs on assign_bit and the system goes into a reboot loop.
> > When I set the TLMM address to f000000, I don't see this issue at all.
> > 
> 
> The cause for that is quite likely that gc->get_direction() will read
> the configuration from gpio<i>'s registers and gpio4 in your system is
> reserved for use by some trusted application.
> 
> When you change the TLMM address you avoid this problem by just reading
> random registers outside the region that contains protected registers.
> 
> 
> Adjust the gpio-reserved-ranges in your device's tlmm node to mark gpio4
> (probably 4 pins long) as "invalid", gpiolib will then not touch them.
> 
> Regards,
> Bjorn

Thanks, Bjorn. That makes sense. I'll resubmit with the changes to my
device's TLMM node and will drop this patch.

-Katherine
> 
> > > 
> > > 
> > > -- 
> > > ~Vinod

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

end of thread, other threads:[~2021-12-08 23:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 19:05 [RESEND PATCH 0/2] arm64: dts: sm8350: add support for Microsoft Surface Duo 2 Katherine Perez
2021-11-22 19:05 ` [PATCH v2 1/2] arm64: dts: add minimal DTS for Microsoft Surface Duo2 Katherine Perez
2021-11-23  4:00   ` Vinod Koul
2021-11-30 17:44     ` Katherine Perez
2021-11-22 19:05 ` [PATCH v2 2/2] arm64: dts: sm8350: fix tlmm base address Katherine Perez
2021-11-23  4:03   ` Vinod Koul
2021-12-01  3:15     ` Bjorn Andersson
2021-12-07  1:16       ` Katherine Perez
2021-12-07 11:46       ` Vinod Koul
2021-12-08  2:21         ` Katherine Perez
2021-12-08  2:35           ` Bjorn Andersson
2021-12-08 23:18             ` Katherine Perez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).