Subject: bug in cbb power reactivation (patch)
To: None <tech-kern@netbsd.org>
From: David Young <dyoung@pobox.com>
List: tech-kern
Date: 04/16/2006 22:55:41
--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--