Re: [REPOST #3 PATCH v2] Input: atkbd - make repeat period more accurate.

Hi George,

On Sat, Jul 21, 2012 at 11:42:28PM -0400, George Spelvin wrote:

This replaces some inaccurate lookup tables with an exact
computation.  Although the diff adds source comments,
it shrinks binary size.  (By only 50 bytes, but hey.)

AT keyboard repeat rates are multiples of 1/240 second
expressed in a 0.2.3 bit floating point format.  That
is, possible values are ({8..15} << {0..3}) / 240 s.

OK.

In addition to a slightly inaccurate lookup table, the
old code would round up to the next repeat period.
E.g. to get a period of 9/60 = 0.15 seconds, you had to
ask for no more than 149 ms; if you asked for 150, it
would round up to 167.

This works as intended - it was designed to never have faster than
requested.

 The new code rounds to nearest.

User-visible changes to the repeat periods reported by EVIOCGREP:

Old	 37	109	116	149	182	232
Exact	 37.50	108.33	116.66	150.00	183.33	233.33
New	 38	108	117	150	183	233

Old	270	303	370	435	470
Exact	266.66	300.00	366.66	433.33	466.66
New	267	300	367	433	467

This does not bother utilities like kbdrate(8).

Signed-off-by: George Spelvin <redacted>

I am sorry but I have to ask - is this your real name?

---
 drivers/input/keyboard/atkbd.c |   47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

Now that 3.5 is out, I'm posting this for a FOURTH time,
hoping for some comments of any sort.


Now that I've tweaked it (v1 had an error in rounding near exponent
range boundaries), I think it's ready for merging upstream.


One possible bug I observed in the code that calls this:

Users of the KDKBDREP ioctl seem to assume that it returns the actual
values set, but I'm not sure it really works that way; I don't think
the command to change the parameters makes its way through the event
queue and atkbd's schedule_delayed_work() to actually set dev->rep[]
to the rounded values before kbd_rate_helper returns them to userspace.

If desired, the fix that's most obvious to me would be to split this
function in two: perform the conversion to a command byte synchronously,
and only defer the actual ps2_command().

Yes, I agree, this is a problem.

Thanks.

-- 
Dmitry