--tKW2IUtsqtDRztdT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

The other day I noticed that "ifconfig rtw0 down up" (rtw0 at cardbus)
triggered a worrisome "cbb0: power on failed?" message.  Ditto ath0,
atw0, and other Cardbus WLAN cards.  However, if I ran "ifconfig rtw0
down ; sleep 5 ; ifconfig rtw0 up", I didn't see the worrisome message.

Digging deeper, I found a bug in pccbb_power().  After pccbb_power()
commands a cardslot to power-up/-down, the Cardbus bridge will fire
a power-cycle interrupt when the power state has actually changed.
When pccbb_power() powers-down a cardslot, it doesn't wait around for the
power-cycle interrupt.  When pccbb_power() powers-up a cardslot, it does
wait for the interrupt.  If a pccbb_power(UP) follows a pccbb_power(DOWN)
very closely, pccbb_power() will interpret the power-cycle interrupt for
the powerup->powerdown transition as "power-up complete," read the power-state
bit and, finding that power has NOT been activated, complain, "cbb0:
power on failed?"  Then pccbb_power() exits before power-activation is
complete, falsely indicating that the power-activation *is* complete,
so that driver attach/enable routines may blithely configure a card that
is not fully powered-up.  Yuck.

I have attached a patch that makes pccbb_power() consume the power-cycle
event *always*, even on a powerup->powerdown transition.  Also, it DTRT
before interrupts have been enabled.  Comments, please.

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

--tKW2IUtsqtDRztdT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pccbb.c-patch"

Index: pccbb.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pccbb.c,v
retrieving revision 1.128
diff -u -p -u -p -r1.128 pccbb.c
--- pccbb.c	5 Apr 2006 22:16:42 -0000	1.128
+++ pccbb.c	17 Apr 2006 03:54:18 -0000
@@ -1103,8 +1103,7 @@ pccbbintr(arg)
 
 	if (sockevent & CB_SOCKET_EVENT_POWER) {
 		/* XXX: Does not happen when attaching a 16-bit card */
-		sc->sc_pwrcycle++;
-		wakeup(&sc->sc_pwrcycle);
+		wakeup(sc);
 	}
 
 	return (1);
@@ -1294,10 +1293,11 @@ pccbb_power(ct, command)
 	int command;
 {
 	struct pccbb_softc *sc = (struct pccbb_softc *)ct;
-	u_int32_t status, sock_ctrl, reg_ctrl;
+	u_int32_t status, osock_ctrl, sock_ctrl, reg_ctrl;
 	bus_space_tag_t memt = sc->sc_base_memt;
 	bus_space_handle_t memh = sc->sc_base_memh;
-	int on = 0, pwrcycle;
+	int error, on = 0, s, times;
+	struct timeval before, after, diff;
 
 	DPRINTF(("pccbb_power: %s and %s [0x%x]\n",
 	    (command & CARDBUS_VCCMASK) == CARDBUS_VCC_UC ? "CARDBUS_VCC_UC" :
@@ -1314,7 +1314,7 @@ pccbb_power(ct, command)
 	    "UNKNOWN", command));
 
 	status = bus_space_read_4(memt, memh, CB_SOCKET_STAT);
-	sock_ctrl = bus_space_read_4(memt, memh, CB_SOCKET_CTRL);
+	osock_ctrl = sock_ctrl = bus_space_read_4(memt, memh, CB_SOCKET_CTRL);
 
 	switch (command & CARDBUS_VCCMASK) {
 	case CARDBUS_VCC_UC:
@@ -1364,36 +1364,36 @@ pccbb_power(ct, command)
 		break;
 	}
 
-	pwrcycle = sc->sc_pwrcycle;
+	aprint_debug("%s: osock_ctrl %#" PRIx32 " sock_ctrl %#" PRIx32 "\n",
+	    device_xname(&sc->sc_dev), osock_ctrl, sock_ctrl);
 
-#if 0
-	DPRINTF(("sock_ctrl: 0x%x\n", sock_ctrl));
-#endif
-	bus_space_write_4(memt, memh, CB_SOCKET_CTRL, sock_ctrl);
 
-	if (on) {
-		int s, error = 0;
-		struct timeval before, after, diff;
+	microtime(&before);
+	s = splbio();
+	bus_space_write_4(memt, memh, CB_SOCKET_CTRL, sock_ctrl);
 
-		microtime(&before);
-		s = splbio();
-		while (pwrcycle == sc->sc_pwrcycle) {
-			/*
-			 * XXX: Set timeout to 200ms because power cycle event
-			 * will never happen when attaching a 16-bit card.
-			 */
-			if ((error = tsleep(&sc->sc_pwrcycle, PWAIT, "pccpwr",
-			    hz / 5)) == EWOULDBLOCK)
+	if (cold) {
+		for (times = 2000; --times >= 0; ) {
+			status = bus_space_read_4(memt, memh, CB_SOCKET_STAT);
+			if ((status & CB_SOCKET_STAT_PWRCYCLE) != 0 && on)
 				break;
+			else if ((status & CB_SOCKET_STAT_PWRCYCLE) == 0 && !on)
+				break;
+			DELAY(100);
 		}
-		splx(s);
-		microtime(&after);
-		timersub(&after, &before, &diff);
-		aprint_debug("%s: wait took%s %ld.%06lds\n",
-			sc->sc_dev.dv_xname,
-		    	error == EWOULDBLOCK ? " too long" : "",
-		    	diff.tv_sec, diff.tv_usec);
+		error = (times == 0) ? EWOULDBLOCK : 0;
+	} else {
+		/*
+		 * XXX: Set timeout to 200ms because power cycle event
+		 * will never happen when attaching a 16-bit card.
+		 */
+		error = tsleep(sc, PWAIT, "pccpwr", hz / 5);
 	}
+	splx(s);
+	microtime(&after);
+	timersub(&after, &before, &diff);
+	aprint_debug("%s: wait took%s %ld.%06lds\n", device_xname(&sc->sc_dev),
+	    error == EWOULDBLOCK ? " too long" : "", diff.tv_sec, diff.tv_usec);
 
 	status = bus_space_read_4(memt, memh, CB_SOCKET_STAT);
 

--tKW2IUtsqtDRztdT--