[PATCH 3/3] usb: ci_udc: verify all dtds are inactive before completing request
Ye Li
ye.li at oss.nxp.com
Fri Jul 10 11:28:09 CEST 2026
Hi Mattijs,
On 6/16/2026 5:30 PM, Mattijs Korpershoek wrote:
> Hi Ye,
>
> Thank you for the patch.
>
> On Fri, May 22, 2026 at 15:55, Ye Li <ye.li at nxp.com> wrote:
>
>> From: Jason He <jason.he_1 at nxp.com>
>>
>> According to device mode spec, the ACTIVE status field of dtds should
>> be check to determine whether the transfers completed successfully.
>> However, this is not implemented in handle_ep_complete.
>> When two EPs are enabled and transferring, EPa requests with multiple dtds
>> and EPb request with one dtd. Irq is triggred on EPb. The udc_irq handler
>> finds both EPb's and EPa's ENDPTCOMPLETE=1 while not all of EPa's dtds
>> have been completed. Because ACTIVE status is not checked, this case
>> causes crash in ci_udc driver.
>>
>> Signed-off-by: Jason He <jason.he_1 at nxp.com>
>> Signed-off-by: Ye Li <ye.li at nxp.com>
>> ---
>> drivers/usb/gadget/ci_udc.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
>> index 0baad83ef90..53796887dac 100644
>> --- a/drivers/usb/gadget/ci_udc.c
>> +++ b/drivers/usb/gadget/ci_udc.c
>> @@ -733,6 +733,17 @@ static void handle_ep_complete(struct ci_ep *ci_ep)
>> ci_invalidate_qtd(num);
>> ci_req = list_first_entry(&ci_ep->queue, struct ci_req, queue);
>>
>> + /* Check all dtd are completed, otherwise return for next irq process */
>> + next_td = item;
>> + for (j = 0; j < ci_req->dtd_count; j++) {
>> + ci_invalidate_td(next_td);
>> + if (next_td->info & INFO_ACTIVE)
>> + return;
>> + if (j != ci_req->dtd_count - 1)
>> + next_td = (struct ept_queue_item *)(unsigned long)
>> + next_td->next;
>> + }
>
> A very similar loop (that walks all the dtds) is just below:
>
>> +
>> next_td = item;
>> len = 0;
>> for (j = 0; j < ci_req->dtd_count; j++) {
>
> Can't we merge both loops together?
>
This loop will free each dtd. If the ACTIVE flag is set in the middle of
loop, previous dtds should NOT be freed since this transfer is not done.
That's why we add a loop to check ACTIVE prior this loop.
If merge two loops together, another list to temporally record dtds for
free is needed. Then have to traverse the list to free each one when no
ACTIVE found. This traversing will add a loop too.
Best regards,
Ye Li
>> --
>> 2.37.1
More information about the U-Boot
mailing list