On Tue, Aug 01, 2023 at 06:36:53AM -0700, Badhri Jagan Sridharan wrote: > Hi Alan, > > @@ -1639,7 +1644,11 @@ static void gadget_unbind_driver(struct > > usb_gadget_disable_async_callbacks(udc); > > if (gadget->irq) > > synchronize_irq(gadget->irq); > > + mutex_unlock(&udc->connect_lock); > > + > > In my humble opinion, this should be OK. > I was wondering what would happen if soft_connect_store() is invoked > right after the udc->connect_lock is dropped. One of your previous > patches(1016fc0c096c USB: gadget: Fix obscure lockdep violation for > udc_mutex) already prevents this race by making soft_connect_store() > acquire device_lock(&udc->gadget->dev); and hence avoids the race. I wouldn't go so far as to say that all the problems have been fixed. There's nothing to prevent the user from writing to the soft_connect attribute immediately after gadget_unbind_driver() finishes. Doing so would cause usb_gadget_udc_start_locked() and usb_gadget_connect_locked() to run. The first would tell the UDC driver to turn the controller on, and the second would do essentially nothing (because the allow_connect flag is clear). But this would still leave the controller on when it should be off. Maybe we can chalk this outcome up to user error. Alan Stern
