[<prev] [next>] [<thread-prev] [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
Powered by blists - more mailing lists