RE: Query: DWC3: Isoc ep_queue when xinprogress

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Paul Zimmerman
> Sent: Wednesday, March 28, 2012 3:55 PM
> 
> > From: Paul Zimmerman
> > Sent: Wednesday, March 28, 2012 1:52 PM
> > So the Update Transfer command is required.

< snip >

> > 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;
> 

Hi Pratyush,

I tried my "isoc support for gadget zero and usbtest" patches with
the dwc3 driver with the above patch, and the isoc IN tests from
usbtest still didn't work. I discovered that I needed to queue up
8 requests in source_sink_start_ep(), or else the transfer would
stop after the first 4 packets were sent.

So I made this change on top of my patch to source_sink_start_ep()
in f_sourcesink.c:

 	int			status;
 	int			i, size, status;
 
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < 8; i++) {
 		if (is_iso) {
 			switch (speed) {
 			case USB_SPEED_SUPER:

and now the isoc IN tests from usbtest are working.

This makes sense, because the dwc3 driver is only setting the IOC
bit in every 8th TRB, so if less than 8 TRBs are queued up
initially, an interrupt event will never be generated, so the
gadget driver will never be notified so it can queue up the next
requests.

So if you change your gadget driver to queue up at least 8
requests, I think it will work.

-- 
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux