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
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 <[email protected]>
---
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
From: Kshitiz Godara <[email protected]>
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 <[email protected]>
Signed-off-by: Rajendra Nayak <[email protected]>
---
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
From: Kshitiz Godara <[email protected]>
Add Touchscreen and touchpad hid-over-i2c node
Signed-off-by: Kshitiz Godara <[email protected]>
Signed-off-by: Rajendra Nayak <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Rajendra Nayak <[email protected]>
> ---
> 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";
On Tue, Nov 23, 2021 at 12:30:13PM +0530, Rajendra Nayak wrote:
> From: Kshitiz Godara <[email protected]>
>
> Add Touchscreen and touchpad hid-over-i2c node
to which board(s)?
> Signed-off-by: Kshitiz Godara <[email protected]>
> Signed-off-by: Rajendra Nayak <[email protected]>
> ---
> 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.
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 <[email protected]>
>> ---
>> 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
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 <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Rajendra Nayak <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>
>> Add Touchscreen and touchpad hid-over-i2c node
>
> to which board(s)?
will update
>
>> Signed-off-by: Kshitiz Godara <[email protected]>
>> Signed-off-by: Rajendra Nayak <[email protected]>
>> ---
>> 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