Updates to -current

Sean Bruno <[email protected]>
Newsgroups gmane.os.freebsd.devel.firewire
Message-ID <[email protected]>
Here is an update to -current that I am working on.

This update fixes the following:

SBP was not properly detaching from CAM if a target is powered off or
disconnected from the firewire bus.  This would manifest itself by
causing things like "camcontrol rescan all" to hang indefinitely.

SBP would have panic'd in sbp_orb_pointer() if fw_asyreq() failed.

Add locking during BUS_RESET conditions to keep the bus_reset handler
from firing before the code was finished asserting the BUS_RESET
condition.

Remove FWOHCI_INTFILT as the bus reset handler really does WAY more than
it should.  

Rework the generation indicator/flag to more properly adhere to the
specifications.  In this context, the generation flag should be 0 or 1
to indicate that the CROM has changed.  It should not be incremented
indefinitely without bounds.

_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-firewire
To unsubscribe, send any mail to "[email protected]"
firewire.diff (text/x-patch, 9.3 KB)
Index: fwohci_pci.c
===================================================================
--- fwohci_pci.c	(revision 187939)
+++ fwohci_pci.c	(working copy)
@@ -334,14 +334,11 @@
 		return ENXIO;
 	}
 
-
 	err = bus_setup_intr(self, sc->irq_res,
-			INTR_TYPE_NET | INTR_MPSAFE,
-#if FWOHCI_INTFILT
-		     fwohci_filt, NULL, sc, &sc->ih);
-#else
-		     NULL, (driver_intr_t *) fwohci_intr, sc, &sc->ih);
-#endif
+				INTR_TYPE_NET | INTR_MPSAFE,
+				NULL, (driver_intr_t *) fwohci_intr,
+				sc, &sc->ih);
+
 #if defined(__DragonFly__) || __FreeBSD_version < 500000
 	/* XXX splcam() should mask this irq for sbp.c*/
 	err = bus_setup_intr(self, sc->irq_res, INTR_TYPE_CAM,
Index: firewire.c
===================================================================
--- firewire.c	(revision 187939)
+++ firewire.c	(working copy)
@@ -430,6 +430,16 @@
 
 	fwdev_makedev(sc);
 
+	fc->crom_src_buf = (struct crom_src_buf *)malloc(
+				sizeof(struct crom_src_buf),
+				M_FW, M_NOWAIT | M_ZERO);
+	fc->topology_map = (struct fw_topology_map *)malloc(
+				sizeof(struct fw_topology_map),
+				M_FW, M_NOWAIT | M_ZERO);
+	fc->speed_map = (struct fw_speed_map *)malloc(
+				sizeof(struct fw_speed_map),
+				M_FW, M_NOWAIT | M_ZERO);
+
 	mtx_init(&fc->wait_lock, "fwwait", NULL, MTX_DEF);
 	mtx_init(&fc->tlabel_lock, "fwtlabel", NULL, MTX_DEF);
 	CALLOUT_INIT(&fc->timeout_callout);
@@ -451,7 +461,9 @@
 	bus_generic_attach(dev);
 
 	/* bus_reset */
+	FW_GLOCK(fc);
 	fw_busreset(fc, FWBUSNOTREADY);
+	FW_GUNLOCK(fc);
 	fc->ibr(fc);
 
 	return 0;
@@ -642,11 +654,6 @@
 {
 	struct crom_src *src;
 
-	fc->crom_src_buf = (struct crom_src_buf *)
-		malloc(sizeof(struct crom_src_buf), M_FW, M_WAITOK | M_ZERO);
-	if (fc->crom_src_buf == NULL)
-		return;
-
 	src = &fc->crom_src_buf->src;
 	bzero(src, sizeof(struct crom_src));
 
@@ -663,7 +670,7 @@
 	src->businfo.cyc_clk_acc = 100;
 	src->businfo.max_rec = fc->maxrec;
 	src->businfo.max_rom = MAXROM_4;
-	src->businfo.generation = 1;
+	src->businfo.generation = 0;
 	src->businfo.link_spd = fc->speed;
 
 	src->businfo.eui64.hi = fc->eui.hi;
@@ -682,9 +689,6 @@
 	struct crom_src *src;
 	struct crom_chunk *root;
 
-	if (fc->crom_src_buf == NULL)
-		fw_init_crom(fc);
-
 	buf =  fc->crom_src_buf;
 	src = fc->crom_src;
 	root = fc->crom_root;
@@ -715,18 +719,18 @@
 	struct firewire_dev_comm *fdc;
 	struct crom_src *src;
 	device_t *devlistp;
-	void *newrom;
 	int i, devcnt;
 
-	switch(fc->status){
-	case FWBUSMGRELECT:
+	FW_GLOCK_ASSERT(fc);
+	if (fc->status == FWBUSMGRELECT)
 		callout_stop(&fc->bmr_callout);
-		break;
-	default:
-		break;
-	}
+
 	fc->status = new_status;
 	fw_reset_csr(fc);
+
+	if (fc->status == FWBUSNOTREADY)
+		fw_init_crom(fc);
+
 	fw_reset_crom(fc);
 
 	if (device_get_children(fc->bdev, &devlistp, &devcnt) == 0) {
@@ -739,19 +743,19 @@
 		free(devlistp, M_TEMP);
 	}
 
-	newrom = malloc(CROMSIZE, M_FW, M_NOWAIT | M_ZERO);
 	src = &fc->crom_src_buf->src;
-	crom_load(src, (uint32_t *)newrom, CROMSIZE);
-	if (bcmp(newrom, fc->config_rom, CROMSIZE) != 0) {
-		/* bump generation and reload */
-		src->businfo.generation ++;
-		/* generation must be between 0x2 and 0xF */
-		if (src->businfo.generation < 2)
-			src->businfo.generation ++;
-		crom_load(src, (uint32_t *)newrom, CROMSIZE);
-		bcopy(newrom, (void *)fc->config_rom, CROMSIZE);
-	}
-	free(newrom, M_FW);
+	/*
+	 * If the old config rom needs to be overwritten,
+	 * bump the businfo.generation indicator to 
+	 * indicate that we need to be reprobed
+	 */
+	if (bcmp(src, fc->config_rom, CROMSIZE) != 0) {
+		/* generation is a 2 bit field */
+		/* valid values are only from 0 - 3 */
+		src->businfo.generation = 1;
+		bcopy(src, (void *)fc->config_rom, CROMSIZE);
+	} else
+		src->businfo.generation = 0;
 }
 
 /* Call once after reboot */
@@ -807,13 +811,7 @@
 		fc->ir[i]->maxq = FWMAXQUEUE;
 		fc->it[i]->maxq = FWMAXQUEUE;
 	}
-/* Initialize csr registers */
-	fc->topology_map = (struct fw_topology_map *)malloc(
-				sizeof(struct fw_topology_map),
-				M_FW, M_NOWAIT | M_ZERO);
-	fc->speed_map = (struct fw_speed_map *)malloc(
-				sizeof(struct fw_speed_map),
-				M_FW, M_NOWAIT | M_ZERO);
+
 	CSRARC(fc, TOPO_MAP) = 0x3f1 << 16;
 	CSRARC(fc, TOPO_MAP + 4) = 1;
 	CSRARC(fc, SPED_MAP) = 0x3f1 << 16;
@@ -1559,7 +1557,7 @@
 	/* unchanged ? */
 	if (bcmp(&csr[0], &fwdev->csrrom[0], sizeof(uint32_t) * 5) == 0) {
 		if (firewire_debug)
-			printf("node%d: crom unchanged\n", node);
+			device_printf(fc->dev, "node%d: crom unchanged\n", node);
 		return (0);
 	}
 
Index: fwohci.c
===================================================================
--- fwohci.c	(revision 187939)
+++ fwohci.c	(working copy)
@@ -1003,7 +1003,7 @@
 	if (maxdesc < db_tr->dbcnt) {
 		maxdesc = db_tr->dbcnt;
 		if (firewire_debug)
-			device_printf(sc->fc.dev, "maxdesc: %d\n", maxdesc);
+			device_printf(sc->fc.dev, "%s: maxdesc %d\n", __func__, maxdesc);
 	}
 	/* last db */
 	LAST_DB(db_tr, db);
@@ -1842,6 +1842,7 @@
 	struct firewire_comm *fc = (struct firewire_comm *)sc;
 	uint32_t node_id, plen;
 
+	FW_GLOCK_ASSERT(fc);
 	if ((stat & OHCI_INT_PHY_BUS_R) && (fc->status != FWBUSRESET)) {
 		fc->status = FWBUSRESET;
 		/* Disable bus reset interrupt until sid recv. */
@@ -1884,8 +1885,8 @@
 		plen = OREAD(sc, OHCI_SID_CNT);
 
 		fc->nodeid = node_id & 0x3f;
-		device_printf(fc->dev, "node_id=0x%08x, gen=%d, ",
-			node_id, (plen >> 16) & 0xff);
+		device_printf(fc->dev, "node_id=0x%08x, SelfID Count=%d, ",
+				fc->nodeid, (plen >> 16) & 0xff);
 		if (!(node_id & OHCI_NODE_VALID)) {
 			printf("Bus reset failure\n");
 			goto sidout;
@@ -1996,9 +1997,11 @@
 {
 	struct fwohci_softc *sc = (struct fwohci_softc *)arg;
 
+	FW_GLOCK(&sc->fc);
 	fw_busreset(&sc->fc, FWBUSRESET);
 	OWRITE(sc, OHCI_CROMHDR, ntohl(sc->fc.config_rom[0]));
 	OWRITE(sc, OHCI_BUS_OPT, ntohl(sc->fc.config_rom[2]));
+	FW_GUNLOCK(&sc->fc);
 }
 
 static void
@@ -2010,6 +2013,10 @@
 	int i, plen;
 
 
+	/*
+	 * We really should have locking
+	 * here.  Not sure why it's not
+	 */
 	plen = OREAD(sc, OHCI_SID_CNT);
 
 	if (plen & OHCI_SID_ERR) {
@@ -2029,14 +2036,13 @@
 	}
 	for (i = 0; i < plen / 4; i ++)
 		buf[i] = FWOHCI_DMA_READ(sc->sid_buf[i+1]);
-#if 1 /* XXX needed?? */
+
 	/* pending all pre-bus_reset packets */
 	fwohci_txd(sc, &sc->atrq);
 	fwohci_txd(sc, &sc->atrs);
 	fwohci_arcv(sc, &sc->arrs, -1);
 	fwohci_arcv(sc, &sc->arrq, -1);
 	fw_drain_txq(fc);
-#endif
 	fw_sidrcv(fc, buf, plen);
 	free(buf, M_FW);
 }
@@ -2061,6 +2067,7 @@
 {
 	uint32_t stat, irstat, itstat;
 
+	FW_GLOCK_ASSERT(&sc->fc);
 	stat = OREAD(sc, FWOHCI_INTSTAT);
 	if (stat == 0xffffffff) {
 		device_printf(sc->fc.dev, 
@@ -2090,29 +2097,24 @@
 	return (FILTER_HANDLED);
 }
 
-int
-fwohci_filt(void *arg)
+void
+fwohci_intr(void *arg)
 {
 	struct fwohci_softc *sc = (struct fwohci_softc *)arg;
 
-	if (!(sc->intmask & OHCI_INT_EN)) {
-		/* polling mode */
-		return (FILTER_STRAY);
-	}
-	return (fwohci_check_stat(sc));
+	FW_GLOCK(&sc->fc);
+	fwohci_check_stat(sc);
+	FW_GUNLOCK(&sc->fc);
 }
 
 void
-fwohci_intr(void *arg)
-{
-	fwohci_filt(arg);
-}
-
-void
 fwohci_poll(struct firewire_comm *fc, int quick, int count)
 {
 	struct fwohci_softc *sc = (struct fwohci_softc *)fc;
+
+	FW_GLOCK(fc);
 	fwohci_check_stat(sc);
+	FW_GUNLOCK(fc);
 }
 
 static void
@@ -2466,6 +2468,7 @@
 	device_printf(fc->dev, "Initiate bus reset\n");
 	sc = (struct fwohci_softc *)fc;
 
+	FW_GLOCK(fc);
 	/*
 	 * Make sure our cached values from the config rom are
 	 * initialised.
@@ -2486,6 +2489,7 @@
 	fun |= FW_PHY_ISBR | FW_PHY_RHB;
 	fun = fwphy_wrdata(sc, FW_PHY_ISBR_REG, fun);
 #endif
+	FW_GUNLOCK(fc);
 }
 
 void
Index: sbp.c
===================================================================
--- sbp.c	(revision 187939)
+++ sbp.c	(working copy)
@@ -493,6 +493,7 @@
 			/* lost device */
 			sbp_cam_detach_sdev(sdev);
 			sbp_free_sdev(sdev);
+			target->luns[lun] = NULL;
 		}
 	}
 
@@ -781,7 +782,9 @@
 					SBP_UNLOCK(sbp);
 				}
 				sdev->status = SBP_DEV_RETRY;
-				sbp_abort_all_ocbs(sdev, CAM_SCSI_BUS_RESET);
+				sbp_cam_detach_sdev(sdev);
+				sbp_free_sdev(sdev);
+				target->luns[i] = NULL;
 				break;
 			case SBP_DEV_PROBE:
 			case SBP_DEV_TOATTACH:
@@ -1255,11 +1258,18 @@
 		htonl(((sdev->target->sbp->fd.fc->nodeid | FWLOCALBUS )<< 16));
 	xfer->send.payload[1] = htonl((uint32_t)ocb->bus_addr);
 
+	/*
+	 * sbp_xfer_free() will attempt to acquire
+	 * the SBP lock on entrance.  Also, this removes
+	 * a LOR between the firewire layer and sbp
+	 */
+	SBP_UNLOCK(sdev->target->sbp);
 	if(fw_asyreq(xfer->fc, -1, xfer) != 0){
 			sbp_xfer_free(xfer);
 			ocb->ccb->ccb_h.status = CAM_REQ_INVALID;
 			xpt_done(ocb->ccb);
 	}
+	SBP_LOCK(sdev->target->sbp);
 }
 
 static void
@@ -2123,6 +2133,7 @@
 		    sdev->ocb[i].dmamap);
 	fwdma_free(sdev->target->sbp->fd.fc, &sdev->dma);
 	free(sdev, M_SBP);
+	sdev = NULL;
 }
 
 static void
Index: fwohcivar.h
===================================================================
--- fwohcivar.h	(revision 187939)
+++ fwohcivar.h	(working copy)
@@ -37,12 +37,6 @@
 
 #include <sys/taskqueue.h>
 
-#if defined(__DragonFly__) ||  __FreeBSD_version < 700043
-#define FWOHCI_INTFILT	0
-#else
-#define FWOHCI_INTFILT	1
-#endif
-
 typedef struct fwohci_softc {
 	struct firewire_comm fc;
 	bus_space_tag_t bst;
@@ -84,7 +78,7 @@
 } fwohci_softc_t;
 
 void fwohci_intr (void *arg);
-int fwohci_filt (void *arg);
+void fwohci_filt (void *arg);
 int fwohci_init (struct fwohci_softc *, device_t);
 void fwohci_poll (struct firewire_comm *, int, int);
 void fwohci_reset (struct fwohci_softc *, device_t);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.