[PATCH] Update ADB drivers in 2.5

This updates the ADB driver and the three low-level ADB bus adaptor
drivers used on powermacs.  The files affected are all in
drivers/macintosh; they are adb.c, macio-adb.c, via-cuda.c and
via-pmu.c.

The main changes in this patch are:

- Remove the use of global cli/sti and replace them with local cli/sti,
  spinlocks and semaphores as appropriate.
- Use DECLARE_WORK/schedule_work instead of tq_struct/schedule_task.
- Improvements to the PMU interrupt handling and sleep/wakeup code.

drivers/macintosh/adb.c

@@ -34,8 +34,10 @@
 #include <linux/wait.h>
 #include <linux/init.h>
 #include <linux/delay.h>
+#include <linux/spinlock.h>
 #include <linux/completion.h>
 #include <asm/uaccess.h>
+#include <asm/semaphore.h>
 #ifdef CONFIG_PPC
 #include <asm/prom.h>
 #include <asm/hydra.h>
@@ -75,8 +77,8 @@ static struct adb_driver *adb_driver_list[] = {
 
 struct adb_driver *adb_controller;
 struct notifier_block *adb_client_list = NULL;
-static int adb_got_sleep = 0;
-static int adb_inited = 0;
+static int adb_got_sleep;
+static int adb_inited;
 static pid_t adb_probe_task_pid;
 static DECLARE_MUTEX(adb_probe_mutex);
 static struct completion adb_probe_task_comp;
@@ -94,7 +96,7 @@ static struct pmu_sleep_notifier adb_sleep_notifier = {
 static int adb_scan_bus(void);
 static int do_adb_reset_bus(void);
 static void adbdev_init(void);
-
+static int try_handler_change(int, int);
 
 static struct adb_handler {
 	void (*handler)(unsigned char *, int, struct pt_regs *, int);
@@ -102,6 +104,18 @@ static struct adb_handler {
 	int handler_id;
 } adb_handler[16];
 
+/*
+ * The adb_handler_sem mutex protects all accesses to the original_address
+ * and handler_id fields of adb_handler[i] for all i, and changes to the
+ * handler field.
+ * Accesses to the handler field are protected by the adb_handler_lock
+ * rwlock.  It is held across all calls to any handler, so that by the
+ * time adb_unregister returns, we know that the old handler isn't being
+ * called.
+ */
+static DECLARE_MUTEX(adb_handler_sem);
+static rwlock_t adb_handler_lock = RW_LOCK_UNLOCKED;
+
 #if 0
 static void printADBreply(struct adb_request *req)
 {
@@ -254,25 +268,18 @@ __adb_probe_task(void *data)
 		SIGCHLD | CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
 }
 
+static DECLARE_WORK(adb_reset_work, __adb_probe_task, NULL);
+
 int
 adb_reset_bus(void)
 {
-	static struct tq_struct tqs = {
-		routine:	__adb_probe_task,
-	};
-
 	if (__adb_probe_sync) {
 		do_adb_reset_bus();
 		return 0;
 	}
 
 	down(&adb_probe_mutex);
-
-	/* Create probe thread as a child of keventd */
-	if (current_is_keventd())
-		__adb_probe_task(NULL);
-	else
-		schedule_task(&tqs);
+	schedule_work(&adb_reset_work);
 	return 0;
 }
 
@@ -372,7 +379,6 @@ static int
 do_adb_reset_bus(void)
 {
 	int ret, nret, devs;
-	unsigned long flags;
 	
 	if (adb_controller == NULL)
 		return -ENXIO;
@@ -392,10 +398,10 @@ do_adb_reset_bus(void)
 		adb_wait_ms(500);
 	}
 
-	save_flags(flags);
-	cli();
+	down(&adb_handler_sem);
+	write_lock_irq(&adb_handler_lock);
 	memset(adb_handler, 0, sizeof(adb_handler));
-	restore_flags(flags);
+	write_unlock_irq(&adb_handler_lock);
 
 	/* That one is still a bit synchronous, oh well... */
 	if (adb_controller->reset_bus)
@@ -413,6 +419,7 @@ do_adb_reset_bus(void)
 		if (adb_controller->autopoll)
 			adb_controller->autopoll(devs);
 	}
+	up(&adb_handler_sem);
 
 	nret = notifier_call_chain(&adb_client_list, ADB_MSG_POST_RESET, NULL);
 	if (nret & NOTIFY_STOP_MASK)
@@ -512,30 +519,41 @@ adb_register(int default_id, int handler_id, struct adb_ids *ids,
 {
 	int i;
 
+	down(&adb_handler_sem);
 	ids->nids = 0;
 	for (i = 1; i < 16; i++) {
 		if ((adb_handler[i].original_address == default_id) &&
 		    (!handler_id || (handler_id == adb_handler[i].handler_id) || 
-		    adb_try_handler_change(i, handler_id))) {
+		    try_handler_change(i, handler_id))) {
 			if (adb_handler[i].handler != 0) {
 				printk(KERN_ERR
 				       "Two handlers for ADB device %d\n",
 				       default_id);
 				continue;
 			}
+			write_lock_irq(&adb_handler_lock);
 			adb_handler[i].handler = handler;
+			write_unlock_irq(&adb_handler_lock);
 			ids->id[ids->nids++] = i;
 		}
 	}
+	up(&adb_handler_sem);
 	return ids->nids;
 }
 
 int
 adb_unregister(int index)
 {
-	if (!adb_handler[index].handler)
-		return -ENODEV;
+	int ret = -ENODEV;
+
+	down(&adb_handler_sem);
+	write_lock_irq(&adb_handler_lock);
+	if (adb_handler[index].handler) {
+		ret = 0;
 		adb_handler[index].handler = 0;
+	}
+	write_unlock_irq(&adb_handler_lock);
+	up(&adb_handler_sem);
 	return 0;
 }
 
@@ -544,6 +562,7 @@ adb_input(unsigned char *buf, int nb, struct pt_regs *regs, int autopoll)
 {
 	int i, id;
 	static int dump_adb_input = 0;
+	void (*handler)(unsigned char *, int, struct pt_regs *, int);
 
 	/* We skip keystrokes and mouse moves when the sleep process
 	 * has been started. We stop autopoll, but this is another security
@@ -558,14 +577,15 @@ adb_input(unsigned char *buf, int nb, struct pt_regs *regs, int autopoll)
 			printk(" %x", buf[i]);
 		printk(", id = %d\n", id);
 	}
-	if (adb_handler[id].handler != 0) {
-		(*adb_handler[id].handler)(buf, nb, regs, autopoll);
-	}
+	read_lock(&adb_handler_lock);
+	handler = adb_handler[id].handler;
+	if (handler != 0)
+		(*handler)(buf, nb, regs, autopoll);
+	read_unlock(&adb_handler_lock);
 }
 
-/* Try to change handler to new_id. Will return 1 if successful */
-int
-adb_try_handler_change(int address, int new_id)
+/* Try to change handler to new_id. Will return 1 if successful. */
+static int try_handler_change(int address, int new_id)
 {
 	struct adb_request req;
 
@@ -584,11 +604,24 @@ adb_try_handler_change(int address, int new_id)
 	return 1;
 }
 
+int
+adb_try_handler_change(int address, int new_id)
+{
+	int ret;
+
+	down(&adb_handler_sem);
+	ret = try_handler_change(address, new_id);
+	up(&adb_handler_sem);
+	return ret;
+}
+
 int
 adb_get_infos(int address, int *original_address, int *handler_id)
 {
+	down(&adb_handler_sem);
 	*original_address = adb_handler[address].original_address;
 	*handler_id = adb_handler[address].handler_id;
+	up(&adb_handler_sem);
 
 	return (*original_address != 0);
 }

drivers/macintosh/macio-adb.c

@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/sched.h>
+#include <linux/spinlock.h>
 #include <asm/prom.h>
 #include <linux/adb.h>
 #include <asm/io.h>
@@ -57,7 +58,7 @@ struct adb_regs {
 
 static volatile struct adb_regs *adb;
 static struct adb_request *current_req, *last_req;
-static unsigned char adb_rbuf[16];
+static spinlock_t macio_lock = SPIN_LOCK_UNLOCKED;
 
 static int macio_probe(void);
 static int macio_init(void);
@@ -66,7 +67,6 @@ static int macio_send_request(struct adb_request *req, int sync);
 static int macio_adb_autopoll(int devs);
 static void macio_adb_poll(void);
 static int macio_adb_reset_bus(void);
-static void completed(void);
 
 struct adb_driver macio_adb_driver = {
 	"MACIO",
@@ -107,19 +107,19 @@ int macio_init(void)
 	adb = (volatile struct adb_regs *)
 		ioremap(adbs->addrs->address, sizeof(struct adb_regs));
 
-	if (request_irq(adbs->intrs[0].line, macio_adb_interrupt,
-			0, "ADB", (void *)0)) {
-		printk(KERN_ERR "ADB: can't get irq %d\n",
-		       adbs->intrs[0].line);
-		return -EAGAIN;
-	}
-
 	out_8(&adb->ctrl.r, 0);
 	out_8(&adb->intr.r, 0);
 	out_8(&adb->error.r, 0);
 	out_8(&adb->active_hi.r, 0xff); /* for now, set all devices active */
 	out_8(&adb->active_lo.r, 0xff);
 	out_8(&adb->autopoll.r, APE);
+
+	if (request_irq(adbs->intrs[0].line, macio_adb_interrupt,
+			0, "ADB", (void *)0)) {
+		printk(KERN_ERR "ADB: can't get irq %d\n",
+		       adbs->intrs[0].line);
+		return -EAGAIN;
+	}
 	out_8(&adb->intr_enb.r, DFB | TAG);
 
 	printk("adb: mac-io driver 1.0 for unified ADB\n");
@@ -129,16 +129,27 @@ int macio_init(void)
 
 static int macio_adb_autopoll(int devs)
 {
+	unsigned long flags;
+	
+	spin_lock_irqsave(&macio_lock, flags);
 	out_8(&adb->active_hi.r, devs >> 8);
 	out_8(&adb->active_lo.r, devs);
 	out_8(&adb->autopoll.r, devs? APE: 0);
+	spin_unlock_irqrestore(&macio_lock, flags);
 	return 0;
 }
 
 static int macio_adb_reset_bus(void)
 {
+	unsigned long flags;
 	int timeout = 1000000;
 
+	/* Hrm... we may want to not lock interrupts for so
+	 * long ... oh well, who uses that chip anyway ? :)
+	 * That function will be seldomly used during boot
+	 * on rare machines, so...
+	 */
+	spin_lock_irqsave(&macio_lock, flags);
 	out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) | ADB_RST);
 	while ((in_8(&adb->ctrl.r) & ADB_RST) != 0) {
 		if (--timeout == 0) {
@@ -146,13 +157,14 @@ static int macio_adb_reset_bus(void)
 			return -1;
 		}
 	}
+	spin_unlock_irqrestore(&macio_lock, flags);
 	return 0;
 }
 
 /* Send an ADB command */
 static int macio_send_request(struct adb_request *req, int sync)
 {
-	unsigned long mflags;
+	unsigned long flags;
 	int i;
 	
 	if (req->data[0] != ADB_PACKET)
@@ -167,8 +179,7 @@ static int macio_send_request(struct adb_request *req, int sync)
 	req->complete = 0;
 	req->reply_len = 0;
 
-	save_flags(mflags);
-	cli();
+	spin_lock_irqsave(&macio_lock, flags);
 	if (current_req != 0) {
 		last_req->next = req;
 		last_req = req;
@@ -176,7 +187,7 @@ static int macio_send_request(struct adb_request *req, int sync)
 		current_req = last_req = req;
 		out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) | TAR);
 	}
-	restore_flags(mflags);
+	spin_unlock_irqrestore(&macio_lock, flags);
 	
 	if (sync) {
 		while (!req->complete)
@@ -190,7 +201,12 @@ static void macio_adb_interrupt(int irq, void *arg, struct pt_regs *regs)
 {
 	int i, n, err;
 	struct adb_request *req;
+	unsigned char ibuf[16];
+	int ibuf_len = 0;
+	int complete = 0;
+	int autopoll = 0;
 	
+	spin_lock(&macio_lock);
 	if (in_8(&adb->intr.r) & TAG) {
 		if ((req = current_req) != 0) {
 			/* put the current request in */
@@ -202,7 +218,10 @@ static void macio_adb_interrupt(int irq, void *arg, struct pt_regs *regs)
 				out_8(&adb->ctrl.r, DTB + CRE);
 			} else {
 				out_8(&adb->ctrl.r, DTB);
-				completed();
+				current_req = req->next;
+				complete = 1;
+				if (current_req)
+					out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) | TAR);
 			}
 		}
 		out_8(&adb->intr.r, 0);
@@ -218,39 +237,42 @@ static void macio_adb_interrupt(int irq, void *arg, struct pt_regs *regs)
 				for (i = 0; i < req->reply_len; ++i)
 					req->reply[i] = in_8(&adb->data[i].r);
 			}
-			completed();
+			current_req = req->next;
+			complete = 1;
+			if (current_req)
+				out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) | TAR);
 		} else if (err == 0) {
 			/* autopoll data */
 			n = in_8(&adb->dcount.r) & HMB;
 			for (i = 0; i < n; ++i)
-				adb_rbuf[i] = in_8(&adb->data[i].r);
-			adb_input(adb_rbuf, n, regs,
-				  in_8(&adb->dcount.r) & APD);
+				ibuf[i] = in_8(&adb->data[i].r);
+			ibuf_len = n;
+			autopoll = (in_8(&adb->dcount.r) & APD) != 0;
 		}
 		out_8(&adb->error.r, 0);
 		out_8(&adb->intr.r, 0);
 	}
-}
-
-static void completed(void)
-{
-	struct adb_request *req = current_req;
-
+	spin_unlock(&macio_lock);
+	if (complete && req) {
+	    void (*done)(struct adb_request *) = req->done;
+	    mb();
 	    req->complete = 1;
-	current_req = req->next;
-	if (current_req)
-		out_8(&adb->ctrl.r, in_8(&adb->ctrl.r) | TAR);
-	if (req->done)
-		(*req->done)(req);
+	    /* Here, we assume that if the request has a done member, the
+    	     * struct request will survive to setting req->complete to 1
+	     */
+	    if (done)
+		(*done)(req);
+	}
+	if (ibuf_len)
+		adb_input(ibuf, ibuf_len, regs, autopoll);
 }
 
 static void macio_adb_poll(void)
 {
 	unsigned long flags;
 
-	save_flags(flags);
-	cli();
+	local_irq_save(flags);
 	if (in_8(&adb->intr.r) != 0)
 		macio_adb_interrupt(0, 0, 0);
-	restore_flags(flags);
+	local_irq_restore(flags);
 }

drivers/macintosh/via-cuda.c

@@ -17,6 +17,7 @@
 #include <linux/sched.h>
 #include <linux/adb.h>
 #include <linux/cuda.h>
+#include <linux/spinlock.h>
 #ifdef CONFIG_PPC
 #include <asm/prom.h>
 #include <asm/machdep.h>
@@ -31,6 +32,7 @@
 #include <linux/init.h>
 
 static volatile unsigned char *via;
+static spinlock_t cuda_lock = SPIN_LOCK_UNLOCKED;
 
 #ifdef CONFIG_MAC
 #define CUDA_IRQ IRQ_MAC_ADB
@@ -386,8 +388,8 @@ cuda_write(struct adb_request *req)
     req->sent = 0;
     req->complete = 0;
     req->reply_len = 0;
-    save_flags(flags); cli();
 
+    spin_lock_irqsave(&cuda_lock, flags);
     if (current_req != 0) {
 	last_req->next = req;
 	last_req = req;
@@ -397,15 +399,14 @@ cuda_write(struct adb_request *req)
 	if (cuda_state == idle)
 	    cuda_start();
     }
+    spin_unlock_irqrestore(&cuda_lock, flags);
 
-    restore_flags(flags);
     return 0;
 }
 
 static void
 cuda_start()
 {
-    unsigned long flags;
     struct adb_request *req;
 
     /* assert cuda_state == idle */
@@ -413,41 +414,46 @@ cuda_start()
     req = current_req;
     if (req == 0)
 	return;
-    save_flags(flags); cli();
-    if ((via[B] & TREQ) == 0) {
-	restore_flags(flags);
+    if ((via[B] & TREQ) == 0)
 	return;			/* a byte is coming in from the CUDA */
-    }
 
     /* set the shift register to shift out and send a byte */
     via[ACR] |= SR_OUT; eieio();
     via[SR] = req->data[0]; eieio();
     via[B] &= ~TIP;
     cuda_state = sent_first_byte;
-    restore_flags(flags);
 }
 
 void
 cuda_poll()
 {
+    if (via[IFR] & SR_INT) {
 	unsigned long flags;
 
-    save_flags(flags);
-    cli();
-    if (via[IFR] & SR_INT)
+	/* cuda_interrupt only takes a normal lock, we disable
+	 * interrupts here to avoid re-entering and thus deadlocking.
+	 * An option would be to disable only the IRQ source with
+	 * disable_irq(), would that work on m68k ? --BenH
+	 */
+	local_irq_save(flags);
 	cuda_interrupt(0, 0, 0);
-    restore_flags(flags);
+	local_irq_restore(flags);
+    }
 }
 
 static void
 cuda_interrupt(int irq, void *arg, struct pt_regs *regs)
 {
     int x, status;
-    struct adb_request *req;
+    struct adb_request *req = NULL;
+    unsigned char ibuf[16];
+    int ibuf_len = 0;
+    int complete = 0;
     
     if ((via[IFR] & SR_INT) == 0)
 	return;
 
+    spin_lock(&cuda_lock);
     status = (~via[B] & (TIP|TREQ)) | (via[ACR] & SR_OUT); eieio();
     /* printk("cuda_interrupt: state=%d status=%x\n", cuda_state, status); */
     switch (cuda_state) {
@@ -502,8 +508,7 @@ cuda_interrupt(int irq, void *arg, struct pt_regs *regs)
 		cuda_state = awaiting_reply;
 	    } else {
 		current_req = req->next;
-		if (req->done)
-		    (*req->done)(req);
+		complete = 1;
 		/* not sure about this */
 		cuda_state = idle;
 		cuda_start();
@@ -544,12 +549,18 @@ cuda_interrupt(int irq, void *arg, struct pt_regs *regs)
 		    memmove(req->reply, req->reply + 2, req->reply_len);
 		}
 	    }
-	    req->complete = 1;
 	    current_req = req->next;
-	    if (req->done)
-		(*req->done)(req);
+	    complete = 1;
 	} else {
-	    cuda_input(cuda_rbuf, reply_ptr - cuda_rbuf, regs);
+	    /* This is tricky. We must break the spinlock to call
+	     * cuda_input. However, doing so means we might get
+	     * re-entered from another CPU getting an interrupt
+	     * or calling cuda_poll(). I ended up using the stack
+	     * (it's only for 16 bytes) and moving the actual
+	     * call to cuda_input to outside of the lock.
+	     */
+	    ibuf_len = reply_ptr - cuda_rbuf;
+	    memcpy(ibuf, cuda_rbuf, ibuf_len);
 	}
 	if (status == TREQ) {
 	    via[B] &= ~TIP; eieio();
@@ -565,6 +576,19 @@ cuda_interrupt(int irq, void *arg, struct pt_regs *regs)
     default:
 	printk("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
     }
+    spin_unlock(&cuda_lock);
+    if (complete && req) {
+    	void (*done)(struct adb_request *) = req->done;
+    	mb();
+    	req->complete = 1;
+    	/* Here, we assume that if the request has a done member, the
+    	 * struct request will survive to setting req->complete to 1
+    	 */
+    	if (done)
+		(*done)(req);
+    }
+    if (ibuf_len)
+	cuda_input(ibuf, ibuf_len, regs);
 }
 
 static void