diff mbox series

[RFC] usb: gadget: uvc: sane shutdown on soft streamoff

Message ID 20230402200122.2919202-1-m.grzeschik@pengutronix.de
State New
Headers show
Series [RFC] usb: gadget: uvc: sane shutdown on soft streamoff | expand

Commit Message

Michael Grzeschik April 2, 2023, 8:01 p.m. UTC
Pending requests in the gadget hardware get dequeued and returned with
ECONNRESET when the available endpoint is not available anymore. This
can be caused by an unplugged cable or the decision to shutdown the
stream, e.g. by switching the alt setting.

In both cases the returned completion handler is marking the gadget
with UVC_QUEUE_DISCONNECTED by calling uvcg_queue_cancel.

Since in userspace applications there might be two threads, one for the
bufferqueueing and one to handle the uvc events. It is likely that the
bufferqueueing thread did not receive the UVC_EVENT_STREAMOFF coming
from the alt_setting change early enough and still tries to queue a
buffer into the already disconnected marked device.

This leads buf_prepare to return ENODEV, which usually makes the
userspace application quit.

To fix the soft-shutdown case this patch is marking the alt setting
change before disabling the endpoint. This way the still completing
requests on the disabled endpoint can call uvcg_queue_cancel without
marking the device disconnected.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
Hi Laurent!

We are running into this issue in gstreamer when the host is stopping
the stream. In fact I am unsure if this is not also an issue when the
real unplug will appear.

Since the v4l2 device is available all the time, and the streamoff
callback is cleaning up all the pending buffers in uvc_video_enable(0),
also the ones that got queued in this short time window of:

 alt_setting(0) -> userspace event handling -> streamoff ioctl

Would it not be also possible to just drop the whole
UVC_QUEUE_DISCONNECTED mechanism?

Thanks,
Michael

 drivers/usb/gadget/function/f_uvc.c     | 3 ++-
 drivers/usb/gadget/function/uvc_video.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 5e919fb6583301..01ab8c07d85be9 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -337,6 +337,8 @@  uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		if (uvc->state != UVC_STATE_STREAMING)
 			return 0;
 
+		uvc->state = UVC_STATE_CONNECTED;
+
 		if (uvc->video.ep)
 			usb_ep_disable(uvc->video.ep);
 
@@ -344,7 +346,6 @@  uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		v4l2_event.type = UVC_EVENT_STREAMOFF;
 		v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		uvc->state = UVC_STATE_CONNECTED;
 		return 0;
 
 	case 1:
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index dd1c6b2ca7c6f3..2f36fef3824f8e 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -265,9 +265,10 @@  uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
 		queue->flags |= UVC_QUEUE_DROP_INCOMPLETE;
 		break;
 
-	case -ESHUTDOWN:	/* disconnect from host. */
+	case -ESHUTDOWN:	/* disconnect from host or streamoff pending */
 		uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
-		uvcg_queue_cancel(queue, 1);
+		uvcg_queue_cancel(queue,
+				  uvc->state != UVC_STATE_STREAMING ? 0 : 1);
 		break;
 
 	default: