[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<CH3PR12MB87267AEF07FD710412DD6ABFB128A@CH3PR12MB8726.namprd12.prod.outlook.com>
Date: Mon, 11 Aug 2025 15:37:08 +0000
From: "Chary Chennoju, Srikanth" <srikanth.chary-chennoju@....com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: "Thinh.Nguyen@...opsys.com" <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>, "Kalluri,
Punnaiah Choudary" <punnaiah.choudary.kalluri@....com>
Subject: RE: [PATCH 3/3] usb: gadget: f_sourcesink: Addition of SSP endpoint
companion for Isochronous transfers
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Greg,
Please let me know if there are any comments for this patch.
Thanks & Regards,
Srikanth
> -----Original Message-----
> From: Chary Chennoju, Srikanth
> Sent: Monday, July 7, 2025 4:09 PM
> To: Greg KH <gregkh@...uxfoundation.org>
> Cc: Thinh.Nguyen@...opsys.com; m.grzeschik@...gutronix.de;
> Chris.Wulff@...mp.com; tiwai@...e.de; linux-usb@...r.kernel.org; linux-
> kernel@...r.kernel.org; Kalluri, Punnaiah Choudary
> <punnaiah.choudary.kalluri@....com>
> Subject: RE: [PATCH 3/3] usb: gadget: f_sourcesink: Addition of SSP endpoint
> companion for Isochronous transfers
>
> Hi Greg,
>
> Thank you for your comments. Please find the inline comments.
>
> Regards,
> Srikanth
>
> > -----Original Message-----
> > From: Greg KH <gregkh@...uxfoundation.org>
> > Sent: Friday, July 4, 2025 5:34 PM
> > To: Chary Chennoju, Srikanth <srikanth.chary-chennoju@....com>
> > Cc: Thinh.Nguyen@...opsys.com; m.grzeschik@...gutronix.de;
> > Chris.Wulff@...mp.com; tiwai@...e.de; linux-usb@...r.kernel.org;
> > linux- kernel@...r.kernel.org; Kalluri, Punnaiah Choudary
> > <punnaiah.choudary.kalluri@....com>
> > Subject: Re: [PATCH 3/3] usb: gadget: f_sourcesink: Addition of SSP
> > endpoint companion for Isochronous transfers
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On Fri, Jul 04, 2025 at 05:10:13PM +0530, Srikanth Chary Chennoju wrote:
> > > This patch is created to support super speed plus endpoint for
> > > Isochronous 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.
> >
> > You do have a full 72 characters wide, you can use it :) [Srikanth] :
> > I will use 72 characters for next version of patch.
> > >
> > > Signed-off-by: Srikanth Chary Chennoju
> > > <srikanth.chary-chennoju@....com>
> > > ---
> > > drivers/usb/gadget/function/f_sourcesink.c | 23
> > > ++++++++++++++++++++--
> > > 1 file changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/function/f_sourcesink.c
> > > b/drivers/usb/gadget/function/f_sourcesink.c
> > > index 84f3b3bc7669..6499e95e0e9c 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),
> > > +};
> > > +
> > > 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,
> >
> > Odd spacing :(
> >
> > Please follow the format that was previously there.
> > [Srikanth] : When I ran checkpatch script with "strict" option, it throwed out
> two Checks. Please find the snippet below. Due to that I removed the space.
> > CHECK: No space is necessary after a cast
> #53: FILE: drivers/usb/gadget/function/f_sourcesink.c:279:
> + (struct usb_descriptor_header *) &ssp_iso_source_comp_desc,
>
> CHECK: No space is necessary after a cast
> #56: FILE: drivers/usb/gadget/function/f_sourcesink.c:282:
> + (struct usb_descriptor_header *) &ssp_iso_sink_comp_desc,
> > > NULL,
> > > };
> > >
> > > @@ -428,7 +442,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 = 0x80 | ss->isoc_mult;
> >
> > What is 0x80 for? Is that a #define somewhere?
> > [Srikanth] : There is no define. I can create for it.
> > > 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); @@
> > > -437,12 +451,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 = 0x80 | ss->isoc_mult;
> >
> > Same here.
> >
> > > 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) * 2;
> > > + ssp_iso_sink_comp_desc.dwBytesPerInterval = ss->isoc_maxpacket *
> > > + (ss->isoc_mult + 1) * (ss->isoc_maxburst + 1) * 2;
> >
> > Why * 2?
> > [Srikanth] : Please find the attached snippet. For Super Speed, it is 48K bytes
> per uFrame. For Super Speed plus, it is 96Kbytes per uFrame. As per your
> suggestion for the above case, I can create a define here as well.
> > thanks,
> >
> > greg k-h
Powered by blists - more mailing lists