Hi, Anurag Kumar Vulisha <[email protected]> writes: > When bulk streams are enabled for an endpoint, there can > be a condition where the gadget controller waits for the > host to issue prime transaction and the host controller > waits for the gadget to issue ERDY. This condition could > create a deadlock. To avoid such potential deadlocks, a > timer is started after queuing any request for the stream > capable endpoint in usb_ep_queue(). The gadget driver is > expected to stop the timer if a valid stream event is found. > If no stream event is found, the timer expires after the > STREAM_TIMEOUT_MS value and a callback function registered > by gadget driver to endpoint ops->stream_timeout API would be > called, so that the gadget driver can handle this situation. > This kind of behaviour is observed in dwc3 controller and > expected to be generic issue with other controllers supporting > bulk streams also. > > Signed-off-by: Anurag Kumar Vulisha <[email protected]> > --- > Changes in v6: > 1. This patch is newly added in this series to add timer into udc/core.c > --- > drivers/usb/gadget/udc/core.c | 71 > ++++++++++++++++++++++++++++++++++++++++++- > include/linux/usb/gadget.h | 12 ++++++++ > 2 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index af88b48..41cc23b 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, > /* ------------------------------------------------------------------------- > */ > > /** > + * usb_ep_stream_timeout - callback function for endpoint stream timeout > timer > + * @arg: pointer to struct timer_list > + * > + * This function gets called only when bulk streams are enabled in the > endpoint > + * and only after ep->stream_timeout_timer has expired. The timer gets > expired > + * only when the gadget controller failed to find a valid stream event for > this > + * endpoint. On timer expiry, this function calls the endpoint-specific > timeout > + * handler registered to endpoint ops->stream_timeout API. > + */ > +static void usb_ep_stream_timeout(struct timer_list *arg) > +{ > + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); > + > + if (ep->stream_capable && ep->ops->stream_timeout)
why is the timer only for stream endpoints? What if we want to run this
on non-stream capable eps?
> + ep->ops->stream_timeout(ep);
you don't ned an extra operation here, ep_dequeue should be enough.
> @@ -102,6 +132,10 @@ int usb_ep_enable(struct usb_ep *ep)
> if (ret)
> goto out;
>
> + if (ep->stream_capable)
> + timer_setup(&ep->stream_timeout_timer,
> + usb_ep_stream_timeout, 0);
the timer should be per-request, not per-endpoint. Otherwise in case of
multiple requests being queued, you can't give them different timeouts
> @@ -269,6 +321,13 @@ int usb_ep_queue(struct usb_ep *ep,
>
> ret = ep->ops->queue(ep, req, gfp_flags);
>
> + if (ep->stream_capable) {
> + ep->stream_timeout_timer.expires = jiffies +
> + msecs_to_jiffies(STREAM_TIMEOUT_MS);
timeout value should be passed by the gadget driver. Add a new
usb_ep_queue_timeout() that takes the new argument. Rename the current
usb_ep_queue() to static int __usb_ep_queue() with an extra argument for
timeout and introduce usb_ep_queue() without the argument, calling
__usb_ep_queue() passing timeout as 0.
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index e5cd84a..2ebaef0 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -144,6 +144,7 @@ struct usb_ep_ops {
>
> int (*fifo_status) (struct usb_ep *ep);
> void (*fifo_flush) (struct usb_ep *ep);
> + void (*stream_timeout) (struct usb_ep *ep);
why?
--
balbi
signature.asc
Description: PGP signature
