[PATCH] hid: Fix Speedlink VAD Cezanne support for some devices

[PATCH] hid: Fix Speedlink VAD Cezanne support for some devices

[Stefan Kriwanek]

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 <mail@stefankriwanek.de>

---


--- linux-source-3.5.0/drivers/hid/hid-speedlink.c	2012-07-21 22:58:29.000000000 +0200
+++ linux-source-3.5.0-speedlink/drivers/hid/hid-speedlink.c	2013-07-30 12:24:22.113302655 +0200
@@ -3,7 +3,7 @@
  *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
  *  the HID descriptor.
  *
- *  Copyright (c) 2011 Stefan Kriwanek <mail@stefankriwanek.de>
+ *  Copyright (c) 2011, 2013 Stefan Kriwanek <dev@stefankriwanek.de>
  */
 
 /*
@@ -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)

Re: [PATCH] hid: Fix Speedlink VAD Cezanne support for some devices

[Jiri Kosina]

On Tue, 30 Jul 2013, Stefan Kriwanek wrote:

> 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).

Hi Stefan,

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

> 
> Signed-off-by: Stefan Kriwanek <mail@stefankriwanek.de>
> 
> ---
> 
> 
> --- linux-source-3.5.0/drivers/hid/hid-speedlink.c	2012-07-21 22:58:29.000000000 +0200
> +++ linux-source-3.5.0-speedlink/drivers/hid/hid-speedlink.c	2013-07-30 12:24:22.113302655 +0200
> @@ -3,7 +3,7 @@
>   *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
>   *  the HID descriptor.
>   *
> - *  Copyright (c) 2011 Stefan Kriwanek <mail@stefankriwanek.de>
> + *  Copyright (c) 2011, 2013 Stefan Kriwanek <dev@stefankriwanek.de>
>   */
>  
>  /*
> @@ -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,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] hid: Fix Speedlink VAD Cezanne support for some devices

[Stefan Kriwanek]

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 <mail@stefankriwanek.de>

---

--- linux-source-3.5.0/drivers/hid/hid-speedlink.c
+++ linux-source-3.5.0-speedlink/drivers/hid/hid-speedlink.c
@@ -3,7 +3,7 @@
  *  Fixes "jumpy" cursor and removes nonexistent keyboard LEDS from
  *  the HID descriptor.
  *
- *  Copyright (c) 2011 Stefan Kriwanek <mail@stefankriwanek.de>
+ *  Copyright (c) 2011, 2013 Stefan Kriwanek <dev@stefankriwanek.de>
  */
 
 /*
@@ -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)

Re: [PATCH] hid: Fix Speedlink VAD Cezanne support for some devices

[Jiri Kosina]

On Sun, 25 Aug 2013, Stefan Kriwanek wrote:

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

Applied, thanks Stefan.

-- 
Jiri Kosina
SUSE Labs