Re: [PATCH v5 2/3] HID: quirks: Refactor ELAN 400 and 401 handling

From: Jeffrey Hugo
Date: Wed Jun 12 2019 - 10:53:18 EST


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@xxxxxxxxxx>
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@xxxxxxxxx>
---
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.