Hi, Jassi Brar writes: >> > +static void __max3420_start(struct max3420_udc *udc) >> > +{ >> > + u8 val; >> > + >> > + /* Need this delay if bus-powered */ >> > + msleep_interruptible(250); >> >> should you check if you're bus powered? >> > for some reason, even for self-powered, it helped reliability. Perhaps update the comment, in that case? It would be better if we had a proper explanation for this sleep here. >> > +static int max3420_thread(void *dev_id) >> >> Why do you need this thread? Sure you can't live without it? >> > All the slow spi-bus transfers are handled at one place here without > blocking any api call. IMO it is cleaner and easier to manage. Fair enough, I won't complain. But it looks odd :-p -- balbi