2017-04-27 20:13 GMT+03:00 Bin Liu <b-liu@xxxxxx>: > On Thu, Apr 27, 2017 at 07:26:31PM +0300, Matwey V. Kornilov wrote: >> 2017-04-27 18:35 GMT+03:00 Bin Liu <b-liu@xxxxxx>: >> > Hi Matwey, >> > >> > On Thu, Apr 27, 2017 at 01:20:33PM +0300, Matwey V. Kornilov wrote: >> >> This commit changes the order of actions undertaken in >> >> musb_advance_schedule() in order to overcome issue with broken >> >> isochronous transfer [1]. >> >> >> >> There is no harm to split musb_giveback into two pieces. The first >> >> unlinks finished urb, the second givebacks it. The issue here that >> >> givebacking may be quite time-consuming due to urb->complete() call. >> >> As it happens in case of pwc-driven web cameras. It may take about 0.5 >> >> ms to call __musb_giveback() that calls urb->callback() internally. >> >> Under specific circumstances setting MUSB_RXCSR_H_REQPKT in subsequent >> >> musb_start_urb() for the next urb will be too late to produce physical >> >> IN packet. Since auto req is not used by this module the exchange >> >> would be as the following: >> >> >> >> [ ] 7.220456 d= 0.000997 [182 + 3.667] [ 3] IN : 4.5 >> >> [ T ] 7.220459 d= 0.000003 [182 + 7.000] [800] DATA0: [skipped] >> >> [ ] 7.222456 d= 0.001997 [184 + 3.667] [ 3] IN : 4.5 >> >> [ ] 7.222459 d= 0.000003 [184 + 7.000] [ 3] DATA0: 00 00 >> >> >> >> It is known that missed IN in isochronous mode makes some >> >> perepherial broken. For instance, pwc-driven or uvc-driven >> >> web cameras. >> >> In order to workaround this issue we postpone calling >> >> urb->callback() after setting MUSB_RXCSR_H_REQPKT for the >> >> next urb if there is the next urb pending in queue. >> >> >> >> [1] https://www.spinics.net/lists/linux-usb/msg145747.html >> >> >> >> Fixes: f551e1352983 ("Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb return in bottom half"") >> >> Signed-off-by: Matwey V. Kornilov <matwey@xxxxxxxxxx> >> > >> > Thanks for the effort of working on this long standing issue, I know you >> > have spent alot of time on it, but what I am thinking is instead of >> > workaround the problem we need to understand the root cause - why >> > uvc-video takes longer to exec the urb callback, why only am335x >> > reported this issue. This is on my backlog, just seems never got time >> > for it... >> >> Have you tried other SoCs with Invetra MUSB? > > That is the plan, I got an A20 board, but haven't bring it up yet. > >> >> > >> > Ideally MUSB driver should be just using HCD_BH flag and let the core to >> > handle the urb callback, regardless the usb transfer types. >> >> I think the only reason why everything worked before with HCD_BH is >> that execution of urb->callback() was placed after musb_start(). The >> order of operations matters. >> However, you said that something was also wrong with HCD_BH and other >> peripherals. > > HCD_BH flag cause some issues which are docummented in the commit log of > f551e1352983. > But even with HCD_BH flag, it didn't work for uvc webcams, it still misses > IN tokens. It might helps pwc webcams though. pwc webcams work with HCD_BH fine indeed. > >> > The MUSB drivers are already messy and complicated enough for >> > maintenance, I'd like to understand the root cause of the delay first >> > before decide how to solve the issue. >> > >> >> I feel from playing with OpenVizsla that REQPKT should be set well in >> advance. So, time window to set the flag is actually smaller than 1 >> ms. >> urb->callback() is never takes longer than 0.4 ms for pwc driver, but >> INs are skipped. > > Setting REQPKT in advance might be the solution, but I'd like to > understand why only Isoch transfer shows such issue, and why only am335x > reports this issue. The later concerns me more if this would be a > system issue not only in usb module. 0.4 ms is about 100000 CPU cycles given that CPU is running at 275Mhz (which is the lowest cpufreq). Long time ago, I run pwc webcam with SIS Vortex86 at 200Mhz It worked fine. I would not say that it is extraordinary value. Do you think that somewhere CPU cycles are wasted globally for some reason? > >> At the same time musb_host doesn't utilize AutoReq here. I think many >> other USB host controllers (OHCI?) just rely on hardware to send IN >> packets in time. > > For Isoch transfer, MUSB has to be programmed for every transaction, but > urb callback takes too long with spinlock held, which cause MUSB misses > issuing IN tokens. > > Regards, > -Bin. > -- With best regards, Matwey V. Kornilov. Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia 119234, Moscow, Universitetsky pr-k 13, +7 (495) 9392382 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
