[PATCH v6 0/5] Basic DT support for Lenovo Miix 630

[PATCH v6 0/5] 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 is tacked
onto the end of the series.

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

v6:
-Export the elan_i2c DT and ACPI ids so that hid-quirks can use them
-Use the elan_i2c ids within hid-quirks to reduce duplication
-Add DTs for the Asus and HP devices since the DT seems finalized, and
folks have been asking

v5:
-Split out elan_i2c changes into their own patch
-Use a static list of strings to match
-Fixed typo of "whitelist"
-Dropped incorrect thermal zones
-Dropped tags from Bjorn and Lee since the functional should be
identical, but the code is structured different

v4:
-Changed the hid-quirks ELAN handling around per Benjamin Tissoires
-Dropped new DT binding

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 (5):
  Input: elan_i2c: Export the device id whitelist
  HID: quirks: Refactor ELAN 400 and 401 handling
  arm64: dts: qcom: Add Lenovo Miix 630
  arm64: dts: qcom: Add HP Envy x2
  arm64: dts: qcom: Add Asus NovaGo TP370QL

 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/msm8998-clamshell.dtsi      | 240 ++++++++++++++++++
 .../boot/dts/qcom/msm8998-lenovo-miix-630.dts |  30 +++
 drivers/hid/hid-quirks.c                      |  78 +++++-
 drivers/input/mouse/elan_i2c_core.c           |   4 +
 5 files changed, 342 insertions(+), 11 deletions(-)
 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 v6 1/5] Input: elan_i2c: Export the device id whitelist

[Jeffrey Hugo]

Elan_i2c and hid-quirks work in conjunction to decide which devices each
driver will handle.  Elan_i2c has a whitelist of devices that should be
consumed by hid-quirks so that there is one master list of devices to
handoff between the drivers.  Put the ids in a header file so that
hid-quirks can consume it instead of duplicating the list.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/input/mouse/elan_i2c_core.c | 54 +----------------------
 include/linux/input/elan-i2c-ids.h  | 68 +++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 53 deletions(-)
 create mode 100644 include/linux/input/elan-i2c-ids.h

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 65cd325eabc3..74585712e979 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -37,6 +37,7 @@
 #include <linux/completion.h>
 #include <linux/of.h>
 #include <linux/property.h>
+#include <linux/input/elan-i2c-ids.h>
 #include <linux/regulator/consumer.h>
 #include <asm/unaligned.h>
 
@@ -1375,63 +1376,10 @@ static const struct i2c_device_id elan_id[] = {
 MODULE_DEVICE_TABLE(i2c, elan_id);
 
 #ifdef CONFIG_ACPI
-static const struct acpi_device_id elan_acpi_id[] = {
-	{ "ELAN0000", 0 },
-	{ "ELAN0100", 0 },
-	{ "ELAN0600", 0 },
-	{ "ELAN0601", 0 },
-	{ "ELAN0602", 0 },
-	{ "ELAN0603", 0 },
-	{ "ELAN0604", 0 },
-	{ "ELAN0605", 0 },
-	{ "ELAN0606", 0 },
-	{ "ELAN0607", 0 },
-	{ "ELAN0608", 0 },
-	{ "ELAN0609", 0 },
-	{ "ELAN060B", 0 },
-	{ "ELAN060C", 0 },
-	{ "ELAN060F", 0 },
-	{ "ELAN0610", 0 },
-	{ "ELAN0611", 0 },
-	{ "ELAN0612", 0 },
-	{ "ELAN0615", 0 },
-	{ "ELAN0616", 0 },
-	{ "ELAN0617", 0 },
-	{ "ELAN0618", 0 },
-	{ "ELAN0619", 0 },
-	{ "ELAN061A", 0 },
-	{ "ELAN061B", 0 },
-	{ "ELAN061C", 0 },
-	{ "ELAN061D", 0 },
-	{ "ELAN061E", 0 },
-	{ "ELAN061F", 0 },
-	{ "ELAN0620", 0 },
-	{ "ELAN0621", 0 },
-	{ "ELAN0622", 0 },
-	{ "ELAN0623", 0 },
-	{ "ELAN0624", 0 },
-	{ "ELAN0625", 0 },
-	{ "ELAN0626", 0 },
-	{ "ELAN0627", 0 },
-	{ "ELAN0628", 0 },
-	{ "ELAN0629", 0 },
-	{ "ELAN062A", 0 },
-	{ "ELAN062B", 0 },
-	{ "ELAN062C", 0 },
-	{ "ELAN062D", 0 },
-	{ "ELAN0631", 0 },
-	{ "ELAN0632", 0 },
-	{ "ELAN1000", 0 },
-	{ }
-};
 MODULE_DEVICE_TABLE(acpi, elan_acpi_id);
 #endif
 
 #ifdef CONFIG_OF
-static const struct of_device_id elan_of_match[] = {
-	{ .compatible = "elan,ekth3000" },
-	{ /* sentinel */ }
-};
 MODULE_DEVICE_TABLE(of, elan_of_match);
 #endif
 
diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h
new file mode 100644
index 000000000000..8130bbebbdda
--- /dev/null
+++ b/include/linux/input/elan-i2c-ids.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Elan I2C Touchpad devide whitelist
+ *
+ * Copyright (C) 2019 Jeffrey Hugo.  All rights reserved.
+ */
+
+#ifndef __ELAN_I2C_IDS_H
+#define __ELAN_I2C_IDS_H
+
+#include <linux/mod_devicetable.h>
+
+static const struct acpi_device_id elan_acpi_id[] = {
+	{ "ELAN0000", 0 },
+	{ "ELAN0100", 0 },
+	{ "ELAN0600", 0 },
+	{ "ELAN0601", 0 },
+	{ "ELAN0602", 0 },
+	{ "ELAN0603", 0 },
+	{ "ELAN0604", 0 },
+	{ "ELAN0605", 0 },
+	{ "ELAN0606", 0 },
+	{ "ELAN0607", 0 },
+	{ "ELAN0608", 0 },
+	{ "ELAN0609", 0 },
+	{ "ELAN060B", 0 },
+	{ "ELAN060C", 0 },
+	{ "ELAN060F", 0 },
+	{ "ELAN0610", 0 },
+	{ "ELAN0611", 0 },
+	{ "ELAN0612", 0 },
+	{ "ELAN0615", 0 },
+	{ "ELAN0616", 0 },
+	{ "ELAN0617", 0 },
+	{ "ELAN0618", 0 },
+	{ "ELAN0619", 0 },
+	{ "ELAN061A", 0 },
+	{ "ELAN061B", 0 },
+	{ "ELAN061C", 0 },
+	{ "ELAN061D", 0 },
+	{ "ELAN061E", 0 },
+	{ "ELAN061F", 0 },
+	{ "ELAN0620", 0 },
+	{ "ELAN0621", 0 },
+	{ "ELAN0622", 0 },
+	{ "ELAN0623", 0 },
+	{ "ELAN0624", 0 },
+	{ "ELAN0625", 0 },
+	{ "ELAN0626", 0 },
+	{ "ELAN0627", 0 },
+	{ "ELAN0628", 0 },
+	{ "ELAN0629", 0 },
+	{ "ELAN062A", 0 },
+	{ "ELAN062B", 0 },
+	{ "ELAN062C", 0 },
+	{ "ELAN062D", 0 },
+	{ "ELAN0631", 0 },
+	{ "ELAN0632", 0 },
+	{ "ELAN1000", 0 },
+	{ }
+};
+
+static const struct of_device_id elan_of_match[] = {
+	{ .compatible = "elan,ekth3000" },
+	{ /* sentinel */ }
+};
+
+#endif /* __ELAN_I2C_IDS_H */
-- 
2.17.1

[PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Jeffrey Hugo]

There needs to be coordination between hid-quirks and the elan_i2c driver
about which devices are handled by what drivers.  Currently, both use
whitelists, which results in valid devices being unhandled by default,
when they should not be rejected by hid-quirks.  This is quickly becoming
an issue.

Since elan_i2c has a maintained whitelist of what devices it will handle,
which is now in a header file that hid-quirks can access, use that to
implement a blacklist in hid-quirks so that only the devices that need to
be handled by elan_i2c get rejected by hid-quirks, and everything else is
handled by default.

Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index e5ca6fe2ca57..bd81bb090222 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -16,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/input/elan-i2c-ids.h>
 
 #include "hid-ids.h"
 
@@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
 
 bool hid_ignore(struct hid_device *hdev)
 {
+	int i;
+
 	if (hdev->quirks & HID_QUIRK_NO_IGNORE)
 		return false;
 	if (hdev->quirks & HID_QUIRK_IGNORE)
@@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
 		break;
 	case USB_VENDOR_ID_ELAN:
 		/*
-		 * Many Elan devices have a product id of 0x0401 and are handled
-		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
-		 * is not (and cannot be) handled by that driver ->
-		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
+		 * Blacklist of everything that gets handled by the elan_i2c
+		 * input driver.  This avoids disabling valid touchpads and
+		 * other ELAN devices.
 		 */
-		if (hdev->product == 0x0401 &&
-		    strncmp(hdev->name, "ELAN0800", 8) != 0)
-			return true;
-		/* Same with product id 0x0400 */
-		if (hdev->product == 0x0400 &&
-		    strncmp(hdev->name, "QTEC0001", 8) != 0)
-			return true;
+		if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
+			for (i = 0; strlen(elan_acpi_id[i].id); ++i)
+				if (!strncmp(hdev->name, elan_acpi_id[i].id,
+					     strlen(elan_acpi_id[i].id)))
+					return true;
+			for (i = 0; strlen(elan_of_match[i].name); ++i)
+				if (!strncmp(hdev->name, elan_of_match[i].name,
+					     strlen(elan_of_match[i].name)))
+					return true;
+		}
 		break;
 	}
 
-- 
2.17.1

[PATCH v6 3/5] 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      | 240 ++++++++++++++++++
 .../boot/dts/qcom/msm8998-lenovo-miix-630.dts |  30 +++
 3 files changed, 271 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..9682d4dd7496
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
@@ -0,0 +1,240 @@
+// 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 {
+	};
+
+	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..407c6a32911c
--- /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 = "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

[PATCH v6 4/5] arm64: dts: qcom: Add HP Envy x2

[Jeffrey Hugo]

This adds the initial DT for the HP Envy x2 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-hp-envy-x2.dts      | 30 +++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index c3e4307bcbd4..76436f33a013 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-hp-envy-x2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.dts b/arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.dts
new file mode 100644
index 000000000000..24073127091f
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-hp-envy-x2.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 = "HP Envy x2";
+	compatible = "hp,envy-x2", "qcom,msm8998";
+};
+
+&blsp1_i2c6 {
+	status = "okay";
+
+	keyboard@3a {
+		compatible = "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_LOW>;
+};
-- 
2.17.1

[PATCH v6 5/5] arm64: dts: qcom: Add Asus NovaGo TP370QL

[Jeffrey Hugo]

This adds the initial DT for the Asus NovaGo TP370QL 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 +
 .../dts/qcom/msm8998-asus-novago-tp370ql.dts  | 47 +++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 76436f33a013..5cd1844a6d33 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-asus-novago-tp370ql.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-hp-envy-x2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dts b/arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dts
new file mode 100644
index 000000000000..db5821be1e2f
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8998-asus-novago-tp370ql.dts
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019, Jeffrey Hugo. All rights reserved. */
+
+/dts-v1/;
+
+#include "msm8998-clamshell.dtsi"
+
+/ {
+	model = "Asus NovaGo TP370QL";
+	compatible = "asus,novago-tp370ql", "qcom,msm8998";
+};
+
+&blsp1_i2c6 {
+	status = "okay";
+
+	touchpad@15 {
+		compatible = "hid-over-i2c";
+		interrupt-parent = <&tlmm>;
+		interrupts = <0x7b IRQ_TYPE_LEVEL_LOW>;
+		reg = <0x15>;
+		hid-descr-addr = <0x0001>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&touchpad>;
+	};
+
+	keyboard@3a {
+		compatible = "hid-over-i2c";
+		interrupt-parent = <&tlmm>;
+		interrupts = <0x25 IRQ_TYPE_LEVEL_LOW>;
+		reg = <0x3a>;
+		hid-descr-addr = <0x0001>;
+	};
+};
+
+&sdhc2 {
+	cd-gpios = <&tlmm 95 GPIO_ACTIVE_HIGH>;
+};
+
+&tlmm {
+	touchpad: touchpad {
+		config {
+			pins = "gpio123";
+			bias-pull-up;
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Dmitry Torokhov]

On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
> There needs to be coordination between hid-quirks and the elan_i2c driver
> about which devices are handled by what drivers.  Currently, both use
> whitelists, which results in valid devices being unhandled by default,
> when they should not be rejected by hid-quirks.  This is quickly becoming
> an issue.
> 
> Since elan_i2c has a maintained whitelist of what devices it will handle,
> which is now in a header file that hid-quirks can access, use that to
> implement a blacklist in hid-quirks so that only the devices that need to
> be handled by elan_i2c get rejected by hid-quirks, and everything else is
> handled by default.
> 
> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e5ca6fe2ca57..bd81bb090222 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -16,6 +16,7 @@
>  #include <linux/export.h>
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
> +#include <linux/input/elan-i2c-ids.h>
>  
>  #include "hid-ids.h"
>  
> @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>  
>  bool hid_ignore(struct hid_device *hdev)
>  {
> +	int i;
> +
>  	if (hdev->quirks & HID_QUIRK_NO_IGNORE)
>  		return false;
>  	if (hdev->quirks & HID_QUIRK_IGNORE)
> @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
>  		break;
>  	case USB_VENDOR_ID_ELAN:
>  		/*
> -		 * Many Elan devices have a product id of 0x0401 and are handled
> -		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
> -		 * is not (and cannot be) handled by that driver ->
> -		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
> +		 * Blacklist of everything that gets handled by the elan_i2c
> +		 * input driver.  This avoids disabling valid touchpads and
> +		 * other ELAN devices.
>  		 */
> -		if (hdev->product == 0x0401 &&
> -		    strncmp(hdev->name, "ELAN0800", 8) != 0)
> -			return true;
> -		/* Same with product id 0x0400 */
> -		if (hdev->product == 0x0400 &&
> -		    strncmp(hdev->name, "QTEC0001", 8) != 0)
> -			return true;
> +		if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
> +			for (i = 0; strlen(elan_acpi_id[i].id); ++i)
> +				if (!strncmp(hdev->name, elan_acpi_id[i].id,
> +					     strlen(elan_acpi_id[i].id)))
> +					return true;
> +			for (i = 0; strlen(elan_of_match[i].name); ++i)
> +				if (!strncmp(hdev->name, elan_of_match[i].name,
> +					     strlen(elan_of_match[i].name)))
> +					return true;

Do we really need to blacklist the OF case here? I thought that in ACPI
case we have clashes as HID gets matched by elan_i2c and CID is matched
by i2c-hid, but I do not believe we'll run into the same situation on OF
systems.

Thanks.

-- 
Dmitry

Re: Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Jeffrey Hugo]

On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:
> On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
>> There needs to be coordination between hid-quirks and the elan_i2c driver
>> about which devices are handled by what drivers.  Currently, both use
>> whitelists, which results in valid devices being unhandled by default,
>> when they should not be rejected by hid-quirks.  This is quickly becoming
>> an issue.
>>
>> Since elan_i2c has a maintained whitelist of what devices it will handle,
>> which is now in a header file that hid-quirks can access, use that to
>> implement a blacklist in hid-quirks so that only the devices that need to
>> be handled by elan_i2c get rejected by hid-quirks, and everything else is
>> handled by default.
>>
>> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>> ---
>>   drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
>>   1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index e5ca6fe2ca57..bd81bb090222 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/export.h>
>>   #include <linux/slab.h>
>>   #include <linux/mutex.h>
>> +#include <linux/input/elan-i2c-ids.h>
>>   
>>   #include "hid-ids.h"
>>   
>> @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>>   
>>   bool hid_ignore(struct hid_device *hdev)
>>   {
>> +	int i;
>> +
>>   	if (hdev->quirks & HID_QUIRK_NO_IGNORE)
>>   		return false;
>>   	if (hdev->quirks & HID_QUIRK_IGNORE)
>> @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
>>   		break;
>>   	case USB_VENDOR_ID_ELAN:
>>   		/*
>> -		 * Many Elan devices have a product id of 0x0401 and are handled
>> -		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
>> -		 * is not (and cannot be) handled by that driver ->
>> -		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
>> +		 * Blacklist of everything that gets handled by the elan_i2c
>> +		 * input driver.  This avoids disabling valid touchpads and
>> +		 * other ELAN devices.
>>   		 */
>> -		if (hdev->product == 0x0401 &&
>> -		    strncmp(hdev->name, "ELAN0800", 8) != 0)
>> -			return true;
>> -		/* Same with product id 0x0400 */
>> -		if (hdev->product == 0x0400 &&
>> -		    strncmp(hdev->name, "QTEC0001", 8) != 0)
>> -			return true;
>> +		if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
>> +			for (i = 0; strlen(elan_acpi_id[i].id); ++i)
>> +				if (!strncmp(hdev->name, elan_acpi_id[i].id,
>> +					     strlen(elan_acpi_id[i].id)))
>> +					return true;
>> +			for (i = 0; strlen(elan_of_match[i].name); ++i)
>> +				if (!strncmp(hdev->name, elan_of_match[i].name,
>> +					     strlen(elan_of_match[i].name)))
>> +					return true;
> 
> Do we really need to blacklist the OF case here? I thought that in ACPI
> case we have clashes as HID gets matched by elan_i2c and CID is matched
> by i2c-hid, but I do not believe we'll run into the same situation on OF
> systems.

I think its the safer approach.

On an OF system, such as patch 3 in the series, the "hid-over-i2c" will 
end up running through this (kind of the whole reason why this series 
exists).  The vendor and product ids will still match, so we'll end up 
going through the lists to see if the hdev->name (the compatible string) 
will match the blacklist.  "hid-over-i2c" won't match the blacklist, but 
if there is a more specific compatible, it might.

In that case, not matching OF would work, however how it could break 
today is if both "hid-over-i2c" and "elan,ekth3000" were listed for the 
same device, and elan_i2c was not compiled.  In that case, if we skip 
the OF part of the black list, hid-quirks will not reject the device, 
and you'll probably have some odd behavior instead of the obvious "the 
device doesn't work because the correct driver isn't present" behavior.

While that scenario might be far fetched since having both 
"hid-over-i2c" and "elan,ekth3000" probably violates the OF bindings, 
its still safer to include the OF case in the blacklist against future 
scenarios.

Re: Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Benjamin Tissoires]

On Thu, Jun 13, 2019 at 12:20 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>
> On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:
> > On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
> >> There needs to be coordination between hid-quirks and the elan_i2c driver
> >> about which devices are handled by what drivers.  Currently, both use
> >> whitelists, which results in valid devices being unhandled by default,
> >> when they should not be rejected by hid-quirks.  This is quickly becoming
> >> an issue.
> >>
> >> Since elan_i2c has a maintained whitelist of what devices it will handle,
> >> which is now in a header file that hid-quirks can access, use that to
> >> implement a blacklist in hid-quirks so that only the devices that need to
> >> be handled by elan_i2c get rejected by hid-quirks, and everything else is
> >> handled by default.
> >>
> >> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> >> ---
> >>   drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
> >>   1 file changed, 16 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> >> index e5ca6fe2ca57..bd81bb090222 100644
> >> --- a/drivers/hid/hid-quirks.c
> >> +++ b/drivers/hid/hid-quirks.c
> >> @@ -16,6 +16,7 @@
> >>   #include <linux/export.h>
> >>   #include <linux/slab.h>
> >>   #include <linux/mutex.h>
> >> +#include <linux/input/elan-i2c-ids.h>
> >>
> >>   #include "hid-ids.h"
> >>
> >> @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
> >>
> >>   bool hid_ignore(struct hid_device *hdev)
> >>   {
> >> +    int i;
> >> +
> >>      if (hdev->quirks & HID_QUIRK_NO_IGNORE)
> >>              return false;
> >>      if (hdev->quirks & HID_QUIRK_IGNORE)
> >> @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
> >>              break;
> >>      case USB_VENDOR_ID_ELAN:
> >>              /*
> >> -             * Many Elan devices have a product id of 0x0401 and are handled
> >> -             * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
> >> -             * is not (and cannot be) handled by that driver ->
> >> -             * Ignore all 0x0401 devs except for the ELAN0800 dev.
> >> +             * Blacklist of everything that gets handled by the elan_i2c
> >> +             * input driver.  This avoids disabling valid touchpads and
> >> +             * other ELAN devices.
> >>               */
> >> -            if (hdev->product == 0x0401 &&
> >> -                strncmp(hdev->name, "ELAN0800", 8) != 0)
> >> -                    return true;
> >> -            /* Same with product id 0x0400 */
> >> -            if (hdev->product == 0x0400 &&
> >> -                strncmp(hdev->name, "QTEC0001", 8) != 0)
> >> -                    return true;
> >> +            if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
> >> +                    for (i = 0; strlen(elan_acpi_id[i].id); ++i)
> >> +                            if (!strncmp(hdev->name, elan_acpi_id[i].id,
> >> +                                         strlen(elan_acpi_id[i].id)))
> >> +                                    return true;
> >> +                    for (i = 0; strlen(elan_of_match[i].name); ++i)
> >> +                            if (!strncmp(hdev->name, elan_of_match[i].name,
> >> +                                         strlen(elan_of_match[i].name)))
> >> +                                    return true;
> >
> > Do we really need to blacklist the OF case here? I thought that in ACPI
> > case we have clashes as HID gets matched by elan_i2c and CID is matched
> > by i2c-hid, but I do not believe we'll run into the same situation on OF
> > systems.
>
> I think its the safer approach.
>
> On an OF system, such as patch 3 in the series, the "hid-over-i2c" will
> end up running through this (kind of the whole reason why this series
> exists).  The vendor and product ids will still match, so we'll end up
> going through the lists to see if the hdev->name (the compatible string)
> will match the blacklist.  "hid-over-i2c" won't match the blacklist, but
> if there is a more specific compatible, it might.
>
> In that case, not matching OF would work, however how it could break
> today is if both "hid-over-i2c" and "elan,ekth3000" were listed for the
> same device, and elan_i2c was not compiled.  In that case, if we skip
> the OF part of the black list, hid-quirks will not reject the device,
> and you'll probably have some odd behavior instead of the obvious "the
> device doesn't work because the correct driver isn't present" behavior.
>
> While that scenario might be far fetched since having both
> "hid-over-i2c" and "elan,ekth3000" probably violates the OF bindings,
> its still safer to include the OF case in the blacklist against future
> scenarios.
>
>

Dmitry, if you are happy with Jeffrey's answer, feel free to take this
through your tree and add:
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I don't expect any major conflicts given on where the code is located.

Cheers,
Benjamin

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

[Rob Clark]

On Thu, Jun 13, 2019 at 10:17 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> 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>
> ---

[snip]

> 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..407c6a32911c
> --- /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";
> +};


So, I'm not sure if there is some precedent for this (but maybe we
haven't really had this problem before).. but as I mentioned on
#arch64-laptops, I think we should put vendor/product/board-id strings
from SMBIOS table in the dts files.  That could be used by grub to
find the correct dtb file to load in a generic way.  (Ie, look for a
match of all three strings, and maybe fallback to a match on just
vendor+product??)

At any rate, how the strings are used can be refined later.  But I
think we should include the strings from the beginning for anything
that is booting via UEFI.  It's perhaps more useful than the
compatible string.

BR,
-R

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

[Rob Clark]

On Fri, Jun 14, 2019 at 6:44 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Thu, Jun 13, 2019 at 10:17 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> >
> > 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>
> > ---
>
> [snip]
>
> > 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..407c6a32911c
> > --- /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";
> > +};
>
>
> So, I'm not sure if there is some precedent for this (but maybe we
> haven't really had this problem before).. but as I mentioned on
> #arch64-laptops, I think we should put vendor/product/board-id strings
> from SMBIOS table in the dts files.  That could be used by grub to
> find the correct dtb file to load in a generic way.  (Ie, look for a
> match of all three strings, and maybe fallback to a match on just
> vendor+product??)
>
> At any rate, how the strings are used can be refined later.  But I
> think we should include the strings from the beginning for anything
> that is booting via UEFI.  It's perhaps more useful than the
> compatible string.
>


perhaps something like:

   dmi-compatible = "LENOVO 81JL/LNVNB161216", "LENOVO 81JL";

??

(well, those are the strings from my yoga c630, not sure what they are
on the miix 630.. but you get the idea)

BR,
-R

Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Jeffrey Hugo]

On 6/13/2019 2:55 AM, Benjamin Tissoires wrote:
> On Thu, Jun 13, 2019 at 12:20 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>>
>> On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:
>>> On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
>>>> There needs to be coordination between hid-quirks and the elan_i2c driver
>>>> about which devices are handled by what drivers.  Currently, both use
>>>> whitelists, which results in valid devices being unhandled by default,
>>>> when they should not be rejected by hid-quirks.  This is quickly becoming
>>>> an issue.
>>>>
>>>> Since elan_i2c has a maintained whitelist of what devices it will handle,
>>>> which is now in a header file that hid-quirks can access, use that to
>>>> implement a blacklist in hid-quirks so that only the devices that need to
>>>> be handled by elan_i2c get rejected by hid-quirks, and everything else is
>>>> handled by default.
>>>>
>>>> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>>>> ---
>>>>    drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
>>>>    1 file changed, 16 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>>>> index e5ca6fe2ca57..bd81bb090222 100644
>>>> --- a/drivers/hid/hid-quirks.c
>>>> +++ b/drivers/hid/hid-quirks.c
>>>> @@ -16,6 +16,7 @@
>>>>    #include <linux/export.h>
>>>>    #include <linux/slab.h>
>>>>    #include <linux/mutex.h>
>>>> +#include <linux/input/elan-i2c-ids.h>
>>>>
>>>>    #include "hid-ids.h"
>>>>
>>>> @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>>>>
>>>>    bool hid_ignore(struct hid_device *hdev)
>>>>    {
>>>> +    int i;
>>>> +
>>>>       if (hdev->quirks & HID_QUIRK_NO_IGNORE)
>>>>               return false;
>>>>       if (hdev->quirks & HID_QUIRK_IGNORE)
>>>> @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
>>>>               break;
>>>>       case USB_VENDOR_ID_ELAN:
>>>>               /*
>>>> -             * Many Elan devices have a product id of 0x0401 and are handled
>>>> -             * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
>>>> -             * is not (and cannot be) handled by that driver ->
>>>> -             * Ignore all 0x0401 devs except for the ELAN0800 dev.
>>>> +             * Blacklist of everything that gets handled by the elan_i2c
>>>> +             * input driver.  This avoids disabling valid touchpads and
>>>> +             * other ELAN devices.
>>>>                */
>>>> -            if (hdev->product == 0x0401 &&
>>>> -                strncmp(hdev->name, "ELAN0800", 8) != 0)
>>>> -                    return true;
>>>> -            /* Same with product id 0x0400 */
>>>> -            if (hdev->product == 0x0400 &&
>>>> -                strncmp(hdev->name, "QTEC0001", 8) != 0)
>>>> -                    return true;
>>>> +            if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
>>>> +                    for (i = 0; strlen(elan_acpi_id[i].id); ++i)
>>>> +                            if (!strncmp(hdev->name, elan_acpi_id[i].id,
>>>> +                                         strlen(elan_acpi_id[i].id)))
>>>> +                                    return true;
>>>> +                    for (i = 0; strlen(elan_of_match[i].name); ++i)
>>>> +                            if (!strncmp(hdev->name, elan_of_match[i].name,
>>>> +                                         strlen(elan_of_match[i].name)))
>>>> +                                    return true;
>>>
>>> Do we really need to blacklist the OF case here? I thought that in ACPI
>>> case we have clashes as HID gets matched by elan_i2c and CID is matched
>>> by i2c-hid, but I do not believe we'll run into the same situation on OF
>>> systems.
>>
>> I think its the safer approach.
>>
>> On an OF system, such as patch 3 in the series, the "hid-over-i2c" will
>> end up running through this (kind of the whole reason why this series
>> exists).  The vendor and product ids will still match, so we'll end up
>> going through the lists to see if the hdev->name (the compatible string)
>> will match the blacklist.  "hid-over-i2c" won't match the blacklist, but
>> if there is a more specific compatible, it might.
>>
>> In that case, not matching OF would work, however how it could break
>> today is if both "hid-over-i2c" and "elan,ekth3000" were listed for the
>> same device, and elan_i2c was not compiled.  In that case, if we skip
>> the OF part of the black list, hid-quirks will not reject the device,
>> and you'll probably have some odd behavior instead of the obvious "the
>> device doesn't work because the correct driver isn't present" behavior.
>>
>> While that scenario might be far fetched since having both
>> "hid-over-i2c" and "elan,ekth3000" probably violates the OF bindings,
>> its still safer to include the OF case in the blacklist against future
>> scenarios.
>>
>>
> 
> Dmitry, if you are happy with Jeffrey's answer, feel free to take this
> through your tree and add:
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> I don't expect any major conflicts given on where the code is located.

Ping?

Dmitry, are you happy with things?  I would really like to see this 
queued for 5.3, and it seems like the window to do so is rapidly closing.

Re: Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Dmitry Torokhov]

On Wed, Jun 12, 2019 at 04:20:42PM -0600, Jeffrey Hugo wrote:
> On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:
> > On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
> > > There needs to be coordination between hid-quirks and the elan_i2c driver
> > > about which devices are handled by what drivers.  Currently, both use
> > > whitelists, which results in valid devices being unhandled by default,
> > > when they should not be rejected by hid-quirks.  This is quickly becoming
> > > an issue.
> > > 
> > > Since elan_i2c has a maintained whitelist of what devices it will handle,
> > > which is now in a header file that hid-quirks can access, use that to
> > > implement a blacklist in hid-quirks so that only the devices that need to
> > > be handled by elan_i2c get rejected by hid-quirks, and everything else is
> > > handled by default.
> > > 
> > > Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > > ---
> > >   drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
> > >   1 file changed, 16 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > > index e5ca6fe2ca57..bd81bb090222 100644
> > > --- a/drivers/hid/hid-quirks.c
> > > +++ b/drivers/hid/hid-quirks.c
> > > @@ -16,6 +16,7 @@
> > >   #include <linux/export.h>
> > >   #include <linux/slab.h>
> > >   #include <linux/mutex.h>
> > > +#include <linux/input/elan-i2c-ids.h>
> > >   #include "hid-ids.h"
> > > @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
> > >   bool hid_ignore(struct hid_device *hdev)
> > >   {
> > > +	int i;
> > > +
> > >   	if (hdev->quirks & HID_QUIRK_NO_IGNORE)
> > >   		return false;
> > >   	if (hdev->quirks & HID_QUIRK_IGNORE)
> > > @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
> > >   		break;
> > >   	case USB_VENDOR_ID_ELAN:
> > >   		/*
> > > -		 * Many Elan devices have a product id of 0x0401 and are handled
> > > -		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
> > > -		 * is not (and cannot be) handled by that driver ->
> > > -		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
> > > +		 * Blacklist of everything that gets handled by the elan_i2c
> > > +		 * input driver.  This avoids disabling valid touchpads and
> > > +		 * other ELAN devices.
> > >   		 */
> > > -		if (hdev->product == 0x0401 &&
> > > -		    strncmp(hdev->name, "ELAN0800", 8) != 0)
> > > -			return true;
> > > -		/* Same with product id 0x0400 */
> > > -		if (hdev->product == 0x0400 &&
> > > -		    strncmp(hdev->name, "QTEC0001", 8) != 0)
> > > -			return true;
> > > +		if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
> > > +			for (i = 0; strlen(elan_acpi_id[i].id); ++i)
> > > +				if (!strncmp(hdev->name, elan_acpi_id[i].id,
> > > +					     strlen(elan_acpi_id[i].id)))
> > > +					return true;
> > > +			for (i = 0; strlen(elan_of_match[i].name); ++i)
> > > +				if (!strncmp(hdev->name, elan_of_match[i].name,
> > > +					     strlen(elan_of_match[i].name)))
> > > +					return true;
> > 
> > Do we really need to blacklist the OF case here? I thought that in ACPI
> > case we have clashes as HID gets matched by elan_i2c and CID is matched
> > by i2c-hid, but I do not believe we'll run into the same situation on OF
> > systems.
> 
> I think its the safer approach.
> 
> On an OF system, such as patch 3 in the series, the "hid-over-i2c" will end
> up running through this (kind of the whole reason why this series exists).
> The vendor and product ids will still match, so we'll end up going through
> the lists to see if the hdev->name (the compatible string) will match the
> blacklist.  "hid-over-i2c" won't match the blacklist, but if there is a more
> specific compatible, it might.
> 
> In that case, not matching OF would work, however how it could break today
> is if both "hid-over-i2c" and "elan,ekth3000" were listed for the same
> device, and elan_i2c was not compiled.  In that case, if we skip the OF part
> of the black list, hid-quirks will not reject the device, and you'll
> probably have some odd behavior instead of the obvious "the device doesn't
> work because the correct driver isn't present" behavior.
> 
> While that scenario might be far fetched since having both "hid-over-i2c"
> and "elan,ekth3000" probably violates the OF bindings, its still safer to
> include the OF case in the blacklist against future scenarios.

Yes, I believe it is quite far fetched. We are talking about someone
setting compatible sting to something that is decidedly not compatible.
I.e. we know that devices driven by elan_i2c are not compatible with
hi-over-i2c driver/protocol, so why do we expect that they both will be
specified in the same compatible string? I know ACPI case is messier in
this regard as 2 drivers look at the different data items when
evaluating whether they should bind to the device, but here we are
dealing with the same string.

Thanks.

-- 
Dmitry

Re: [PATCH v6 2/5] HID: quirks: Refactor ELAN 400 and 401 handling

[Jeffrey Hugo]

On 6/19/2019 11:10 AM, Dmitry Torokhov wrote:
> On Wed, Jun 12, 2019 at 04:20:42PM -0600, Jeffrey Hugo wrote:
>> On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:
>>> On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:
>>>> There needs to be coordination between hid-quirks and the elan_i2c driver
>>>> about which devices are handled by what drivers.  Currently, both use
>>>> whitelists, which results in valid devices being unhandled by default,
>>>> when they should not be rejected by hid-quirks.  This is quickly becoming
>>>> an issue.
>>>>
>>>> Since elan_i2c has a maintained whitelist of what devices it will handle,
>>>> which is now in a header file that hid-quirks can access, use that to
>>>> implement a blacklist in hid-quirks so that only the devices that need to
>>>> be handled by elan_i2c get rejected by hid-quirks, and everything else is
>>>> handled by default.
>>>>
>>>> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>>>> ---
>>>>    drivers/hid/hid-quirks.c | 27 ++++++++++++++++-----------
>>>>    1 file changed, 16 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>>>> index e5ca6fe2ca57..bd81bb090222 100644
>>>> --- a/drivers/hid/hid-quirks.c
>>>> +++ b/drivers/hid/hid-quirks.c
>>>> @@ -16,6 +16,7 @@
>>>>    #include <linux/export.h>
>>>>    #include <linux/slab.h>
>>>>    #include <linux/mutex.h>
>>>> +#include <linux/input/elan-i2c-ids.h>
>>>>    #include "hid-ids.h"
>>>> @@ -914,6 +915,8 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>>>>    bool hid_ignore(struct hid_device *hdev)
>>>>    {
>>>> +	int i;
>>>> +
>>>>    	if (hdev->quirks & HID_QUIRK_NO_IGNORE)
>>>>    		return false;
>>>>    	if (hdev->quirks & HID_QUIRK_IGNORE)
>>>> @@ -978,18 +981,20 @@ bool hid_ignore(struct hid_device *hdev)
>>>>    		break;
>>>>    	case USB_VENDOR_ID_ELAN:
>>>>    		/*
>>>> -		 * Many Elan devices have a product id of 0x0401 and are handled
>>>> -		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
>>>> -		 * is not (and cannot be) handled by that driver ->
>>>> -		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
>>>> +		 * Blacklist of everything that gets handled by the elan_i2c
>>>> +		 * input driver.  This avoids disabling valid touchpads and
>>>> +		 * other ELAN devices.
>>>>    		 */
>>>> -		if (hdev->product == 0x0401 &&
>>>> -		    strncmp(hdev->name, "ELAN0800", 8) != 0)
>>>> -			return true;
>>>> -		/* Same with product id 0x0400 */
>>>> -		if (hdev->product == 0x0400 &&
>>>> -		    strncmp(hdev->name, "QTEC0001", 8) != 0)
>>>> -			return true;
>>>> +		if ((hdev->product == 0x0401 || hdev->product == 0x0400)) {
>>>> +			for (i = 0; strlen(elan_acpi_id[i].id); ++i)
>>>> +				if (!strncmp(hdev->name, elan_acpi_id[i].id,
>>>> +					     strlen(elan_acpi_id[i].id)))
>>>> +					return true;
>>>> +			for (i = 0; strlen(elan_of_match[i].name); ++i)
>>>> +				if (!strncmp(hdev->name, elan_of_match[i].name,
>>>> +					     strlen(elan_of_match[i].name)))
>>>> +					return true;
>>>
>>> Do we really need to blacklist the OF case here? I thought that in ACPI
>>> case we have clashes as HID gets matched by elan_i2c and CID is matched
>>> by i2c-hid, but I do not believe we'll run into the same situation on OF
>>> systems.
>>
>> I think its the safer approach.
>>
>> On an OF system, such as patch 3 in the series, the "hid-over-i2c" will end
>> up running through this (kind of the whole reason why this series exists).
>> The vendor and product ids will still match, so we'll end up going through
>> the lists to see if the hdev->name (the compatible string) will match the
>> blacklist.  "hid-over-i2c" won't match the blacklist, but if there is a more
>> specific compatible, it might.
>>
>> In that case, not matching OF would work, however how it could break today
>> is if both "hid-over-i2c" and "elan,ekth3000" were listed for the same
>> device, and elan_i2c was not compiled.  In that case, if we skip the OF part
>> of the black list, hid-quirks will not reject the device, and you'll
>> probably have some odd behavior instead of the obvious "the device doesn't
>> work because the correct driver isn't present" behavior.
>>
>> While that scenario might be far fetched since having both "hid-over-i2c"
>> and "elan,ekth3000" probably violates the OF bindings, its still safer to
>> include the OF case in the blacklist against future scenarios.
> 
> Yes, I believe it is quite far fetched. We are talking about someone
> setting compatible sting to something that is decidedly not compatible.
> I.e. we know that devices driven by elan_i2c are not compatible with
> hi-over-i2c driver/protocol, so why do we expect that they both will be
> specified in the same compatible string? I know ACPI case is messier in
> this regard as 2 drivers look at the different data items when
> evaluating whether they should bind to the device, but here we are
> dealing with the same string.

Alright.  Sounds like you really want the DT matching dropped, so I'll 
update the series with a new version ASAP that drops that.