Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 30 Jan 2009 23:44:02 -0800
From:      Sean Bruno <sean.bruno@dsl-only.net>
To:        freebsd-firewire <freebsd-firewire@FreeBSD.org>
Cc:        scottl@freebsd.org
Subject:   Updates to -current
Message-ID:  <1233387842.5501.12.camel@localhost.localdomain>

next in thread | raw e-mail | index | archive | help

--=-QNJOa6ajW4EoGtwe+FA6
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

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.

--=-QNJOa6ajW4EoGtwe+FA6
Content-Disposition: attachment; filename="firewire.diff"
Content-Type: text/x-patch; name="firewire.diff"; charset="UTF-8"
Content-Transfer-Encoding: 7bit

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);

--=-QNJOa6ajW4EoGtwe+FA6--




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1233387842.5501.12.camel>