On Sun, Dec 19, 2004 at 01:52:06AM +0000, Mikkel Krautz wrote:
> On Sat, 18 Dec 2004 08:53:31 -0800, Greg KH <[email protected]> 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 <[email protected]> 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 <[email protected]>
> ---
>
>
> 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
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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > ---
> >
> >
> > 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
>