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