Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Gene Heskett]

On Friday 17 December 2004 21:12, Mikkel Krautz wrote:
>Hi!
>
>This patch adds the option "USB HID Mouse Interrupt Polling
> Interval" to drivers/usb/input/Kconfig, and a few lines of code to
>drivers/usb/input/hid-core.c, to make the config option function.
>
>It allows people to change the interval, at which their USB HID mice
>are polled at. This is extremely useful for people who require high
>precision, or just likes the feeling of a very precise mouse. ;)
>
>As the Kconfig help implies, setting a lower polling interval is
> known to work on several mice produced by Logitech and Microsoft. I
> only have a Logitech MX500 to test it on. My results have been
> positive, and so have many other people's.
>
>Mikkel Krautz
>
Mikkel, could you please turn off the word wrap in your MTA's
composer and repost this?  I'd like to try it.

>
>
>Signed-off-by: Mikkel Krautz <krautz@gmail.com>
>---
>
>--- clean/drivers/usb/input/Kconfig
>+++ dirty/drivers/usb/input/Kconfig
>@@ -24,6 +24,38 @@
> 	  To compile this driver as a module, choose M here: the
> 	  module will be called usbhid.

[...]

-- 
Cheers, Gene
"There are four boxes to be used in defense of liberty:
 soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
99.30% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com attorneys please note, additions to this message
by Gene Heskett are:
Copyright 2004 by Maurice Eugene Heskett, all rights reserved.

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Greg KH]

On Sat, Dec 18, 2004 at 02:12:50AM +0000, Mikkel Krautz wrote:
> Hi!
> 
> This patch adds the option "USB HID Mouse Interrupt Polling Interval"
> to drivers/usb/input/Kconfig, and a few lines of code to
> drivers/usb/input/hid-core.c, to make the config option function.
> 
> It allows people to change the interval, at which their USB HID mice
> are polled at. This is extremely useful for people who require high
> precision, or just likes the feeling of a very precise mouse. ;)
> 
> As the Kconfig help implies, setting a lower polling interval is known
> to work on several mice produced by Logitech and Microsoft. I only
> have a Logitech MX500 to test it on. My results have been positive,
> and so have many other people's.

Why not just make it a sysfs file, so you can tune it per device?  That
way you also don't have to make it a Kconfig option.

thanks,

greg k-h

[PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Hi!

This patch adds the option "USB HID Mouse Interrupt Polling Interval"
to drivers/usb/input/Kconfig, and a few lines of code to
drivers/usb/input/hid-core.c, to make the config option function.

It allows people to change the interval, at which their USB HID mice
are polled at. This is extremely useful for people who require high
precision, or just likes the feeling of a very precise mouse. ;)

As the Kconfig help implies, setting a lower polling interval is known
to work on several mice produced by Logitech and Microsoft. I only
have a Logitech MX500 to test it on. My results have been positive,
and so have many other people's.

Mikkel Krautz




Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---

--- clean/drivers/usb/input/Kconfig
+++ dirty/drivers/usb/input/Kconfig
@@ -24,6 +24,38 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called usbhid.
 
+config USB_HID_MOUSE_POLLING_INTERVAL
+	int "USB HID Mouse Interrupt Polling Interval"
+	default 10
+	depends on USB_HID
+	help
+	  The "USB HID Mouse Interrupt Polling Interval" is the interval, at
+	  which your USB HID mouse is to be polled at. The interval is
+	  specified in miliseconds.
+
+	  Decreasing the interval will, of course, give you a much more
+	  precise mouse.
+
+	  Generally speaking, a polling interval of 2 ms should be more than
+	  enough for most people, and is great for gaming and other things
+	  that require high precision.
+
+	  An interval lower than the default is not guaranteed work on your
+	  specific piece of hardware. If you want to play it safe, don't
+	  change this value.
+
+	  Now, if you indeed want to feel the joy of a precise mouse, the
+	  following mice are known to work without problems, when the interval
+	  is set to at least 2 ms:
+
+	    * Logitech's MX-family
+	    * Logitech Mouse Man Dual Optical
+	    * Logitech iFeel
+	    * Microsoft Intellimouse Explorer
+	    * Microsoft Intellimouse Optical 1.1
+
+	  If unsure, keep it at 10 ms.
+
 comment "Input core support is needed for USB HID input layer or HIDBP
support"
 	depends on USB_HID && INPUT=n
 
--- clean/drivers/usb/input/hid-core.c
+++ dirty/drivers/usb/input/hid-core.c
@@ -37,7 +37,7 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
@@ -1663,6 +1663,12 @@
 		if ((endpoint->bmAttributes & 3) != 3)		/* Not an interrupt endpoint
*/
 			continue;
 
+		/* Set the interrupt polling interval of mice, to the one specified
in the config. */
+		if (hid->collection->usage == HID_GD_MOUSE
+				&& CONFIG_USB_HID_MOUSE_POLLING_INTERVAL > 0
+				&& CONFIG_USB_HID_MOUSE_POLLING_INTERVAL < 255)
+			endpoint->bInterval = CONFIG_USB_HID_MOUSE_POLLING_INTERVAL;
+
 		/* handle potential highspeed HID correctly */
 		interval = endpoint->bInterval;
 		if (dev->speed == USB_SPEED_HIGH)

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Greg KH]

On Sat, Dec 18, 2004 at 04:42:46AM +0000, Mikkel Krautz wrote:
> Greg KH wrote:
> >On Sat, Dec 18, 2004 at 02:12:50AM +0000, Mikkel Krautz wrote:
> >>
> >>This patch adds the option "USB HID Mouse Interrupt Polling Interval"
> >>to drivers/usb/input/Kconfig, and a few lines of code to
> >>drivers/usb/input/hid-core.c, to make the config option function.
> >>
> >>It allows people to change the interval, at which their USB HID mice
> >>are polled at. This is extremely useful for people who require high
> >>precision, or just likes the feeling of a very precise mouse. ;)
> >>
> >>As the Kconfig help implies, setting a lower polling interval is known
> >>to work on several mice produced by Logitech and Microsoft. I only
> >>have a Logitech MX500 to test it on. My results have been positive,
> >>and so have many other people's.
> >
> >Why not just make it a sysfs file, so you can tune it per device?  That
> >way you also don't have to make it a Kconfig option.
> >
> I'm not too familiar with sysfs, so I really don't know.

Poke around in it, I think it's the natural place for stuff like this.

> The interval is set when the device is configured - that's only once.

So it can never change?  Why not add that feature at the same time?

> Therefore I think a static value in Kconfig is fine. Wouldn't a sysfs 
> entry be a little overkill for this?

What about makeing it a module paramater then, that is exported to
sysfs?  That makes it easier to adjust on the fly (before the mouse is
inserted), and doesn't require the kernel to be rebuilt.

Just trying to make things easier for users :)

thanks,

greg k-h

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Gene Heskett]

On Friday 17 December 2004 23:22, Mikkel Krautz wrote:
>On Fri, 17 Dec 2004 19:43:23 -0500, Gene Heskett
>
><gene.heskett@verizon.net> wrote:
> > Mikkel, could you please turn off the word wrap in your MTA's
> > composer and repost this? I'd like to try it.
>
>Sorry about that.
>Here's a repost. Now even with a diffstat:
>
Looks good, saved  it, thanks.

-- 
Cheers, Gene
"There are four boxes to be used in defense of liberty:
 soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
99.30% setiathome rank, not too shabby for a WV hillbilly
Yahoo.com attorneys please note, additions to this message
by Gene Heskett are:
Copyright 2004 by Maurice Eugene Heskett, all rights reserved.

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

On Fri, 17 Dec 2004 19:43:23 -0500, Gene Heskett 
<gene.heskett@verizon.net> wrote:
 > Mikkel, could you please turn off the word wrap in your MTA's
 > composer and repost this? I'd like to try it.

Sorry about that.
Here's a repost. Now even with a diffstat:




Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---

Kconfig | 32 ++++++++++++++++++++++++++++++++
hid-core.c | 8 +++++++-
2 files changed, 39 insertions(+), 1 deletion(-)


--- clean/drivers/usb/input/Kconfig
+++ dirty/drivers/usb/input/Kconfig
@@ -24,6 +24,38 @@
To compile this driver as a module, choose M here: the
module will be called usbhid.

+config USB_HID_MOUSE_POLLING_INTERVAL
+ int "USB HID Mouse Interrupt Polling Interval"
+ default 10
+ depends on USB_HID
+ help
+ The "USB HID Mouse Interrupt Polling Interval" is the interval, at
+ which your USB HID mouse is to be polled at. The interval is
+ specified in miliseconds.
+
+ Decreasing the interval will, of course, give you a much more
+ precise mouse.
+
+ Generally speaking, a polling interval of 2 ms should be more than
+ enough for most people, and is great for gaming and other things
+ that require high precision.
+
+ An interval lower than the default is not guaranteed work on your
+ specific piece of hardware. If you want to play it safe, don't
+ change this value.
+
+ Now, if you indeed want to feel the joy of a precise mouse, the
+ following mice are known to work without problems, when the interval
+ is set to at least 2 ms:
+
+ * Logitech's MX-family
+ * Logitech Mouse Man Dual Optical
+ * Logitech iFeel
+ * Microsoft Intellimouse Explorer
+ * Microsoft Intellimouse Optical 1.1
+
+ If unsure, keep it at 10 ms.
+
comment "Input core support is needed for USB HID input layer or HIDBP 
support"
depends on USB_HID && INPUT=n

--- clean/drivers/usb/input/hid-core.c
+++ dirty/drivers/usb/input/hid-core.c
@@ -37,7 +37,7 @@
* Version Information
*/

-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
#define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
#define DRIVER_DESC "USB HID core driver"
#define DRIVER_LICENSE "GPL"
@@ -1663,6 +1663,12 @@
if ((endpoint->bmAttributes & 3) != 3) /* Not an interrupt endpoint */
continue;

+ /* Set the interrupt polling interval of mice, to the one specified in 
the config. */
+ if (hid->collection->usage == HID_GD_MOUSE
+ && CONFIG_USB_HID_MOUSE_POLLING_INTERVAL > 0
+ && CONFIG_USB_HID_MOUSE_POLLING_INTERVAL < 255)
+ endpoint->bInterval = CONFIG_USB_HID_MOUSE_POLLING_INTERVAL;
+
/* handle potential highspeed HID correctly */
interval = endpoint->bInterval;
if (dev->speed == USB_SPEED_HIGH)

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Greg KH wrote:

>On Sat, Dec 18, 2004 at 02:12:50AM +0000, Mikkel Krautz wrote:
>  
>
>>Hi!
>>
>>This patch adds the option "USB HID Mouse Interrupt Polling Interval"
>>to drivers/usb/input/Kconfig, and a few lines of code to
>>drivers/usb/input/hid-core.c, to make the config option function.
>>
>>It allows people to change the interval, at which their USB HID mice
>>are polled at. This is extremely useful for people who require high
>>precision, or just likes the feeling of a very precise mouse. ;)
>>
>>As the Kconfig help implies, setting a lower polling interval is known
>>to work on several mice produced by Logitech and Microsoft. I only
>>have a Logitech MX500 to test it on. My results have been positive,
>>and so have many other people's.
>>    
>>
>
>Why not just make it a sysfs file, so you can tune it per device?  That
>way you also don't have to make it a Kconfig option.
>
>thanks,
>
>greg k-h
>
>  
>
I'm not too familiar with sysfs, so I really don't know.

The interval is set when the device is configured - that's only once.

Therefore I think a static value in Kconfig is fine. Wouldn't a sysfs 
entry be a little overkill for this?


Mikkel Krautz

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

*Shrug*

Ugh stupid me. This just doesn't seem to be my day.

I apologise.

Let's hope this is the last time:




Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---

Kconfig | 32 ++++++++++++++++++++++++++++++++
hid-core.c | 8 +++++++-
2 files changed, 39 insertions(+), 1 deletion(-)


--- clean/drivers/usb/input/Kconfig
+++ dirty/drivers/usb/input/Kconfig
@@ -24,6 +24,38 @@
       To compile this driver as a module, choose M here: the
       module will be called usbhid.
 
+config USB_HID_MOUSE_POLLING_INTERVAL
+    int "USB HID Mouse Interrupt Polling Interval"
+    default 10
+    depends on USB_HID
+    help
+      The "USB HID Mouse Interrupt Polling Interval" is the interval, at
+      which your USB HID mouse is to be polled at. The interval is
+      specified in miliseconds.
+
+      Decreasing the interval will, of course, give you a much more
+      precise mouse.
+
+      Generally speaking, a polling interval of 2 ms should be more than
+      enough for most people, and is great for gaming and other things
+      that require high precision.
+
+      An interval lower than the default is not guaranteed work on your
+      specific piece of hardware. If you want to play it safe, don't
+      change this value.
+
+      Now, if you indeed want to feel the joy of a precise mouse, the
+      following mice are known to work without problems, when the interval
+      is set to at least 2 ms:
+
+        * Logitech's MX-family
+        * Logitech Mouse Man Dual Optical
+        * Logitech iFeel
+        * Microsoft Intellimouse Explorer
+        * Microsoft Intellimouse Optical 1.1
+
+      If unsure, keep it at 10 ms.
+
 comment "Input core support is needed for USB HID input layer or HIDBP 
support"
     depends on USB_HID && INPUT=n
 
--- clean/drivers/usb/input/hid-core.c
+++ dirty/drivers/usb/input/hid-core.c
@@ -37,7 +37,7 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
@@ -1663,6 +1663,12 @@
         if ((endpoint->bmAttributes & 3) != 3)        /* Not an 
interrupt endpoint */
             continue;
 
+        /* Set the interrupt polling interval of mice, to the one 
specified in the config. */
+        if (hid->collection->usage == HID_GD_MOUSE
+                && CONFIG_USB_HID_MOUSE_POLLING_INTERVAL > 0
+                && CONFIG_USB_HID_MOUSE_POLLING_INTERVAL < 255)
+            endpoint->bInterval = CONFIG_USB_HID_MOUSE_POLLING_INTERVAL;
+
         /* handle potential highspeed HID correctly */
         interval = endpoint->bInterval;
         if (dev->speed == USB_SPEED_HIGH)

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Marcel Holtmann]

Hi Mikkel,

> @@ -1910,6 +1916,7 @@
>  
>  module_init(hid_init);
>  module_exit(hid_exit);
> +module_param(hid_mouse_polling_interval, int, 644);

I think the use of module_param_named() makes more sense here.

Regards

Marcel

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Greg KH]

On Sat, Dec 18, 2004 at 05:39:25PM +0000, Mikkel Krautz wrote:
> On Fri, 17 Dec 2004 18:59:48 -0800, Greg KH <greg@kroah.com> wrote:
> > What about makeing it a module paramater then, that is exported to
> > sysfs?  That makes it easier to adjust on the fly (before the mouse is
> > inserted), and doesn't require the kernel to be rebuilt.
> 
> I really like the idea. I'm start to think that this is the ideal way to 
> accomplish this.
> 
> Here's a new patch. Let's hope it doesn't wrap!

It was eaten :(

> module_init(hid_init);
> module_exit(hid_exit);
> +module_param(hid_mouse_polling_interval, int, 644);

0644, or use the proper #defines instead.

thanks,

greg k-h

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

On Fri, 17 Dec 2004 18:59:48 -0800, Greg KH <greg@kroah.com> wrote:
 > What about makeing it a module paramater then, that is exported to
 > sysfs?  That makes it easier to adjust on the fly (before the mouse is
 > inserted), and doesn't require the kernel to be rebuilt.

I really like the idea. I'm start to think that this is the ideal way to 
accomplish this.

Here's a new patch. Let's hope it doesn't wrap!




Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---


 hid-core.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletion(-)



--- dirty/drivers/usb/input/hid-core.c
+++ clean/drivers/usb/input/hid-core.c
@@ -37,11 +37,12 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
 
+static unsigned int hid_mouse_polling_interval;
 static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", 
"Joystick",
                 "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
 
@@ -1663,6 +1664,11 @@
         if ((endpoint->bmAttributes & 3) != 3)        /* Not an 
interrupt endpoint */
             continue;
 
+        /* Change the polling interval of mice. */
+        if (hid->collection->usage == HID_GD_MOUSE
+                && hid_mouse_polling_interval > 0)
+            endpoint->bInterval = hid_mouse_polling_interval;
+       
         /* handle potential highspeed HID correctly */
         interval = endpoint->bInterval;
         if (dev->speed == USB_SPEED_HIGH)
@@ -1910,6 +1916,7 @@
 
 module_init(hid_init);
 module_exit(hid_exit);
+module_param(hid_mouse_polling_interval, int, 644);
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);

 

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

On Sat, 18 Dec 2004 16:40:44 +0100, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Mikkel,
> 
> > @@ -1910,6 +1916,7 @@
> >
> >  module_init(hid_init);
> >  module_exit(hid_exit);
> > +module_param(hid_mouse_polling_interval, int, 644);
> 
> I think the use of module_param_named() makes more sense here.
> 
> Regards
> 
> Marcel
> 
> 

Thanks, I'll change it. Do you think I should add a MODULE_PARM_DESC too?

Mikkel

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

On Sat, 18 Dec 2004 08:53:31 -0800, Greg KH <greg@kroah.com> wrote:
> On Sat, Dec 18, 2004 at 05:39:25PM +0000, Mikkel Krautz wrote:
> > On Fri, 17 Dec 2004 18:59:48 -0800, Greg KH <greg@kroah.com> wrote:
> > > What about makeing it a module paramater then, that is exported to
> > > sysfs?  That makes it easier to adjust on the fly (before the mouse is
> > > inserted), and doesn't require the kernel to be rebuilt.
> >
> > I really like the idea. I'm start to think that this is the ideal way to
> > accomplish this.
> >
> > Here's a new patch. Let's hope it doesn't wrap!
> 
> It was eaten :(
> 
> > module_init(hid_init);
> > module_exit(hid_exit);
> > +module_param(hid_mouse_polling_interval, int, 644);
> 
> 0644, or use the proper #defines instead.
> 
> thanks,
> 
> greg k-h
> 

Here's an updated version, with your and Marcel's suggestions:





Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---


 hid-core.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletion(-)


--- clean/drivers/usb/input/hid-core.c
+++ dirty/drviers/usb/input/hid-core.c
@@ -37,11 +37,12 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
 
+static unsigned int hid_mousepoll_interval;
 static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
 				"Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
 
@@ -1663,6 +1664,11 @@
 		if ((endpoint->bmAttributes & 3) != 3)		/* Not an interrupt endpoint */
 			continue;
 
+		/* Change the polling interval of mice. */
+		if (hid->collection->usage == HID_GD_MOUSE
+				&& hid_mousepoll_interval > 0)
+			endpoint->bInterval = hid_mousepoll_interval;
+		
 		/* handle potential highspeed HID correctly */
 		interval = endpoint->bInterval;
 		if (dev->speed == USB_SPEED_HIGH)
@@ -1910,6 +1916,7 @@
 
 module_init(hid_init);
 module_exit(hid_exit);
+module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Vojtech Pavlik]

On Sun, Dec 19, 2004 at 01:52:06AM +0000, Mikkel Krautz wrote:
> On Sat, 18 Dec 2004 08:53:31 -0800, Greg KH <greg@kroah.com> wrote:
> > On Sat, Dec 18, 2004 at 05:39:25PM +0000, Mikkel Krautz wrote:
> > > On Fri, 17 Dec 2004 18:59:48 -0800, Greg KH <greg@kroah.com> wrote:
> > > > What about makeing it a module paramater then, that is exported to
> > > > sysfs?  That makes it easier to adjust on the fly (before the mouse is
> > > > inserted), and doesn't require the kernel to be rebuilt.
> > >
> > > I really like the idea. I'm start to think that this is the ideal way to
> > > accomplish this.
> > >
> > > Here's a new patch. Let's hope it doesn't wrap!
> > 
> > It was eaten :(
> > 
> > > module_init(hid_init);
> > > module_exit(hid_exit);
> > > +module_param(hid_mouse_polling_interval, int, 644);
> > 
> > 0644, or use the proper #defines instead.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Here's an updated version, with your and Marcel's suggestions:

Some more suggestions:

A MODULE_PARM_DESC() would be good, as well as a patch to
kernel-parameters.txt.

Also, it'd be better instead of changing the bInterval in the endpoint
descriptor to change the "interval" variable. This way one wouldn't need
to think about whether the device is Full-speed or High-speed when
setting the parameter. Also the endpoint descriptor should be considered
read only. 

> Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> ---
> 
> 
>  hid-core.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletion(-)
> 
> 
> --- clean/drivers/usb/input/hid-core.c
> +++ dirty/drviers/usb/input/hid-core.c
> @@ -37,11 +37,12 @@
>   * Version Information
>   */
>  
> -#define DRIVER_VERSION "v2.0"
> +#define DRIVER_VERSION "v2.01"
>  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
>  #define DRIVER_DESC "USB HID core driver"
>  #define DRIVER_LICENSE "GPL"
>  
> +static unsigned int hid_mousepoll_interval;
>  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
>  				"Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
>  
> @@ -1663,6 +1664,11 @@
>  		if ((endpoint->bmAttributes & 3) != 3)		/* Not an interrupt endpoint */
>  			continue;
>  
> +		/* Change the polling interval of mice. */
> +		if (hid->collection->usage == HID_GD_MOUSE
> +				&& hid_mousepoll_interval > 0)
> +			endpoint->bInterval = hid_mousepoll_interval;
> +		
>  		/* handle potential highspeed HID correctly */
>  		interval = endpoint->bInterval;
>  		if (dev->speed == USB_SPEED_HIGH)
> @@ -1910,6 +1916,7 @@
>  
>  module_init(hid_init);
>  module_exit(hid_exit);
> +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
>  
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_DESCRIPTION(DRIVER_DESC);
> 

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Thanks for the suggestions. :)

I'm wondering if there perhaps is a more clean way to do this? Perhaps
something that would give the user the possibility to change the
polling interface per device, or something like that, perhaps exported
to sysfs? Though, since you mentioned that the endpoint descriptor
should be considered read only, I guess not.

And oh, would adding other parameters make sense? For instance for
gamepads, joysticks etc.? Also, is there any chance of this getting
into the driver? :)

Thanks,
Mikkel

On Sun, 6 Feb 2005 20:07:24 +0100, Vojtech Pavlik <vojtech@suse.de> wrote:
> On Sun, Dec 19, 2004 at 01:52:06AM +0000, Mikkel Krautz wrote:
> > On Sat, 18 Dec 2004 08:53:31 -0800, Greg KH <greg@kroah.com> wrote:
> > > On Sat, Dec 18, 2004 at 05:39:25PM +0000, Mikkel Krautz wrote:
> > > > On Fri, 17 Dec 2004 18:59:48 -0800, Greg KH <greg@kroah.com> wrote:
> > > > > What about makeing it a module paramater then, that is exported to
> > > > > sysfs?  That makes it easier to adjust on the fly (before the mouse is
> > > > > inserted), and doesn't require the kernel to be rebuilt.
> > > >
> > > > I really like the idea. I'm start to think that this is the ideal way to
> > > > accomplish this.
> > > >
> > > > Here's a new patch. Let's hope it doesn't wrap!
> > >
> > > It was eaten :(
> > >
> > > > module_init(hid_init);
> > > > module_exit(hid_exit);
> > > > +module_param(hid_mouse_polling_interval, int, 644);
> > >
> > > 0644, or use the proper #defines instead.
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
> >
> > Here's an updated version, with your and Marcel's suggestions:
> 
> Some more suggestions:
> 
> A MODULE_PARM_DESC() would be good, as well as a patch to
> kernel-parameters.txt.
> 
> Also, it'd be better instead of changing the bInterval in the endpoint
> descriptor to change the "interval" variable. This way one wouldn't need
> to think about whether the device is Full-speed or High-speed when
> setting the parameter. Also the endpoint descriptor should be considered
> read only.
> 
> > Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> > ---
> >
> >
> >  hid-core.c |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletion(-)
> >
> >
> > --- clean/drivers/usb/input/hid-core.c
> > +++ dirty/drviers/usb/input/hid-core.c
> > @@ -37,11 +37,12 @@
> >   * Version Information
> >   */
> >
> > -#define DRIVER_VERSION "v2.0"
> > +#define DRIVER_VERSION "v2.01"
> >  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
> >  #define DRIVER_DESC "USB HID core driver"
> >  #define DRIVER_LICENSE "GPL"
> >
> > +static unsigned int hid_mousepoll_interval;
> >  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
> >                               "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> >
> > @@ -1663,6 +1664,11 @@
> >               if ((endpoint->bmAttributes & 3) != 3)          /* Not an interrupt endpoint */
> >                       continue;
> >
> > +             /* Change the polling interval of mice. */
> > +             if (hid->collection->usage == HID_GD_MOUSE
> > +                             && hid_mousepoll_interval > 0)
> > +                     endpoint->bInterval = hid_mousepoll_interval;
> > +
> >               /* handle potential highspeed HID correctly */
> >               interval = endpoint->bInterval;
> >               if (dev->speed == USB_SPEED_HIGH)
> > @@ -1910,6 +1916,7 @@
> >
> >  module_init(hid_init);
> >  module_exit(hid_exit);
> > +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> >
> >  MODULE_AUTHOR(DRIVER_AUTHOR);
> >  MODULE_DESCRIPTION(DRIVER_DESC);
> >
> 
> --
> Vojtech Pavlik
> SuSE Labs, SuSE CR
>

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Vojtech Pavlik]

On Tue, Feb 08, 2005 at 04:15:53PM +0100, zyphr wrote:
> Something looks odd.
> I've tested this with 2.6.11-rc3-bk5 + your lasted patch
> 
> cat /sys/module/usbhid/parameters/mousepoll says it's at 2ms
> but if I check /proc/bus/usb/devices it's reading 10ms
> 
> I've used parameter under /etc/modules (debian sarge)
> in there I have added: usbhid mousepoll=2
> 
> this used to work ok with:
> http://omfg.linux.dk/pub/configurable-hid-mouse-polling/archive/chmp-r5-add-modparam.patch
> 
> rmmod usbhid and then "modprobe usbhid mousepoll=x" with a different value,
> does change the vualue under /sys
> but under /proc/bus/usb/devices it keeps reading the same value (10ms)
> I also tried replugging the mouse, the value's stay the same.
> 
> I am just a just user, so maybe I did something wrong =)

This is correct. The new patch is not modifying the device descriptors,
which should be considered read-only by driver code, it only changes
only the actual polling behavior.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Oliver Neukum]

Am Dienstag, 8. Februar 2005 16:15 schrieb zyphr:
> Something looks odd.
> I've tested this with 2.6.11-rc3-bk5 + your lasted patch
> 
> cat /sys/module/usbhid/parameters/mousepoll says it's at 2ms
> but if I check /proc/bus/usb/devices it's reading 10ms

You're reading the device descriptor. It is unchangeable.
Using the module parameter you can make the driver ignore
the value specified in the descriptor, not change the descriptor.

	Regards
		Oliver

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[zyphr]

[-- Attachment #1: Type: text/plain, Size: 3415 bytes --]

Something looks odd.
I've tested this with 2.6.11-rc3-bk5 + your lasted patch

cat /sys/module/usbhid/parameters/mousepoll says it's at 2ms
but if I check /proc/bus/usb/devices it's reading 10ms

I've used parameter under /etc/modules (debian sarge)
in there I have added: usbhid mousepoll=2

this used to work ok with:
http://omfg.linux.dk/pub/configurable-hid-mouse-polling/archive/chmp-r5-add-modparam.patch

rmmod usbhid and then "modprobe usbhid mousepoll=x" with a different value,
does change the vualue under /sys
but under /proc/bus/usb/devices it keeps reading the same value (10ms)
I also tried replugging the mouse, the value's stay the same.

I am just a just user, so maybe I did something wrong =)

Thanks,

Joep

On Mon, 7 Feb 2005 19:57:06 +0100, Mikkel Krautz <krautz@gmail.com> wrote:
> This includes the kernel-parameters.txt-patch, and the hid-core.c-patch, without the extra else-statement.
> 
> Thanks,
> Mikkel
> 
> Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> ---
> --- clean/Documentation/kernel-parameters.txt
> +++ dirty/Documentation/kernel-parameters.txt
> @@ -73,6 +73,7 @@
>         SWSUSP  Software suspension is enabled.
>         TS      Appropriate touchscreen support is enabled.
>         USB     USB support is enabled.
> +       USBHID  USB Human Interface Device support is enabled.
>         V4L     Video For Linux support is enabled.
>         VGA     The VGA console has been enabled.
>         VT      Virtual terminal support is enabled.
> @@ -1393,6 +1394,9 @@
>                         Format: <io>,<irq>
> 
>         usb-handoff     [HW] Enable early USB BIOS -> OS handoff
> +
> +       usbhid.mousepoll=
> +                       [USBHID] The interval which mice are to be polled at.
> 
>         video=          [FB] Frame buffer configuration
>                         See Documentation/fb/modedb.txt.
> --- clean/drivers/usb/input/hid-core.c
> +++ dirty/drivers/usb/input/hid-core.c
> @@ -37,13 +37,20 @@
>   * Version Information
>   */
> 
> -#define DRIVER_VERSION "v2.0"
> +#define DRIVER_VERSION "v2.01"
>  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
>  #define DRIVER_DESC "USB HID core driver"
>  #define DRIVER_LICENSE "GPL"
> 
>  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
>                                 "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> +/*
> + * Module parameters.
> + */
> +
> +static unsigned int hid_mousepoll_interval;
> +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> +MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
> 
>  /*
>   * Register a new report for a device.
> @@ -1695,6 +1702,10 @@
>                 if (dev->speed == USB_SPEED_HIGH)
>                         interval = 1 << (interval - 1);
> 
> +               /* Change the polling interval of mice. */
> +               if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
> +                       interval = hid_mousepoll_interval;
> +
>                 if (endpoint->bEndpointAddress & USB_DIR_IN) {
>                         if (hid->urbin)
>                                 continue;
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

[-- Attachment #2: dmesg_2.6.11-rc3-bk5.txt --]
[-- Type: plain/text, Size: 11776 bytes --]

[-- Attachment #3: proc.bus.usb.devices.txt --]
[-- Type: plain/text, Size: 1905 bytes --]

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

This includes the kernel-parameters.txt-patch, and the hid-core.c-patch, without the extra else-statement.

Thanks,
Mikkel

Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---
--- clean/Documentation/kernel-parameters.txt
+++ dirty/Documentation/kernel-parameters.txt
@@ -73,6 +73,7 @@
 	SWSUSP	Software suspension is enabled.
 	TS	Appropriate touchscreen support is enabled.
 	USB	USB support is enabled.
+	USBHID	USB Human Interface Device support is enabled.
 	V4L	Video For Linux support is enabled.
 	VGA	The VGA console has been enabled.
 	VT	Virtual terminal support is enabled.
@@ -1393,6 +1394,9 @@
 			Format: <io>,<irq>
 
 	usb-handoff	[HW] Enable early USB BIOS -> OS handoff
+
+	usbhid.mousepoll=
+			[USBHID] The interval which mice are to be polled at.
  
 	video=		[FB] Frame buffer configuration
 			See Documentation/fb/modedb.txt.
--- clean/drivers/usb/input/hid-core.c
+++ dirty/drivers/usb/input/hid-core.c
@@ -37,13 +37,20 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
 
 static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
 				"Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
+/*
+ * Module parameters.
+ */
+
+static unsigned int hid_mousepoll_interval;
+module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
+MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
 
 /*
  * Register a new report for a device.
@@ -1695,6 +1702,10 @@
 		if (dev->speed == USB_SPEED_HIGH)
 			interval = 1 << (interval - 1);
 
+		/* Change the polling interval of mice. */
+		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
+			interval = hid_mousepoll_interval;
+		
 		if (endpoint->bEndpointAddress & USB_DIR_IN) {
 			if (hid->urbin)
 				continue;

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Alright, I'll send it in a bit!

Thanks,
Mikkel


On Mon, 7 Feb 2005 19:38:18 +0100, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Mon, Feb 07, 2005 at 06:56:50PM +0100, Mikkel Krautz wrote:
> > Sorry, I now realise that my way of doing this only sets
> > hid_mousepoll_interval to the latest-plugged-in mouse's polling
> > interval.
> >
> > How should I handle this? Just remove the line, and keep the "0 feature"?
> 
> Yes. And please merge both the patches (documentation and code) into one.
> 
> >
> > Thanks,
> > Mikkel
> >
> >
> > On Mon, 7 Feb 2005 18:51:46 +0100, Mikkel Krautz <krautz@gmail.com> wrote:
> > > Are you talking about the following line?
> > >
> > > +               else
> > > +                       hid_mousepoll_interval = interval;
> > >
> > > If so, I put it there, to fill a tiny gap, i felt was missing.
> > >
> > > If no parameter is passed, hid_mousepoll_interval is obviously 0.
> > >
> > > If a user, who doesn't pass the parameter to usbhid, reads
> > > '/sys/module/usbhid/parameters/mousepoll', the answer would be "0",
> > > which is incorrect, no?
> > >
> > > Thanks,
> > > Mikkel
> > >
> > > On Mon, 7 Feb 2005 18:43:03 +0100, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > > On Mon, Feb 07, 2005 at 04:44:24PM +0100, Mikkel Krautz wrote:
> > > > > And, here's an updated version of hid-core.c:
> > > > >
> > > > > Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> > > > > ---
> > > > > --- clean/drivers/usb/input/hid-core.c
> > > > > +++ dirty/drivers/usb/input/hid-core.c
> > > > > @@ -37,13 +37,20 @@
> > > > >   * Version Information
> > > > >   */
> > > > >
> > > > > -#define DRIVER_VERSION "v2.0"
> > > > > +#define DRIVER_VERSION "v2.01"
> > > > >  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
> > > > >  #define DRIVER_DESC "USB HID core driver"
> > > > >  #define DRIVER_LICENSE "GPL"
> > > > >
> > > > >  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
> > > > >                               "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> > > > > +/*
> > > > > + * Module parameters.
> > > > > + */
> > > > > +
> > > > > +static unsigned int hid_mousepoll_interval;
> > > > > +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> > > > > +MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
> > > > >
> > > > >  /*
> > > > >   * Register a new report for a device.
> > > > > @@ -1695,6 +1702,12 @@
> > > > >               if (dev->speed == USB_SPEED_HIGH)
> > > > >                       interval = 1 << (interval - 1);
> > > > >
> > > > > +             /* Change the polling interval of mice. */
> > > > > +             if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
> > > > > +                     interval = hid_mousepoll_interval;
> > > > > +             else
> > > > > +                     hid_mousepoll_interval = interval;
> > > >
> > > > This line is trying to achieve what?
> > > >
> > > > > +
> > > > >               if (endpoint->bEndpointAddress & USB_DIR_IN) {
> > > > >                       if (hid->urbin)
> > > > >                               continue;
> > > > >
> > > > >
> > > >
> > > > --
> > > > Vojtech Pavlik
> > > > SuSE Labs, SuSE CR
> > > >
> > >
> >
> 
> --
> Vojtech Pavlik
> SuSE Labs, SuSE CR
>

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Sorry, I now realise that my way of doing this only sets
hid_mousepoll_interval to the latest-plugged-in mouse's polling
interval.

How should I handle this? Just remove the line, and keep the "0 feature"?

Thanks,
Mikkel


On Mon, 7 Feb 2005 18:51:46 +0100, Mikkel Krautz <krautz@gmail.com> wrote:
> Are you talking about the following line?
> 
> +               else
> +                       hid_mousepoll_interval = interval;
> 
> If so, I put it there, to fill a tiny gap, i felt was missing.
> 
> If no parameter is passed, hid_mousepoll_interval is obviously 0.
> 
> If a user, who doesn't pass the parameter to usbhid, reads
> '/sys/module/usbhid/parameters/mousepoll', the answer would be "0",
> which is incorrect, no?
> 
> Thanks,
> Mikkel
> 
> On Mon, 7 Feb 2005 18:43:03 +0100, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > On Mon, Feb 07, 2005 at 04:44:24PM +0100, Mikkel Krautz wrote:
> > > And, here's an updated version of hid-core.c:
> > >
> > > Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> > > ---
> > > --- clean/drivers/usb/input/hid-core.c
> > > +++ dirty/drivers/usb/input/hid-core.c
> > > @@ -37,13 +37,20 @@
> > >   * Version Information
> > >   */
> > >
> > > -#define DRIVER_VERSION "v2.0"
> > > +#define DRIVER_VERSION "v2.01"
> > >  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
> > >  #define DRIVER_DESC "USB HID core driver"
> > >  #define DRIVER_LICENSE "GPL"
> > >
> > >  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
> > >                               "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> > > +/*
> > > + * Module parameters.
> > > + */
> > > +
> > > +static unsigned int hid_mousepoll_interval;
> > > +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> > > +MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
> > >
> > >  /*
> > >   * Register a new report for a device.
> > > @@ -1695,6 +1702,12 @@
> > >               if (dev->speed == USB_SPEED_HIGH)
> > >                       interval = 1 << (interval - 1);
> > >
> > > +             /* Change the polling interval of mice. */
> > > +             if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
> > > +                     interval = hid_mousepoll_interval;
> > > +             else
> > > +                     hid_mousepoll_interval = interval;
> >
> > This line is trying to achieve what?
> >
> > > +
> > >               if (endpoint->bEndpointAddress & USB_DIR_IN) {
> > >                       if (hid->urbin)
> > >                               continue;
> > >
> > >
> >
> > --
> > Vojtech Pavlik
> > SuSE Labs, SuSE CR
> >
>

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Are you talking about the following line?

+		else
+			hid_mousepoll_interval = interval;

If so, I put it there, to fill a tiny gap, i felt was missing.

If no parameter is passed, hid_mousepoll_interval is obviously 0.

If a user, who doesn't pass the parameter to usbhid, reads
'/sys/module/usbhid/parameters/mousepoll', the answer would be "0",
which is incorrect, no?

Thanks,
Mikkel

On Mon, 7 Feb 2005 18:43:03 +0100, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Mon, Feb 07, 2005 at 04:44:24PM +0100, Mikkel Krautz wrote:
> > And, here's an updated version of hid-core.c:
> >
> > Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> > ---
> > --- clean/drivers/usb/input/hid-core.c
> > +++ dirty/drivers/usb/input/hid-core.c
> > @@ -37,13 +37,20 @@
> >   * Version Information
> >   */
> >
> > -#define DRIVER_VERSION "v2.0"
> > +#define DRIVER_VERSION "v2.01"
> >  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
> >  #define DRIVER_DESC "USB HID core driver"
> >  #define DRIVER_LICENSE "GPL"
> >
> >  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
> >                               "Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> > +/*
> > + * Module parameters.
> > + */
> > +
> > +static unsigned int hid_mousepoll_interval;
> > +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> > +MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
> >
> >  /*
> >   * Register a new report for a device.
> > @@ -1695,6 +1702,12 @@
> >               if (dev->speed == USB_SPEED_HIGH)
> >                       interval = 1 << (interval - 1);
> >
> > +             /* Change the polling interval of mice. */
> > +             if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
> > +                     interval = hid_mousepoll_interval;
> > +             else
> > +                     hid_mousepoll_interval = interval;
> 
> This line is trying to achieve what?
> 
> > +
> >               if (endpoint->bEndpointAddress & USB_DIR_IN) {
> >                       if (hid->urbin)
> >                               continue;
> >
> >
> 
> --
> Vojtech Pavlik
> SuSE Labs, SuSE CR
>

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Domen Puncer]

On 07/02/05 07:59 -0800, Randy.Dunlap wrote:
> Mikkel Krautz wrote:
> >And, here's an updated version of hid-core.c:
> >
> >Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> >---
> >--- clean/drivers/usb/input/hid-core.c
> >+++ dirty/drivers/usb/input/hid-core.c
...
> >+
> >+static unsigned int hid_mousepoll_interval;
> >+module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
> 
> Why is it writable by root?  IOW, will writing a new value to it
> change the operational value dynamically?
> 
> Also, from the kernel-parameters.txt patch:
> +	usbhid.mousepoll=
> +		[USBHID] The interval at wich mice are to be polled at.
> 
> (a) "which"
> (b) drop one of the "at"s... either one.

Is listing module parameters in kernel-parameters.txt the right thing
to do? (There are lots of them, not many are listed)

I see some options that might be better:
- Kconfig magic which extracts module_param* and MODULE_PARM_DESC from
  sources and appends them to help text.
- a userspace script, that goes trough all modules and generates
  kernel-module-parameters.txt for example.
- modinfo like tool (but i think it would require source or descriptions
  compiled in kernel)



	Domen

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Randy.Dunlap]

Domen Puncer wrote:
> On 07/02/05 07:59 -0800, Randy.Dunlap wrote:
> 
>>>+static unsigned int hid_mousepoll_interval;
>>>+module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
>>
>>Why is it writable by root?  IOW, will writing a new value to it
>>change the operational value dynamically?
>>
>>Also, from the kernel-parameters.txt patch:
>>+	usbhid.mousepoll=
>>+		[USBHID] The interval at wich mice are to be polled at.
>>
>>(a) "which"
>>(b) drop one of the "at"s... either one.
> 
> 
> Is listing module parameters in kernel-parameters.txt the right thing
> to do? (There are lots of them, not many are listed)

It's currently the right thing to do, but some automated overhaul
sure would make sense.

> I see some options that might be better:
> - Kconfig magic which extracts module_param* and MODULE_PARM_DESC from
>   sources and appends them to help text.
> - a userspace script, that goes trough all modules and generates
>   kernel-module-parameters.txt for example.
> - modinfo like tool (but i think it would require source or descriptions
>   compiled in kernel)

-- 
~Randy

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Randy.Dunlap wrote:
> Why is it writable by root?  IOW, will writing a new value to it
> change the operational value dynamically?

Yes, sort of. It requires a re-plug of the mouse, though. :)

> Also, from the kernel-parameters.txt patch:
> (a) "which"
> (b) drop one of the "at"s... either one.

Oops, thanks!

Here's an updated version of the kernel-parameters patch:

Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---
--- clean/Documentation/kernel-parameters.txt
+++ dirty/Documentation/kernel-parameters.txt
@@ -73,6 +73,7 @@
 	SWSUSP	Software suspension is enabled.
 	TS	Appropriate touchscreen support is enabled.
 	USB	USB support is enabled.
+	USBHID	USB Human Interface Device support is enabled.
 	V4L	Video For Linux support is enabled.
 	VGA	The VGA console has been enabled.
 	VT	Virtual terminal support is enabled.
@@ -1393,6 +1394,9 @@
 			Format: <io>,<irq>
 
 	usb-handoff	[HW] Enable early USB BIOS -> OS handoff
+
+	usbhid.mousepoll=
+			[USBHID] The interval which mice are to be polled at.
  
 	video=		[FB] Frame buffer configuration
 			See Documentation/fb/modedb.txt.

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Randy.Dunlap]

Mikkel Krautz wrote:
> And, here's an updated version of hid-core.c:
> 
> Signed-off-by: Mikkel Krautz <krautz@gmail.com>
> ---
> --- clean/drivers/usb/input/hid-core.c
> +++ dirty/drivers/usb/input/hid-core.c
> @@ -37,13 +37,20 @@
>   * Version Information
>   */
>  
> -#define DRIVER_VERSION "v2.0"
> +#define DRIVER_VERSION "v2.01"
>  #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
>  #define DRIVER_DESC "USB HID core driver"
>  #define DRIVER_LICENSE "GPL"
>  
>  static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
>  				"Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
> +/*
> + * Module parameters.
> + */
> +
> +static unsigned int hid_mousepoll_interval;
> +module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);

Why is it writable by root?  IOW, will writing a new value to it
change the operational value dynamically?

Also, from the kernel-parameters.txt patch:
+	usbhid.mousepoll=
+		[USBHID] The interval at wich mice are to be polled at.

(a) "which"
(b) drop one of the "at"s... either one.

> +MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
>  
>  /*
>   * Register a new report for a device.
> @@ -1695,6 +1702,12 @@
>  		if (dev->speed == USB_SPEED_HIGH)
>  			interval = 1 << (interval - 1);
>  
> +		/* Change the polling interval of mice. */
> +		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
> +			interval = hid_mousepoll_interval;
> +		else
> +			hid_mousepoll_interval = interval;
> +		
>  		if (endpoint->bEndpointAddress & USB_DIR_IN) {
>  			if (hid->urbin)
>  				continue;


-- 
~Randy

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

And, here's an updated version of hid-core.c:

Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---
--- clean/drivers/usb/input/hid-core.c
+++ dirty/drivers/usb/input/hid-core.c
@@ -37,13 +37,20 @@
  * Version Information
  */
 
-#define DRIVER_VERSION "v2.0"
+#define DRIVER_VERSION "v2.01"
 #define DRIVER_AUTHOR "Andreas Gal, Vojtech Pavlik"
 #define DRIVER_DESC "USB HID core driver"
 #define DRIVER_LICENSE "GPL"
 
 static char *hid_types[] = {"Device", "Pointer", "Mouse", "Device", "Joystick",
 				"Gamepad", "Keyboard", "Keypad", "Multi-Axis Controller"};
+/*
+ * Module parameters.
+ */
+
+static unsigned int hid_mousepoll_interval;
+module_param_named(mousepoll, hid_mousepoll_interval, uint, 0644);
+MODULE_PARM_DESC(mousepoll, "Polling interval of mice");
 
 /*
  * Register a new report for a device.
@@ -1695,6 +1702,12 @@
 		if (dev->speed == USB_SPEED_HIGH)
 			interval = 1 << (interval - 1);
 
+		/* Change the polling interval of mice. */
+		if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)
+			interval = hid_mousepoll_interval;
+		else
+			hid_mousepoll_interval = interval;
+		
 		if (endpoint->bEndpointAddress & USB_DIR_IN) {
 			if (hid->urbin)
 				continue;

Re: [PATCH] hid-core: Configurable USB HID Mouse Interrupt Polling Interval

[Mikkel Krautz]

Here's an updated version of kernel-parameters.txt:

Signed-off-by: Mikkel Krautz <krautz@gmail.com>
---
--- clean/Documentation/kernel-parameters.txt
+++ dirty/Documentation/kernel-parameters.txt
@@ -73,6 +73,7 @@
 	SWSUSP	Software suspension is enabled.
 	TS	Appropriate touchscreen support is enabled.
 	USB	USB support is enabled.
+	USBHID	USB Human Interface Device support is enabled.
 	V4L	Video For Linux support is enabled.
 	VGA	The VGA console has been enabled.
 	VT	Virtual terminal support is enabled.
@@ -1393,6 +1394,9 @@
 			Format: <io>,<irq>
 
 	usb-handoff	[HW] Enable early USB BIOS -> OS handoff
+
+	usbhid.mousepoll=
+			[USBHID] The interval at wich mice are to be polled at.
  
 	video=		[FB] Frame buffer configuration
 			See Documentation/fb/modedb.txt.