Re: [PATCH v5] usb: gadget: uvc: add bulk transfer support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jing - thanks for the patch

On 03/11/2022 06:13, Jing Leng wrote:
The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

Here shows an example of the configfs differences:
   if [ $BULK -eq 1 ]; then
       echo "bulk" > functions/$FUNC/streaming_transfer
       echo $(( 1024 * N )) > functions/$FUNC/streaming_maxpacket
   else
       echo "isoc" > functions/$FUNC/streaming_transfer
       echo 1024 > functions/$FUNC/streaming_maxpacket
   fi

Signed-off-by: Jing Leng <jleng@xxxxxxxxxxxxx>
---
ChangeLog v4->v5:
- Rebase the patch.
- Make email addresses ('From' and 'Signed-off-by') consistent.
ChangeLog v3->v4:
- echo "bulk" > functions/$FUNC/streaming_transfer to set bulk mode
ChangeLog v2->v3:
- Mistakenly deleted the definition of i and USBDHDR when porting from my workdir.
- Reported-by: kernel test robot <lkp@xxxxxxxxx>
ChangeLog v1->v2:
- Handle imagesize in uvc_v4l2_set_format. If it's not handled,
- switching from low resolution to high resolution will fail to play.
---
  .../ABI/testing/configfs-usb-gadget-uvc       |   8 +-
  Documentation/usb/gadget-testing.rst          |   1 +
  drivers/usb/gadget/function/f_uvc.c           | 228 +++++++++++++-----
  drivers/usb/gadget/function/u_uvc.h           |   1 +
  drivers/usb/gadget/function/uvc.h             |   2 +
  drivers/usb/gadget/function/uvc_configfs.c    |  64 ++++-
  drivers/usb/gadget/function/uvc_queue.c       |  14 +-
  drivers/usb/gadget/function/uvc_video.c       |  16 +-
  8 files changed, 261 insertions(+), 73 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 611b23e6488d..ffa2fd8c7fcd 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -3,12 +3,14 @@ Date:		Dec 2014
  KernelVersion:	4.0
  Description:	UVC function directory
- =================== =============================
+		===================	===================================
  		streaming_maxburst	0..15 (ss only)
-		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
+		streaming_maxpacket	isoc: 1..1023 (fs), 1..3072 (hs/ss)
+					bulk: 1024..0x40000000


I'm not really sure that I like this way of representing things, since what you're setting with streaming_maxpacket here is not wMaxPacketSize but the internal max_payload_size variable. I think that that's apt to be quite confusing to people, since the possible values you've listed disagree with the specs. It also precludes setting non-maximum values for full-speed endpoints, which ought to be able to support 8, 16 and 32 bits too. I'd prefer another attribute / some other way of determining the max_payload_size and full control over the bulk endpoint sizes through streaming_maxpacket.


Also, might be worth bobbing another update onto Documentation/usb/gadget-testing.rst

  		streaming_interval	1..16
+		streaming_transfer	isoc/bulk
  		function_name		string [32]
-		===================	=============================
+		===================	===================================
What: /config/usb-gadget/gadget/functions/uvc.name/control
  Date:		Dec 2014
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 2278c9ffb74a..880af56abe8b 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -793,6 +793,7 @@ The uvc function provides these attributes in its function directory:
  	streaming_maxpacket maximum packet size this endpoint is capable of
  			    sending or receiving when this configuration is
  			    selected
+	streaming_transfer  specify data transmission mode (isoc/bulk)
  	function_name       name of the interface
  	=================== ================================================
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..8a4df750e00d 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -29,6 +29,8 @@
  #include "uvc_v4l2.h"
  #include "uvc_video.h"
+#define USBDHDR(p) ((struct usb_descriptor_header *)(p))
+
  unsigned int uvc_gadget_trace_param;
  module_param_named(trace, uvc_gadget_trace_param, uint, 0644);
  MODULE_PARM_DESC(trace, "Trace level bitmask");
@@ -181,19 +183,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
  	 */
  };
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
  	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
  	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
  	NULL,
  };
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
  	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
  	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
  	NULL,
  };
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
  	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
  	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
  	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -204,6 +206,10 @@ static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
   * Control requests
   */
+
+static int uvc_function_set_alt(struct usb_function *f,
+		unsigned int interface, unsigned int alt);
+


Can we just move the function definition here?

  static void
  uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
  {
@@ -219,6 +225,13 @@ uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
  		uvc_event->data.length = req->actual;
  		memcpy(&uvc_event->data.data, req->buf, req->actual);
  		v4l2_event_queue(&uvc->vdev, &v4l2_event);
+
+		/*
+		 * Bulk mode only has one alt, so we should set STREAM ON after
+		 * responding the SET UVC_VS_COMMIT_CONTROL request.
+		 */
+		if (uvc->state == UVC_STATE_BULK_SETTING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 1);
  	}
  }


Given bulk mode only has one alt, perhaps it's better to add new functions for stream on/off and call those (including in uvc_function_set_alt()) to make it clear what's happening.


@@ -228,6 +241,9 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
  	struct uvc_device *uvc = to_uvc(f);
  	struct v4l2_event v4l2_event;
  	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
+	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
+	unsigned int cs = le16_to_cpu(ctrl->wValue) >> 8 & 0xff;
if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
  		uvcg_info(f, "invalid request type\n");
@@ -245,6 +261,21 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
  	uvc->event_setup_out = !(ctrl->bRequestType & USB_DIR_IN);
  	uvc->event_length = le16_to_cpu(ctrl->wLength);
+ /*
+	 * Bulk mode only has one alt, when the SET UVC_VS_COMMIT_CONTROL request
+	 * is received, if the streaming is in transit, we need to set STREAM OFF,
+	 * if the uvc state is UVC_STATE_BULK_WAITING, we only need to change it.
+	 */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK &&
+		uvc->event_setup_out &&
+		uvc->streaming_intf == interface &&
+		cs == UVC_VS_COMMIT_CONTROL) {
+		if (uvc->state == UVC_STATE_STREAMING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 0);
+		else if (uvc->state == UVC_STATE_BULK_WAITING)
+			uvc->state = UVC_STATE_BULK_SETTING;
+	}


As far as I can tell this path doesn't quite work out; when I test the code I don't get uvc_function_set_alt(1, 0) being called, so the streamoff isn't happening correctly when in bulk mode. uvc_video_stop_streaming() in drivers/media/usb/uvc/uvc_video.c has this comment:


/*
* UVC doesn't specify how to inform a bulk-based device
* when the video stream is stopped. Windows sends a
* CLEAR_FEATURE(HALT) request to the video streaming
* bulk endpoint, mimic the same behaviour.
*/


Perhaps it would be good to accommodate that method?


+
  	memset(&v4l2_event, 0, sizeof(v4l2_event));
  	v4l2_event.type = UVC_EVENT_SETUP;
  	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
@@ -255,9 +286,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
void uvc_function_setup_continue(struct uvc_device *uvc)
  {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
  	struct usb_composite_dev *cdev = uvc->func.config->cdev;
- usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+		usb_composite_setup_continue(cdev);
  }
static int
@@ -282,6 +316,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
  	struct usb_composite_dev *cdev = f->config->cdev;
  	struct v4l2_event v4l2_event;
  	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
  	int ret;
uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -314,15 +349,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
  	if (interface != uvc->streaming_intf)
  		return -EINVAL;
- /* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/


I think this probably should stay (uncommented) to be strictly compliant, with the other changes to this function moved to stream on/off functions.

-
  	switch (alt) {
  	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state == UVC_STATE_CONNECTED)
+				uvc->state = UVC_STATE_BULK_WAITING;
+			else if (uvc->state == UVC_STATE_STREAMING)
+				uvc->state = UVC_STATE_BULK_SETTING;
+			else
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
+		}
if (uvc->video.ep)
  			usb_ep_disable(uvc->video.ep);
@@ -331,12 +370,19 @@ 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;
+		if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+			uvc->state = UVC_STATE_CONNECTED;
+
  		return 0;
case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state != UVC_STATE_BULK_SETTING)
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
+		}
if (!uvc->video.ep)
  			return -EINVAL;
@@ -598,62 +644,101 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
  	unsigned int max_packet_size;
  	struct usb_ep *ep;
  	struct f_uvc_opts *opts;
+	int i = 0;
  	int ret = -EINVAL;
uvcg_info(f, "%s()\n", __func__); opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters. */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
- /*
-	 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		if (opts->streaming_maxpacket % 1024 != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+


As I say, I think that these ought to be set by streaming_maxpacket

+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_maxpacket;
  	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters. */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
- uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/*
+		 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
- uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
- /* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
+
Unneeded blank line
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
- uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc->video.max_payload_size = 0;
+	}
/* Allocate endpoints. */
  	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -667,7 +752,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
  		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
  					  &uvc_ss_streaming_comp);
  	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);


This doesn't seem right.

  	else
  		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
@@ -709,6 +794,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
  	uvc->streaming_intf = ret;
  	opts->streaming_interface = ret;
+ /* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
  	/* Copy descriptors */
  	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
  	if (IS_ERR(f->fs_descriptors)) {
@@ -872,6 +979,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
opts->streaming_interval = 1;
  	opts->streaming_maxpacket = 1024;
+	opts->streaming_transfer = USB_ENDPOINT_SYNC_ASYNC | USB_ENDPOINT_XFER_ISOC;
  	snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
ret = uvcg_attach_configfs(opts);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 24b8681b0d6f..88cd235e0ea8 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
  	unsigned int					streaming_interval;
  	unsigned int					streaming_maxpacket;
  	unsigned int					streaming_maxburst;
+	unsigned int					streaming_transfer;
unsigned int control_interface;
  	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e14..b00cba322643 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -123,6 +123,8 @@ enum uvc_state {
  	UVC_STATE_DISCONNECTED,
  	UVC_STATE_CONNECTED,
  	UVC_STATE_STREAMING,
+	UVC_STATE_BULK_WAITING,
+	UVC_STATE_BULK_SETTING,
  };
struct uvc_device {


I think it's worth splitting the new configfs attribute at least into a separate patch - this combined one does a lot.

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 4303a3283ba0..ef7c4f6630d4 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2348,11 +2348,72 @@ end:									\
  UVC_ATTR(f_uvc_opts_, cname, cname)
UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
-UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
+UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 0x40000000U);
  UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
#undef UVCG_OPTS_ATTR +#define UVCG_OPTS_ATTR_TRANSFER(cname, aname) \
+static ssize_t f_uvc_opts_##cname##_show(				\
+	struct config_item *item, char *page)				\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int result;							\
+	char *str;							\
+									\
+	mutex_lock(&opts->lock);					\
+	switch (opts->cname) {						\
+	case USB_ENDPOINT_XFER_BULK:					\
+		str = "bulk";						\
+		break;							\
+	case USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC:		\
+		str = "isoc";						\
+		break;							\
+	default:							\
+		str = "unknown";					\
+		break;							\
+	}								\
+	result = sprintf(page, "%s\n", str);				\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t								\
+f_uvc_opts_##cname##_store(struct config_item *item,			\
+			   const char *page, size_t len)		\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int ret = 0;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	if (!strncmp(page, "bulk", 4))					\
+		opts->cname = USB_ENDPOINT_XFER_BULK;			\
+	else if (!strncmp(page, "isoc", 4))				\
+		opts->cname = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC; \
+	else {								\
+		ret = -EINVAL;						\
+		goto end;						\
+	}								\
+									\
+	ret = len;							\
+									\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+UVC_ATTR(f_uvc_opts_, cname, cname)
+
+UVCG_OPTS_ATTR_TRANSFER(streaming_transfer, streaming_transfer);


Is it worth the macro for a single attribute? Particularly since the set/store is too specific for it to be reusable in the future anyway. I think I'd just write the bare function.

+
+#undef UVCG_OPTS_ATTR_TRANSFER
+
  #define UVCG_OPTS_STRING_ATTR(cname, aname)				\
  static ssize_t f_uvc_opts_string_##cname##_show(struct config_item *item,\
  					 char *page)			\
@@ -2399,6 +2460,7 @@ static struct configfs_attribute *uvc_attrs[] = {
  	&f_uvc_opts_attr_streaming_interval,
  	&f_uvc_opts_attr_streaming_maxpacket,
  	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_transfer,
  	&f_uvc_opts_string_attr_function_name,
  	NULL,
  };
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 0aa3d7e1f3cc..2695b3ed29b5 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -54,9 +54,13 @@ static int uvc_queue_setup(struct vb2_queue *vq,
sizes[0] = video->imagesize; - req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
/* We divide by two, to increase the chance to run
  	 * into fewer requests for smaller framesizes.
@@ -143,7 +147,9 @@ int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2
  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
  	queue->queue.ops = &uvc_queue_qops;
  	queue->queue.lock = lock;
-	if (cdev->gadget->sg_supported) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
  		queue->queue.mem_ops = &vb2_dma_sg_memops;
  		queue->use_sg = 1;
  	} else {
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index dd1c6b2ca7c6..70737340acfb 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -271,8 +271,10 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
  		break;
default:
-		uvcg_warn(&video->uvc->func,
-			  "VS request completed with status %d.\n",
+		if (uvc->state == UVC_STATE_BULK_WAITING ||
+			uvc->state == UVC_STATE_BULK_SETTING)
+			break;
+		uvcg_warn(&uvc->func, "VS request completed with status %d.\n",
  			  req->status);
  		uvcg_queue_cancel(queue, 0);
  	}
@@ -328,9 +330,13 @@ uvc_video_alloc_requests(struct uvc_video *video)
BUG_ON(video->req_size); - req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
  	if (video->ureq == NULL)



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux