Subject: ncr53c9x changes proposed
To: None <tech-kern@NetBSD.ORG>
From: Gordon W. Ross <gwr@mc.com>
List: tech-kern
Date: 03/28/1997 12:02:12
Here are a few changes I propose for the ncr53c9x driver:
(1) Always schedule a timeout before the first occasion where
we return from the driver expecting to come back due to an
interrupt, because the interrupt might not happen. This
makes the driver imune to problems where the SCSI chip may
fail to complete a selection under some circumstances.
(and generally makes the driver more "bullet-proof 8^)
(2) Do the untimeout in ncr53c9x_done instead of just before
almost every call to ncr53c9x_done as was done previously.
(3) Make ncr53c9x_sense schedule its own timeout for the new
command it is starting (request sense), separate from the
timeout for the command that just completed.
(4) Allow separate control over the "per-target trace" feature
via the NCR53C9X_DEBUG value. (If I include the per-target
trace buffers in the softc, it takes painfully long for kgdb
over a serial line to print out the softc - like a minute!)
(5) Add alignment padding to some structs (be nice to m68k...)
Please review and comment.
Thanks,
Gordon
diff -rc ../dev.orig/ic/ncr53c9x.c ./ic/ncr53c9x.c
*** ../dev.orig/ic/ncr53c9x.c Wed Mar 26 20:12:53 1997
--- ./ic/ncr53c9x.c Fri Mar 28 11:27:43 1997
***************
*** 289,300 ****
sc->sc_state = NCR_CLEANING;
if ((ecb = sc->sc_nexus) != NULL) {
ecb->xs->error = XS_DRIVER_STUFFUP;
- untimeout(ncr53c9x_timeout, ecb);
ncr53c9x_done(sc, ecb);
}
while ((ecb = sc->nexus_list.tqh_first) != NULL) {
ecb->xs->error = XS_DRIVER_STUFFUP;
- untimeout(ncr53c9x_timeout, ecb);
ncr53c9x_done(sc, ecb);
}
}
--- 289,298 ----
***************
*** 417,422 ****
--- 415,429 ----
/* new state NCR_SELECTING */
sc->sc_state = NCR_SELECTING;
+ /*
+ * Schedule the timeout now, the first time we will go away
+ * expecting to come back due to an interrupt, because it is
+ * always possible that the interrupt might never happen.
+ */
+ if ((ecb->xs->flags & SCSI_POLL) == 0)
+ timeout(ncr53c9x_timeout, ecb,
+ (ecb->timeout * hz) / 1000);
+
NCRCMD(sc, NCRCMD_FLUSH);
/*
***************
*** 650,655 ****
--- 657,663 ----
ecb->daddr = (char *)&xs->sense;
ecb->dleft = sizeof(struct scsi_sense_data);
ecb->flags |= ECB_SENSE;
+ ecb->timeout = NCR_SENSE_TIMEOUT;
ti->senses++;
if (ecb->flags & ECB_NEXUS)
ti->lubusy &= ~(1 << sc_link->lun);
***************
*** 677,682 ****
--- 685,692 ----
NCR_TRACE(("[ncr53c9x_done(error:%x)] ", xs->error));
+ untimeout(ncr53c9x_timeout, ecb);
+
/*
* Now, if we've come here with no error code, i.e. we've kept the
* initial XS_NOERROR, and the status code signals that we should
***************
*** 1447,1452 ****
--- 1457,1463 ----
goto reset;
}
printf("sending REQUEST SENSE\n");
+ untimeout(ncr53c9x_timeout, ecb);
ncr53c9x_sense(sc, ecb);
goto out;
}
***************
*** 1499,1504 ****
--- 1510,1516 ----
case NCR_SELECTING:
sc->sc_msgpriq = sc->sc_msgout = sc->sc_msgoutq = 0;
sc->sc_flags = 0;
+ ecb = sc->sc_nexus;
if (sc->sc_espintr & NCRINTR_RESEL) {
/*
***************
*** 1508,1517 ****
*/
if (sc->sc_state == NCR_SELECTING) {
NCR_MISC(("backoff selector "));
! sc_link = sc->sc_nexus->xs->sc_link;
ti = &sc->sc_tinfo[sc_link->target];
TAILQ_INSERT_HEAD(&sc->ready_list,
! sc->sc_nexus, chain);
ecb = sc->sc_nexus = NULL;
}
sc->sc_state = NCR_RESELECTED;
--- 1520,1530 ----
*/
if (sc->sc_state == NCR_SELECTING) {
NCR_MISC(("backoff selector "));
! untimeout(ncr53c9x_timeout, ecb);
! sc_link = ecb->xs->sc_link;
ti = &sc->sc_tinfo[sc_link->target];
TAILQ_INSERT_HEAD(&sc->ready_list,
! ecb, chain);
ecb = sc->sc_nexus = NULL;
}
sc->sc_state = NCR_RESELECTED;
***************
*** 1651,1659 ****
sc->sc_dleft = ecb->dleft;
/* On our first connection, schedule a timeout. */
! if ((ecb->xs->flags & SCSI_POLL) == 0)
! timeout(ncr53c9x_timeout, ecb,
! (ecb->timeout * hz) / 1000);
sc->sc_state = NCR_CONNECTED;
break;
--- 1664,1670 ----
sc->sc_dleft = ecb->dleft;
/* On our first connection, schedule a timeout. */
! /* XXX - Now done in _sched to catch select hang. */
sc->sc_state = NCR_CONNECTED;
break;
***************
*** 1830,1836 ****
return 1;
finish:
- untimeout(ncr53c9x_timeout, ecb);
ncr53c9x_done(sc, ecb);
goto out;
--- 1841,1846 ----
***************
*** 1895,1901 ****
sc->sc_state, sc->sc_nexus, sc->sc_phase, sc->sc_prevphase,
(long)sc->sc_dleft, sc->sc_msgpriq, sc->sc_msgout,
NCRDMA_ISACTIVE(sc) ? "DMA active" : "");
! #if NCR53C9X_DEBUG > 0
printf("TRACE: %s.", ecb->trace);
#endif
--- 1905,1911 ----
sc->sc_state, sc->sc_nexus, sc->sc_phase, sc->sc_prevphase,
(long)sc->sc_dleft, sc->sc_msgpriq, sc->sc_msgout,
NCRDMA_ISACTIVE(sc) ? "DMA active" : "");
! #if NCR53C9X_DEBUG > 1
printf("TRACE: %s.", ecb->trace);
#endif
diff -rc ../dev.orig/ic/ncr53c9xvar.h ./ic/ncr53c9xvar.h
*** ../dev.orig/ic/ncr53c9xvar.h Mon Mar 17 11:04:59 1997
--- ./ic/ncr53c9xvar.h Fri Mar 28 11:21:15 1997
***************
*** 60,68 ****
--- 60,70 ----
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+ /* Set this to 1 for normal debug, or 2 for per-target tracing. */
#define NCR53C9X_DEBUG 0
#define NCR_ABORT_TIMEOUT 2000 /* time to wait for abort */
+ #define NCR_SENSE_TIMEOUT 1000 /* time to wait for sense */
#define FREQTOCCF(freq) (((freq + 4) / 5))
***************
*** 101,112 ****
char *daddr; /* Saved data pointer */
int dleft; /* Residue */
u_char stat; /* SCSI status byte */
! #if NCR53C9X_DEBUG > 0
char trace[1000];
#endif
};
! #if NCR53C9X_DEBUG > 0
#define ECB_TRACE(ecb, msg, a, b) do { \
const char *f = "[" msg "]"; \
int n = strlen((ecb)->trace); \
--- 103,115 ----
char *daddr; /* Saved data pointer */
int dleft; /* Residue */
u_char stat; /* SCSI status byte */
+ u_char pad[3];
! #if NCR53C9X_DEBUG > 1
char trace[1000];
#endif
};
! #if NCR53C9X_DEBUG > 1
#define ECB_TRACE(ecb, msg, a, b) do { \
const char *f = "[" msg "]"; \
int n = strlen((ecb)->trace); \
***************
*** 138,143 ****
--- 141,147 ----
#define T_RSELECTOFF 0x20 /* .. */
u_char period; /* Period suggestion */
u_char offset; /* Offset suggestion */
+ u_char pad[3];
} tinfo_t;
/* Register a linenumber (for debugging) */