[PATCH v8 0/6] mt76: channel switch support for USB devices

[PATCH v8 0/6] mt76: channel switch support for USB devices

[Markus Theil]

This patch series adds channel switch support for mt76 usb interfaces.
When testing, I noticed that between 5 or 7 consecutive beacons had the
identical channel switch count set. After some debugging I found out,
that beacon copying over usb took far too long (up to 700ms for one call
of mt76x02u_pre_tbtt_work).

Therefore the first five patches speed up beacon copying and provide beaconing
fixes. The last patch enables channel switch support also for usb interfaces.

Thanks to Stanislaw and Lorenzo for their help.

v8:
* fix mbss beaconing
* fix adding vifs with idx 8
* fix memory leaks by dropping beacon buffer
* permanently enable 7 additional bss to save another usb call and make
  beacon masking easier
* added beacon_prepare call again, which now also clears beacon_data_mask

v7:
* fix mbss beacon settings (incorrect try)
* fix compilation with latest upstream

v6:
* use min_t in mt76u_copy
* use round_up in mt76u_copy
* use additional copy for mmio beacon set again

v5:
* ommit empty mt76x2u_channel_switch_beacon
* copy txwi into beacon skb

v4:
* use multiple of 4 len for usb copy again

v3:
* fixed checkpatch errors

v2:
* correctly track beacon data mask
* clean-ups
* make channel switch fn static (reported by kbuild test robot)

Markus Theil (6):
  mt76: mt76x02: ommit beacon slot clearing
  mt76: mt76x02: split beaconing
  mt76: mt76x02: add check for invalid vif idx
  mt76: mt76x02: remove a copy call for usb speedup
  mt76: speed up usb bulk copy
  mt76: mt76x02: add channel switch support for usb interfaces

 drivers/net/wireless/mediatek/mt76/mt76.h     |  2 +-
 drivers/net/wireless/mediatek/mt76/mt76x02.h  |  1 -
 .../wireless/mediatek/mt76/mt76x02_beacon.c   | 83 ++++++++-----------
 .../net/wireless/mediatek/mt76/mt76x02_mac.c  |  2 +
 .../net/wireless/mediatek/mt76/mt76x02_mac.h  |  5 +-
 .../net/wireless/mediatek/mt76/mt76x02_mmio.c |  4 +
 .../wireless/mediatek/mt76/mt76x02_usb_core.c | 21 +++--
 .../net/wireless/mediatek/mt76/mt76x02_util.c |  6 +-
 drivers/net/wireless/mediatek/mt76/usb.c      | 24 ++++--
 9 files changed, 79 insertions(+), 69 deletions(-)

-- 
2.24.0

[PATCH v8 1/6] mt76: mt76x02: ommit beacon slot clearing

[Markus Theil]

mt76 hw does not send beacons from beacon slots, if the corresponding
bitmask is set accordingly. Therefore we can ommit clearing the beacon
memory. Clearing uses many usb calls, if usb drivers are used. These
calls unnecessarily slow down the beacon tasklet. Thanks to Stanislaw
Gruzska for pointing this out.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 4209209ac940..403866496640 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -58,8 +58,6 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
 			dev->beacon_data_mask |= BIT(bcn_idx);
 	} else {
 		dev->beacon_data_mask &= ~BIT(bcn_idx);
-		for (i = 0; i < beacon_len; i += 4)
-			mt76_wr(dev, beacon_addr + i, 0);
 	}
 
 	mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
@@ -241,17 +239,11 @@ EXPORT_SYMBOL_GPL(mt76x02_enqueue_buffered_bc);
 
 void mt76x02_init_beacon_config(struct mt76x02_dev *dev)
 {
-	int i;
-
 	mt76_clear(dev, MT_BEACON_TIME_CFG, (MT_BEACON_TIME_CFG_TIMER_EN |
 					     MT_BEACON_TIME_CFG_TBTT_EN |
 					     MT_BEACON_TIME_CFG_BEACON_TX));
 	mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE);
 	mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff);
-
-	for (i = 0; i < 8; i++)
-		mt76x02_mac_set_beacon(dev, i, NULL);
-
 	mt76x02_set_beacon_offsets(dev);
 }
 EXPORT_SYMBOL_GPL(mt76x02_init_beacon_config);
-- 
2.24.0

[PATCH v8 2/6] mt76: mt76x02: split beaconing

[Markus Theil]

Sending beacons to the hardware always happens in batches. In order to
speed up beacon processing on usb devices, this patch splits out common
code an calls it only once (mt76x02_mac_set_beacon_prepare,
mt76x02_mac_set_beacon_finish).

Beacons are sequentially written into the beacon memory area, by
tracking its usage with the dev->beacon_data_mask. MBSS support
is fixed by reversing the beacon_data_mask when setting it inverted
as the bypass mask.

The code is also adapted for the mmio part of the driver, but should not
have any performance implication there.

MBSS tests were performed with AVM AC860 USB NIC with temporary support
for 5 BSS'. Different combinations of active vifs were created and
brought up. Afterwards connection and data transfer was tested for the
announced BSS'.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 drivers/net/wireless/mediatek/mt76/mt76x02.h  |  1 -
 .../wireless/mediatek/mt76/mt76x02_beacon.c   | 56 +++++++------------
 .../net/wireless/mediatek/mt76/mt76x02_mac.c  |  2 +
 .../net/wireless/mediatek/mt76/mt76x02_mac.h  |  5 +-
 .../net/wireless/mediatek/mt76/mt76x02_mmio.c |  4 ++
 .../wireless/mediatek/mt76/mt76x02_usb_core.c | 14 ++---
 6 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02.h b/drivers/net/wireless/mediatek/mt76/mt76x02.h
index 0ca0bbfe8769..944bdc9b249b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02.h
@@ -93,7 +93,6 @@ struct mt76x02_dev {
 
 	const struct mt76x02_beacon_ops *beacon_ops;
 
-	struct sk_buff *beacons[8];
 	u8 beacon_data_mask;
 
 	u8 tbtt_count;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index 403866496640..b01a6745ed47 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -6,6 +6,7 @@
  */
 
 #include "mt76x02.h"
+#include <linux/bitrev.h>
 
 static void mt76x02_set_beacon_offsets(struct mt76x02_dev *dev)
 {
@@ -47,54 +48,38 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
 	int beacon_len = dev->beacon_ops->slot_size;
 	int beacon_addr = MT_BEACON_BASE + (beacon_len * bcn_idx);
 	int ret = 0;
-	int i;
-
-	/* Prevent corrupt transmissions during update */
-	mt76_set(dev, MT_BCN_BYPASS_MASK, BIT(bcn_idx));
 
 	if (skb) {
 		ret = mt76x02_write_beacon(dev, beacon_addr, skb);
 		if (!ret)
 			dev->beacon_data_mask |= BIT(bcn_idx);
-	} else {
-		dev->beacon_data_mask &= ~BIT(bcn_idx);
 	}
 
-	mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
-
 	return ret;
 }
 
-int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
-			   struct sk_buff *skb)
+void mt76x02_mac_set_beacon_prepare(struct mt76x02_dev *dev)
 {
-	bool force_update = false;
-	int bcn_idx = 0;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(dev->beacons); i++) {
-		if (vif_idx == i) {
-			force_update = !!dev->beacons[i] ^ !!skb;
-			dev_kfree_skb(dev->beacons[i]);
-			dev->beacons[i] = skb;
-			__mt76x02_mac_set_beacon(dev, bcn_idx, skb);
-		} else if (force_update && dev->beacons[i]) {
-			__mt76x02_mac_set_beacon(dev, bcn_idx,
-						 dev->beacons[i]);
-		}
-
-		bcn_idx += !!dev->beacons[i];
-	}
+	/* Prevent corrupt transmissions during update */
+	mt76_set(dev, MT_BCN_BYPASS_MASK, 0xffff);
+	dev->beacon_data_mask = 0;
+}
+EXPORT_SYMBOL_GPL(mt76x02_mac_set_beacon_prepare);
 
-	for (i = bcn_idx; i < ARRAY_SIZE(dev->beacons); i++) {
-		if (!(dev->beacon_data_mask & BIT(i)))
-			break;
+void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
+{
+	mt76_wr(dev, MT_BCN_BYPASS_MASK,
+		0xff00 | ~bitrev8(dev->beacon_data_mask));
+}
+EXPORT_SYMBOL_GPL(mt76x02_mac_set_beacon_finish);
 
-		__mt76x02_mac_set_beacon(dev, i, NULL);
-	}
+int mt76x02_mac_set_beacon(struct mt76x02_dev *dev,
+			   struct sk_buff *skb)
+{
+	int bcn_idx = ffz(dev->beacon_data_mask);
 
-	mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
-		       bcn_idx - 1);
+	__mt76x02_mac_set_beacon(dev, bcn_idx, skb);
+	dev_kfree_skb(skb);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mt76x02_mac_set_beacon);
@@ -114,7 +99,6 @@ void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
 		dev->mt76.beacon_mask |= BIT(mvif->idx);
 	} else {
 		dev->mt76.beacon_mask &= ~BIT(mvif->idx);
-		mt76x02_mac_set_beacon(dev, mvif->idx, NULL);
 	}
 
 	if (!!old_mask == !!dev->mt76.beacon_mask)
@@ -180,7 +164,7 @@ mt76x02_update_beacon_iter(void *priv, u8 *mac, struct ieee80211_vif *vif)
 	if (!skb)
 		return;
 
-	mt76x02_mac_set_beacon(dev, mvif->idx, skb);
+	mt76x02_mac_set_beacon(dev, skb);
 }
 EXPORT_SYMBOL_GPL(mt76x02_update_beacon_iter);
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 4460548f346a..285ab0f491d0 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -741,6 +741,8 @@ void mt76x02_mac_setaddr(struct mt76x02_dev *dev, const u8 *addr)
 		get_unaligned_le16(dev->mt76.macaddr + 4) |
 		FIELD_PREP(MT_MAC_BSSID_DW1_MBSS_MODE, 3) | /* 8 APs + 8 STAs */
 		MT_MAC_BSSID_DW1_MBSS_LOCAL_BIT);
+	/* enable 7 additional beacon slots and control them with bypass mask */
+	mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N, 7);
 
 	for (i = 0; i < 16; i++)
 		mt76x02_mac_set_bssid(dev, i, null_addr);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index 7d946aa77182..f67f66f65ee0 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -201,10 +201,11 @@ void mt76x02_mac_work(struct work_struct *work);
 
 void mt76x02_mac_cc_reset(struct mt76x02_dev *dev);
 void mt76x02_mac_set_bssid(struct mt76x02_dev *dev, u8 idx, const u8 *addr);
-int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
-			   struct sk_buff *skb);
+int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, struct sk_buff *skb);
 void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
 				   struct ieee80211_vif *vif, bool enable);
+void mt76x02_mac_set_beacon_prepare(struct mt76x02_dev *dev);
+void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev);
 
 void mt76x02_edcca_init(struct mt76x02_dev *dev);
 #endif
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index 4e2371c926d8..ae35780aaff1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -24,10 +24,14 @@ static void mt76x02_pre_tbtt_tasklet(unsigned long arg)
 
 	mt76x02_resync_beacon_timer(dev);
 
+	mt76x02_mac_set_beacon_prepare(dev);
+
 	ieee80211_iterate_active_interfaces_atomic(mt76_hw(dev),
 		IEEE80211_IFACE_ITER_RESUME_ALL,
 		mt76x02_update_beacon_iter, dev);
 
+	mt76x02_mac_set_beacon_finish(dev);
+
 	mt76_csa_check(&dev->mt76);
 
 	if (dev->mt76.csa_complete)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index d03d3c8e296c..90c024f12302 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -208,6 +208,8 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
 
 	mt76x02_resync_beacon_timer(dev);
 
+	mt76x02_mac_set_beacon_prepare(dev);
+
 	ieee80211_iterate_active_interfaces(mt76_hw(dev),
 		IEEE80211_IFACE_ITER_RESUME_ALL,
 		mt76x02_update_beacon_iter, dev);
@@ -217,9 +219,11 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
 
 	for (i = nbeacons; i < N_BCN_SLOTS; i++) {
 		skb = __skb_dequeue(&data.q);
-		mt76x02_mac_set_beacon(dev, i, skb);
+		mt76x02_mac_set_beacon(dev, skb);
 	}
 
+	mt76x02_mac_set_beacon_finish(dev);
+
 	mt76x02u_restart_pre_tbtt_timer(dev);
 }
 
@@ -244,19 +248,11 @@ static void mt76x02u_pre_tbtt_enable(struct mt76x02_dev *dev, bool en)
 
 static void mt76x02u_beacon_enable(struct mt76x02_dev *dev, bool en)
 {
-	int i;
-
 	if (WARN_ON_ONCE(!dev->mt76.beacon_int))
 		return;
 
 	if (en) {
 		mt76x02u_start_pre_tbtt_timer(dev);
-	} else {
-		/* Timer is already stopped, only clean up
-		 * PS buffered frames if any.
-		 */
-		for (i = 0; i < N_BCN_SLOTS; i++)
-			mt76x02_mac_set_beacon(dev, i, NULL);
 	}
 }
 
-- 
2.24.0

[PATCH v8 3/6] mt76: mt76x02: add check for invalid vif idx

[Markus Theil]

On adding vifs the idx can become 1 + (7 & 7) = 8 for APs.
Check against that, as only AP vif idx 0-7 is possible.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index bdc83838d346..1142aa39a226 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -325,7 +325,9 @@ mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	if (vif->type == NL80211_IFTYPE_STATION)
 		idx += 8;
 
-	if (dev->vif_mask & BIT(idx))
+	/* vif is already set or idx is 8 for AP/Mesh/... */
+	if (dev->vif_mask & BIT(idx) ||
+	    vif->type != NL80211_IFTYPE_STATION && idx > 7)
 		return -EBUSY;
 
 	dev->vif_mask |= BIT(idx);
-- 
2.24.0

[PATCH v8 4/6] mt76: mt76x02: remove a copy call for usb speedup

[Markus Theil]

This patch removes a mt76_wr_copy call from the beacon path to hw.
The skb which is used in this place gets therefore build with txwi
inside its data. For mt76 usb drivers, this saves one synchronuous
copy call over usb, which lets the beacon work complete faster.

In mmio case, there is not enough headroom to put the txwi into the
skb, it is therefore using an additional mt76_wr_copy, which is fast
over mmio. Thanks Stanislaw for pointing this out.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 .../wireless/mediatek/mt76/mt76x02_beacon.c   | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index b01a6745ed47..043cb03c86a2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -27,15 +27,26 @@ static int
 mt76x02_write_beacon(struct mt76x02_dev *dev, int offset, struct sk_buff *skb)
 {
 	int beacon_len = dev->beacon_ops->slot_size;
-	struct mt76x02_txwi txwi;
 
 	if (WARN_ON_ONCE(beacon_len < skb->len + sizeof(struct mt76x02_txwi)))
 		return -ENOSPC;
 
-	mt76x02_mac_write_txwi(dev, &txwi, skb, NULL, NULL, skb->len);
+	/* USB devices already reserve enough skb headroom for txwi's. This
+	 * helps to save slow copies over USB.
+	 */
+	if (mt76_is_usb(&dev->mt76)) {
+		struct mt76x02_txwi *txwi;
+
+		mt76_insert_hdr_pad(skb);
+		txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi));
+		mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len);
+		skb_push(skb, sizeof(*txwi));
+	} else {
+		struct mt76x02_txwi txwi;
 
-	mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
-	offset += sizeof(txwi);
+		mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
+		offset += sizeof(txwi);
+	}
 
 	mt76_wr_copy(dev, offset, skb->data, skb->len);
 	return 0;
-- 
2.24.0

[PATCH v8 5/6] mt76: speed up usb bulk copy

[Markus Theil]

Use larger batches for usb copy to speed this operation up. Otherwise it
would be too slow for copying new beacons or broadcast frames over usb.
Assure, that always a multiple of 4 Bytes is copied, as outlined in
850e8f6fbd "mt76: round up length on mt76_wr_copy" from Felix Fietkau.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 +-
 drivers/net/wireless/mediatek/mt76/usb.c  | 24 +++++++++++++++++------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index fb077760347a..1981912de1f9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -382,7 +382,7 @@ enum mt76u_out_ep {
 struct mt76_usb {
 	struct mutex usb_ctrl_mtx;
 	union {
-		u8 data[32];
+		u8 data[128];
 		__le32 reg_val;
 	};
 
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index d6d47081e281..97b263ce3872 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -149,18 +149,30 @@ static void mt76u_copy(struct mt76_dev *dev, u32 offset,
 		       const void *data, int len)
 {
 	struct mt76_usb *usb = &dev->usb;
-	const u32 *val = data;
-	int i, ret;
+	const u8 *val = data;
+	int ret;
+	int current_batch_size;
+	int i = 0;
+
+	/* Assure that always a multiple of 4 bytes are copied,
+	 * otherwise beacons can be corrupted.
+	 * See: "mt76: round up length on mt76_wr_copy"
+	 * Commit 850e8f6fbd5d0003b0
+	 */
+	len = round_up(len, 4);
 
 	mutex_lock(&usb->usb_ctrl_mtx);
-	for (i = 0; i < DIV_ROUND_UP(len, 4); i++) {
-		put_unaligned(val[i], (u32 *)usb->data);
+	while (i < len) {
+		current_batch_size = min_t(int, sizeof(usb->data), len - i);
+		memcpy(usb->data, val + i, current_batch_size);
 		ret = __mt76u_vendor_request(dev, MT_VEND_MULTI_WRITE,
 					     USB_DIR_OUT | USB_TYPE_VENDOR,
-					     0, offset + i * 4, usb->data,
-					     sizeof(u32));
+					     0, offset + i, usb->data,
+					     current_batch_size);
 		if (ret < 0)
 			break;
+
+		i += current_batch_size;
 	}
 	mutex_unlock(&usb->usb_ctrl_mtx);
 }
-- 
2.24.0

[PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Markus Theil]

This patch enables channel switch support on mt76 usb interfaces.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c | 7 +++++++
 drivers/net/wireless/mediatek/mt76/mt76x02_util.c     | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index 90c024f12302..b9bd6bfb2a9d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
 
 	mt76x02_mac_set_beacon_prepare(dev);
 
+	mt76_csa_check(&dev->mt76);
+	if (dev->mt76.csa_complete) {
+		mt76_csa_finish(&dev->mt76);
+		goto out;
+	}
+
 	ieee80211_iterate_active_interfaces(mt76_hw(dev),
 		IEEE80211_IFACE_ITER_RESUME_ALL,
 		mt76x02_update_beacon_iter, dev);
@@ -224,6 +230,7 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
 
 	mt76x02_mac_set_beacon_finish(dev);
 
+out:
 	mt76x02u_restart_pre_tbtt_timer(dev);
 }
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 1142aa39a226..85df9bfadd4c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -166,7 +166,6 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
 		wiphy->reg_notifier = mt76x02_regd_notifier;
 		wiphy->iface_combinations = mt76x02_if_comb;
 		wiphy->n_iface_combinations = ARRAY_SIZE(mt76x02_if_comb);
-		wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;
 
 		/* init led callbacks */
 		if (IS_ENABLED(CONFIG_MT76_LEDS)) {
@@ -176,6 +175,7 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
 		}
 	}
 
+	wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;
 	wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_VHT_IBSS);
 
 	hw->sta_data_size = sizeof(struct mt76x02_sta);
-- 
2.24.0

Re: [PATCH v8 3/6] mt76: mt76x02: add check for invalid vif idx

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3492 bytes --]

Hi Markus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on next-20191122]
[cannot apply to v5.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Markus-Theil/mt76-channel-switch-support-for-USB-devices/20191124-083548
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net//wireless/mediatek/mt76/mt76x02_util.c: In function 'mt76x02_add_interface':
>> drivers/net//wireless/mediatek/mt76/mt76x02_util.c:330:42: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
         vif->type != NL80211_IFTYPE_STATION && idx > 7)
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~

vim +330 drivers/net//wireless/mediatek/mt76/mt76x02_util.c

   296	
   297	int
   298	mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
   299	{
   300		struct mt76x02_dev *dev = hw->priv;
   301		unsigned int idx = 0;
   302	
   303		/* Allow to change address in HW if we create first interface. */
   304		if (!dev->vif_mask &&
   305		    (((vif->addr[0] ^ dev->mt76.macaddr[0]) & ~GENMASK(4, 1)) ||
   306		     memcmp(vif->addr + 1, dev->mt76.macaddr + 1, ETH_ALEN - 1)))
   307			mt76x02_mac_setaddr(dev, vif->addr);
   308	
   309		if (vif->addr[0] & BIT(1))
   310			idx = 1 + (((dev->mt76.macaddr[0] ^ vif->addr[0]) >> 2) & 7);
   311	
   312		/*
   313		 * Client mode typically only has one configurable BSSID register,
   314		 * which is used for bssidx=0. This is linked to the MAC address.
   315		 * Since mac80211 allows changing interface types, and we cannot
   316		 * force the use of the primary MAC address for a station mode
   317		 * interface, we need some other way of configuring a per-interface
   318		 * remote BSSID.
   319		 * The hardware provides an AP-Client feature, where bssidx 0-7 are
   320		 * used for AP mode and bssidx 8-15 for client mode.
   321		 * We shift the station interface bss index by 8 to force the
   322		 * hardware to recognize the BSSID.
   323		 * The resulting bssidx mismatch for unicast frames is ignored by hw.
   324		 */
   325		if (vif->type == NL80211_IFTYPE_STATION)
   326			idx += 8;
   327	
   328		/* vif is already set or idx is 8 for AP/Mesh/... */
   329		if (dev->vif_mask & BIT(idx) ||
 > 330		    vif->type != NL80211_IFTYPE_STATION && idx > 7)
   331			return -EBUSY;
   332	
   333		dev->vif_mask |= BIT(idx);
   334	
   335		mt76x02_vif_init(dev, vif, idx);
   336		return 0;
   337	}
   338	EXPORT_SYMBOL_GPL(mt76x02_add_interface);
   339	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59165 bytes --]

Re: [PATCH v8 4/6] mt76: mt76x02: remove a copy call for usb speedup

[Stanislaw Gruszka]

On Thu, Nov 21, 2019 at 06:59:59PM +0100, Markus Theil wrote:
> +	if (mt76_is_usb(&dev->mt76)) {
> +		struct mt76x02_txwi *txwi;
> +
> +		mt76_insert_hdr_pad(skb);
> +		txwi = (struct mt76x02_txwi *)(skb->data - sizeof(*txwi));
> +		mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len);
> +		skb_push(skb, sizeof(*txwi));
> +	} else {
> +		struct mt76x02_txwi txwi;

Here lack mt76x02_mac_write_txwi()

>  
> -	mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> -	offset += sizeof(txwi);
> +		mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> +		offset += sizeof(txwi);

> +	}

Stanislaw

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Stanislaw Gruszka]

On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> +{
> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
> +		0xff00 | ~bitrev8(dev->beacon_data_mask));

Since you arrange beacon slots continues starting from 0
(i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
I think it would make sense to keep
MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.

But no strong opinion here, code with bitrev8 looks fine too.

>  static void mt76x02u_beacon_enable(struct mt76x02_dev *dev, bool en)
>  {
> -	int i;
> -
>  	if (WARN_ON_ONCE(!dev->mt76.beacon_int))
>  		return;
>  
>  	if (en) {
>  		mt76x02u_start_pre_tbtt_timer(dev);
> -	} else {
> -		/* Timer is already stopped, only clean up
> -		 * PS buffered frames if any.
> -		 */

Please keep comment that timer is already disabled and
nothing else is needed.

Stanislaw

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Stanislaw Gruszka]

On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> index 90c024f12302..b9bd6bfb2a9d 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
>  
>  	mt76x02_mac_set_beacon_prepare(dev);
>  
> +	mt76_csa_check(&dev->mt76);
> +	if (dev->mt76.csa_complete) {
> +		mt76_csa_finish(&dev->mt76);
> +		goto out;
> +	}

mmio counterpart setup beacons on CSA, but do not sent buffered
PS frames. Perhaps here we should do the same. However not sending
beacons on one TBTT looks ok to me as well.

Stanislaw

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Markus Theil]

On 11/25/19 2:00 PM, Stanislaw Gruszka wrote:
> On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
>> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
>> +{
>> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
>> +		0xff00 | ~bitrev8(dev->beacon_data_mask));
> Since you arrange beacon slots continues starting from 0
> (i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
> I think it would make sense to keep
> MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.
>
> But no strong opinion here, code with bitrev8 looks fine too.
I'd like to keep the bitrev8 code, as it saves a copy over usb for usb
devices, if MT_MAC_BSSID_DW_BEACON_N is kept constant.
bitrev8 should be a rather cheap operation compared to a copy over some
form of bus.
>>  static void mt76x02u_beacon_enable(struct mt76x02_dev *dev, bool en)
>>  {
>> -	int i;
>> -
>>  	if (WARN_ON_ONCE(!dev->mt76.beacon_int))
>>  		return;
>>  
>>  	if (en) {
>>  		mt76x02u_start_pre_tbtt_timer(dev);
>> -	} else {
>> -		/* Timer is already stopped, only clean up
>> -		 * PS buffered frames if any.
>> -		 */
> Please keep comment that timer is already disabled and
> nothing else is needed.
>
> Stanislaw
>
Ok, will keep it in an updated version.

Markus

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Markus Theil]

On 11/25/19 2:04 PM, Stanislaw Gruszka wrote:
> On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>> index 90c024f12302..b9bd6bfb2a9d 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
>>  
>>  	mt76x02_mac_set_beacon_prepare(dev);
>>  
>> +	mt76_csa_check(&dev->mt76);
>> +	if (dev->mt76.csa_complete) {
>> +		mt76_csa_finish(&dev->mt76);
>> +		goto out;
>> +	}
> mmio counterpart setup beacons on CSA, but do not sent buffered
> PS frames. Perhaps here we should do the same. However not sending
> beacons on one TBTT looks ok to me as well.
>
> Stanislaw
>
If I change the order of beacon iteration and csa check, the following warning in mac80211 gets triggered:

	/* the counter should never reach 0 */
	WARN_ON_ONCE(!beacon->csa_current_counter);

Dmesg output looks like this:

[  153.829617] ------------[ cut here ]------------
[  153.829752] WARNING: CPU: 0 PID: 224 at net/mac80211/tx.c:4318 __ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
[  153.829756] Modules linked in: ccm bridge stp llc nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci amd64_edac_mod mt76x2u edac_mce_amd mt76x2_common ath10k_core mt76x02_usb kvm_amd mt76_usb mt76x02_lib mt76 kvm ath mac80211 irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel cfg80211 pcengines_apuv2 gpio_keys_polled aesni_intel input_polldev crypto_simd gpio_amd_fch cryptd glue_helper igb pcspkr k10temp fam15h_power rfkill sp5100_tco i2c_piix4 libarc4 ccp i2c_algo_bit dca rng_core uio_pdrv_genirq uio leds_gpio evdev mac_hid pinctrl_amd coreboot_table acpi_cpufreq ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod ahci sdhci_pci libahci cqhci libata sdhci crc32c_intel xhci_pci ehci_pci mmc_core xhci_hcd scsi_mod ehci_hcd gpio_keys
[  153.829948] CPU: 0 PID: 224 Comm: kworker/0:1H Not tainted 5.4.0-rc7-1-01110-g19b7e21c55c8 #32
[  153.829952] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
[  153.829966] Workqueue: events_highpri mt76x02u_pre_tbtt_work [mt76x02_usb]
[  153.830067] RIP: 0010:__ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
[  153.830077] Code: 4c 89 4a 18 c3 48 8b 47 10 4c 89 4f 10 48 89 4e 18 48 89 46 20 4c 89 08 eb a1 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <0f> 0b c3 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 54 53 48 89 fb
[  153.830082] RSP: 0018:ffffbde8803d7c80 EFLAGS: 00010246
[  153.830089] RAX: 0000000000000003 RBX: ffffbde8803d7d20 RCX: ffffa3b01df8f658
[  153.830093] RDX: ffffbde8803d7d20 RSI: ffffa3b02901d450 RDI: ffffbde8803d7ce0
[  153.830097] RBP: ffffa3b0267007a0 R08: ffffffff8d011040 R09: 0000000000000000
[  153.830101] R10: ffffa3b029908098 R11: ffffa3b02ab2ab38 R12: 0000000000000000
[  153.830105] R13: ffffa3b02901d450 R14: 0000000000000000 R15: ffffa3b0284eae00
[  153.830111] FS:  0000000000000000(0000) GS:ffffa3b02aa00000(0000) knlGS:0000000000000000
[  153.830115] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  153.830119] CR2: 00007fbb6b5c9f30 CR3: 0000000126ee2000 CR4: 00000000000406f0
[  153.830124] Call Trace:
[  153.830203]  __ieee80211_beacon_get+0x4c2/0x4d0 [mac80211]
[  153.830312]  ieee80211_beacon_get_tim+0x41/0x150 [mac80211]
[  153.830336]  mt76x02_update_beacon_iter+0x2d/0x40 [mt76x02_lib]
[  153.830352]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
[  153.830420]  __iterate_interfaces+0x74/0x110 [mac80211]
[  153.830469]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
[  153.830566]  ieee80211_iterate_interfaces+0x3a/0x50 [mac80211]
[  153.830580]  mt76x02u_pre_tbtt_work+0x96/0x220 [mt76x02_usb]
[  153.830600]  process_one_work+0x1e2/0x3b0
[  153.830610]  worker_thread+0x4a/0x3d0
[  153.830623]  kthread+0xfb/0x130
[  153.830631]  ? process_one_work+0x3b0/0x3b0
[  153.830639]  ? kthread_park+0x90/0x90
[  153.830650]  ret_from_fork+0x22/0x40
[  153.830665] ---[ end trace 7a658e5cbfd0f9d1 ]---

Markus

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Markus Theil]

On 11/25/19 3:51 PM, Markus Theil wrote:
> On 11/25/19 2:04 PM, Stanislaw Gruszka wrote:
>> On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>> index 90c024f12302..b9bd6bfb2a9d 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
>>>  
>>>  	mt76x02_mac_set_beacon_prepare(dev);
>>>  
>>> +	mt76_csa_check(&dev->mt76);
>>> +	if (dev->mt76.csa_complete) {
>>> +		mt76_csa_finish(&dev->mt76);
>>> +		goto out;
>>> +	}
>> mmio counterpart setup beacons on CSA, but do not sent buffered
>> PS frames. Perhaps here we should do the same. However not sending
>> beacons on one TBTT looks ok to me as well.
>>
>> Stanislaw
>>
> If I change the order of beacon iteration and csa check, the following warning in mac80211 gets triggered:
>
> 	/* the counter should never reach 0 */
> 	WARN_ON_ONCE(!beacon->csa_current_counter);
>
> Dmesg output looks like this:
>
> [  153.829617] ------------[ cut here ]------------
> [  153.829752] WARNING: CPU: 0 PID: 224 at net/mac80211/tx.c:4318 __ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
> [  153.829756] Modules linked in: ccm bridge stp llc nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci amd64_edac_mod mt76x2u edac_mce_amd mt76x2_common ath10k_core mt76x02_usb kvm_amd mt76_usb mt76x02_lib mt76 kvm ath mac80211 irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel cfg80211 pcengines_apuv2 gpio_keys_polled aesni_intel input_polldev crypto_simd gpio_amd_fch cryptd glue_helper igb pcspkr k10temp fam15h_power rfkill sp5100_tco i2c_piix4 libarc4 ccp i2c_algo_bit dca rng_core uio_pdrv_genirq uio leds_gpio evdev mac_hid pinctrl_amd coreboot_table acpi_cpufreq ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod ahci sdhci_pci libahci cqhci libata sdhci crc32c_intel xhci_pci ehci_pci mmc_core xhci_hcd scsi_mod ehci_hcd gpio_keys
> [  153.829948] CPU: 0 PID: 224 Comm: kworker/0:1H Not tainted 5.4.0-rc7-1-01110-g19b7e21c55c8 #32
> [  153.829952] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
> [  153.829966] Workqueue: events_highpri mt76x02u_pre_tbtt_work [mt76x02_usb]
> [  153.830067] RIP: 0010:__ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
> [  153.830077] Code: 4c 89 4a 18 c3 48 8b 47 10 4c 89 4f 10 48 89 4e 18 48 89 46 20 4c 89 08 eb a1 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <0f> 0b c3 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 54 53 48 89 fb
> [  153.830082] RSP: 0018:ffffbde8803d7c80 EFLAGS: 00010246
> [  153.830089] RAX: 0000000000000003 RBX: ffffbde8803d7d20 RCX: ffffa3b01df8f658
> [  153.830093] RDX: ffffbde8803d7d20 RSI: ffffa3b02901d450 RDI: ffffbde8803d7ce0
> [  153.830097] RBP: ffffa3b0267007a0 R08: ffffffff8d011040 R09: 0000000000000000
> [  153.830101] R10: ffffa3b029908098 R11: ffffa3b02ab2ab38 R12: 0000000000000000
> [  153.830105] R13: ffffa3b02901d450 R14: 0000000000000000 R15: ffffa3b0284eae00
> [  153.830111] FS:  0000000000000000(0000) GS:ffffa3b02aa00000(0000) knlGS:0000000000000000
> [  153.830115] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  153.830119] CR2: 00007fbb6b5c9f30 CR3: 0000000126ee2000 CR4: 00000000000406f0
> [  153.830124] Call Trace:
> [  153.830203]  __ieee80211_beacon_get+0x4c2/0x4d0 [mac80211]
> [  153.830312]  ieee80211_beacon_get_tim+0x41/0x150 [mac80211]
> [  153.830336]  mt76x02_update_beacon_iter+0x2d/0x40 [mt76x02_lib]
> [  153.830352]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
> [  153.830420]  __iterate_interfaces+0x74/0x110 [mac80211]
> [  153.830469]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
> [  153.830566]  ieee80211_iterate_interfaces+0x3a/0x50 [mac80211]
> [  153.830580]  mt76x02u_pre_tbtt_work+0x96/0x220 [mt76x02_usb]
> [  153.830600]  process_one_work+0x1e2/0x3b0
> [  153.830610]  worker_thread+0x4a/0x3d0
> [  153.830623]  kthread+0xfb/0x130
> [  153.830631]  ? process_one_work+0x3b0/0x3b0
> [  153.830639]  ? kthread_park+0x90/0x90
> [  153.830650]  ret_from_fork+0x22/0x40
> [  153.830665] ---[ end trace 7a658e5cbfd0f9d1 ]---
>
> Markus
>
In my current local changes I've decoupled checking csa_check and mt76_csa_finish, like it is in the mmio case. As usb has no tbtt interrupt,
I just schedule a delayed work around the estimated beacon transmission time and finish csa there. I'll send another series, if this works.

Markus

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Stanislaw Gruszka]

On Mon, Nov 25, 2019 at 03:07:59PM +0100, Markus Theil wrote:
> On 11/25/19 2:00 PM, Stanislaw Gruszka wrote:
> > On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
> >> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> >> +{
> >> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
> >> +		0xff00 | ~bitrev8(dev->beacon_data_mask));
> > Since you arrange beacon slots continues starting from 0
> > (i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
> > I think it would make sense to keep
> > MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.
> >
> > But no strong opinion here, code with bitrev8 looks fine too.
> I'd like to keep the bitrev8 code, as it saves a copy over usb for usb
> devices, if MT_MAC_BSSID_DW_BEACON_N is kept constant.
> bitrev8 should be a rather cheap operation compared to a copy over some
> form of bus.

This make sense. I tested the code on MT7630E and after adding missed
write_txwi function, it works fine. So I think bitrev8 code is ok.

Stanislaw

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Stanislaw Gruszka]

On Mon, Nov 25, 2019 at 04:32:42PM +0100, Markus Theil wrote:
> 
> On 11/25/19 3:51 PM, Markus Theil wrote:
> > On 11/25/19 2:04 PM, Stanislaw Gruszka wrote:
> >> On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
> >>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> >>> index 90c024f12302..b9bd6bfb2a9d 100644
> >>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> >>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> >>> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
> >>>  
> >>>  	mt76x02_mac_set_beacon_prepare(dev);
> >>>  
> >>> +	mt76_csa_check(&dev->mt76);
> >>> +	if (dev->mt76.csa_complete) {
> >>> +		mt76_csa_finish(&dev->mt76);
> >>> +		goto out;
> >>> +	}
> >> mmio counterpart setup beacons on CSA, but do not sent buffered
> >> PS frames. Perhaps here we should do the same. However not sending
> >> beacons on one TBTT looks ok to me as well.
> >>
> >> Stanislaw
> >>
> > If I change the order of beacon iteration and csa check, the following warning in mac80211 gets triggered:
> >
> > 	/* the counter should never reach 0 */
> > 	WARN_ON_ONCE(!beacon->csa_current_counter);
> >
> > Dmesg output looks like this:
> >
> > [  153.829617] ------------[ cut here ]------------
> > [  153.829752] WARNING: CPU: 0 PID: 224 at net/mac80211/tx.c:4318 __ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
> > [  153.829756] Modules linked in: ccm bridge stp llc nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci amd64_edac_mod mt76x2u edac_mce_amd mt76x2_common ath10k_core mt76x02_usb kvm_amd mt76_usb mt76x02_lib mt76 kvm ath mac80211 irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel cfg80211 pcengines_apuv2 gpio_keys_polled aesni_intel input_polldev crypto_simd gpio_amd_fch cryptd glue_helper igb pcspkr k10temp fam15h_power rfkill sp5100_tco i2c_piix4 libarc4 ccp i2c_algo_bit dca rng_core uio_pdrv_genirq uio leds_gpio evdev mac_hid pinctrl_amd coreboot_table acpi_cpufreq ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod ahci sdhci_pci libahci cqhci libata sdhci crc32c_intel xhci_pci ehci_pci mmc_core xhci_hcd scsi_mod ehci_hcd gpio_keys
> > [  153.829948] CPU: 0 PID: 224 Comm: kworker/0:1H Not tainted 5.4.0-rc7-1-01110-g19b7e21c55c8 #32
> > [  153.829952] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
> > [  153.829966] Workqueue: events_highpri mt76x02u_pre_tbtt_work [mt76x02_usb]
> > [  153.830067] RIP: 0010:__ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
> > [  153.830077] Code: 4c 89 4a 18 c3 48 8b 47 10 4c 89 4f 10 48 89 4e 18 48 89 46 20 4c 89 08 eb a1 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <0f> 0b c3 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 54 53 48 89 fb
> > [  153.830082] RSP: 0018:ffffbde8803d7c80 EFLAGS: 00010246
> > [  153.830089] RAX: 0000000000000003 RBX: ffffbde8803d7d20 RCX: ffffa3b01df8f658
> > [  153.830093] RDX: ffffbde8803d7d20 RSI: ffffa3b02901d450 RDI: ffffbde8803d7ce0
> > [  153.830097] RBP: ffffa3b0267007a0 R08: ffffffff8d011040 R09: 0000000000000000
> > [  153.830101] R10: ffffa3b029908098 R11: ffffa3b02ab2ab38 R12: 0000000000000000
> > [  153.830105] R13: ffffa3b02901d450 R14: 0000000000000000 R15: ffffa3b0284eae00
> > [  153.830111] FS:  0000000000000000(0000) GS:ffffa3b02aa00000(0000) knlGS:0000000000000000
> > [  153.830115] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  153.830119] CR2: 00007fbb6b5c9f30 CR3: 0000000126ee2000 CR4: 00000000000406f0
> > [  153.830124] Call Trace:
> > [  153.830203]  __ieee80211_beacon_get+0x4c2/0x4d0 [mac80211]
> > [  153.830312]  ieee80211_beacon_get_tim+0x41/0x150 [mac80211]
> > [  153.830336]  mt76x02_update_beacon_iter+0x2d/0x40 [mt76x02_lib]
> > [  153.830352]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
> > [  153.830420]  __iterate_interfaces+0x74/0x110 [mac80211]
> > [  153.830469]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
> > [  153.830566]  ieee80211_iterate_interfaces+0x3a/0x50 [mac80211]
> > [  153.830580]  mt76x02u_pre_tbtt_work+0x96/0x220 [mt76x02_usb]
> > [  153.830600]  process_one_work+0x1e2/0x3b0
> > [  153.830610]  worker_thread+0x4a/0x3d0
> > [  153.830623]  kthread+0xfb/0x130
> > [  153.830631]  ? process_one_work+0x3b0/0x3b0
> > [  153.830639]  ? kthread_park+0x90/0x90
> > [  153.830650]  ret_from_fork+0x22/0x40
> > [  153.830665] ---[ end trace 7a658e5cbfd0f9d1 ]---
> >
> > Markus
> >
> In my current local changes I've decoupled checking csa_check and mt76_csa_finish, like it is in the mmio case. As usb has no tbtt interrupt,
> I just schedule a delayed work around the estimated beacon transmission time and finish csa there. I'll send another series, if this works.

I would prefer not to add yet another delayed work. Does the warning
still happen if you arrange code like this?

        mt76x02_mac_set_beacon_prepare(dev);

        ieee80211_iterate_active_interfaces(mt76_hw(dev),
                IEEE80211_IFACE_ITER_RESUME_ALL,
                mt76x02_update_beacon_iter, dev);

        mt76_csa_check(&dev->mt76);
        if (dev->mt76.csa_complete) {
                mt76_csa_finish(&dev->mt76);
                goto out;
        }

        nbeacons = hweight8(dev->mt76.beacon_mask);
        mt76x02_enqueue_buffered_bc(dev, &data, N_BCN_SLOTS - nbeacons);
	...
out:
        mt76x02_mac_set_beacon_finish(dev);
        mt76x02u_restart_pre_tbtt_timer(dev);

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Felix Fietkau]

On 2019-11-25 17:59, Stanislaw Gruszka wrote:
> On Mon, Nov 25, 2019 at 03:07:59PM +0100, Markus Theil wrote:
>> On 11/25/19 2:00 PM, Stanislaw Gruszka wrote:
>> > On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
>> >> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
>> >> +{
>> >> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
>> >> +		0xff00 | ~bitrev8(dev->beacon_data_mask));
>> > Since you arrange beacon slots continues starting from 0
>> > (i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
>> > I think it would make sense to keep
>> > MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.
>> >
>> > But no strong opinion here, code with bitrev8 looks fine too.
>> I'd like to keep the bitrev8 code, as it saves a copy over usb for usb
>> devices, if MT_MAC_BSSID_DW_BEACON_N is kept constant.
>> bitrev8 should be a rather cheap operation compared to a copy over some
>> form of bus.
> 
> This make sense. I tested the code on MT7630E and after adding missed
> write_txwi function, it works fine. So I think bitrev8 code is ok.
I find the use of bitrev8/ffz a bit convoluted. If I understand the code
right, wouldn't it be equivalent to keeping beacon_data_count instead of
beacon_data_mask and doing:
mt76_wr(dev, MT_BCN_BYPASS_MASK,
	0xffff & ~((1 << dev->beacon_data_count) - 1));
If so, I would strongly prefer the beacon_data_count variant. It's also
cheaper to calculate, though that probably doesn't matter, since it's
not really a hot path.

- Felix

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Stanislaw Gruszka]

On Mon, Nov 25, 2019 at 06:12:56PM +0100, Felix Fietkau wrote:
> On 2019-11-25 17:59, Stanislaw Gruszka wrote:
> > On Mon, Nov 25, 2019 at 03:07:59PM +0100, Markus Theil wrote:
> >> On 11/25/19 2:00 PM, Stanislaw Gruszka wrote:
> >> > On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
> >> >> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> >> >> +{
> >> >> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
> >> >> +		0xff00 | ~bitrev8(dev->beacon_data_mask));
> >> > Since you arrange beacon slots continues starting from 0
> >> > (i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
> >> > I think it would make sense to keep
> >> > MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.
> >> >
> >> > But no strong opinion here, code with bitrev8 looks fine too.
> >> I'd like to keep the bitrev8 code, as it saves a copy over usb for usb
> >> devices, if MT_MAC_BSSID_DW_BEACON_N is kept constant.
> >> bitrev8 should be a rather cheap operation compared to a copy over some
> >> form of bus.
> > 
> > This make sense. I tested the code on MT7630E and after adding missed
> > write_txwi function, it works fine. So I think bitrev8 code is ok.
> I find the use of bitrev8/ffz a bit convoluted. If I understand the code
> right, wouldn't it be equivalent to keeping beacon_data_count instead of
> beacon_data_mask and doing:
> mt76_wr(dev, MT_BCN_BYPASS_MASK,
> 	0xffff & ~((1 << dev->beacon_data_count) - 1));
> If so, I would strongly prefer the beacon_data_count variant. It's also
> cheaper to calculate, though that probably doesn't matter, since it's
> not really a hot path.

Yes, with new code simple count can be used instead of mask. I did not
pointed this, as it can be further optimization on top of Markus
patches.

Stanislaw 

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Markus Theil]

On 11/25/19 6:02 PM, Stanislaw Gruszka wrote:
> On Mon, Nov 25, 2019 at 04:32:42PM +0100, Markus Theil wrote:
>> On 11/25/19 3:51 PM, Markus Theil wrote:
>>> On 11/25/19 2:04 PM, Stanislaw Gruszka wrote:
>>>> On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
>>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>> index 90c024f12302..b9bd6bfb2a9d 100644
>>>>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
>>>>>  
>>>>>  	mt76x02_mac_set_beacon_prepare(dev);
>>>>>  
>>>>> +	mt76_csa_check(&dev->mt76);
>>>>> +	if (dev->mt76.csa_complete) {
>>>>> +		mt76_csa_finish(&dev->mt76);
>>>>> +		goto out;
>>>>> +	}
>>>> mmio counterpart setup beacons on CSA, but do not sent buffered
>>>> PS frames. Perhaps here we should do the same. However not sending
>>>> beacons on one TBTT looks ok to me as well.
>>>>
>>>> Stanislaw
>>>>
>>> If I change the order of beacon iteration and csa check, the following warning in mac80211 gets triggered:
>>>
>>> 	/* the counter should never reach 0 */
>>> 	WARN_ON_ONCE(!beacon->csa_current_counter);
>>>
>>> Dmesg output looks like this:
>>>
>>> [  153.829617] ------------[ cut here ]------------
>>> [  153.829752] WARNING: CPU: 0 PID: 224 at net/mac80211/tx.c:4318 __ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
>>> [  153.829756] Modules linked in: ccm bridge stp llc nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci amd64_edac_mod mt76x2u edac_mce_amd mt76x2_common ath10k_core mt76x02_usb kvm_amd mt76_usb mt76x02_lib mt76 kvm ath mac80211 irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel cfg80211 pcengines_apuv2 gpio_keys_polled aesni_intel input_polldev crypto_simd gpio_amd_fch cryptd glue_helper igb pcspkr k10temp fam15h_power rfkill sp5100_tco i2c_piix4 libarc4 ccp i2c_algo_bit dca rng_core uio_pdrv_genirq uio leds_gpio evdev mac_hid pinctrl_amd coreboot_table acpi_cpufreq ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod ahci sdhci_pci libahci cqhci libata sdhci crc32c_intel xhci_pci ehci_pci mmc_core xhci_hcd scsi_mod ehci_hcd gpio_keys
>>> [  153.829948] CPU: 0 PID: 224 Comm: kworker/0:1H Not tainted 5.4.0-rc7-1-01110-g19b7e21c55c8 #32
>>> [  153.829952] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
>>> [  153.829966] Workqueue: events_highpri mt76x02u_pre_tbtt_work [mt76x02_usb]
>>> [  153.830067] RIP: 0010:__ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
>>> [  153.830077] Code: 4c 89 4a 18 c3 48 8b 47 10 4c 89 4f 10 48 89 4e 18 48 89 46 20 4c 89 08 eb a1 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <0f> 0b c3 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 54 53 48 89 fb
>>> [  153.830082] RSP: 0018:ffffbde8803d7c80 EFLAGS: 00010246
>>> [  153.830089] RAX: 0000000000000003 RBX: ffffbde8803d7d20 RCX: ffffa3b01df8f658
>>> [  153.830093] RDX: ffffbde8803d7d20 RSI: ffffa3b02901d450 RDI: ffffbde8803d7ce0
>>> [  153.830097] RBP: ffffa3b0267007a0 R08: ffffffff8d011040 R09: 0000000000000000
>>> [  153.830101] R10: ffffa3b029908098 R11: ffffa3b02ab2ab38 R12: 0000000000000000
>>> [  153.830105] R13: ffffa3b02901d450 R14: 0000000000000000 R15: ffffa3b0284eae00
>>> [  153.830111] FS:  0000000000000000(0000) GS:ffffa3b02aa00000(0000) knlGS:0000000000000000
>>> [  153.830115] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  153.830119] CR2: 00007fbb6b5c9f30 CR3: 0000000126ee2000 CR4: 00000000000406f0
>>> [  153.830124] Call Trace:
>>> [  153.830203]  __ieee80211_beacon_get+0x4c2/0x4d0 [mac80211]
>>> [  153.830312]  ieee80211_beacon_get_tim+0x41/0x150 [mac80211]
>>> [  153.830336]  mt76x02_update_beacon_iter+0x2d/0x40 [mt76x02_lib]
>>> [  153.830352]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
>>> [  153.830420]  __iterate_interfaces+0x74/0x110 [mac80211]
>>> [  153.830469]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
>>> [  153.830566]  ieee80211_iterate_interfaces+0x3a/0x50 [mac80211]
>>> [  153.830580]  mt76x02u_pre_tbtt_work+0x96/0x220 [mt76x02_usb]
>>> [  153.830600]  process_one_work+0x1e2/0x3b0
>>> [  153.830610]  worker_thread+0x4a/0x3d0
>>> [  153.830623]  kthread+0xfb/0x130
>>> [  153.830631]  ? process_one_work+0x3b0/0x3b0
>>> [  153.830639]  ? kthread_park+0x90/0x90
>>> [  153.830650]  ret_from_fork+0x22/0x40
>>> [  153.830665] ---[ end trace 7a658e5cbfd0f9d1 ]---
>>>
>>> Markus
>>>
>> In my current local changes I've decoupled checking csa_check and mt76_csa_finish, like it is in the mmio case. As usb has no tbtt interrupt,
>> I just schedule a delayed work around the estimated beacon transmission time and finish csa there. I'll send another series, if this works.
> I would prefer not to add yet another delayed work. Does the warning
> still happen if you arrange code like this?
>
>         mt76x02_mac_set_beacon_prepare(dev);
>
>         ieee80211_iterate_active_interfaces(mt76_hw(dev),
>                 IEEE80211_IFACE_ITER_RESUME_ALL,
>                 mt76x02_update_beacon_iter, dev);
>
>         mt76_csa_check(&dev->mt76);
>         if (dev->mt76.csa_complete) {
>                 mt76_csa_finish(&dev->mt76);
>                 goto out;
>         }
>
>         nbeacons = hweight8(dev->mt76.beacon_mask);
>         mt76x02_enqueue_buffered_bc(dev, &data, N_BCN_SLOTS - nbeacons);
> 	...
> out:
>         mt76x02_mac_set_beacon_finish(dev);
>         mt76x02u_restart_pre_tbtt_timer(dev);
>
>
You're code works, if I add some locking. ieee80211_iterate_active_interfaces -> ieee80211_iterate_active_interfaces_atomic did the trick for me. Otherwise I get the dmesg output from above.
After using ieee80211_iterate_active_interfaces_atomic I got the following dmesg output after/while a channel switch:

[   63.115806] divide error: 0000 [#1] PREEMPT SMP NOPTI
[   63.121054] CPU: 0 PID: 225 Comm: kworker/u8:2 Tainted: G        W         5.4.0-rc7-1-01110-g19b7e21c55c8 #39
[   63.131331] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
[   63.138399] Workqueue: mt76u mt76u_tx_status_data [mt76_usb]
[   63.144258] RIP: 0010:mt76_calc_rx_airtime+0x12b/0x150 [mt76]
[   63.150281] Code: 8d 34 76 48 8d 34 b1 0f b6 4e 07 66 85 c9 74 25 66 83 f9 01 75 1c b9 24 00 00 00 89 d0 0f b7 76 04 c1 e0 05 8d 04 d0 01 c0 99 <f7> fe 01 c8 c3 31 c0 c3 0f 0b c3 44 89 c8 83 e0 01 3c 01 19 c9 83
[   63.169699] RSP: 0018:ffffbd694032fcf0 EFLAGS: 00010216
[   63.175176] RAX: 0000000000002850 RBX: 0000000000000001 RCX: 00000000000000ca
[   63.182535] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffa20068253538
[   63.189983] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[   63.197381] R10: 0000000000000000 R11: ffffbd694032fcb0 R12: ffffbd694032fdb0
[   63.204782] R13: 0000000000000000 R14: ffffbd694032fd00 R15: ffffa20068251e40
[   63.212228] FS:  0000000000000000(0000) GS:ffffa2006aa00000(0000) knlGS:0000000000000000
[   63.220614] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   63.226500] CR2: 00007fe65ba85cb0 CR3: 00000001293b6000 CR4: 00000000000406f0
[   63.233841] Call Trace:
[   63.236410]  mt76_calc_tx_airtime+0xf4/0x190 [mt76]
[   63.241464]  mt76x02_send_tx_status+0x1cd/0x3f0 [mt76x02_lib]
[   63.247430]  mt76x02_tx_status_data+0x54/0x80 [mt76x02_lib]
[   63.253186]  mt76u_tx_status_data+0x63/0xc0 [mt76_usb]
[   63.258451]  process_one_work+0x1e2/0x3b0
[   63.262533]  worker_thread+0x4a/0x3d0
[   63.266306]  kthread+0xfb/0x130
[   63.269550]  ? process_one_work+0x3b0/0x3b0
[   63.273893]  ? kthread_park+0x90/0x90
[   63.277677]  ret_from_fork+0x22/0x40
[   63.281411] Modules linked in: ccm bridge stp llc mt76x2u mt76x2_common mt76x02_usb mt76_usb mt76x02_lib mt76 nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci ath10k_core amd64_edac_mod edac_mce_amd ath kvm_amd mac80211 kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel pcengines_apuv2 cfg80211 gpio_keys_polled crypto_simd input_polldev gpio_amd_fch cryptd igb glue_helper pcspkr fam15h_power sp5100_tco k10temp i2c_piix4 rfkill i2c_algo_bit ccp libarc4 dca rng_core uio_pdrv_genirq evdev leds_gpio uio mac_hid coreboot_table acpi_cpufreq pinctrl_amd sr_mod cdrom ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod usb_storage ahci libahci libata xhci_pci sdhci_pci xhci_hcd scsi_mod cqhci sdhci ehci_pci crc32c_intel ehci_hcd mmc_core gpio_keys
[   63.365937] ---[ end trace f13e9cdc5f55db9e ]---
[   63.370802] RIP: 0010:mt76_calc_rx_airtime+0x12b/0x150 [mt76]
[   63.376807] Code: 8d 34 76 48 8d 34 b1 0f b6 4e 07 66 85 c9 74 25 66 83 f9 01 75 1c b9 24 00 00 00 89 d0 0f b7 76 04 c1 e0 05 8d 04 d0 01 c0 99 <f7> fe 01 c8 c3 31 c0 c3 0f 0b c3 44 89 c8 83 e0 01 3c 01 19 c9 83
[   63.396220] RSP: 0018:ffffbd694032fcf0 EFLAGS: 00010216
[   63.401660] RAX: 0000000000002850 RBX: 0000000000000001 RCX: 00000000000000ca
[   63.409145] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffa20068253538
[   63.416505] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[   63.423988] R10: 0000000000000000 R11: ffffbd694032fcb0 R12: ffffbd694032fdb0
[   63.431425] R13: 0000000000000000 R14: ffffbd694032fd00 R15: ffffa20068251e40
[   63.438793] FS:  0000000000000000(0000) GS:ffffa2006aa00000(0000) knlGS:0000000000000000
[   63.447141] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   63.453216] CR2: 00007fe65ba85cb0 CR3: 00000001293b6000 CR4: 00000000000406f0

I would guess, that 
	mt76_calc_legacy_rate_duration: duration += (len * 10) / rate->bitrate;
triggers this kind of message.

Re: [PATCH v8 6/6] mt76: mt76x02: add channel switch support for usb interfaces

[Markus Theil]

On 11/25/19 7:30 PM, Markus Theil wrote:
> On 11/25/19 6:02 PM, Stanislaw Gruszka wrote:
>> On Mon, Nov 25, 2019 at 04:32:42PM +0100, Markus Theil wrote:
>>> On 11/25/19 3:51 PM, Markus Theil wrote:
>>>> On 11/25/19 2:04 PM, Stanislaw Gruszka wrote:
>>>>> On Thu, Nov 21, 2019 at 07:00:01PM +0100, Markus Theil wrote:
>>>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>>> index 90c024f12302..b9bd6bfb2a9d 100644
>>>>>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
>>>>>> @@ -210,6 +210,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
>>>>>>  
>>>>>>  	mt76x02_mac_set_beacon_prepare(dev);
>>>>>>  
>>>>>> +	mt76_csa_check(&dev->mt76);
>>>>>> +	if (dev->mt76.csa_complete) {
>>>>>> +		mt76_csa_finish(&dev->mt76);
>>>>>> +		goto out;
>>>>>> +	}
>>>>> mmio counterpart setup beacons on CSA, but do not sent buffered
>>>>> PS frames. Perhaps here we should do the same. However not sending
>>>>> beacons on one TBTT looks ok to me as well.
>>>>>
>>>>> Stanislaw
>>>>>
>>>> If I change the order of beacon iteration and csa check, the following warning in mac80211 gets triggered:
>>>>
>>>> 	/* the counter should never reach 0 */
>>>> 	WARN_ON_ONCE(!beacon->csa_current_counter);
>>>>
>>>> Dmesg output looks like this:
>>>>
>>>> [  153.829617] ------------[ cut here ]------------
>>>> [  153.829752] WARNING: CPU: 0 PID: 224 at net/mac80211/tx.c:4318 __ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
>>>> [  153.829756] Modules linked in: ccm bridge stp llc nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci amd64_edac_mod mt76x2u edac_mce_amd mt76x2_common ath10k_core mt76x02_usb kvm_amd mt76_usb mt76x02_lib mt76 kvm ath mac80211 irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel cfg80211 pcengines_apuv2 gpio_keys_polled aesni_intel input_polldev crypto_simd gpio_amd_fch cryptd glue_helper igb pcspkr k10temp fam15h_power rfkill sp5100_tco i2c_piix4 libarc4 ccp i2c_algo_bit dca rng_core uio_pdrv_genirq uio leds_gpio evdev mac_hid pinctrl_amd coreboot_table acpi_cpufreq ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod ahci sdhci_pci libahci cqhci libata sdhci crc32c_intel xhci_pci ehci_pci mmc_core xhci_hcd scsi_mod ehci_hcd gpio_keys
>>>> [  153.829948] CPU: 0 PID: 224 Comm: kworker/0:1H Not tainted 5.4.0-rc7-1-01110-g19b7e21c55c8 #32
>>>> [  153.829952] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
>>>> [  153.829966] Workqueue: events_highpri mt76x02u_pre_tbtt_work [mt76x02_usb]
>>>> [  153.830067] RIP: 0010:__ieee80211_csa_update_counter.isra.0.part.0+0x5/0x10 [mac80211]
>>>> [  153.830077] Code: 4c 89 4a 18 c3 48 8b 47 10 4c 89 4f 10 48 89 4e 18 48 89 46 20 4c 89 08 eb a1 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 <0f> 0b c3 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 54 53 48 89 fb
>>>> [  153.830082] RSP: 0018:ffffbde8803d7c80 EFLAGS: 00010246
>>>> [  153.830089] RAX: 0000000000000003 RBX: ffffbde8803d7d20 RCX: ffffa3b01df8f658
>>>> [  153.830093] RDX: ffffbde8803d7d20 RSI: ffffa3b02901d450 RDI: ffffbde8803d7ce0
>>>> [  153.830097] RBP: ffffa3b0267007a0 R08: ffffffff8d011040 R09: 0000000000000000
>>>> [  153.830101] R10: ffffa3b029908098 R11: ffffa3b02ab2ab38 R12: 0000000000000000
>>>> [  153.830105] R13: ffffa3b02901d450 R14: 0000000000000000 R15: ffffa3b0284eae00
>>>> [  153.830111] FS:  0000000000000000(0000) GS:ffffa3b02aa00000(0000) knlGS:0000000000000000
>>>> [  153.830115] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [  153.830119] CR2: 00007fbb6b5c9f30 CR3: 0000000126ee2000 CR4: 00000000000406f0
>>>> [  153.830124] Call Trace:
>>>> [  153.830203]  __ieee80211_beacon_get+0x4c2/0x4d0 [mac80211]
>>>> [  153.830312]  ieee80211_beacon_get_tim+0x41/0x150 [mac80211]
>>>> [  153.830336]  mt76x02_update_beacon_iter+0x2d/0x40 [mt76x02_lib]
>>>> [  153.830352]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
>>>> [  153.830420]  __iterate_interfaces+0x74/0x110 [mac80211]
>>>> [  153.830469]  ? mt76x02_add_buffered_bc+0x80/0x80 [mt76x02_lib]
>>>> [  153.830566]  ieee80211_iterate_interfaces+0x3a/0x50 [mac80211]
>>>> [  153.830580]  mt76x02u_pre_tbtt_work+0x96/0x220 [mt76x02_usb]
>>>> [  153.830600]  process_one_work+0x1e2/0x3b0
>>>> [  153.830610]  worker_thread+0x4a/0x3d0
>>>> [  153.830623]  kthread+0xfb/0x130
>>>> [  153.830631]  ? process_one_work+0x3b0/0x3b0
>>>> [  153.830639]  ? kthread_park+0x90/0x90
>>>> [  153.830650]  ret_from_fork+0x22/0x40
>>>> [  153.830665] ---[ end trace 7a658e5cbfd0f9d1 ]---
>>>>
>>>> Markus
>>>>
>>> In my current local changes I've decoupled checking csa_check and mt76_csa_finish, like it is in the mmio case. As usb has no tbtt interrupt,
>>> I just schedule a delayed work around the estimated beacon transmission time and finish csa there. I'll send another series, if this works.
>> I would prefer not to add yet another delayed work. Does the warning
>> still happen if you arrange code like this?
>>
>>         mt76x02_mac_set_beacon_prepare(dev);
>>
>>         ieee80211_iterate_active_interfaces(mt76_hw(dev),
>>                 IEEE80211_IFACE_ITER_RESUME_ALL,
>>                 mt76x02_update_beacon_iter, dev);
>>
>>         mt76_csa_check(&dev->mt76);
>>         if (dev->mt76.csa_complete) {
>>                 mt76_csa_finish(&dev->mt76);
>>                 goto out;
>>         }
>>
>>         nbeacons = hweight8(dev->mt76.beacon_mask);
>>         mt76x02_enqueue_buffered_bc(dev, &data, N_BCN_SLOTS - nbeacons);
>> 	...
>> out:
>>         mt76x02_mac_set_beacon_finish(dev);
>>         mt76x02u_restart_pre_tbtt_timer(dev);
>>
>>
> You're code works, if I add some locking. ieee80211_iterate_active_interfaces -> ieee80211_iterate_active_interfaces_atomic did the trick for me. Otherwise I get the dmesg output from above.
> After using ieee80211_iterate_active_interfaces_atomic I got the following dmesg output after/while a channel switch:
Got the output again, also with ieee80211_iterate_active_interfaces_atomic.
> [   63.115806] divide error: 0000 [#1] PREEMPT SMP NOPTI
> [   63.121054] CPU: 0 PID: 225 Comm: kworker/u8:2 Tainted: G        W         5.4.0-rc7-1-01110-g19b7e21c55c8 #39
> [   63.131331] Hardware name: PC Engines apu2/apu2, BIOS v4.10.0.3 11/07/2019
> [   63.138399] Workqueue: mt76u mt76u_tx_status_data [mt76_usb]
> [   63.144258] RIP: 0010:mt76_calc_rx_airtime+0x12b/0x150 [mt76]
> [   63.150281] Code: 8d 34 76 48 8d 34 b1 0f b6 4e 07 66 85 c9 74 25 66 83 f9 01 75 1c b9 24 00 00 00 89 d0 0f b7 76 04 c1 e0 05 8d 04 d0 01 c0 99 <f7> fe 01 c8 c3 31 c0 c3 0f 0b c3 44 89 c8 83 e0 01 3c 01 19 c9 83
> [   63.169699] RSP: 0018:ffffbd694032fcf0 EFLAGS: 00010216
> [   63.175176] RAX: 0000000000002850 RBX: 0000000000000001 RCX: 00000000000000ca
> [   63.182535] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffa20068253538
> [   63.189983] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [   63.197381] R10: 0000000000000000 R11: ffffbd694032fcb0 R12: ffffbd694032fdb0
> [   63.204782] R13: 0000000000000000 R14: ffffbd694032fd00 R15: ffffa20068251e40
> [   63.212228] FS:  0000000000000000(0000) GS:ffffa2006aa00000(0000) knlGS:0000000000000000
> [   63.220614] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   63.226500] CR2: 00007fe65ba85cb0 CR3: 00000001293b6000 CR4: 00000000000406f0
> [   63.233841] Call Trace:
> [   63.236410]  mt76_calc_tx_airtime+0xf4/0x190 [mt76]
> [   63.241464]  mt76x02_send_tx_status+0x1cd/0x3f0 [mt76x02_lib]
> [   63.247430]  mt76x02_tx_status_data+0x54/0x80 [mt76x02_lib]
> [   63.253186]  mt76u_tx_status_data+0x63/0xc0 [mt76_usb]
> [   63.258451]  process_one_work+0x1e2/0x3b0
> [   63.262533]  worker_thread+0x4a/0x3d0
> [   63.266306]  kthread+0xfb/0x130
> [   63.269550]  ? process_one_work+0x3b0/0x3b0
> [   63.273893]  ? kthread_park+0x90/0x90
> [   63.277677]  ret_from_fork+0x22/0x40
> [   63.281411] Modules linked in: ccm bridge stp llc mt76x2u mt76x2_common mt76x02_usb mt76_usb mt76x02_lib mt76 nft_masq nft_chain_nat nf_nat nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c nf_tables_set nf_tables nfnetlink ath10k_pci ath10k_core amd64_edac_mod edac_mce_amd ath kvm_amd mac80211 kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel pcengines_apuv2 cfg80211 gpio_keys_polled crypto_simd input_polldev gpio_amd_fch cryptd igb glue_helper pcspkr fam15h_power sp5100_tco k10temp i2c_piix4 rfkill i2c_algo_bit ccp libarc4 dca rng_core uio_pdrv_genirq evdev leds_gpio uio mac_hid coreboot_table acpi_cpufreq pinctrl_amd sr_mod cdrom ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 sd_mod usb_storage ahci libahci libata xhci_pci sdhci_pci xhci_hcd scsi_mod cqhci sdhci ehci_pci crc32c_intel ehci_hcd mmc_core gpio_keys
> [   63.365937] ---[ end trace f13e9cdc5f55db9e ]---
> [   63.370802] RIP: 0010:mt76_calc_rx_airtime+0x12b/0x150 [mt76]
> [   63.376807] Code: 8d 34 76 48 8d 34 b1 0f b6 4e 07 66 85 c9 74 25 66 83 f9 01 75 1c b9 24 00 00 00 89 d0 0f b7 76 04 c1 e0 05 8d 04 d0 01 c0 99 <f7> fe 01 c8 c3 31 c0 c3 0f 0b c3 44 89 c8 83 e0 01 3c 01 19 c9 83
> [   63.396220] RSP: 0018:ffffbd694032fcf0 EFLAGS: 00010216
> [   63.401660] RAX: 0000000000002850 RBX: 0000000000000001 RCX: 00000000000000ca
> [   63.409145] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffa20068253538
> [   63.416505] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [   63.423988] R10: 0000000000000000 R11: ffffbd694032fcb0 R12: ffffbd694032fdb0
> [   63.431425] R13: 0000000000000000 R14: ffffbd694032fd00 R15: ffffa20068251e40
> [   63.438793] FS:  0000000000000000(0000) GS:ffffa2006aa00000(0000) knlGS:0000000000000000
> [   63.447141] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   63.453216] CR2: 00007fe65ba85cb0 CR3: 00000001293b6000 CR4: 00000000000406f0
>
> I would guess, that 
> 	mt76_calc_legacy_rate_duration: duration += (len * 10) / rate->bitrate;
> triggers this kind of message.

Re: [PATCH v8 2/6] mt76: mt76x02: split beaconing

[Stanislaw Gruszka]

On Mon, Nov 25, 2019 at 06:12:56PM +0100, Felix Fietkau wrote:
> On 2019-11-25 17:59, Stanislaw Gruszka wrote:
> > On Mon, Nov 25, 2019 at 03:07:59PM +0100, Markus Theil wrote:
> >> On 11/25/19 2:00 PM, Stanislaw Gruszka wrote:
> >> > On Thu, Nov 21, 2019 at 06:59:57PM +0100, Markus Theil wrote:
> >> >> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> >> >> +{
> >> >> +	mt76_wr(dev, MT_BCN_BYPASS_MASK,
> >> >> +		0xff00 | ~bitrev8(dev->beacon_data_mask));
> >> > Since you arrange beacon slots continues starting from 0
> >> > (i.e. 0,1,2 instead of "random" vif_idx values like 0,4,6),
> >> > I think it would make sense to keep
> >> > MT_MAC_BSSID_DW1_MBEACON_N = bcn_idx - 1 and set mask unchanged.
> >> >
> >> > But no strong opinion here, code with bitrev8 looks fine too.
> >> I'd like to keep the bitrev8 code, as it saves a copy over usb for usb
> >> devices, if MT_MAC_BSSID_DW_BEACON_N is kept constant.
> >> bitrev8 should be a rather cheap operation compared to a copy over some
> >> form of bus.
> > 
> > This make sense. I tested the code on MT7630E and after adding missed
> > write_txwi function, it works fine. So I think bitrev8 code is ok.
> I find the use of bitrev8/ffz a bit convoluted. If I understand the code
> right, wouldn't it be equivalent to keeping beacon_data_count instead of
> beacon_data_mask and doing:
> mt76_wr(dev, MT_BCN_BYPASS_MASK,
> 	0xffff & ~((1 << dev->beacon_data_count) - 1));

If we want to keep constatn MT_MAC_BSSID_DW1_MBEACON_N=7 , I think
correct formula would be:

0xff00 | ~(0xff00 >> dev->beacon_data_count)

Anyway something simpler than bitrev8 can be used to calculate
bypass mask.

Stanislaw