Most of these changes to isp are to allow for isp.ko unloading.

We also revive loop down freezes. We also externaliz within isp
isp_prt_endcmd so something outside the core module can print
something about a command completing. Also some work in progress to
assist in handling timed out commands better.

Partially Sponsored by: Panasas
Approved by:	re (kib)
MFC after:	1 month

5 files changed, 186 insertions, 59 deletions

diff --git a/sys/dev/isp/isp.c b/sys/dev/isp/isp.c
index 95f2add24743..dfba62e6e041 100644
--- a/sys/dev/isp/isp.c
+++ b/sys/dev/isp/isp.c
@@ -102,7 +102,6 @@ static const uint8_t alpa_map[] = {
 /*
  * Local function prototypes.
  */
-static void isp_prt_endcmd(ispsoftc_t *, XS_T *);
 static int isp_parse_async(ispsoftc_t *, uint16_t);
 static int isp_parse_async_fc(ispsoftc_t *, uint16_t);
 static int isp_handle_other_response(ispsoftc_t *, int, isphdr_t *, uint32_t *);
@@ -5391,7 +5390,7 @@ out:
  * Support routines.
  */
 
-static void
+void
 isp_prt_endcmd(ispsoftc_t *isp, XS_T *xs)
 {
 	char cdbstr[16 * 5 + 1];
diff --git a/sys/dev/isp/isp_freebsd.c b/sys/dev/isp/isp_freebsd.c
index 120ae11e5bfd..8d62f5ebdece 100644
--- a/sys/dev/isp/isp_freebsd.c
+++ b/sys/dev/isp/isp_freebsd.c
@@ -108,15 +108,13 @@ isp_attach_chan(ispsoftc_t *isp, struct cam_devq *devq, int chan)
 		return (EIO);
 	}
 	ISP_UNLOCK(isp);
-
-	if (xpt_create_path(&path, NULL, cam_sim_path(sim), CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD) != CAM_REQ_CMP) {
+	if (xpt_create_path_unlocked(&path, NULL, cam_sim_path(sim), CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD) != CAM_REQ_CMP) {
 		ISP_LOCK(isp);
 		xpt_bus_deregister(cam_sim_path(sim));
 		ISP_UNLOCK(isp);
 		cam_sim_free(sim, FALSE);
 		return (ENXIO);
 	}
-
 	xpt_setup_ccb(&csa.ccb_h, path, 5);
 	csa.ccb_h.func_code = XPT_SASYNC_CB;
 	csa.event_enable = AC_LOST_DEVICE;
@@ -190,12 +188,16 @@ isp_attach(ispsoftc_t *isp)
 
 	isp->isp_osinfo.ehook.ich_func = isp_intr_enable;
 	isp->isp_osinfo.ehook.ich_arg = isp;
+	/*
+	 * Haha. Set this first, because if we're loaded as a module isp_intr_enable
+	 * will be called right awawy, which will clear isp_osinfo.ehook_active,
+	 * which would be unwise to then set again later.
+	 */
+	isp->isp_osinfo.ehook_active = 1;
 	if (config_intrhook_establish(&isp->isp_osinfo.ehook) != 0) {
 		isp_prt(isp, ISP_LOGERR, "could not establish interrupt enable hook");
 		return (-EIO);
 	}
-	isp->isp_osinfo.ehook_active = 1;
-
 
 	/*
 	 * Create the device queue for our SIM(s).
@@ -252,20 +254,33 @@ unwind:
 	return (-1);
 }
 
-void
+int
 isp_detach(ispsoftc_t *isp)
 {
+	struct cam_sim *sim;
+	struct cam_path *path;
+	struct ccb_setasync csa;
 	int chan;
 
 	ISP_LOCK(isp);
+	for (chan = isp->isp_nchan - 1; chan >= 0; chan -= 1) {
+		if (IS_FC(isp)) {
+			sim = ISP_FC_PC(isp, chan)->sim;
+			path = ISP_FC_PC(isp, chan)->path;
+		} else {
+			sim = ISP_SPI_PC(isp, chan)->sim;
+			path = ISP_SPI_PC(isp, chan)->path;
+		}
+		if (sim->refcount > 2) {
+			ISP_UNLOCK(isp);
+			return (EBUSY);
+		}
+	}
 	if (isp->isp_osinfo.timer_active) {
 		callout_stop(&isp->isp_osinfo.tmo);
 		isp->isp_osinfo.timer_active = 0;
 	}
-	ISP_UNLOCK(isp);
 	for (chan = isp->isp_nchan - 1; chan >= 0; chan -= 1) {
-		struct cam_sim *sim;
-		struct cam_path *path;
 		if (IS_FC(isp)) {
 			sim = ISP_FC_PC(isp, chan)->sim;
 			path = ISP_FC_PC(isp, chan)->path;
@@ -273,12 +288,17 @@ isp_detach(ispsoftc_t *isp)
 			sim = ISP_SPI_PC(isp, chan)->sim;
 			path = ISP_SPI_PC(isp, chan)->path;
 		}
+		xpt_setup_ccb(&csa.ccb_h, path, 5);
+		csa.ccb_h.func_code = XPT_SASYNC_CB;
+		csa.event_enable = 0;
+		csa.callback = isp_cam_async;
+		csa.callback_arg = sim;
+		xpt_action((union ccb *)&csa);
 		xpt_free_path(path);
-		ISP_LOCK(isp);
 		xpt_bus_deregister(cam_sim_path(sim));
-		ISP_UNLOCK(isp);
 		cam_sim_free(sim, FALSE);
 	}
+	ISP_UNLOCK(isp);
 	if (isp->isp_osinfo.cdev) {
 		destroy_dev(isp->isp_osinfo.cdev);
 		isp->isp_osinfo.cdev = NULL;
@@ -291,6 +311,7 @@ isp_detach(ispsoftc_t *isp)
 		cam_simq_free(isp->isp_osinfo.devq);
 		isp->isp_osinfo.devq = NULL;
 	}
+	return (0);
 }
 
 static void
@@ -309,6 +330,20 @@ isp_freeze_loopdown(ispsoftc_t *isp, int chan, char *msg)
 	}
 }
 
+static void
+isp_unfreeze_loopdown(ispsoftc_t *isp, int chan)
+{
+	if (IS_FC(isp)) {
+		struct isp_fc *fc = ISP_FC_PC(isp, chan);
+		int wasfrozen = fc->simqfrozen & SIMQFRZ_LOOPDOWN;
+		fc->simqfrozen &= ~SIMQFRZ_LOOPDOWN;
+		if (wasfrozen && fc->simqfrozen == 0) {
+			isp_prt(isp, ISP_LOGSANCFG|ISP_LOGDEBUG0, "%s: Chan %d releasing simq", __func__, chan);
+			xpt_release_simq(fc->sim, 1);
+		}
+	}
+}
+
 
 static int
 ispioctl(struct cdev *dev, u_long c, caddr_t addr, int flags, struct thread *td)
@@ -702,6 +737,7 @@ isp_intr_enable(void *arg)
 			}
 		}
 	}
+	isp->isp_osinfo.ehook_active = 0;
 	ISP_UNLOCK(isp);
 	/* Release our hook so that the boot can continue. */
 	config_intrhook_disestablish(&isp->isp_osinfo.ehook);
@@ -1308,7 +1344,7 @@ done:
 		xpt_print(ccb->ccb_h.path, "now disabled for target mode\n");
 	}
 	if (tptr) {
-		rls_lun_statep(isp, tptr);
+		destroy_lun_state(isp, tptr);
 	}
 	isp->isp_osinfo.rptr = NULL;
 	isp->isp_osinfo.tmbusy = 0;
@@ -3787,19 +3823,20 @@ static void
 isp_cam_async(void *cbarg, uint32_t code, struct cam_path *path, void *arg)
 {
 	struct cam_sim *sim;
+	int bus, tgt;
 	ispsoftc_t *isp;
 
 	sim = (struct cam_sim *)cbarg;
 	isp = (ispsoftc_t *) cam_sim_softc(sim);
+	bus = cam_sim_bus(sim);
+	tgt = xpt_path_target_id(path);
+
 	switch (code) {
 	case AC_LOST_DEVICE:
 		if (IS_SCSI(isp)) {
 			uint16_t oflags, nflags;
-			int bus = cam_sim_bus(sim);
 			sdparam *sdp = SDPARAM(isp, bus);
-			int tgt;
 
-			tgt = xpt_path_target_id(path);
 			if (tgt >= 0) {
 				nflags = sdp->isp_devparam[tgt].nvrm_flags;
 #ifndef	ISP_TARGET_MODE
@@ -3843,11 +3880,32 @@ isp_watchdog(void *arg)
 {
 	struct ccb_scsiio *xs = arg;
 	ispsoftc_t *isp;
-	uint32_t handle;
+	uint32_t ohandle = ISP_HANDLE_FREE, handle;
 
 	isp = XS_ISP(xs);
 
 	handle = isp_find_handle(isp, xs);
+
+	if (handle != ISP_HANDLE_FREE && !XS_CMD_WPEND_P(xs)) {
+		isp_xs_prt(isp, xs, ISP_LOGWARN, "first watchdog (handle 0x%x) timed out- deferring for grace period", handle);
+		callout_reset(&PISP_PCMD(xs)->wdog, 2 * hz, isp_watchdog, xs);
+		XS_CMD_S_WPEND(xs);
+		return;
+	}
+	XS_C_TACTIVE(xs);
+
+	/*
+	 * Hand crank the interrupt code just to be sure the command isn't stuck somewhere.
+	 */
+	if (handle != ISP_HANDLE_FREE) {
+		uint32_t isr;
+		uint16_t sema, mbox;
+		if (ISP_READ_ISR(isp, &isr, &sema, &mbox) != 0) {
+			isp_intr(isp, isr, sema, mbox);
+		}
+		ohandle = handle;
+		handle = isp_find_handle(isp, xs);
+	}
 	if (handle != ISP_HANDLE_FREE) {
 		/*
 		 * Try and make sure the command is really dead before
@@ -3884,7 +3942,14 @@ isp_watchdog(void *arg)
 		isp_destroy_handle(isp, handle);
 		isp_prt(isp, ISP_LOGERR, "%s: timeout for handle 0x%x", __func__, handle);
 		XS_SETERR(xs, CAM_CMD_TIMEOUT);
+		isp_prt_endcmd(isp, xs);
 		isp_done(xs);
+	} else {
+		if (ohandle != ISP_HANDLE_FREE) {
+			isp_prt(isp, ISP_LOGWARN, "%s: timeout for handle 0x%x, recovered during interrupt", __func__, ohandle);
+		} else {
+			isp_prt(isp, ISP_LOGWARN, "%s: timeout for handle already free", __func__);
+		}
 	}
 }
 
@@ -4013,7 +4078,7 @@ isp_ldt_task(void *arg, int pending)
 	ispsoftc_t *isp = fc->isp;
 	int chan = fc - isp->isp_osinfo.pc.fc;
 	fcportdb_t *lp;
-	int dbidx, tgt;
+	int dbidx, tgt, i;
 
 	ISP_LOCK(isp);
 	isp_prt(isp, ISP_LOGSANCFG|ISP_LOGDEBUG0, "Chan %d Loop Down Timer expired @ %lu", chan, (unsigned long) time_uptime);
@@ -4035,6 +4100,23 @@ isp_ldt_task(void *arg, int pending)
 		/*
 		 * XXX: CLEAN UP AND COMPLETE ANY PENDING COMMANDS FIRST!
 		 */
+		
+
+		for (i = 0; i < isp->isp_maxcmds; i++) {
+			struct ccb_scsiio *xs;
+
+			if (!ISP_VALID_HANDLE(isp, isp->isp_xflist[i].handle)) {
+				continue;
+			}
+			if ((xs = isp->isp_xflist[i].cmd) == NULL) {
+				continue;
+                        }
+			if (dbidx != (FCPARAM(isp, chan)->isp_dev_map[XS_TGT(xs)] - 1)) {
+				continue;
+			}
+			isp_prt(isp, ISP_LOGWARN, "command handle 0x%08x for %d.%d.%d orphaned by loop down timeout",
+			    isp->isp_xflist[i].handle, chan, XS_TGT(xs), XS_LUN(xs));
+		}
 
 		/*
 		 * Mark that we've announced that this device is gone....
@@ -4056,6 +4138,9 @@ isp_ldt_task(void *arg, int pending)
 		isp_make_gone(isp, chan, tgt);
 	}
 
+	if (FCPARAM(isp, chan)->role & ISP_ROLE_INITIATOR) {
+		isp_unfreeze_loopdown(isp, chan);
+	}
 	/*
 	 * The loop down timer has expired. Wake up the kthread
 	 * to notice that fact (or make it false).
@@ -4063,6 +4148,7 @@ isp_ldt_task(void *arg, int pending)
 	fc->loop_dead = 1;
 	fc->loop_down_time = fc->loop_down_limit+1;
 	wakeup(fc);
+	ISP_UNLOCK(isp);
 }
 
 static void
@@ -4076,7 +4162,7 @@ isp_kthread(void *arg)
 	mtx_lock(&isp->isp_osinfo.lock);
 
 	for (;;) {
-		int wasfrozen, lb, lim;
+		int lb, lim;
 
 		isp_prt(isp, ISP_LOGSANCFG|ISP_LOGDEBUG0, "%s: Chan %d checking FC state", __func__, chan);
 		lb = isp_fc_runstate(isp, chan, 250000);
@@ -4147,12 +4233,7 @@ isp_kthread(void *arg)
 		 */
 
 		if (FCPARAM(isp, chan)->loop_seen_once || fc->loop_dead) {
-			wasfrozen = fc->simqfrozen & SIMQFRZ_LOOPDOWN;
-			fc->simqfrozen &= ~SIMQFRZ_LOOPDOWN;
-			if (wasfrozen && fc->simqfrozen == 0) {
-				isp_prt(isp, ISP_LOGSANCFG|ISP_LOGDEBUG0, "%s: Chan %d releasing simq", __func__, chan);
-				xpt_release_simq(fc->sim, 1);
-			}
+			isp_unfreeze_loopdown(isp, chan);
 		}
 
 		isp_prt(isp, ISP_LOGSANCFG|ISP_LOGDEBUG0, "%s: Chan %d sleep time %d", __func__, chan, slp);
@@ -4251,6 +4332,7 @@ isp_action(struct cam_sim *sim, union ccb *ccb)
 				ts = 60*1000;
 			}
 			ts = isp_mstohz(ts);
+			XS_S_TACTIVE(ccb);
 			callout_reset(&PISP_PCMD(ccb)->wdog, ts, isp_watchdog, ccb);
 			break;
 		case CMD_RQLATER:
@@ -4777,7 +4859,9 @@ isp_action(struct cam_sim *sim, union ccb *ccb)
 			/*
 			 * Set base transfer capabilities for Fibre Channel, for this HBA.
 			 */
-			if (IS_24XX(isp)) {
+			if (IS_25XX(isp)) {
+				cpi->base_transfer_speed = 8000000;
+			} else if (IS_24XX(isp)) {
 				cpi->base_transfer_speed = 4000000;
 			} else if (IS_23XX(isp)) {
 				cpi->base_transfer_speed = 2000000;
@@ -4823,6 +4907,7 @@ void
 isp_done(XS_T *sccb)
 {
 	ispsoftc_t *isp = XS_ISP(sccb);
+	uint32_t status;
 
 	if (XS_NOERR(sccb))
 		XS_SETERR(sccb, CAM_REQ_CMP);
@@ -4837,8 +4922,10 @@ isp_done(XS_T *sccb)
 	}
 
 	sccb->ccb_h.status &= ~CAM_SIM_QUEUED;
-	if ((sccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
-		isp_prt(isp, ISP_LOGDEBUG0, "target %d lun %d CAM status 0x%x SCSI status 0x%x", XS_TGT(sccb), XS_LUN(sccb), sccb->ccb_h.status, sccb->scsi_status);
+	status = sccb->ccb_h.status & CAM_STATUS_MASK;
+	if (status != CAM_REQ_CMP) {
+		if (status != CAM_SEL_TIMEOUT)
+			isp_prt(isp, ISP_LOGDEBUG0, "target %d lun %d CAM status 0x%x SCSI status 0x%x", XS_TGT(sccb), XS_LUN(sccb), sccb->ccb_h.status, sccb->scsi_status);
 		if ((sccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
 			sccb->ccb_h.status |= CAM_DEV_QFRZN;
 			xpt_freeze_devq(sccb->ccb_h.path, 1);
@@ -4850,7 +4937,8 @@ isp_done(XS_T *sccb)
 	}
 
 	XS_CMD_S_DONE(sccb);
-	callout_stop(&PISP_PCMD(sccb)->wdog);
+	if (XS_TACTIVE_P(sccb))
+		callout_stop(&PISP_PCMD(sccb)->wdog);
 	XS_CMD_S_CLEAR(sccb);
 	isp_free_pcmd(isp, (union ccb *) sccb);
 	xpt_done((union ccb *) sccb);
diff --git a/sys/dev/isp/isp_freebsd.h b/sys/dev/isp/isp_freebsd.h
index cfa2fd541d12..1fb5fc56a89f 100644
--- a/sys/dev/isp/isp_freebsd.h
+++ b/sys/dev/isp/isp_freebsd.h
@@ -582,7 +582,7 @@ default:							\
  * prototypes for isp_pci && isp_freebsd to share
  */
 extern int isp_attach(ispsoftc_t *);
-extern void isp_detach(ispsoftc_t *);
+extern int isp_detach(ispsoftc_t *);
 extern void isp_uninit(ispsoftc_t *);
 extern uint64_t isp_default_wwn(ispsoftc_t *, int, int, int);
 
@@ -600,12 +600,23 @@ extern int isp_autoconfig;
  * Platform private flags
  */
 #define	ISP_SPRIV_ERRSET	0x1
+#define	ISP_SPRIV_TACTIVE	0x2
 #define	ISP_SPRIV_DONE		0x8
+#define	ISP_SPRIV_WPEND		0x10
+
+#define	XS_S_TACTIVE(sccb)	(sccb)->ccb_h.spriv_field0 |= ISP_SPRIV_TACTIVE
+#define	XS_C_TACTIVE(sccb)	(sccb)->ccb_h.spriv_field0 &= ~ISP_SPRIV_TACTIVE
+#define	XS_TACTIVE_P(sccb)	((sccb)->ccb_h.spriv_field0 & ISP_SPRIV_TACTIVE)
 
 #define	XS_CMD_S_DONE(sccb)	(sccb)->ccb_h.spriv_field0 |= ISP_SPRIV_DONE
 #define	XS_CMD_C_DONE(sccb)	(sccb)->ccb_h.spriv_field0 &= ~ISP_SPRIV_DONE
 #define	XS_CMD_DONE_P(sccb)	((sccb)->ccb_h.spriv_field0 & ISP_SPRIV_DONE)
 
+#define	XS_CMD_S_WPEND(sccb)	(sccb)->ccb_h.spriv_field0 |= ISP_SPRIV_WPEND
+#define	XS_CMD_C_WPEND(sccb)	(sccb)->ccb_h.spriv_field0 &= ~ISP_SPRIV_WPEND
+#define	XS_CMD_WPEND_P(sccb)	((sccb)->ccb_h.spriv_field0 & ISP_SPRIV_WPEND)
+
+
 #define	XS_CMD_S_CLEAR(sccb)	(sccb)->ccb_h.spriv_field0 = 0
 
 /*
diff --git a/sys/dev/isp/isp_pci.c b/sys/dev/isp/isp_pci.c
index 5eb073957bc1..b6b23fafa59d 100644
--- a/sys/dev/isp/isp_pci.c
+++ b/sys/dev/isp/isp_pci.c
@@ -347,7 +347,11 @@ static int isp_pci_detach (device_t);
 struct isp_pcisoftc {
 	ispsoftc_t			pci_isp;
 	device_t			pci_dev;
-	struct resource *		pci_reg;
+	struct resource *		regs;
+	void *				irq;
+	int				iqd;
+	int				rtp;
+	int				rgd;
 	void *				ih;
 	int16_t				pci_poff[_NREG_BLKS];
 	bus_dma_tag_t			dmat;
@@ -645,8 +649,7 @@ isp_get_specific_options(device_t dev, int chan, ispsoftc_t *isp)
 static int
 isp_pci_attach(device_t dev)
 {
-	struct resource *regs, *irq;
-	int rtp, rgd, iqd, i, m1, m2, locksetup = 0;
+	int i, m1, m2, locksetup = 0;
 	int isp_nvports = 0;
 	uint32_t data, cmd, linesz, did;
 	struct isp_pcisoftc *pcs;
@@ -687,32 +690,31 @@ isp_pci_attach(device_t dev)
 	isp_get_pci_options(dev, &m1, &m2);
 
 	linesz = PCI_DFLT_LNSZ;
-	irq = regs = NULL;
-	rgd = rtp = iqd = 0;
+	pcs->irq = pcs->regs = NULL;
+	pcs->rgd = pcs->rtp = pcs->iqd = 0;
 
 	cmd = pci_read_config(dev, PCIR_COMMAND, 2);
 	if (cmd & m1) {
-		rtp = (m1 == PCIM_CMD_MEMEN)? SYS_RES_MEMORY : SYS_RES_IOPORT;
-		rgd = (m1 == PCIM_CMD_MEMEN)? MEM_MAP_REG : IO_MAP_REG;
-		regs = bus_alloc_resource_any(dev, rtp, &rgd, RF_ACTIVE);
+		pcs->rtp = (m1 == PCIM_CMD_MEMEN)? SYS_RES_MEMORY : SYS_RES_IOPORT;
+		pcs->rgd = (m1 == PCIM_CMD_MEMEN)? MEM_MAP_REG : IO_MAP_REG;
+		pcs->regs = bus_alloc_resource_any(dev, pcs->rtp, &pcs->rgd, RF_ACTIVE);
 	}
-	if (regs == NULL && (cmd & m2)) {
-		rtp = (m2 == PCIM_CMD_MEMEN)? SYS_RES_MEMORY : SYS_RES_IOPORT;
-		rgd = (m2 == PCIM_CMD_MEMEN)? MEM_MAP_REG : IO_MAP_REG;
-		regs = bus_alloc_resource_any(dev, rtp, &rgd, RF_ACTIVE);
+	if (pcs->regs == NULL && (cmd & m2)) {
+		pcs->rtp = (m2 == PCIM_CMD_MEMEN)? SYS_RES_MEMORY : SYS_RES_IOPORT;
+		pcs->rgd = (m2 == PCIM_CMD_MEMEN)? MEM_MAP_REG : IO_MAP_REG;
+		pcs->regs = bus_alloc_resource_any(dev, pcs->rtp, &pcs->rgd, RF_ACTIVE);
 	}
-	if (regs == NULL) {
+	if (pcs->regs == NULL) {
 		device_printf(dev, "unable to map any ports\n");
 		goto bad;
 	}
 	if (bootverbose) {
-		device_printf(dev, "using %s space register mapping\n", (rgd == IO_MAP_REG)? "I/O" : "Memory");
+		device_printf(dev, "using %s space register mapping\n", (pcs->rgd == IO_MAP_REG)? "I/O" : "Memory");
 	}
-	isp->isp_bus_tag = rman_get_bustag(regs);
-	isp->isp_bus_handle = rman_get_bushandle(regs);
+	isp->isp_bus_tag = rman_get_bustag(pcs->regs);
+	isp->isp_bus_handle = rman_get_bushandle(pcs->regs);
 
 	pcs->pci_dev = dev;
-	pcs->pci_reg = regs;
 	pcs->pci_poff[BIU_BLOCK >> _BLK_REG_SHFT] = BIU_REGS_OFF;
 	pcs->pci_poff[MBOX_BLOCK >> _BLK_REG_SHFT] = PCI_MBOX_REGS_OFF;
 	pcs->pci_poff[SXP_BLOCK >> _BLK_REG_SHFT] = PCI_SXP_REGS_OFF;
@@ -931,13 +933,13 @@ isp_pci_attach(device_t dev)
 			pcs->msicount = 1;
 		}
 		if (pci_alloc_msi(dev, &pcs->msicount) == 0) {
-			iqd = 1;
+			pcs->iqd = 1;
 		} else {
-			iqd = 0;
+			pcs->iqd = 0;
 		}
 	}
-	irq = bus_alloc_resource_any(dev, SYS_RES_IRQ, &iqd, RF_ACTIVE | RF_SHAREABLE);
-	if (irq == NULL) {
+	pcs->irq = bus_alloc_resource_any(dev, SYS_RES_IRQ, &pcs->iqd, RF_ACTIVE | RF_SHAREABLE);
+	if (pcs->irq == NULL) {
 		device_printf(dev, "could not allocate interrupt\n");
 		goto bad;
 	}
@@ -946,7 +948,7 @@ isp_pci_attach(device_t dev)
 	mtx_init(&isp->isp_osinfo.lock, "isp", NULL, MTX_DEF);
 	locksetup++;
 
-	if (isp_setup_intr(dev, irq, ISP_IFLAGS, NULL, isp_platform_intr, isp, &pcs->ih)) {
+	if (isp_setup_intr(dev, pcs->irq, ISP_IFLAGS, NULL, isp_platform_intr, isp, &pcs->ih)) {
 		device_printf(dev, "could not setup interrupt\n");
 		goto bad;
 	}
@@ -982,19 +984,19 @@ isp_pci_attach(device_t dev)
 
 bad:
 	if (pcs->ih) {
-		(void) bus_teardown_intr(dev, irq, pcs->ih);
+		(void) bus_teardown_intr(dev, pcs->irq, pcs->ih);
 	}
 	if (locksetup) {
 		mtx_destroy(&isp->isp_osinfo.lock);
 	}
-	if (irq) {
-		(void) bus_release_resource(dev, SYS_RES_IRQ, iqd, irq);
+	if (pcs->irq) {
+		(void) bus_release_resource(dev, SYS_RES_IRQ, pcs->iqd, pcs->irq);
 	}
 	if (pcs->msicount) {
 		pci_release_msi(dev);
 	}
-	if (regs) {
-		(void) bus_release_resource(dev, rtp, rgd, regs);
+	if (pcs->regs) {
+		(void) bus_release_resource(dev, pcs->rtp, pcs->rgd, pcs->regs);
 	}
 	if (pcs->pci_isp.isp_param) {
 		free(pcs->pci_isp.isp_param, M_DEVBUF);
@@ -1012,14 +1014,36 @@ isp_pci_detach(device_t dev)
 {
 	struct isp_pcisoftc *pcs;
 	ispsoftc_t *isp;
+	int status;
 
 	pcs = device_get_softc(dev);
 	if (pcs == NULL) {
 		return (ENXIO);
 	}
 	isp = (ispsoftc_t *) pcs;
-	ISP_DISABLE_INTS(isp);
+	status = isp_detach(isp);
+	if (status)
+		return (status);
+	ISP_LOCK(isp);
+	isp_uninit(isp);
+	if (pcs->ih) {
+		(void) bus_teardown_intr(dev, pcs->irq, pcs->ih);
+	}
+	ISP_UNLOCK(isp);
 	mtx_destroy(&isp->isp_osinfo.lock);
+	(void) bus_release_resource(dev, SYS_RES_IRQ, pcs->iqd, pcs->irq);
+	if (pcs->msicount) {
+		pci_release_msi(dev);
+	}
+	(void) bus_release_resource(dev, pcs->rtp, pcs->rgd, pcs->regs);
+	if (pcs->pci_isp.isp_param) {
+		free(pcs->pci_isp.isp_param, M_DEVBUF);
+		pcs->pci_isp.isp_param = NULL;
+	}
+	if (pcs->pci_isp.isp_osinfo.pc.ptr) {
+		free(pcs->pci_isp.isp_osinfo.pc.ptr, M_DEVBUF);
+		pcs->pci_isp.isp_osinfo.pc.ptr = NULL;
+	}
 	return (0);
 }
 
diff --git a/sys/dev/isp/ispvar.h b/sys/dev/isp/ispvar.h
index d7937949c488..55edc17228cf 100644
--- a/sys/dev/isp/ispvar.h
+++ b/sys/dev/isp/ispvar.h
@@ -953,6 +953,11 @@ void isp_async(ispsoftc_t *, ispasync_t, ...);
 #define	ISPASYNC_CHANGE_OTHER	2
 
 /*
+ * Platform Independent Error Prinout
+ */
+void isp_prt_endcmd(ispsoftc_t *, XS_T *);
+
+/*
  * Platform Dependent Error and Debug Printout
  *
  * Two required functions for each platform must be provided: