On 6/13/2019 2:55 AM, Benjamin Tissoires wrote:

On Thu, Jun 13, 2019 at 12:20 AM Jeffrey Hugo [off-list ref] wrote:

quoted

On 6/12/2019 3:46 PM, Dmitry Torokhov wrote:

quoted

On Wed, Jun 12, 2019 at 02:27:21PM -0700, Jeffrey Hugo wrote:

quoted

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 <redacted>
Signed-off-by: Jeffrey Hugo <redacted>
---
   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 <redacted>

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.