Hi Lothar, On Wed, Jul 15, 2026 at 07:23, Lothar Waßmann <[email protected]> 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 <[email protected]> >> Sent: Thursday, July 9, 2026 17:15 >> To: Lothar Waßmann; [email protected] >> 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 <[email protected]> >> 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 <[email protected]> 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
