msm8916 + wcn3620 mainline support?

[Bob Copeland]

On Tue, Jan 20, 2015 at 08:05:04AM +0800, Andy Green wrote:
> > RE stability problems, I have a couple of locking-related patches that
> > aren't upstream yet (or in wcn36xx-devel) that might be worth a look.
> > I'll clean them up and send along for discussion.
> 
> I'd be very happy to give them a try.

Give this a go - it looked to me like it was possible for ring list
to get corrupted when driver and irq were both manipulating it, and
existing skb lock was by itself insufficient.

>From 96ff443d35aef0f76bbdec03028fee062b6fd84d Mon Sep 17 00:00:00 2001
From: Bob Copeland <me at bobcopeland.com>
Date: Tue, 20 Jan 2015 12:11:59 -0500
Subject: [PATCH] wcn36xx: add locking around ring buffer accesses

Signed-off-by: Bob Copeland <me at bobcopeland.com>
---
 drivers/net/wireless/ath/wcn36xx/dxe.c |   27 +++++++++++++++++++--------
 drivers/net/wireless/ath/wcn36xx/dxe.h |    1 +
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
index 086549b..26085f7 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -79,6 +79,7 @@ static int wcn36xx_dxe_allocate_ctl_block(struct wcn36xx_dxe_ch *ch)
 	struct wcn36xx_dxe_ctl *cur_ctl = NULL;
 	int i;
 
+	spin_lock_init(&ch->lock);
 	for (i = 0; i < ch->desc_num; i++) {
 		cur_ctl = kzalloc(sizeof(*cur_ctl), GFP_KERNEL);
 		if (!cur_ctl)
@@ -345,7 +346,7 @@ void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status)
 
 static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
 {
-	struct wcn36xx_dxe_ctl *ctl = ch->tail_blk_ctl;
+	struct wcn36xx_dxe_ctl *ctl;
 	struct ieee80211_tx_info *info;
 	unsigned long flags;
 
@@ -354,6 +355,8 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
 	 * completely full head and tail are pointing to the same element
 	 * and while-do will not make any cycles.
 	 */
+	spin_lock_irqsave(&ch->lock, flags);
+	ctl = ch->tail_blk_ctl;
 	do {
 		if (ctl->desc->ctrl & WCN36XX_DXE_CTRL_VALID_MASK)
 			break;
@@ -365,12 +368,12 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
 				/* Keep frame until TX status comes */
 				ieee80211_free_txskb(wcn->hw, ctl->skb);
 			}
-			spin_lock_irqsave(&ctl->skb_lock, flags);
+			spin_lock(&ctl->skb_lock);
 			if (wcn->queues_stopped) {
 				wcn->queues_stopped = false;
 				ieee80211_wake_queues(wcn->hw);
 			}
-			spin_unlock_irqrestore(&ctl->skb_lock, flags);
+			spin_unlock(&ctl->skb_lock);
 
 			ctl->skb = NULL;
 		}
@@ -379,6 +382,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
 	       !(ctl->desc->ctrl & WCN36XX_DXE_CTRL_VALID_MASK));
 
 	ch->tail_blk_ctl = ctl;
+	spin_unlock_irqrestore(&ch->lock, flags);
 }
 
 static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev)
@@ -596,12 +600,14 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
 	struct wcn36xx_dxe_desc *desc = NULL;
 	struct wcn36xx_dxe_ch *ch = NULL;
 	unsigned long flags;
+	int ret;
 
 	ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch;
 
+	spin_lock_irqsave(&ch->lock, flags);
 	ctl = ch->head_blk_ctl;
 
-	spin_lock_irqsave(&ctl->next->skb_lock, flags);
+	spin_lock(&ctl->next->skb_lock);
 
 	/*
 	 * If skb is not null that means that we reached the tail of the ring
@@ -611,10 +617,11 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
 	if (NULL != ctl->next->skb) {
 		ieee80211_stop_queues(wcn->hw);
 		wcn->queues_stopped = true;
-		spin_unlock_irqrestore(&ctl->next->skb_lock, flags);
+		spin_unlock(&ctl->next->skb_lock);
+		spin_unlock_irqrestore(&ch->lock, flags);
 		return -EBUSY;
 	}
-	spin_unlock_irqrestore(&ctl->next->skb_lock, flags);
+	spin_unlock(&ctl->next->skb_lock);
 
 	ctl->skb = NULL;
 	desc = ctl->desc;
@@ -640,7 +647,8 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
 	desc = ctl->desc;
 	if (ctl->bd_cpu_addr) {
 		wcn36xx_err("bd_cpu_addr cannot be NULL for skb DXE\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto unlock;
 	}
 
 	desc->src_addr_l = dma_map_single(NULL,
@@ -679,7 +687,10 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
 			ch->reg_ctrl, ch->def_ctrl);
 	}
 
-	return 0;
+	ret = 0;
+unlock:
+	spin_unlock_irqrestore(&ch->lock, flags);
+	return ret;
 }
 
 int wcn36xx_dxe_init(struct wcn36xx *wcn)
diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.h b/drivers/net/wireless/ath/wcn36xx/dxe.h
index 35ee7e9..f869fd3 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.h
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.h
@@ -243,6 +243,7 @@ struct wcn36xx_dxe_ctl {
 };
 
 struct wcn36xx_dxe_ch {
+	spinlock_t			lock;
 	enum wcn36xx_dxe_ch_type	ch_type;
 	void				*cpu_addr;
 	dma_addr_t			dma_addr;
-- 
1.7.10.4




-- 
Bob Copeland %% http://bobcopeland.com/