[PATCH v3 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[PATCH v3 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Manu Gautam]

Allow userspace to pass SuperSpeed descriptors and
handle them in the driver accordingly.
This change doesn't modify existing desc_header and thereby
keeps the ABI changes backward compatible i.e. existing
userspace drivers compiled with old header (functionfs.h)
would continue to work with the updated kernel.

Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
---
 drivers/usb/gadget/f_fs.c           | 165 ++++++++++++++++++++++++++++--------
 drivers/usb/gadget/u_fs.h           |   8 +-
 include/uapi/linux/usb/functionfs.h |   5 ++
 3 files changed, 140 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 306a2b5..65bc861 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -122,8 +122,8 @@ struct ffs_ep {
 	struct usb_ep			*ep;	/* P: ffs->eps_lock */
 	struct usb_request		*req;	/* P: epfile->mutex */
 
-	/* [0]: full speed, [1]: high speed */
-	struct usb_endpoint_descriptor	*descs[2];
+	/* [0]: full speed, [1]: high speed, [2]: super speed */
+	struct usb_endpoint_descriptor	*descs[3];
 
 	u8				num;
 
@@ -1184,9 +1184,12 @@ static void ffs_data_reset(struct ffs_data *ffs)
 	ffs->stringtabs = NULL;
 
 	ffs->raw_descs_length = 0;
-	ffs->raw_fs_descs_length = 0;
+	ffs->raw_fs_hs_descs_length = 0;
+	ffs->raw_ss_descs_offset = 0;
+	ffs->raw_ss_descs_length = 0;
 	ffs->fs_descs_count = 0;
 	ffs->hs_descs_count = 0;
+	ffs->ss_descs_count = 0;
 
 	ffs->strings_count = 0;
 	ffs->interfaces_count = 0;
@@ -1329,7 +1332,20 @@ static int ffs_func_eps_enable(struct ffs_function *func)
 	spin_lock_irqsave(&func->ffs->eps_lock, flags);
 	do {
 		struct usb_endpoint_descriptor *ds;
-		ds = ep->descs[ep->descs[1] ? 1 : 0];
+		int desc_idx;
+
+		if (ffs->gadget->speed == USB_SPEED_SUPER)
+			desc_idx = 2;
+		else if (ffs->gadget->speed == USB_SPEED_HIGH)
+			desc_idx = 1;
+		else
+			desc_idx = 0;
+
+		ds = ep->descs[desc_idx];
+		if (!ds) {
+			ret = -EINVAL;
+			break;
+		}
 
 		ep->ep->driver_data = ep;
 		ep->ep->desc = ds;
@@ -1464,6 +1480,12 @@ static int __must_check ffs_do_desc(char *data, unsigned len,
 	}
 		break;
 
+	case USB_DT_SS_ENDPOINT_COMP:
+		pr_vdebug("EP SS companion descriptor\n");
+		if (length != sizeof(struct usb_ss_ep_comp_descriptor))
+			goto inv_length;
+		break;
+
 	case USB_DT_OTHER_SPEED_CONFIG:
 	case USB_DT_INTERFACE_POWER:
 	case USB_DT_DEBUG:
@@ -1574,8 +1596,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
 static int __ffs_data_got_descs(struct ffs_data *ffs,
 				char *const _data, size_t len)
 {
-	unsigned fs_count, hs_count;
-	int fs_len, ret = -EINVAL;
+	unsigned fs_count, hs_count, ss_count = 0;
+	int fs_len, hs_len, ss_len, ss_magic, ret = -EINVAL;
 	char *data = _data;
 
 	ENTER();
@@ -1586,9 +1608,6 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
 	fs_count = get_unaligned_le32(data +  8);
 	hs_count = get_unaligned_le32(data + 12);
 
-	if (!fs_count && !hs_count)
-		goto einval;
-
 	data += 16;
 	len  -= 16;
 
@@ -1607,22 +1626,59 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
 	}
 
 	if (likely(hs_count)) {
-		ret = ffs_do_descs(hs_count, data, len,
+		hs_len = ffs_do_descs(hs_count, data, len,
 				   __ffs_data_do_entity, ffs);
-		if (unlikely(ret < 0))
+		if (unlikely(hs_len < 0)) {
+			ret = hs_len;
+			goto error;
+		}
+	} else {
+		hs_len = 0;
+	}
+
+	if ((len >= hs_len + 8)) {
+		/* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */
+		ss_magic = get_unaligned_le32(data + hs_len);
+		if (ss_magic != FUNCTIONFS_SS_DESC_MAGIC)
+			goto einval;
+
+		ss_count = get_unaligned_le32(data + hs_len + 4);
+		data += hs_len + 8;
+		len  -= hs_len + 8;
+	} else {
+		data += hs_len;
+		len  -= hs_len;
+	}
+
+	if (!fs_count && !hs_count && !ss_count)
+		goto einval;
+
+	if (ss_count) {
+		ss_len = ffs_do_descs(ss_count, data, len,
+				   __ffs_data_do_entity, ffs);
+		if (unlikely(ss_len < 0)) {
+			ret = ss_len;
 			goto error;
+		}
+		ret = ss_len;
 	} else {
+		ss_len = 0;
 		ret = 0;
 	}
 
 	if (unlikely(len != ret))
 		goto einval;
 
-	ffs->raw_fs_descs_length = fs_len;
-	ffs->raw_descs_length    = fs_len + ret;
-	ffs->raw_descs           = _data;
-	ffs->fs_descs_count      = fs_count;
-	ffs->hs_descs_count      = hs_count;
+	ffs->raw_fs_hs_descs_length	 = fs_len + hs_len;
+	ffs->raw_ss_descs_length	 = ss_len;
+	ffs->raw_descs_length		 = ffs->raw_fs_hs_descs_length + ss_len;
+	ffs->raw_descs			 = _data;
+	ffs->fs_descs_count		 = fs_count;
+	ffs->hs_descs_count		 = hs_count;
+	ffs->ss_descs_count		 = ss_count;
+	/* ss_descs? present @ header + hs_fs_descs + ss_magic + ss_count */
+	if (ffs->ss_descs_count)
+		ffs->raw_ss_descs_offset = 16 + ffs->raw_fs_hs_descs_length + 8;
 
 	return 0;
 
@@ -1850,16 +1906,23 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
 	 * If hs_descriptors is not NULL then we are reading hs
 	 * descriptors now
 	 */
-	const int isHS = func->function.hs_descriptors != NULL;
-	unsigned idx;
+	const int is_hs = func->function.hs_descriptors != NULL;
+	const int is_ss = func->function.ss_descriptors != NULL;
+	unsigned ep_desc_id, idx;
 
 	if (type != FFS_DESCRIPTOR)
 		return 0;
 
-	if (isHS)
+	if (is_ss) {
+		func->function.ss_descriptors[(long)valuep] = desc;
+		ep_desc_id = 2;
+	} else if (is_hs) {
 		func->function.hs_descriptors[(long)valuep] = desc;
-	else
+		ep_desc_id = 1;
+	} else {
 		func->function.fs_descriptors[(long)valuep]    = desc;
+		ep_desc_id = 0;
+	}
 
 	if (!desc || desc->bDescriptorType != USB_DT_ENDPOINT)
 		return 0;
@@ -1867,13 +1930,13 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
 	idx = (ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) - 1;
 	ffs_ep = func->eps + idx;
 
-	if (unlikely(ffs_ep->descs[isHS])) {
+	if (unlikely(ffs_ep->descs[ep_desc_id])) {
 		pr_vdebug("two %sspeed descriptors for EP %d\n",
-			  isHS ? "high" : "full",
+			  is_ss ? "super" : "high/full",
 			  ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
 		return -EINVAL;
 	}
-	ffs_ep->descs[isHS] = ds;
+	ffs_ep->descs[ep_desc_id] = ds;
 
 	ffs_dump_mem(": Original  ep desc", ds, ds->bLength);
 	if (ffs_ep->ep) {
@@ -2017,8 +2080,10 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	const int full = !!func->ffs->fs_descs_count;
 	const int high = gadget_is_dualspeed(func->gadget) &&
 		func->ffs->hs_descs_count;
+	const int super = gadget_is_superspeed(func->gadget) &&
+		func->ffs->ss_descs_count;
 
-	int ret;
+	int fs_len, hs_len, ret;
 
 	/* Make it a single chunk, less management later on */
 	vla_group(d);
@@ -2027,15 +2092,16 @@ static int _ffs_func_bind(struct usb_configuration *c,
 		full ? ffs->fs_descs_count + 1 : 0);
 	vla_item_with_sz(d, struct usb_descriptor_header *, hs_descs,
 		high ? ffs->hs_descs_count + 1 : 0);
+	vla_item_with_sz(d, struct usb_descriptor_header *, ss_descs,
+		super ? ffs->ss_descs_count + 1 : 0);
 	vla_item_with_sz(d, short, inums, ffs->interfaces_count);
-	vla_item_with_sz(d, char, raw_descs,
-		high ? ffs->raw_descs_length : ffs->raw_fs_descs_length);
+	vla_item_with_sz(d, char, raw_descs, ffs->raw_descs_length);
 	char *vlabuf;
 
 	ENTER();
 
-	/* Only high speed but not supported by gadget? */
-	if (unlikely(!(full | high)))
+	/* Only high/super speed but not supported by gadget? */
+	if (unlikely(!(full | high | super)))
 		return -ENOTSUPP;
 
 	/* Allocate a single chunk, less management later on */
@@ -2045,8 +2111,16 @@ static int _ffs_func_bind(struct usb_configuration *c,
 
 	/* Zero */
 	memset(vla_ptr(vlabuf, d, eps), 0, d_eps__sz);
+	/* Copy only raw (hs,fs) descriptors (until ss_magic and ss_count) */
 	memcpy(vla_ptr(vlabuf, d, raw_descs), ffs->raw_descs + 16,
-	       d_raw_descs__sz);
+		ffs->raw_fs_hs_descs_length);
+	/* Copy SS descriptors */
+	if (func->ffs->ss_descs_count)
+		memcpy(vla_ptr(vlabuf, d, raw_descs) +
+				ffs->raw_fs_hs_descs_length,
+		       ffs->raw_descs + ffs->raw_ss_descs_offset,
+		       ffs->raw_ss_descs_length);
+
 	memset(vla_ptr(vlabuf, d, inums), 0xff, d_inums__sz);
 	for (ret = ffs->eps_count; ret; --ret) {
 		struct ffs_ep *ptr;
@@ -2068,33 +2142,51 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	 */
 	if (likely(full)) {
 		func->function.fs_descriptors = vla_ptr(vlabuf, d, fs_descs);
-		ret = ffs_do_descs(ffs->fs_descs_count,
+		fs_len = ffs_do_descs(ffs->fs_descs_count,
 				   vla_ptr(vlabuf, d, raw_descs),
 				   d_raw_descs__sz,
 				   __ffs_func_bind_do_descs, func);
-		if (unlikely(ret < 0))
+		if (unlikely(fs_len < 0)) {
+			ret = fs_len;
 			goto error;
+		}
 	} else {
-		ret = 0;
+		fs_len = 0;
 	}
 
 	if (likely(high)) {
 		func->function.hs_descriptors = vla_ptr(vlabuf, d, hs_descs);
-		ret = ffs_do_descs(ffs->hs_descs_count,
-				   vla_ptr(vlabuf, d, raw_descs) + ret,
-				   d_raw_descs__sz - ret,
+		hs_len = ffs_do_descs(ffs->hs_descs_count,
+				   vla_ptr(vlabuf, d, raw_descs) + fs_len,
+				   d_raw_descs__sz - fs_len,
 				   __ffs_func_bind_do_descs, func);
+		if (unlikely(hs_len < 0)) {
+			ret = hs_len;
+			goto error;
+		}
+	} else {
+		hs_len = 0;
+	}
+
+	if (likely(super)) {
+		func->function.ss_descriptors = vla_ptr(vlabuf, d, ss_descs);
+		ret = ffs_do_descs(ffs->ss_descs_count,
+			   vla_ptr(vlabuf, d, raw_descs) + fs_len + hs_len,
+			   d_raw_descs__sz - fs_len - hs_len,
+			   __ffs_func_bind_do_descs, func);
 		if (unlikely(ret < 0))
 			goto error;
 	}
 
+
 	/*
 	 * Now handle interface numbers allocation and interface and
 	 * endpoint numbers rewriting.  We can do that in one go
 	 * now.
 	 */
 	ret = ffs_do_descs(ffs->fs_descs_count +
-			   (high ? ffs->hs_descs_count : 0),
+			   (high ? ffs->hs_descs_count : 0) +
+			   (super ? ffs->ss_descs_count : 0),
 			   vla_ptr(vlabuf, d, raw_descs), d_raw_descs__sz,
 			   __ffs_func_bind_do_nums, func);
 	if (unlikely(ret < 0))
@@ -2441,6 +2533,7 @@ static void ffs_func_unbind(struct usb_configuration *c,
 	 */
 	func->function.fs_descriptors = NULL;
 	func->function.hs_descriptors = NULL;
+	func->function.ss_descriptors = NULL;
 	func->interfaces_nums = NULL;
 
 	ffs_event_add(ffs, FUNCTIONFS_UNBIND);
diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h
index bc2d371..1580194 100644
--- a/drivers/usb/gadget/u_fs.h
+++ b/drivers/usb/gadget/u_fs.h
@@ -213,13 +213,17 @@ struct ffs_data {
 	 * Real descriptors are 16 bytes after raw_descs (so you need
 	 * to skip 16 bytes (ie. ffs->raw_descs + 16) to get to the
 	 * first full speed descriptor).  raw_descs_length and
-	 * raw_fs_descs_length do not have those 16 bytes added.
+	 * raw_fs_hs_descs_length do not have those 16 bytes added.
+	 * ss_desc are 8 bytes (ss_magic + count) pass the hs_descs
 	 */
 	const void			*raw_descs;
 	unsigned			raw_descs_length;
-	unsigned			raw_fs_descs_length;
+	unsigned			raw_fs_hs_descs_length;
+	unsigned			raw_ss_descs_offset;
+	unsigned			raw_ss_descs_length;
 	unsigned			fs_descs_count;
 	unsigned			hs_descs_count;
+	unsigned			ss_descs_count;
 
 	unsigned short			strings_count;
 	unsigned short			interfaces_count;
diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
index d6b0128..1ab79a2 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -13,6 +13,7 @@ enum {
 	FUNCTIONFS_STRINGS_MAGIC     = 2
 };
 
+#define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C
 
 #ifndef __KERNEL__
 
@@ -50,7 +51,11 @@ struct usb_functionfs_descs_head {
  * |  12 | hs_count  | LE32         | number of high-speed descriptors     |
  * |  16 | fs_descrs | Descriptor[] | list of full-speed descriptors       |
  * |     | hs_descrs | Descriptor[] | list of high-speed descriptors       |
+ * |     | ss_magic  | LE32         | FUNCTIONFS_SS_DESC_MAGIC             |
+ * |     | ss_count  | LE32         | number of super-speed descriptors    |
+ * |     | ss_descrs | Descriptor[] | list of super-speed descriptors      |
  *
+ * ss_magic: if present then it implies that SS_DESCs are also present.
  * descs are just valid USB descriptors and have the following format:
  *
  * | off | name            | type | description              |
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Michal Nazarewicz]

[-- Attachment #1: Type: text/plain, Size: 14835 bytes --]

On Fri, Dec 20 2013, Manu Gautam wrote:
> Allow userspace to pass SuperSpeed descriptors and
> handle them in the driver accordingly.
> This change doesn't modify existing desc_header and thereby
> keeps the ABI changes backward compatible i.e. existing
> userspace drivers compiled with old header (functionfs.h)
> would continue to work with the updated kernel.
>
> Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> ---
>  drivers/usb/gadget/f_fs.c           | 165 ++++++++++++++++++++++++++++--------
>  drivers/usb/gadget/u_fs.h           |   8 +-
>  include/uapi/linux/usb/functionfs.h |   5 ++
>  3 files changed, 140 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index 306a2b5..65bc861 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -122,8 +122,8 @@ struct ffs_ep {
>  	struct usb_ep			*ep;	/* P: ffs->eps_lock */
>  	struct usb_request		*req;	/* P: epfile->mutex */
>  
> -	/* [0]: full speed, [1]: high speed */
> -	struct usb_endpoint_descriptor	*descs[2];
> +	/* [0]: full speed, [1]: high speed, [2]: super speed */
> +	struct usb_endpoint_descriptor	*descs[3];
>  
>  	u8				num;
>  
> @@ -1184,9 +1184,12 @@ static void ffs_data_reset(struct ffs_data *ffs)
>  	ffs->stringtabs = NULL;
>  
>  	ffs->raw_descs_length = 0;
> -	ffs->raw_fs_descs_length = 0;
> +	ffs->raw_fs_hs_descs_length = 0;
> +	ffs->raw_ss_descs_offset = 0;
> +	ffs->raw_ss_descs_length = 0;
>  	ffs->fs_descs_count = 0;
>  	ffs->hs_descs_count = 0;
> +	ffs->ss_descs_count = 0;
>  
>  	ffs->strings_count = 0;
>  	ffs->interfaces_count = 0;
> @@ -1329,7 +1332,20 @@ static int ffs_func_eps_enable(struct ffs_function *func)
>  	spin_lock_irqsave(&func->ffs->eps_lock, flags);
>  	do {
>  		struct usb_endpoint_descriptor *ds;
> -		ds = ep->descs[ep->descs[1] ? 1 : 0];
> +		int desc_idx;
> +
> +		if (ffs->gadget->speed == USB_SPEED_SUPER)
> +			desc_idx = 2;
> +		else if (ffs->gadget->speed == USB_SPEED_HIGH)
> +			desc_idx = 1;
> +		else
> +			desc_idx = 0;
> +
> +		ds = ep->descs[desc_idx];
> +		if (!ds) {
> +			ret = -EINVAL;
> +			break;
> +		}

I don't like this.  Why are we failing if descriptors for given speed
are missing?  If they are, we should fall back to lower speed.

	do {
		ds = ep->descs[desc_idx];
	} while (!ds && --desc_idx >= 0);
	if (desc_idx < 0) {
			ret = -EINVAL;
			break;
		}

Or something similar.  Point is, why aren't we failing dawn to high/low
speed if ep->descs[2] is NULL?

>  
>  		ep->ep->driver_data = ep;
>  		ep->ep->desc = ds;
> @@ -1464,6 +1480,12 @@ static int __must_check ffs_do_desc(char *data, unsigned len,
>  	}
>  		break;
>  
> +	case USB_DT_SS_ENDPOINT_COMP:
> +		pr_vdebug("EP SS companion descriptor\n");
> +		if (length != sizeof(struct usb_ss_ep_comp_descriptor))
> +			goto inv_length;
> +		break;
> +
>  	case USB_DT_OTHER_SPEED_CONFIG:
>  	case USB_DT_INTERFACE_POWER:
>  	case USB_DT_DEBUG:
> @@ -1574,8 +1596,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
>  static int __ffs_data_got_descs(struct ffs_data *ffs,
>  				char *const _data, size_t len)
>  {
> -	unsigned fs_count, hs_count;
> -	int fs_len, ret = -EINVAL;
> +	unsigned fs_count, hs_count, ss_count = 0;
> +	int fs_len, hs_len, ss_len, ss_magic, ret = -EINVAL;
>  	char *data = _data;
>  
>  	ENTER();
> @@ -1586,9 +1608,6 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
>  	fs_count = get_unaligned_le32(data +  8);
>  	hs_count = get_unaligned_le32(data + 12);
>  
> -	if (!fs_count && !hs_count)
> -		goto einval;
> -
>  	data += 16;
>  	len  -= 16;
>  
> @@ -1607,22 +1626,59 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
>  	}
>  
>  	if (likely(hs_count)) {
> -		ret = ffs_do_descs(hs_count, data, len,
> +		hs_len = ffs_do_descs(hs_count, data, len,
>  				   __ffs_data_do_entity, ffs);
> -		if (unlikely(ret < 0))
> +		if (unlikely(hs_len < 0)) {
> +			ret = hs_len;
> +			goto error;
> +		}

		data += hs_len;
		len  -= hs_len;

> +	} else {
> +		hs_len = 0;
> +	}
> +
> +	if ((len >= hs_len + 8)) {

With the above len -= hs_len, this just becomes “len >= 8”.

Nit: too many parenthesise.  Having “((…))” makes me think there's
assignment inside which there's no.

> +		/* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */
> +		ss_magic = get_unaligned_le32(data + hs_len);
> +		if (ss_magic != FUNCTIONFS_SS_DESC_MAGIC)
> +			goto einval;

The temporary variable doesn't really serve any purpose here, and with
the above “data += hs_len” this becomes:

		if (get_unaligned_le32(data) != FUNCTIONFS_SS_DESC_MAGIC)
			goto einval;

> +
> +		ss_count = get_unaligned_le32(data + hs_len + 4);
> +		data += hs_len + 8;
> +		len  -= hs_len + 8;
> +	} else {
> +		data += hs_len;
> +		len  -= hs_len;
> +	}
> +
> +	if (!fs_count && !hs_count && !ss_count)
> +		goto einval;
> +
> +	if (ss_count) {
> +		ss_len = ffs_do_descs(ss_count, data, len,
> +				   __ffs_data_do_entity, ffs);
> +		if (unlikely(ss_len < 0)) {
> +			ret = ss_len;
>  			goto error;
> +		}
> +		ret = ss_len;
>  	} else {
> +		ss_len = 0;
>  		ret = 0;
>  	}
>  
>  	if (unlikely(len != ret))
>  		goto einval;
>  
> -	ffs->raw_fs_descs_length = fs_len;
> -	ffs->raw_descs_length    = fs_len + ret;
> -	ffs->raw_descs           = _data;
> -	ffs->fs_descs_count      = fs_count;
> -	ffs->hs_descs_count      = hs_count;
> +	ffs->raw_fs_hs_descs_length	 = fs_len + hs_len;
> +	ffs->raw_ss_descs_length	 = ss_len;
> +	ffs->raw_descs_length		 = ffs->raw_fs_hs_descs_length + ss_len;

Nit: I would keep this consistent in the way that I'd just reference
local variables:

	ffs->raw_descs_length		 = fs_len + hs_len + ss_len;

> +	ffs->raw_descs			 = _data;
> +	ffs->fs_descs_count		 = fs_count;
> +	ffs->hs_descs_count		 = hs_count;
> +	ffs->ss_descs_count		 = ss_count;
> +	/* ss_descs? present @ header + hs_fs_descs + ss_magic + ss_count */
> +	if (ffs->ss_descs_count)
> +		ffs->raw_ss_descs_offset = 16 + ffs->raw_fs_hs_descs_length + 8;
>  
>  	return 0;
>  
> @@ -1850,16 +1906,23 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
>  	 * If hs_descriptors is not NULL then we are reading hs
>  	 * descriptors now
>  	 */
> -	const int isHS = func->function.hs_descriptors != NULL;
> -	unsigned idx;
> +	const int is_hs = func->function.hs_descriptors != NULL;
> +	const int is_ss = func->function.ss_descriptors != NULL;
> +	unsigned ep_desc_id, idx;
>  
>  	if (type != FFS_DESCRIPTOR)
>  		return 0;
>  
> -	if (isHS)
> +	if (is_ss) {
> +		func->function.ss_descriptors[(long)valuep] = desc;
> +		ep_desc_id = 2;
> +	} else if (is_hs) {
>  		func->function.hs_descriptors[(long)valuep] = desc;
> -	else
> +		ep_desc_id = 1;
> +	} else {
>  		func->function.fs_descriptors[(long)valuep]    = desc;
> +		ep_desc_id = 0;
> +	}
>  
>  	if (!desc || desc->bDescriptorType != USB_DT_ENDPOINT)
>  		return 0;
> @@ -1867,13 +1930,13 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
>  	idx = (ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) - 1;
>  	ffs_ep = func->eps + idx;
>  
> -	if (unlikely(ffs_ep->descs[isHS])) {
> +	if (unlikely(ffs_ep->descs[ep_desc_id])) {
>  		pr_vdebug("two %sspeed descriptors for EP %d\n",
> -			  isHS ? "high" : "full",
> +			  is_ss ? "super" : "high/full",

			  is_ss ? "super" : (is_hs "high" : "full"),

>  			  ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
>  		return -EINVAL;
>  	}
> -	ffs_ep->descs[isHS] = ds;
> +	ffs_ep->descs[ep_desc_id] = ds;
>  
>  	ffs_dump_mem(": Original  ep desc", ds, ds->bLength);
>  	if (ffs_ep->ep) {
> @@ -2017,8 +2080,10 @@ static int _ffs_func_bind(struct usb_configuration *c,
>  	const int full = !!func->ffs->fs_descs_count;
>  	const int high = gadget_is_dualspeed(func->gadget) &&
>  		func->ffs->hs_descs_count;
> +	const int super = gadget_is_superspeed(func->gadget) &&
> +		func->ffs->ss_descs_count;
>  
> -	int ret;
> +	int fs_len, hs_len, ret;
>  
>  	/* Make it a single chunk, less management later on */
>  	vla_group(d);
> @@ -2027,15 +2092,16 @@ static int _ffs_func_bind(struct usb_configuration *c,
>  		full ? ffs->fs_descs_count + 1 : 0);
>  	vla_item_with_sz(d, struct usb_descriptor_header *, hs_descs,
>  		high ? ffs->hs_descs_count + 1 : 0);
> +	vla_item_with_sz(d, struct usb_descriptor_header *, ss_descs,
> +		super ? ffs->ss_descs_count + 1 : 0);
>  	vla_item_with_sz(d, short, inums, ffs->interfaces_count);
> -	vla_item_with_sz(d, char, raw_descs,
> -		high ? ffs->raw_descs_length : ffs->raw_fs_descs_length);
> +	vla_item_with_sz(d, char, raw_descs, ffs->raw_descs_length);
>  	char *vlabuf;
>  
>  	ENTER();
>  
> -	/* Only high speed but not supported by gadget? */
> -	if (unlikely(!(full | high)))
> +	/* Only high/super speed but not supported by gadget? */

The comment cloud be improved, e.g.:

	/* Has descriptions only for speeds gadgets does not support. */

> +	if (unlikely(!(full | high | super)))
>  		return -ENOTSUPP;
>  
>  	/* Allocate a single chunk, less management later on */
> @@ -2045,8 +2111,16 @@ static int _ffs_func_bind(struct usb_configuration *c,
>  
>  	/* Zero */
>  	memset(vla_ptr(vlabuf, d, eps), 0, d_eps__sz);
> +	/* Copy only raw (hs,fs) descriptors (until ss_magic and ss_count) */
>  	memcpy(vla_ptr(vlabuf, d, raw_descs), ffs->raw_descs + 16,
> -	       d_raw_descs__sz);
> +		ffs->raw_fs_hs_descs_length);
> +	/* Copy SS descriptors */
> +	if (func->ffs->ss_descs_count)
> +		memcpy(vla_ptr(vlabuf, d, raw_descs) +
> +				ffs->raw_fs_hs_descs_length,
> +		       ffs->raw_descs + ffs->raw_ss_descs_offset,
> +		       ffs->raw_ss_descs_length);
> +
>  	memset(vla_ptr(vlabuf, d, inums), 0xff, d_inums__sz);
>  	for (ret = ffs->eps_count; ret; --ret) {
>  		struct ffs_ep *ptr;
> @@ -2068,33 +2142,51 @@ static int _ffs_func_bind(struct usb_configuration *c,
>  	 */
>  	if (likely(full)) {
>  		func->function.fs_descriptors = vla_ptr(vlabuf, d, fs_descs);
> -		ret = ffs_do_descs(ffs->fs_descs_count,
> +		fs_len = ffs_do_descs(ffs->fs_descs_count,
>  				   vla_ptr(vlabuf, d, raw_descs),
>  				   d_raw_descs__sz,
>  				   __ffs_func_bind_do_descs, func);

Nit: indention of the arguments.

> -		if (unlikely(ret < 0))
> +		if (unlikely(fs_len < 0)) {
> +			ret = fs_len;
>  			goto error;
> +		}
>  	} else {
> -		ret = 0;
> +		fs_len = 0;
>  	}
>  
>  	if (likely(high)) {
>  		func->function.hs_descriptors = vla_ptr(vlabuf, d, hs_descs);
> -		ret = ffs_do_descs(ffs->hs_descs_count,
> -				   vla_ptr(vlabuf, d, raw_descs) + ret,
> -				   d_raw_descs__sz - ret,
> +		hs_len = ffs_do_descs(ffs->hs_descs_count,
> +				   vla_ptr(vlabuf, d, raw_descs) + fs_len,
> +				   d_raw_descs__sz - fs_len,
>  				   __ffs_func_bind_do_descs, func);
> +		if (unlikely(hs_len < 0)) {
> +			ret = hs_len;
> +			goto error;
> +		}
> +	} else {
> +		hs_len = 0;
> +	}
> +
> +	if (likely(super)) {
> +		func->function.ss_descriptors = vla_ptr(vlabuf, d, ss_descs);
> +		ret = ffs_do_descs(ffs->ss_descs_count,
> +			   vla_ptr(vlabuf, d, raw_descs) + fs_len + hs_len,
> +			   d_raw_descs__sz - fs_len - hs_len,
> +			   __ffs_func_bind_do_descs, func);
>  		if (unlikely(ret < 0))
>  			goto error;
>  	}
>  
> +
>  	/*
>  	 * Now handle interface numbers allocation and interface and
>  	 * endpoint numbers rewriting.  We can do that in one go
>  	 * now.
>  	 */
>  	ret = ffs_do_descs(ffs->fs_descs_count +
> -			   (high ? ffs->hs_descs_count : 0),
> +			   (high ? ffs->hs_descs_count : 0) +
> +			   (super ? ffs->ss_descs_count : 0),
>  			   vla_ptr(vlabuf, d, raw_descs), d_raw_descs__sz,
>  			   __ffs_func_bind_do_nums, func);
>  	if (unlikely(ret < 0))
> @@ -2441,6 +2533,7 @@ static void ffs_func_unbind(struct usb_configuration *c,
>  	 */
>  	func->function.fs_descriptors = NULL;
>  	func->function.hs_descriptors = NULL;
> +	func->function.ss_descriptors = NULL;
>  	func->interfaces_nums = NULL;
>  
>  	ffs_event_add(ffs, FUNCTIONFS_UNBIND);
> diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h
> index bc2d371..1580194 100644
> --- a/drivers/usb/gadget/u_fs.h
> +++ b/drivers/usb/gadget/u_fs.h
> @@ -213,13 +213,17 @@ struct ffs_data {
>  	 * Real descriptors are 16 bytes after raw_descs (so you need
>  	 * to skip 16 bytes (ie. ffs->raw_descs + 16) to get to the
>  	 * first full speed descriptor).  raw_descs_length and
> -	 * raw_fs_descs_length do not have those 16 bytes added.
> +	 * raw_fs_hs_descs_length do not have those 16 bytes added.
> +	 * ss_desc are 8 bytes (ss_magic + count) pass the hs_descs
>  	 */
>  	const void			*raw_descs;
>  	unsigned			raw_descs_length;
> -	unsigned			raw_fs_descs_length;
> +	unsigned			raw_fs_hs_descs_length;
> +	unsigned			raw_ss_descs_offset;
> +	unsigned			raw_ss_descs_length;
>  	unsigned			fs_descs_count;
>  	unsigned			hs_descs_count;
> +	unsigned			ss_descs_count;
>  
>  	unsigned short			strings_count;
>  	unsigned short			interfaces_count;
> diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> index d6b0128..1ab79a2 100644
> --- a/include/uapi/linux/usb/functionfs.h
> +++ b/include/uapi/linux/usb/functionfs.h
> @@ -13,6 +13,7 @@ enum {
>  	FUNCTIONFS_STRINGS_MAGIC     = 2
>  };
>  
> +#define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C
>  
>  #ifndef __KERNEL__
>  
> @@ -50,7 +51,11 @@ struct usb_functionfs_descs_head {
>   * |  12 | hs_count  | LE32         | number of high-speed descriptors     |
>   * |  16 | fs_descrs | Descriptor[] | list of full-speed descriptors       |
>   * |     | hs_descrs | Descriptor[] | list of high-speed descriptors       |
> + * |     | ss_magic  | LE32         | FUNCTIONFS_SS_DESC_MAGIC             |
> + * |     | ss_count  | LE32         | number of super-speed descriptors    |
> + * |     | ss_descrs | Descriptor[] | list of super-speed descriptors      |
>   *
> + * ss_magic: if present then it implies that SS_DESCs are also present.
>   * descs are just valid USB descriptors and have the following format:
>   *
>   * | off | name            | type | description              |
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation


-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v3 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode

[Manu Gautam]

On 12/20/2013 8:47 PM, Michal Nazarewicz wrote:
> On Fri, Dec 20 2013, Manu Gautam wrote:
> 
> I don't like this.  Why are we failing if descriptors for given speed
> are missing?  If they are, we should fall back to lower speed.
> 
> 	do {
> 		ds = ep->descs[desc_idx];
> 	} while (!ds && --desc_idx >= 0);
> 	if (desc_idx < 0) {
> 			ret = -EINVAL;
> 			break;
> 		}
> 
> Or something similar.  Point is, why aren't we failing dawn to high/low
> speed if ep->descs[2] is NULL?
>

agreed.

>>  	if (likely(hs_count)) {
>> -		ret = ffs_do_descs(hs_count, data, len,
>> +		hs_len = ffs_do_descs(hs_count, data, len,
>>  				   __ffs_data_do_entity, ffs);
>> -		if (unlikely(ret < 0))
>> +		if (unlikely(hs_len < 0)) {
>> +			ret = hs_len;
>> +			goto error;
>> +		}
> 
> 		data += hs_len;
> 		len  -= hs_len;
> 
>> +	} else {
>> +		hs_len = 0;
>> +	}
>> +
>> +	if ((len >= hs_len + 8)) {
> 
> With the above len -= hs_len, this just becomes “len >= 8”.
> 
> Nit: too many parenthesise.  Having “((…))” makes me think there's
> assignment inside which there's no.
>

I will correct this.

>> +		/* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */
>> +		ss_magic = get_unaligned_le32(data + hs_len);
>> +		if (ss_magic != FUNCTIONFS_SS_DESC_MAGIC)
>> +			goto einval;
> 
> The temporary variable doesn't really serve any purpose here, and with
> the above “data += hs_len” this becomes:
> 
> 		if (get_unaligned_le32(data) != FUNCTIONFS_SS_DESC_MAGIC)
> 			goto einval;
> 

Agreed. I will correct this and below assignments.

>> +
>> +		ss_count = get_unaligned_le32(data + hs_len + 4);
>> +		data += hs_len + 8;
>> +		len  -= hs_len + 8;
>> +	} else {
>> +		data += hs_len;
>> +		len  -= hs_len;
>> +	}


>> +	ffs->raw_fs_hs_descs_length	 = fs_len + hs_len;
>> +	ffs->raw_ss_descs_length	 = ss_len;
>> +	ffs->raw_descs_length		 = ffs->raw_fs_hs_descs_length + ss_len;
> 
> Nit: I would keep this consistent in the way that I'd just reference
> local variables:
> 
> 	ffs->raw_descs_length		 = fs_len + hs_len + ss_len;
>

Agreed.

>> +	if (unlikely(ffs_ep->descs[ep_desc_id])) {
>>  		pr_vdebug("two %sspeed descriptors for EP %d\n",
>> -			  isHS ? "high" : "full",
>> +			  is_ss ? "super" : "high/full",
> 
> 			  is_ss ? "super" : (is_hs "high" : "full"),
> 

will change this.

>> -	/* Only high speed but not supported by gadget? */
>> -	if (unlikely(!(full | high)))
>> +	/* Only high/super speed but not supported by gadget? */
> 
> The comment cloud be improved, e.g.:
> 
> 	/* Has descriptions only for speeds gadgets does not support. */
> 

ok.

>> +		fs_len = ffs_do_descs(ffs->fs_descs_count,
>>  				   vla_ptr(vlabuf, d, raw_descs),
>>  				   d_raw_descs__sz,
>>  				   __ffs_func_bind_do_descs, func);
> 
> Nit: indention of the arguments.

I will fix this.




-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html