2005-02-07 15:45:53

by Mikkel Krautz

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

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

Signed-off-by: Mikkel Krautz <[email protected]>
---
--- 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;



2005-02-07 16:17:45

by Randy.Dunlap

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

Mikkel Krautz wrote:
> And, here's an updated version of hid-core.c:
>
> Signed-off-by: Mikkel Krautz <[email protected]>
> ---
> --- 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

2005-02-07 17:17:12

by Domen Puncer

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

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 <[email protected]>
> >---
> >--- 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

2005-02-07 17:19:56

by Randy.Dunlap

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

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