[PATCH] HID: rmi: do not handle touchscreens through hid-rmi

[PATCH] HID: rmi: do not handle touchscreens through hid-rmi

[Benjamin Tissoires]

Currently, hid-rmi drives every Synaptics product, but the touchscreens
on the Windows tablets should be handled through hid-multitouch.

Instead of providing a long list of PIDs, rely on the scan_report
capability to detect which should go to hid-multitouch, and which
should not go to hid-rmi.

We introduce a generic HID_GROUP_HAVE_SPECIAL_DRIVER which can be reused
amoung other drivers if they want to have a catch rule but still
have multitouch devices handled through hid-multitouch.

related bug:
https://bugzilla.kernel.org/show_bug.cgi?id=74241
https://bugzilla.redhat.com/show_bug.cgi?id=1089583

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Hi Jiri,

well, this is the patch we are going to carry in Fedora as we are already
carrying hid-rmi.
Still, I am not 100% happy with it, especially the HID_GROUP_HAVE_SPECIAL_DRIVER
thing.

I would also say we should re-think the way the scanning is called in
hid_add_device because I am starting using more and more the scanning capability
to decide if the device should be handled by generic, a special driver or simply
ignored.

Anyway, this could be decided later. If you prefer me to use a hid-rmi specific
group like HID_GROUP_RMI instead of HID_GROUP_HAVE_SPECIAL_DRIVER, I can also
update the patch.

Cheers,
Benjamin

 drivers/hid/hid-core.c | 10 ++++++++--
 drivers/hid/hid-rmi.c  |  6 ++++--
 include/linux/hid.h    | 13 +++++++++++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index f05255d..46b27f3 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -776,6 +776,14 @@ static int hid_scan_report(struct hid_device *hid)
 	    (hid->group == HID_GROUP_MULTITOUCH))
 		hid->group = HID_GROUP_MULTITOUCH_WIN_8;
 
+	/*
+	* Vendor specific handlings
+	*/
+	if ((hid->vendor == USB_VENDOR_ID_SYNAPTICS) &&
+	    (hid->group == HID_GROUP_GENERIC))
+		/* hid-rmi should take care of them, not hid-generic */
+		hid->group = HID_GROUP_HAVE_SPECIAL_DRIVER;
+
 	vfree(parser);
 	return 0;
 }
@@ -1882,8 +1890,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
-	{ HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_THINGM, USB_DEVICE_ID_BLINK1) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 7da9509..3980955 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -861,8 +861,10 @@ static void rmi_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id rmi_id[] = {
-	{ HID_I2C_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
+	{ HID_DEVICE(BUS_I2C, HID_GROUP_HAVE_SPECIAL_DRIVER,
+		USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
+	{ HID_DEVICE(BUS_USB, HID_GROUP_HAVE_SPECIAL_DRIVER,
+		USB_VENDOR_ID_SYNAPTICS, HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, rmi_id);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 54f855b..888acfd 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -304,6 +304,19 @@ struct hid_item {
 #define HID_GROUP_MULTITOUCH			0x0002
 #define HID_GROUP_SENSOR_HUB			0x0003
 #define HID_GROUP_MULTITOUCH_WIN_8		0x0004
+#define HID_GROUP_HAVE_SPECIAL_DRIVER		0xffff
+/*
+ * HID_GROUP_HAVE_SPECIAL_DRIVER should be used when the device needs to be
+ * scanned in case it is handled by either hid-multitouch, hid-generic,
+ * hid-sensor-hub or any other generic hid driver.
+ *
+ * Devices declared in hid_have_special_driver[] in hid-core.c can use
+ * HID_GROUP_ANY instead because there will be not overlap between their
+ * specific driver and a generic one.
+ *
+ * Note: HID_GROUP_ANY is declared in linux/mod_devicetable.h
+ * and has a value of 0x0000
+ */
 
 /*
  * This is the global environment of the parser. This information is
-- 
1.9.0

Re: [PATCH] HID: rmi: do not handle touchscreens through hid-rmi

[Benjamin Tissoires]

On Wed, Apr 23, 2014 at 10:31 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Currently, hid-rmi drives every Synaptics product, but the touchscreens
> on the Windows tablets should be handled through hid-multitouch.
>
> Instead of providing a long list of PIDs, rely on the scan_report
> capability to detect which should go to hid-multitouch, and which
> should not go to hid-rmi.
>
> We introduce a generic HID_GROUP_HAVE_SPECIAL_DRIVER which can be reused
> amoung other drivers if they want to have a catch rule but still
> have multitouch devices handled through hid-multitouch.
>
> related bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=74241
> https://bugzilla.redhat.com/show_bug.cgi?id=1089583
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Hi Jiri,
>
> well, this is the patch we are going to carry in Fedora as we are already
> carrying hid-rmi.
> Still, I am not 100% happy with it, especially the HID_GROUP_HAVE_SPECIAL_DRIVER
> thing.
>
> I would also say we should re-think the way the scanning is called in
> hid_add_device because I am starting using more and more the scanning capability
> to decide if the device should be handled by generic, a special driver or simply
> ignored.
>
> Anyway, this could be decided later. If you prefer me to use a hid-rmi specific
> group like HID_GROUP_RMI instead of HID_GROUP_HAVE_SPECIAL_DRIVER, I can also
> update the patch.
>

Hi Jiri,

looks like this one did not attracted your eye yet. Can you ack / nack it?
Without it, hid-rmi will break all the multitouch screens from
Synaptics, which will be a little bit worrying :)

Cheers,
Benjamin

Re: [PATCH] HID: rmi: do not handle touchscreens through hid-rmi

[Jiri Kosina]

On Wed, 23 Apr 2014, Benjamin Tissoires wrote:

> Currently, hid-rmi drives every Synaptics product, but the touchscreens
> on the Windows tablets should be handled through hid-multitouch.
> 
> Instead of providing a long list of PIDs, rely on the scan_report
> capability to detect which should go to hid-multitouch, and which
> should not go to hid-rmi.
> 
> We introduce a generic HID_GROUP_HAVE_SPECIAL_DRIVER which can be reused
> amoung other drivers if they want to have a catch rule but still
> have multitouch devices handled through hid-multitouch.
> 
> related bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=74241
> https://bugzilla.redhat.com/show_bug.cgi?id=1089583
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Hi Benjamin,

sorry for not responding sooner.

Yes, I have to say I find HID_GROUP_HAVE_SPECIAL_DRIVER very disgusting. 
Especially its (non-)relation to hid_have_special_driver[] is very 
confusing.

If you could update this to be RMI-specific (i.e. create a special 
HID_GROUP for RMI devices), I'll merge it right away.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: rmi: do not handle touchscreens through hid-rmi

[Benjamin Tissoires]

On May 20 2014 or thereabouts, Jiri Kosina wrote:
> On Wed, 23 Apr 2014, Benjamin Tissoires wrote:
> 
> > Currently, hid-rmi drives every Synaptics product, but the touchscreens
> > on the Windows tablets should be handled through hid-multitouch.
> > 
> > Instead of providing a long list of PIDs, rely on the scan_report
> > capability to detect which should go to hid-multitouch, and which
> > should not go to hid-rmi.
> > 
> > We introduce a generic HID_GROUP_HAVE_SPECIAL_DRIVER which can be reused
> > amoung other drivers if they want to have a catch rule but still
> > have multitouch devices handled through hid-multitouch.
> > 
> > related bug:
> > https://bugzilla.kernel.org/show_bug.cgi?id=74241
> > https://bugzilla.redhat.com/show_bug.cgi?id=1089583
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Hi Benjamin,
> 
> sorry for not responding sooner.

It's not a big deal. Actually, the Canonical folks reminded me the state
of this patch that I completely forgot too :)

> 
> Yes, I have to say I find HID_GROUP_HAVE_SPECIAL_DRIVER very disgusting. 
> Especially its (non-)relation to hid_have_special_driver[] is very 
> confusing.
> 
> If you could update this to be RMI-specific (i.e. create a special 
> HID_GROUP for RMI devices), I'll merge it right away.

Ok, I'll add this to the pile of things to do today.

Cheers,
Benjamin