Re: [PATCH v2 1/2] usb: gadget: mass_storage: add SuperSpeed descriptor support

[Mattijs Korpershoek <mkorpershoek@kernel.org>]

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