cbb patch: fixing the can't reinsert a card problem

M. Warner Losh imp at bsdimp.com
Thu Jun 21 05:45:55 UTC 2007


I've converted the main cbb card status change interrupt from a
ithread to a filter.  It didn't make sense to schedule a thread to
wakeup a thread to me, so I've modified the status and power change
routines a little so that we use a filter (fast interrupt) instead of
an ithread.  To do this, I had to ditch a lot of mutex locking and
condvars, and go to direct wakeup calls.

This should make things more reliable.  When we get real filters, this
should cut down on the work done in the ithread.  I believe that this
also makes the detection of insert/removal more reliable.  Before this
change, if I was to insert a card that didnt attach, then I couldn't
get future card insert/remove events to happen.

Please test this and let me know what you think.  I plan on committing
this once I get some confirmation from the testers.  If you could also
review it too and let me know what you think...

Warner
-------------- next part --------------
Index: pccbb.c
===================================================================
RCS file: /cache/ncvs/src/sys/dev/pccbb/pccbb.c,v
retrieving revision 1.164
diff -u -r1.164 pccbb.c
--- pccbb.c	4 Jun 2007 05:59:44 -0000	1.164
+++ pccbb.c	21 Jun 2007 05:21:55 -0000
@@ -344,7 +344,7 @@
 	sc->flags |= CBB_KTHREAD_DONE;
 	while (sc->flags & CBB_KTHREAD_RUNNING) {
 		DEVPRINTF((sc->dev, "Waiting for thread to die\n"));
-		cv_broadcast(&sc->cv);
+		wakeup(&sc->intrhand);
 		msleep(sc->event_thread, &sc->mtx, PWAIT, "cbbun", 0);
 	}
 	mtx_unlock(&sc->mtx);
@@ -353,8 +353,6 @@
 	bus_release_resource(brdev, SYS_RES_MEMORY, CBBR_SOCKBASE,
 	    sc->base_res);
 	mtx_destroy(&sc->mtx);
-	cv_destroy(&sc->cv);
-	cv_destroy(&sc->powercv);
 	return (0);
 }
 
@@ -435,11 +433,8 @@
 	}
 	free(devlist, M_TEMP);
 
-	if (wake > 0) {
-		mtx_lock(&sc->mtx);
-		cv_signal(&sc->cv);
-		mtx_unlock(&sc->mtx);
-	}
+	if (wake > 0)
+		wakeup(&sc->intrhand);
 }
 
 void
@@ -519,12 +514,14 @@
 		 * a chance to run.
 		 */
 		mtx_lock(&sc->mtx);
-		cbb_setb(sc, CBB_SOCKET_MASK, CBB_SOCKET_MASK_CD);
-		cv_wait(&sc->cv, &sc->mtx);
+		printf("Setting change request and waiting\n");
+		cbb_setb(sc, CBB_SOCKET_MASK, CBB_SOCKET_MASK_CD | CBB_SOCKET_MASK_CSTS);
+		msleep(&sc->intrhand, &sc->mtx, PZERO, "-", 0);
 		err = 0;
+		printf("Wokeup after change, debouncing\n");
 		while (err != EWOULDBLOCK &&
 		    (sc->flags & CBB_KTHREAD_DONE) == 0)
-			err = cv_timedwait(&sc->cv, &sc->mtx, hz / 4);
+			err = msleep(&sc->intrhand, &sc->mtx, PZERO, "-", hz / 5);
 	}
 	DEVPRINTF((sc->dev, "Thread terminating\n"));
 	sc->flags &= ~CBB_KTHREAD_RUNNING;
@@ -800,7 +797,7 @@
 		sane = 10;
 		while (!(cbb_get(sc, CBB_SOCKET_STATE) & CBB_STATE_POWER_CYCLE) &&
 		    cnt == sc->powerintr && sane-- > 0)
-			cv_timedwait(&sc->powercv, &sc->mtx, hz / 20);
+			msleep(&sc->powerintr, &sc->mtx, PZERO, "-", hz / 20);
 		mtx_unlock(&sc->mtx);
 		/*
 		 * The TOPIC95B requires a little bit extra time to get
@@ -1534,9 +1531,7 @@
 	cbb_setb(sc, CBB_SOCKET_MASK, CBB_SOCKET_MASK_CD);
 
 	/* Signal the thread to wakeup. */
-	mtx_lock(&sc->mtx);
-	cv_signal(&sc->cv);
-	mtx_unlock(&sc->mtx);
+	wakeup(&sc->intrhand);
 
 	error = bus_generic_resume(self);
 
Index: pccbb_pci.c
===================================================================
RCS file: /cache/ncvs/src/sys/dev/pccbb/pccbb_pci.c,v
retrieving revision 1.25
diff -u -r1.25 pccbb_pci.c
--- pccbb_pci.c	4 Jun 2007 05:59:44 -0000	1.25
+++ pccbb_pci.c	21 Jun 2007 05:32:50 -0000
@@ -117,7 +117,7 @@
 		pci_read_config(DEV, REG, SIZE) MASK1) MASK2, SIZE)
 
 static void cbb_chipinit(struct cbb_softc *sc);
-static void cbb_pci_intr(void *arg);
+static int cbb_pci_filt(void *arg);
 
 static struct yenta_chipinfo {
 	uint32_t yc_id;
@@ -310,8 +310,6 @@
 
 	parent = device_get_parent(brdev);
 	mtx_init(&sc->mtx, device_get_nameunit(brdev), "cbb", MTX_DEF);
-	cv_init(&sc->cv, "cbb cv");
-	cv_init(&sc->powercv, "cbb cv");
 	sc->chipset = cbb_chipset(pci_get_devid(brdev), NULL);
 	sc->dev = brdev;
 	sc->cbdev = NULL;
@@ -328,7 +326,6 @@
 	if (!sc->base_res) {
 		device_printf(brdev, "Could not map register memory\n");
 		mtx_destroy(&sc->mtx);
-		cv_destroy(&sc->cv);
 		return (ENOMEM);
 	} else {
 		DEVPRINTF((brdev, "Found memory at %08lx\n",
@@ -410,7 +407,7 @@
 	}
 
 	if (bus_setup_intr(brdev, sc->irq_res, INTR_TYPE_AV | INTR_MPSAFE,
-	    NULL, cbb_pci_intr, sc, &sc->intrhand)) {
+	    cbb_pci_filt, NULL, sc, &sc->intrhand)) {
 		device_printf(brdev, "couldn't establish interrupt\n");
 		goto err;
 	}
@@ -445,7 +442,6 @@
 		    sc->base_res);
 	}
 	mtx_destroy(&sc->mtx);
-	cv_destroy(&sc->cv);
 	return (ENOMEM);
 }
 
@@ -680,8 +676,9 @@
 	return (0);
 }
 
-static void
-cbb_pci_intr(void *arg)
+#define DELTA (CBB_SOCKET_MASK_CD)
+static int
+cbb_pci_filt(void *arg)
 {
 	struct cbb_softc *sc = arg;
 	uint32_t sockevent;
@@ -699,9 +696,6 @@
 	 */
 	sockevent = cbb_get(sc, CBB_SOCKET_EVENT);
 	if (sockevent != 0 && (sockevent & ~CBB_SOCKET_EVENT_VALID_MASK) == 0) {
-		/* ack the interrupt */
-		cbb_set(sc, CBB_SOCKET_EVENT, sockevent);
-
 		/*
 		 * If anything has happened to the socket, we assume that
 		 * the card is no longer OK, and we shouldn't call its
@@ -715,23 +709,22 @@
 		 * of the pccard software used a similar trick and achieved
 		 * excellent results.
 		 */
-		if (sockevent & CBB_SOCKET_EVENT_CD) {
-			mtx_lock(&sc->mtx);
-			cbb_clrb(sc, CBB_SOCKET_MASK, CBB_SOCKET_MASK_CD);
+		if (sockevent & DELTA) {
+			cbb_clrb(sc, CBB_SOCKET_MASK, DELTA);
+			cbb_set(sc, CBB_SOCKET_EVENT, DELTA);
 			sc->cardok = 0;
 			cbb_disable_func_intr(sc);
-			cv_signal(&sc->cv);
-			mtx_unlock(&sc->mtx);
+			wakeup(&sc->intrhand);
 		}
 		/*
 		 * If we get a power interrupt, wakeup anybody that might
 		 * be waiting for one.
 		 */
 		if (sockevent & CBB_SOCKET_EVENT_POWER) {
-			mtx_lock(&sc->mtx);
+			cbb_clrb(sc, CBB_SOCKET_MASK, CBB_SOCKET_EVENT_POWER);
+			cbb_set(sc, CBB_SOCKET_EVENT, CBB_SOCKET_EVENT_POWER);
 			sc->powerintr++;
-			cv_signal(&sc->powercv);
-			mtx_unlock(&sc->mtx);
+			wakeup((void *)&sc->powerintr);
 		}
 	}
 	/*
@@ -747,6 +740,7 @@
 	 * the event independent of the CBB_SOCKET_EVENT_CD above.
 	 */
 	exca_getb(&sc->exca[0], EXCA_CSC);
+	return FILTER_HANDLED;
 }
 
 /************************************************************************/
Index: pccbbvar.h
===================================================================
RCS file: /cache/ncvs/src/sys/dev/pccbb/pccbbvar.h,v
retrieving revision 1.31
diff -u -r1.31 pccbbvar.h
--- pccbbvar.h	4 Jun 2007 05:59:44 -0000	1.31
+++ pccbbvar.h	21 Jun 2007 05:24:13 -0000
@@ -66,8 +66,6 @@
 	unsigned int	secbus;
 	unsigned int	subbus;
 	struct mtx	mtx;
-	struct cv	cv;
-	struct cv	powercv;
 	int		cardok;
 	u_int32_t	flags;
 #define	CBB_16BIT_CARD		0x20000000
@@ -88,7 +86,7 @@
 	device_t	cbdev;
 	struct proc	*event_thread;
 	void (*chipinit)(struct cbb_softc *);
-	volatile int	powerintr;
+	int	powerintr;
 };
 
 /* result of detect_card */


More information about the freebsd-mobile mailing list