[PATCH] Refactor TX path
Eugene Krasnikov
k.eugene.e at gmail.com
Fri Jul 5 15:24:17 EDT 2013
Hi All,
This patch is a refactoring of TX path. There are a lot of changes in
this patch so i would really appreciate your review and feedback. What
else can be improved?
This is pull request to make review process easier
https://github.com/KrasnikovEugene/wcn36xx/pull/68
Mean while i will test the patch as much as possible. Will go through
all possible usecases in both STA and AP mode.
2013/7/5 Eugene Krasnikov <k.eugene.e at gmail.com>:
> With this patch all parameters will be first passed to txrx
> module and only after that to dxe. This will allow passing
> less parameters to the functions. Also make sure that data
> frames will get as fast as possible to the HW.
>
> Short function description:
>
> wcn36xx_start_tx – fills in BD and trigger dxe to start
> transmitting.
>
> wcn36xx_fill_tx_pdu – fills is PDU part of BD.
>
> wcn36xx_dxe_tx_frame – transmits one frame from the head
> of the descriptors.
>
> Signed-off-by: Eugene Krasnikov <k.eugene.e at gmail.com>
> ---
> dxe.c | 59 +++++---------------------
> dxe.h | 13 ++----
> main.c | 35 +---------------
> txrx.c | 139 +++++++++++++++++++++++++++++++++++++++++++++-----------------
> txrx.h | 16 ++++++--
> wcn36xx.h | 1 +
> 6 files changed, 130 insertions(+), 133 deletions(-)
>
> diff --git a/dxe.c b/dxe.c
> index 7237f4f..5e0dd89 100644
> --- a/dxe.c
> +++ b/dxe.c
> @@ -24,6 +24,13 @@
> #include "wcn36xx.h"
> #include "txrx.h"
>
> +void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low)
> +{
> + struct wcn36xx_dxe_ch *ch = is_low ?
> + &wcn->dxe_tx_l_ch :
> + &wcn->dxe_tx_h_ch;
> + return ch->head_blk_ctl->bd_cpu_addr;
> +}
> static void wcn36xx_dxe_write_register(struct wcn36xx *wcn, int addr, int data)
> {
> wcn36xx_dbg(WCN36XX_DBG_DXE,
> @@ -525,63 +532,19 @@ void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn)
> }
> }
>
> -int wcn36xx_dxe_tx(struct wcn36xx *wcn,
> - struct sk_buff *skb,
> - u8 broadcast,
> - bool is_high,
> - u32 header_len,
> - bool tx_ack,
> - struct wcn_sta *sta_priv)
> +int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
> + struct sk_buff *skb,
> + bool is_low)
> {
> struct wcn36xx_dxe_ctl *ctl = NULL;
> struct wcn36xx_dxe_desc *desc = NULL;
> struct wcn36xx_dxe_ch *ch = NULL;
> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> - unsigned long flags;
> -
> - ch = is_high ? &wcn->dxe_tx_h_ch : &wcn->dxe_tx_l_ch;
> -
> - if (tx_ack) {
> - wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested");
> - spin_lock_irqsave(&wcn->dxe_lock, flags);
> - if (wcn->tx_ack_skb) {
> - spin_unlock_irqrestore(&wcn->dxe_lock, flags);
> - wcn36xx_warn("tx_ack_skb already set");
> - ieee80211_free_txskb(wcn->hw, skb);
> - return -EINVAL;
> - }
> -
> - wcn->tx_ack_skb = skb;
> - spin_unlock_irqrestore(&wcn->dxe_lock, flags);
>
> - /* Only one at a time is supported by fw. Stop the TX queues
> - * until the ack status gets back.
> - *
> - * TODO: Add watchdog in case FW does not answer
> - */
> - ieee80211_stop_queues(wcn->hw);
> - }
> + ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch;
>
> ctl = ch->head_blk_ctl;
> ctl->skb = NULL;
> desc = ctl->desc;
> - if (!ctl->bd_cpu_addr) {
> - /* TX DXE are used in pairs. One for the BD and one for the
> - actual frame. The BD DXE's has a preallocated buffer while
> - the skb ones does not. If this isn't true something is really
> - wierd. TODO: Recover from this situation
> - */
> -
> - wcn36xx_error("bd_cpu_addr may not be NULL for BD DXE");
> - return -EINVAL;
> - }
> -
> - wcn36xx_prepare_tx_bd(ctl->bd_cpu_addr, skb->len, header_len);
> - wcn36xx_fill_tx_bd(wcn, ctl->bd_cpu_addr, broadcast,
> - hdr, tx_ack, sta_priv);
> -
> - ctl = ch->head_blk_ctl;
> - desc = ctl->desc;
>
> /* Set source address of the BD we send */
> desc->src_addr_l = ctl->bd_phy_addr;
> diff --git a/dxe.h b/dxe.h
> index 9274d51..0b154bb 100644
> --- a/dxe.h
> +++ b/dxe.h
> @@ -222,8 +222,6 @@ struct wcn36xx_dxe_mem_pool {
> void *virt_addr;
> dma_addr_t phy_addr;
> };
> -struct wcn36xx;
> -struct wcn_sta;
> int wcn36xx_dxe_allocate_mem_pools(struct wcn36xx *wcn);
> void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn);
> void wcn36xx_rx_ready_work(struct work_struct *work);
> @@ -232,12 +230,9 @@ void wcn36xx_dxe_free_ctl_blks(struct wcn36xx *wcn);
> int wcn36xx_dxe_init(struct wcn36xx *wcn);
> void wcn36xx_dxe_deinit(struct wcn36xx *wcn);
> int wcn36xx_dxe_init_channels(struct wcn36xx *wcn);
> -int wcn36xx_dxe_tx(struct wcn36xx *wcn,
> - struct sk_buff *skb,
> - u8 broadcast,
> - bool is_high,
> - u32 header_len,
> - bool tx_compl,
> - struct wcn_sta *sta_priv);
> +int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
> + struct sk_buff *skb,
> + bool is_low);
> void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status);
> +void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low);
> #endif /* _DXE_H_ */
> diff --git a/main.c b/main.c
> index 74646b0..cbfe403 100644
> --- a/main.c
> +++ b/main.c
> @@ -14,7 +14,6 @@
> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
>
> -#include <linux/etherdevice.h>
> #include <linux/module.h>
> #include <linux/wcnss_wlan.h>
> #include "wcn36xx.h"
> @@ -309,46 +308,14 @@ static void wcn36xx_tx(struct ieee80211_hw *hw,
> struct ieee80211_tx_control *control,
> struct sk_buff *skb)
> {
> - struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> - struct ieee80211_mgmt *mgmt;
> - bool high, bcast, tx_compl;
> - u32 header_len = 0;
> struct wcn36xx *wcn = hw->priv;
> struct wcn_sta *sta_priv = NULL;
>
> if (control->sta)
> sta_priv = (struct wcn_sta *)control->sta->drv_priv;
>
> - tx_compl = info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS;
> - mgmt = (struct ieee80211_mgmt *)skb->data;
> -
> - high = !(ieee80211_is_data(mgmt->frame_control) ||
> - ieee80211_is_data_qos(mgmt->frame_control));
> -
> - bcast = is_broadcast_ether_addr(mgmt->da) ||
> - is_multicast_ether_addr(mgmt->da);
> -
> - /*
> - * In joining state trick hardware that probe is sent as unicast even
> - * if address is broadcast.
> - */
> - if (wcn->is_joining && ieee80211_is_probe_req(mgmt->frame_control))
> - bcast = false;
> - wcn36xx_dbg(WCN36XX_DBG_TX,
> - "tx skb %p len %d fc %04x sn %d %s %s",
> - skb, skb->len, __le16_to_cpu(mgmt->frame_control),
> - IEEE80211_SEQ_TO_SN(__le16_to_cpu(mgmt->seq_ctrl)),
> - high ? "high" : "low", bcast ? "bcast" : "ucast");
> -
> - wcn36xx_dbg_dump(WCN36XX_DBG_TX_DUMP, "", skb->data, skb->len);
> -
> - header_len = ieee80211_is_data_qos(mgmt->frame_control) ?
> - sizeof(struct ieee80211_qos_hdr) :
> - sizeof(struct ieee80211_hdr_3addr);
> - wcn36xx_dxe_tx(hw->priv, skb, bcast, high,
> - header_len, tx_compl, sta_priv);
> + wcn36xx_start_tx(wcn, sta_priv, skb);
> }
> -
> static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> struct ieee80211_vif *vif,
> struct ieee80211_sta *sta,
> diff --git a/txrx.c b/txrx.c
> index 3b85727..5c35ec0 100644
> --- a/txrx.c
> +++ b/txrx.c
> @@ -73,52 +73,85 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
>
> return 0;
> }
> -void wcn36xx_prepare_tx_bd(struct wcn36xx_tx_bd *bd, u32 len, u32 header_len)
> +
> +void wcn36xx_fill_tx_pdu(struct wcn36xx_tx_bd *bd,
> + u32 mpdu_header_len,
> + u32 len)
> {
> - memset(bd, 0, sizeof(*bd));
> - bd->pdu.mpdu_header_len = header_len;
> + bd->pdu.mpdu_header_len = mpdu_header_len;
> bd->pdu.mpdu_header_off = sizeof(*bd);
> bd->pdu.mpdu_data_off = bd->pdu.mpdu_header_len +
> bd->pdu.mpdu_header_off;
> bd->pdu.mpdu_len = len;
> + bd->pdu.tid = WCN36XX_TID;
> }
> -void wcn36xx_fill_tx_bd(struct wcn36xx *wcn, struct wcn36xx_tx_bd *bd,
> - u8 broadcast, struct ieee80211_hdr *hdr,
> - bool tx_compl, struct wcn_sta *sta_priv)
> +
> +int wcn36xx_start_tx(struct wcn36xx *wcn,
> + struct wcn_sta *sta_priv,
> + struct sk_buff *skb)
> {
> - bd->dpu_rf = WCN36XX_BMU_WQ_TX;
> - bd->pdu.tid = WCN36XX_TID;
> - bd->pdu.reserved3 = 0xd;
> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + unsigned long flags;
> + bool is_low = ieee80211_is_data(hdr->frame_control);
> + bool bcast = is_broadcast_ether_addr(hdr->addr1) ||
> + is_multicast_ether_addr(hdr->addr1);
> + struct wcn36xx_tx_bd *bd = wcn36xx_dxe_get_next_bd(wcn, is_low);
> +
> + if (!bd) {
> + /*
> + * TX DXE are used in pairs. One for the BD and one for the
> + * actual frame. The BD DXE's has a preallocated buffer while
> + * the skb ones does not. If this isn't true something is really
> + * wierd. TODO: Recover from this situation
> + */
> +
> + wcn36xx_error("bd address may not be NULL for BD DXE");
> + return -EINVAL;
> + }
>
> - if (broadcast) {
> - /* broadcast */
> - bd->ub = 1;
> - bd->queue_id = WCN36XX_TX_B_WQ_ID;
> + wcn36xx_dbg(WCN36XX_DBG_TX,
> + "tx skb %p len %d fc %04x sn %d %s %s",
> + skb, skb->len, __le16_to_cpu(hdr->frame_control),
> + IEEE80211_SEQ_TO_SN(__le16_to_cpu(hdr->seq_ctrl)),
> + is_low ? "low" : "high", bcast ? "bcast" : "ucast");
>
> - /* default rate for broadcast */
> - if (ieee80211_is_mgmt(hdr->frame_control))
> - bd->bd_rate = (wcn->band == IEEE80211_BAND_5GHZ) ?
> - WCN36XX_BD_RATE_CTRL :
> - WCN36XX_BD_RATE_MGMT;
> - /* No ack needed not unicast */
> - bd->ack_policy = 1;
> - } else {
> - bd->queue_id = WCN36XX_TX_U_WQ_ID;
> - /* default rate for unicast */
> - bd->ack_policy = 0;
> - if (ieee80211_is_data(hdr->frame_control))
> - bd->bd_rate = WCN36XX_BD_RATE_DATA;
> - else if (ieee80211_is_mgmt(hdr->frame_control))
> - bd->bd_rate = (wcn->band == IEEE80211_BAND_5GHZ) ?
> - WCN36XX_BD_RATE_CTRL :
> - WCN36XX_BD_RATE_MGMT;
> - else if (ieee80211_is_ctl(hdr->frame_control))
> - bd->bd_rate = WCN36XX_BD_RATE_CTRL;
> - else
> - wcn36xx_warn("frame control type unknown");
> + wcn36xx_dbg_dump(WCN36XX_DBG_TX_DUMP, "", skb->data, skb->len);
> +
> + memset(bd, 0, sizeof(*bd));
> +
> + bd->dpu_rf = WCN36XX_BMU_WQ_TX;
> +
> + bd->tx_comp = info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS;
> + if (bd->tx_comp) {
> + wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested");
> + spin_lock_irqsave(&wcn->dxe_lock, flags);
> + if (wcn->tx_ack_skb) {
> + spin_unlock_irqrestore(&wcn->dxe_lock, flags);
> + wcn36xx_warn("tx_ack_skb already set");
> + ieee80211_free_txskb(wcn->hw, skb);
> + return -EINVAL;
> + }
> +
> + wcn->tx_ack_skb = skb;
> + spin_unlock_irqrestore(&wcn->dxe_lock, flags);
> +
> + /* Only one at a time is supported by fw. Stop the TX queues
> + * until the ack status gets back.
> + *
> + * TODO: Add watchdog in case FW does not answer
> + */
> + ieee80211_stop_queues(wcn->hw);
> }
>
> - if (ieee80211_is_data(hdr->frame_control)) {
> + wcn36xx_fill_tx_pdu(bd, ieee80211_is_data_qos(hdr->frame_control) ?
> + sizeof(struct ieee80211_qos_hdr) :
> + sizeof(struct ieee80211_hdr_3addr),
> + skb->len);
> +
> + /* Data frames served first*/
> + if (is_low) {
> + bd->bd_rate = WCN36XX_BD_RATE_DATA;
> bd->dpu_sign = wcn->current_vif->ucast_dpu_signature;
> bd->queue_id = 0;
> bd->sta_index = wcn->current_vif->sta_index;
> @@ -126,15 +159,45 @@ void wcn36xx_fill_tx_bd(struct wcn36xx *wcn, struct wcn36xx_tx_bd *bd,
> if (ieee80211_is_nullfunc(hdr->frame_control) ||
> (sta_priv && !sta_priv->is_data_encrypted))
> bd->dpu_ne = 1;
> -
> + if (bcast) {
> + bd->ub = 1;
> + bd->ack_policy = 1;
> + }
> } else {
> bd->sta_index = wcn->current_vif->self_sta_index;
> bd->dpu_desc_idx = wcn->current_vif->self_dpu_desc_index;
> bd->dpu_ne = 1;
> - }
>
> - bd->tx_comp = tx_compl;
> + /* default rate for unicast */
> + if (ieee80211_is_mgmt(hdr->frame_control))
> + bd->bd_rate = (wcn->band == IEEE80211_BAND_5GHZ) ?
> + WCN36XX_BD_RATE_CTRL :
> + WCN36XX_BD_RATE_MGMT;
> + else if (ieee80211_is_ctl(hdr->frame_control))
> + bd->bd_rate = WCN36XX_BD_RATE_CTRL;
> + else
> + wcn36xx_warn("frame control type unknown");
> +
> + /*
> + * In joining state trick hardware that probe is sent as
> + * unicast even if address is broadcast.
> + */
> + if (wcn->is_joining &&
> + ieee80211_is_probe_req(hdr->frame_control))
> + bcast = false;
> +
> + if (bcast) {
> + /* broadcast */
> + bd->ub = 1;
> + /* No ack needed not unicast */
> + bd->ack_policy = 1;
> + bd->queue_id = WCN36XX_TX_B_WQ_ID;
> + } else
> + bd->queue_id = WCN36XX_TX_U_WQ_ID;
> + }
>
> buff_to_be((u32 *)bd, sizeof(*bd)/sizeof(u32));
> bd->tx_bd_sign = 0xbdbdbdbd;
> +
> + return wcn36xx_dxe_tx_frame(wcn, skb, is_low);
> }
> diff --git a/txrx.h b/txrx.h
> index ffcbd17..bf2994f 100644
> --- a/txrx.h
> +++ b/txrx.h
> @@ -17,6 +17,7 @@
> #ifndef _TXRX_H_
> #define _TXRX_H_
>
> +#include <linux/etherdevice.h>
> #include "wcn36xx.h"
>
> /* TODO describe all properties */
> @@ -147,9 +148,16 @@ struct wcn36xx_tx_bd {
> u32 header_cks:16;
> u32 reserved7:6;*/
> };
> +
> +struct wcn_sta;
> +struct wcn36xx;
> +
> int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb);
> -void wcn36xx_prepare_tx_bd(struct wcn36xx_tx_bd *bd, u32 len, u32 header_len);
> -void wcn36xx_fill_tx_bd(struct wcn36xx *wcn, struct wcn36xx_tx_bd *bd,
> - u8 broadcast, struct ieee80211_hdr *hdr,
> - bool tx_compl, struct wcn_sta *sta_priv);
> +void wcn36xx_fill_tx_pdu(struct wcn36xx_tx_bd *bd,
> + u32 mpdu_header_len,
> + u32 len);
> +int wcn36xx_start_tx(struct wcn36xx *wcn,
> + struct wcn_sta *sta_priv,
> + struct sk_buff *skb);
> +
> #endif /* _TXRX_H_ */
> diff --git a/wcn36xx.h b/wcn36xx.h
> index 78a0f7b..89ffc2f 100644
> --- a/wcn36xx.h
> +++ b/wcn36xx.h
> @@ -27,6 +27,7 @@
>
> #include "hal.h"
> #include "smd.h"
> +#include "txrx.h"
> #include "dxe.h"
>
> #define DRIVER_PREFIX "wcn36xx: "
> --
> 1.7.11.3
>
--
Best regards,
Eugene
More information about the wcn36xx
mailing list