Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 0/3] Basic DT support for Lenovo Miix 630
@ 2019-06-06 16:10 Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 1/3] Input: elan_i2c: Add comment about link between elan_i2c and hid-quirks Jeffrey Hugo
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-06 16:10 UTC (permalink / raw)
  To: unlisted-recipients:; (no To-header on input)
  Cc: bjorn.andersson, benjamin.tissoires, dmitry.torokhov, jikos,
	lee.jones, robh+dt, mark.rutland, agross, david.brown, hdegoede,
	linux-input, devicetree, linux-arm-msm, linux-kernel,
	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

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 (3):
  Input: elan_i2c: Add comment about link between elan_i2c and
    hid-quirks
  HID: quirks: Refactor ELAN 400 and 401 handling
  arm64: dts: qcom: Add Lenovo Miix 630

 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


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

* [PATCH v5 1/3] Input: elan_i2c: Add comment about link between elan_i2c and hid-quirks
  2019-06-06 16:10 [PATCH v5 0/3] Basic DT support for Lenovo Miix 630 Jeffrey Hugo
@ 2019-06-06 16:13 ` Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 3/3] arm64: dts: qcom: Add Lenovo Miix 630 Jeffrey Hugo
  2 siblings, 0 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-06 16:13 UTC (permalink / raw)
  To: benjamin.tissoires, dmitry.torokhov, jikos
  Cc: bjorn.andersson, lee.jones, robh+dt, mark.rutland, agross,
	david.brown, hdegoede, linux-input, devicetree, linux-arm-msm,
	linux-kernel, Jeffrey Hugo

Elan_i2c and hid-quirks work in conjunction to decide which devices each
driver will handle.  Document this link in elan_i2c as a reminder that
updates to elan_i2c need to be mirrored to hid-quirks.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/input/mouse/elan_i2c_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 65cd325eabc3..4cb49925a8b4 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1374,6 +1374,10 @@ static const struct i2c_device_id elan_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, elan_id);
 
+/*
+ * when these whitelists get updated, the corresponding blacklist in hid-quirks
+ * needs to be updated to match.
+ */
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id elan_acpi_id[] = {
 	{ "ELAN0000", 0 },
-- 
2.17.1


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

* [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-06 16:10 [PATCH v5 0/3] Basic DT support for Lenovo Miix 630 Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 1/3] Input: elan_i2c: Add comment about link between elan_i2c and hid-quirks Jeffrey Hugo
@ 2019-06-06 16:13 ` Jeffrey Hugo
  2019-06-12  0:35   ` Dmitry Torokhov
  2019-06-06 16:13 ` [PATCH v5 3/3] arm64: dts: qcom: Add Lenovo Miix 630 Jeffrey Hugo
  2 siblings, 1 reply; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-06 16:13 UTC (permalink / raw)
  To: benjamin.tissoires, dmitry.torokhov, jikos
  Cc: bjorn.andersson, lee.jones, robh+dt, mark.rutland, agross,
	david.brown, hdegoede, linux-input, devicetree, linux-arm-msm,
	linux-kernel, 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,
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.  The downside is the whitelist and
blacklist need to be kept in sync.

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

diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index e5ca6fe2ca57..edebd0700e3d 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
 	{ }
 };
 
+/* 
+ * List of device names that elan_i2c is handling and HID should ignore.  Must
+ * be kept in sync with elan_i2c
+ */
+static const char *hid_elan_i2c_ignore[] = {
+	"ELAN0000",
+	"ELAN0100",
+	"ELAN0600",
+	"ELAN0601",
+	"ELAN0602",
+	"ELAN0603",
+	"ELAN0604",
+	"ELAN0605",
+	"ELAN0606",
+	"ELAN0607",
+	"ELAN0608",
+	"ELAN0609",
+	"ELAN060B",
+	"ELAN060C",
+	"ELAN060F",
+	"ELAN0610",
+	"ELAN0611",
+	"ELAN0612",
+	"ELAN0613",
+	"ELAN0614",
+	"ELAN0615",
+	"ELAN0616",
+	"ELAN0617",
+	"ELAN0618",
+	"ELAN0619",
+	"ELAN061A",
+	"ELAN061B",
+	"ELAN061C",
+	"ELAN061D",
+	"ELAN061E",
+	"ELAN061F",
+	"ELAN0620",
+	"ELAN0621",
+	"ELAN0622",
+	"ELAN0623",
+	"ELAN0624",
+	"ELAN0625",
+	"ELAN0626",
+	"ELAN0627",
+	"ELAN0628",
+	"ELAN0629",
+	"ELAN062A",
+	"ELAN062B",
+	"ELAN062C",
+	"ELAN062D",
+	"ELAN0631",
+	"ELAN0632",
+	"ELAN1000",
+	"elan,ekth3000",
+	"" /* sentinel */
+};
 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 +1036,16 @@ 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 should be kept in sync with the whitelist
+		 * that exists in that 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(hid_elan_i2c_ignore[i]); ++i)
+				if (!strncmp(hdev->name, hid_elan_i2c_ignore[i],
+					    strlen(hid_elan_i2c_ignore[i])))
+					return true;
 		break;
 	}
 
-- 
2.17.1


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

* [PATCH v5 3/3] arm64: dts: qcom: Add Lenovo Miix 630
  2019-06-06 16:10 [PATCH v5 0/3] Basic DT support for Lenovo Miix 630 Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 1/3] Input: elan_i2c: Add comment about link between elan_i2c and hid-quirks Jeffrey Hugo
  2019-06-06 16:13 ` [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling Jeffrey Hugo
@ 2019-06-06 16:13 ` Jeffrey Hugo
  2 siblings, 0 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-06 16:13 UTC (permalink / raw)
  To: bjorn.andersson, agross, david.brown
  Cc: benjamin.tissoires, dmitry.torokhov, jikos, lee.jones, robh+dt,
	mark.rutland, hdegoede, linux-input, devicetree, linux-arm-msm,
	linux-kernel, 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


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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-06 16:13 ` [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling Jeffrey Hugo
@ 2019-06-12  0:35   ` Dmitry Torokhov
  2019-06-12 14:29     ` Jeffrey Hugo
  2019-06-12 14:45     ` Jiri Kosina
  0 siblings, 2 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2019-06-12  0:35 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: benjamin.tissoires, jikos, bjorn.andersson, lee.jones, robh+dt,
	mark.rutland, agross, david.brown, hdegoede, linux-input,
	devicetree, linux-arm-msm, linux-kernel

On Thu, Jun 06, 2019 at 09:13:22AM -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,
> 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.  The downside is the whitelist and
> blacklist need to be kept in sync.
> 
> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c | 78 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 67 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index e5ca6fe2ca57..edebd0700e3d 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>  	{ }
>  };
>  
> +/* 
> + * List of device names that elan_i2c is handling and HID should ignore.  Must
> + * be kept in sync with elan_i2c
> + */
> +static const char *hid_elan_i2c_ignore[] = {

If this is a copy of elan whitelist, then, if we do not want to bother
with sharing it in object form (as a elan-i2c-ids module), can we at
least move it into include/linux/input/elan-i2c-ids.h and consume from
hid-quirks.c?

Thanks.

-- 
Dmitry

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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12  0:35   ` Dmitry Torokhov
@ 2019-06-12 14:29     ` Jeffrey Hugo
  2019-06-12 14:37       ` Lee Jones
  2019-06-12 14:45     ` Jiri Kosina
  1 sibling, 1 reply; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-12 14:29 UTC (permalink / raw)
  To: Dmitry Torokhov, Jeffrey Hugo
  Cc: benjamin.tissoires, jikos, bjorn.andersson, lee.jones, robh+dt,
	mark.rutland, agross, david.brown, hdegoede, linux-input,
	devicetree, linux-arm-msm, linux-kernel

On 6/11/2019 6:35 PM, Dmitry Torokhov wrote:
> On Thu, Jun 06, 2019 at 09:13:22AM -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,
>> 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.  The downside is the whitelist and
>> blacklist need to be kept in sync.
>>
>> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>> ---
>>   drivers/hid/hid-quirks.c | 78 ++++++++++++++++++++++++++++++++++------
>>   1 file changed, 67 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index e5ca6fe2ca57..edebd0700e3d 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>>   	{ }
>>   };
>>   
>> +/*
>> + * List of device names that elan_i2c is handling and HID should ignore.  Must
>> + * be kept in sync with elan_i2c
>> + */
>> +static const char *hid_elan_i2c_ignore[] = {
> 
> If this is a copy of elan whitelist, then, if we do not want to bother
> with sharing it in object form (as a elan-i2c-ids module), can we at
> least move it into include/linux/input/elan-i2c-ids.h and consume from
> hid-quirks.c?

I can put it in a shared header file, however elan-i2c and hid-quirks
would need to be updated in the same change to prevent a breakage, but
that would seem to violate a concern Benjamin brought up in v4 given
that elan-i2c is maintained in your input tree, and hid-quirks is
maintained in his hid tree.

Are you ok with the elan-i2c changes going through Benjamin's hid tree?


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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12 14:29     ` Jeffrey Hugo
@ 2019-06-12 14:37       ` Lee Jones
  2019-06-12 14:48         ` Jeffrey Hugo
  0 siblings, 1 reply; 12+ messages in thread
From: Lee Jones @ 2019-06-12 14:37 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Dmitry Torokhov, Jeffrey Hugo, benjamin.tissoires, jikos,
	bjorn.andersson, robh+dt, mark.rutland, agross, david.brown,
	hdegoede, linux-input, devicetree, linux-arm-msm, linux-kernel

On Wed, 12 Jun 2019, Jeffrey Hugo wrote:

> On 6/11/2019 6:35 PM, Dmitry Torokhov wrote:
> > On Thu, Jun 06, 2019 at 09:13:22AM -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,
> > > 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.  The downside is the whitelist and
> > > blacklist need to be kept in sync.
> > > 
> > > Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > > ---
> > >   drivers/hid/hid-quirks.c | 78 ++++++++++++++++++++++++++++++++++------
> > >   1 file changed, 67 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > > index e5ca6fe2ca57..edebd0700e3d 100644
> > > --- a/drivers/hid/hid-quirks.c
> > > +++ b/drivers/hid/hid-quirks.c
> > > @@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
> > >   	{ }
> > >   };
> > > +/*
> > > + * List of device names that elan_i2c is handling and HID should ignore.  Must
> > > + * be kept in sync with elan_i2c
> > > + */
> > > +static const char *hid_elan_i2c_ignore[] = {
> > 
> > If this is a copy of elan whitelist, then, if we do not want to bother
> > with sharing it in object form (as a elan-i2c-ids module), can we at
> > least move it into include/linux/input/elan-i2c-ids.h and consume from
> > hid-quirks.c?
> 
> I can put it in a shared header file, however elan-i2c and hid-quirks
> would need to be updated in the same change to prevent a breakage, but
> that would seem to violate a concern Benjamin brought up in v4 given
> that elan-i2c is maintained in your input tree, and hid-quirks is
> maintained in his hid tree.
> 
> Are you ok with the elan-i2c changes going through Benjamin's hid tree?

We co-ordinate cross-subsystem merges all the time.  That is never a
reason to not do the 'right thing (tm)'.  If this information can be
held in a single, central place, without the need for constant
re-alignment, I'm all for it.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12  0:35   ` Dmitry Torokhov
  2019-06-12 14:29     ` Jeffrey Hugo
@ 2019-06-12 14:45     ` Jiri Kosina
  2019-06-12 15:13       ` Dmitry Torokhov
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Kosina @ 2019-06-12 14:45 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jeffrey Hugo, benjamin.tissoires, bjorn.andersson, lee.jones,
	robh+dt, mark.rutland, agross, david.brown, hdegoede,
	linux-input, devicetree, linux-arm-msm, linux-kernel

On Tue, 11 Jun 2019, Dmitry Torokhov wrote:

> > +static const char *hid_elan_i2c_ignore[] = {
> 
> If this is a copy of elan whitelist, then, if we do not want to bother
> with sharing it in object form (as a elan-i2c-ids module), can we at
> least move it into include/linux/input/elan-i2c-ids.h and consume from
> hid-quirks.c?

Let's just not duplicate it in both objects. Why not properly export it 
from hid_quirks?

Thanks,

-- 
Jiri Kosina
SUSE Labs


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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12 14:37       ` Lee Jones
@ 2019-06-12 14:48         ` Jeffrey Hugo
  0 siblings, 0 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-12 14:48 UTC (permalink / raw)
  To: Lee Jones
  Cc: Dmitry Torokhov, Jeffrey Hugo, benjamin.tissoires, jikos,
	bjorn.andersson, robh+dt, mark.rutland, agross, david.brown,
	hdegoede, linux-input, devicetree, linux-arm-msm, linux-kernel

On 6/12/2019 8:37 AM, Lee Jones wrote:
> On Wed, 12 Jun 2019, Jeffrey Hugo wrote:
> 
>> On 6/11/2019 6:35 PM, Dmitry Torokhov wrote:
>>> On Thu, Jun 06, 2019 at 09:13:22AM -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,
>>>> 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.  The downside is the whitelist and
>>>> blacklist need to be kept in sync.
>>>>
>>>> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>>>> ---
>>>>    drivers/hid/hid-quirks.c | 78 ++++++++++++++++++++++++++++++++++------
>>>>    1 file changed, 67 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>>>> index e5ca6fe2ca57..edebd0700e3d 100644
>>>> --- a/drivers/hid/hid-quirks.c
>>>> +++ b/drivers/hid/hid-quirks.c
>>>> @@ -912,8 +912,66 @@ static const struct hid_device_id hid_mouse_ignore_list[] = {
>>>>    	{ }
>>>>    };
>>>> +/*
>>>> + * List of device names that elan_i2c is handling and HID should ignore.  Must
>>>> + * be kept in sync with elan_i2c
>>>> + */
>>>> +static const char *hid_elan_i2c_ignore[] = {
>>>
>>> If this is a copy of elan whitelist, then, if we do not want to bother
>>> with sharing it in object form (as a elan-i2c-ids module), can we at
>>> least move it into include/linux/input/elan-i2c-ids.h and consume from
>>> hid-quirks.c?
>>
>> I can put it in a shared header file, however elan-i2c and hid-quirks
>> would need to be updated in the same change to prevent a breakage, but
>> that would seem to violate a concern Benjamin brought up in v4 given
>> that elan-i2c is maintained in your input tree, and hid-quirks is
>> maintained in his hid tree.
>>
>> Are you ok with the elan-i2c changes going through Benjamin's hid tree?
> 
> We co-ordinate cross-subsystem merges all the time.  That is never a
> reason to not do the 'right thing (tm)'.  If this information can be
> held in a single, central place, without the need for constant
> re-alignment, I'm all for it.
> 

I'm aware coordination is a thing.  I'm trying to elicit some 
coordination between Dmitry and Benjamin.


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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12 14:45     ` Jiri Kosina
@ 2019-06-12 15:13       ` Dmitry Torokhov
  2019-06-12 15:21         ` Benjamin Tissoires
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2019-06-12 15:13 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jeffrey Hugo, Benjamin Tissoires, Bjorn Andersson, Lee Jones,
	Rob Herring, Mark Rutland, agross, David Brown, Hans de Goede,
	linux-input, DTML, open list:ARM/QUALCOMM SUPPORT, lkml

On Wed, Jun 12, 2019 at 7:45 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 11 Jun 2019, Dmitry Torokhov wrote:
>
> > > +static const char *hid_elan_i2c_ignore[] = {
> >
> > If this is a copy of elan whitelist, then, if we do not want to bother
> > with sharing it in object form (as a elan-i2c-ids module), can we at
> > least move it into include/linux/input/elan-i2c-ids.h and consume from
> > hid-quirks.c?
>
> Let's just not duplicate it in both objects. Why not properly export it
> from hid_quirks?

Strictly speaking Elan does not depend on HID; exporting it from
quirks would mean adding this dependency. This also mean that you
can't make Elan built-in while keeping HID as a module (I think this
at least used to be config on some Chromebooks).

Thanks.

-- 
Dmitry

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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12 15:13       ` Dmitry Torokhov
@ 2019-06-12 15:21         ` Benjamin Tissoires
  2019-06-12 15:38           ` Jeffrey Hugo
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Tissoires @ 2019-06-12 15:21 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Jeffrey Hugo, Bjorn Andersson, Lee Jones,
	Rob Herring, Mark Rutland, Andy Gross, David Brown,
	Hans de Goede, linux-input, DTML, open list:ARM/QUALCOMM SUPPORT,
	lkml

On Wed, Jun 12, 2019 at 5:14 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Jun 12, 2019 at 7:45 AM Jiri Kosina <jikos@kernel.org> wrote:
> >
> > On Tue, 11 Jun 2019, Dmitry Torokhov wrote:
> >
> > > > +static const char *hid_elan_i2c_ignore[] = {
> > >
> > > If this is a copy of elan whitelist, then, if we do not want to bother
> > > with sharing it in object form (as a elan-i2c-ids module), can we at
> > > least move it into include/linux/input/elan-i2c-ids.h and consume from
> > > hid-quirks.c?
> >
> > Let's just not duplicate it in both objects. Why not properly export it
> > from hid_quirks?
>
> Strictly speaking Elan does not depend on HID; exporting it from
> quirks would mean adding this dependency. This also mean that you
> can't make Elan built-in while keeping HID as a module (I think this
> at least used to be config on some Chromebooks).
>

I also think it would me things cleaner to have the list of devices in elan_i2c.
If we put the list of devices supported by elan_i2c in a header, and
have HID read this .h file directly, there will be no runtime
dependency.

I am sure we can work something out to remove Jeffrey's fears.

Cheers,
Benjamin

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

* Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling
  2019-06-12 15:21         ` Benjamin Tissoires
@ 2019-06-12 15:38           ` Jeffrey Hugo
  0 siblings, 0 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2019-06-12 15:38 UTC (permalink / raw)
  To: Benjamin Tissoires, Dmitry Torokhov
  Cc: Jiri Kosina, Jeffrey Hugo, Bjorn Andersson, Lee Jones,
	Rob Herring, Mark Rutland, Andy Gross, David Brown,
	Hans de Goede, linux-input, DTML, open list:ARM/QUALCOMM SUPPORT,
	lkml

On 6/12/2019 9:21 AM, Benjamin Tissoires wrote:
> On Wed, Jun 12, 2019 at 5:14 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>>
>> On Wed, Jun 12, 2019 at 7:45 AM Jiri Kosina <jikos@kernel.org> wrote:
>>>
>>> On Tue, 11 Jun 2019, Dmitry Torokhov wrote:
>>>
>>>>> +static const char *hid_elan_i2c_ignore[] = {
>>>>
>>>> If this is a copy of elan whitelist, then, if we do not want to bother
>>>> with sharing it in object form (as a elan-i2c-ids module), can we at
>>>> least move it into include/linux/input/elan-i2c-ids.h and consume from
>>>> hid-quirks.c?
>>>
>>> Let's just not duplicate it in both objects. Why not properly export it
>>> from hid_quirks?
>>
>> Strictly speaking Elan does not depend on HID; exporting it from
>> quirks would mean adding this dependency. This also mean that you
>> can't make Elan built-in while keeping HID as a module (I think this
>> at least used to be config on some Chromebooks).
>>
> 
> I also think it would me things cleaner to have the list of devices in elan_i2c.
> If we put the list of devices supported by elan_i2c in a header, and
> have HID read this .h file directly, there will be no runtime
> dependency.
> 
> I am sure we can work something out to remove Jeffrey's fears.

Works for me.  I plan to have the next rev posted by the end of the week.


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

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 16:10 [PATCH v5 0/3] Basic DT support for Lenovo Miix 630 Jeffrey Hugo
2019-06-06 16:13 ` [PATCH v5 1/3] Input: elan_i2c: Add comment about link between elan_i2c and hid-quirks Jeffrey Hugo
2019-06-06 16:13 ` [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling Jeffrey Hugo
2019-06-12  0:35   ` Dmitry Torokhov
2019-06-12 14:29     ` Jeffrey Hugo
2019-06-12 14:37       ` Lee Jones
2019-06-12 14:48         ` Jeffrey Hugo
2019-06-12 14:45     ` Jiri Kosina
2019-06-12 15:13       ` Dmitry Torokhov
2019-06-12 15:21         ` Benjamin Tissoires
2019-06-12 15:38           ` Jeffrey Hugo
2019-06-06 16:13 ` [PATCH v5 3/3] arm64: dts: qcom: Add Lenovo Miix 630 Jeffrey Hugo

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org linux-arm-msm@archiver.kernel.org
	public-inbox-index linux-arm-msm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox