[PATCH] bugs in 2.2.2-7 drivers/char/pc_keyb.c

Julian Bradfield (jcb@dcs.ed.ac.uk)
Fri, 30 Apr 1999 19:35:00 +0100 (BST)


Problem: since kernel 2.2.2, the Synaptics touchpad configurer tpconfig
has not worked on my system (Digital Hinote Ultra 2000). It times out
waiting for an ack from the mouse. (And various more exotic things
happen on the second etc. attempts.)

Suggested analysis: the keyboard and mouse handler
linux/drivers/char/pc_keyb.c is broken. In 2.2.2, C. Scott Ananian
made various changes to fix the problem that mouse ACKs, generated from
commands sent during psaux open(), were being passed to user programs.
However, there is (I believe) a deeper problem, revealed by Scott's
fix. Namely, when a command is sent to the mouse, the driver should
wait for the ACK before writing another command byte; otherwise the ACK
may be lost. The pre-2.2.2 driver failed to do this, but because it also
failed to intercept its own ACKs, tpconfig did get the ACKs it was expecting.

Proposed fix: when writing a command byte to the mouse in the routines
aux_write_dev and aux_write_ack, the driver should wait until any pending
ACK has been received. This is achieved by an mdelay loop inside the spinlock;
in practice, the loop is only executed once, but a timeout of 5 is implemented.

Questions: does this analysis and fix seem plausible? (It works for me,
so I'm happy.)

Patch follows:

--- linux-orig/drivers/char/pc_keyb.c Fri Apr 30 12:34:07 1999
+++ linux/drivers/char/pc_keyb.c Fri Apr 30 19:39:34 1999
@@ -13,6 +13,8 @@
* Code fixes to handle mouse ACKs properly.
* C. Scott Ananian <cananian@alumni.princeton.edu> 1999-01-29.
*
+ * Code fixes to *really* handle mouse ACKs properly.
+ * Julian Bradfield <jcb@dcs.ed.ac.uk> 1999-04-30.
*/

#include <linux/config.h>
@@ -77,8 +79,13 @@

static struct aux_queue *queue; /* Mouse data buffer. */
static int aux_count = 0;
-/* used when we send commands to the mouse that expect an ACK. */
+/* used when we (as opposed to the user programs using the aux device)
+ send commands to the mouse that expect an ACK. */
static unsigned char mouse_reply_expected = 0;
+/* used to make sure we read acks from the mouse before we
+ write another byte */
+static unsigned char mouse_ack_pending = 0;
+#define MOUSE_ACK_TIMEOUT 5 /* actually, 1 seems to be enough usually */

#define AUX_INTS_OFF (KBD_MODE_KCC | KBD_MODE_DISABLE_MOUSE | KBD_MODE_SYS | KBD_MODE_KBD_INT)
#define AUX_INTS_ON (KBD_MODE_KCC | KBD_MODE_SYS | KBD_MODE_MOUSE_INT | KBD_MODE_KBD_INT)
@@ -393,6 +400,10 @@
static inline void handle_mouse_event(unsigned char scancode)
{
#ifdef CONFIG_PSMOUSE
+ if (mouse_ack_pending)
+ /* it needn't actually be an ack, it could be an echo;
+ but every byte sent to the mouse results in a byte back */
+ mouse_ack_pending = 0;
if (mouse_reply_expected) {
if (scancode == AUX_ACK) {
mouse_reply_expected--;
@@ -761,12 +772,23 @@
static void aux_write_dev(int val)
{
unsigned long flags;
+ int loop = 0;

spin_lock_irqsave(&kbd_controller_lock, flags);
kb_wait();
+ /* if we haven't yet received the ack from the previous
+ write, we must wait for it to arrive; otherwise we
+ lose it. (At least, I think this is what is happening.) */
+ while (mouse_ack_pending && loop++ < MOUSE_ACK_TIMEOUT ) {
+ mdelay(1);
+ handle_kbd_event();
+ }
+ if ( mouse_ack_pending )
+ printk(KERN_WARNING "mouse ack timeout\n") ;
outb(KBD_CCMD_WRITE_MOUSE, KBD_CNTL_REG);
kb_wait();
outb(val, KBD_DATA_REG);
+ mouse_ack_pending = 1;
spin_unlock_irqrestore(&kbd_controller_lock, flags);
}

@@ -776,13 +798,21 @@
static void aux_write_ack(int val)
{
unsigned long flags;
+ int loop = 0;

spin_lock_irqsave(&kbd_controller_lock, flags);
kb_wait();
+ while (mouse_ack_pending && loop++ < MOUSE_ACK_TIMEOUT) {
+ mdelay(1);
+ handle_kbd_event();
+ }
+ if ( mouse_ack_pending )
+ printk(KERN_WARNING "mouse ack timeout\n") ;
outb(KBD_CCMD_WRITE_MOUSE, KBD_CNTL_REG);
kb_wait();
outb(val, KBD_DATA_REG);
- /* we expect an ACK in response. */
+ mouse_ack_pending = 1;
+ /* we will deal with the ACK ourselves. */
mouse_reply_expected++;
kb_wait();
spin_unlock_irqrestore(&kbd_controller_lock, flags);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/