diff mbox

hid: Fix Speedlink VAD Cezanne support for some devices

Message ID 51F7979C.4030306@stefankriwanek.de
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

Stefan Kriwanek July 30, 2013, 10:38 a.m. UTC
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>

---



--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jiri Kosina July 30, 2013, 8:59 p.m. UTC | #1
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,
diff mbox

Patch

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