Hi Laurent,
Thanks for your review comments.
On 4/29/2013 4:23 PM, Laurent Pinchart wrote:
Hi Bhupesh,
Thank you for the patch. Please see below for comments.
On Thursday 11 April 2013 15:04:04 Bhupesh Sharma wrote:
This patch adds the support for Bulk endpoint to be used as video streaming
endpoint, on basis of a module parameter.
By default, the gadget still supports Isochronous endpoint for video
streaming, but if the module parameter 'bulk_streaming_ep' is set to 1, we
can support Bulk endpoint as well, which is useful for UDC's which don't
support Isochronous endpoints.
The important difference between the two implementations is that,
alt-settings in a video streaming interface are supported only for
Isochronous endpoints as there are different alt-settings for
zero-bandwidth and full-bandwidth use-cases, but the same is not true for
Bulk endpoints, as they support only a single alt-setting.
Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx>
---
Note that to ease review and integration of this patch, I have rebased it
on Laurent's UVC gadget git tree available here (head uvc-gadget):
git://linuxtv.org/pinchartl/uvcvideo.git
This will allow the patch to be pulled into Felipe's repo in one go
after review and any subsequent rework (if required).
Thank you.
drivers/usb/gadget/f_uvc.c | 321 +++++++++++++++++++++++++++++--------
drivers/usb/gadget/uvc.h | 2 +
drivers/usb/gadget/uvc_v4l2.c | 17 ++-
drivers/usb/gadget/uvc_video.c | 13 ++-
4 files changed, 286 insertions(+), 67 deletions(-)
diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index 38dcedd..e5953eb 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -45,6 +45,11 @@ static unsigned int streaming_maxburst;
module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(streaming_maxburst, "0 - 15 (ss only)");
+static bool bulk_streaming_ep;
+module_param(bulk_streaming_ep, bool, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(bulk_streaming_ep, "0 (Use ISOC video streaming ep) / "
+ "1 (Use BULK video streaming ep)");
+
Do you think an endpoint type property with values equal to "isoc" or "bulk"
would be more explicit ?
Yes. Explicit values seem better. So I will add this in V2.
Any value other than ISOC and BULK will be an flagged as an invalid
module argument.
/* ------------------------------------------------------------------------
* Function descriptors
*/
@@ -135,6 +140,19 @@ static struct usb_interface_descriptor
uvc_streaming_intf_alt0 __initdata = { .iInterface = 0,
};
+static struct usb_interface_descriptor uvc_bulk_streaming_intf_alt0
+__initdata = {
+ .bLength = USB_DT_INTERFACE_SIZE,
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bInterfaceNumber = UVC_INTF_VIDEO_STREAMING,
+ .bAlternateSetting = 0,
+ .bNumEndpoints = 1,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = UVC_SC_VIDEOSTREAMING,
+ .bInterfaceProtocol = 0x00,
+ .iInterface = 0,
+};
+
For consistency, could you please rename uvc_streaming_intf_alt* to
uvc_isoc_streaming_intf_alt* (in a separate preparation patch) ?
Ok.
static struct usb_interface_descriptor uvc_streaming_intf_alt1 __initdata =
{ .bLength = USB_DT_INTERFACE_SIZE,
.bDescriptorType = USB_DT_INTERFACE,
@@ -160,6 +178,18 @@ static struct usb_endpoint_descriptor
uvc_fs_streaming_ep __initdata = { .bInterval = 0,
};
+static struct usb_endpoint_descriptor uvc_fs_bulk_streaming_ep __initdata =
{ + .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+ /* The wMaxPacketSize and bInterval values will be initialized from
+ * module parameters.
+ */
+ .wMaxPacketSize = 0,
+ .bInterval = 0,
You can remove these two lines (I've sent a patch to do so for the rest of the
structures).
Ok.
+};
+
static struct usb_endpoint_descriptor uvc_hs_streaming_ep __initdata = {
.bLength = USB_DT_ENDPOINT_SIZE,
.bDescriptorType = USB_DT_ENDPOINT,
@@ -173,6 +203,18 @@ static struct usb_endpoint_descriptor
uvc_hs_streaming_ep __initdata = { .bInterval = 0,
};
+static struct usb_endpoint_descriptor uvc_hs_bulk_streaming_ep __initdata =
{ + .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+ /* The wMaxPacketSize and bInterval values will be initialized from
+ * module parameters.
+ */
+ .wMaxPacketSize = 0,
+ .bInterval = 0,
You can remove these two lines as well.
Ok.
+};
+
Can you please group the bulk streaming endpoint descriptors together (either
before or after the isoc streaming endpoint descriptors) ?
Sure.
static struct usb_endpoint_descriptor uvc_ss_streaming_ep __initdata = {
.bLength = USB_DT_ENDPOINT_SIZE,
.bDescriptorType = USB_DT_ENDPOINT,
@@ -187,6 +229,19 @@ static struct usb_endpoint_descriptor
uvc_ss_streaming_ep __initdata = { .bInterval = 0,
};
+static struct usb_endpoint_descriptor uvc_ss_bulk_streaming_ep __initdata =
{ + .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK,
+ /* The wMaxPacketSize and bInterval values will be initialized from
+ * module parameters.
+ */
+ .wMaxPacketSize = 0,
+ .bInterval = 0,
Those two lines can be removed as well.
Ok.
+};
+
static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp __initdata =
{ .bLength = sizeof(uvc_ss_streaming_comp),
.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
@@ -196,18 +251,38 @@ static struct usb_ss_ep_comp_descriptor
uvc_ss_streaming_comp __initdata = { .wBytesPerInterval =
cpu_to_le16(1024),
};
+static struct usb_ss_ep_comp_descriptor uvc_ss_bulk_streaming_comp
+__initdata = {
+ .bLength = sizeof(uvc_ss_bulk_streaming_comp),
+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+ /* The following 3 values can be tweaked if necessary. */
+ .bMaxBurst = 0,
+ .bmAttributes = 0,
+ .wBytesPerInterval = cpu_to_le16(1024),
Please replace these four lines with
/* The bMaxBurst, bmAttributes and wBytesPerInterval values will be
* initialized from module parameters.
*/
+};
+
Ok.
static const struct usb_descriptor_header * const 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_fs_bulk_streaming[] =
{ + (struct usb_descriptor_header *) &uvc_fs_bulk_streaming_ep,
+ NULL,
+};
+
static const struct usb_descriptor_header * const 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_hs_bulk_streaming[] =
{ + (struct usb_descriptor_header *) &uvc_hs_bulk_streaming_ep,
+ NULL,
+};
+
static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
@@ -215,6 +290,12 @@ static const struct usb_descriptor_header * const
uvc_ss_streaming[] = { NULL,
};
+static const struct usb_descriptor_header * const uvc_ss_bulk_streaming[] =
{ + (struct usb_descriptor_header *) &uvc_ss_bulk_streaming_ep,
+ (struct usb_descriptor_header *) &uvc_ss_bulk_streaming_comp,
+ NULL,
+};
+
Can you please group the bulk streaming endpoint descriptors together here as
well ?
Sure.
/*
--------------------------------------------------------------------------
* Control requests
*/
@@ -285,7 +366,17 @@ uvc_function_get_alt(struct usb_function *f, unsigned
interface) else if (interface != uvc->streaming_intf)
return -EINVAL;
else
- return uvc->state == UVC_STATE_STREAMING ? 1 : 0;
+ /*
+ * Alt settings in an interface are supported only for
+ * ISOC endpoints as there are different alt-settings for
+ * zero-bandwidth and full-bandwidth cases, but the same
+ * is not true for BULK endpoints, as they have a single
+ * alt-setting.
+ */
+ if (!bulk_streaming_ep)
+ return uvc->state == UVC_STATE_STREAMING ? 1 : 0;
+ else
+ return 0;
What about just
- else
- return uvc->state == UVC_STATE_STREAMING ? 1 : 0;
+ else if (bulk_streaming_ep)
+ /* Alternate settings are not supported for bulk endpoints. */
+ return 0;
+ else
+ return uvc->state == UVC_STATE_STREAMING ? 1 : 0;
Ok. Will add this in V2.
}
static int
@@ -317,45 +408,91 @@ uvc_function_set_alt(struct usb_function *f, unsigned
interface, unsigned alt) if (interface != uvc->streaming_intf)
return -EINVAL;
The number of nested statements below is getting a bit large for my taste.
Before addressing that, I have some doubts regarding the logic below.
uvc_function_set_alt() will be once to select alt setting 0 in response to
SET_CONFIGURATION (see set_config() in composite.c) and once due to a
SET_INTERFACE request sent by the host UVC driver at initiazation time (not at
stream start timee). As SET_INTERFACE doesn't make much sense for interfaces
with bulk endpoints only (but is definitely valid), the Windows UVC driver
might not issue that extra SET_INTERFACE call. Even if it does selecting alt
setting 0 doesn't mean that we should start streaming. I'm thus not sure if
this is the best place to handle stream start for bulk devices.
Sure. If you see my last patch on the UVC test application:
"[Test Application PATCH 1/2] UVC gadget: Add support for integration
with a video-capture device and other fixes"
you will notice that the STREAMON event passed to user space application
doesn't start the streaming on UVC gadget in case of Bulk endpoints.
Instead it is done at the time the UVC webcam gadget receives a
"UVC_VS_COMMIT_CONTROL" command from USB host.
This is in my opinion a shortcoming of the UVC specification, there's no
explicit indication sent by the host to the device that it should start
streaming on a bulk endpoint. I don't know how other devices handle that, they
might start streaming when they receive the first bulk request, and stop after
a timeout. The gadget framework would need to be extended to inform drivers
about those conditions (or at least about the first condition, the second one
could be detected in drivers).
Correct. I saw atleast one commercial webcam starting streaming on Bulk
video streaming endpoint, when it received the UVC_VS_COMMIT_CONTROL
command from USB host.
So, I used a similar approach and it works fine atleast in the use-case
I tested.
- /* TODO
- if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
- return alt ? -EINVAL : 0;
- */
+ if (!bulk_streaming_ep) {
+ switch (alt) {
+ case 0:
+ if (uvc->state != UVC_STATE_STREAMING)
+ return 0;
+
+ if (uvc->video.ep)
+ usb_ep_disable(uvc->video.ep);
+
+ memset(&v4l2_event, 0, sizeof(v4l2_event));
+ v4l2_event.type = UVC_EVENT_STREAMOFF;
+ v4l2_event_queue(uvc->vdev, &v4l2_event);
- switch (alt) {
- case 0:
- if (uvc->state != UVC_STATE_STREAMING)
+ uvc->state = UVC_STATE_CONNECTED;
return 0;
- if (uvc->video.ep)
- usb_ep_disable(uvc->video.ep);
+ case 1:
+ if (uvc->state != UVC_STATE_CONNECTED)
+ return 0;
- memset(&v4l2_event, 0, sizeof(v4l2_event));
- v4l2_event.type = UVC_EVENT_STREAMOFF;
- v4l2_event_queue(uvc->vdev, &v4l2_event);
+ if (uvc->video.ep) {
+ ret = config_ep_by_speed
+ (f->config->cdev->gadget,
+ &(uvc->func), uvc->video.ep);
+ if (ret)
+ return ret;
+ usb_ep_enable(uvc->video.ep);
+ }
- uvc->state = UVC_STATE_CONNECTED;
- return 0;
+ memset(&v4l2_event, 0, sizeof(v4l2_event));
+ v4l2_event.type = UVC_EVENT_STREAMON;
+ v4l2_event_queue(uvc->vdev, &v4l2_event);
+ return USB_GADGET_DELAYED_STATUS;
- case 1:
- if (uvc->state != UVC_STATE_CONNECTED)
+ default:
+ return -EINVAL;
+ }
+ } else {
+ switch (uvc->state) {
+ case UVC_STATE_CONNECTED:
+ if (!uvc->video.ep->driver_data) {
When can this happen ?
I will check this again and get back.
+ /*
+ * Enable the video streaming endpoint,
+ * but don't change the 'uvc->state'.
+ */
+ if (uvc->video.ep) {
+ ret = config_ep_by_speed
+ (f->config->cdev->gadget,
+ &(uvc->func), uvc->video.ep);
+ if (ret)
+ return ret;
+ ret = usb_ep_enable(uvc->video.ep);
+ if (ret)
+ return ret;
+
+ uvc->video.ep->driver_data = uvc;
+ }
+ } else {
+ memset(&v4l2_event, 0, sizeof(v4l2_event));
+ v4l2_event.type = UVC_EVENT_STREAMON;
+ v4l2_event_queue(uvc->vdev, &v4l2_event);
+
+ uvc->state = UVC_STATE_STREAMING;
+ }
return 0;
- if (uvc->video.ep) {
- ret = config_ep_by_speed(f->config->cdev->gadget,
- &(uvc->func), uvc->video.ep);
- if (ret)
- return ret;
- usb_ep_enable(uvc->video.ep);
- }
+ case UVC_STATE_STREAMING:
+ if (uvc->video.ep->driver_data) {
+ if (uvc->video.ep) {
+ ret = usb_ep_disable(uvc->video.ep);
+ if (ret)
+ return ret;
+ }
+ }
- memset(&v4l2_event, 0, sizeof(v4l2_event));
- v4l2_event.type = UVC_EVENT_STREAMON;
- v4l2_event_queue(uvc->vdev, &v4l2_event);
- return USB_GADGET_DELAYED_STATUS;
+ memset(&v4l2_event, 0, sizeof(v4l2_event));
+ v4l2_event.type = UVC_EVENT_STREAMOFF;
+ v4l2_event_queue(uvc->vdev, &v4l2_event);
- default:
- return -EINVAL;
+ uvc->state = UVC_STATE_CONNECTED;
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
}
}
@@ -450,6 +587,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
usb_device_speed speed) const struct uvc_descriptor_header * const
*uvc_streaming_cls;
const struct usb_descriptor_header * const *uvc_streaming_std;
const struct usb_descriptor_header * const *src;
+ struct usb_interface_descriptor *streaming_intf_alt0;
struct usb_descriptor_header **dst;
struct usb_descriptor_header **hdr;
unsigned int control_size;
@@ -492,12 +630,17 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
usb_device_speed speed) * uvc_{fs|hs}_streaming
*/
+ if (!bulk_streaming_ep)
Could you test uvc->bulk_streaming_ep instead of the module parameters (here
and in all the other locations) ? This would allow parsing the module
parameter string value at init time (see above) into a numerical value.
Ok.
+ streaming_intf_alt0 = &uvc_streaming_intf_alt0;
+ else
+ streaming_intf_alt0 = &uvc_bulk_streaming_intf_alt0;
+
/* Count descriptors and compute their size. */
control_size = 0;
streaming_size = 0;
bytes = uvc_iad.bLength + uvc_control_intf.bLength
+ uvc_control_ep.bLength + uvc_control_cs_ep.bLength
- + uvc_streaming_intf_alt0.bLength;
+ + streaming_intf_alt0->bLength;
if (speed == USB_SPEED_SUPER) {
bytes += uvc_ss_control_comp.bLength;
@@ -547,7 +690,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
usb_device_speed speed) UVC_COPY_DESCRIPTOR(mem, dst,
&uvc_ss_control_comp);
UVC_COPY_DESCRIPTOR(mem, dst, &uvc_control_cs_ep);
- UVC_COPY_DESCRIPTOR(mem, dst, &uvc_streaming_intf_alt0);
+ UVC_COPY_DESCRIPTOR(mem, dst, streaming_intf_alt0);
uvc_streaming_header = mem;
UVC_COPY_DESCRIPTORS(mem, dst,
@@ -594,11 +737,19 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f)
INFO(cdev, "uvc_function_bind\n");
+ /* Use Bulk endpoint for video streaming?. */
+ uvc->video.bulk_streaming_ep = bulk_streaming_ep;
+
/* Sanity check the streaming endpoint module parameters.
*/
- streaming_interval = clamp(streaming_interval, 1U, 16U);
- streaming_maxpacket = clamp(streaming_maxpacket, 1U, 3072U);
- streaming_maxburst = min(streaming_maxburst, 15U);
+ if (!bulk_streaming_ep) {
+ streaming_interval = clamp(streaming_interval, 1U, 16U);
+ streaming_maxpacket = clamp(streaming_maxpacket, 1U, 3072U);
+ streaming_maxburst = min(streaming_maxburst, 15U);
+ } else {
+ streaming_maxpacket = clamp(streaming_maxpacket, 1U, 1024U);
+ streaming_maxburst = min(streaming_maxburst, 15U);
+ }
/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
* module parameters.
@@ -617,19 +768,34 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f) max_packet_size = streaming_maxpacket / 3;
}
Wouldn't the code below become much simpler if we merged the isoc and bulk
endpoint descriptors and just patched the endpoint type at runtime ?
Only patching the endpoint type will not help. There are various other
differences in ISOC and BULK endpoint properties that must be handled here:
- mult for USBSS: valid for ISOC endpoints, and not defined for BULK
endpoints.
- maxpkt size variations for different speeds.
- uvc_fs_streaming_ep.wMaxPacketSize = min(streaming_maxpacket, 1023U);
- uvc_fs_streaming_ep.bInterval = streaming_interval;
+ if (!bulk_streaming_ep) {
+ uvc_fs_streaming_ep.wMaxPacketSize = min(streaming_maxpacket,
+ 1023U);
+ uvc_fs_streaming_ep.bInterval = streaming_interval;
+
+ uvc_hs_streaming_ep.wMaxPacketSize = max_packet_size;
+ uvc_hs_streaming_ep.wMaxPacketSize |= ((max_packet_mult - 1)
+ << 11);
+ uvc_hs_streaming_ep.bInterval = streaming_interval;
+
+ uvc_ss_streaming_ep.wMaxPacketSize = max_packet_size;
+ uvc_ss_streaming_ep.bInterval = streaming_interval;
+ uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
+ uvc_ss_streaming_comp.bMaxBurst = streaming_maxburst;
+ uvc_ss_streaming_comp.wBytesPerInterval =
+ max_packet_size * max_packet_mult * streaming_maxburst;
+ } else {
+ uvc_fs_bulk_streaming_ep.wMaxPacketSize =
+ min(streaming_maxpacket, 64U);
- uvc_hs_streaming_ep.wMaxPacketSize = max_packet_size;
- uvc_hs_streaming_ep.wMaxPacketSize |= ((max_packet_mult - 1) << 11);
- uvc_hs_streaming_ep.bInterval = streaming_interval;
+ uvc_hs_bulk_streaming_ep.wMaxPacketSize =
+ min(streaming_maxpacket, 512U);
- uvc_ss_streaming_ep.wMaxPacketSize = max_packet_size;
- uvc_ss_streaming_ep.bInterval = streaming_interval;
- uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
- uvc_ss_streaming_comp.bMaxBurst = streaming_maxburst;
- uvc_ss_streaming_comp.wBytesPerInterval =
- max_packet_size * max_packet_mult * streaming_maxburst;
+ uvc_ss_bulk_streaming_ep.wMaxPacketSize = max_packet_size;
+ uvc_ss_streaming_comp.bMaxBurst = streaming_maxburst;
+ uvc_ss_streaming_comp.wBytesPerInterval =
+ max_packet_size * streaming_maxburst;
+ }
/* Allocate endpoints. */
ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -640,13 +806,31 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f) uvc->control_ep = ep;
ep->driver_data = uvc;
- if (gadget_is_superspeed(c->cdev->gadget))
- 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);
- else
- ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
+ if (gadget_is_superspeed(c->cdev->gadget)) {
+ if (!bulk_streaming_ep)
+ ep = usb_ep_autoconfig_ss(cdev->gadget,
+ &uvc_ss_streaming_ep,
+ &uvc_ss_streaming_comp);
+ else
+ ep = usb_ep_autoconfig_ss(cdev->gadget,
+ &uvc_ss_bulk_streaming_ep,
+ &uvc_ss_bulk_streaming_comp);
+
+ } else if (gadget_is_dualspeed(cdev->gadget)) {
+ if (!bulk_streaming_ep)
+ ep = usb_ep_autoconfig(cdev->gadget,
+ &uvc_hs_streaming_ep);
+ else
+ ep = usb_ep_autoconfig(cdev->gadget,
+ &uvc_hs_bulk_streaming_ep);
+ } else {
+ if (!bulk_streaming_ep)
+ ep = usb_ep_autoconfig(cdev->gadget,
+ &uvc_fs_streaming_ep);
+ else
+ ep = usb_ep_autoconfig(cdev->gadget,
+ &uvc_fs_bulk_streaming_ep);
+ }
if (!ep) {
INFO(cdev, "Unable to allocate streaming EP\n");
@@ -655,9 +839,18 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f) uvc->video.ep = ep;
ep->driver_data = uvc;
- uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
- uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
- uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
+ if (!bulk_streaming_ep) {
+ uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
+ uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
+ uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
+ } else {
+ uvc_fs_bulk_streaming_ep.bEndpointAddress =
+ uvc->video.ep->address;
+ uvc_hs_bulk_streaming_ep.bEndpointAddress =
+ uvc->video.ep->address;
+ uvc_ss_bulk_streaming_ep.bEndpointAddress =
+ uvc->video.ep->address;
+ }
/* Allocate interface IDs. */
if ((ret = usb_interface_id(c, f)) < 0)
@@ -668,8 +861,12 @@ uvc_function_bind(struct usb_configuration *c, struct
usb_function *f)
if ((ret = usb_interface_id(c, f)) < 0)
goto error;
- uvc_streaming_intf_alt0.bInterfaceNumber = ret;
- uvc_streaming_intf_alt1.bInterfaceNumber = ret;
+ if (!bulk_streaming_ep) {
+ uvc_streaming_intf_alt0.bInterfaceNumber = ret;
+ uvc_streaming_intf_alt1.bInterfaceNumber = ret;
+ } else {
+ uvc_bulk_streaming_intf_alt0.bInterfaceNumber = ret;
+ }
uvc->streaming_intf = ret;
/* Copy descriptors */
@@ -806,8 +1003,12 @@ uvc_bind_config(struct usb_configuration *c,
uvc_control_intf.iInterface =
uvc_en_us_strings[UVC_STRING_CONTROL_IDX].id;
ret = uvc_en_us_strings[UVC_STRING_STREAMING_IDX].id;
- uvc_streaming_intf_alt0.iInterface = ret;
- uvc_streaming_intf_alt1.iInterface = ret;
+ if (!bulk_streaming_ep) {
+ uvc_streaming_intf_alt0.iInterface = ret;
+ uvc_streaming_intf_alt1.iInterface = ret;
+ } else {
+ uvc_bulk_streaming_intf_alt0.bInterfaceNumber = ret;
+ }
}
/* Register the function. */
diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
index 817e9e1..8756853 100644
--- a/drivers/usb/gadget/uvc.h
+++ b/drivers/usb/gadget/uvc.h
@@ -133,6 +133,8 @@ struct uvc_video
struct uvc_video_queue queue;
unsigned int fid;
+
+ bool bulk_streaming_ep;
If you agree with the comments above this should be moved to the uvc_device
structure. I would call it streaming_ep_type and use the USB_ENDPOINT_XFER_*
values.
Ok.
};
enum uvc_state
diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
index ad48e81..c9656dc 100644
--- a/drivers/usb/gadget/uvc_v4l2.c
+++ b/drivers/usb/gadget/uvc_v4l2.c
@@ -250,11 +250,20 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd,
void *arg) return ret;
/*
- * Complete the alternate setting selection setup phase now that
- * userspace is ready to provide video frames.
+ * Alt settings in an interface are supported only for ISOC
+ * endpoints as there are different alt-settings for
+ * zero-bandwidth and full-bandwidth cases, but the same is not
+ * true for BULK endpoints, as they have a single alt-setting.
*/
- uvc_function_setup_continue(uvc);
- uvc->state = UVC_STATE_STREAMING;
+ if (!video->bulk_streaming_ep) {
You can invert the check and return 0 directly. The indentation level below
would then be lowered.
Ok.
+ /*
+ * Complete the alternate setting selection setup
+ * phase now that userspace is ready to provide video
+ * frames.
+ */
+ uvc_function_setup_continue(uvc);
+ uvc->state = UVC_STATE_STREAMING;
+ }
return 0;
}
diff --git a/drivers/usb/gadget/uvc_video.c b/drivers/usb/gadget/uvc_video.c
index 71e896d..87ac526 100644
--- a/drivers/usb/gadget/uvc_video.c
+++ b/drivers/usb/gadget/uvc_video.c
@@ -238,9 +238,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 + 1);
+ if (!video->bulk_streaming_ep)
+ req_size = video->ep->maxpacket
+ * max_t(unsigned int, video->ep->maxburst, 1)
+ * (video->ep->mult + 1);
+ else
+ req_size = video->ep->maxpacket
+ * max_t(unsigned int, video->ep->maxburst, 1);
What is the value of video->ep->mult for bulk endpoints ? If it's always 0
there's no need to change the code here.
Actually USB3.0 specs do not define MULT parameter for Bulk endpoints,
so I am not sure if assuming the same to have a value of 0 here will be
in SYNC with the specs. That's why I differentiated the two paths in the
patch.
for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
video->req_buffer[i] = kmalloc(req_size, GFP_KERNEL);
@@ -387,6 +391,9 @@ uvc_video_init(struct uvc_video *video)
video->height = 240;
video->imagesize = 320 * 240 * 2;
+ if (video->bulk_streaming_ep)
+ video->max_payload_size = video->imagesize;
+
/* Initialize the video buffers queue. */
uvc_queue_init(&video->queue, V4L2_BUF_TYPE_VIDEO_OUTPUT);
return 0;
Regards,
Bhupesh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
