From owner-svn-src-user@FreeBSD.ORG Sun Sep 11 16:42:04 2011 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4BB3B106566B; Sun, 11 Sep 2011 16:42:04 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 3A1518FC0A; Sun, 11 Sep 2011 16:42:04 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p8BGg4Ne086920; Sun, 11 Sep 2011 16:42:04 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p8BGg4L3086913; Sun, 11 Sep 2011 16:42:04 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201109111642.p8BGg4L3086913@svn.freebsd.org> From: Adrian Chadd Date: Sun, 11 Sep 2011 16:42:04 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r225479 - in user/adrian/if_ath_tx/sys/dev/ath: . ath_hal ath_hal/ar5212 ath_hal/ar5416 X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Sep 2011 16:42:04 -0000 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) \