hid: Fix Speedlink VAD Cezanne support for some devices

PATCH Applied Stefan Kriwanek 2 participants 4 messages Jul 30, 2013 – Aug 26, 2013 HID

lore

Share & tools

lore

Stefan Kriwanek

author

2013-07-30 10:56 UTC #0 lore raw

PATCH 1/1 hid: Fix Speedlink VAD Cezanne support for some devices

Hello,

I obtained a mouse device "Speedlink VAD Cezanne" that needs more aggressive fixing than already done in the driver. It, however, reports itself the same as the model hid-speedlink.c was written for.

The patch applies to any kernel from 3.1 to current 3.11-rc3, since the file hasn't ever been touched since then.

Please also note that I made sure through testing that this patch would not interfere with the proper working of a device that is bug-free: The driver drops EV_REL events with abs(val) >= 256, which are not achievable even on the highest laser resolution hardware setting. Hence I believe it is safe to also apply to the older kernels (that is, if your policy does allow that).

Signed-off-by Stefan Kriwanek <[email protected]>

---

drivers/hid/hid-speedlink.c +2 -2

@@ -3,7 +3,7 @@
  *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
  *  the HID descriptor.
  *
- *  Copyright (c) 2011 Stefan Kriwanek <[email protected]>
+ *  Copyright (c) 2011, 2013 Stefan Kriwanek <[email protected]>
  */
 
 /*

@@ -49,7 +49,7 @@ static int speedlink_event(struct hid_de
 {
 	/* No other conditions due to usage_table. */
 	/* Fix "jumpy" cursor (invalid events sent by device). */
-	if (value == 256)
+	if (abs(value) >= 256)
 		return 1;
 	/* Drop useless distance 0 events (on button clicks etc.) as well */
 	if (value == 0)

Jiri Kosina

2013-07-30 20:59 UTC #1 lore raw

Quoted 7 lines · Stefan Kriwanek

I obtained a mouse device "Speedlink VAD Cezanne" that needs more aggressive fixing than already done in the driver. It, however, reports itself the same as the model hid-speedlink.c was written for.
The patch applies to any kernel from 3.1 to current 3.11-rc3, since the file hasn't ever been touched since then.

Quoted 6 lines

Please also note that I made sure through testing that this patch would not interfere with the proper working of a device that is bug-free: The driver drops EV_REL events with abs(val) >= 256, which are not achievable even on the highest laser resolution hardware setting. Hence I believe it is safe to also apply to the older kernels (that is, if your policy does allow that).

Hi Stefan,

thanks for the debugging effort and the patch. One minor thing ...

Quoted 18 lines

Signed-off-by Stefan Kriwanek <[email protected]>

---

drivers/hid/hid-speedlink.c +1 -1

@@ -3,7 +3,7 @@
  *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
  *  the HID descriptor.
  *
- *  Copyright (c) 2011 Stefan Kriwanek <[email protected]>
+ *  Copyright (c) 2011, 2013 Stefan Kriwanek <[email protected]>
  */
 
 /*

drivers/hid/hid-speedlink.c static int speedlink_event(struct hid_de · line 52

Quoted 5 lines

drivers/hid/hid-speedlink.c +1 -1

@@ -49,7 +49,7 @@ static int speedlink_event(struct hid_de
 {
 	/* No other conditions due to usage_table. */
 	/* Fix "jumpy" cursor (invalid events sent by device). */
-	if (value == 256)
+	if (abs(value) >= 256)

I think it'd be valuable to have brief description of why the more strict condition is necessary (and correct at the same time) also in the code comment preceeding the if(), not just in the patch changelog.

Could you please refresh your patch and resend?

Thanks,

Signature · 2 lines

Jiri Kosina
SUSE Labs

Stefan Kriwanek

author

2013-08-25 08:46 UTC #2 lore raw

Some devices of the "Speedlink VAD Cezanne" model need more aggressive fixing than already done.

I made sure through testing that this patch would not interfere with the proper working of a device that is bug-free. (The driver drops EV_REL events with abs(val) >= 256, which are not achievable even on the highest laser resolution hardware setting.) Hence, I believe it is safe to also apply to older kernels (that is, if your policy allows).

The patch applies to any kernel from 3.1 to current 3.11-rc6.

Signed-off-by Stefan Kriwanek <[email protected]>

---

drivers/hid/hid-speedlink.c +8 -3

@@ -3,7 +3,7 @@
  *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
  *  the HID descriptor.
  *
- *  Copyright (c) 2011 Stefan Kriwanek <[email protected]>
+ *  Copyright (c) 2011, 2013 Stefan Kriwanek <[email protected]>
  */
 
 /*

@@ -48,8 +48,13 @@ static int speedlink_event(struct hid_de
 		struct hid_usage *usage, __s32 value)
 {
 	/* No other conditions due to usage_table. */
-	/* Fix "jumpy" cursor (invalid events sent by device). */
-	if (value == 256)
+
+	/* This fixes the "jumpy" cursor occuring due to invalid events sent
+	 * by the device. Some devices only send them with value==+256, others
+	 * don't. However, catching abs(value)>=256 is restrictive enough not
+	 * to interfere with devices that were bug-free (has been tested).
+	 */
+	if (abs(value) >= 256)
 		return 1;
 	/* Drop useless distance 0 events (on button clicks etc.) as well */
 	if (value == 0)

Jiri Kosina

Applied

2013-08-26 11:51 UTC #3 lore raw

Quoted 1 line · Stefan Kriwanek

Some devices of the "Speedlink VAD Cezanne" model need more aggressive fixing than already done.

Applied, thanks Stefan.

Signature · 2 lines

Jiri Kosina
SUSE Labs