lkml.org 
[lkml]   [2018]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable endpoints
Date

Hi,

Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> 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 <anurag.kumar.vulisha@xilinx.com>
> ---
> 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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2018-11-14 14:59    [W:0.145 / U:4.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site