diff mbox series

[v4] usb: gadget: f_uac2: fix superspeed transfer

Message ID 20220218095948.4077-1-3090101217@zju.edu.cn
State New
Headers show
Series [v4] usb: gadget: f_uac2: fix superspeed transfer | expand

Commit Message

Jing Leng Feb. 18, 2022, 9:59 a.m. UTC
From: Jing Leng <jleng@ambarella.com>

On page 362 of the USB3.2 specification (
https://usb.org/sites/default/files/usb_32_20210125.zip),
The 'SuperSpeed Endpoint Companion Descriptor' shall only be returned
by Enhanced SuperSpeed devices that are operating at Gen X speed.
Each endpoint described in an interface is followed by a 'SuperSpeed
Endpoint Companion Descriptor'.

If users use SuperSpeed UDC, host can't recognize the device if endpoint
doesn't have 'SuperSpeed Endpoint Companion Descriptor' followed.

Currently in the uac2 driver code:
1. ss_epout_desc_comp follows ss_epout_desc;
2. ss_epin_fback_desc_comp follows ss_epin_fback_desc;
3. ss_epin_desc_comp follows ss_epin_desc;
4. Only ss_ep_int_desc endpoint doesn't have 'SuperSpeed Endpoint
Companion Descriptor' followed, so we should add it.

Fixes: eaf6cbe09920 ("usb: gadget: f_uac2: add volume and mute support")
Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v3->v4:
- Add "Fixes:" tag in the changelog area
ChangeLog v2->v3:
- Remove static variables which are explicitly initialized to 0
- Remove redundant modification "case USB_SPEED_SUPER_PLUS:"
ChangeLog v1->v2:
- Update more detailed description of the PATCH
---
 drivers/usb/gadget/function/f_uac2.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Jack Pham July 21, 2022, 12:26 a.m. UTC | #1
Hi Greg,

On Fri, Feb 18, 2022 at 05:59:48PM +0800, 3090101217@zju.edu.cn wrote:
> From: Jing Leng <jleng@ambarella.com>
> 
> On page 362 of the USB3.2 specification (
> https://usb.org/sites/default/files/usb_32_20210125.zip),
> The 'SuperSpeed Endpoint Companion Descriptor' shall only be returned
> by Enhanced SuperSpeed devices that are operating at Gen X speed.
> Each endpoint described in an interface is followed by a 'SuperSpeed
> Endpoint Companion Descriptor'.
> 
> If users use SuperSpeed UDC, host can't recognize the device if endpoint
> doesn't have 'SuperSpeed Endpoint Companion Descriptor' followed.
> 
> Currently in the uac2 driver code:
> 1. ss_epout_desc_comp follows ss_epout_desc;
> 2. ss_epin_fback_desc_comp follows ss_epin_fback_desc;
> 3. ss_epin_desc_comp follows ss_epin_desc;
> 4. Only ss_ep_int_desc endpoint doesn't have 'SuperSpeed Endpoint
> Companion Descriptor' followed, so we should add it.
> 
> Fixes: eaf6cbe09920 ("usb: gadget: f_uac2: add volume and mute support")
> Signed-off-by: Jing Leng <jleng@ambarella.com>
> ---
> ChangeLog v3->v4:
> - Add "Fixes:" tag in the changelog area
> ChangeLog v2->v3:
> - Remove static variables which are explicitly initialized to 0
> - Remove redundant modification "case USB_SPEED_SUPER_PLUS:"
> ChangeLog v1->v2:
> - Update more detailed description of the PATCH

I don't see this patch in any of your trees, so I'm assuming it must not
have made it into your inbox.  If that's the case I would like to
resubmit on Jing's behalf as it does fix a legitimate issue with
enabling the UAC2 gadget in SuperSpeed.

Jack
Greg Kroah-Hartman July 27, 2022, 12:14 p.m. UTC | #2
On Wed, Jul 20, 2022 at 05:26:59PM -0700, Jack Pham wrote:
> Hi Greg,
> 
> On Fri, Feb 18, 2022 at 05:59:48PM +0800, 3090101217@zju.edu.cn wrote:
> > From: Jing Leng <jleng@ambarella.com>
> > 
> > On page 362 of the USB3.2 specification (
> > https://usb.org/sites/default/files/usb_32_20210125.zip),
> > The 'SuperSpeed Endpoint Companion Descriptor' shall only be returned
> > by Enhanced SuperSpeed devices that are operating at Gen X speed.
> > Each endpoint described in an interface is followed by a 'SuperSpeed
> > Endpoint Companion Descriptor'.
> > 
> > If users use SuperSpeed UDC, host can't recognize the device if endpoint
> > doesn't have 'SuperSpeed Endpoint Companion Descriptor' followed.
> > 
> > Currently in the uac2 driver code:
> > 1. ss_epout_desc_comp follows ss_epout_desc;
> > 2. ss_epin_fback_desc_comp follows ss_epin_fback_desc;
> > 3. ss_epin_desc_comp follows ss_epin_desc;
> > 4. Only ss_ep_int_desc endpoint doesn't have 'SuperSpeed Endpoint
> > Companion Descriptor' followed, so we should add it.
> > 
> > Fixes: eaf6cbe09920 ("usb: gadget: f_uac2: add volume and mute support")
> > Signed-off-by: Jing Leng <jleng@ambarella.com>
> > ---
> > ChangeLog v3->v4:
> > - Add "Fixes:" tag in the changelog area
> > ChangeLog v2->v3:
> > - Remove static variables which are explicitly initialized to 0
> > - Remove redundant modification "case USB_SPEED_SUPER_PLUS:"
> > ChangeLog v1->v2:
> > - Update more detailed description of the PATCH
> 
> I don't see this patch in any of your trees, so I'm assuming it must not
> have made it into your inbox.  If that's the case I would like to
> resubmit on Jing's behalf as it does fix a legitimate issue with
> enabling the UAC2 gadget in SuperSpeed.

Thank you.  Reviewing the other patches sent by this author would also
be greatly appreciated as I have not accepted them due to them no one
else speaking up.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 097a709549d6..b5baefe14013 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -282,6 +282,12 @@  static struct usb_endpoint_descriptor ss_ep_int_desc = {
 	.bInterval = 4,
 };
 
+static struct usb_ss_ep_comp_descriptor ss_ep_int_desc_comp = {
+	.bLength = sizeof(ss_ep_int_desc_comp),
+	.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+	.wBytesPerInterval = cpu_to_le16(6),
+};
+
 /* Audio Streaming OUT Interface - Alt0 */
 static struct usb_interface_descriptor std_as_out_if0_desc = {
 	.bLength = sizeof std_as_out_if0_desc,
@@ -595,7 +601,8 @@  static struct usb_descriptor_header *ss_audio_desc[] = {
 	(struct usb_descriptor_header *)&in_feature_unit_desc,
 	(struct usb_descriptor_header *)&io_out_ot_desc,
 
-  (struct usb_descriptor_header *)&ss_ep_int_desc,
+	(struct usb_descriptor_header *)&ss_ep_int_desc,
+	(struct usb_descriptor_header *)&ss_ep_int_desc_comp,
 
 	(struct usb_descriptor_header *)&std_as_out_if0_desc,
 	(struct usb_descriptor_header *)&std_as_out_if1_desc,
@@ -723,6 +730,7 @@  static void setup_headers(struct f_uac2_opts *opts,
 	struct usb_ss_ep_comp_descriptor *epout_desc_comp = NULL;
 	struct usb_ss_ep_comp_descriptor *epin_desc_comp = NULL;
 	struct usb_ss_ep_comp_descriptor *epin_fback_desc_comp = NULL;
+	struct usb_ss_ep_comp_descriptor *ep_int_desc_comp = NULL;
 	struct usb_endpoint_descriptor *epout_desc;
 	struct usb_endpoint_descriptor *epin_desc;
 	struct usb_endpoint_descriptor *epin_fback_desc;
@@ -750,6 +758,7 @@  static void setup_headers(struct f_uac2_opts *opts,
 		epin_fback_desc = &ss_epin_fback_desc;
 		epin_fback_desc_comp = &ss_epin_fback_desc_comp;
 		ep_int_desc = &ss_ep_int_desc;
+		ep_int_desc_comp = &ss_ep_int_desc_comp;
 	}
 
 	i = 0;
@@ -778,8 +787,11 @@  static void setup_headers(struct f_uac2_opts *opts,
 	if (EPOUT_EN(opts))
 		headers[i++] = USBDHDR(&io_out_ot_desc);
 
-	if (FUOUT_EN(opts) || FUIN_EN(opts))
+	if (FUOUT_EN(opts) || FUIN_EN(opts)) {
 		headers[i++] = USBDHDR(ep_int_desc);
+		if (ep_int_desc_comp)
+			headers[i++] = USBDHDR(ep_int_desc_comp);
+	}
 
 	if (EPOUT_EN(opts)) {
 		headers[i++] = USBDHDR(&std_as_out_if0_desc);