Messages in this thread | | | From | Robert Jarzmik <> | Subject | Re: [PATCH 1/2] usb: gadget: r8a66597: Fix two possible sleep-in-atomic-context bugs in init_controller() | Date | Fri, 29 Jun 2018 15:21:19 +0200 |
| |
Felipe Balbi <balbi@kernel.org> writes:
> Hi, > > Robert Jarzmik <robert.jarzmik@free.fr> writes: >>>> And as bonus question, why is it better to have mdelay() calls in the driver ? >>> >>> As a bugfix, it's the smallest fix possible, right? Ideally, we wouldn't >>> need either of them. Perhaps there's a bit which can be polled instead? >> Ideally yes. Do you remember if a "threaded interrupt" might use msleep() ? I >> seem to remember that they can, so won't that be another alternative ? > > yeah, unless, of course, you have a spinlock held. ;-) Ah yes, unless that :)
I would have proposed to call the disconnect out of the spinlock path, but looking at the r8a66592_usb_disconnect(), with its spinlock flip-flop, I loose heart ...
And even if I still think no mdelay() should be used, because of the kernel stall (and global uniprocessor stall), I won't argue anymore. After all, if you let in the mdelay(), perhaps the maintainers will agree to review their architecture and drop the locks or sleeps in interrupt context in a follow-up patch, who knows ...
Cheers.
-- Robert
| |