lkml.org 
[lkml]   [2006]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: USB input ati_remote autorepeat problem
Vojtech Pavlik wrote:
> On Tue, Jun 27, 2006 at 02:46:39PM -0700, Nish Aravamudan wrote:
>
>> On 6/27/06, Marko Macek <Marko.Macek@gmx.net> wrote:
>>
>>> Hello!
>>>
>>> I have problems with autorepeat in ati_remote (drivers/usb/input) driver
>>> in "recent" kernels: all keys start repeating immediately without some
>>> delay.
>>>
>>> This makes some things, like changing the channel prev/next or toggling
>>> fullscreen, etc... impossible/hard.
>>>
>>> The problem seems to be related to FILTER_TIME and HZ=250 (which I
>>> forgot to change).
>>>
>>> FILTER_TIME is defined to HZ / 20, and since 250 is not divisible by 20,
>>> the time will be too short to ignore enough events.
>>>
>>> Defining FILTER_TIME to HZ / 20 + 1 seems to fix things, but I'm not
>>> sure if there are any bad side effects.
>>>
>> Can you try just defining it to msecs_to_jiffies(50)? That should
>> handle the various HZ cases just fine.
>>
>
> Indeed, that would be thr right solution. Even better would be to
>
> #define FILTER_TIME 50 /* 50 msec */
>
> and later use
>
> msecs_to_jiffies(FILTER_TIME)
>
> in the code.
There is still a problem (reproducible in HZ=100, at least), because
msec_to_jiffies

#if HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC % HZ)
return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);

Calculates 5 ticks for 50ms, which might seem to be correct, but it
really isn't, since 5 ticks can happen in as little as 40 (+eps) ms.

I wonder if this usage of msec_to_jiffies is correct (seems wrong to me).

A working (but not clean) patch might look like this:

--- linux-2.6.17.orig/drivers/usb/input/ati_remote.c 2006-06-29 21:18:15.000000000 +0200

+++ linux-2.6.17/drivers/usb/input/ati_remote.c 2006-07-02 20:10:17.000000000 +0200

@@ -155,9 +155,8 @@

* events. The hardware generates 5 events for the first keypress

* and we have to take this into account for an accurate repeat

* behaviour.

- * (HZ / 20) == 50 ms and works well for me.

*/
-#define FILTER_TIME (HZ / 20)

+#define FILTER_TIME 51 /* msec */


struct ati_remote {
struct input_dev *idev;

@@ -470,7 +469,7 @@

/* Filter duplicate events which happen "too close" together. */
if ((ati_remote->old_data[0] == data[1]) &&
(ati_remote->old_data[1] == data[2]) &&

- time_before(jiffies, ati_remote->old_jiffies + FILTER_TIME)) {
+ time_before(jiffies, ati_remote->old_jiffies + msecs_to_jiffies(FILTER_TIME))) {
ati_remote->repeat_count++;

} else {
ati_remote->repeat_count = 0;


Some googling reveals that an old patch used HZ >> 4 (HZ / 16) instead
of HZ / 20;

Perhaps using msec_to_jiffies(50) + 1 would be the correct fix?

Mark

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

\
 
 \ /
  Last update: 2006-07-02 20:41    [W:1.647 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site