On Tue, Nov 01, 2011 at 02:24:48PM -0400, John Baboval wrote:
Attached is a re-write of my previous patch that changes i8042_flush to be timeout based rather than buffer size based.... Which turns out to not be significantly different, since the udelay in the loop is the basis for the timing. I created the I8042_FLUSH_TIMEOUT delay, and used it similarly to the I8042_CTL_TIMEOUT define; however the flush timeout is much shorter.
I was tempted to make the "50" that is hard coded into the udelays into a define to make the timeout defines more readable, but it seemed like unnecessary refactoring that is outside of the scope of this change.
As a tangentially related aside, I found my bug in the device model that was causing the queue to be full so early as well. So the keyboard is working for me even without this patch now.
Increasing the timeout from 800us to 15ms is annoying for those without a ps2 port. I was thinking something more like what's below. However, now that I think about it, the dprintf() could cause a delay when writing to the serial port which could skew the timing. Maybe just leave it alone if you're no longer seeing the issue (or alternatively the dprintf() could be removed from the loop).
-Kevin
--- a/src/ps2port.c +++ b/src/ps2port.c @@ -19,8 +19,7 @@
// Timeout value. #define I8042_CTL_TIMEOUT 10000 - -#define I8042_BUFFER_SIZE 16 +#define I8042_FLUSH_TIMEOUT 1 // ms
static int i8042_wait_read(void) @@ -56,14 +55,15 @@ static int i8042_flush(void) { dprintf(7, "i8042_flush\n"); - int i; - for (i=0; i<I8042_BUFFER_SIZE; i++) { + u64 end = calc_future_tsc(I8042_FLUSH_TIMEOUT); + for (;;) { u8 status = inb(PORT_PS2_STATUS); if (! (status & I8042_STR_OBF)) return 0; - udelay(50); u8 data = inb(PORT_PS2_DATA); dprintf(7, "i8042 flushed %x (status=%x)\n", data, status); + if (check_tsc(end)) + break; }
warn_timeout();