usb:gadget: Add SuperSpeed support to the Gadget Framework

This patch adds the SuperSpeed functionality to the gadget framework. In order not to force all the gadget drivers to supply SuperSpeed descriptors when operating in SuperSpeed mode the following approach was taken:
If we're operating in SuperSpeed mode and the gadget driver didn't supply SuperSpeed descriptors, the composite layer will automatically create SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added.
Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was added.

Signed-off-by Tatyana Brokhman <[email protected]>

drivers/usb/gadget/composite.c +97 -0

+/**
+ * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * with default values
+ * @f: pointer to usb_function to generate the descriptors for
+ *
+ * This function receives a pointer to usb_function and adds
+ * missing super speed descriptors in the ss_descriptor field
+ * according to its hs_descriptors field.
+ *
+ * This function copies f->hs_descriptors while updating the
+ * endpoint descriptor and adding endpoint companion descriptor.
+ */
+static void create_ss_descriptors(struct usb_function *f)
+{
+	unsigned bytes;		/* number of bytes to allocate */
+	unsigned n_desc;	/* number of descriptors */
+	void *mem;		/* allocated memory to copy to */
+	struct usb_descriptor_header **tmp;
+	struct usb_endpoint_descriptor	*ep_desc ;
+	struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+	struct usb_descriptor_header **src = f->hs_descriptors;
+
+	if (!f->hs_descriptors)
+		return;
+
+	/*
+	 * Count number of EPs (in order to know how many SS_EP_COMPANION
+	 * descriptors to add), the total number of descriptors and the sum of
+	 * each descriptor bLength field in order to know how much memory to
+	 * allocate.
+	 */
+	for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
+		if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
+			bytes += default_ep_comp_desc.bLength;
+			n_desc++;
+		}
+		bytes += (*tmp)->bLength;
+	}
+
+	bytes += (n_desc + 1) * sizeof(*tmp);
+	mem = kmalloc(bytes, GFP_KERNEL);
+	if (!mem)
+		return;
+
+	/*
+	 * Fill in pointers starting at "tmp", to descriptors copied starting
+	 * at "mem" and return "ret"
+	 */
+	tmp = mem;
+	f->ss_descriptors  = mem;
+	mem += (n_desc + 1) * sizeof(*tmp);
+	while (*src) {
+		/* Copy the original descriptor */
+		memcpy(mem, *src, (*src)->bLength);
+		switch ((*src)->bDescriptorType) {
+		case USB_DT_ENDPOINT:
+			/* update ep descriptor */
+			ep_desc = (struct usb_endpoint_descriptor *)mem;
+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_CONTROL:
+				ep_desc->wMaxPacketSize = cpu_to_le16(512);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_BULK:
+				ep_desc->wMaxPacketSize = cpu_to_le16(1024);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				break;
+			}
+			*tmp = mem;
+			tmp++;
+			mem += (*src)->bLength;
+			/* add ep companion descriptor */
+			memcpy(mem, &default_ep_comp_desc,
+			       default_ep_comp_desc.bLength);
+			*tmp = mem;
+			tmp++;
+			/* Update wBytesPerInterval for periodic endpoints */
+			ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				ep_comp_desc->wBytesPerInterval =
+					ep_desc->wMaxPacketSize;
+				break;
+			}
+			mem += default_ep_comp_desc.bLength;
+			break;
+		default:
+			*tmp = mem;
+			tmp++;
+			mem += (*src)->bLength;
+			break;

drivers/usb/gadget/composite.c +4 -0

+
 /*-------------------------------------------------------------------------*/
 /**
  * next_ep_desc() - advance to the next EP descriptor

@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
 	struct usb_endpoint_descriptor *chosen_desc = NULL;
 	struct usb_descriptor_header **speed_desc = NULL;
 
+	struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
+	int want_comp_desc = 0;
+
 	struct usb_descriptor_header **d_spd; /* cursor for speed desc */
 
 	if (!g || !f || !_ep)

drivers/usb/gadget/composite.c +4 -3

 	INFO(cdev, "%s speed config #%d: %s\n",
 		({ char *speed;
 		switch (gadget->speed) {
-		case USB_SPEED_LOW:	speed = "low"; break;
-		case USB_SPEED_FULL:	speed = "full"; break;
-		case USB_SPEED_HIGH:	speed = "high"; break;
+		case USB_SPEED_LOW:
+			speed = "low";
+			break;
+		case USB_SPEED_FULL:

Quoted 3 lines

This patch adds the SuperSpeed functionality to the gadget framework. In order not to force all the gadget drivers to supply SuperSpeed descriptors when operating in SuperSpeed mode the following approach

was

Quoted 2 lines

taken:
If we're operating in SuperSpeed mode and the gadget driver didn't

supply

Quoted 4 lines

SuperSpeed descriptors, the composite layer will automatically create SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added. Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode

was

Quoted 1 line

added.

IMHO, this patch tries to do too much in one huge patch. It needs major splitting.

Also, I don't get why you decided to go the path of letting composite.c generate the super speed descriptors instead of expecting gadget drivers
to pass those should they support super speed.

I would like the other way much better: first, it's what we already do for full/low speed and high speed. Second, we can easily see the descriptors by opening the gadget driver's code.

There are a few more comments below, but this patch needs major, major splitting.

Quoted 2 lines

Signed-off-by Tatyana Brokhman <[email protected]>

missing the tear line --- here

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/usb/gadget/composite.c

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber

Quoted 4 lines

index fb7e488..d5fe1c2 100644

drivers/usb/gadget/composite.c +0 -0

@@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber

string");

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string"); · line 79

Quoted 7 lines

drivers/usb/gadget/composite.c +4 -0

 static char composite_manufacturer[50];
 
+/* Default endpoint companion descriptor */
+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
+		.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+		.bLength = 0x06,

there's a define for this:

#define USB_DT_SS_EP_COMP_SIZE 6

Quoted 18 lines

drivers/usb/gadget/composite.c +18 -0

+/**
+ * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * with default values
+ * @f: pointer to usb_function to generate the descriptors for
+ *
+ * This function receives a pointer to usb_function and adds
+ * missing super speed descriptors in the ss_descriptor field
+ * according to its hs_descriptors field.
+ *
+ * This function copies f->hs_descriptors while updating the
+ * endpoint descriptor and adding endpoint companion descriptor.
+ */
+static void create_ss_descriptors(struct usb_function *f)
+{
+	unsigned bytes;		/* number of bytes to allocate */
+	unsigned n_desc;	/* number of descriptors */
+	void *mem;		/* allocated memory to copy to */
+	struct usb_descriptor_header **tmp;

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");

Quoted 10 lines

drivers/usb/gadget/composite.c +10 -0

+	struct usb_endpoint_descriptor	*ep_desc ;
+	struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+	struct usb_descriptor_header **src = f->hs_descriptors;
+
+	if (!f->hs_descriptors)
+		return;
+
+	/*
+	 * Count number of EPs (in order to know how many SS_EP_COMPANION
+	 * descriptors to add), the total number of descriptors and the

sum of

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber · line 107

Quoted 1 line

drivers/usb/gadget/composite.c +1 -0

+	 * each descriptor bLength field in order to know how much memory

to

Quoted 7 lines

drivers/usb/gadget/composite.c +7 -0

+	 * allocate.
+	 */
+	for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
+		if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
+			bytes += default_ep_comp_desc.bLength;
+			n_desc++;
+		}

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");

Quoted 10 lines

drivers/usb/gadget/composite.c +10 -0

+		bytes += (*tmp)->bLength;
+	}
+
+	bytes += (n_desc + 1) * sizeof(*tmp);
+	mem = kmalloc(bytes, GFP_KERNEL);
+	if (!mem)
+		return;
+
+	/*
+	 * Fill in pointers starting at "tmp", to descriptors copied

starting

Quoted 27 lines

drivers/usb/gadget/composite.c +27 -0

+	 * at "mem" and return "ret"
+	 */
+	tmp = mem;
+	f->ss_descriptors  = mem;
+	mem += (n_desc + 1) * sizeof(*tmp);
+	while (*src) {
+		/* Copy the original descriptor */
+		memcpy(mem, *src, (*src)->bLength);
+		switch ((*src)->bDescriptorType) {
+		case USB_DT_ENDPOINT:
+			/* update ep descriptor */
+			ep_desc = (struct usb_endpoint_descriptor *)mem;
+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_CONTROL:
+				ep_desc->wMaxPacketSize = cpu_to_le16(512);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_BULK:
+				ep_desc->wMaxPacketSize = cpu_to_le16(1024);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				break;
+			}
+			*tmp = mem;

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");

Quoted 8 lines

drivers/usb/gadget/composite.c +8 -0

+			tmp++;
+			mem += (*src)->bLength;
+			/* add ep companion descriptor */
+			memcpy(mem, &default_ep_comp_desc,
+			       default_ep_comp_desc.bLength);
+			*tmp = mem;
+			tmp++;
+			/* Update wBytesPerInterval for periodic endpoints

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber · line 160

Quoted 1 line

drivers/usb/gadget/composite.c +1 -0

+			ep_comp_desc = (struct usb_ss_ep_comp_descriptor

*)mem;

Quoted 15 lines

drivers/usb/gadget/composite.c +15 -0

+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				ep_comp_desc->wBytesPerInterval =
+					ep_desc->wMaxPacketSize;
+				break;
+			}
+			mem += default_ep_comp_desc.bLength;
+			break;
+		default:
+			*tmp = mem;
+			tmp++;
+			mem += (*src)->bLength;
+			break;

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string"); · line 191

Quoted 10 lines

drivers/usb/gadget/composite.c +10 -0

+		}
+		src++;
+	}
+	/*
+	 * The last (struct usb_descriptor_header *) in the descriptors
+	 * vector is NULL
+	 */
+	*tmp = NULL;
+	f->ss_desc_allocated = true;
+}

I would rather expect gadget drivers to provide their own descriptors just like we do for full/low speed and high speed descriptors. Why you decided to take this approach ?

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber · line 187

Quoted 2 lines

drivers/usb/gadget/composite.c +1 -0

+
 /*------------------------------------------------------------------

-------*/

Quoted 13 lines

drivers/usb/gadget/composite.c +3 -0

 /**
  * next_ep_desc() - advance to the next EP descriptor

@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
 	struct usb_endpoint_descriptor *chosen_desc = NULL;
 	struct usb_descriptor_header **speed_desc = NULL;
 
+	struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
+	int want_comp_desc = 0;
+
 	struct usb_descriptor_header **d_spd; /* cursor for speed desc */
 
 	if (!g || !f || !_ep)

drivers/usb/gadget/composite.c int config_ep_by_speed(struct usb_gadget *g, · line 253

Quoted 9 lines

drivers/usb/gadget/composite.c +6 -0

@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
 
 	/* select desired speed */
 	switch (g->speed) {
+	case USB_SPEED_SUPER:
+		if (gadget_is_superspeed(g)) {
+			speed_desc = f->ss_descriptors;
+			want_comp_desc = 1;
+			break;
+		}

same here, I would expect gadget drivers to be changed providing the correct descriptors. Just like I did when I sent my version.

In my opinion, it would be easier that way. We could see the descriptors
just by opening the gadget driver code. Besides, different gadget drivers might want different "sane defaults".

Quoted 10 lines

drivers/usb/gadget/composite.c +4 -3

 	INFO(cdev, "%s speed config #%d: %s\n",
 		({ char *speed;
 		switch (gadget->speed) {
-		case USB_SPEED_LOW:	speed = "low"; break;
-		case USB_SPEED_FULL:	speed = "full"; break;
-		case USB_SPEED_HIGH:	speed = "high"; break;
+		case USB_SPEED_LOW:
+			speed = "low";
+			break;
+		case USB_SPEED_FULL:

drivers/usb/gadget/composite.c static int set_config(struct usb_composite_dev *cdev, · line 748

Quoted 10 lines

drivers/usb/gadget/composite.c +8 -0

+			speed = "full";
+			break;
+		case USB_SPEED_HIGH:
+			speed = "high";
+			break;
+		case USB_SPEED_SUPER:
+			speed = "super";
+			break;
 		default:		speed = "?"; break;
 		} ; speed; }), number, c ? c->label : "unconfigured");

this part isn't part of this patch.

--
balbi

Hi

Quoted 19 lines · Tatyana Brokhman

Quoted 3 lines

This patch adds the SuperSpeed functionality to the gadget framework. In order not to force all the gadget drivers to supply SuperSpeed descriptors when operating in SuperSpeed mode the following approach

was

Quoted 2 lines

taken:
If we're operating in SuperSpeed mode and the gadget driver didn't

supply

Quoted 4 lines

SuperSpeed descriptors, the composite layer will automatically create SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added. Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode

was

Quoted 1 line

added.

IMHO, this patch tries to do too much in one huge patch. It needs major splitting.

Quoted 9 lines

Also, I don't get why you decided to go the path of letting composite.c generate the super speed descriptors instead of expecting gadget drivers
to pass those should they support super speed.

I would like the other way much better: first, it's what we already do for full/low speed and high speed. Second, we can easily see the descriptors by opening the gadget driver's code.

Our goal was to provide a generic solution that will enable existing gadget drivers to operate in SS mode with minimum changes to their code. If a certain gadget driver wishes to operate in SS mode and the values in the descriptors are important to it, this FD should define its own SS descriptors as already done for HS/LS and those (SS) descriptors will be chosen.

Please note that a gadget driver can also choose not to have SS descriptors generated for it all, even if it didn't provide any of its own and thus choosing not to operate in SS mode.

If you look at our implementation of the UAS gadget driver you'll see that we define the SS descriptors just as is done for HS/LS and those descriptors are chosen in enumeration.

I believe that in the future all gadget drivers will define their own SS descriptors with values that are suitable for that gadget driver and the create_ss_descriptors() will become obsolete. As I've already mentioned this is just a way to allow all the existing drivers to operate in SS mode with minimum changes to their code. So this patch doesn't contradict your approach, just extends it.

Quoted 2 lines

There are a few more comments below, but this patch needs major, major splitting.

I'll address your comments and split it up in the next version. I would like to give Greg and others a chance to comment as well before uploading another version.

Quoted 1 line

I believe that in the future all gadget drivers will define their own

SS

Quoted 1 line

descriptors with values that are suitable for that gadget driver and

the

Quoted 1 line

create_ss_descriptors() will become obsolete. As I've already

mentioned this

Quoted 1 line

is just a way to allow all the existing drivers to operate in SS mode

with

Quoted 2 lines

minimum changes to their code. So this patch doesn't contradict your approach, just extends it.

but that's not a good approach. You're adding bloat which will be removed soon. Problem is that once it's there, we will have to support it for a long long time as we won't know if there are any out of tree gadget drivers depending on that, forcing us on publising of Documentation/feature-removal-schedule.txt that we will remove that layer in e.g. 4 major revisions. So, IMHO, better not to add it.

This patch adds the SuperSpeed functionality to the gadget framework. In order not to force all the gadget drivers to supply SuperSpeed descriptors when operating in SuperSpeed mode the following approach was taken:
If we're operating in SuperSpeed mode and the gadget driver didn't supply SuperSpeed descriptors, the composite layer will automatically create SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added.
Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was added.

Signed-off-by Tatyana Brokhman <[email protected]>

drivers/usb/gadget/Kconfig +0 -0

index bc5123c..b4130bc 100644

drivers/usb/gadget/Kconfig +7 -0

+	help
+	  Enabling this feature enables Super Speed support in the Gadget
+	  driver. It means that gadget drivers should provide extra (SuperSpeed)
+	  descriptors to the host.
+	  For composite devices: if SuperSpeed descriptors weren't supplied by
+	  the FD, they will be automatically generated with default values.
+
 #
 # USB Gadget Drivers
 #

drivers/usb/gadget/composite.c +0 -0

index fb7e488..d5fe1c2 100644

drivers/usb/gadget/composite.c +9 -0

+};
+
+/**
+ * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * with default values
+ * @f: pointer to usb_function to generate the descriptors for
+ *
+ * This function receives a pointer to usb_function and adds
+ * missing super speed descriptors in the ss_descriptor field

drivers/usb/gadget/composite.c +51 -0

+
+	if (!f->hs_descriptors)
+		return;
+
+	/*
+	 * Count number of EPs (in order to know how many SS_EP_COMPANION
+	 * descriptors to add), the total number of descriptors and the sum of
+	 * each descriptor bLength field in order to know how much memory to
+	 * allocate.
+	 */
+	for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
+		if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
+			bytes += default_ep_comp_desc.bLength;
+			n_desc++;
+		}
+		bytes += (*tmp)->bLength;
+	}
+
+	bytes += (n_desc + 1) * sizeof(*tmp);
+	mem = kmalloc(bytes, GFP_KERNEL);
+	if (!mem)
+		return;
+
+	/*
+	 * Fill in pointers starting at "tmp", to descriptors copied starting
+	 * at "mem" and return "ret"
+	 */
+	tmp = mem;
+	f->ss_descriptors  = mem;
+	mem += (n_desc + 1) * sizeof(*tmp);
+	while (*src) {
+		/* Copy the original descriptor */
+		memcpy(mem, *src, (*src)->bLength);
+		switch ((*src)->bDescriptorType) {
+		case USB_DT_ENDPOINT:
+			/* update ep descriptor */
+			ep_desc = (struct usb_endpoint_descriptor *)mem;
+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_CONTROL:
+				ep_desc->wMaxPacketSize = cpu_to_le16(512);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_BULK:
+				ep_desc->wMaxPacketSize = cpu_to_le16(1024);
+				ep_desc->bInterval = 0;
+				break;
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				break;
+			}

drivers/usb/gadget/composite.c +71 -1

+			switch (ep_desc->bmAttributes &
+				USB_ENDPOINT_XFERTYPE_MASK) {
+			case USB_ENDPOINT_XFER_INT:
+			case USB_ENDPOINT_XFER_ISOC:
+				ep_comp_desc->wBytesPerInterval =
+					ep_desc->wMaxPacketSize;
+				break;
+			}
+			mem += default_ep_comp_desc.bLength;
+			break;
+		default:
+			*tmp = mem;
+			tmp++;
+			mem += (*src)->bLength;
+			break;
+		}
+		src++;
+	}
+	/*
+	 * The last (struct usb_descriptor_header *) in the descriptors
+	 * vector is NULL
+	 */
+	*tmp = NULL;
+	f->ss_desc_allocated = true;
+}
+
 /*-------------------------------------------------------------------------*/
 /**
 * next_ep_desc() - advance to the next EP descriptor

@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
 	struct usb_endpoint_descriptor *chosen_desc = NULL;
 	struct usb_descriptor_header **speed_desc = NULL;
 
+	struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
+	int want_comp_desc = 0;
+
 	struct usb_descriptor_header **d_spd; /* cursor for speed desc */
 
 	if (!g || !f || !_ep)

@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
 
 	/* select desired speed */
 	switch (g->speed) {
+	case USB_SPEED_SUPER:
+		if (gadget_is_superspeed(g)) {
+			speed_desc = f->ss_descriptors;
+			want_comp_desc = 1;
+			break;
+		}
+		/* else: Fall trough */
 	case USB_SPEED_HIGH:
 		if (gadget_is_dualspeed(g)) {
 			speed_desc = f->hs_descriptors;

@@ -147,7 +274,31 @@ ep_found:
 	/* commit results */
 	_ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
 	_ep->desc = chosen_desc;
-
+	_ep->comp_desc = NULL;
+	_ep->maxburst = 0;
+	_ep->mult = 0;
+	if (want_comp_desc) {
+		/*
+		 * Companion descriptor should follow EP descriptor
+		 * USB 3.0 spec, #9.6.7
+		 */
+		comp_desc = (struct usb_ss_ep_comp_descriptor *)*(++d_spd);
+		if (!comp_desc ||
+		    (comp_desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP))
+			return -EIO;
+		_ep->comp_desc = comp_desc;
+		if (g->speed == USB_SPEED_SUPER) {
+			int xfer_type = _ep->bEndpointAddress &
+				USB_ENDPOINT_XFERTYPE_MASK ;
+			if (xfer_type == USB_ENDPOINT_XFER_BULK)
+				_ep->maxburst = comp_desc->bMaxBurst;
+			else if (xfer_type == USB_ENDPOINT_XFER_INT)
+				_ep->maxburst = comp_desc->bMaxBurst;
+			else if (xfer_type == USB_ENDPOINT_XFER_ISOC)
+				/* mult: bits 1:0 of bmAttributes */
+				_ep->mult = comp_desc->bmAttributes & 0x3;
+		}
+	}
 	return 0;
 }
 

@@ -187,6 +338,14 @@ int usb_add_function(struct usb_configuration *config,
 			list_del(&function->list);
 			function->config = NULL;
 		}
+		/*
+		 * Add SS descriptors if there are any. This has to be done
+		 * after the bind since we need the hs_descriptors to be set in
+		 * usb_function and some of the FDs does it in the bind.
+		 */
+		if ((gadget_is_superspeed(config->cdev->gadget)) &&
+		    (!function->ss_not_capable) && (!function->ss_descriptors))
+			create_ss_descriptors(function);
 	} else
 		value = 0;
 

@@ -199,6 +358,8 @@ int usb_add_function(struct usb_configuration *config,
 		config->fullspeed = true;
 	if (!config->highspeed && function->hs_descriptors)
 		config->highspeed = true;
+	if (!config->superspeed && function->ss_descriptors)
+		config->superspeed = true;
 

done:
if (value)

drivers/usb/gadget/composite.c +22 -6

@@ -342,7 +503,9 @@ static int config_buf(struct usb_configuration *config,
 	list_for_each_entry(f, &config->functions, list) {
 		struct usb_descriptor_header **descriptors;
 
-		if (speed == USB_SPEED_HIGH)
+		if (speed == USB_SPEED_SUPER)
+			descriptors = f->ss_descriptors;
+		else if (speed == USB_SPEED_HIGH)
 			descriptors = f->hs_descriptors;
 		else
 			descriptors = f->descriptors;

@@ -368,9 +531,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
 	u8				type = w_value >> 8;
 	enum usb_device_speed		speed = USB_SPEED_UNKNOWN;
 
-	if (gadget_is_dualspeed(gadget)) {
-		int			hs = 0;
-
+	if (gadget->speed == USB_SPEED_SUPER)
+		speed = gadget->speed;
+	else if (gadget_is_dualspeed(gadget)) {
+		int	hs = 0;
 		if (gadget->speed == USB_SPEED_HIGH)
 			hs = 1;
 		if (type == USB_DT_OTHER_SPEED_CONFIG)

@@ -384,7 +548,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
 	w_value &= 0xff;
 	list_for_each_entry(c, &cdev->configs, list) {
 		/* ignore configs that won't work at this speed */
-		if (speed == USB_SPEED_HIGH) {
+		if (speed == USB_SPEED_SUPER) {
+			if (!c->superspeed)
+				continue;
+		} else if (speed == USB_SPEED_HIGH) {
 			if (!c->highspeed)
 				continue;
 		} else {

@@ -404,16 +571,22 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
 	struct usb_configuration	*c;
 	unsigned			count = 0;
 	int				hs = 0;
+	int				ss = 0;
 
 	if (gadget_is_dualspeed(gadget)) {
 		if (gadget->speed == USB_SPEED_HIGH)
 			hs = 1;
+		if (gadget->speed == USB_SPEED_SUPER)
+			ss = 1;
 		if (type == USB_DT_DEVICE_QUALIFIER)
 			hs = !hs;
 	}
 	list_for_each_entry(c, &cdev->configs, list) {
 		/* ignore configs that won't work at this speed */
-		if (hs) {
+		if (ss) {
+			if (!c->superspeed)
+				continue;
+		} else if (hs) {
 			if (!c->highspeed)
 				continue;
 		} else {

@@ -425,6 +598,73 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
 	return count;
 }
 
+/**
+ * bos() - prepares the BOS descriptor.
+ * @cdev: pointer to usb_composite device to generate the bos
+ *	descriptor for

drivers/usb/gadget/composite.c +35 -12

+
+	return bos->wTotalLength;
+}
+
 static void device_qual(struct usb_composite_dev *cdev)
 {
 	struct usb_qualifier_descriptor	*qual = cdev->req->buf;

@@ -468,27 +708,42 @@ static int set_config(struct usb_composite_dev *cdev,
 	unsigned		power = gadget_is_otg(gadget) ? 8 : 100;
 	int			tmp;
 
-	if (cdev->config)
-		reset_config(cdev);
-
 	if (number) {
 		list_for_each_entry(c, &cdev->configs, list) {
 			if (c->bConfigurationValue == number) {
+				/*
+				 * Need to disable the FDs of the previous
+				 * configuration
+				 */
+				if (cdev->config)
+					reset_config(cdev);
 				result = 0;
 				break;
 			}
 		}
 		if (result < 0)
 			goto done;
-	} else
+	} else { /* Zero configuration value - need to reset the config */
+		if (cdev->config)
+			reset_config(cdev);
 		result = 0;
+	}
 
 	INFO(cdev, "%s speed config #%d: %s\n",
 		({ char *speed;
 		switch (gadget->speed) {
-		case USB_SPEED_LOW:	speed = "low"; break;
-		case USB_SPEED_FULL:	speed = "full"; break;
-		case USB_SPEED_HIGH:	speed = "high"; break;
+		case USB_SPEED_LOW:
+			speed = "low";
+			break;
+		case USB_SPEED_FULL:
+			speed = "full";
+			break;
+		case USB_SPEED_HIGH:
+			speed = "high";
+			break;
+		case USB_SPEED_SUPER:
+			speed = "super";
+			break;
 		default:		speed = "?"; break;
 		} ; speed; }), number, c ? c->label : "unconfigured");
 

@@ -511,7 +766,9 @@ static int set_config(struct usb_composite_dev *cdev,
 		 * function's setup callback instead of the current
 		 * configuration's setup callback.
 		 */
-		if (gadget->speed == USB_SPEED_HIGH)
+		if (gadget->speed == USB_SPEED_SUPER)
+			descriptors = f->ss_descriptors;
+		else if (gadget->speed == USB_SPEED_HIGH)
 			descriptors = f->hs_descriptors;
 		else
 			descriptors = f->descriptors;

@@ -596,14 +853,14 @@ int usb_add_config(struct usb_composite_dev *cdev,
 	} else {
 		unsigned	i;
 
-		DBG(cdev, "cfg %d/%p speeds:%s%s\n",
+		DBG(cdev, "cfg %d/%p speeds:%s%s%s\n",
 			config->bConfigurationValue, config,
+			config->superspeed ? " super" : "",
 			config->highspeed ? " high" : "",
 			config->fullspeed
 				? (gadget_is_dualspeed(cdev->gadget)
 					? " full"
-					: " full/low")
-				: "");
+					: " full/low") : "");
 
 		for (i = 0; i < MAX_CONFIG_INTERFACES; i++) {
 			struct usb_function	*f = config->interface[i];

@@ -721,7 +978,8 @@ static int get_string(struct usb_composite_dev *cdev,
 		return s->bLength;
 	}
 
-	/* Otherwise, look up and return a specified string.  First
+	/*
+	 * Otherwise, look up and return a specified string.  First
 	 * check if the string has not been overridden.
 	 */
 	if (cdev->manufacturer_override == id)

@@ -876,6 +1134,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
 	struct usb_request		*req = cdev->req;
 	int				value = -EOPNOTSUPP;
+	int				status = 0;
 	u16				w_index = le16_to_cpu(ctrl->wIndex);
 	u8				intf = w_index & 0xFF;
 	u16				w_value = le16_to_cpu(ctrl->wValue);

drivers/usb/gadget/composite.c +6 -2

+				cdev->desc.bcdUSB = cpu_to_le16(0x0210);
+
 			value = min(w_length, (u16) sizeof cdev->desc);
 			memcpy(req->buf, &cdev->desc, value);
 			break;
 		case USB_DT_DEVICE_QUALIFIER:
-			if (!gadget_is_dualspeed(gadget))
+			if (!gadget_is_dualspeed(gadget) ||
+			    gadget->speed >= USB_SPEED_SUPER)
 				break;
 			device_qual(cdev);
 			value = min_t(int, w_length,
 				sizeof(struct usb_qualifier_descriptor));
 			break;
 		case USB_DT_OTHER_SPEED_CONFIG:
-			if (!gadget_is_dualspeed(gadget))
+			if (!gadget_is_dualspeed(gadget) ||
+			    gadget->speed >= USB_SPEED_SUPER)
 				break;
 			/* FALLTHROUGH */
 		case USB_DT_CONFIG:

drivers/usb/gadget/composite.c +18 -0

+				value = bos(cdev);
+				value = min(w_length, (u16) value);
+			}
+			break;
 		}
 		break;
 

@@ -987,6 +1262,65 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		*((u8 *)req->buf) = value;
 		value = min(w_length, (u16) 1);
 		break;
+
+	/*
+	 * USB 3.0 additions:
+	 * Function driver should handle get_status request. If such cb
+	 * wasn't supplied we respond with default value = 0
+	 * Note: function driver should supply such cb only for the first
+	 * interface of the function
+	 */
+	case USB_REQ_GET_STATUS:
+		if (!gadget_is_superspeed(gadget))
+			goto unknown;
+		if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
+			goto unknown;
+		value = 2;	/* This is the length of the get_status reply */

drivers/usb/gadget/composite.c +15 -0

+		break;
+	/*
+	 * Function drivers should handle SetFeature/ClearFeature
+	 * (FUNCTION_SUSPEND) request. function_suspend cb should be supplied
+	 * only for the first interface of the function
+	 */
+	case USB_REQ_CLEAR_FEATURE:
+	case USB_REQ_SET_FEATURE:
+		if (!gadget_is_superspeed(gadget))
+			goto unknown;
+		if (ctrl->bRequestType != (USB_DIR_OUT | USB_RECIP_INTERFACE))
+			goto unknown;
+		switch (w_value) {
+		case USB_INTRF_FUNC_SUSPEND:
+			if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)

drivers/usb/gadget/composite.c +10 -0

+			if (value < 0) {
+				ERROR(cdev, "func_suspend() returned "
+					    "error %d\n", value);
+				value = 0;
+			}
+			break;
+		default:
+			break;
+		}
+		break;
 	default:

unknown:
VDBG(cdev,

drivers/usb/gadget/composite.c +5 -0

+				usb_free_descriptors(f->ss_descriptors);
+			/* may free memory for "f" */
 		}
 		list_del(&c->list);
 		if (c->unbind) {

@@ -1346,6 +1683,9 @@ int usb_composite_probe(struct usb_composite_driver *driver,
 		driver->iProduct = driver->name;
 	composite_driver.function =  (char *) driver->name;
 	composite_driver.driver.name = driver->name;
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+	composite_driver.speed = USB_SPEED_SUPER;
+#endif /* CONFIG_USB_GADGET_SUPERSPEED */
 	composite = driver;
 	composite_gadget_bind = bind;
 

drivers/usb/gadget/epautoconf.c +3 -3

index de3461a..35c5818 100644
@@ -142,13 +142,13 @@ ep_matches (
 	max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
 	switch (type) {
 	case USB_ENDPOINT_XFER_INT:
-		/* INT:  limit 64 bytes full speed, 1024 high speed */
+		/* INT:  limit 64 bytes full speed, 1024 high/super speed */
 		if (!gadget->is_dualspeed && max > 64)
 			return 0;
 		/* FALLTHROUGH */
 
 	case USB_ENDPOINT_XFER_ISOC:
-		/* ISO:  limit 1023 bytes full speed, 1024 high speed */
+		/* ISO:  limit 1023 bytes full speed, 1024 high/super speed */
 		if (ep->maxpacket < max)
 			return 0;
 		if (!gadget->is_dualspeed && max > 1023)

@@ -183,7 +183,7 @@ ep_matches (
 	}
 
 	/* report (variable) full speed bulk maxpacket */
-	if (USB_ENDPOINT_XFER_BULK == type) {
+	if ((USB_ENDPOINT_XFER_BULK == type) & !gadget->is_dualspeed) {
 		int size = ep->maxpacket;
 
 		/* min() doesn't work on bitfields with gcc-3.5 */

include/linux/usb/composite.h +25 -0

index 4f56e44..e9fa7d8 100644
@@ -51,6 +51,18 @@ struct usb_configuration;
 * @hs_descriptors: Table of high speed descriptors, using interface and
 *	string identifiers assigned during @bind().  If this pointer is null,
 *	the function will not be available at high speed.
+ * @ss_descriptors: Table of super speed descriptors. If
+ *	wasnt supplied by the FD during @bind() and
+ *	!ss_not_capble, will be generated automaticly with
+ *	default values while working in superspeed mode. If this
+ *	pointer is null after initiation, the function will not
+ *	be available at super speed.
+ * @ss_not_capable: This flag is used by the FD to indicate if
+ *	this function is SS capble. Meaning: if SS descriptors
+ *	weren't supplied by the FD, and the flag is set ss
+ *	descriptors will NOT be automatically generated
+ * @ss_desc_allocated: This flag indicates whether the ss descriptors were
+ *	dynamically allocated (and needs to be released).
 * @config: assigned when @usb_add_function() is called; this is the
 *	configuration with which this function is associated.
 * @bind: Before the gadget can register, all of its functions bind() to the

@@ -69,6 +81,10 @@ struct usb_configuration;
 * @setup: Used for interface-specific control requests.
 * @suspend: Notifies functions when the host stops sending USB traffic.
 * @resume: Notifies functions when the host restarts USB traffic.
+ * @get_status: Returns function status as a reply to
+ *	GetStatus() request when the recepient is Interface.
+ * @func_suspend: callback to be called when
+ *	SetFeature(FUNCTION_SUSPEND) is reseived
 *
 * A single USB function uses one or more interfaces, and should in most
 * cases support operation at both full and high speeds.  Each function is

@@ -98,6 +114,10 @@ struct usb_function {
 	struct usb_gadget_strings	**strings;
 	struct usb_descriptor_header	**descriptors;
 	struct usb_descriptor_header	**hs_descriptors;
+	struct usb_descriptor_header	**ss_descriptors;
+
+	unsigned			ss_desc_allocated:1;
+	unsigned			ss_not_capable:1;
 
 	struct usb_configuration	*config;
 

@@ -124,6 +144,10 @@ struct usb_function {
 	void			(*suspend)(struct usb_function *);
 	void			(*resume)(struct usb_function *);
 
+	/* USB 3.0 additions */
+	int			(*get_status)(struct usb_function *);
+	int			(*func_suspend)(struct usb_function *,
+						u8 suspend_opt);
 	/* private: */
 	/* internals */
 	struct list_head		list;

@@ -229,6 +253,7 @@ struct usb_configuration {
 	struct list_head	list;
 	struct list_head	functions;
 	u8			next_interface_id;
+	unsigned		superspeed:1;
 	unsigned		highspeed:1;
 	unsigned		fullspeed:1;
 	struct usb_function	*interface[MAX_CONFIG_INTERFACES];

include/linux/usb/gadget.h +8 -0

index 1bb7b3e..92c2d62 100644
@@ -131,11 +131,15 @@ struct usb_ep_ops {
 * @maxpacket:The maximum packet size used on this endpoint.  The initial
 *	value can sometimes be reduced (hardware allowing), according to
 *      the endpoint descriptor used to configure the endpoint.
+ * @mult: multiplier, 'mult' value for SS Isoc EPs
+ * @maxburst: the maximum number of bursts supported by this EP (for usb3)
 * @driver_data:for use by the gadget driver.
 * @bEndpointAddress: used to identify the endpoint when finding
 *	descriptor that matches connection speed
 * @desc: endpoint descriptor.  This pointer is set before the endpoint is
 *	enabled and remains valid until the endpoint is disabled.
+ * @comp_desc: In case of SuperSpeed support, this is the endpoint companion
+ *	descriptor that is used to configure the endpoint
 *
 * the bus controller driver lists all the general purpose endpoints in
 * gadget->ep_list.  the control endpoint (gadget->ep0) is not in that list,

@@ -148,8 +152,12 @@ struct usb_ep {
 	const struct usb_ep_ops	*ops;
 	struct list_head	ep_list;
 	unsigned		maxpacket:16;
+	unsigned				mult:2;
+	unsigned				pad:1;
+	unsigned				maxburst:4;
 	u8				bEndpointAddress;
 	const struct usb_endpoint_descriptor	*desc;
+	const struct usb_ss_ep_comp_descriptor	*comp_desc;
 };
 
 /*-------------------------------------------------------------------------*/

include/linux/usb/gadget.h +15 -0

+#define USB_DEFULT_U2_DEV_EXIT_LAT	0x1F4	/* Less then 500 microsec */
+};
+
+
 struct usb_gadget;
 
 /* the rest of the api to the controller hardware: device operations,

@@ -431,6 +447,7 @@ struct usb_gadget_ops {
 	int	(*pullup) (struct usb_gadget *, int is_on);
 	int	(*ioctl)(struct usb_gadget *,
 				unsigned code, unsigned long param);
+	void	(*get_config_params)(struct usb_dcd_config_params *);
 };
 
 /**

@@ -522,6 +539,23 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
 }
 
 /**
+ * gadget_is_superspeed() - return true if the hardware handles
+ * supperspeed
+ * @g: controller that might support supper speed
+ */
+static inline int gadget_is_superspeed(struct usb_gadget *g)
+{
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+	/* runtime test would check "g->is_superspeed" ... that might be
+	 * useful to work around hardware bugs, but is mostly pointless
+	 */

style, some endianess fixups

Signed-off-by Sebastian Andrzej Siewior <[email protected]>

---
 drivers/usb/gadget/Kconfig     |    1 -
 drivers/usb/gadget/composite.c |   62 +++++++++++++++++----------------------
 include/linux/usb/gadget.h     |    2 +-
 3 files changed, 28 insertions(+), 37 deletions(-)

drivers/usb/gadget/Kconfig +0 -1

index b4130bc..21429c7 100644
@@ -609,7 +609,6 @@ config USB_GADGET_SUPERSPEED
 	boolean "Gadget operating in Super Speed"
 	depends on USB_GADGET
 	depends on USB_GADGET_DUALSPEED
-	default n
 	help
 	  Enabling this feature enables Super Speed support in the Gadget
 	  driver. It means that gadget drivers should provide extra (SuperSpeed)

drivers/usb/gadget/composite.c +0 -0

index d5fe1c2..a94b7b7 100644

Signed-off-by Sebastian Andrzej Siewior <[email protected]>

---
  drivers/usb/gadget/composite.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)

drivers/usb/gadget/composite.c +3 -0

index 92cc238..ac30e2f 100644
@@ -74,10 +74,12 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
  static char composite_manufacturer[50];
 
  /* Default endpoint companion descriptor */
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
  static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
  		.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
  		.bLength = sizeof(struct usb_ss_ep_comp_descriptor),
  };
+#endif
 
  /**
   * usb_create_ss_descriptors() - Generate SuperSpeed descriptors

@@ -93,6 +95,7 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
   */
  void usb_create_ss_descriptors(struct usb_function *f)
  {
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
  	struct usb_ss_ep_comp_descriptor	*ep_comp_desc;
  	struct usb_endpoint_descriptor		*ep_desc;
  	struct usb_descriptor_header		**src = f->hs_descriptors;

Hello.

Cleaned-up example:

(in header)

    #ifndef CONFIG_NET_FUNKINESS
    static inline void init_funky_net (struct net_device *d) {}
    #endif

(in the code itself)

    dev = alloc_etherdev (sizeof(struct funky_private));
    if (!dev)
        return -ENODEV;
    init_funky_net(dev);

WBR, Sergei

Quoted 1 line · Sergei Shtylyov

Hello.

Hello Sergei,

Quoted 10 lines

Cleaned-up example:
(in header)

    #ifndef CONFIG_NET_FUNKINESS
    static inline void init_funky_net (struct net_device *d) {}
    #endif
 (in the code itself)
    dev = alloc_etherdev (sizeof(struct funky_private));
    if (!dev)
        return -ENODEV;
    init_funky_net(dev);

The problem here is that the code is included via

   #include "composite.c

so we don't really use header files. The alternative would be to use the gadget_is_super_speed() function but that one takes a gadget as argument. Preferences?

-----Original Message-----
From: Sebastian Andrzej Siewior [mailto:[email protected]]
Sent: Tuesday, April 12, 2011 10:35 PM
To: Tatyana Brokhman
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]
Subject: Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework

Hi Tatyana,

I tried to fixup this patch according to my review so you can focus on the more imporant things :)

Sebastian

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string"); · line 79

Quoted 4 lines

drivers/usb/gadget/composite.c +4 -0

+/* Default endpoint companion descriptor */
+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
+		.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+		.bLength = 0x06,

sizeof() ?

Review

Quoted 1 line

(quoted diff) +1 -0

+		.bMaxBurst = 0, /* the default is we don't support bursting

*/

drivers/usb/gadget/composite.c MODULE_PARM_DESC(iSerialNumber, "SerialNumber string"); · line 82

Quoted 2 lines

drivers/usb/gadget/composite.c +2 -0

+		.bmAttributes = 0, /* 2^0 streams supported */
+		.wBytesPerInterval = 0,

It is already 0 :) It makes sense to set to 0 if you want to point out something specific.

drivers/usb/gadget/composite.c static int count_configs(struct usb_composite_dev *cdev, unsigned type) · line 648

Quoted 4 lines

drivers/usb/gadget/composite.c +4 -0

+	ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
+	ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+	ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
+	ss_cap->bmAttributes = 0; /* LTM is not supported yet */

Who's fault is it? Isn't this always supported by the SS-udc?

Review composite_setup(struct usb_gadget *gadget,

Quoted 1 line

(quoted diff) +0 -0

@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget,

const struct usb_ctrlrequest *ctrl)

drivers/usb/gadget/composite.c composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) · line 1167

Quoted 6 lines

drivers/usb/gadget/composite.c +3 -0

 		case USB_DT_DEVICE:
 			cdev->desc.bNumConfigurations =
 				count_configs(cdev, USB_DT_DEVICE);
+			cdev->desc.bMaxPacketSize0 =
+				cdev->gadget->ep0->maxpacket;
+			if (gadget->speed >= USB_SPEED_SUPER)

there is something after super speed?

drivers/usb/gadget/composite.c composite_unbind(struct usb_gadget *gadget) · line 1446

Quoted 8 lines

drivers/usb/gadget/composite.c +2 -1

@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
 				DBG(cdev, "unbind function '%s'/%p\n",
 						f->name, f);
 				f->unbind(c, f);
-				/* may free memory for "f" */
 			}
+			/* Free memory allocated for ss descriptors */
+			if (f->ss_desc_allocated && f->ss_descriptors)

check for f->ss_descriptors is not required since kfree(null) is fine. However this does no fly. f->unbind() will free the memmory of f. So

moving the comment does not delay the de-allocation :)
The comment is should probably use "will" instead of "may" since all gadgets I checked do this.
The conditional allocation/free of the ss descriptor is kind of nasty. Wouldn't it be better to add create_ss_descriptors() for every gadget driver that needs it? This isn't that much. And in unbind you could always free all three of them using one function like free_all_descritpors().