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 <linux@xxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html