Wesley Cheng <wcheng@xxxxxxxxxxxxxx> writes: > Hi, > > On 9/16/2021 10:17 PM, Felipe Balbi wrote: >> >> Hi, >> >> Wesley Cheng <wcheng@xxxxxxxxxxxxxx> writes: >> >>> There is a race present where the DWC3 runtime resume runs in parallel >>> to the UDC unbind sequence. This will eventually lead to a possible >>> scenario where we are enabling the run/stop bit, without a valid >>> composition defined. >>> >>> Thread#1 (handling UDC unbind): >>> usb_gadget_remove_driver() >>> -->usb_gadget_disconnect() >>> -->dwc3_gadget_pullup(0) >>> --> continue UDC unbind sequence >>> -->Thread#2 is running in parallel here >>> >>> Thread#2 (handing next cable connect) >>> __dwc3_set_mode() >>> -->pm_runtime_get_sync() >>> -->dwc3_gadget_resume() >>> -->dwc->gadget_driver is NOT NULL yet >>> -->dwc3_gadget_run_stop(1) >>> --> _dwc3gadget_start() >>> ... >>> >>> Fix this by tracking the pullup disable routine, and avoiding resuming >>> of the DWC3 gadget. Once the UDC is re-binded, that will trigger the >>> pullup enable routine, which would handle enabling the DWC3 gadget. >>> >>> Signed-off-by: Wesley Cheng <wcheng@xxxxxxxxxxxxxx> > > Thanks, Felipe! > >> >> This looks okay to me, but needs to be tested by a few folks ;-) >> >> Acked-by: Felipe Balbi <balbi@xxxxxxxxxx> >> > Yes, would be good to get some functions using > usb_gadget_activate/deactivate(). It should be OK for those situations > as well, but just to make sure :) IIRC, the UVC function relies on those. You could give it a shot ;-) -- balbi
