On Fri, Feb 22, 2019 at 10:53:19AM +0100, Lorenzo Bianconi wrote:
> > Use usb_bulk_msg for reading MCU command responses. This simplify code
> > a lot.
> >
> > Together with 97a3005759c ("mt76usb: allow mt76u_bulk_msg be used
> > for reads") it also fix possible problems with rx data buffers
> > not being aligned and contained within single page. After doing
> > page_frag_alloc(1024) consecutive page_frag_alloc(PAGE_SIZE) will
> > alloc PAGE_SIZE buffer at PAGE_SIZE - 1024 offset.
> >
> > Signed-off-by: Stanislaw Gruszka <[email protected]>
> > ---
> > drivers/net/wireless/mediatek/mt76/mt76.h | 3 +-
> > drivers/net/wireless/mediatek/mt76/mt76x0/usb.c | 11 --------
> > .../net/wireless/mediatek/mt76/mt76x02_usb_mcu.c | 32
> > +++++++---------------
> > drivers/net/wireless/mediatek/mt76/mt76x2/usb.c | 11 --------
> > drivers/net/wireless/mediatek/mt76/usb.c | 1 -
> > drivers/net/wireless/mediatek/mt76/usb_mcu.c | 31
> > +++------------------
> > 6 files changed, 15 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> > b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 6092646014c4..c9b5eb9b0582 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -384,8 +384,7 @@ struct mt76_usb {
> >
> > struct mt76u_mcu {
> > struct mutex mutex;
> > - struct completion cmpl;
>
> Hi Stanislaw,
>
> I was reviewing this approach and I guess it is a little bit racey since now
> we
> are not sure that when the device is removed or suspended the pending mcu
> commands
> are terminated and we do not have any api to stop usb transactions.
> Are we sure when we access mt76x02_dev/mt76_dev structure it has not been
> already removed?
> Maybe we need to maintain the completion in mt76u_mcu and use it to wait the
> mcu
> commands are terminated.
I don't think so. On suspend we do mt76xxu_mac_stop() which access registers
using mcu commands, all should be synchronized by mcu->mutex . All other
process that could use mcu command should be already stopped. Actually suspend
can be simplified.
Device remove should be properly handled by -ENODEV error returned by
usb_bulk_msg.
Stanislaw
