linux-kernel - Re: [PATCH v2 3/3] usb: gadget: f_sourcesink: Addition of SSP endpoint for ISOC transfers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250829214916.fqxyu2baer7apkl7@synopsys.com>
Date: Fri, 29 Aug 2025 21:49:17 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Srikanth Chary Chennoju <srikanth.chary-chennoju@....com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "m.grzeschik@...gutronix.de" <m.grzeschik@...gutronix.de>,
        "Chris.Wulff@...mp.com" <Chris.Wulff@...mp.com>,
        "tiwai@...e.de" <tiwai@...e.de>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "punnaiah.choudary.kalluri@....com" <punnaiah.choudary.kalluri@....com>
Subject: Re: [PATCH v2 3/3] usb: gadget: f_sourcesink: Addition of SSP
 endpoint for ISOC transfers

On Thu, Aug 28, 2025, Srikanth Chary Chennoju wrote:
> This patch is created to support super speed plus endpoint for Isoc
> transfers. Now super speed endpoint companion is accompanied by super
> speed plus endpoint companion. With this change we could see the Isoc IN
> and OUT performance reaching to ~749MB/sec which is 96K per uframe.
> The performance numbers are confirmed through Lecroy trace.
> 
> Signed-off-by: Srikanth Chary Chennoju <srikanth.chary-chennoju@....com>
> ---
>  drivers/usb/gadget/function/f_sourcesink.c | 23 ++++++++++++++++++++--
>  include/uapi/linux/usb/ch9.h               |  2 ++
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
> index a3a69166c343..79efb6295725 100644
> --- a/drivers/usb/gadget/function/f_sourcesink.c
> +++ b/drivers/usb/gadget/function/f_sourcesink.c
> @@ -232,6 +232,12 @@ static struct usb_ss_ep_comp_descriptor ss_iso_source_comp_desc = {
>  	.wBytesPerInterval =	cpu_to_le16(1024),
>  };
>  
> +static struct usb_ssp_isoc_ep_comp_descriptor ssp_iso_source_comp_desc = {
> +	.bLength =		USB_DT_SSP_ISOC_EP_COMP_SIZE,
> +	.bDescriptorType =	USB_DT_SSP_ISOC_ENDPOINT_COMP,
> +	.dwBytesPerInterval =	cpu_to_le32(1024),

Why set the dwBytesPerInterval and wBytesPerInterval when they will be
overwritten on bind later?

> +};
> +
>  static struct usb_endpoint_descriptor ss_iso_sink_desc = {
>  	.bLength =		USB_DT_ENDPOINT_SIZE,
>  	.bDescriptorType =	USB_DT_ENDPOINT,
> @@ -250,6 +256,12 @@ static struct usb_ss_ep_comp_descriptor ss_iso_sink_comp_desc = {
>  	.wBytesPerInterval =	cpu_to_le16(1024),
>  };
>  
> +static struct usb_ssp_isoc_ep_comp_descriptor ssp_iso_sink_comp_desc = {
> +	.bLength =		USB_DT_SSP_ISOC_EP_COMP_SIZE,
> +	.bDescriptorType =	USB_DT_SSP_ISOC_ENDPOINT_COMP,
> +	.dwBytesPerInterval =	cpu_to_le32(1024),
> +};
> +
>  static struct usb_descriptor_header *ss_source_sink_descs[] = {
>  	(struct usb_descriptor_header *) &source_sink_intf_alt0,
>  	(struct usb_descriptor_header *) &ss_source_desc,
> @@ -264,8 +276,10 @@ static struct usb_descriptor_header *ss_source_sink_descs[] = {
>  	(struct usb_descriptor_header *) &ss_sink_comp_desc,
>  	(struct usb_descriptor_header *) &ss_iso_source_desc,
>  	(struct usb_descriptor_header *) &ss_iso_source_comp_desc,
> +	(struct usb_descriptor_header *) &ssp_iso_source_comp_desc,
>  	(struct usb_descriptor_header *) &ss_iso_sink_desc,
>  	(struct usb_descriptor_header *) &ss_iso_sink_comp_desc,
> +	(struct usb_descriptor_header *) &ssp_iso_sink_comp_desc,
>  	NULL,
>  };
>  
> @@ -423,7 +437,7 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
>  	 */
>  	ss_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
>  	ss_iso_source_desc.bInterval = ss->isoc_interval;
> -	ss_iso_source_comp_desc.bmAttributes = ss->isoc_mult;
> +	ss_iso_source_comp_desc.bmAttributes = USB_DT_SSP_ISOC_COMP | ss->isoc_mult;
>  	ss_iso_source_comp_desc.bMaxBurst = ss->isoc_maxburst;
>  	ss_iso_source_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
>  		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
> @@ -432,12 +446,17 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
>  
>  	ss_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
>  	ss_iso_sink_desc.bInterval = ss->isoc_interval;
> -	ss_iso_sink_comp_desc.bmAttributes = ss->isoc_mult;
> +	ss_iso_sink_comp_desc.bmAttributes = USB_DT_SSP_ISOC_COMP | ss->isoc_mult;

Don't automatically set USB_DT_SSP_ISOC_COMP if deviec is not connected
in SSP. probably need to create a separate set of descriptors for SSP.

>  	ss_iso_sink_comp_desc.bMaxBurst = ss->isoc_maxburst;
>  	ss_iso_sink_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
>  		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
>  	ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>  
> +	ssp_iso_source_comp_desc.dwBytesPerInterval = ss->isoc_maxpacket *
> +	  (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1) * USB_LANE_SPEED_MANTISSA_GEN2_BY_GEN1;
> +	ssp_iso_sink_comp_desc.dwBytesPerInterval = ss->isoc_maxpacket *
> +	  (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1) * USB_LANE_SPEED_MANTISSA_GEN2_BY_GEN1;
> +

Missing cpu_to_le* endian conversions.

>  	ret = usb_assign_descriptors(f, fs_source_sink_descs,
>  			hs_source_sink_descs, ss_source_sink_descs,
>  			ss_source_sink_descs);
> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> index 8003243a4937..22782c5cb2f3 100644
> --- a/include/uapi/linux/usb/ch9.h
> +++ b/include/uapi/linux/usb/ch9.h
> @@ -702,6 +702,8 @@ struct usb_ssp_isoc_ep_comp_descriptor {
>  } __attribute__ ((packed));
>  
>  #define USB_DT_SSP_ISOC_EP_COMP_SIZE		8
> +#define USB_DT_SSP_ISOC_COMP			(1 << 7) /*support for SSP ISOC EP COMP*/

Put this under bmAttributes

> +#define USB_LANE_SPEED_MANTISSA_GEN2_BY_GEN1    2

There's no GEN2_BY_GEN1. There's Gen2x1. Also lane speed mantissa is not
2 for Gen2x1. What you probably wanted to put is probably gen value
multiply by lane count?

BR,
Thinh

>  
>  /*-------------------------------------------------------------------------*/
>  
> -- 
> 2.25.1
> 

BR,
Thinh