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