diff mbox series

[v4] usb: gadget: f_uvc: add super speed plus transfer support

Message ID 20221103060041.25866-1-jleng@ambarella.com
State New
Headers
Series [v4] usb: gadget: f_uvc: add super speed plus transfer support |

Commit Message

Jing Leng Nov. 3, 2022, 6 a.m. UTC 
  UVC driver doesn't set ssp_descriptors in struct usb_function,
it doesn't support USB_SPEED_SUPER_PLUS transfer. So we can
refer to USB_SPEED_SUPER to realize the support of
USB_SPEED_SUPER_PLUS.

If users use a USB device controller that speed can be up to
USB_SPEED_SUPER_PLUS (10 Gbps), downgrading to USB_SPEED_SUPER
(5 Gbps) is not a good performance. In addition, it triggers a
warning "configfs-gadget gadget: uvc doesn't hold the descriptors
for current speed".

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v3->v4:
- Rebase the patch.
- Make email addresses ('From' and 'Signed-off-by') consistent.
ChangeLog v2->v3:
- Modify the title and description of the PATCH
- It is a feature but not a bug
ChangeLog v1->v2:
- Update more detailed description of the PATCH
---
 drivers/usb/gadget/function/f_uvc.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Jing Leng Nov. 3, 2022, 10:39 a.m. UTC | #1 
Hi Kieran,

The patch is everthing that is required to support USB_SPEED_SUPER_PLUS.

I don't have a way to disabling this trailing legal statement when sending emails, it is automatically added by Ambarella mail server, not added by me.
Ambarella follows the GPLv2 protocol when submitting any linux kernel patches to the Linux community.

Thanks!

-----Original Message-----
From: Kieran Bingham <kieran.bingham@ideasonboard.com> 
Sent: Thursday, November 3, 2022 6:05 PM
To: Jing Leng <jleng@ambarella.com>; balbi@kernel.org; gregkh@linuxfoundation.org; laurent.pinchart@ideasonboard.com
Cc: linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; Jing Leng <jleng@ambarella.com>; Dan Scally <dan.scally@ideasonboard.com>
Subject: [EXT] Re: [PATCH v4] usb: gadget: f_uvc: add super speed plus transfer support

Hi Jing,

Quoting Jing Leng (2022-11-03 06:00:41)
> UVC driver doesn't set ssp_descriptors in struct usb_function, it 
> doesn't support USB_SPEED_SUPER_PLUS transfer. So we can refer to 
> USB_SPEED_SUPER to realize the support of USB_SPEED_SUPER_PLUS.

This is a bit confusing to read. Do you mean that we should (later?) add more functionality to support USB_SPEED_SUPER_PLUS? Or is this everthing that is required ?

> If users use a USB device controller that speed can be up to 
> USB_SPEED_SUPER_PLUS (10 Gbps), downgrading to USB_SPEED_SUPER
> (5 Gbps) is not a good performance. In addition, it triggers a warning 
> "configfs-gadget gadget: uvc doesn't hold the descriptors for current 
> speed".
> 
> Signed-off-by: Jing Leng <jleng@ambarella.com>
> ---
> ChangeLog v3->v4:
> - Rebase the patch.
> - Make email addresses ('From' and 'Signed-off-by') consistent.
> ChangeLog v2->v3:
> - Modify the title and description of the PATCH
> - It is a feature but not a bug
> ChangeLog v1->v2:
> - Update more detailed description of the PATCH
> ---
>  drivers/usb/gadget/function/f_uvc.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c 
> b/drivers/usb/gadget/function/f_uvc.c
> index 6e196e06181e..6ffaf594be9d 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -482,6 +482,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
>         void *mem;
>  
>         switch (speed) {
> +       case USB_SPEED_SUPER_PLUS:
>         case USB_SPEED_SUPER:
>                 uvc_control_desc = uvc->desc.ss_control;
>                 uvc_streaming_cls = uvc->desc.ss_streaming; @@ -526,7 
> +527,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
>               + uvc_control_ep.bLength + uvc_control_cs_ep.bLength
>               + uvc_streaming_intf_alt0.bLength;
>  
> -       if (speed == USB_SPEED_SUPER) {
> +       if (speed == USB_SPEED_SUPER || speed == USB_SPEED_SUPER_PLUS) 
> + {
>                 bytes += uvc_ss_control_comp.bLength;
>                 n_desc = 6;
>         } else {
> @@ -570,7 +571,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
>         uvc_control_header->baInterfaceNr[0] = uvc->streaming_intf;
>  
>         UVC_COPY_DESCRIPTOR(mem, dst, &uvc_control_ep);
> -       if (speed == USB_SPEED_SUPER)
> +       if (speed == USB_SPEED_SUPER || speed == USB_SPEED_SUPER_PLUS)
>                 UVC_COPY_DESCRIPTOR(mem, dst, &uvc_ss_control_comp);
>  
>         UVC_COPY_DESCRIPTOR(mem, dst, &uvc_control_cs_ep); @@ -733,6 
> +734,15 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>                 }
>         }
>  
> +       if (gadget_is_superspeed_plus(c->cdev->gadget)) {
> +               f->ssp_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER_PLUS);
> +               if (IS_ERR(f->ssp_descriptors)) {
> +                       ret = PTR_ERR(f->ssp_descriptors);
> +                       f->ssp_descriptors = NULL;
> +                       goto error;
> +               }
> +       }
> +
>         /* Preallocate control endpoint request. */
>         uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
>         uvc->control_buf = kmalloc(UVC_MAX_REQUEST_SIZE, GFP_KERNEL);
> --
> 2.17.1
> 
> 
> **********************************************************************
> This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.

Do you have a way to disabling this trailing legal statement when sending emails?

It's really not appropriate for sending to public mailinglists, and if you are really asserting that this patch is confidential and proprietary
- then it probably can't be integrated into the kernel ...

--
Kieran

######################################################################
This EXTERNAL email has been scanned by Proofpoint Email Protect service.
Greg KH Nov. 3, 2022, 1:30 p.m. UTC | #2 
On Thu, Nov 03, 2022 at 02:00:41PM +0800, Jing Leng wrote:
> **********************************************************************
> This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.

Now deleted, sorry.

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..6ffaf594be9d 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -482,6 +482,7 @@  uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	void *mem;
 
 	switch (speed) {
+	case USB_SPEED_SUPER_PLUS:
 	case USB_SPEED_SUPER:
 		uvc_control_desc = uvc->desc.ss_control;
 		uvc_streaming_cls = uvc->desc.ss_streaming;
@@ -526,7 +527,7 @@  uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	      + uvc_control_ep.bLength + uvc_control_cs_ep.bLength
 	      + uvc_streaming_intf_alt0.bLength;
 
-	if (speed == USB_SPEED_SUPER) {
+	if (speed == USB_SPEED_SUPER || speed == USB_SPEED_SUPER_PLUS) {
 		bytes += uvc_ss_control_comp.bLength;
 		n_desc = 6;
 	} else {
@@ -570,7 +571,7 @@  uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	uvc_control_header->baInterfaceNr[0] = uvc->streaming_intf;
 
 	UVC_COPY_DESCRIPTOR(mem, dst, &uvc_control_ep);
-	if (speed == USB_SPEED_SUPER)
+	if (speed == USB_SPEED_SUPER || speed == USB_SPEED_SUPER_PLUS)
 		UVC_COPY_DESCRIPTOR(mem, dst, &uvc_ss_control_comp);
 
 	UVC_COPY_DESCRIPTOR(mem, dst, &uvc_control_cs_ep);
@@ -733,6 +734,15 @@  uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		}
 	}
 
+	if (gadget_is_superspeed_plus(c->cdev->gadget)) {
+		f->ssp_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER_PLUS);
+		if (IS_ERR(f->ssp_descriptors)) {
+			ret = PTR_ERR(f->ssp_descriptors);
+			f->ssp_descriptors = NULL;
+			goto error;
+		}
+	}
+
 	/* Preallocate control endpoint request. */
 	uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
 	uvc->control_buf = kmalloc(UVC_MAX_REQUEST_SIZE, GFP_KERNEL);