diff mbox series

[v2,2/2] usb: dwc3: gadget: Report isoc scheduled frame number

Message ID 84f7883818b412fcb42b6705d4d6ad14adfd0eb9.1541645048.git.thinhn@synopsys.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] usb: gadget: Add start_frame to usb_request | expand

Commit Message

Thinh Nguyen Nov. 8, 2018, 2:47 a.m. UTC
Report the scheduled frame number of an isochronous request.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Change in v2:
 - Capture frame number at request cleanup

 drivers/usb/dwc3/gadget.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Felipe Balbi Nov. 8, 2018, 7:08 a.m. UTC | #1
Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
> Report the scheduled frame number of an isochronous request.
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
> Change in v2:
>  - Capture frame number at request cleanup
>
>  drivers/usb/dwc3/gadget.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 679c12e14522..5e5e799699de 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2345,6 +2345,10 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
>  
>  	req->request.actual = req->request.length - req->remaining;
>  
> +	/* Report scheduled frame number for isoc transfers */
> +	if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
> +		req->request.start_frame = dep->frame_number;

yeah, this is really wrong. Since you're setting start_frame when
cleaning up the request, this means that dep->frame_number was already
updated due to XferComplete/XferInProgress. This means thart start_frame
will report frame number of completion event, not start. So your
debugging will be misleading.
Thinh Nguyen Nov. 8, 2018, 7:40 p.m. UTC | #2
Hi,

On 11/7/2018 11:08 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
>> Report the scheduled frame number of an isochronous request.
>>
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>> Change in v2:
>>  - Capture frame number at request cleanup
>>
>>  drivers/usb/dwc3/gadget.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 679c12e14522..5e5e799699de 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2345,6 +2345,10 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
>>  
>>  	req->request.actual = req->request.length - req->remaining;
>>  
>> +	/* Report scheduled frame number for isoc transfers */
>> +	if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
>> +		req->request.start_frame = dep->frame_number;
> yeah, this is really wrong. Since you're setting start_frame when
> cleaning up the request, this means that dep->frame_number was already
> updated due to XferComplete/XferInProgress. This means thart start_frame
> will report frame number of completion event, not start. So your
> debugging will be misleading.
>
You're right. I'll make the fix.

Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 679c12e14522..5e5e799699de 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2345,6 +2345,10 @@  static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
 
 	req->request.actual = req->request.length - req->remaining;
 
+	/* Report scheduled frame number for isoc transfers */
+	if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
+		req->request.start_frame = dep->frame_number;
+
 	if (!dwc3_gadget_ep_request_completed(req) &&
 			req->num_pending_sgs) {
 		__dwc3_gadget_kick_transfer(dep);