Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 20 Apr 2009 13:34:17 -0700
From:      Sean Bruno <sean.bruno@dsl-only.net>
To:        Andreas Tobler <andreast-list@fgznet.ch>
Cc:        Scott Long <scottl@samsco.org>, freebsd-firewire <freebsd-firewire@FreeBSD.org>, Marius Strobl <marius@alchemy.franken.de>
Subject:   Re: fwochi.c and bus_space_barrier()
Message-ID:  <1240259657.29756.61.camel@localhost.localdomain>
In-Reply-To: <49ECCF52.6030304@fgznet.ch>
References:  <1239382529.21481.7.camel@localhost.localdomain> <20090411154000.GG8143@alchemy.franken.de> <1239600457.24831.8.camel@localhost.localdomain> <49E2F2FA.6000204@fgznet.ch> <1239639423.24831.85.camel@localhost.localdomain> <20090413170537.GI8143@alchemy.franken.de> <1239643406.24831.95.camel@localhost.localdomain> <20090413173528.GJ8143@alchemy.franken.de> <1239646889.24831.135.camel@localhost.localdomain> <20090414184741.GK8143@alchemy.franken.de> <49E4DF9F.1090804@fgznet.ch> <1239814413.15474.2.camel@localhost.localdomain> <49E61B4D.1050209@fgznet.ch> <1239819547.15474.5.camel@localhost.localdomain> <49E633C7.9030909@fgznet.ch> <1239826803.15474.48.camel@localhost.localdomain> <49E7931C.8050603@fgznet.ch> <1240248579.29756.4.camel@localhost.localdomain> <49ECC0B6.5000804@fgznet.ch> <1240255386.29756.6.camel@localhost.localdomain> <49ECCF52.6030304@fgznet.ch>

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

--=-ClXtXx87u+H1AQ1HnQVd
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

On Mon, 2009-04-20 at 21:38 +0200, Andreas Tobler wrote:
> Sean Bruno wrote:
> > On Mon, 2009-04-20 at 20:36 +0200, Andreas Tobler wrote:
> >> resetting OHCI...done (loop=0)
> > 
> > 
> > Can you recomplile with firewire_debug = 1 and resend the output?
> > 
> > I'm interested in:
> > device_printf(sc->fc.dev, "%s: OHCI_INT_REG_FAIL.\n", __func__);
> > 
> > If that doesn't get printed, then I need to debug a bit further.
> 
> I always use firewire_debug=1, in the last try even > 1. All the traces 
> I sent are with firewire_debug=1.
> 
> I didn't see the above, I suspect the early OWRITE/READ in rddata are 
> too early for the silicon. Unfortunately adding printf's there, cures 
> the issue.
> 
> Andreas

I *think* this section of fwphy_rddata() is suspect:
        /*
         * Setup command to PHY
         */
        fun = PHYDEV_RDCMD | (addr << PHYDEV_REGADDR);
        OWRITE(sc, OHCI_PHYACCESS, fun);
        bus_space_barrier(sc->bst, sc->bsh, OHCI_PHYACCESS,
                                4, BUS_SPACE_BARRIER_WRITE);


According to the specification, this access is illegal if SCLK has not
started.  So, there's no way out of this error without a pause() after
LPS is set in fwohci_probe_phy().  

Although this adventure did teach me a great deal regarding firewire.
Thank you for the challenging problem.

Find the final version of my update attached.  Let me know what you find
with it.

Sean

--=-ClXtXx87u+H1AQ1HnQVd
Content-Disposition: attachment; filename="fwohci.c.diff"
Content-Type: text/x-patch; name="fwohci.c.diff"; charset="UTF-8"
Content-Transfer-Encoding: 7bit

Index: fwohci.c
===================================================================
--- fwohci.c	(revision 191322)
+++ fwohci.c	(working copy)
@@ -280,7 +280,8 @@
 
 	fun = (PHYDEV_WRCMD | (addr << PHYDEV_REGADDR) | (data << PHYDEV_WRDATA));
 	OWRITE(sc, OHCI_PHYACCESS, fun);
-	DELAY(100);
+	bus_space_barrier(sc->bst, sc->bsh, OHCI_PHYACCESS,
+				4, BUS_SPACE_BARRIER_WRITE);
 
 	return(fwphy_rddata( sc, addr));
 }
@@ -316,43 +317,62 @@
 fwphy_rddata(struct fwohci_softc *sc,  u_int addr)
 {
 	uint32_t fun, stat;
-	u_int i, retry = 0;
+	int retry = 0;
 
 	addr &= 0xf;
-#define MAX_RETRY 100
-again:
+
+
+	/*
+ 	 * Read requested data from OHCI PHY
+ 	 * If we generate an INT_REG_FAIL error
+ 	 * from our request, most likely SCLK has
+ 	 * not been started yet.  pause() and retry.
+ 	 * Mechanics of RDCMD and RDDONE are in 
+ 	 * ohci 1.1 Table 5-19
+ 	 */
+
+	/* Clear error register */
+	stat = OREAD(sc, FWOHCI_INTSTAT);
+	stat &= ~OHCI_INT_REG_FAIL;
 	OWRITE(sc, FWOHCI_INTSTATCLR, OHCI_INT_REG_FAIL);
+
+retry_command:
+	/* Issue requested command to PHY */
 	fun = PHYDEV_RDCMD | (addr << PHYDEV_REGADDR);
 	OWRITE(sc, OHCI_PHYACCESS, fun);
-	for ( i = 0 ; i < MAX_RETRY ; i ++ ){
-		fun = OREAD(sc, OHCI_PHYACCESS);
-		if ((fun & PHYDEV_RDCMD) == 0 && (fun & PHYDEV_RDDONE) != 0)
-			break;
-		DELAY(100);
-	}
-	if(i >= MAX_RETRY) {
-		if (firewire_debug)
-			device_printf(sc->fc.dev, "%s: failed(1).\n", __func__);
-		if (++retry < MAX_RETRY) {
-			DELAY(100);
-			goto again;
-		}
-	}
-	/* Make sure that SCLK is started */
-	stat = OREAD(sc, FWOHCI_INTSTAT);
-	if ((stat & OHCI_INT_REG_FAIL) != 0 ||
+	bus_space_barrier(sc->bst, sc->bsh, OHCI_PHYACCESS,
+				4, BUS_SPACE_BARRIER_WRITE);
+#define MAX_RETRY 5
+	for ( retry = 0 ; retry < MAX_RETRY ; retry++ ){
+		/* Check for error, sleep if error reg set */
+		stat = OREAD(sc, FWOHCI_INTSTAT);
+		if ((stat & OHCI_INT_REG_FAIL) != 0 ||
 			((fun >> PHYDEV_REGADDR) & 0xf) != addr) {
-		if (firewire_debug)
-			device_printf(sc->fc.dev, "%s: failed(2).\n", __func__);
-		if (++retry < MAX_RETRY) {
-			DELAY(100);
-			goto again;
+			if (firewire_debug)
+				device_printf(sc->fc.dev, "%s: "
+					"OHCI_INT_REG_FAIL.\n", __func__);
+			/* Clear error register */
+			stat = OREAD(sc, FWOHCI_INTSTAT);
+			stat &= ~OHCI_INT_REG_FAIL;
+			OWRITE(sc, FWOHCI_INTSTATCLR, OHCI_INT_REG_FAIL);
+			pause("fwphyr", (50 * hz + 999) / 1000);
+			goto retry_command;
+		} else { /* no error, check for command completion */
+			fun = OREAD(sc, OHCI_PHYACCESS);
+			bus_space_barrier(sc->bst, sc->bsh, OHCI_PHYACCESS,
+						4, BUS_SPACE_BARRIER_READ);
+			if ((fun & PHYDEV_RDCMD) == 0 && (fun & PHYDEV_RDDONE) != 0) {
+				retry = MAX_RETRY; /* exit loop. command complete */
+			} else { /* pause for 100 usec to allow command completion */
+				pause("fwphyr", hz/10);
+			}
 		}
+			
 	}
 	if (firewire_debug > 1 || retry >= MAX_RETRY)
 		device_printf(sc->fc.dev, 
-		    "%s:: 0x%x loop=%d, retry=%d\n",
-			__func__, addr, i, retry);
+		    "%s:: 0x%x, retry=%d\n",
+			__func__, addr, retry);
 #undef MAX_RETRY
 	return((fun >> PHYDEV_RDDATA )& 0xff);
 }
@@ -426,20 +446,45 @@
 static int
 fwohci_probe_phy(struct fwohci_softc *sc, device_t dev)
 {
-	uint32_t reg, reg2;
+	uint32_t lps, reg, reg2;
+	int lps_counter = 0;
 	int e1394a = 1;
-/*
- * probe PHY parameters
- * 0. to prove PHY version, whether compliance of 1394a.
- * 1. to probe maximum speed supported by the PHY and 
- *    number of port supported by core-logic.
- *    It is not actually available port on your PC .
- */
+
+	/*
+	 * Enable LPS(Link Power Status as per 
+	 * section 5.7 of OHCI v1.1
+	 * This allows PHY communication after
+	 * a hard/soft reset
+	 *
+	 * Some users report that the code
+	 * will crash without the pause due
+	 * to the lps bit being set and the 
+	 * PHY not being up.  Implement pause
+	 * here to work around this error.
+	 */
 	OWRITE(sc, OHCI_HCCCTL, OHCI_HCC_LPS);
-	DELAY(500);
-
+	bus_space_barrier(sc->bst, sc->bsh, OHCI_HCCCTL, 4, BUS_SPACE_BARRIER_WRITE);
+	for (lps = 0, lps_counter = 0; !lps && lps_counter < 3; lps_counter++) {
+		pause("fwlps", (50 * hz + 999) / 1000);
+		lps = (OREAD(sc, OHCI_HCCCTL) & OHCI_HCC_LPS);
+	}
+	/*
+ 	* probe PHY parameters
+ 	* 0. to prove PHY version, whether compliance of 1394a.
+ 	* 1. to probe maximum speed supported by the PHY and 
+ 	*    number of port supported by core-logic.
+ 	*    It is not actually available port on your PC .
+ 	*/
 	reg = fwphy_rddata(sc, FW_PHY_SPD_REG);
 
+	/*
+	 * ref 1394-2000 table 5B-1
+	 * ref 1394-1995 table J.12
+	 * If Extended is not set
+	 * Assume 1394-1995
+	 * If Extended is set
+	 * Assume 1394-2000(1394a)
+	 */
 	if((reg >> 5) != 7 ){
 		sc->fc.mode &= ~FWPHYASYST;
 		sc->fc.nport = reg & FW_PHY_NP;
@@ -453,12 +498,12 @@
 			"Phy 1394 only %s, %d ports.\n",
 			linkspeed[sc->fc.speed], sc->fc.nport);
 	}else{
-		reg2 = fwphy_rddata(sc, FW_PHY_ESPD_REG);
 		sc->fc.mode |= FWPHYASYST;
 		sc->fc.nport = reg & FW_PHY_NP;
+		reg2 = fwphy_rddata(sc, FW_PHY_ESPD_REG);
 		sc->fc.speed = (reg2 & FW_PHY_ESPD) >> 5;
 		if (sc->fc.speed > MAX_SPEED) {
-			device_printf(dev, "invalid speed %d (fixed to %d).\n",
+			device_printf(dev, "invalid extended speed %d (fixed to %d).\n",
 				sc->fc.speed, MAX_SPEED);
 			sc->fc.speed = MAX_SPEED;
 		}
@@ -468,11 +513,7 @@
 
 		/* check programPhyEnable */
 		reg2 = fwphy_rddata(sc, 5);
-#if 0
-		if (e1394a && (OREAD(sc, OHCI_HCCCTL) & OHCI_HCC_PRPHY)) {
-#else	/* XXX force to enable 1394a */
 		if (e1394a) {
-#endif
 			if (firewire_debug)
 				device_printf(dev,
 					"Enable 1394a Enhancements\n");
@@ -488,6 +529,13 @@
 		reg2 = fwphy_wrdata(sc, 5, reg2);
 	}
 
+	/*
+	 * Re-read and check for extended 1394a
+	 * PHY Register map.
+	 * Then set the Contender bit on.
+	 * This makes us a possible bus or isoc
+	 * resource manager. (ieee 1394a-2000, 5B.1)
+	 */
 	reg = fwphy_rddata(sc, FW_PHY_SPD_REG);
 	if((reg >> 5) == 7 ){
 		reg = fwphy_rddata(sc, 4);

--=-ClXtXx87u+H1AQ1HnQVd--




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