[2/3] usb: dwc3: gadget: Don't skip updating remaining data

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Hi Felipe,

On 7/29/2018 11:08 PM, Felipe Balbi wrote:
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> DWC3 must check for the BUFSIZ and update the req->remaining
>> regardless of transfer alignment. Returning early from transfer OUT
>> unalignment will skip updating the req->remaining.
>>
>> Fixes: c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
>> ---
>>  drivers/usb/dwc3/gadget.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 032ea7d709ba..a5b8387a37ba 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2246,6 +2246,9 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>  	if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
>>  		trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
>>  
>> +	count = trb->size & DWC3_TRB_SIZE_MASK;
>> +	req->remaining += count;
>> +
>>  	/*
>>  	 * If we're dealing with unaligned size OUT transfer, we will be left
>>  	 * with one TRB pending in the ring. We need to manually clear HWO bit
>> @@ -2256,9 +2259,6 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>>  		return 1;
>>  	}
>>  
>> -	count = trb->size & DWC3_TRB_SIZE_MASK;
>> -	req->remaining += count;
>> -
> this shouldn't be necessary, actually. The remaining TRB which was added
> to fix alignment issues, is simply because DWC3 doesn't like OUT
> transfers whose size are not aligned to wMaxPacketSize. We *know* we
> have e.g. 511 bytes to receive from host, but dwc3 wants me to write
> 1024 bytes on the TRB (for superspeed), so I need to add this chained
> TRB for the remaining 513 bytes.
>
> I don't expect host to send us more than 511 bytes, if it does, the host
> is faulty and I don't want to operate on those bytes anyway.
>

Right. But that's not the issue here. I realized that I did not describe
the issue fully...

The issue here is the reporting of the actual number of bytes
transferred. For isoc OUT transfers, the last TRB of the Buffer
Descriptor will be retired with HWO = 0 (dwc_usb3 and dwc_usb31
programming guide 4.3.7). So, the check for unaligned && HWO will be
false and will not return early. As a result, req->remaining will still
be updated with the BUFSIZ count, and the remaining will be 513+ in your
example. The 'actual' bytes written calculation will be wrong (e.g 511 -
513 = -2).

We can't rely on the current check to determine whether it's the last
TRB. My current solution is to update req->remaining for all TRB
completions. However, we have to also update the 'actual' bytes
calculation for OUT transfers.

Thanks,
Thinh
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html