U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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 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 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 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 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

* 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox