[PATCH v2 1/2] usb: dwc3: gadget: Fix request completion check

[PATCH v2 1/2] usb: dwc3: gadget: Fix request completion check

[Thinh Nguyen]

A request may not be completed because not all the TRBs are prepared for
it. This happens when we run out of available TRBs. When some TRBs are
completed, the driver needs to prepare the rest of the TRBs for the
request. The check dwc3_gadget_ep_request_completed() shouldn't be
checking the amount of data received but rather the number of pending
TRBs. Revise this request completion check.

Cc: stable@vger.kernel.org
Fixes: e0c42ce590fe ("usb: dwc3: gadget: simplify IOC handling")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Changes in v2:
 - Add Cc: stable tag

 drivers/usb/dwc3/gadget.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1a4fc03742aa..c45853b14cff 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2550,14 +2550,7 @@ static int dwc3_gadget_ep_reclaim_trb_linear(struct dwc3_ep *dep,
 
 static bool dwc3_gadget_ep_request_completed(struct dwc3_request *req)
 {
-	/*
-	 * For OUT direction, host may send less than the setup
-	 * length. Return true for all OUT requests.
-	 */
-	if (!req->direction)
-		return true;
-
-	return req->request.actual == req->request.length;
+	return req->num_pending_sgs == 0;
 }
 
 static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
@@ -2581,8 +2574,7 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
 
 	req->request.actual = req->request.length - req->remaining;
 
-	if (!dwc3_gadget_ep_request_completed(req) ||
-			req->num_pending_sgs) {
+	if (!dwc3_gadget_ep_request_completed(req)) {
 		__dwc3_gadget_kick_transfer(dep);
 		goto out;
 	}
-- 
2.11.0

[PATCH v2 2/2] usb: dwc3: gadget: Continue to process pending requests

[Thinh Nguyen]

If there are still pending requests because no TRB was available,
prepare more when started requests are completed.

Introduce dwc3_gadget_ep_should_continue() to check for incomplete and
pending requests to resume updating new TRBs to the controller's TRB
cache.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Changes in v2:
 - None

 drivers/usb/dwc3/gadget.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c45853b14cff..3c4024d34434 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2574,10 +2574,8 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
 
 	req->request.actual = req->request.length - req->remaining;
 
-	if (!dwc3_gadget_ep_request_completed(req)) {
-		__dwc3_gadget_kick_transfer(dep);
+	if (!dwc3_gadget_ep_request_completed(req))
 		goto out;
-	}
 
 	dwc3_gadget_giveback(dep, req, status);
 
@@ -2601,6 +2599,24 @@ static void dwc3_gadget_ep_cleanup_completed_requests(struct dwc3_ep *dep,
 	}
 }
 
+static bool dwc3_gadget_ep_should_continue(struct dwc3_ep *dep)
+{
+	struct dwc3_request	*req;
+
+	if (!list_empty(&dep->pending_list))
+		return true;
+
+	/*
+	 * We only need to check the first entry of the started list. We can
+	 * assume the completed requests are removed from the started list.
+	 */
+	req = next_request(&dep->started_list);
+	if (!req)
+		return false;
+
+	return !dwc3_gadget_ep_request_completed(req);
+}
+
 static void dwc3_gadget_endpoint_frame_from_event(struct dwc3_ep *dep,
 		const struct dwc3_event_depevt *event)
 {
@@ -2630,6 +2646,8 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
 
 	if (stop)
 		dwc3_stop_active_transfer(dep, true, true);
+	else if (dwc3_gadget_ep_should_continue(dep))
+		__dwc3_gadget_kick_transfer(dep);
 
 	/*
 	 * WORKAROUND: This is the 2nd half of U1/U2 -> U0 workaround.
-- 
2.11.0

Re: [PATCH v2 1/2] usb: dwc3: gadget: Fix request completion check

[Thinh Nguyen]

Hi Felipe,

Thinh Nguyen wrote:
> A request may not be completed because not all the TRBs are prepared for
> it. This happens when we run out of available TRBs. When some TRBs are
> completed, the driver needs to prepare the rest of the TRBs for the
> request. The check dwc3_gadget_ep_request_completed() shouldn't be
> checking the amount of data received but rather the number of pending
> TRBs. Revise this request completion check.
>
> Cc: stable@vger.kernel.org
> Fixes: e0c42ce590fe ("usb: dwc3: gadget: simplify IOC handling")
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
> Changes in v2:
>   - Add Cc: stable tag
>
>   drivers/usb/dwc3/gadget.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 1a4fc03742aa..c45853b14cff 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2550,14 +2550,7 @@ static int dwc3_gadget_ep_reclaim_trb_linear(struct dwc3_ep *dep,
>   
>   static bool dwc3_gadget_ep_request_completed(struct dwc3_request *req)
>   {
> -	/*
> -	 * For OUT direction, host may send less than the setup
> -	 * length. Return true for all OUT requests.
> -	 */
> -	if (!req->direction)
> -		return true;
> -
> -	return req->request.actual == req->request.length;
> +	return req->num_pending_sgs == 0;
>   }
>   
>   static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
> @@ -2581,8 +2574,7 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
>   
>   	req->request.actual = req->request.length - req->remaining;
>   
> -	if (!dwc3_gadget_ep_request_completed(req) ||
> -			req->num_pending_sgs) {
> +	if (!dwc3_gadget_ep_request_completed(req)) {
>   		__dwc3_gadget_kick_transfer(dep);
>   		goto out;
>   	}

Since you'll be picking this up for the rc cycle for your fix patches, 
should I split this series to resend and wait for this patch to be 
merged first before I resend the patch 2/2?
Let me know how you'd like to proceed.

Thanks,
Thinh

Re: [PATCH v2 1/2] usb: dwc3: gadget: Fix request completion check

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]


Hi Thinh,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> Hi Felipe,
>
> Thinh Nguyen wrote:
>> A request may not be completed because not all the TRBs are prepared for
>> it. This happens when we run out of available TRBs. When some TRBs are
>> completed, the driver needs to prepare the rest of the TRBs for the
>> request. The check dwc3_gadget_ep_request_completed() shouldn't be
>> checking the amount of data received but rather the number of pending
>> TRBs. Revise this request completion check.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: e0c42ce590fe ("usb: dwc3: gadget: simplify IOC handling")
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>> Changes in v2:
>>   - Add Cc: stable tag
>>
>>   drivers/usb/dwc3/gadget.c | 12 ++----------
>>   1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1a4fc03742aa..c45853b14cff 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2550,14 +2550,7 @@ static int dwc3_gadget_ep_reclaim_trb_linear(struct dwc3_ep *dep,
>>   
>>   static bool dwc3_gadget_ep_request_completed(struct dwc3_request *req)
>>   {
>> -	/*
>> -	 * For OUT direction, host may send less than the setup
>> -	 * length. Return true for all OUT requests.
>> -	 */
>> -	if (!req->direction)
>> -		return true;
>> -
>> -	return req->request.actual == req->request.length;
>> +	return req->num_pending_sgs == 0;
>>   }
>>   
>>   static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
>> @@ -2581,8 +2574,7 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
>>   
>>   	req->request.actual = req->request.length - req->remaining;
>>   
>> -	if (!dwc3_gadget_ep_request_completed(req) ||
>> -			req->num_pending_sgs) {
>> +	if (!dwc3_gadget_ep_request_completed(req)) {
>>   		__dwc3_gadget_kick_transfer(dep);
>>   		goto out;
>>   	}
>
> Since you'll be picking this up for the rc cycle for your fix patches, 
> should I split this series to resend and wait for this patch to be 
> merged first before I resend the patch 2/2?
> Let me know how you'd like to proceed.

That's okay. Usually it's better to have the series split, but since
it's only two patches, I can manage :-) I'll just leave patch 2 unread
:-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]