Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jun 2007 23:44:19 -0600 (MDT)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        mobile@freebsd.org
Subject:   cbb patch: fixing the can't reinsert a card problem
Message-ID:  <20070620.234419.-1540389296.imp@bsdimp.com>

next in thread | raw e-mail | index | archive | help
----Next_Part(Wed_Jun_20_23_44_19_2007_461)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

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(Wed_Jun_20_23_44_19_2007_461)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="cbb-1.diff"

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 */

----Next_Part(Wed_Jun_20_23_44_19_2007_461)----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070620.234419.-1540389296.imp>