Re: [PATCH] USB: DWC3: Fix stop active transfer

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

 



On Tue, Jun 12, 2012 at 03:58:56PM +0530, Pratyush Anand wrote:
> On 6/12/2012 3:54 PM, Pratyush ANAND wrote:
> >When ever active transfer is stopped, callback for all the queued
> >request must be called.
> >
> >Signed-off-by: Pratyush Anand<pratyush.anand@xxxxxx>
> >---
> >  drivers/usb/dwc3/gadget.c |   62 ++++++++++++++++++++++++++-------------------
> >  1 files changed, 36 insertions(+), 26 deletions(-)
> >
> >diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >index cc5423f..e8800bd 100644
> >--- a/drivers/usb/dwc3/gadget.c
> >+++ b/drivers/usb/dwc3/gadget.c
> >@@ -551,37 +551,47 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> >  }
> >
> >  static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum);
> >-static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
> >+static void __dwc3_remove_queued_request(struct dwc3 *dwc,
> >+		struct dwc3_ep *dep, int status)
> >  {
> >  	struct dwc3_request		*req;
> >
> >-	if (!list_empty(&dep->req_queued)) {
> >-		dwc3_stop_active_transfer(dwc, dep->number);
> >+	dwc3_stop_active_transfer(dwc, dep->number);
> >
> >-		/*
> >-		 * NOTICE: We are violating what the Databook says about the
> >-		 * EndTransfer command. Ideally we would _always_ wait for the
> >-		 * EndTransfer Command Completion IRQ, but that's causing too
> >-		 * much trouble synchronizing between us and gadget driver.
> >-		 *
> >-		 * We have discussed this with the IP Provider and it was
> >-		 * suggested to giveback all requests here, but give HW some
> >-		 * extra time to synchronize with the interconnect. We're using
> >-		 * an arbitraty 100us delay for that.
> >-		 *
> >-		 * Note also that a similar handling was tested by Synopsys
> >-		 * (thanks a lot Paul) and nothing bad has come out of it.
> >-		 * In short, what we're doing is:
> >-		 *
> >-		 * - Issue EndTransfer WITH CMDIOC bit set
> >-		 * - Wait 100us
> >-		 * - giveback all requests to gadget driver
> >-		 */
> >-		udelay(100);
> >+	/*
> >+	 * NOTICE: We are violating what the Databook says about the
> >+	 * EndTransfer command. Ideally we would _always_ wait for the
> >+	 * EndTransfer Command Completion IRQ, but that's causing too
> >+	 * much trouble synchronizing between us and gadget driver.
> >+	 *
> >+	 * We have discussed this with the IP Provider and it was
> >+	 * suggested to giveback all requests here, but give HW some
> >+	 * extra time to synchronize with the interconnect. We're using
> >+	 * an arbitraty 100us delay for that.
> >+	 *
> >+	 * Note also that a similar handling was tested by Synopsys
> >+	 * (thanks a lot Paul) and nothing bad has come out of it.
> >+	 * In short, what we're doing is:
> >+	 *
> >+	 * - Issue EndTransfer WITH CMDIOC bit set
> >+	 * - Wait 100us
> >+	 * - giveback all requests to gadget driver
> >+	 */
> >+	udelay(100);
> >
> >+	while (!list_empty(&dep->req_queued)) {
> >  		req = next_request(&dep->req_queued);
> >-		dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
> >+
> >+		dwc3_gadget_giveback(dep, req, status);
> >  	}
> >+}
> >+
> >+static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
> >+{
> >+	struct dwc3_request		*req;
> >+
> >+	if (!list_empty(&dep->req_queued))
> >+		__dwc3_remove_queued_request(dwc, dep, -ESHUTDOWN);
> >
> >  	while (!list_empty(&dep->request_list)) {
> >  		req = next_request(&dep->request_list);
> >@@ -1202,8 +1212,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> >  				break;
> >  		}
> >  		if (r == req) {
> >-			/* wait until it is processed */
> >-			dwc3_stop_active_transfer(dwc, dep->number);
> >+			__dwc3_remove_queued_request(dwc, dep, -ECONNRESET);
> >+
> 
> Probably, its wrong. I need to call callback only for dequeued
> request here, not for all.
> In the disable, it should be for all.

correct. Gadget driver should dequeue each request, but the fact is that
stop transfer command should be sent only once, no matter how many TRBs
we queued together, so that'll be a bit tricky, I guess.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


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

  Powered by Linux