In my current workspace the kernel used is 4.17.7 . Unfortunately I don't have the resources now to verify with newer version but I might look into that later if it's necessary. I've only compared source code between my worktree and mainline sources and verified that code around this area still looks almost exactly the same (with minor changes like renames of API calls here and there). Looking forward for Minas opinion. pon., 20 sie 2018 o 13:20 Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> napisał(a): > > > 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. > > -- > balbi
