On 11/20/19 10:28 AM, Stanislaw Gruszka wrote:
> On Tue, Nov 19, 2019 at 04:47:43PM +0100, Markus Theil wrote:
>> 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). Making this split breaks beacon
>> enabling/disabling per vif. This is fixed by adding a call to set the
>> bypass mask, if beaconing should be disabled for a vif. Otherwise the
>> beacon is send after the next beacon interval.
>>
>> The code is also adapted for the mmio part of the driver, but should not
>> have any performance implication there.
>>
>> Signed-off-by: Markus Theil <markus.theil@xxxxxxxxxxxxx>
>> ---
>> .../wireless/mediatek/mt76/mt76x02_beacon.c | 44 +++++++------------
>> .../net/wireless/mediatek/mt76/mt76x02_mac.h | 1 +
>> .../net/wireless/mediatek/mt76/mt76x02_mmio.c | 5 +++
>> .../wireless/mediatek/mt76/mt76x02_usb_core.c | 5 +++
>> 4 files changed, 26 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
>> index 403866496640..09013adae854 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
>> @@ -47,10 +47,6 @@ __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);
>> @@ -60,41 +56,30 @@ __mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 bcn_idx,
>> 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_finish(struct mt76x02_dev *dev)
>> {
>> - bool force_update = false;
>> - int bcn_idx = 0;
>> int i;
>> + int bcn_idx = 0;
>>
>> - 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]);
>> - }
>> -
>> + for (i = 0; i < hweight8(dev->mt76.beacon_mask); ++i)
>> bcn_idx += !!dev->beacons[i];
> This looks wrong since we do not calculate all beacons, only
> up to hweight8(dev->mt76.beacon_mask).
>
> But since we need to calculate number of all beacons we can just
> use hweight8(dev->mt76.beacon_mask) directly.
You're right that I made a mistake here. Using
hweight8(dev->mt76.beacon_mask) would be wrong
for usb devices, which may setup buffered broadcast or multicast frames
as additional beacons.
My updated patch therefore uses hweight8(dev->mt76.beacon_data_mask)
directly.
>> - }
>> -
>> - for (i = bcn_idx; i < ARRAY_SIZE(dev->beacons); i++) {
>> - if (!(dev->beacon_data_mask & BIT(i)))
>> - break;
>> -
>> - __mt76x02_mac_set_beacon(dev, i, NULL);
>> - }
>>
>> mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
>> bcn_idx - 1);
>> +
>> + mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
> I'm not sure if this is correct for multi bss.
>
> In MT7620 manual BCM_BAYPASS_MASK is described as below:
>
> "
> Directly bypasses the Tx Beacon frame with the specified
> Beacon number. Bit0=Nth Beacon, bit1=(N- 1)th Beacon,... etc.
> N is the number of Beacons defined in the MULTI_BCN_NUM field in the
> MAC_BSSID_DW1(offset: 0x1014) register.
> 0: Disable
> 1: Enable
> "
>
> Assuming manual is correct (it could be wrong) bypass mask should be
> calculated differently.
>
> Stanislaw
>
I tested the updated code with my usb nic and an mbss with 2 ap vifs.
Both beacons were transmitted. Maybe the manual is wrong in this place.
Markus
