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 <sgruszka@xxxxxxxxxx> > > --- > > 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