| Message ID | 20191104150341.13896-1-markus.theil@tu-ilmenau.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | mt76: mt76x02: fix num slots in beacon config init | expand |
> mt76x02 mmio and usb devices use a different number of beacon slots (8 > vs. 5). Consider this in mt76x02_init_beacon_config. > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> > --- > drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > index 4209209ac940..b7412953ff26 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > @@ -249,7 +249,7 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev) > mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); > mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); > > - for (i = 0; i < 8; i++) > + for (i = 0; i < dev->beacon_ops->nslots; i++) > mt76x02_mac_set_beacon(dev, i, NULL); > > mt76x02_set_beacon_offsets(dev); Hi Markus, mt76x02_init_beacon_config is run just at bootstrap and it is used to clean all beacon RAM memory. It can't see any issue with the current code. Regards, Lorenzo > -- > 2.17.1 >
On 04.11.19 16:45, Lorenzo Bianconi wrote: >> mt76x02 mmio and usb devices use a different number of beacon slots (8 >> vs. 5). Consider this in mt76x02_init_beacon_config. >> >> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> >> --- >> drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> index 4209209ac940..b7412953ff26 100644 >> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c >> @@ -249,7 +249,7 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev) >> mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); >> mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); >> >> - for (i = 0; i < 8; i++) >> + for (i = 0; i < dev->beacon_ops->nslots; i++) >> mt76x02_mac_set_beacon(dev, i, NULL); >> >> mt76x02_set_beacon_offsets(dev); > Hi Markus, > > mt76x02_init_beacon_config is run just at bootstrap and it is used to clean all > beacon RAM memory. It can't see any issue with the current code. > > Regards, > Lorenzo > >> -- >> 2.17.1 >> Hi Lorenzo, I just thought this function should overwrite all 8192 byte beacon RAM memory. If the loop count is set to 8 it would overwrite 8 x 1024 = 8192 byte in the mmio case and 8 x 1638 = 13104 byte in the USB case. 1638 is 8192 / N_BCN_SLOTS. N_BCN_SLOTS is currently 5 for USB. mt76x02_beacon.c has no further checks for beacon_ops->nslots in the case of setting a beacon. Markus
On Nov 04, Markus Theil wrote: > On 04.11.19 16:45, Lorenzo Bianconi wrote: > >> mt76x02 mmio and usb devices use a different number of beacon slots (8 > >> vs. 5). Consider this in mt76x02_init_beacon_config. > >> > >> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> > >> --- > >> drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> index 4209209ac940..b7412953ff26 100644 > >> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> @@ -249,7 +249,7 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev) > >> mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); > >> mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); > >> > >> - for (i = 0; i < 8; i++) > >> + for (i = 0; i < dev->beacon_ops->nslots; i++) > >> mt76x02_mac_set_beacon(dev, i, NULL); > >> > >> mt76x02_set_beacon_offsets(dev); > > Hi Markus, > > > > mt76x02_init_beacon_config is run just at bootstrap and it is used to clean all > > beacon RAM memory. It can't see any issue with the current code. > > > > Regards, > > Lorenzo > > > >> -- > >> 2.17.1 > >> > Hi Lorenzo, > > I just thought this function should overwrite all 8192 byte beacon RAM > memory. If the loop count is set to 8 it would overwrite 8 x 1024 = 8192 > byte in the mmio case and 8 x 1638 = 13104 byte in the USB case. 1638 is > 8192 / N_BCN_SLOTS. N_BCN_SLOTS is currently 5 for USB. mt76x02_beacon.c > has no further checks for beacon_ops->nslots in the case of setting a > beacon. ops..I did not notice beacon_len in __mt76x02_mac_set_beacon Lorenzo > > Markus > >
On Mon, Nov 04, 2019 at 05:07:16PM +0100, Markus Theil wrote: > On 04.11.19 16:45, Lorenzo Bianconi wrote: > >> mt76x02 mmio and usb devices use a different number of beacon slots (8 > >> vs. 5). Consider this in mt76x02_init_beacon_config. > >> > >> Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> > >> --- > >> drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> index 4209209ac940..b7412953ff26 100644 > >> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c > >> @@ -249,7 +249,7 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev) > >> mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); > >> mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); > >> > >> - for (i = 0; i < 8; i++) > >> + for (i = 0; i < dev->beacon_ops->nslots; i++) > >> mt76x02_mac_set_beacon(dev, i, NULL); > >> > >> mt76x02_set_beacon_offsets(dev); > > Hi Markus, > > > > mt76x02_init_beacon_config is run just at bootstrap and it is used to clean all > > beacon RAM memory. It can't see any issue with the current code. > > > > Regards, > > Lorenzo > > > >> -- > >> 2.17.1 > >> > Hi Lorenzo, > > I just thought this function should overwrite all 8192 byte beacon RAM > memory. If the loop count is set to 8 it would overwrite 8 x 1024 = 8192 > byte in the mmio case and 8 x 1638 = 13104 byte in the USB case. 1638 is > 8192 / N_BCN_SLOTS. N_BCN_SLOTS is currently 5 for USB. mt76x02_beacon.c > has no further checks for beacon_ops->nslots in the case of setting a > beacon. We do not override beacon SRAM memory in mt76x02_init_beacon_config() as bcn_idx increase only if !!dev->beacons[i], so we 8 times nullify beacon 0 . Patch is fine though, but things can be optimized more ... We possibly can skip beacon SRAM nullification at all (even if there is garbage in the memory, beacon will not be sent if blocked by MT_BCN_BYPASS_MASK = 0xffff). Or nullify SRAM at once. And just set proper MT_MAC_BSSID_DW1_MBEACON_N value. Stanislaw
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c index 4209209ac940..b7412953ff26 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c @@ -249,7 +249,7 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev) mt76_set(dev, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); - for (i = 0; i < 8; i++) + for (i = 0; i < dev->beacon_ops->nslots; i++) mt76x02_mac_set_beacon(dev, i, NULL); mt76x02_set_beacon_offsets(dev);
mt76x02 mmio and usb devices use a different number of beacon slots (8 vs. 5). Consider this in mt76x02_init_beacon_config. Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> --- drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)