2019-11-18 22:17:46

by Markus Theil

[permalink] [raw]
Subject: [PATCH v4 0/4] mt76: channel switch support for USB devices

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 three patches speed up beacon copying and the last
patch enables channel switch support also for usb interfaces.

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

v3:
* fixed checkpatch errors

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

Markus Theil (4):
mt76: mt76x02: ommit beacon slot clearing
mt76: mt76x02: split beaconing
mt76: speed up usb bulk copy
mt76: mt76x02: add channel switch support for usb interfaces

drivers/net/wireless/mediatek/mt76/mt76.h | 2 +-
.../wireless/mediatek/mt76/mt76x02_beacon.c | 52 ++++++-------------
.../net/wireless/mediatek/mt76/mt76x02_mac.h | 1 +
.../net/wireless/mediatek/mt76/mt76x02_mmio.c | 5 ++
.../wireless/mediatek/mt76/mt76x02_usb_core.c | 12 +++++
.../net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
.../wireless/mediatek/mt76/mt76x2/usb_main.c | 7 +++
drivers/net/wireless/mediatek/mt76/usb.c | 24 ++++++---
8 files changed, 60 insertions(+), 45 deletions(-)

--
2.24.0


2019-11-18 22:20:20

by Markus Theil

[permalink] [raw]
Subject: [PATCH v4 3/4] mt76: speed up usb bulk copy

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 <[email protected]>
---
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 8aec7ccf2d79..7a6f5d097a3d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -383,7 +383,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 20c6fe510e9d..f1f67b0f8265 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 = DIV_ROUND_UP(len, 4) * 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((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

2019-11-18 22:20:20

by Markus Theil

[permalink] [raw]
Subject: [PATCH v4 4/4] mt76: mt76x02: add channel switch support for usb interfaces

This patch enables channel switch support on mt76 usb interfaces.

Signed-off-by: Markus Theil <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c | 7 +++++++
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c | 7 +++++++
3 files changed, 15 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 8a2a90fb5663..891825043117 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -182,6 +182,12 @@ static void mt76x02u_pre_tbtt_work(struct work_struct *work)
/* Prevent corrupt transmissions during update */
mt76_set(dev, MT_BCN_BYPASS_MASK, 0xffff);

+ 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);
@@ -196,6 +202,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 414b22399d93..3f95e5b24e1d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -174,7 +174,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)) {
@@ -184,6 +183,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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
index eb73cb856c81..a236b5249a86 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
@@ -100,6 +100,12 @@ mt76x2u_config(struct ieee80211_hw *hw, u32 changed)
return err;
}

+static void mt76x2u_channel_switch_beacon(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct cfg80211_chan_def *chandef)
+{
+}
+
const struct ieee80211_ops mt76x2u_ops = {
.tx = mt76x02_tx,
.start = mt76x2u_start,
@@ -121,4 +127,5 @@ const struct ieee80211_ops mt76x2u_ops = {
.get_survey = mt76_get_survey,
.set_tim = mt76_set_tim,
.release_buffered_frames = mt76_release_buffered_frames,
+ .channel_switch_beacon = mt76x2u_channel_switch_beacon,
};
--
2.24.0

2019-11-19 11:22:39

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] mt76: mt76x02: add channel switch support for usb interfaces

On Mon, Nov 18, 2019 at 11:15:40PM +0100, Markus Theil wrote:
> +static void mt76x2u_channel_switch_beacon(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct cfg80211_chan_def *chandef)
> +{
> +}
> +
> const struct ieee80211_ops mt76x2u_ops = {
> .tx = mt76x02_tx,
> .start = mt76x2u_start,
> @@ -121,4 +127,5 @@ const struct ieee80211_ops mt76x2u_ops = {
> .get_survey = mt76_get_survey,
> .set_tim = mt76_set_tim,
> .release_buffered_frames = mt76_release_buffered_frames,
> + .channel_switch_beacon = mt76x2u_channel_switch_beacon,

Is this needed ? Seems mac80211 check against this op being NULL
in drv_channel_switch_beacon() and it is not used otherwise.

Stanislaw


2019-11-19 12:15:44

by Markus Theil

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] mt76: mt76x02: add channel switch support for usb interfaces

On 11/19/19 12:19 PM, Stanislaw Gruszka wrote:
> On Mon, Nov 18, 2019 at 11:15:40PM +0100, Markus Theil wrote:
>> +static void mt76x2u_channel_switch_beacon(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + struct cfg80211_chan_def *chandef)
>> +{
>> +}
>> +
>> const struct ieee80211_ops mt76x2u_ops = {
>> .tx = mt76x02_tx,
>> .start = mt76x2u_start,
>> @@ -121,4 +127,5 @@ const struct ieee80211_ops mt76x2u_ops = {
>> .get_survey = mt76_get_survey,
>> .set_tim = mt76_set_tim,
>> .release_buffered_frames = mt76_release_buffered_frames,
>> + .channel_switch_beacon = mt76x2u_channel_switch_beacon,
> Is this needed ? Seems mac80211 check against this op being NULL
> in drv_channel_switch_beacon() and it is not used otherwise.
>
> Stanislaw
I checked if this call is really needed and you're right, it is not
needed. I'll send v5 with this fix.

2019-11-19 12:18:58

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] mt76: speed up usb bulk copy

> 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 <[email protected]>
> ---
> 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 8aec7ccf2d79..7a6f5d097a3d 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -383,7 +383,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 20c6fe510e9d..f1f67b0f8265 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 = DIV_ROUND_UP(len, 4) * 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((int)sizeof(usb->data), len - i);

What about using min_t() here?

> + 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
>


Attachments:
(No filename) (2.42 kB)
signature.asc (235.00 B)
Download all attachments

2019-11-19 12:29:54

by Markus Theil

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] mt76: speed up usb bulk copy

On 11/19/19 1:15 PM, Lorenzo Bianconi wrote:
>> 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 <[email protected]>
>> ---
>> 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 8aec7ccf2d79..7a6f5d097a3d 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
>> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
>> @@ -383,7 +383,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 20c6fe510e9d..f1f67b0f8265 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 = DIV_ROUND_UP(len, 4) * 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((int)sizeof(usb->data), len - i);
> What about using min_t() here?
Sorry, read it too late for v5. I'll wait a bit for other comments and
include it in a next version.
>> + 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
>>
--
Markus Theil

Technische Universit?t Ilmenau, Fachgebiet Telematik/Rechnernetze
Postfach 100565
98684 Ilmenau, Germany

Phone: +49 3677 69-4582
Email: markus[dot]theil[at]tu-ilmenau[dot]de
Web: http://www.tu-ilmenau.de/telematik