Hi Thinh, Thank you for the patch.
On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <[email protected]> wrote: > From: Dan Tran <[email protected]> > > Add SS bulk endpoint descriptors (1024-byte MPS, bMaxBurst=15) to > storage_common.c to support SuperSpeed connections. Extend fsg_ep_desc() > to select them when operating at SuperSpeed, and wire up ss_descriptors > in fsg_bind(). Free ss_descriptors in fsg_unbind() to match. > > Signed-off-by: Dan Tran <[email protected]> > Signed-off-by: Thinh Nguyen <[email protected]> > --- > Changes in v2: > - Removed internal Reviewed-by tags > > > drivers/usb/gadget/f_mass_storage.c | 25 ++++++++++++-- > drivers/usb/gadget/storage_common.c | 52 ++++++++++++++++++++++++++++- > 2 files changed, 74 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/f_mass_storage.c > b/drivers/usb/gadget/f_mass_storage.c > index 87ed25e8bb3a..a2f34c100482 100644 > --- a/drivers/usb/gadget/f_mass_storage.c > +++ b/drivers/usb/gadget/f_mass_storage.c > @@ -2225,14 +2225,16 @@ reset: > > /* Enable the endpoints */ > d = fsg_ep_desc(common->gadget, > - &fsg_fs_bulk_in_desc, &fsg_hs_bulk_in_desc); > + &fsg_fs_bulk_in_desc, &fsg_hs_bulk_in_desc, > + &fsg_ss_bulk_in_desc); > rc = enable_endpoint(common, fsg->bulk_in, d); > if (rc) > goto reset; > fsg->bulk_in_enabled = 1; > > d = fsg_ep_desc(common->gadget, > - &fsg_fs_bulk_out_desc, &fsg_hs_bulk_out_desc); > + &fsg_fs_bulk_out_desc, &fsg_hs_bulk_out_desc, > + &fsg_ss_bulk_out_desc); > rc = enable_endpoint(common, fsg->bulk_out, d); > if (rc) > goto reset; > @@ -2653,6 +2655,7 @@ static void fsg_unbind(struct usb_configuration *c, > struct usb_function *f) > fsg_common_release(fsg->common); > free(fsg->function.descriptors); > free(fsg->function.hs_descriptors); > + free(fsg->function.ss_descriptors); > kfree(fsg); > } > > @@ -2701,6 +2704,24 @@ static int fsg_bind(struct usb_configuration *c, > struct usb_function *f) > return -ENOMEM; > } > } > + > + if (gadget_is_superspeed(gadget)) { > + unsigned int max_burst = min_t(unsigned int, FSG_BUFLEN / 1024, > 15); I can see that this looks similar to what we have in Linux with commit 4bb99b7c82ba ("usb: gadget: storage: add superspeed support") However, the Linux patch added a comment as well: + /* Calculate bMaxBurst, we know packet size is 1024 */ + max_burst = min_t(unsigned, FSG_BUFLEN / 1024, 15); Why can't we do the same here? > + > + fsg_ss_bulk_in_desc.bEndpointAddress = > + fsg_fs_bulk_in_desc.bEndpointAddress; > + fsg_ss_bulk_in_comp_desc.bMaxBurst = max_burst; > + fsg_ss_bulk_out_desc.bEndpointAddress = > + fsg_fs_bulk_out_desc.bEndpointAddress; > + fsg_ss_bulk_out_comp_desc.bMaxBurst = max_burst; > + f->ss_descriptors = usb_copy_descriptors(fsg_ss_function); > + if (unlikely(!f->ss_descriptors)) { > + free(f->hs_descriptors); > + free(f->descriptors); > + return -ENOMEM; > + } > + } > + > return 0; > > autoconf_fail: > diff --git a/drivers/usb/gadget/storage_common.c > b/drivers/usb/gadget/storage_common.c > index 7e4b542f7ce5..d745649eadb7 100644 > --- a/drivers/usb/gadget/storage_common.c > +++ b/drivers/usb/gadget/storage_common.c > @@ -531,11 +531,61 @@ static struct usb_descriptor_header *fsg_hs_function[] > = { > NULL, > }; > > +/* > + * USB 3.0 requires SuperSpeed descriptors > + */ > +static struct usb_endpoint_descriptor > +fsg_ss_bulk_in_desc = { > + .bLength = USB_DT_ENDPOINT_SIZE, > + .bDescriptorType = USB_DT_ENDPOINT, > + > + /* bEndpointAddress copied from fs_bulk_in_desc during fsg_bind() */ > + .bmAttributes = USB_ENDPOINT_XFER_BULK, > + .wMaxPacketSize = cpu_to_le16(1024), > +}; > + > +static struct usb_ss_ep_comp_descriptor fsg_ss_bulk_in_comp_desc = { > + .bLength = sizeof(fsg_ss_bulk_in_comp_desc), > + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP, > + /* bMaxBurst set during fsg_bind() */ > +}; > + > +static struct usb_endpoint_descriptor > +fsg_ss_bulk_out_desc = { > + .bLength = USB_DT_ENDPOINT_SIZE, > + .bDescriptorType = USB_DT_ENDPOINT, > + > + /* bEndpointAddress copied from fs_bulk_out_desc during fsg_bind() */ > + .bmAttributes = USB_ENDPOINT_XFER_BULK, > + .wMaxPacketSize = cpu_to_le16(1024), > +}; > + > +static struct usb_ss_ep_comp_descriptor fsg_ss_bulk_out_comp_desc = { > + .bLength = sizeof(fsg_ss_bulk_out_comp_desc), > + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP, > + /* bMaxBurst set during fsg_bind() */ > +}; > + > +static struct usb_descriptor_header *fsg_ss_function[] = { > +#ifndef FSG_NO_OTG > + (struct usb_descriptor_header *)&fsg_otg_desc, > +#endif > + (struct usb_descriptor_header *)&fsg_intf_desc, > + (struct usb_descriptor_header *)&fsg_ss_bulk_in_desc, > + (struct usb_descriptor_header *)&fsg_ss_bulk_in_comp_desc, > + (struct usb_descriptor_header *)&fsg_ss_bulk_out_desc, > + (struct usb_descriptor_header *)&fsg_ss_bulk_out_comp_desc, > + NULL, > +}; > + > /* Maxpacket and other transfer characteristics vary by speed. */ > static struct usb_endpoint_descriptor * > fsg_ep_desc(struct usb_gadget *g, struct usb_endpoint_descriptor *fs, > - struct usb_endpoint_descriptor *hs) > + struct usb_endpoint_descriptor *hs, > + struct usb_endpoint_descriptor *ss) > { > + if (g->speed >= USB_SPEED_SUPER) > + return ss; I can see that this looks similar to what we have in Linux with commit 4bb99b7c82ba ("usb: gadget: storage: add superspeed support") However, Linux uses the following diff instead: + if (gadget_is_superspeed(g) && g->speed == USB_SPEED_SUPER) + return ss; Is there a reason for not doing the same here? > if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH) > return hs; > return fs; > -- > 2.53.0
