Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
To: None <port-alpha-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Stephen Jones <smj@sdf.lonestar.org>
List: netbsd-bugs
Date: 07/19/2007 19:05:05
The following reply was made to PR port-alpha/30560; it has been noted by GNATS.

From: Stephen Jones <smj@sdf.lonestar.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
Date: Thu, 19 Jul 2007 19:04:30 +0000 (UTC)

 Hi, sure I will give it a shot!  I've got a CS20 with current going at
 the moment.
 
 > The following reply was made to PR port-alpha/30560; it has been noted by GNATS.
 > 
 > From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
 > To: gnats-bugs@NetBSD.org
 > Cc: smj@sdf.lonestar.org, tsutsui@ceres.dti.ne.jp
 > Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
 > Date: Thu, 19 Jul 2007 22:40:25 +0900
 > 
 >  > He also suggests that NetBSD should adopt OpenBSD's 'fix':
 >  > http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/fxpreg.h?sortby=date#rev1.7
 >  
 >  Could you try this patch (for -current)?
 >  It seems working on my AlphaPC164 with i82550.
 >  ---
 >  
 >  Index: i82557.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/dev/ic/i82557.c,v
 >  retrieving revision 1.102
 >  diff -u -r1.102 i82557.c
 >  --- i82557.c	9 Jul 2007 21:00:36 -0000	1.102
 >  +++ i82557.c	19 Jul 2007 13:34:31 -0000
 >  @@ -189,7 +189,7 @@
 >   void	fxp_stop(struct ifnet *, int);
 >   
 >   void	fxp_txintr(struct fxp_softc *);
 >  -void	fxp_rxintr(struct fxp_softc *);
 >  +int	fxp_rxintr(struct fxp_softc *);
 >   
 >   int	fxp_rx_hwcksum(struct mbuf *, const struct fxp_rfa *);
 >   
 >  @@ -916,7 +916,7 @@
 >   			break;
 >   		m = NULL;
 >   
 >  -		if (sc->sc_txpending == FXP_NTXCB) {
 >  +		if (sc->sc_txpending == FXP_NTXCB - 1) {
 >   			FXP_EVCNT_INCR(&sc->sc_ev_txstall);
 >   			break;
 >   		}
 >  @@ -1067,7 +1067,7 @@
 >   #endif
 >   	}
 >   
 >  -	if (sc->sc_txpending == FXP_NTXCB) {
 >  +	if (sc->sc_txpending == FXP_NTXCB - 1) {
 >   		/* No more slots; notify upper layer. */
 >   		ifp->if_flags |= IFF_OACTIVE;
 >   	}
 >  @@ -1084,9 +1084,23 @@
 >   		 * Cause the chip to interrupt and suspend command
 >   		 * processing once the last packet we've enqueued
 >   		 * has been transmitted.
 >  +		 *
 >  +		 * To avoid a race between updating status bits
 >  +		 * by the fxp chip and clearing command bits
 >  +		 * by this function on machines which don't have
 >  +		 * atomic methods to clear/set bits in memory
 >  +		 * smaller than 32bits (both cb_status and cb_command
 >  +		 * members are uint16_t and in the same 32bit word),
 >  +		 * we have to prepare a dummy TX descriptor which has
 >  +		 * NOP command and just causes a TX completion interrupt.
 >   		 */
 >  -		FXP_CDTX(sc, sc->sc_txlast)->txd_txcb.cb_command |=
 >  -		    htole16(FXP_CB_COMMAND_I | FXP_CB_COMMAND_S);
 >  +		sc->sc_txpending++;
 >  +		sc->sc_txlast = FXP_NEXTTX(sc->sc_txlast);
 >  +		txd = FXP_CDTX(sc, sc->sc_txlast);
 >  +		/* BIG_ENDIAN: no need to swap to store 0 */
 >  +		txd->txd_txcb.cb_status = 0;
 >  +		txd->txd_txcb.cb_command = htole16(FXP_CB_COMMAND_NOP |
 >  +		    FXP_CB_COMMAND_I | FXP_CB_COMMAND_S);
 >   		FXP_CDTXSYNC(sc, sc->sc_txlast,
 >   		    BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
 >   
 >  @@ -1121,7 +1135,7 @@
 >   	struct fxp_softc *sc = arg;
 >   	struct ifnet *ifp = &sc->sc_ethercom.ec_if;
 >   	bus_dmamap_t rxmap;
 >  -	int claimed = 0;
 >  +	int claimed = 0, rnr;
 >   	u_int8_t statack;
 >   
 >   	if (!device_is_active(&sc->sc_dev) || sc->sc_enabled == 0)
 >  @@ -1152,20 +1166,12 @@
 >   		 * condition exists, get whatever packets we can and
 >   		 * re-start the receiver.
 >   		 */
 >  -		if (statack & (FXP_SCB_STATACK_FR | FXP_SCB_STATACK_RNR)) {
 >  +		rnr = (statack & (FXP_SCB_STATACK_RNR | FXP_SCB_STATACK_SWI)) ?
 >  +		    1 : 0;
 >  +		if (statack & (FXP_SCB_STATACK_FR | FXP_SCB_STATACK_RNR |
 >  +		    FXP_SCB_STATACK_SWI)) {
 >   			FXP_EVCNT_INCR(&sc->sc_ev_rxintr);
 >  -			fxp_rxintr(sc);
 >  -		}
 >  -
 >  -		if (statack & FXP_SCB_STATACK_RNR) {
 >  -			fxp_scb_wait(sc);
 >  -			fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_ABORT);
 >  -			rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
 >  -			fxp_scb_wait(sc);
 >  -			CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
 >  -			    rxmap->dm_segs[0].ds_addr +
 >  -			    RFA_ALIGNMENT_FUDGE);
 >  -			fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
 >  +			rnr |= fxp_rxintr(sc);
 >   		}
 >   
 >   		/*
 >  @@ -1188,6 +1194,17 @@
 >   					(void) fxp_init(ifp);
 >   			}
 >   		}
 >  +
 >  +		if (rnr) {
 >  +			fxp_scb_wait(sc);
 >  +			fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_ABORT);
 >  +			rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
 >  +			fxp_scb_wait(sc);
 >  +			CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
 >  +			    rxmap->dm_segs[0].ds_addr +
 >  +			    RFA_ALIGNMENT_FUDGE);
 >  +			fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
 >  +		}
 >   	}
 >   
 >   #if NRND > 0
 >  @@ -1218,6 +1235,11 @@
 >   		FXP_CDTXSYNC(sc, i,
 >   		    BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
 >   
 >  +		/* skip dummy NOP TX descriptor */
 >  +		if ((le16toh(txd->txd_txcb.cb_command) & FXP_CB_COMMAND_CMD)
 >  +		    == FXP_CB_COMMAND_NOP)
 >  +			continue;
 >  +
 >   		txstat = le16toh(txd->txd_txcb.cb_status);
 >   
 >   		if ((txstat & FXP_CB_STATUS_C) == 0)
 >  @@ -1301,7 +1323,7 @@
 >   /*
 >    * Handle receive interrupts.
 >    */
 >  -void
 >  +int
 >   fxp_rxintr(struct fxp_softc *sc)
 >   {
 >   	struct ethercom *ec = &sc->sc_ethercom;
 >  @@ -1309,8 +1331,11 @@
 >   	struct mbuf *m, *m0;
 >   	bus_dmamap_t rxmap;
 >   	struct fxp_rfa *rfa;
 >  +	int rnr;
 >   	u_int16_t len, rxstat;
 >   
 >  +	rnr = 0;
 >  +
 >   	for (;;) {
 >   		m = sc->sc_rxq.ifq_head;
 >   		rfa = FXP_MTORFA(m);
 >  @@ -1321,13 +1346,16 @@
 >   
 >   		rxstat = le16toh(rfa->rfa_status);
 >   
 >  +		if ((rxstat & FXP_RFA_STATUS_RNR) != 0)
 >  +			rnr = 1;
 >  +
 >   		if ((rxstat & FXP_RFA_STATUS_C) == 0) {
 >   			/*
 >   			 * We have processed all of the
 >   			 * receive buffers.
 >   			 */
 >   			FXP_RFASYNC(sc, m, BUS_DMASYNC_PREREAD);
 >  -			return;
 >  +			return rnr;
 >   		}
 >   
 >   		IF_DEQUEUE(&sc->sc_rxq, m);
 >  @@ -1923,11 +1951,13 @@
 >   	/*
 >   	 * Initialize receiver buffer area - RFA.
 >   	 */
 >  +#if 0
 >   	rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
 >   	fxp_scb_wait(sc);
 >   	CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
 >   	    rxmap->dm_segs[0].ds_addr + RFA_ALIGNMENT_FUDGE);
 >   	fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
 >  +#endif
 >   
 >   	if (sc->sc_flags & FXPF_MII) {
 >   		/*
 >  @@ -1943,6 +1973,16 @@
 >   	ifp->if_flags &= ~IFF_OACTIVE;
 >   
 >   	/*
 >  +	 * Request a software generated interrupt that will be used to 
 >  +	 * (re)start the RU processing.  If we direct the chip to start
 >  +	 * receiving from the start of queue now, instead of letting the
 >  +	 * interrupt handler first process all received packets, we run
 >  +	 * the risk of having it overwrite mbuf clusters while they are
 >  +	 * being processed or after they have been returned to the pool.
 >  +	 */
 >  +	CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, FXP_SCB_INTRCNTL_REQUEST_SWI);
 >  + 
 >  +	/*
 >   	 * Start the one second timer.
 >   	 */
 >   	callout_reset(&sc->sc_callout, hz, fxp_tick, sc);
 >  Index: i82557reg.h
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/dev/ic/i82557reg.h,v
 >  retrieving revision 1.18
 >  diff -u -r1.18 i82557reg.h
 >  --- i82557reg.h	11 Dec 2005 12:21:26 -0000	1.18
 >  +++ i82557reg.h	19 Jul 2007 13:34:31 -0000
 >  @@ -143,6 +143,7 @@
 >   #define FXP_SCB_COMMAND_RU_BASE		6
 >   #define FXP_SCB_COMMAND_RU_RBDRESUME	7
 >   
 >  +#define FXP_SCB_INTRCNTL_REQUEST_SWI	0x02
 >   /*
 >    * Command block definitions
 >    */
 >  @@ -368,6 +369,7 @@
 >   #define FXP_CB_STATUS_C		0x8000
 >   
 >   /* commands */
 >  +#define FXP_CB_COMMAND_CMD	0x0007	/* XXX how about FXPF_IPCB case? */
 >   #define FXP_CB_COMMAND_NOP	0x0
 >   #define FXP_CB_COMMAND_IAS	0x1
 >   #define FXP_CB_COMMAND_CONFIG	0x2
 >  @@ -409,7 +411,7 @@
 >   	volatile u_int8_t cksum_stat;
 >   	volatile u_int8_t zerocopy_stat;
 >   	volatile u_int8_t unused[8];
 >  -};
 >  +}__attribute__((__packed__));
 >   
 >   #define	RFA_SIZE		16
 >   #define	RFA_EXT_SIZE		32
 >  Index: i82557var.h
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/dev/ic/i82557var.h,v
 >  retrieving revision 1.35
 >  diff -u -r1.35 i82557var.h
 >  --- i82557var.h	11 Dec 2005 12:21:26 -0000	1.35
 >  +++ i82557var.h	19 Jul 2007 13:34:31 -0000
 >  @@ -318,7 +318,8 @@
 >   	__rfa->size = htole16(FXP_RXBUFSIZE((sc), (m)));		\
 >   	/* BIG_ENDIAN: no need to swap to store 0 */			\
 >   	__rfa->rfa_status = 0;						\
 >  -	__rfa->rfa_control = htole16(FXP_RFA_CONTROL_EL);		\
 >  +	__rfa->rfa_control =						\
 >  +	    htole16(FXP_RFA_CONTROL_EL | FXP_RFA_CONTROL_S);		\
 >   	/* BIG_ENDIAN: no need to swap to store 0 */			\
 >   	__rfa->actual_size = 0;						\
 >   									\
 >  @@ -341,7 +342,8 @@
 >   		    BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);	\
 >   		memcpy((void *)&__p_rfa->link_addr, &__v,		\
 >   		    sizeof(__v));					\
 >  -		__p_rfa->rfa_control &= htole16(~FXP_RFA_CONTROL_EL);	\
 >  +		__p_rfa->rfa_control &= htole16(~(FXP_RFA_CONTROL_EL|	\
 >  +		    FXP_RFA_CONTROL_S));				\
 >   		FXP_RFASYNC((sc), __p_m,				\
 >   		    BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);		\
 >   	}								\
 >  
 >  ---
 >  Izumi Tsutsui
 >  
 >