cw1200: Fix for buffered multicasts in AP-PS mode: bugfix.

Bugfix of Change-Id I15b134bf847913a907f00293ae99d7a71bbc7343.
 + Locking semantic is simplified.
 + Overcomplicated check of aid0_bit_timestamp removed: FW is doing it.
 + wsm_get_tx_queue_and_mask() mistakely ignored suspend_multicast.

Change-Id: I0152fefd167dbb8a6fdc8dd8f5582b10fb645006
Signed-off-by: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>
Reviewed-on: http://gerrit.lud.stericsson.com/gerrit/28152
Reviewed-by: Bartosz MARKOWSKI <bartosz.markowski@tieto.com>
Tested-by: Bartosz MARKOWSKI <bartosz.markowski@tieto.com>
Reviewed-on: http://gerrit.lud.stericsson.com/gerrit/33492
Reviewed-by: Philippe LANGLAIS <philippe.langlais@stericsson.com>

5 files changed, 45 insertions, 62 deletions

diff --git a/drivers/staging/cw1200/ap.c b/drivers/staging/cw1200/ap.c
index adff7d51fc2..55f254a6da6 100755
--- a/drivers/staging/cw1200/ap.c
+++ b/drivers/staging/cw1200/ap.c
@@ -89,18 +89,28 @@ void cw1200_sta_notify(struct ieee80211_hw *dev, struct ieee80211_vif *vif,
 		(struct cw1200_sta_priv *)&sta->drv_priv;
 	u32 bit = BIT(sta_priv->link_id);
 
+	spin_lock_bh(&priv->buffered_multicasts_lock);
 	switch (notify_cmd) {
 		case STA_NOTIFY_SLEEP:
+			if (priv->buffered_multicasts &&
+					!priv->sta_asleep_mask)
+				queue_work(priv->workqueue,
+					&priv->multicast_start_work);
 			priv->sta_asleep_mask |= bit;
 			break;
 		case STA_NOTIFY_AWAKE:
 			priv->sta_asleep_mask &= ~bit;
+			if (priv->tx_multicast &&
+					!priv->sta_asleep_mask)
+				queue_work(priv->workqueue,
+					&priv->multicast_stop_work);
 			cw1200_bh_wakeup(priv);
 			break;
 	}
+	spin_unlock_bh(&priv->buffered_multicasts_lock);
 }
 
-static int cw1200_set_tim_impl(struct cw1200_common *priv)
+static int cw1200_set_tim_impl(struct cw1200_common *priv, bool aid0_bit_set)
 {
 	struct wsm_template_frame frame = {
 		.frame_type = WSM_FRAME_TYPE_BEACON,
@@ -120,7 +130,7 @@ static int cw1200_set_tim_impl(struct cw1200_common *priv)
 		frame.skb->data[tim_offset + 2] = 0;
 
 		/* Set/reset aid0 bit */
-		if (priv->aid0_bit_set)
+		if (aid0_bit_set)
 			frame.skb->data[tim_offset + 4] |= 1;
 		else
 			frame.skb->data[tim_offset + 4] &= ~1;
@@ -137,7 +147,7 @@ void cw1200_set_tim_work(struct work_struct *work)
 {
 	struct cw1200_common *priv =
 		container_of(work, struct cw1200_common, set_tim_work);
-	(void)cw1200_set_tim_impl(priv);
+	(void)cw1200_set_tim_impl(priv, priv->aid0_bit_set);
 }
 
 int cw1200_set_tim(struct ieee80211_hw *dev, struct ieee80211_sta *sta,
@@ -441,15 +451,11 @@ void cw1200_multicast_start_work(struct work_struct *work)
 	struct cw1200_common *priv =
 		container_of(work, struct cw1200_common, multicast_start_work);
 
-	bool store_timestamp = !priv->aid0_bit_set;
-
-	priv->aid0_bit_set = true;
-	cw1200_set_tim_impl(priv);
-	if (store_timestamp) {
-		unsigned long now = jiffies;
-		if (unlikely(!now))
-			++now;
-		priv->aid0_bit_timestamp = now;
+	if (!priv->aid0_bit_set) {
+		wsm_lock_tx(priv);
+		cw1200_set_tim_impl(priv, true);
+		priv->aid0_bit_set = true;
+		wsm_unlock_tx(priv);
 	}
 }
 
@@ -458,12 +464,12 @@ void cw1200_multicast_stop_work(struct work_struct *work)
         struct cw1200_common *priv =
                 container_of(work, struct cw1200_common, multicast_stop_work);
 
-	/* Flush to make sure frames are sent. */
-	wsm_lock_tx(priv);
-	wsm_unlock_tx(priv);
-
-	priv->aid0_bit_set = false;
-	cw1200_set_tim_impl(priv);
+	if (priv->aid0_bit_set) {
+		wsm_lock_tx(priv);
+		priv->aid0_bit_set = false;
+		cw1200_set_tim_impl(priv, false);
+		wsm_unlock_tx(priv);
+	}
 }
 
 int cw1200_ampdu_action(struct ieee80211_hw *hw,
@@ -500,27 +506,16 @@ void cw1200_suspend_resume(struct cw1200_common *priv,
 		arg->multicast ? "broadcast" : "unicast");
 
 	if (arg->multicast) {
+		spin_lock_bh(&priv->buffered_multicasts_lock);
 		if (arg->stop) {
-			queue_work(priv->workqueue,
-				&priv->multicast_stop_work);
+			priv->tx_multicast = false;
 		} else {
-			/* Handle only if there is data to be sent
-			* and aid0 is set in the beacon. */
-			unsigned long timestamp =
-					priv->aid0_bit_timestamp;
-			/* 20ms grace interval is used to make sure
-			 * the beacon was actually sent by hardware. */
-			static const unsigned long grace_interval =
-					HZ * 20 / 1000;
-
-			if (timestamp &&
-				jiffies - timestamp > grace_interval) {
-
-				priv->aid0_bit_timestamp = 0;
-				priv->suspend_multicast = false;
+			priv->tx_multicast = priv->aid0_bit_set &&
+					priv->buffered_multicasts;
+			if (priv->tx_multicast)
 				cw1200_bh_wakeup(priv);
-			}
 		}
+		spin_unlock_bh(&priv->buffered_multicasts_lock);
 	} else {
 		if (arg->stop)
 			set = unicast;
diff --git a/drivers/staging/cw1200/cw1200.h b/drivers/staging/cw1200/cw1200.h
index 9ed86cd39a1..64bae8b03c0 100644
--- a/drivers/staging/cw1200/cw1200.h
+++ b/drivers/staging/cw1200/cw1200.h
@@ -166,11 +166,10 @@ struct cw1200_common {
 	u32			link_id_map;
 	u32			tx_suspend_mask[4];
 	u32			sta_asleep_mask;
-	bool			suspend_multicast;
 	bool			aid0_bit_set;
-	unsigned long		aid0_bit_timestamp;
 	spinlock_t		buffered_multicasts_lock;
 	bool			buffered_multicasts;
+	bool			tx_multicast;
 	struct work_struct	set_tim_work;
 	struct work_struct	multicast_start_work;
 	struct work_struct	multicast_stop_work;
diff --git a/drivers/staging/cw1200/sta.c b/drivers/staging/cw1200/sta.c
index 4923e9dd22f..c24e5c3c8c8 100644
--- a/drivers/staging/cw1200/sta.c
+++ b/drivers/staging/cw1200/sta.c
@@ -121,7 +121,6 @@ void cw1200_stop(struct ieee80211_hw *dev)
 		 * through the map and reset each link separately. */
 		WARN_ON(priv->link_id_map);
 		priv->sta_asleep_mask = 0;
-		priv->suspend_multicast = false;
 		wsm_reset(priv, &reset);
 		break;
 	case CW1200_JOIN_STATUS_MONITOR:
diff --git a/drivers/staging/cw1200/txrx.c b/drivers/staging/cw1200/txrx.c
index 5439947d742..a87743b453a 100644
--- a/drivers/staging/cw1200/txrx.c
+++ b/drivers/staging/cw1200/txrx.c
@@ -422,7 +422,6 @@ void cw1200_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
 		(struct ieee80211_hdr *)skb->data;
 	struct cw1200_sta_priv *sta_priv =
 		(struct cw1200_sta_priv *)&tx_info->control.sta->drv_priv;
-	bool obtain_lock;
 	int link_id = 0;
 	int ret;
 
@@ -512,21 +511,18 @@ void cw1200_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
 		if (cw1200_handle_action_tx(priv, skb))
 			goto drop;
 
-	obtain_lock = (link_id == CW1200_LINK_ID_AFTER_DTIM);
-
-	if (obtain_lock)
-		spin_lock_bh(&priv->buffered_multicasts_lock);
+	spin_lock_bh(&priv->buffered_multicasts_lock);
 
 	if (link_id == CW1200_LINK_ID_AFTER_DTIM &&
 			!priv->buffered_multicasts) {
 		priv->buffered_multicasts = true;
-		queue_work(priv->workqueue,
+		if (priv->sta_asleep_mask)
+			queue_work(priv->workqueue,
 				&priv->multicast_start_work);
 	}
 	ret = cw1200_queue_put(&priv->tx_queue[queue], priv, skb,
 			link_id);
-	if (obtain_lock)
-		spin_unlock_bh(&priv->buffered_multicasts_lock);
+	spin_unlock_bh(&priv->buffered_multicasts_lock);
 
 	if (!WARN_ON(ret))
 		cw1200_bh_wakeup(priv);
diff --git a/drivers/staging/cw1200/wsm.c b/drivers/staging/cw1200/wsm.c
index 4db5129394e..035f8e3d8ce 100644
--- a/drivers/staging/cw1200/wsm.c
+++ b/drivers/staging/cw1200/wsm.c
@@ -1427,7 +1427,7 @@ static int wsm_get_tx_queue_and_mask(struct cw1200_common *priv,
 	int mcasts = 0;
 
 	/* Search for a queue with multicast frames buffered */
-	if (priv->sta_asleep_mask && !priv->suspend_multicast) {
+	if (priv->tx_multicast) {
 		tx_allowed_mask = BIT(CW1200_LINK_ID_AFTER_DTIM);
 		for (i = 0; i < 4; ++i) {
 			mcasts += cw1200_queue_get_num_queued(
@@ -1447,6 +1447,7 @@ static int wsm_get_tx_queue_and_mask(struct cw1200_common *priv,
 		tx_allowed_mask = ~priv->sta_asleep_mask;
 		if (priv->sta_asleep_mask) {
 			tx_allowed_mask |= ~priv->tx_suspend_mask[i];
+			tx_allowed_mask &= ~BIT(CW1200_LINK_ID_AFTER_DTIM);
 		} else {
 			tx_allowed_mask |= BIT(CW1200_LINK_ID_AFTER_DTIM);
 		}
@@ -1491,34 +1492,27 @@ int wsm_get_tx(struct cw1200_common *priv, u8 **data,
 		spin_unlock(&priv->wsm_cmd.lock);
 	} else {
 		for (;;) {
-			bool obtain_lock = priv->sta_asleep_mask &&
-					!priv->suspend_multicast;
 			int ret;
 
 			if (atomic_add_return(0, &priv->tx_lock))
 				break;
 
-			if (obtain_lock)
-				spin_lock_bh(
-					&priv->buffered_multicasts_lock);
+			spin_lock_bh(&priv->buffered_multicasts_lock);
 
 			ret = wsm_get_tx_queue_and_mask(priv, &queue,
 					&tx_allowed_mask, &more);
 
 			if (priv->buffered_multicasts &&
-					priv->sta_asleep_mask &&
-					!priv->suspend_multicast &&
-					(ret || tx_allowed_mask !=
-					 BIT(CW1200_LINK_ID_AFTER_DTIM))) {
+					(ret || !more) &&
+					(priv->tx_multicast ||
+					 !priv->sta_asleep_mask)) {
 				priv->buffered_multicasts = false;
-				priv->suspend_multicast = true;
-				queue_work(priv->workqueue,
-					&priv->multicast_stop_work);
+				if (priv->tx_multicast)
+					queue_work(priv->workqueue,
+						&priv->multicast_stop_work);
 			}
 
-			if (obtain_lock)
-				spin_unlock_bh(
-					&priv->buffered_multicasts_lock);
+			spin_unlock_bh(&priv->buffered_multicasts_lock);
 
 			if (ret)
 				break;