Hi,
On 5/30/2018 11:49 PM, Felipe Balbi wrote:
> Paul Zimmerman <pauldzim@xxxxxxxxx> writes:
>
>> Hi Felipe,
>>
>> Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> writes:
>>
>> < snip >
>>
>>> thinking about this a little more. This extra list_empty() check is
>>> not wrong at all :-) I've amended this series with the 3 patches
>>> below. I'll resend the series once I've given more time for people to
>>> test. Patches have been updated to the branch on kernel.org as well,
>>> btw.
>> < snip >
>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 9d4dc8bed644..9cf89f3cf203 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1233,6 +1233,8 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>>> return DWC3_DSTS_SOFFN(reg);
>>> }
>>>
>>> +#define DWC3_ALIGN_FRAME(d) (((d)->frame_number + (d)->interval) & ~((d)->interval - 1))
>>> +
>>> static void __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>>> {
>>> if (list_empty(&dep->pending_list)) {
>>> @@ -1242,11 +1244,7 @@ static void __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>>> return;
>>> }
>>>
>>> - /*
>>> - * Schedule the first trb for one interval in the future or at
>>> - * least 4 microframes.
>>> - */
>>> - dep->frame_number += max_t(u32, 4, dep->interval);
>>> + dep->frame_number = DWC3_ALIGN_FRAME(dep);
>>> __dwc3_gadget_kick_transfer(dep);
>>> }
>> Pretty sure this could cause problems. Instead of starting at least 4 uframes
>> in the future, this will now try to start at the next aligned uframe. If
>> dep->interval is very small (say 1) and we are almost at the end of the
>> current uframe, there might not be enough time?
> perhaps, but I haven't seen that happen yet. Guess I'll get to this when
> I see such a problem.
>
I did some quick tests against DWC_usb3 controller v3.10a in high-speed
and super-speed. We'll start to run into issue when if DWC3 needs to
prepare 64+ TRBs after XferNotReady for isoc transfers with service
interval of 1 uframe (start_transfer command will fail with bus-expiry).
It may be better to calculate for the starting future uframe in a proper
way.
BR,
Thinh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
