[PATCH v3 0/3] Basic DT support for Lenovo Miix 630

[PATCH v3 0/3] Basic DT support for Lenovo Miix 630

[Jeffrey Hugo]

The Lenovo Miix 630 is one of three ARM based (specifically Qualcomm
MSM8998) laptops that comes with Windows, and seems to have a dedicated
following of folks intrested to get Linux up and running on it.

This series adds support for the basic functionality this is validated
towork using devicetree.  Although the laptops do feed ACPI to Windows,
the existing MSM8998 support in mainline is DT based, so DT provides a
quick path to functionality while ACPI support is investigated.

The three devices are very similar, but do have differences in the set
of peripherals supported, so the idea is that the vast majority of the
support for all three can live in a common include, which should reduce
overall duplication.  Adding support for the other two devices as a
follow on should involve minimal work.

The bleeding edge work for these laptops and work in progress can be
found at https://github.com/aarch64-laptops/prebuilt

v3:
-Changed "clam" to "clamshell"
-Defined a dt binding for the combo Elan keyboard + touchpad device
-Adjusted the HID quirk to be correct for dt boot
-Removed extranious comment in board dts
-Fixed board level compatible

v2:
-Changed "cls" to "clam" since feedback indicated "cls" is too opaque,
but
"clamshell" is a mouthfull.  "clam" seems to be a happy medium.

Jeffrey Hugo (3):
  dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c
  HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT
  arm64: dts: qcom: Add Lenovo Miix 630

 .../bindings/input/elan,combo400-i2c.txt      |  11 +
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/msm8998-clamshell.dtsi      | 278 ++++++++++++++++++
 .../boot/dts/qcom/msm8998-lenovo-miix-630.dts |  30 ++
 drivers/hid/hid-quirks.c                      |   3 +-
 5 files changed, 322 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts

-- 
2.17.1

[PATCH v3 1/3] dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c

[Jeffrey Hugo]

The Elan 400 combo keyboard/touchpad over i2c device is a distinct device
from the Elan 400 standalone touchpad device.  The combo device has been
found in the Lenovo Miix 630 and HP Envy x2 laptops.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 .../devicetree/bindings/input/elan,combo400-i2c.txt   | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt

diff --git a/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
new file mode 100644
index 000000000000..fb700a29148d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
@@ -0,0 +1,11 @@
+Elantech 0400 I2C combination Keyboard/Touchpad
+
+This binding describes an Elan device with pid 0x0400, that is a combination
+keyboard + touchpad device.  This binding does not cover an Elan device with
+pid 0x0400 that is solely a standalone touchpad device.
+
+Required properties:
+- compatible: should be "elan,combo400-i2c"
+
+This binding is compatible with the HID over I2C binding, which is specified
+in hid-over-i2c.txt in this directory.
-- 
2.17.1

[PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT

[Jeffrey Hugo]

Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
+ touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
thus if we want the quirk to work properly when booting via DT instead of
ACPI, we need to key off the DT id as well.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/hid/hid-quirks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 77ffba48cc73..00c08f8318b8 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
 			return true;
 		/* Same with product id 0x0400 */
 		if (hdev->product == 0x0400 &&
-		    strncmp(hdev->name, "QTEC0001", 8) != 0)
+		    (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
+		     strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))
 			return true;
 		break;
 	}
-- 
2.17.1

[PATCH v3 3/3] arm64: dts: qcom: Add Lenovo Miix 630

[Jeffrey Hugo]

This adds the initial DT for the Lenovo Miix 630 laptop.  Supported
functionality includes USB (host), microSD-card, keyboard, and trackpad.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/msm8998-clamshell.dtsi      | 278 ++++++++++++++++++
 .../boot/dts/qcom/msm8998-lenovo-miix-630.dts |  30 ++
 3 files changed, 309 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 21d548f02d39..c3e4307bcbd4 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -6,6 +6,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8992-bullhead-rev-101.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-angler-rev-101.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
new file mode 100644
index 000000000000..1a341d4b1597
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019, Jeffrey Hugo. All rights reserved. */
+
+/*
+ * Common include for MSM8998 clamshell devices, ie the Lenovo Miix 630,
+ * Asus NovaGo TP370QL, and HP Envy x2.  All three devices are basically the
+ * same, with differences in peripherals.
+ */
+
+#include "msm8998.dtsi"
+#include "pm8998.dtsi"
+#include "pm8005.dtsi"
+
+/ {
+	chosen {
+	};
+
+	thermal-zones {
+		battery-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&tsens0 0>;
+
+			trips {
+				battery_crit: trip0 {
+					temperature = <60000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+
+		skin-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&tsens1 5>;
+
+			trips {
+				skin_alert: trip0 {
+					temperature = <44000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				skip_crit: trip1 {
+					temperature = <70000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+};
+
+&qusb2phy {
+	status = "okay";
+
+	vdda-pll-supply = <&vreg_l12a_1p8>;
+	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
+};
+
+&rpm_requests {
+	pm8998-regulators {
+		compatible = "qcom,rpm-pm8998-regulators";
+
+		vdd_s1-supply = <&vph_pwr>;
+		vdd_s2-supply = <&vph_pwr>;
+		vdd_s3-supply = <&vph_pwr>;
+		vdd_s4-supply = <&vph_pwr>;
+		vdd_s5-supply = <&vph_pwr>;
+		vdd_s6-supply = <&vph_pwr>;
+		vdd_s7-supply = <&vph_pwr>;
+		vdd_s8-supply = <&vph_pwr>;
+		vdd_s9-supply = <&vph_pwr>;
+		vdd_s10-supply = <&vph_pwr>;
+		vdd_s11-supply = <&vph_pwr>;
+		vdd_s12-supply = <&vph_pwr>;
+		vdd_s13-supply = <&vph_pwr>;
+		vdd_l1_l27-supply = <&vreg_s7a_1p025>;
+		vdd_l2_l8_l17-supply = <&vreg_s3a_1p35>;
+		vdd_l3_l11-supply = <&vreg_s7a_1p025>;
+		vdd_l4_l5-supply = <&vreg_s7a_1p025>;
+		vdd_l6-supply = <&vreg_s5a_2p04>;
+		vdd_l7_l12_l14_l15-supply = <&vreg_s5a_2p04>;
+		vdd_l9-supply = <&vph_pwr>;
+		vdd_l10_l23_l25-supply = <&vph_pwr>;
+		vdd_l13_l19_l21-supply = <&vph_pwr>;
+		vdd_l16_l28-supply = <&vph_pwr>;
+		vdd_l18_l22-supply = <&vph_pwr>;
+		vdd_l20_l24-supply = <&vph_pwr>;
+		vdd_l26-supply = <&vreg_s3a_1p35>;
+		vdd_lvs1_lvs2-supply = <&vreg_s4a_1p8>;
+
+		vreg_s3a_1p35: s3 {
+			regulator-min-microvolt = <1352000>;
+			regulator-max-microvolt = <1352000>;
+		};
+		vreg_s4a_1p8: s4 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-allow-set-load;
+		};
+		vreg_s5a_2p04: s5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+		vreg_s7a_1p025: s7 {
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1028000>;
+		};
+		vreg_l1a_0p875: l1 {
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-allow-set-load;
+		};
+		vreg_l2a_1p2: l2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-allow-set-load;
+		};
+		vreg_l3a_1p0: l3 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1000000>;
+		};
+		vreg_l5a_0p8: l5 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <800000>;
+		};
+		vreg_l6a_1p8: l6 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <1808000>;
+		};
+		vreg_l7a_1p8: l7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l8a_1p2: l8 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+		vreg_l9a_1p8: l9 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l10a_1p8: l10 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l11a_1p0: l11 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1000000>;
+		};
+		vreg_l12a_1p8: l12 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l13a_2p95: l13 {
+			regulator-min-microvolt = <1808000>;
+			regulator-max-microvolt = <2960000>;
+		};
+		vreg_l14a_1p88: l14 {
+			regulator-min-microvolt = <1880000>;
+			regulator-max-microvolt = <1880000>;
+		};
+		vreg_15a_1p8: l15 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+		vreg_l16a_2p7: l16 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2704000>;
+		};
+		vreg_l17a_1p3: l17 {
+			regulator-min-microvolt = <1304000>;
+			regulator-max-microvolt = <1304000>;
+		};
+		vreg_l18a_2p7: l18 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2704000>;
+		};
+		vreg_l19a_3p0: l19 {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+		};
+		vreg_l20a_2p95: l20 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
+		};
+		vreg_l21a_2p95: l21 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-allow-set-load;
+			regulator-system-load = <800000>;
+		};
+		vreg_l22a_2p85: l22 {
+			regulator-min-microvolt = <2864000>;
+			regulator-max-microvolt = <2864000>;
+		};
+		vreg_l23a_3p3: l23 {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3312000>;
+		};
+		vreg_l24a_3p075: l24 {
+			regulator-min-microvolt = <3088000>;
+			regulator-max-microvolt = <3088000>;
+		};
+		vreg_l25a_3p3: l25 {
+			regulator-min-microvolt = <3104000>;
+			regulator-max-microvolt = <3312000>;
+		};
+		vreg_l26a_1p2: l26 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+		vreg_l28_3p0: l28 {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3008000>;
+		};
+
+		vreg_lvs1a_1p8: lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_lvs2a_1p8: lvs2 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+	};
+};
+
+&tlmm {
+	gpio-reserved-ranges = <0 4>, <81 4>;
+
+	touchpad: touchpad {
+		config {
+			pins = "gpio123";
+			bias-pull-up;           /* pull up */
+		};
+	};
+};
+
+&sdhc2 {
+	status = "okay";
+
+	vmmc-supply = <&vreg_l21a_2p95>;
+	vqmmc-supply = <&vreg_l13a_2p95>;
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_clk_on  &sdc2_cmd_on  &sdc2_data_on  &sdc2_cd_on>;
+	pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
+};
+
+&usb3 {
+	status = "okay";
+};
+
+&usb3_dwc3 {
+	dr_mode = "host"; /* Force to host until we have Type-C hooked up */
+};
+
+&usb3phy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l2a_1p2>;
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts b/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts
new file mode 100644
index 000000000000..c2b43f7ed137
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019, Jeffrey Hugo. All rights reserved. */
+
+/dts-v1/;
+
+#include "msm8998-clamshell.dtsi"
+
+/ {
+	model = "Lenovo Miix 630";
+	compatible = "lenovo,miix-630", "qcom,msm8998";
+};
+
+&blsp1_i2c6 {
+	status = "okay";
+
+	keyboard@3a {
+		compatible = "elan,combo400-i2c", "hid-over-i2c";
+		interrupt-parent = <&tlmm>;
+		interrupts = <0x79 IRQ_TYPE_LEVEL_LOW>;
+		reg = <0x3a>;
+		hid-descr-addr = <0x0001>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&touchpad>;
+	};
+};
+
+&sdhc2 {
+	cd-gpios = <&tlmm 95 GPIO_ACTIVE_HIGH>;
+};
-- 
2.17.1

Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT

[Benjamin Tissoires]

On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
> thus if we want the quirk to work properly when booting via DT instead of
> ACPI, we need to key off the DT id as well.
>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 77ffba48cc73..00c08f8318b8 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
>                         return true;
>                 /* Same with product id 0x0400 */
>                 if (hdev->product == 0x0400 &&
> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))

I think we are taking the problem the wrong way here.

When I first introduced 6ccfe64, I thought 0x0400 would be reserved
for the elan_i2c touchpads only. But it turns out we are deliberately
disabling valid HID touchpads hoping that they would be picked up by
elan_i2c when elan_i2c has its own whitelist of devices.

How about we turn this into list with the matching ones from elan_i2c:
if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
   (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
    strncmp(hdev->name, "ELAN0100", 8) == 0 ||
    ...
    strncmp(hdev->name, "ELAN1000", 8) == 0))
      return true;

So next time we need to force binding a HID touchpad to elan_i2c, we
can just blacklist here and whitelist it in elan_i2c.

Cheers,
Benjamin

>                         return true;
>                 break;
>         }
> --
> 2.17.1
>

Re: [PATCH v3 1/3] dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c

[Benjamin Tissoires]

On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> The Elan 400 combo keyboard/touchpad over i2c device is a distinct device
> from the Elan 400 standalone touchpad device.  The combo device has been
> found in the Lenovo Miix 630 and HP Envy x2 laptops.
>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---

With my comments in 2/3, I wonder if you need this patch at all then.

Cheers,
Benjamin

>  .../devicetree/bindings/input/elan,combo400-i2c.txt   | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
>
> diff --git a/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> new file mode 100644
> index 000000000000..fb700a29148d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> @@ -0,0 +1,11 @@
> +Elantech 0400 I2C combination Keyboard/Touchpad
> +
> +This binding describes an Elan device with pid 0x0400, that is a combination
> +keyboard + touchpad device.  This binding does not cover an Elan device with
> +pid 0x0400 that is solely a standalone touchpad device.
> +
> +Required properties:
> +- compatible: should be "elan,combo400-i2c"
> +
> +This binding is compatible with the HID over I2C binding, which is specified
> +in hid-over-i2c.txt in this directory.
> --
> 2.17.1
>

Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT

[Hans de Goede]

Hi,

On 18-04-19 11:34, Benjamin Tissoires wrote:
> On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>>
>> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
>> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
>> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
>> thus if we want the quirk to work properly when booting via DT instead of
>> ACPI, we need to key off the DT id as well.
>>
>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>> ---
>>   drivers/hid/hid-quirks.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index 77ffba48cc73..00c08f8318b8 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
>>                          return true;
>>                  /* Same with product id 0x0400 */
>>                  if (hdev->product == 0x0400 &&
>> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
>> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
>> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))
> 
> I think we are taking the problem the wrong way here.
> 
> When I first introduced 6ccfe64, I thought 0x0400 would be reserved
> for the elan_i2c touchpads only. But it turns out we are deliberately
> disabling valid HID touchpads hoping that they would be picked up by
> elan_i2c when elan_i2c has its own whitelist of devices.
> 
> How about we turn this into list with the matching ones from elan_i2c:
> if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
>     (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
>      strncmp(hdev->name, "ELAN0100", 8) == 0 ||
>      ...
>      strncmp(hdev->name, "ELAN1000", 8) == 0))
>        return true;
> 
> So next time we need to force binding a HID touchpad to elan_i2c, we
> can just blacklist here and whitelist it in elan_i2c.

This indeed sounds like a better way forward with this.

Regards,

Hans

Re: [PATCH v3 2/3] HID: quirks: Fix keyboard + touchpad on Lenovo Miix 630 for DT

[Jeffrey Hugo]

On Thu, Apr 18, 2019 at 3:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 18-04-19 11:34, Benjamin Tissoires wrote:
> > On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> >>
> >> Following up on commit 2bafa1e96254 ("HID: quirks: Fix keyboard + touchpad
> >> on Lenovo Miix 630"), the devicetree (DT) identifier for the combo keyboard
> >> + touchpad device is "elan,combo400-i2c", which differs from the ACPI ID,
> >> thus if we want the quirk to work properly when booting via DT instead of
> >> ACPI, we need to key off the DT id as well.
> >>
> >> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> >> ---
> >>   drivers/hid/hid-quirks.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> >> index 77ffba48cc73..00c08f8318b8 100644
> >> --- a/drivers/hid/hid-quirks.c
> >> +++ b/drivers/hid/hid-quirks.c
> >> @@ -997,7 +997,8 @@ bool hid_ignore(struct hid_device *hdev)
> >>                          return true;
> >>                  /* Same with product id 0x0400 */
> >>                  if (hdev->product == 0x0400 &&
> >> -                   strncmp(hdev->name, "QTEC0001", 8) != 0)
> >> +                   (strncmp(hdev->name, "QTEC0001", 8) != 0 ||
> >> +                    strncmp(hdev->name, "elan,combo400-i2c", 17) != 0))
> >
> > I think we are taking the problem the wrong way here.
> >
> > When I first introduced 6ccfe64, I thought 0x0400 would be reserved
> > for the elan_i2c touchpads only. But it turns out we are deliberately
> > disabling valid HID touchpads hoping that they would be picked up by
> > elan_i2c when elan_i2c has its own whitelist of devices.
> >
> > How about we turn this into list with the matching ones from elan_i2c:
> > if ((hdev->product == 0x0400 || hdev->product == 0x0401) &&
> >     (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
> >      strncmp(hdev->name, "ELAN0100", 8) == 0 ||
> >      ...
> >      strncmp(hdev->name, "ELAN1000", 8) == 0))
> >        return true;
> >
> > So next time we need to force binding a HID touchpad to elan_i2c, we
> > can just blacklist here and whitelist it in elan_i2c.
>
> This indeed sounds like a better way forward with this.

This sounds good to me.  Let me implement it and test with the Miix
630.  Thanks for the suggestion.

Re: [PATCH v3 1/3] dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c

[Rob Herring]

On Thu, Apr 18, 2019 at 11:35:42AM +0200, Benjamin Tissoires wrote:
> On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> >
> > The Elan 400 combo keyboard/touchpad over i2c device is a distinct device
> > from the Elan 400 standalone touchpad device.  The combo device has been
> > found in the Lenovo Miix 630 and HP Envy x2 laptops.
> >
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> 
> With my comments in 2/3, I wonder if you need this patch at all then.

I don't really follow the discussion in 2/3, but you should still have 
specific compatibles even if right now you don't need them.

> 
> Cheers,
> Benjamin
> 
> >  .../devicetree/bindings/input/elan,combo400-i2c.txt   | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> > new file mode 100644
> > index 000000000000..fb700a29148d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> > @@ -0,0 +1,11 @@
> > +Elantech 0400 I2C combination Keyboard/Touchpad
> > +
> > +This binding describes an Elan device with pid 0x0400, that is a combination
> > +keyboard + touchpad device.  This binding does not cover an Elan device with
> > +pid 0x0400 that is solely a standalone touchpad device.
> > +
> > +Required properties:
> > +- compatible: should be "elan,combo400-i2c"
> > +
> > +This binding is compatible with the HID over I2C binding, which is specified
> > +in hid-over-i2c.txt in this directory.

Separate is fine, but we've been adding compatibles to hid-over-i2c.txt.

Rob

Re: [PATCH v3 3/3] arm64: dts: qcom: Add Lenovo Miix 630

[Bjorn Andersson]

On Mon 15 Apr 09:11 PDT 2019, Jeffrey Hugo wrote:
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
[..]
> +	thermal-zones {
> +		battery-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 0>;

I guess you inherited the battery and skin thermal nodes from my MTP
dts. Unfortunately after talking to Amit I think I got these wrong, and
they should be &pmi8998_adc 0 and 5 instead.

Can you confirm this?

> +
> +			trips {
> +				battery_crit: trip0 {
> +					temperature = <60000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};

Regards,
Bjorn

Re: [PATCH v3 1/3] dt-bindings: input: add Elan 400 combo keyboard/touchpad over i2c

[Jeffrey Hugo]

On Fri, Apr 26, 2019 at 4:49 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Apr 18, 2019 at 11:35:42AM +0200, Benjamin Tissoires wrote:
> > On Mon, Apr 15, 2019 at 6:11 PM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> > >
> > > The Elan 400 combo keyboard/touchpad over i2c device is a distinct device
> > > from the Elan 400 standalone touchpad device.  The combo device has been
> > > found in the Lenovo Miix 630 and HP Envy x2 laptops.
> > >
> > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > > ---
> >
> > With my comments in 2/3, I wonder if you need this patch at all then.
>
> I don't really follow the discussion in 2/3, but you should still have
> specific compatibles even if right now you don't need them.
>
> >
> > Cheers,
> > Benjamin
> >
> > >  .../devicetree/bindings/input/elan,combo400-i2c.txt   | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> > > new file mode 100644
> > > index 000000000000..fb700a29148d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/elan,combo400-i2c.txt
> > > @@ -0,0 +1,11 @@
> > > +Elantech 0400 I2C combination Keyboard/Touchpad
> > > +
> > > +This binding describes an Elan device with pid 0x0400, that is a combination
> > > +keyboard + touchpad device.  This binding does not cover an Elan device with
> > > +pid 0x0400 that is solely a standalone touchpad device.
> > > +
> > > +Required properties:
> > > +- compatible: should be "elan,combo400-i2c"
> > > +
> > > +This binding is compatible with the HID over I2C binding, which is specified
> > > +in hid-over-i2c.txt in this directory.
>
> Separate is fine, but we've been adding compatibles to hid-over-i2c.txt.

Are you just referring to "wacom,w9013" ?

Re: [PATCH v3 3/3] arm64: dts: qcom: Add Lenovo Miix 630

[Jeffrey Hugo]

On Fri, Apr 26, 2019 at 10:42 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 15 Apr 09:11 PDT 2019, Jeffrey Hugo wrote:
> > diff --git a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> [..]
> > +     thermal-zones {
> > +             battery-thermal {
> > +                     polling-delay-passive = <250>;
> > +                     polling-delay = <1000>;
> > +
> > +                     thermal-sensors = <&tsens0 0>;
>
> I guess you inherited the battery and skin thermal nodes from my MTP
> dts. Unfortunately after talking to Amit I think I got these wrong, and
> they should be &pmi8998_adc 0 and 5 instead.
>
> Can you confirm this?

Yeah, I pulled thermal from the MTP.  Someone pointed this out on the
v4 series.  I haven't circled back to it yet.

>
> > +
> > +                     trips {
> > +                             battery_crit: trip0 {
> > +                                     temperature = <60000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +             };
>
> Regards,
> Bjorn