@@ -831,6 +831,32 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
return ret;
}
+int wcn36xx_dxe_set_tx_ack_skb(struct wcn36xx *wcn, struct sk_buff *skb)
+{
+ unsigned long flags;
+
+ 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\n");
+ return -EINVAL;
+ }
+
+ wcn->tx_ack_skb = skb;
+ spin_unlock_irqrestore(&wcn->dxe_lock, flags);
+
+ return 0;
+}
+
+void wcn36xx_dxe_unset_tx_ack_skb(struct wcn36xx *wcn)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wcn->dxe_lock, flags);
+ wcn->tx_ack_skb = NULL;
+ spin_unlock_irqrestore(&wcn->dxe_lock, flags);
+}
+
int wcn36xx_dxe_init(struct wcn36xx *wcn)
{
int reg_data = 0, ret;
@@ -467,4 +467,6 @@ 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);
+int wcn36xx_dxe_set_tx_ack_skb(struct wcn36xx *wcn, struct sk_buff *skb);
+void wcn36xx_dxe_unset_tx_ack_skb(struct wcn36xx *wcn);
#endif /* _DXE_H_ */
@@ -502,7 +502,6 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
struct wcn36xx_vif *vif_priv = NULL;
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);
@@ -524,15 +523,8 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
if (info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS) {
wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested\n");
- 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\n");
+ if (wcn36xx_dxe_set_tx_ack_skb(wcn, 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.
@@ -562,10 +554,7 @@ int wcn36xx_start_tx(struct wcn36xx *wcn,
/* If the skb has not been transmitted,
* don't keep a reference to it.
*/
- spin_lock_irqsave(&wcn->dxe_lock, flags);
- wcn->tx_ack_skb = NULL;
- spin_unlock_irqrestore(&wcn->dxe_lock, flags);
-
+ wcn36xx_dxe_unset_tx_ack_skb(wcn);
ieee80211_wake_queues(wcn->hw);
}
Looking at the code here we see that txrx.c is taking the dxe.c lock to set and unset the current TX skbuff pointer. There is no obvious logical bug however, it is a layering violation to share locks like this. Lets tidy up the code a bit by making access functions to set and unset the TX sbuff. This makes it easier to read and reason about this code without having to switch between multiple files. Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware") Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/net/wireless/ath/wcn36xx/dxe.c | 26 +++++++++++++++++++++++++ drivers/net/wireless/ath/wcn36xx/dxe.h | 2 ++ drivers/net/wireless/ath/wcn36xx/txrx.c | 15 ++------------ 3 files changed, 30 insertions(+), 13 deletions(-) -- 2.33.0 _______________________________________________ wcn36xx mailing list wcn36xx@lists.infradead.org http://lists.infradead.org/mailman/listinfo/wcn36xx