> From: Paul Zimmerman > Sent: Wednesday, March 28, 2012 1:52 PM > > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Felipe Balbi > > Sent: Monday, March 26, 2012 5:56 AM > > > > On Mon, Mar 26, 2012 at 05:37:30PM +0530, Pratyush Anand wrote: > > > Hello Felipe, > > > > > > I see one scenario where I do not find dwc3 driver working correctly > > > for isoc IN. I just wanted to clarify if my understanding is correct. > > > > > > 1. Gadget driver (in my case uac2 audio driver with some > > > modifications) submits 16 request using ep_queue in the beginning. > > > 2. As soon as host sends first IN token , xfernotready interrupt is received. > > > 3. dwc3_gadget_start_isoc starts isoc transfer and programs IOC bit in > > > each 8th trb (DWC3_TRB_NUM / 4). > > > 4. So I receive two xferinprogress interrupt against submitted (16) requests. > > > 5. Upon each xferinprogress interrupt reception, complete is called (8 > > > times) .A new request is submitted (ep_queue) each time > > > when complete is called. > > > > > > Till this point, I find everything correct. > > > > > > Now I do not see anyway to start isoc transfer once again for newly > > > added requests. dwc3_gadget_start_isoc is called only if > > > xfernotready is received. But If I see section 9.3.5 and 9.3.6 of dw > > > specs, then I should not expect a xfernotready interrupt, rather a > > > xferinprogress interrupt in the above scenario. > > > > > > So I must add a mechanism to call dwc3_gadget_start_isoc , also when > > > xferinprogress is received. > > > > > > Please let me know, if I am not correct. > > > > In case of ISOC transfers, we consume requests in a ring list fashion. > > There's a link TRB which points back to the beginning of the TRB array. > > When you queue a request again, the same TRB is used for it so there's > > no need to restart the transfer. > > > > Now, that hasn't been stressed enough and if you found a problem with > > it, please send us patches ;-) They are much welcome. > > Hi Felipe, > > Actually, Pratyush is correct here. If you look at the paragraph > "Adding intervals to a transfer" in the "Isochronous Transfer > Programming Model" section of the databook, it describes the circular > TRB list, and says this: > > Assume TRB 1 represents the data for the first interval and > TRB 2 represents the data for the second interval. Because > IOC=1, when the core completes the first interval, it will > issue an XferInProgress event which indicates to software > that TRB 1 can be analyzed to retrieve the results from > interval 1. TRB 1 can the be re-used to represent the data > for the third interval by setting HWO=1 and issuing an > Update Transfer command. > > So the Update Transfer command is required. > > The reason for this is that the core will cache a certain number > of TRBs in its internal memory. If it already has a TRB in its > cache, then even though the software updates that TRB and sets > its HWO bit, the core won't see that unless an Update Transfer > command is issued to tell it to flush its TRB cache and reload > it. > > Now, it's possible that if the TRB list is big enough, and > software stays far enough ahead of the hardware, this situation > won't arise. But I don't think you can depend on that always > being the case. So for reliable operation, you should issue an > Update Transfer command whenever you update one or more TRBs > that are part of an active transfer. Something like the attached patch, I think. Warning: this has been compile-tested only! Pratyush, maybe you can use this as a starting point? diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 5255fe9..6529a81 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -930,10 +930,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep, u16 cmd_param, } dep->flags |= DWC3_EP_BUSY; - dep->res_trans_idx = dwc3_gadget_ep_get_transfer_index(dwc, - dep->number); - WARN_ON_ONCE(!dep->res_trans_idx); + if (start_new) { + dep->res_trans_idx = dwc3_gadget_ep_get_transfer_index(dwc, + dep->number); + WARN_ON_ONCE(!dep->res_trans_idx); + } return 0; } @@ -979,16 +981,22 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req) * until the next request is queued. The following * code is handling exactly that. */ - if (dep->flags & DWC3_EP_PENDING_REQUEST) { + if ((dep->flags & DWC3_EP_PENDING_REQUEST) || + (usb_endpoint_xfer_isoc(dep->desc) && + (dep->flags & DWC3_EP_BUSY))) { int ret; - int start_trans; + int start_trans = 1; + int trans_idx = dep->res_trans_idx; - start_trans = 1; if (usb_endpoint_xfer_isoc(dep->desc) && - (dep->flags & DWC3_EP_BUSY)) + (dep->flags & DWC3_EP_BUSY)) { start_trans = 0; + WARN_ON_ONCE(!trans_idx); + } else { + trans_idx = 0; + } - ret = __dwc3_gadget_kick_transfer(dep, 0, start_trans); + ret = __dwc3_gadget_kick_transfer(dep, trans_idx, start_trans); if (ret && ret != -EBUSY) { struct dwc3 *dwc = dep->dwc; -- Paul -- 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
