On Wed, May 31, 2023, Elson Serrao wrote: > > > On 5/31/2023 4:20 PM, Thinh Nguyen wrote: > > On Wed, May 31, 2023, Elson Roy Serrao wrote: > > > Consider a scenario where cable disconnect happens when there is an active > > > usb reqest queued to the UDC. As part of the disconnect we would issue an > > > end transfer with no interrupt-on-completion before giving back this > > > request. Since we are giving back the request without skipping TRBs the > > > num_trbs field of dwc3_request still holds the stale value previously used. > > > Function drivers re-use same request for a given bind-unbind session and > > > hence their dwc3_request context gets preserved across cable > > > disconnect/connect. When such a request gets re-queued after cable connect, > > > > Why would we preserve the request after a disconnect? The request is > > associated with an endpoint, and after disconnect, the endpoint is no > > longer valid. Shouldn't the request be freed then? > > > > > Function drivers generally allocate usb requests during bind when an > endpoint is allocated to it (through usb_ep_autoconfig). These requests are > freed when an unbind is called as the function is no longer associated with > any end point. The function driver is free to re-use these requests > throughout this bind-unbind session. The only restriction is that the > function drivers wont be able to queue any requests as long as the endpoint > is disabled. But that doesn't enforce function drivers to free the requests > with ep_disable(). Even though the endpoint is disabled with cable > disconnect, that endpoint is still associated with that particular function > driver until that function is unbound. > > As an example below is how f_ncm driver allocates and frees the requests > during bind/unbind > > Bind() > ... > ep = usb_ep_autoconfig(cdev->gadget, &fs_ncm_notify_desc); > if (!ep) > goto fail; > ncm->notify = ep; > > status = -ENOMEM; > > /* allocate notification request and buffer */ > ncm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL); > ... > > The endpoint is enabled later when set_alt is received and disabled in > ncm_disable when the connection goes down (cable disconnect scenario) > > > Unbind() > .... > kfree(ncm->notify_req->buf); > usb_ep_free_request(ncm->notify, ncm->notify_req); > > I see similar implementation in other function drivers as well. That is, > keep the usb requests allocated throughout the bind-unbind session and > independent of ep_enable/ep_disable . > > Thanks > Elson > Thanks for the clarification. Then you just need to reset the num_trbs count when giving back the request. Can we do that in dwc3_gadget_del_and_unmap_request()? Please add a fix tag. Thanks, Thinh
