Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 11 Sep 2011 16:42:04 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-user@freebsd.org
Subject:   svn commit: r225479 - in user/adrian/if_ath_tx/sys/dev/ath: . ath_hal ath_hal/ar5212 ath_hal/ar5416
Message-ID:  <201109111642.p8BGg4L3086913@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Sun Sep 11 16:42:03 2011
New Revision: 225479
URL: http://svn.freebsd.org/changeset/base/225479

Log:
  Fix up the InterReq flag in the TX descriptor; and always set it when
  doing aggregate frame TX.
  
  It turns out that the AR5416 (and AR5212!) seems to want the InterReq
  flag to be consistently set in all descriptors in the given frame.
  This is what the datasheet says.
  
  Both linux ath9k and the reference driver (at least when under linux)
  seem to get away without this HAL change because the TX (sub) frame
  is only ever one skb in length. Thus the intermediary frames never
  needed this bit set.
  
  FreeBSD's implementation here involves TX'ing mbuf chains, so each
  descriptor in the subframe now needs this flag set consistently.
  
  Secondly, always set the InterReq flag (HAL_TXDESC_INTREQ) on all
  subframes in an aggregate - the interrupt mitigation hardware
  (for AR5416 and later, but not AR9130) will handle limiting TX
  interrupts for us. This is in line with what ath9k does.
  
  Although I haven't tested this change separate from the stuff I'm
  about to commit, this change was the final change needed to fix the
  odd TX hangs I was seeing, where a TXQ would complete but no TX
  interrupt occured. This wouldn't matter in the past as a subsequent
  TX (any TX!) would have been enough to kick the queue along.
  Without it, I'd get occasional device timeouts even though the TXQ
  consisted of a frame (aggregate or otherwise) whose status descriptor
  indicated it was completed.
  
  Whilst I'm here, do the same for the AR5212 - the AR5213 datasheet
  indicates much the same is needed.

Modified:
  user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ah.h
  user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5212/ar5212_xmit.c
  user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416.h
  user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416_xmit.c
  user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
  user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h

Modified: user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ah.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ah.h	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ah.h	Sun Sep 11 16:42:03 2011	(r225479)
@@ -989,9 +989,9 @@ struct ath_hal {
 
 	/* 802.11n Functions */
 	HAL_BOOL  __ahdecl(*ah_chainTxDesc)(struct ath_hal *,
-				struct ath_desc *, u_int, u_int, HAL_PKT_TYPE,
-				u_int, HAL_CIPHER, uint8_t, u_int, HAL_BOOL,
-				HAL_BOOL);
+				struct ath_desc *, u_int, u_int, u_int,
+				HAL_PKT_TYPE, u_int, HAL_CIPHER, uint8_t,
+				u_int, HAL_BOOL, HAL_BOOL);
 	HAL_BOOL  __ahdecl(*ah_setupFirstTxDesc)(struct ath_hal *,
 				struct ath_desc *, u_int, u_int, u_int,
 				u_int, u_int, u_int, u_int, u_int);

Modified: user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5212/ar5212_xmit.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5212/ar5212_xmit.c	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5212/ar5212_xmit.c	Sun Sep 11 16:42:03 2011	(r225479)
@@ -800,12 +800,14 @@ ar5212FillTxDesc(struct ath_hal *ah, str
 		 * copy the multi-rate transmit parameters from
 		 * the first frame for processing on completion. 
 		 */
-		ads->ds_ctl0 = 0;
 		ads->ds_ctl1 = segLen;
 #ifdef AH_NEED_DESC_SWAP
+		ads->ds_ctl0 = __bswap32(AR5212DESC_CONST(ds0)->ds_ctl0)
+		    & AR_TxInterReq;
 		ads->ds_ctl2 = __bswap32(AR5212DESC_CONST(ds0)->ds_ctl2);
 		ads->ds_ctl3 = __bswap32(AR5212DESC_CONST(ds0)->ds_ctl3);
 #else
+		ads->ds_ctl0 = AR5212DESC_CONST(ds0)->ds_ctl0 & AR_TxInterReq;
 		ads->ds_ctl2 = AR5212DESC_CONST(ds0)->ds_ctl2;
 		ads->ds_ctl3 = AR5212DESC_CONST(ds0)->ds_ctl3;
 #endif
@@ -813,7 +815,12 @@ ar5212FillTxDesc(struct ath_hal *ah, str
 		/*
 		 * Intermediate descriptor in a multi-descriptor frame.
 		 */
-		ads->ds_ctl0 = 0;
+#ifdef AH_NEED_DESC_SWAP
+		ads->ds_ctl0 = __bswap32(AR5212DESC_CONST(ds0)->ds_ctl0)
+		    & AR_TxInterReq;
+#else
+		ads->ds_ctl0 = AR5212DESC_CONST(ds0)->ds_ctl0 & AR_TxInterReq;
+#endif
 		ads->ds_ctl1 = segLen | AR_More;
 		ads->ds_ctl2 = 0;
 		ads->ds_ctl3 = 0;

Modified: user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416.h	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416.h	Sun Sep 11 16:42:03 2011	(r225479)
@@ -321,7 +321,7 @@ extern	int ar5416SetupTxQueue(struct ath
 	        const HAL_TXQ_INFO *qInfo);
 
 extern	HAL_BOOL ar5416ChainTxDesc(struct ath_hal *ah, struct ath_desc *ds,
-		u_int pktLen, u_int hdrLen, HAL_PKT_TYPE type, u_int keyIx,
+		u_int flags, u_int pktLen, u_int hdrLen, HAL_PKT_TYPE type, u_int keyIx,
 		HAL_CIPHER cipher, uint8_t delims, u_int segLen, HAL_BOOL firstSeg,
 		HAL_BOOL lastSeg);
 extern	HAL_BOOL ar5416SetupFirstTxDesc(struct ath_hal *ah, struct ath_desc *ds,

Modified: user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416_xmit.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416_xmit.c	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/ath_hal/ar5416/ar5416_xmit.c	Sun Sep 11 16:42:03 2011	(r225479)
@@ -303,12 +303,14 @@ ar5416FillTxDesc(struct ath_hal *ah, str
 		 * copy the multi-rate transmit parameters from
 		 * the first frame for processing on completion. 
 		 */
-		ads->ds_ctl0 = 0;
 		ads->ds_ctl1 = segLen;
 #ifdef AH_NEED_DESC_SWAP
+		ads->ds_ctl0 = __bswap32(AR5416DESC_CONST(ds0)->ds_ctl0)
+		    & AR_TxIntrReq;
 		ads->ds_ctl2 = __bswap32(AR5416DESC_CONST(ds0)->ds_ctl2);
 		ads->ds_ctl3 = __bswap32(AR5416DESC_CONST(ds0)->ds_ctl3);
 #else
+		ads->ds_ctl0 = AR5416DESC_CONST(ds0)->ds_ctl0 & AR_TxIntrReq;
 		ads->ds_ctl2 = AR5416DESC_CONST(ds0)->ds_ctl2;
 		ads->ds_ctl3 = AR5416DESC_CONST(ds0)->ds_ctl3;
 #endif
@@ -316,7 +318,12 @@ ar5416FillTxDesc(struct ath_hal *ah, str
 		/*
 		 * Intermediate descriptor in a multi-descriptor frame.
 		 */
-		ads->ds_ctl0 = 0;
+#ifdef AH_NEED_DESC_SWAP
+		ads->ds_ctl0 = __bswap32(AR5416DESC_CONST(ds0)->ds_ctl0)
+		    & AR_TxIntrReq;
+#else
+		ads->ds_ctl0 = AR5416DESC_CONST(ds0)->ds_ctl0 & AR_TxIntrReq;
+#endif
 		ads->ds_ctl1 = segLen | AR_TxMore;
 		ads->ds_ctl2 = 0;
 		ads->ds_ctl3 = 0;
@@ -331,6 +338,7 @@ ar5416FillTxDesc(struct ath_hal *ah, str
  */
 HAL_BOOL
 ar5416ChainTxDesc(struct ath_hal *ah, struct ath_desc *ds,
+	u_int flags,
 	u_int pktLen,
 	u_int hdrLen,
 	HAL_PKT_TYPE type,
@@ -369,7 +377,15 @@ ar5416ChainTxDesc(struct ath_hal *ah, st
 	 */
 	OS_MEMZERO(ds->ds_hw, AR5416_DESC_TX_CTL_SZ);
 
-	ads->ds_ctl0 = (pktLen & AR_FrameLen);
+	/*
+	 * XXX VEOL should only be for the last descriptor in the chain.
+	 * XXX I'm not sure if clrdestmask is ok in intermediary descs or
+	 * XXX   required at the end of a sub-frame.
+	 */
+	ads->ds_ctl0 = (pktLen & AR_FrameLen)
+//		| (flags & HAL_TXDESC_VEOL ? AR_VEOL : 0)
+//		| (flags & HAL_TXDESC_CLRDMASK ? AR_ClrDestMask : 0)
+		| (flags & HAL_TXDESC_INTREQ ? AR_TxIntrReq : 0);
 	ads->ds_ctl1 = (type << AR_FrameType_S)
 			| (isaggr ? (AR_IsAggr | AR_MoreAggr) : 0);
 	ads->ds_ctl2 = 0;

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c	Sun Sep 11 16:42:03 2011	(r225479)
@@ -355,6 +355,7 @@ ath_tx_chaindesclist_subframe(struct ath
 		 * This includes enabling the aggregate flags if needed.
 		 */
 		ath_hal_chaintxdesc(ah, ds,
+		    bf->bf_state.bfs_flags | HAL_TXDESC_INTREQ,
 		    bf->bf_state.bfs_pktlen,
 		    bf->bf_state.bfs_hdrlen,
 		    HAL_PKT_TYPE_AMPDU,	/* forces aggregate bits to be set */

Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h
==============================================================================
--- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sun Sep 11 16:28:07 2011	(r225478)
+++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h	Sun Sep 11 16:42:03 2011	(r225479)
@@ -842,9 +842,9 @@ void	ath_intr(void *);
 		_txr0, _txtr0, _antm, _rcr, _rcd) \
 	((*(_ah)->ah_setupFirstTxDesc)((_ah), (_ds), (_aggrlen), (_flags), \
 	(_txpower), (_txr0), (_txtr0), (_antm), (_rcr), (_rcd)))
-#define	ath_hal_chaintxdesc(_ah, _ds, _pktlen, _hdrlen, _type, _keyix, \
+#define	ath_hal_chaintxdesc(_ah, _ds, _flags, _pktlen, _hdrlen, _type, _keyix, \
 	_cipher, _delims, _seglen, _first, _last) \
-	((*(_ah)->ah_chainTxDesc)((_ah), (_ds), (_pktlen), (_hdrlen), \
+	((*(_ah)->ah_chainTxDesc)((_ah), (_ds), (_flags), (_pktlen), (_hdrlen), \
 	(_type), (_keyix), (_cipher), (_delims), (_seglen), \
 	(_first), (_last)))
 #define	ath_hal_setuplasttxdesc(_ah, _ds, _ds0) \



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