Pontus Fuchs <pontus.fuchs@xxxxxxxxx> writes: > On 2012-10-13 07:41, Kalle Valo wrote: >> Pontus Fuchs <pontus.fuchs@xxxxxxxxx> writes: >> >>> +#define ar5523_err(ar, format, arg...) \ >>> +do { \ >>> + if (!test_bit(AR5523_USB_DISCONNECTED, &ar->flags)) { \ >>> + dev_err(&(ar)->dev->dev, format, ## arg); \ >>> + } \ >>> +} while (0) >> This looks suspicious and might hide something important. It's better to >> fix the real cause than to workaround it in the error log printer. Do >> you know why this was added? > > Yes. The purpose is to hide all the USB errors that happens when you > hot-unplug the dongle. There can be quite a few URBs in flight and > all of them will fail with -ENODEV or -ESHUTDOWN. Instead of checking > for these errors in all the usb submissions and callbacks I added this > flag. Do you still a problem with this approach? IMHO it's bad style to hide that inside a logging macro and might hide some other bugs. It's better to check the state explicitly in the callback. But again this isn't anything important, just something I noticed while reviewing the code. Do what you think is best. -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html