> On Tue, Oct 15, 2019 at 05:16:43PM +0200, Lorenzo Bianconi wrote:
> > Read busy counters not holding cc_lock spinlock since usb read can't be
> > performed in interrupt context. Move cc_active and cc_rx counters out of
> > cc_lock since they are not modified in interrupt context.
> > Grab cc_lock updating cur_cc_bss_rx in mt76_airtime_report and do not
> > hold rx_lock in mt76_update_survey.
> <snip>
> > Fixes: 168aea24f4bb ("mt76: mt76x02u: enable survey support")
>
> I think problem was introduced currently in mt76 driver version
> that is not yet in mainline tree, so this is not right commit.
> On Linus' tree we still read registers outside of cc_lock section.
Hi Stanislaw,
yes, you are right. We should drop the Fixes tag. Thx
>
> void mt76x02_update_channel(struct mt76_dev *mdev)
> {
> ...
>
> busy = mt76_rr(dev, MT_CH_BUSY);
> active = busy + mt76_rr(dev, MT_CH_IDLE);
>
> spin_lock_bh(&dev->mt76.cc_lock);
> state->cc_busy += busy;
> state->cc_active += active;
> spin_unlock_bh(&dev->mt76.cc_lock);
> }
>
> > if (dev->drv->drv_flags & MT_DRV_SW_RX_AIRTIME) {
> > - spin_lock_bh(&dev->rx_lock);
> > - spin_lock(&dev->cc_lock);
> > + spin_lock_bh(&dev->cc_lock);
> > state->cc_bss_rx += dev->cur_cc_bss_rx;
> > dev->cur_cc_bss_rx = 0;
> > - spin_unlock(&dev->cc_lock);
> > - spin_unlock_bh(&dev->rx_lock);
> > + spin_unlock_bh(&dev->cc_lock);
>
> Why dev->rx_lock was needed before and is not needed now ?
Looking at the code I think rx_lock is not needed here since cur_cc_bss_rx is
updated without holding rx_lock in mt76_airtime_report() (we need cc_lock
there).
Regards,
Lorenzo
>
> Stanislaw
signature.asc
Description: PGP signature
