Hi,
Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>> We only care to resume transfer for SG because the request maybe
>>> partially completed. dwc3_gadget_ep_request_completed() doesn't check
>>> that of a request, at least not fully.
>>>
>>> 1) It doesn't account for OUT direction.
>>> 2) It doesn't account for isoc. For isoc, a request maybe completed with
>>> partial data.
>> I would rather fix the function for these cases instead of removing it
>> completely. While at that, also move the req->num_pending_sgs check
>> inside dwc3_gadget_ep_request_completed()
>>
>
> If we want to keep this function, the only thing this function does is
> to check req->num_pending_sgs. We'd only resume the request because
> there are pending TRBs from SG not completed yet. If all the TRBs of a
> request are completed, regardless if all the data are received/sent, we
> don't queue them again. Do you still want to have this function?
The function gives a name to a very specific "concept", that of a
completed request. You can see that even today, the function is super
simple: OUT direction is always completed, IN direction is completed
when actual == length, we're just missing the pending_sgs check. So
something like the hunks below.
One thing I don't get from your patch is why you're completely removing
the function and why isn't req->direction and actual == length not
needed anymore. Could you explain?
hunks:
@@ -2482,7 +2482,8 @@ static int dwc3_gadget_ep_reclaim_trb_linear(struct dwc3_ep *dep,
event, status, false);
}
-static bool dwc3_gadget_ep_request_completed(struct dwc3_request *req)
+static bool dwc3_gadget_ep_request_completed(struct dwc3_ep *dep,
+ struct dwc3_request *req)
{
/*
* For OUT direction, host may send less than the setup
@@ -2491,6 +2492,16 @@ static bool dwc3_gadget_ep_request_completed(struct dwc3_request *req)
if (!req->direction)
return true;
+ /*
+ * If there are pending scatterlist entries, we should
+ * continue processing them.
+ */
+ if (req->num_pending_sgs)
+ return false;
+
+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
+ do_something();
+
return req->request.actual == req->request.length;
}
@@ -2515,8 +2526,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(dep, req))
__dwc3_gadget_kick_transfer(dep);
goto out;
}
--
balbi
Attachment:
signature.asc
Description: PGP signature
