Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 21 Nov 2019 13:31:04 +0100
From:   Stanislaw Gruszka <sgruszka@redhat.com>
To:     Markus Theil <markus.theil@tu-ilmenau.de>
Cc:     nbd@nbd.name, linux-wireless@vger.kernel.org,
        lorenzo.bianconi@redhat.com
Subject: Re: [PATCH v7 2/5] mt76: mt76x02: split beaconing
Message-ID: <20191121123102.GA13833@redhat.com>
References: <20191120222826.14871-1-markus.theil@tu-ilmenau.de>
 <20191120222826.14871-3-markus.theil@tu-ilmenau.de>
MIME-Version: 1.0
In-Reply-To: <20191120222826.14871-3-markus.theil@tu-ilmenau.de>
User-Agent: Mutt/1.8.3 (2017-05-23)
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Wed, Nov 20, 2019 at 11:28:23PM +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.
>=20
> The code is also adapted for the mmio part of the driver, but should not
> have any performance implication there.
<snip>
> +=09 */
> +=09if (mt76_is_usb(&dev->mt76)) {
> +=09=09struct mt76x02_txwi *txwi;
> +
> +=09=09mt76_insert_hdr_pad(skb);
> +=09=09txwi =3D (struct mt76x02_txwi *)(skb->data - sizeof(*txwi));
> +=09=09mt76x02_mac_write_txwi(dev, txwi, skb, NULL, NULL, skb->len);
> +=09=09skb_push(skb, sizeof(*txwi));
> +=09} else {
> +=09=09struct mt76x02_txwi txwi;
> =20
> -=09mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> -=09offset +=3D sizeof(txwi);
> +=09=09mt76_wr_copy(dev, offset, &txwi, sizeof(txwi));
> +=09=09offset +=3D sizeof(txwi);
> +=09}

You merged another patch into this one. Please keep them separated.

> +void mt76x02_mac_set_beacon_finish(struct mt76x02_dev *dev)
> +{
> +=09mt76_rmw_field(dev, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_MBEACON_N,
> +=09=09       hweight8(dev->beacon_data_mask) - 1);
> +=09mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xff00 | ~dev->beacon_data_mask);
> +}

Well, this code still does not look quite right. At least it is not
compatible what the BCN_BYPASS_MASK description said in the manual.

I think the code need serious testing on multi-bss (USB support up
to 2  AP bssids) and with broadcast/multicast with some PS stations
on the network.

In particular adding second bssid and remove first one should
be checked.

Stanislaw