* [PATCH v2 0/2] usb: gadget: add SuperSpeed descriptor support @ 2026-07-09 18:19 Thinh Nguyen 2026-07-09 18:19 ` [PATCH v2 1/2] usb: gadget: mass_storage: " Thinh Nguyen 2026-07-09 18:19 ` [PATCH v2 2/2] usb: gadget: dfu: " Thinh Nguyen 0 siblings, 2 replies; 8+ messages in thread From: Thinh Nguyen @ 2026-07-09 18:19 UTC (permalink / raw) To: Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Mattijs Korpershoek, Patrice Chotard Cc: Tejas Narendra Joglekar, u-boot@lists.denx.de This series adds SuperSpeed descriptor support for the mass_storage and DFU gadget functions so they can operate correctly at SuperSpeed. Dan Tran (2): usb: gadget: mass_storage: add SuperSpeed descriptor support usb: gadget: dfu: add SuperSpeed descriptor support drivers/usb/gadget/f_dfu.c | 6 ++++ drivers/usb/gadget/f_mass_storage.c | 25 ++++++++++++-- drivers/usb/gadget/storage_common.c | 52 ++++++++++++++++++++++++++++- 3 files changed, 80 insertions(+), 3 deletions(-) Changes in v2: - Removed internal Reviewed-by tags base-commit: 964a26cc6d1002343ceba393d86ee4a0cd447385 -- 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] usb: gadget: mass_storage: add SuperSpeed descriptor support 2026-07-09 18:19 [PATCH v2 0/2] usb: gadget: add SuperSpeed descriptor support Thinh Nguyen @ 2026-07-09 18:19 ` Thinh Nguyen 2026-07-23 12:28 ` Mattijs Korpershoek 2026-07-09 18:19 ` [PATCH v2 2/2] usb: gadget: dfu: " Thinh Nguyen 1 sibling, 1 reply; 8+ messages in thread From: Thinh Nguyen @ 2026-07-09 18:19 UTC (permalink / raw) To: Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Mattijs Korpershoek, Patrice Chotard Cc: Tejas Narendra Joglekar, u-boot@lists.denx.de From: Dan Tran <trandan@synopsys.com> 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 <trandan@synopsys.com> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- 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); + + 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; if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH) return hs; return fs; -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] usb: gadget: mass_storage: add SuperSpeed descriptor support 2026-07-09 18:19 ` [PATCH v2 1/2] usb: gadget: mass_storage: " Thinh Nguyen @ 2026-07-23 12:28 ` Mattijs Korpershoek 2026-07-23 20:40 ` Thinh Nguyen 0 siblings, 1 reply; 8+ messages in thread From: Mattijs Korpershoek @ 2026-07-23 12:28 UTC (permalink / raw) To: Thinh Nguyen, Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Mattijs Korpershoek, Patrice Chotard Cc: Tejas Narendra Joglekar, u-boot@lists.denx.de Hi Thinh, Thank you for the patch. On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > From: Dan Tran <trandan@synopsys.com> > > 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 <trandan@synopsys.com> > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] usb: gadget: mass_storage: add SuperSpeed descriptor support 2026-07-23 12:28 ` Mattijs Korpershoek @ 2026-07-23 20:40 ` Thinh Nguyen 2026-07-24 12:25 ` Mattijs Korpershoek 0 siblings, 1 reply; 8+ messages in thread From: Thinh Nguyen @ 2026-07-23 20:40 UTC (permalink / raw) To: Mattijs Korpershoek Cc: Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Patrice Chotard, Tejas Narendra Joglekar, u-boot@lists.denx.de Hi Mattijs, On Thu, Jul 23, 2026, Mattijs Korpershoek wrote: > Hi Thinh, > > Thank you for the patch. > > On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > From: Dan Tran <trandan@synopsys.com> > > > > 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 <trandan@synopsys.com> > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > --- > > 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? The comment doesn't add much beyond what the code already expresses. The variable name max_burst and the division by 1024 make the intent clear. That said, the Linux comment also has a minor inaccuracy: it says "packet size" when it should say "max packet size". If a comment is warranted, I'd prefer to add a corrected one. We can add it if you really think it helps with readability. > > > + > > + 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? The gadget_is_superspeed() is a hardware capability check. It's redundant when we're already checking g->speed for connected speed. BR, Thinh > > > if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH) > > return hs; > > return fs; > > -- > > 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] usb: gadget: mass_storage: add SuperSpeed descriptor support 2026-07-23 20:40 ` Thinh Nguyen @ 2026-07-24 12:25 ` Mattijs Korpershoek 0 siblings, 0 replies; 8+ messages in thread From: Mattijs Korpershoek @ 2026-07-24 12:25 UTC (permalink / raw) To: Thinh Nguyen Cc: Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Patrice Chotard, Tejas Narendra Joglekar, u-boot@lists.denx.de Hi Thinh, On Thu, Jul 23, 2026 at 20:40, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > Hi Mattijs, > > On Thu, Jul 23, 2026, Mattijs Korpershoek wrote: >> Hi Thinh, >> >> Thank you for the patch. >> >> On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: >> >> > From: Dan Tran <trandan@synopsys.com> >> > >> > 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 <trandan@synopsys.com> >> > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> > --- >> > 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? > > The comment doesn't add much beyond what the code already expresses. The > variable name max_burst and the division by 1024 make the intent clear. > > That said, the Linux comment also has a minor inaccuracy: it says > "packet size" when it should say "max packet size". If a comment is > warranted, I'd prefer to add a corrected one. We can add it if you > really think it helps with readability. Overall, my rule of thumb is "keep the code as close as possible to the Linux driver to ease maintenance in U-Boot". Please consider adding the corrected comment for v3. > >> >> > + >> > + 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 [...] >> > + (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? > > The gadget_is_superspeed() is a hardware capability check. It's > redundant when we're already checking g->speed for connected speed. So does that mean that the dualspeed conditional just below is doing a redundant check as well? As for the previous comment, I'd prefer if we can stay closer to the Linux code. If we can't, I'd like to see a strong justification for not doing so. > > BR, > Thinh > >> >> > if (gadget_is_dualspeed(g) && g->speed == USB_SPEED_HIGH) >> > return hs; >> > return fs; >> > -- >> > 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] usb: gadget: dfu: add SuperSpeed descriptor support 2026-07-09 18:19 [PATCH v2 0/2] usb: gadget: add SuperSpeed descriptor support Thinh Nguyen 2026-07-09 18:19 ` [PATCH v2 1/2] usb: gadget: mass_storage: " Thinh Nguyen @ 2026-07-09 18:19 ` Thinh Nguyen 2026-07-23 12:36 ` Mattijs Korpershoek 1 sibling, 1 reply; 8+ messages in thread From: Thinh Nguyen @ 2026-07-09 18:19 UTC (permalink / raw) To: Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Mattijs Korpershoek Cc: Tejas Narendra Joglekar, u-boot@lists.denx.de From: Dan Tran <trandan@synopsys.com> Populate ss_descriptors to support SuperSpeed connections. DFU is control-only so no separate SS descriptor set is needed; reuse the same descriptors across all speeds. Signed-off-by: Dan Tran <trandan@synopsys.com> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- Changes in v2: - Removed internal Reviewed-by tags drivers/usb/gadget/f_dfu.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index ca8b36e077bc..68d2ec7ebbe8 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -227,6 +227,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu) f_dfu->usb_function.strings = dfu_strings; f_dfu->usb_function.hs_descriptors = f_dfu->function; f_dfu->usb_function.descriptors = f_dfu->function; + f_dfu->usb_function.ss_descriptors = f_dfu->function; f_dfu->dfu_state = DFU_STATE_dfuIDLE; } @@ -235,6 +236,7 @@ static inline void to_runtime_mode(struct f_dfu *f_dfu) f_dfu->usb_function.strings = NULL; f_dfu->usb_function.hs_descriptors = dfu_runtime_descs; f_dfu->usb_function.descriptors = dfu_runtime_descs; + f_dfu->usb_function.ss_descriptors = dfu_runtime_descs; } static int handle_upload(struct usb_request *req, u16 len) @@ -752,6 +754,9 @@ static int dfu_bind(struct usb_configuration *c, struct usb_function *f) if (s) g_dnl_set_serialnumber((char *)s); + if (gadget_is_superspeed(cdev->gadget)) + f_dfu->usb_function.ss_descriptors = f_dfu->function; + error: return rv; } @@ -826,6 +831,7 @@ static int dfu_bind_config(struct usb_configuration *c) f_dfu->usb_function.name = "dfu"; f_dfu->usb_function.hs_descriptors = dfu_runtime_descs; f_dfu->usb_function.descriptors = dfu_runtime_descs; + f_dfu->usb_function.ss_descriptors = dfu_runtime_descs; f_dfu->usb_function.bind = dfu_bind; f_dfu->usb_function.unbind = dfu_unbind; f_dfu->usb_function.set_alt = dfu_set_alt; -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: dfu: add SuperSpeed descriptor support 2026-07-09 18:19 ` [PATCH v2 2/2] usb: gadget: dfu: " Thinh Nguyen @ 2026-07-23 12:36 ` Mattijs Korpershoek 2026-07-23 20:43 ` Thinh Nguyen 0 siblings, 1 reply; 8+ messages in thread From: Mattijs Korpershoek @ 2026-07-23 12:36 UTC (permalink / raw) To: Thinh Nguyen, Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Mattijs Korpershoek Cc: Tejas Narendra Joglekar, u-boot@lists.denx.de Hi Thinh, Thank you for the patch. On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > From: Dan Tran <trandan@synopsys.com> > > Populate ss_descriptors to support SuperSpeed connections. DFU is > control-only so no separate SS descriptor set is needed; reuse the > same descriptors across all speeds. > > Signed-off-by: Dan Tran <trandan@synopsys.com> > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > Changes in v2: > - Removed internal Reviewed-by tags > > drivers/usb/gadget/f_dfu.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c > index ca8b36e077bc..68d2ec7ebbe8 100644 > --- a/drivers/usb/gadget/f_dfu.c > +++ b/drivers/usb/gadget/f_dfu.c > @@ -227,6 +227,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu) > f_dfu->usb_function.strings = dfu_strings; > f_dfu->usb_function.hs_descriptors = f_dfu->function; > f_dfu->usb_function.descriptors = f_dfu->function; > + f_dfu->usb_function.ss_descriptors = f_dfu->function; > f_dfu->dfu_state = DFU_STATE_dfuIDLE; > } > > @@ -235,6 +236,7 @@ static inline void to_runtime_mode(struct f_dfu *f_dfu) > f_dfu->usb_function.strings = NULL; > f_dfu->usb_function.hs_descriptors = dfu_runtime_descs; > f_dfu->usb_function.descriptors = dfu_runtime_descs; > + f_dfu->usb_function.ss_descriptors = dfu_runtime_descs; > } > > static int handle_upload(struct usb_request *req, u16 len) > @@ -752,6 +754,9 @@ static int dfu_bind(struct usb_configuration *c, struct usb_function *f) > if (s) > g_dnl_set_serialnumber((char *)s); > > + if (gadget_is_superspeed(cdev->gadget)) > + f_dfu->usb_function.ss_descriptors = f_dfu->function; > + Can you explain why this conditional is needed? I don't see something similar for high speed so this is confusing me. > error: > return rv; > } > @@ -826,6 +831,7 @@ static int dfu_bind_config(struct usb_configuration *c) > f_dfu->usb_function.name = "dfu"; > f_dfu->usb_function.hs_descriptors = dfu_runtime_descs; > f_dfu->usb_function.descriptors = dfu_runtime_descs; > + f_dfu->usb_function.ss_descriptors = dfu_runtime_descs; > f_dfu->usb_function.bind = dfu_bind; > f_dfu->usb_function.unbind = dfu_unbind; > f_dfu->usb_function.set_alt = dfu_set_alt; > -- > 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] usb: gadget: dfu: add SuperSpeed descriptor support 2026-07-23 12:36 ` Mattijs Korpershoek @ 2026-07-23 20:43 ` Thinh Nguyen 0 siblings, 0 replies; 8+ messages in thread From: Thinh Nguyen @ 2026-07-23 20:43 UTC (permalink / raw) To: Mattijs Korpershoek Cc: Thinh Nguyen, Dan Tran, Marek Vasut, Lukasz Majewski, Tom Rini, Tejas Narendra Joglekar, u-boot@lists.denx.de On Thu, Jul 23, 2026, Mattijs Korpershoek wrote: > Hi Thinh, > > Thank you for the patch. > > On Thu, Jul 09, 2026 at 18:19, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > From: Dan Tran <trandan@synopsys.com> > > > > Populate ss_descriptors to support SuperSpeed connections. DFU is > > control-only so no separate SS descriptor set is needed; reuse the > > same descriptors across all speeds. > > > > Signed-off-by: Dan Tran <trandan@synopsys.com> > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > --- > > Changes in v2: > > - Removed internal Reviewed-by tags > > > > drivers/usb/gadget/f_dfu.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c > > index ca8b36e077bc..68d2ec7ebbe8 100644 > > --- a/drivers/usb/gadget/f_dfu.c > > +++ b/drivers/usb/gadget/f_dfu.c > > @@ -227,6 +227,7 @@ static inline void to_dfu_mode(struct f_dfu *f_dfu) > > f_dfu->usb_function.strings = dfu_strings; > > f_dfu->usb_function.hs_descriptors = f_dfu->function; > > f_dfu->usb_function.descriptors = f_dfu->function; > > + f_dfu->usb_function.ss_descriptors = f_dfu->function; > > f_dfu->dfu_state = DFU_STATE_dfuIDLE; > > } > > > > @@ -235,6 +236,7 @@ static inline void to_runtime_mode(struct f_dfu *f_dfu) > > f_dfu->usb_function.strings = NULL; > > f_dfu->usb_function.hs_descriptors = dfu_runtime_descs; > > f_dfu->usb_function.descriptors = dfu_runtime_descs; > > + f_dfu->usb_function.ss_descriptors = dfu_runtime_descs; > > } > > > > static int handle_upload(struct usb_request *req, u16 len) > > @@ -752,6 +754,9 @@ static int dfu_bind(struct usb_configuration *c, struct usb_function *f) > > if (s) > > g_dnl_set_serialnumber((char *)s); > > > > + if (gadget_is_superspeed(cdev->gadget)) > > + f_dfu->usb_function.ss_descriptors = f_dfu->function; > > + > > Can you explain why this conditional is needed? I don't see something > similar for high speed so this is confusing me. You're right. It's not needed. We can remove it. Thanks, Thinh ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-24 12:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-09 18:19 [PATCH v2 0/2] usb: gadget: add SuperSpeed descriptor support Thinh Nguyen 2026-07-09 18:19 ` [PATCH v2 1/2] usb: gadget: mass_storage: " Thinh Nguyen 2026-07-23 12:28 ` Mattijs Korpershoek 2026-07-23 20:40 ` Thinh Nguyen 2026-07-24 12:25 ` Mattijs Korpershoek 2026-07-09 18:19 ` [PATCH v2 2/2] usb: gadget: dfu: " Thinh Nguyen 2026-07-23 12:36 ` Mattijs Korpershoek 2026-07-23 20:43 ` Thinh Nguyen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.