[PATCH 0/4] arm64: dts: qcom: Add support for the sc7280 CRD board

[PATCH 0/4] arm64: dts: qcom: Add support for the sc7280 CRD board

[Rajendra Nayak]

Add support for sc7280 CRD (Compute Reference Design) Board.
It shares the same EC and H1 as the IDP2 boards, comes with an eDP
display and supports both touch and trackpad.
Since the EC and H1 nodes are identical across CRD and IDP2 this
series also adds support for EC/H1 on IDP2 devices.

Kshitiz Godara (2):
  arm64: dts: qcom: sc7280: Define EC and H1 nodes
  arm64: dts: qcom: sc7280-crd: Add Touchscreen and touchpad support

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

 Documentation/devicetree/bindings/arm/qcom.yaml |   2 +
 arch/arm64/boot/dts/qcom/Makefile               |   1 +
 arch/arm64/boot/dts/qcom/sc7280-crd.dts         |  95 ++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi      | 110 ++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts        |   1 +
 5 files changed, 209 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-crd.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi

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

[PATCH 1/4] dt-bindings: arm: qcom: Document qcom,sc7280-crd board

[Rajendra Nayak]

Document the qcom,sc7280-crd board based off sc7280 SoC,
The board is also known as hoglin in the Chrome OS builds,
and given there would be multiple (at least one more) rev
of this board document the google,hoglin-rev0 compatible as well.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index c8808e0..2abfd28 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -203,6 +203,8 @@ properties:
           - enum:
               - qcom,sc7280-idp
               - qcom,sc7280-idp2
+              - qcom,sc7280-crd
+              - google,hoglin-rev0
               - google,piglin
               - google,senor
           - const: qcom,sc7280
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/4] dt-bindings: arm: qcom: Document qcom,sc7280-crd board

[Matthias Kaehlcke]

On Tue, Nov 23, 2021 at 12:30:10PM +0530, Rajendra Nayak wrote:
> Document the qcom,sc7280-crd board based off sc7280 SoC,
> The board is also known as hoglin in the Chrome OS builds,
> and given there would be multiple (at least one more) rev
> of this board document the google,hoglin-rev0 compatible as well.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index c8808e0..2abfd28 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -203,6 +203,8 @@ properties:
>            - enum:
>                - qcom,sc7280-idp
>                - qcom,sc7280-idp2
> +              - qcom,sc7280-crd
> +              - google,hoglin-rev0

I think we also want the generic 'google,hoglin' compatible string, analogous to
for example 'google,lazor' and 'google,lazor-revN'. For lazor there are no
explicit compatible entries for rev3 and above, there were no DT relevant
hardware changes for rev > 3, hence the 'google,lazor' compatible string is
used, without the need to modify the DT for each new HW revision.

Also on my CRD the bootloader thinks it is running on a rev4:

  Compat preference: google,hoglin-rev4 google,hoglin

The board still boots thanks to the 'google,hoglin' entry in my device tree,
but it seems you need to add more revN entries, or start with rev4 if you
don't really care about supporting older revisions. In the later case you
coul only have 'google,hoglin' for now, and add 'rev4' when you add support
for the next revision (supposing it has DT relevant hardware changes). The
sc7180-trogdor boards can serve as an example on how to deal with board
revisions.

Re: [PATCH 1/4] dt-bindings: arm: qcom: Document qcom,sc7280-crd board

[Rajendra Nayak]

On 11/23/2021 8:23 PM, Matthias Kaehlcke wrote:
> On Tue, Nov 23, 2021 at 12:30:10PM +0530, Rajendra Nayak wrote:
>> Document the qcom,sc7280-crd board based off sc7280 SoC,
>> The board is also known as hoglin in the Chrome OS builds,
>> and given there would be multiple (at least one more) rev
>> of this board document the google,hoglin-rev0 compatible as well.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/arm/qcom.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>> index c8808e0..2abfd28 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>> @@ -203,6 +203,8 @@ properties:
>>             - enum:
>>                 - qcom,sc7280-idp
>>                 - qcom,sc7280-idp2
>> +              - qcom,sc7280-crd
>> +              - google,hoglin-rev0
> 
> I think we also want the generic 'google,hoglin' compatible string, analogous to
> for example 'google,lazor' and 'google,lazor-revN'. For lazor there are no
> explicit compatible entries for rev3 and above, there were no DT relevant
> hardware changes for rev > 3, hence the 'google,lazor' compatible string is
> used, without the need to modify the DT for each new HW revision.
> 
> Also on my CRD the bootloader thinks it is running on a rev4:
> 
>    Compat preference: google,hoglin-rev4 google,hoglin
> 
> The board still boots thanks to the 'google,hoglin' entry in my device tree,
> but it seems you need to add more revN entries, or start with rev4 if you
> don't really care about supporting older revisions. In the later case you
> coul only have 'google,hoglin' for now, and add 'rev4' when you add support
> for the next revision (supposing it has DT relevant hardware changes). The
> sc7180-trogdor boards can serve as an example on how to deal with board
> revisions.

Sure, i think i will perhaps go with just the google,hoglin compatible
for now and add revs as and when needed, the fact that these boards have
a few initial revs not used at all seem to make it really confusing

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

[PATCH 2/4] arm64: dts: qcom: sc7280-crd: Add device tree files for CRD

[Rajendra Nayak]

CRD (Compute Reference Design) is a sc7280 based board, largely
derived from the existing IDP board design with some key deltas
1. has EC and H1 over SPI similar to IDP2
2. touchscreen and trackpad support
3. eDP display

We just add the barebones dts file here, subsequent patches will
add support for EC/H1 and other components.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/Makefile       |  1 +
 arch/arm64/boot/dts/qcom/sc7280-crd.dts | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-crd.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6b816eb..b18708c 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -78,6 +78,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-herobrine.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-crd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
new file mode 100644
index 0000000..09d02c2
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * sc7280 CRD board device tree source
+ *
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sc7280-idp.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
+	compatible = "qcom,sc7280-crd", "google,hoglin-rev0", "qcom,sc7280";
+
+	aliases {
+		serial0 = &uart5;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&nvme_pwren {
+	pins = "gpio51";
+};
+
+&nvme_3v3_regulator {
+	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
+};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 2/4] arm64: dts: qcom: sc7280-crd: Add device tree files for CRD

[Matthias Kaehlcke]

On Tue, Nov 23, 2021 at 12:30:11PM +0530, Rajendra Nayak wrote:
> CRD (Compute Reference Design) is a sc7280 based board, largely
> derived from the existing IDP board design with some key deltas
> 1. has EC and H1 over SPI similar to IDP2
> 2. touchscreen and trackpad support
> 3. eDP display
> 
> We just add the barebones dts file here, subsequent patches will
> add support for EC/H1 and other components.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/Makefile       |  1 +
>  arch/arm64/boot/dts/qcom/sc7280-crd.dts | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7280-crd.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 6b816eb..b18708c 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -78,6 +78,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-herobrine.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-crd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> new file mode 100644
> index 0000000..09d02c2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * sc7280 CRD board device tree source
> + *
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7280-idp.dtsi"
> +
> +/ {
> +	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
> +	compatible = "qcom,sc7280-crd", "google,hoglin-rev0", "qcom,sc7280";

As per my comment on the binding there should also be a "google,hoglin"
without a revision suffix, also it seems there are already CRDs with higher
rev numbers.

> +
> +	aliases {
> +		serial0 = &uart5;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +&nvme_pwren {
> +	pins = "gpio51";
> +};
> +
> +&nvme_3v3_regulator {
> +	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +};

uber-nit: 'nvme_3v3_regulator' should be before 'nvme_pwren', assuming
alphabetical/ASCII ordering is used.

Re: [PATCH 2/4] arm64: dts: qcom: sc7280-crd: Add device tree files for CRD

[Rajendra Nayak]

On 11/23/2021 9:44 PM, Matthias Kaehlcke wrote:
> On Tue, Nov 23, 2021 at 12:30:11PM +0530, Rajendra Nayak wrote:
>> CRD (Compute Reference Design) is a sc7280 based board, largely
>> derived from the existing IDP board design with some key deltas
>> 1. has EC and H1 over SPI similar to IDP2
>> 2. touchscreen and trackpad support
>> 3. eDP display
>>
>> We just add the barebones dts file here, subsequent patches will
>> add support for EC/H1 and other components.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/Makefile       |  1 +
>>   arch/arm64/boot/dts/qcom/sc7280-crd.dts | 31 +++++++++++++++++++++++++++++++
>>   2 files changed, 32 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/sc7280-crd.dts
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 6b816eb..b18708c 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -78,6 +78,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-herobrine.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-idp2.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= sc7280-crd.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-discovery.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-nile-pioneer.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> new file mode 100644
>> index 0000000..09d02c2
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> @@ -0,0 +1,31 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * sc7280 CRD board device tree source
>> + *
>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "sc7280-idp.dtsi"
>> +
>> +/ {
>> +	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
>> +	compatible = "qcom,sc7280-crd", "google,hoglin-rev0", "qcom,sc7280";
> 
> As per my comment on the binding there should also be a "google,hoglin"
> without a revision suffix, also it seems there are already CRDs with higher
> rev numbers.

right i will remove the -revs for now.

> 
>> +
>> +	aliases {
>> +		serial0 = &uart5;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +};
>> +
>> +&nvme_pwren {
>> +	pins = "gpio51";
>> +};
>> +
>> +&nvme_3v3_regulator {
>> +	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>> +};
> 
> uber-nit: 'nvme_3v3_regulator' should be before 'nvme_pwren', assuming
> alphabetical/ASCII ordering is used.

thanks will fix
> 

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

[PATCH 3/4] arm64: dts: qcom: sc7280: Define EC and H1 nodes

[Rajendra Nayak]

From: Kshitiz Godara <kgodara@codeaurora.org>

The IDP2 and CRD boards share the EC and H1 parts, so define
all related device nodes into a common file and include them
in the idp2 and crd dts files to avoid duplication.

Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-crd.dts    |   1 +
 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi | 110 +++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp2.dts   |   1 +
 3 files changed, 112 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index 09d02c2..8c2aee6 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -8,6 +8,7 @@
 /dts-v1/;
 
 #include "sc7280-idp.dtsi"
+#include "sc7280-ec-h1.dtsi"
 
 / {
 	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
diff --git a/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
new file mode 100644
index 0000000..78fb5eb
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * sc7280 EC/H1 over SPI (common between IDP2 and CRD)
+ *
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+ap_ec_spi: &spi10 {
+	status = "okay";
+
+	pinctrl-0 = <&qup_spi10_cs_gpio_init_high>, <&qup_spi10_cs_gpio>;
+	cs-gpios = <&tlmm 43 GPIO_ACTIVE_LOW>;
+
+	cros_ec: ec@0 {
+		compatible = "google,cros-ec-spi";
+		reg = <0>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ap_ec_int_l>;
+		spi-max-frequency = <3000000>;
+
+		cros_ec_pwm: ec-pwm {
+			compatible = "google,cros-ec-pwm";
+			#pwm-cells = <1>;
+		};
+
+		i2c_tunnel: i2c-tunnel {
+			compatible = "google,cros-ec-i2c-tunnel";
+			google,remote-bus = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		typec {
+			compatible = "google,cros-ec-typec";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			usb_c0: connector@0 {
+				compatible = "usb-c-connector";
+				reg = <0>;
+				label = "left";
+				power-role = "dual";
+				data-role = "host";
+				try-power-role = "source";
+			};
+
+			usb_c1: connector@1 {
+				compatible = "usb-c-connector";
+				reg = <1>;
+				label = "right";
+				power-role = "dual";
+				data-role = "host";
+				try-power-role = "source";
+			};
+		};
+	};
+};
+
+#include <arm/cros-ec-keyboard.dtsi>
+#include <arm/cros-ec-sbs.dtsi>
+
+ap_h1_spi: &spi14 {
+	status = "okay";
+
+	pinctrl-0 = <&qup_spi14_cs_gpio_init_high>, <&qup_spi14_cs_gpio>;
+	cs-gpios = <&tlmm 59 GPIO_ACTIVE_LOW>;
+
+	cr50: tpm@0 {
+		compatible = "google,cr50";
+		reg = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&h1_ap_int_odl>;
+		spi-max-frequency = <800000>;
+		interrupt-parent = <&tlmm>;
+		interrupts = <104 IRQ_TYPE_EDGE_RISING>;
+	};
+};
+
+&tlmm {
+	ap_ec_int_l: ap-ec-int-l {
+		pins = "gpio18";
+		function = "gpio";
+		input-enable;
+		bias-pull-up;
+		drive-strength = <2>;
+	};
+
+	h1_ap_int_odl: h1-ap-int-odl {
+		pins = "gpio104";
+		function = "gpio";
+		input-enable;
+		bias-pull-up;
+		drive-strength = <2>;
+	};
+
+	qup_spi10_cs_gpio_init_high: qup-spi10-cs-gpio-init-high {
+		pins = "gpio43";
+		output-high;
+		drive-strength = <2>;
+	};
+
+	qup_spi14_cs_gpio_init_high: qup-spi14-cs-gpio-init-high {
+		pins = "gpio59";
+		output-high;
+		drive-strength = <2>;
+	};
+};
+
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
index 3ae9969..208ca69 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
@@ -8,6 +8,7 @@
 /dts-v1/;
 
 #include "sc7280-idp.dtsi"
+#include "sc7280-ec-h1.dtsi"
 
 / {
 	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 3/4] arm64: dts: qcom: sc7280: Define EC and H1 nodes

[Matthias Kaehlcke]

On Tue, Nov 23, 2021 at 12:30:12PM +0530, Rajendra Nayak wrote:

> Subject: arm64: dts: qcom: sc7280: Define EC and H1 nodes

that seems to suggest that EC and H1 nodes are something generic of the
sc7280, however these two chips are only present on systems that target
Chrome OS, and the specific nodes are added are only used by the QCA
sc7280 IDP and CRD, not other sc7280 boards using Chrome OS, like
herobrine. I suggest to change it to "arm64: dts: qcom: sc7280: Define
EC and H1 nodes for IDP/CRD".

> From: Kshitiz Godara <kgodara@codeaurora.org>
> 
> The IDP2 and CRD boards share the EC and H1 parts, so define
> all related device nodes into a common file and include them
> in the idp2 and crd dts files to avoid duplication.
> 
> Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-crd.dts    |   1 +
>  arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi | 110 +++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280-idp2.dts   |   1 +
>  3 files changed, 112 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index 09d02c2..8c2aee6 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -8,6 +8,7 @@
>  /dts-v1/;
>  
>  #include "sc7280-idp.dtsi"
> +#include "sc7280-ec-h1.dtsi"
>  
>  / {
>  	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
> new file mode 100644
> index 0000000..78fb5eb
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi

Similar comment as for the subject, the file name seems to imply
that the include could be useful for any board with an EC and H1,
however it will be only used by the IDP and CRD. Maybe name it
'sc7280-idp-ec-h1.dtsi', from the CRD DT file it is alreay clear
that it is related with the IDP, so it shouldn't be too confusing
that the file name only says IDP.

Also a birdie told me that the EC and H1 configuration is going to
change in future revisions of the CRD, which is another reason for
being more specific with the file name (a sc7280-crd-ec-h1.dtsi
might be needed at that point, or the new not-any-longer-shared
config goes directly into the sc7280-crd-revN.dts.

> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * sc7280 EC/H1 over SPI (common between IDP2 and CRD)
> + *
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +ap_ec_spi: &spi10 {
> +	status = "okay";
> +
> +	pinctrl-0 = <&qup_spi10_cs_gpio_init_high>, <&qup_spi10_cs_gpio>;

Shouldn't this also have <&qup_spi10_data_clk>?

> +	cs-gpios = <&tlmm 43 GPIO_ACTIVE_LOW>;
> +
> +	cros_ec: ec@0 {
> +		compatible = "google,cros-ec-spi";
> +		reg = <0>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&ap_ec_int_l>;
> +		spi-max-frequency = <3000000>;
> +
> +		cros_ec_pwm: ec-pwm {
> +			compatible = "google,cros-ec-pwm";
> +			#pwm-cells = <1>;
> +		};
> +
> +		i2c_tunnel: i2c-tunnel {
> +			compatible = "google,cros-ec-i2c-tunnel";
> +			google,remote-bus = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};
> +
> +		typec {
> +			compatible = "google,cros-ec-typec";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			usb_c0: connector@0 {
> +				compatible = "usb-c-connector";
> +				reg = <0>;
> +				label = "left";
> +				power-role = "dual";
> +				data-role = "host";
> +				try-power-role = "source";
> +			};
> +
> +			usb_c1: connector@1 {
> +				compatible = "usb-c-connector";
> +				reg = <1>;
> +				label = "right";
> +				power-role = "dual";
> +				data-role = "host";
> +				try-power-role = "source";
> +			};
> +		};
> +	};
> +};
> +
> +#include <arm/cros-ec-keyboard.dtsi>
> +#include <arm/cros-ec-sbs.dtsi>
> +
> +ap_h1_spi: &spi14 {
> +	status = "okay";
> +
> +	pinctrl-0 = <&qup_spi14_cs_gpio_init_high>, <&qup_spi14_cs_gpio>;

<&qup_spi14_data_clk> missing?

> +	cs-gpios = <&tlmm 59 GPIO_ACTIVE_LOW>;
> +
> +	cr50: tpm@0 {
> +		compatible = "google,cr50";
> +		reg = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&h1_ap_int_odl>;
> +		spi-max-frequency = <800000>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <104 IRQ_TYPE_EDGE_RISING>;
> +	};
> +};
> +
> +&tlmm {
> +	ap_ec_int_l: ap-ec-int-l {
> +		pins = "gpio18";
> +		function = "gpio";
> +		input-enable;
> +		bias-pull-up;
> +		drive-strength = <2>;

Is the explicit drive-strength setting actually needed?

Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml:

  drive-strength:
    enum: [2, 4, 6, 8, 10, 12, 14, 16]
    default: 2 <=
    description:
      Selects the drive strength for the specified pins, in mA.

The default is 2, hence it shouldn't be necessary it set it explicitly.

> +	};
> +
> +	h1_ap_int_odl: h1-ap-int-odl {
> +		pins = "gpio104";
> +		function = "gpio";
> +		input-enable;
> +		bias-pull-up;
> +		drive-strength = <2>;

see above

> +	};
> +
> +	qup_spi10_cs_gpio_init_high: qup-spi10-cs-gpio-init-high {
> +		pins = "gpio43";
> +		output-high;
> +		drive-strength = <2>;

see above

> +	};
> +
> +	qup_spi14_cs_gpio_init_high: qup-spi14-cs-gpio-init-high {
> +		pins = "gpio59";
> +		output-high;
> +		drive-strength = <2>;

see above

> +	};
> +};
> +
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> index 3ae9969..208ca69 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
> @@ -8,6 +8,7 @@
>  /dts-v1/;
>  
>  #include "sc7280-idp.dtsi"
> +#include "sc7280-ec-h1.dtsi"
>  
>  / {
>  	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";

Re: [PATCH 3/4] arm64: dts: qcom: sc7280: Define EC and H1 nodes

[Rajendra Nayak]

On 11/23/2021 11:10 PM, Matthias Kaehlcke wrote:
> On Tue, Nov 23, 2021 at 12:30:12PM +0530, Rajendra Nayak wrote:
> 
>> Subject: arm64: dts: qcom: sc7280: Define EC and H1 nodes
> 
> that seems to suggest that EC and H1 nodes are something generic of the
> sc7280, however these two chips are only present on systems that target
> Chrome OS, and the specific nodes are added are only used by the QCA
> sc7280 IDP and CRD, not other sc7280 boards using Chrome OS, like
> herobrine. I suggest to change it to "arm64: dts: qcom: sc7280: Define
> EC and H1 nodes for IDP/CRD".

sure makes sense

> 
>> From: Kshitiz Godara <kgodara@codeaurora.org>
>>
>> The IDP2 and CRD boards share the EC and H1 parts, so define
>> all related device nodes into a common file and include them
>> in the idp2 and crd dts files to avoid duplication.
>>
>> Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-crd.dts    |   1 +
>>   arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi | 110 +++++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280-idp2.dts   |   1 +
>>   3 files changed, 112 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index 09d02c2..8c2aee6 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> @@ -8,6 +8,7 @@
>>   /dts-v1/;
>>   
>>   #include "sc7280-idp.dtsi"
>> +#include "sc7280-ec-h1.dtsi"
>>   
>>   / {
>>   	model = "Qualcomm Technologies, Inc. sc7280 CRD platform";
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
>> new file mode 100644
>> index 0000000..78fb5eb
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-ec-h1.dtsi
> 
> Similar comment as for the subject, the file name seems to imply
> that the include could be useful for any board with an EC and H1,
> however it will be only used by the IDP and CRD. Maybe name it
> 'sc7280-idp-ec-h1.dtsi', from the CRD DT file it is alreay clear
> that it is related with the IDP, so it shouldn't be too confusing
> that the file name only says IDP.
> 
> Also a birdie told me that the EC and H1 configuration is going to
> change in future revisions of the CRD, which is another reason for
> being more specific with the file name (a sc7280-crd-ec-h1.dtsi
> might be needed at that point, or the new not-any-longer-shared
> config goes directly into the sc7280-crd-revN.dts.

right I will rename it to sc7280-idp-ec-h1.dtsi

> 
>> @@ -0,0 +1,110 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * sc7280 EC/H1 over SPI (common between IDP2 and CRD)
>> + *
>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +ap_ec_spi: &spi10 {
>> +	status = "okay";
>> +
>> +	pinctrl-0 = <&qup_spi10_cs_gpio_init_high>, <&qup_spi10_cs_gpio>;
> 
> Shouldn't this also have <&qup_spi10_data_clk>?

looks like it does, I'll add

> 
>> +	cs-gpios = <&tlmm 43 GPIO_ACTIVE_LOW>;
>> +
>> +	cros_ec: ec@0 {
>> +		compatible = "google,cros-ec-spi";
>> +		reg = <0>;
>> +		interrupt-parent = <&tlmm>;
>> +		interrupts = <18 IRQ_TYPE_LEVEL_LOW>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&ap_ec_int_l>;
>> +		spi-max-frequency = <3000000>;
>> +
>> +		cros_ec_pwm: ec-pwm {
>> +			compatible = "google,cros-ec-pwm";
>> +			#pwm-cells = <1>;
>> +		};
>> +
>> +		i2c_tunnel: i2c-tunnel {
>> +			compatible = "google,cros-ec-i2c-tunnel";
>> +			google,remote-bus = <0>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +		};
>> +
>> +		typec {
>> +			compatible = "google,cros-ec-typec";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			usb_c0: connector@0 {
>> +				compatible = "usb-c-connector";
>> +				reg = <0>;
>> +				label = "left";
>> +				power-role = "dual";
>> +				data-role = "host";
>> +				try-power-role = "source";
>> +			};
>> +
>> +			usb_c1: connector@1 {
>> +				compatible = "usb-c-connector";
>> +				reg = <1>;
>> +				label = "right";
>> +				power-role = "dual";
>> +				data-role = "host";
>> +				try-power-role = "source";
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +#include <arm/cros-ec-keyboard.dtsi>
>> +#include <arm/cros-ec-sbs.dtsi>
>> +
>> +ap_h1_spi: &spi14 {
>> +	status = "okay";
>> +
>> +	pinctrl-0 = <&qup_spi14_cs_gpio_init_high>, <&qup_spi14_cs_gpio>;
> 
> <&qup_spi14_data_clk> missing?
> 
>> +	cs-gpios = <&tlmm 59 GPIO_ACTIVE_LOW>;
>> +
>> +	cr50: tpm@0 {
>> +		compatible = "google,cr50";
>> +		reg = <0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&h1_ap_int_odl>;
>> +		spi-max-frequency = <800000>;
>> +		interrupt-parent = <&tlmm>;
>> +		interrupts = <104 IRQ_TYPE_EDGE_RISING>;
>> +	};
>> +};
>> +
>> +&tlmm {
>> +	ap_ec_int_l: ap-ec-int-l {
>> +		pins = "gpio18";
>> +		function = "gpio";
>> +		input-enable;
>> +		bias-pull-up;
>> +		drive-strength = <2>;
> 
> Is the explicit drive-strength setting actually needed?
> 
> Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml:
> 
>    drive-strength:
>      enum: [2, 4, 6, 8, 10, 12, 14, 16]
>      default: 2 <=
>      description:
>        Selects the drive strength for the specified pins, in mA.
> 
> The default is 2, hence it shouldn't be necessary it set it explicitly.

right, will remove when i respin
thanks for the review

> 
>> +	};
>> +
>> +	h1_ap_int_odl: h1-ap-int-odl {
>> +		pins = "gpio104";
>> +		function = "gpio";
>> +		input-enable;
>> +		bias-pull-up;
>> +		drive-strength = <2>;
> 
> see above
> 
>> +	};
>> +
>> +	qup_spi10_cs_gpio_init_high: qup-spi10-cs-gpio-init-high {
>> +		pins = "gpio43";
>> +		output-high;
>> +		drive-strength = <2>;
> 
> see above
> 
>> +	};
>> +
>> +	qup_spi14_cs_gpio_init_high: qup-spi14-cs-gpio-init-high {
>> +		pins = "gpio59";
>> +		output-high;
>> +		drive-strength = <2>;
> 
> see above
> 
>> +	};
>> +};
>> +
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> index 3ae9969..208ca69 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp2.dts
>> @@ -8,6 +8,7 @@
>>   /dts-v1/;
>>   
>>   #include "sc7280-idp.dtsi"
>> +#include "sc7280-ec-h1.dtsi"
>>   
>>   / {
>>   	model = "Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform";

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

[PATCH 4/4] arm64: dts: qcom: sc7280-crd: Add Touchscreen and touchpad support

[Rajendra Nayak]

From: Kshitiz Godara <kgodara@codeaurora.org>

Add Touchscreen and touchpad hid-over-i2c node

Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-crd.dts | 63 +++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index 8c2aee6..bef3037 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -23,6 +23,47 @@
 	};
 };
 
+ap_tp_i2c: &i2c0 {
+	status = "okay";
+	clock-frequency = <400000>;
+
+	trackpad: trackpad@15 {
+		compatible = "hid-over-i2c";
+		reg = <0x15>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&tp_int_odl>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+
+		post-power-on-delay-ms = <20>;
+		hid-descr-addr = <0x0001>;
+		vdd-supply = <&vreg_l18b_1p8>;
+
+		wakeup-source;
+	};
+};
+
+ap_ts_pen_1v8: &i2c13 {
+	status = "okay";
+	clock-frequency = <400000>;
+
+	ap_ts: touchscreen@5c {
+		compatible = "hid-over-i2c";
+		reg = <0x5C>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
+
+		interrupt-parent = <&tlmm>;
+		interrupts = <55 IRQ_TYPE_LEVEL_LOW>;
+
+		post-power-on-delay-ms = <500>;
+		hid-descr-addr = <0x0000>;
+
+		vdd-supply = <&vreg_l19b_1p8>;
+	};
+};
+
 &nvme_pwren {
 	pins = "gpio51";
 };
@@ -30,3 +71,25 @@
 &nvme_3v3_regulator {
 	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
 };
+
+&tlmm {
+	tp_int_odl: tp-int-odl {
+		pins = "gpio7";
+		function = "gpio";
+		input-enable;
+		bias-disable;
+	};
+
+	ts_int_l: ts-int-l {
+		pins = "gpio55";
+		function = "gpio";
+		bias-pull-up;
+	};
+
+	ts_reset_l: ts-reset-l {
+		pins = "gpio54";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 4/4] arm64: dts: qcom: sc7280-crd: Add Touchscreen and touchpad support

[Matthias Kaehlcke]

On Tue, Nov 23, 2021 at 12:30:13PM +0530, Rajendra Nayak wrote:
> From: Kshitiz Godara <kgodara@codeaurora.org>
> 
> Add Touchscreen and touchpad hid-over-i2c node

to which board(s)?

> Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-crd.dts | 63 +++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index 8c2aee6..bef3037 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -23,6 +23,47 @@
>  	};
>  };
>  
> +ap_tp_i2c: &i2c0 {
> +	status = "okay";
> +	clock-frequency = <400000>;
> +
> +	trackpad: trackpad@15 {
> +		compatible = "hid-over-i2c";
> +		reg = <0x15>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tp_int_odl>;
> +
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +
> +		post-power-on-delay-ms = <20>;
> +		hid-descr-addr = <0x0001>;
> +		vdd-supply = <&vreg_l18b_1p8>;
> +
> +		wakeup-source;
> +	};
> +};
> +
> +ap_ts_pen_1v8: &i2c13 {
> +	status = "okay";
> +	clock-frequency = <400000>;
> +
> +	ap_ts: touchscreen@5c {
> +		compatible = "hid-over-i2c";
> +		reg = <0x5C>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
> +
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <55 IRQ_TYPE_LEVEL_LOW>;
> +
> +		post-power-on-delay-ms = <500>;
> +		hid-descr-addr = <0x0000>;
> +
> +		vdd-supply = <&vreg_l19b_1p8>;
> +	};
> +};
> +
>  &nvme_pwren {
>  	pins = "gpio51";
>  };
> @@ -30,3 +71,25 @@
>  &nvme_3v3_regulator {
>  	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>  };
> +
> +&tlmm {
> +	tp_int_odl: tp-int-odl {
> +		pins = "gpio7";
> +		function = "gpio";
> +		input-enable;

Not sure about this one, is the explicit 'input-enable' actually needed here?

> +		bias-disable;
> +	};
> +
> +	ts_int_l: ts-int-l {
> +		pins = "gpio55";
> +		function = "gpio";
> +		bias-pull-up;
> +	};
> +
> +	ts_reset_l: ts-reset-l {
> +		pins = "gpio54";
> +		function = "gpio";
> +		bias-disable;
> +		drive-strength = <2>;

As per my comment on "[3/4] arm64: dts: qcom: sc7280: Define EC and H1 nodes" it
seems setting the drive strength to 2 isn't necessary, since that's the default.

Re: [PATCH 4/4] arm64: dts: qcom: sc7280-crd: Add Touchscreen and touchpad support

[Rajendra Nayak]

On 11/23/2021 11:28 PM, Matthias Kaehlcke wrote:
> On Tue, Nov 23, 2021 at 12:30:13PM +0530, Rajendra Nayak wrote:
>> From: Kshitiz Godara <kgodara@codeaurora.org>
>>
>> Add Touchscreen and touchpad hid-over-i2c node
> 
> to which board(s)?

will update

> 
>> Signed-off-by: Kshitiz Godara <kgodara@codeaurora.org>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-crd.dts | 63 +++++++++++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index 8c2aee6..bef3037 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> @@ -23,6 +23,47 @@
>>   	};
>>   };
>>   
>> +ap_tp_i2c: &i2c0 {
>> +	status = "okay";
>> +	clock-frequency = <400000>;
>> +
>> +	trackpad: trackpad@15 {
>> +		compatible = "hid-over-i2c";
>> +		reg = <0x15>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&tp_int_odl>;
>> +
>> +		interrupt-parent = <&tlmm>;
>> +		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +		post-power-on-delay-ms = <20>;
>> +		hid-descr-addr = <0x0001>;
>> +		vdd-supply = <&vreg_l18b_1p8>;
>> +
>> +		wakeup-source;
>> +	};
>> +};
>> +
>> +ap_ts_pen_1v8: &i2c13 {
>> +	status = "okay";
>> +	clock-frequency = <400000>;
>> +
>> +	ap_ts: touchscreen@5c {
>> +		compatible = "hid-over-i2c";
>> +		reg = <0x5C>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
>> +
>> +		interrupt-parent = <&tlmm>;
>> +		interrupts = <55 IRQ_TYPE_LEVEL_LOW>;
>> +
>> +		post-power-on-delay-ms = <500>;
>> +		hid-descr-addr = <0x0000>;
>> +
>> +		vdd-supply = <&vreg_l19b_1p8>;
>> +	};
>> +};
>> +
>>   &nvme_pwren {
>>   	pins = "gpio51";
>>   };
>> @@ -30,3 +71,25 @@
>>   &nvme_3v3_regulator {
>>   	gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>>   };
>> +
>> +&tlmm {
>> +	tp_int_odl: tp-int-odl {
>> +		pins = "gpio7";
>> +		function = "gpio";
>> +		input-enable;
> 
> Not sure about this one, is the explicit 'input-enable' actually needed here?

Maybe not, will test it once after I remove it

> 
>> +		bias-disable;
>> +	};
>> +
>> +	ts_int_l: ts-int-l {
>> +		pins = "gpio55";
>> +		function = "gpio";
>> +		bias-pull-up;
>> +	};
>> +
>> +	ts_reset_l: ts-reset-l {
>> +		pins = "gpio54";
>> +		function = "gpio";
>> +		bias-disable;
>> +		drive-strength = <2>;
> 
> As per my comment on "[3/4] arm64: dts: qcom: sc7280: Define EC and H1 nodes" it
> seems setting the drive strength to 2 isn't necessary, since that's the default.

right I'll remove it, thanks

> 

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