Date: Tue, 31 Jul 2012 17:08:29 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r238949 - head/sys/dev/ath Message-ID: <201207311708.q6VH8Tnb050115@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Tue Jul 31 17:08:29 2012 New Revision: 238949 URL: http://svn.freebsd.org/changeset/base/238949 Log: Shuffle the call to ath_hal_setuplasttxdesc() to _after_ the rate control code is called and remove it from ath_buf_set_rate(). For the legacy (non-11n API) TX routines, ath_hal_filltxdesc() takes care of setting up the intermediary and final descriptors right, complete with copying the rate control info into the final descriptor so the rate modules can grab it. The 11n version doesn't do this - ath_hal_chaintxdesc() doesn't copy the rate control bits over, nor does it clear isaggr/moreaggr/ pad delimiters. So the call to setuplasttxdesc() is needed here. So: * legacy NICs - never call the 11n rate control stuff, so filltxdesc copies the rate control info right; * 11n NICs transmitting legacy or 11n non-aggregate frames - ath_hal_set11nratescenario() is called to setup rate control and then ath_hal_filltxdesc() chains them together - so the rate control info is right; * 11n aggregate frames - set11nratescenario() is called, then ath_hal_chaintxdesc() is called to chain a list of aggregate and subframes together. This requires a call to ath_hal_setuplasttxdesc() to complete things. Tested: * AR9280 in station mode TODO: * I really should make sure that the descriptor contents get blanked out correctly or garbage left over from aggregate frames may show up in non-aggregate frames, leading to badness. Modified: head/sys/dev/ath/if_ath_tx.c head/sys/dev/ath/if_ath_tx_ht.c Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Tue Jul 31 16:55:41 2012 (r238948) +++ head/sys/dev/ath/if_ath_tx.c Tue Jul 31 17:08:29 2012 (r238949) @@ -496,13 +496,6 @@ ath_tx_setds_11n(struct ath_softc *sc, s bf_first->bf_state.bfs_ctsduration); /* - * Setup the last descriptor in the list. - * bf_prev points to the last; bf is NULL here. - */ - ath_hal_setuplasttxdesc(sc->sc_ah, bf_prev->bf_desc, - bf_first->bf_desc); - - /* * Set the first descriptor bf_lastds field to point to * the last descriptor in the last subframe, that's where * the status update will occur. @@ -520,6 +513,13 @@ ath_tx_setds_11n(struct ath_softc *sc, s */ ath_tx_set_ratectrl(sc, bf_first->bf_node, bf_first); + /* + * Setup the last descriptor in the list. + * bf_prev points to the last; bf is NULL here. + */ + ath_hal_setuplasttxdesc(sc->sc_ah, bf_prev->bf_desc, + bf_first->bf_desc); + DPRINTF(sc, ATH_DEBUG_SW_TX_AGGR, "%s: end\n", __func__); } Modified: head/sys/dev/ath/if_ath_tx_ht.c ============================================================================== --- head/sys/dev/ath/if_ath_tx_ht.c Tue Jul 31 16:55:41 2012 (r238948) +++ head/sys/dev/ath/if_ath_tx_ht.c Tue Jul 31 17:08:29 2012 (r238949) @@ -560,14 +560,12 @@ ath_rateseries_print(struct ath_softc *s * This isn't useful for sending beacon frames, which has different needs * wrt what's passed into the rate scenario function. */ - void ath_buf_set_rate(struct ath_softc *sc, struct ieee80211_node *ni, struct ath_buf *bf) { HAL_11N_RATE_SERIES series[4]; struct ath_desc *ds = bf->bf_desc; - struct ath_desc *lastds = NULL; struct ath_hal *ah = sc->sc_ah; int is_pspoll = (bf->bf_state.bfs_atype == HAL_PKT_TYPE_PSPOLL); int ctsrate = bf->bf_state.bfs_ctsrate; @@ -578,13 +576,6 @@ ath_buf_set_rate(struct ath_softc *sc, s ath_rateseries_setup(sc, ni, bf, series); - /* Enforce AR5416 aggregate limit - can't do RTS w/ an agg frame > 8k */ - - /* Enforce RTS and CTS are mutually exclusive */ - - /* Get a pointer to the last tx descriptor in the list */ - lastds = bf->bf_lastds; - #if 0 printf("pktlen: %d; flags 0x%x\n", pktlen, flags); ath_rateseries_print(sc, series); @@ -602,21 +593,6 @@ ath_buf_set_rate(struct ath_softc *sc, s 4, /* number of series */ flags); - /* Setup the last descriptor in the chain */ - /* - * XXX Why is this done here, and not in the upper layer? - * The rate control code stores a copy of the RC info in - * the last descriptor as well as the first, then uses - * the shadow copy in the last descriptor to see what the RC - * decisions were. I'm not sure why; perhaps earlier hardware - * overwrote the first descriptor contents. - * - * In the 802.11n case, it also clears the moreaggr/delim - * fields. Again, this should be done by the caller of - * ath_buf_set_rate(). - */ - ath_hal_setuplasttxdesc(ah, lastds, ds); - /* Set burst duration */ /* * This is only required when doing 11n burst, not aggregation
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201207311708.q6VH8Tnb050115>