Hi Bhupesh,
Thanks for the patch.
On Thursday 17 January 2013 16:23:49 Bhupesh Sharma wrote:
> This patch corrects the VS_INPUT_HEADER.bEndpointAddress and Video
> Streaming.bEndpointAddress values which were incorrectly set to 0 in UVC
> function driver.
>
> As 'usb_ep_autoconfig' function returns an un-claimed usb_ep, and modifies
> the endpoint descriptor's bEndpointAddress, so there is no need to again
> set the bEndpointAddress for Video Streaming and VS_INPUT_HEADER decriptors
> (for all speeds: SS/HS/FS) to the bEndpointAddress obtained for Full Speed
> case from 'usb_ep_autoconfig' function call.
>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx>
> ---
> drivers/usb/gadget/f_uvc.c | 30 ++++++++++++++++++++----------
> 1 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index b34349f..1769f3e 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -550,8 +550,24 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
> usb_device_speed speed) UVC_COPY_DESCRIPTORS(mem, dst,
> (const struct usb_descriptor_header**)uvc_streaming_cls);
> uvc_streaming_header->wTotalLength = cpu_to_le16(streaming_size);
> - uvc_streaming_header->bEndpointAddress =
> - uvc_fs_streaming_ep.bEndpointAddress;
> +
> + switch (speed) {
> + case USB_SPEED_SUPER:
> + uvc_streaming_header->bEndpointAddress =
> + uvc_ss_streaming_ep.bEndpointAddress;
> + break;
> +
> + case USB_SPEED_HIGH:
> + uvc_streaming_header->bEndpointAddress =
> + uvc_hs_streaming_ep.bEndpointAddress;
> + break;
> +
> + case USB_SPEED_FULL:
> + default:
> + uvc_streaming_header->bEndpointAddress =
> + uvc_fs_streaming_ep.bEndpointAddress;
> + break;
> + }
I don't think this will work. A superspeed device will see uvc_copy_descriptor
called 3 times, once for each speed. As only
uvc_ss_streaming_ep.bEndpointAddress is set in that case, high-speed and
full-speed descriptors will have a zero endpoint value.
I propose the following patch:
diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index 02266c5..c8abe79 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -548,8 +548,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
UVC_COPY_DESCRIPTORS(mem, dst,
(const struct usb_descriptor_header**)uvc_streaming_cls);
uvc_streaming_header->wTotalLength = cpu_to_le16(streaming_size);
- uvc_streaming_header->bEndpointAddress =
- uvc_fs_streaming_ep.bEndpointAddress;
+ uvc_streaming_header->bEndpointAddress = uvc->video.ep->address;
UVC_COPY_DESCRIPTORS(mem, dst, uvc_streaming_std);
@@ -636,7 +635,14 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
uvc->control_ep = ep;
ep->driver_data = uvc;
- ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
+ 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 (!ep) {
INFO(cdev, "Unable to allocate streaming EP\n");
goto error;
@@ -644,10 +650,9 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
uvc->video.ep = ep;
ep->driver_data = uvc;
- uvc_hs_streaming_ep.bEndpointAddress =
- uvc_fs_streaming_ep.bEndpointAddress;
- uvc_ss_streaming_ep.bEndpointAddress =
- uvc_fs_streaming_ep.bEndpointAddress;
+ 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;
/* Allocate interface IDs. */
if ((ret = usb_interface_id(c, f)) < 0)
I will squash it into my "usb: gadget/uvc: Configure the streaming endpoint
based on the speed" to avoid bisection issues.
> UVC_COPY_DESCRIPTORS(mem, dst, uvc_streaming_std);
>
> @@ -681,17 +697,11 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f) /* Copy descriptors for FS. */
> f->descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
>
> - if (gadget_is_dualspeed(cdev->gadget)) {
> - uvc_hs_streaming_ep.bEndpointAddress =
> - uvc_fs_streaming_ep.bEndpointAddress;
> + if (gadget_is_dualspeed(cdev->gadget))
> f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
> - }
>
> - if (gadget_is_superspeed(c->cdev->gadget)) {
> - uvc_ss_streaming_ep.bEndpointAddress =
> - uvc_fs_streaming_ep.bEndpointAddress;
> + if (gadget_is_superspeed(c->cdev->gadget))
> f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
> - }
>
> /* Preallocate control endpoint request. */
> uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
--
Regards,
Laurent Pinchart
--
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
