Hi On 8/20/2018 15:20, Felipe Balbi wrote: > > Hi, > > Adrian Ambrożewicz <adrian.ambrozewicz@xxxxxxxxx> writes: >> Hello, >> >> I'm consistently observing kernel warnings related to Mass Storage USB >> Gadget de-initialization flow. After investigation I believe that I've >> found root cause of these warnings, however I'm unable to estimate the >> impact. I would like to put the issue into discussion about possible >> side-effects. >> >> My usage model is to emulate file system using f_mass_storage gadget. >> Whenever I pull the USB cable out I see warning related to >> "dwc2_hsotg_init_fifo" which is commented in the following way: "/* >> Reset fifo map if not correctly cleared during previous session */". > > which kernel are you using? Have you tried latest mainline? (currently > at v4.18). > >> Assumption I've made were the following: >> 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect) >> 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable() >> 3) disconnect handler initializes the fifo maps to clear state >> >> Unfortunately in my case the warning occurs every time when using >> f_mass_storage gadget. By comparison with f_hid gadget I've come up >> with conclusion that it's caused by race condition in the way the >> "disable" flow is implemented in mass storage. >> >> "Correct" flow implemented by HID gadget is the following: >> * dwc2_hsotg_irq (USBRst) >> ** dwc2_hsotg_disconnect >> *** call_gadget(disconnect) >> **** hidg_disable >> ***** usb_ep_disable // fifos utilized by device are cleared in fifo_map >> ** dwc_hsotg_core_init_disconnected >> *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here >> >> By comparison here is flow observed in Mass Storage gadget. Race >> condition is introduced by utilizing separate worker thread to handle >> events: >> THREAD #1 >> * dwc2_hsotg_irq (USBRst) >> ** dwc2_hsotg_disconnect >> *** call_gadget(disconnect) >> **** fsg_disable() >> ***** raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread >> #1 and Thread #2 starts here. >> ** dwc_hsotg_core_init_disconnected >> *** dwc2_hsotg_init_fifo // fifo_map is object of the race between >> this function, and call to usb_ep_disable() >> >> THREAD #2 >> * handle_exception(FSG_STATE_CONFIG_CHANGE) >> ** do_set_interface(NULL) >> *** usb_ep_disable() >> >> My questions are the following: >> - is this known issue? >> - is it risky? What are possible outcomes? >> - If this race condition is dangerous what would be proper fix ? >> Should fsg_disable() call be blocked until handle_exception() finishes >> the cleanup? I know that it's just a WA for "misaligned" Mass Storage >> gadget architecture, but are there alternatives? >> >> I would really appreciate professional insight on that matter, > > Let's see if Minas has anything to say about this. Minas will be in the vacation till Aug 31. He will response as soon as he get back to the office. > Regards, Grigor
