Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: > On Thu, 13 Aug 2009, Vernon Sauder wrote: > >> Robert, > >> From our investigation, one issue we see is that the host sends IN >> requests on endpoint 2 (and gets NAK'd) for 100ms before we send a >> stall. Digging through the code we found this to be caused by a >> spin_lock loop. Here is the trail we followed: >> >> In file_storage.c:finish_reply(), for IN eps, we see it fall to the last >> else which sends the last bit and then halts the ep. In >> halt_bulk_in_endpoint(), it loops until usb_ep_set_halt() does not >> return -EAGIN, sleeping 100ms each cycle. In pxa_ep_set_halt(), the ep >> is locked, and then if (ep->dir_in && (ep_is_full(ep) || >> !list_empty(&ep->queue))), it returns -EAGAIN. However, finish_reply() >> just queued a bit so there will be something in the queue which >> guarantees that it will take at least once through the loop until the ep >> is halted. This causes a 100ms pause each time the IN ep needs to be >> halted (which we see frequently while connecting). Even if we lower the >> timeout to 10ms, the connection time doesn't get any better. Well, according to usb_ep_set_halt() specification, the UDC may answer with -EAGAIN as long as there are bytes queued for transmission. I think pxa27x_udc respects the specification here. And when you say finish_reply() just queued "a bit", would you point me to a specific line of code please ? >> I believe that the pxa27x should be able to do stalls correctly but it >> must not. It seems like the stalls are messing up the connection >> somehow. If so, would you like a patch to add the condition >> gadget_is_pxa27x() in check_parameters() so it sets can_stall to zero >> like gadget_is_at91()? My guess is that queuing a transmission just before stalling an endpoint is somehow a strange behaviour considering usb_ep_set_halt() specification. I see no improvement in disabling "stall endpoint" function here. > Until you can track down the problem more thoroughly, I don't think > this is appropriate. However I will accept a patch to reduce the sleep > time to 10 ms. Yes, neither do I. I don't think disabling stalling is good. Maybe tracking down a bit further in file_storage.c will give you the right clue. I'll make some tests at home, to crosscheck. I don't remember such a big time when I had tested the udc. Cheers. -- Robert -- 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
