Re: Regression in f_fastboot due to commit 6a92e9827650 ("usb: ci_udc: Check ci_ep->desc before use")

[Mattijs Korpershoek via U-Boot <u-boot@lists.u-boot-project.org>]

Hi Lothar,

On Wed, Jul 15, 2026 at 07:23, Lothar Waßmann <LW@KARO-electronics.de> wrote:

> Hi,
>
> On Fri, 10 Jul 2026 21:27:48 +0000 Petr Beneš wrote:
>> Hello Lothar and Mattijs,
>> 
>> I hope web outlook doesn't render my response unreadable.
>> ________________________________________
>> From: Mattijs Korpershoek <mkorpershoek@kernel.org>
>> Sent: Thursday, July 9, 2026 17:15
>> To: Lothar Waßmann; u-boot@lists.denx.de
>> Cc: Petr Beneš
>> Subject: Re: Regression in f_fastboot due to commit 6a92e9827650 ("usb: ci_udc: Check ci_ep->desc before use")
>> 
>> Hi Lothar,
>> 
>> On Wed, Jul 08, 2026 at 07:12, Lothar Waßmann <LW@KARO-electronics.de> wrote:
>> 
>> > Hi,
>> >
>> > commit 6a92e9827650 ("usb: ci_udc: Check ci_ep->desc before use") has
>> > the side effect that an empty buffer is being passed on to
>> > rx_handler_command() in drivers/usb/gadget/f_fastboot.c upon
>> > termination of a fastboot session which leads to a
>> > 'command  not recognized' error message being printed on the console.  
>> 
>> Thank you for reporting this.
>> 
>> >
>> > Obviously without the commit the offending USB request had been silently
>> > dropped but is now delivered to the upper levels.  
>> 
>> Well, it was not silently dropped in case of the ethernet gadget, it was silently corrupting memory.
>> 
>> >
>> > I'm not sure what the correct fix would be.
>> > Should the fastboot driver ignore the empty command buffer or should
>> > the "Moreover, the patch gets rid of possible outstanding requests
>> > if the endpoint's state changes to disabled." part of the commit be
>> > reverted?  
>> 
>> Hmm, maybe I got it wrong. The endpoint got disabled, so what should happen with requests?
>> 
>> linux/usb/gadget.h reads:
>> 
>>  * @complete: Function called when request completes, so this request and
>>  *      its buffer may be re-used.
>>  *      Reads terminate with a short packet, or when the buffer fills,
>>  *      whichever comes first.  When writes terminate, some data bytes
>>  *      will usually still be in flight (often in a hardware fifo).
>>  *      Errors (for reads or writes) stop the queue from advancing
>>  *      until the completion function returns, so that any transfers
>>  *      invalidated by the error may first be dequeued.
>> 
>> Petr, did you observe similar behaviour with the ums gadget?
>> 
>> I didn't see any wrongdoing of the ethernet gadget.
>> >
>> >
> I found a solution for the problem. The ci_udc driver checks the req
> status for -EINPROGRESS in several places, but does never initialize the
> status to that value.
>
> This patch alleviates the problem:
> Index: a/drivers/usb/gadget/ci_udc.c
> ===================================================================
> --- a/drivers/usb/gadget/ci_udc.c
> +++ b/drivers/usb/gadget/ci_udc.c
> @@ -530,6 +530,9 @@ static void ci_ep_submit_next_request(st
>  	head = ci_get_qh(num, in);
>  
>  	ci_req = list_first_entry(&ci_ep->queue, struct ci_req, queue);
> +
> +	ci_req->req.status = -EINPROGRESS;
> +
>  	len = ci_req->req.length;
>  
>  	head->next = (unsigned long)item;
> @@ -753,6 +756,9 @@ static void handle_ep_complete(struct ci
>  		ci_ep_submit_next_request(ci_ep);
>  
>  	ci_req->req.actual = ci_req->req.length - len;
> +	if (ci_req->req.status == -EINPROGRESS)
> +		ci_req->req.status = 0;
> +
>  	ci_debounce(ci_req, in);
>  
>  	DBG("ept%d %s req %p, complete %x\n",
>
> This is similar to how the Linux driver behaves.
>
> I tested with fastboot and mass-storage gadget.

Thank you for investigating this further.

Ye Li <ye.li@nxp.com> has send a similar patch which just got applied here:

      https://git.u-boot-project.org/u-boot/custodians/u-boot-dfu/-/commit/b79147bbd47cd2d50836dc0a57376f4820663637

Can you please check if that works for you as well?

>
>
> Lothar Waßmann