RE: [PATCH v3 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()

[Anurag Kumar Vulisha]

Hi Thinh,

>On 9/7/2018 6:14 AM, Anurag Kumar Vulisha wrote:
>> Availability of TRB's are calculated using dwc3_calc_trbs_left(),
>> which determines available TRB's based on the HWO bit set in a TRB.
>>
>> __dwc3_prepare_one_trb() is called with a TRB which needs to be
>> prepared for transfer. This __dwc3_prepare_one_trb() calls
>> dwc3_calc_trbs_left() to determine total available TRBs and set IOC
>> bit if the total available TRBs are zero. Since the present working
>> TRB(which is passed as an argument to __dwc3_prepare_one_trb() )
>> doesn't have HWO bit already set, there are chances where
>> dwc3_calc_trbs_left() wrongly calculates this present working TRB as
>> free(since the HWO bit is not yet set). This could be a problem. This
>> patch correct this issue by setting HWO bit before calling
>> dwc3_calc_trbs_left()
>
>Please revise your commit message to reflect your new change.
>
Sorry , I missed updating the commit message. Thanks a lot for pointing, will fix it in next version

Thanks,
Anurag Kumar Vulisha
>>
>> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xxxxxxxxxx>
>> Reviewed-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx>
>> ---
>>  Changes in v3:
>> 	1. Corrected the logic for setting HWO bit as suggested by "Thinh Nguyen"
>>
>>  Changes in v2:
>> 	1. Changed the commit message
>> ---
>>  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 032ea7d..8a1622b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -911,8 +911,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep,
>struct dwc3_trb *trb,
>>  	struct usb_gadget	*gadget = &dwc->gadget;
>>  	enum usb_device_speed	speed = gadget->speed;
>>
>> -	dwc3_ep_inc_enq(dep);
>> -
>>  	trb->size = DWC3_TRB_SIZE_LENGTH(length);
>>  	trb->bpl = lower_32_bits(dma);
>>  	trb->bph = upper_32_bits(dma);
>> @@ -991,7 +989,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep,
>struct dwc3_trb *trb,
>>  	}
>>
>>  	if ((!no_interrupt && !chain) ||
>> -			(dwc3_calc_trbs_left(dep) == 0))
>> +			(dwc3_calc_trbs_left(dep) == 1))
>>  		trb->ctrl |= DWC3_TRB_CTRL_IOC;
>>
>>  	if (chain)
>> @@ -1002,6 +1000,8 @@ static void __dwc3_prepare_one_trb(struct
>> dwc3_ep *dep, struct dwc3_trb *trb,
>>
>>  	trb->ctrl |= DWC3_TRB_CTRL_HWO;
>>
>> +	dwc3_ep_inc_enq(dep);
>> +
>>  	trace_dwc3_prepare_trb(dep, trb);
>>  }
>>
>
>BR,
>Thinh