[PATCH 0/4] Add XU support to UVC Gadget

[PATCH 0/4] Add XU support to UVC Gadget

[Daniel Scally]

Hello all

This series adds support for the definition of extension units in configfs for
the UVC Gadget. The XUs are modelled as config_items within a new "extensions"
group under control, which seemed like an appropriate place to put them.

To allow the XU's to be inserted in the function graph, the bSourceID attribute
for the default output terminal is made writeable - users will need to configure
it with the bUnitID of the XU that they want to use as the OT's source. This does
mean that the XUs can _only_ be placed immediately preceding the OT, but I think
that that's fine for now.

The XUs configured through this series have been tested via uvc-gadget, uvcvideo
and uvcdynctrl.

Thanks
Dan


Daniel Scally (4):
  usb: gadget: uvc: Make bSourceID read/write
  usb: gadget: uvc: Generalise helper functions for reuse
  usb: gadget: uvc: Allow definition of XUs in configfs
  usb: gadget: uvc: Copy XU descriptors during .bind()

 drivers/usb/gadget/function/f_uvc.c        |  44 ++
 drivers/usb/gadget/function/u_uvc.h        |   7 +
 drivers/usb/gadget/function/uvc.h          |   1 +
 drivers/usb/gadget/function/uvc_configfs.c | 580 +++++++++++++++++++--
 drivers/usb/gadget/function/uvc_configfs.h |  29 ++
 5 files changed, 607 insertions(+), 54 deletions(-)

-- 
2.34.1

[PATCH 1/4] usb: gadget: uvc: Make bSourceID read/write

[Daniel Scally]

At the moment, the UVC function graph is hardcoded IT -> PU -> OT.
To add XU support we need the ability to insert the XU descriptors
into the chain. To facilitate that, make the output terminal's
bSourceID attribute writeable so that we can configure its source.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/usb/gadget/function/uvc_configfs.c | 59 +++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 4303a3283ba0..832565730d22 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -483,11 +483,68 @@ UVC_ATTR_RO(uvcg_default_output_, cname, aname)
 UVCG_DEFAULT_OUTPUT_ATTR(b_terminal_id, bTerminalID, 8);
 UVCG_DEFAULT_OUTPUT_ATTR(w_terminal_type, wTerminalType, 16);
 UVCG_DEFAULT_OUTPUT_ATTR(b_assoc_terminal, bAssocTerminal, 8);
-UVCG_DEFAULT_OUTPUT_ATTR(b_source_id, bSourceID, 8);
 UVCG_DEFAULT_OUTPUT_ATTR(i_terminal, iTerminal, 8);
 
 #undef UVCG_DEFAULT_OUTPUT_ATTR
 
+static ssize_t uvcg_default_output_b_source_id_show(struct config_item *item,
+						    char *page)
+{
+	struct config_group *group = to_config_group(item);
+	struct f_uvc_opts *opts;
+	struct config_item *opts_item;
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvc_output_terminal_descriptor *cd;
+	int result;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	opts_item = group->cg_item.ci_parent->ci_parent->
+			ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+	cd = &opts->uvc_output_terminal;
+
+	mutex_lock(&opts->lock);
+	result = sprintf(page, "%u\n", le8_to_cpu(cd->bSourceID));
+	mutex_unlock(&opts->lock);
+
+	mutex_unlock(su_mutex);
+
+	return result;
+}
+
+static ssize_t uvcg_default_output_b_source_id_store(struct config_item *item,
+						     const char *page, size_t len)
+{
+	struct config_group *group = to_config_group(item);
+	struct f_uvc_opts *opts;
+	struct config_item *opts_item;
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvc_output_terminal_descriptor *cd;
+	int result;
+	u8 num;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	opts_item = group->cg_item.ci_parent->ci_parent->
+			ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+	cd = &opts->uvc_output_terminal;
+
+	result = kstrtou8(page, 0, &num);
+	if (result)
+		return result;
+
+	mutex_lock(&opts->lock);
+	cd->bSourceID = num;
+	mutex_unlock(&opts->lock);
+
+	mutex_unlock(su_mutex);
+
+	return len;
+}
+UVC_ATTR(uvcg_default_output_, b_source_id, bSourceID);
+
 static struct configfs_attribute *uvcg_default_output_attrs[] = {
 	&uvcg_default_output_attr_b_terminal_id,
 	&uvcg_default_output_attr_w_terminal_type,
-- 
2.34.1

[PATCH 2/4] usb: gadget: uvc: Generalise helper functions for reuse

[Daniel Scally]

the __uvcg_*frm_intrv() helper functions can be helpful when adding
support for similar attributes. Generalise the function names and
move them higher in the file for better coverage. We also need copies
of the functions for different sized targets, so refactor them to
a macro with configurable bit size.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/usb/gadget/function/uvc_configfs.c | 109 +++++++++++----------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 832565730d22..9bacd8573a46 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -596,6 +596,60 @@ static const struct uvcg_config_group_type uvcg_terminal_grp_type = {
 	},
 };
 
+static inline int __uvcg_count_item_entries(char *buf, void *priv)
+{
+	++*((int *)priv);
+	return 0;
+}
+
+#define UVCG_ITEM_ENTRY_FUNCS(bits)					\
+static inline int __uvcg_fill_item_entries_u##bits(char *buf, void *priv)\
+{									\
+	u##bits num, **interv;						\
+	int ret;							\
+									\
+	ret = kstrtou##bits(buf, 0, &num);				\
+	if (ret)							\
+		return ret;						\
+									\
+	interv = priv;							\
+	**interv = num;							\
+	++*interv;							\
+									\
+	return 0;							\
+}									\
+									\
+static int __uvcg_iter_item_entries_u##bits(const char *page, size_t len,\
+				 int (*fun)(char *, void *), void *priv)\
+{									\
+	/* sign, base 2 representation, newline, terminator */		\
+	char buf[1 + sizeof(u##bits) * 8 + 1 + 1];			\
+	const char *pg = page;						\
+	int i, ret;							\
+									\
+	if (!fun)							\
+		return -EINVAL;						\
+									\
+	while (pg - page < len) {					\
+		i = 0;							\
+		while (i < sizeof(buf) && (pg - page < len) &&		\
+				*pg != '\0' && *pg != '\n')		\
+			buf[i++] = *pg++;				\
+		if (i == sizeof(buf))					\
+			return -EINVAL;					\
+		while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))\
+			++pg;						\
+		buf[i] = '\0';						\
+		ret = fun(buf, priv);					\
+		if (ret)						\
+			return ret;					\
+	}								\
+									\
+	return 0;							\
+}
+
+UVCG_ITEM_ENTRY_FUNCS(32)
+
 /* -----------------------------------------------------------------------------
  * control/class/{fs|ss}
  */
@@ -1188,57 +1242,6 @@ static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item,
 	return result;
 }
 
-static inline int __uvcg_count_frm_intrv(char *buf, void *priv)
-{
-	++*((int *)priv);
-	return 0;
-}
-
-static inline int __uvcg_fill_frm_intrv(char *buf, void *priv)
-{
-	u32 num, **interv;
-	int ret;
-
-	ret = kstrtou32(buf, 0, &num);
-	if (ret)
-		return ret;
-
-	interv = priv;
-	**interv = num;
-	++*interv;
-
-	return 0;
-}
-
-static int __uvcg_iter_frm_intrv(const char *page, size_t len,
-				 int (*fun)(char *, void *), void *priv)
-{
-	/* sign, base 2 representation, newline, terminator */
-	char buf[1 + sizeof(u32) * 8 + 1 + 1];
-	const char *pg = page;
-	int i, ret;
-
-	if (!fun)
-		return -EINVAL;
-
-	while (pg - page < len) {
-		i = 0;
-		while (i < sizeof(buf) && (pg - page < len) &&
-				*pg != '\0' && *pg != '\n')
-			buf[i++] = *pg++;
-		if (i == sizeof(buf))
-			return -EINVAL;
-		while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))
-			++pg;
-		buf[i] = '\0';
-		ret = fun(buf, priv);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
 						  const char *page, size_t len)
 {
@@ -1262,7 +1265,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
 		goto end;
 	}
 
-	ret = __uvcg_iter_frm_intrv(page, len, __uvcg_count_frm_intrv, &n);
+	ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_count_item_entries, &n);
 	if (ret)
 		goto end;
 
@@ -1272,7 +1275,7 @@ static ssize_t uvcg_frame_dw_frame_interval_store(struct config_item *item,
 		goto end;
 	}
 
-	ret = __uvcg_iter_frm_intrv(page, len, __uvcg_fill_frm_intrv, &tmp);
+	ret = __uvcg_iter_item_entries_u32(page, len, __uvcg_fill_item_entries_u32, &tmp);
 	if (ret) {
 		kfree(frm_intrv);
 		goto end;
-- 
2.34.1

[PATCH 3/4] usb: gadget: uvc: Allow definition of XUs in configfs

[Daniel Scally]

The UVC gadget at present has no support for extension units. Add the
infrastructure to uvc_configfs.c that allows users to create XUs via
configfs. These will be stored in a new child of uvcg_control_grp_type
with the name "extensions".

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---

Some questions:

	1. Should the bNumControls be freely configurable as it is in the current
	implementation, or should it be set automatically to (bControlSize * 8)?

	2. Currently for bNrInPins / baSourceID and bControlSize / bmControls,
	either can be set which adjusts the other. For example changing bNrInPins
	from 1 to 3 would add two entries (set to 0) to the baSourceID array. If
	the user subsequently wrote 2 values to baSourceID, bNrInPins would be
	automatically lowered to 2. Is that a reasonable approach, or should we
	just allow setting baSourceID and bmControls and set bNrInPins and
	bControlSize to read only?

	3. guidExtensionCode code is read/write as bytes at the moment, which
	leads to an in my opinion rather unintuitive means of writing the value.
	It's how the uncompressed format's guidFormat attribute works, so I went
	with that, but I wonder if it'd be better to have it use the usual format
	string, e.g. 123e4567-e89b-12d3-a456-426614174000?

	4. bUnitID is currently allocated automatically based on the last used
	ID stored in f_uvc_opts. This was to avoid having to do uniqueness
	validation when the attribute is set. Is that alright, or ought bUnitID
	be a writeable attribute with validation instead?

 drivers/usb/gadget/function/f_uvc.c        |   9 +
 drivers/usb/gadget/function/u_uvc.h        |   7 +
 drivers/usb/gadget/function/uvc_configfs.c | 412 +++++++++++++++++++++
 drivers/usb/gadget/function/uvc_configfs.h |  29 ++
 4 files changed, 457 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..eca5f36dfa74 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -842,6 +842,13 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 	od->bSourceID			= 2;
 	od->iTerminal			= 0;
 
+	/*
+	 * With the ability to add XUs to the UVC function graph, we need to be
+	 * able to allocate unique unit IDs to them. The IDs are 1-based, with
+	 * the CT, PU and OT above consuming the first 3.
+	 */
+	opts->last_unit_id		= 3;
+
 	md = &opts->uvc_color_matching;
 	md->bLength			= UVC_DT_COLOR_MATCHING_SIZE;
 	md->bDescriptorType		= USB_DT_CS_INTERFACE;
@@ -870,6 +877,8 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 	opts->ss_control =
 		(const struct uvc_descriptor_header * const *)ctl_cls;
 
+	INIT_LIST_HEAD(&opts->extension_units);
+
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
 	snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 24b8681b0d6f..5119cfe5ee4e 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -28,6 +28,7 @@ struct f_uvc_opts {
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
 	char						function_name[32];
+	unsigned int					last_unit_id;
 
 	/*
 	 * Control descriptors array pointers for full-/high-speed and
@@ -64,6 +65,12 @@ struct f_uvc_opts {
 	struct uvc_descriptor_header			*uvc_fs_control_cls[5];
 	struct uvc_descriptor_header			*uvc_ss_control_cls[5];
 
+	/*
+	 * Control descriptors for extension units. There could be any number
+	 * of these, including none at all.
+	 */
+	struct list_head				extension_units;
+
 	/*
 	 * Streaming descriptors for full-speed, high-speed and super-speed.
 	 * Used by configfs only, must not be touched by legacy gadgets. The
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 9bacd8573a46..f944a75a8063 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -648,8 +648,419 @@ static int __uvcg_iter_item_entries_u##bits(const char *page, size_t len,\
 	return 0;							\
 }
 
+UVCG_ITEM_ENTRY_FUNCS(8)
 UVCG_ITEM_ENTRY_FUNCS(32)
 
+/* -----------------------------------------------------------------------------
+ * control/extensions
+ */
+
+#define UVCG_EXTENSION_ATTR(cname, aname, ro...)			\
+static ssize_t uvcg_extension_##cname##_show(struct config_item *item,	\
+					     char *page)		\
+{									\
+	struct uvcg_extension *xu = to_uvcg_extension(item);		\
+	struct config_item *opts_item;					\
+	struct f_uvc_opts *opts;					\
+	int ret;							\
+									\
+	opts_item = item->ci_parent->ci_parent->ci_parent;		\
+	opts = to_f_uvc_opts(opts_item);				\
+									\
+	mutex_lock(&opts->lock);					\
+	ret = sprintf(page, "%u\n", xu->desc.aname);			\
+	mutex_unlock(&opts->lock);					\
+									\
+	return ret;							\
+}									\
+UVC_ATTR##ro(uvcg_extension_, cname, aname)
+
+UVCG_EXTENSION_ATTR(b_length, bLength, _RO);
+UVCG_EXTENSION_ATTR(b_unit_id, bUnitID, _RO);
+
+static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
+						   const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	int ret;
+	u8 num;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	ret = kstrtou8(page, 0, &num);
+	if (ret)
+		return ret;
+
+	mutex_lock(&opts->lock);
+	xu->desc.bNumControls = num;
+	mutex_unlock(&opts->lock);
+
+	return len;
+}
+UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
+
+/*
+ * In addition to storing bNrInPins, this function needs to realloc the
+ * memory for the baSourceID array and additionally expand bLength.
+ */
+static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
+						 const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	void *tmp_buf;
+	int ret;
+	u8 num;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	ret = kstrtou8(page, 0, &num);
+	if (ret)
+		return ret;
+
+	mutex_lock(&opts->lock);
+
+	if (num == xu->desc.bNrInPins) {
+		ret = len;
+		goto unlock;
+	}
+
+	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
+				 GFP_KERNEL);
+	if (!tmp_buf) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	if (num >= xu->desc.bNrInPins)
+		memset(tmp_buf + xu->desc.bNrInPins, 0,
+		       (num - xu->desc.bNrInPins) * sizeof(u8));
+
+	xu->desc.baSourceID = tmp_buf;
+	xu->desc.bNrInPins = num;
+	xu->desc.bLength = 24 + xu->desc.bNrInPins + xu->desc.bControlSize;
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	return ret;
+}
+UVCG_EXTENSION_ATTR(b_nr_in_pins, bNrInPins);
+
+/*
+ * In addition to storing bControlSize, this function needs to realloc the
+ * memory for the bmControls array and additionally expand bLength.
+ */
+static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
+						   const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	void *tmp_buf;
+	int ret;
+	u8 num;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	ret = kstrtou8(page, 0, &num);
+	if (ret)
+		return ret;
+
+	mutex_lock(&opts->lock);
+
+	if (num == xu->desc.bControlSize) {
+		ret = len;
+		goto unlock;
+	}
+
+	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
+				 GFP_KERNEL);
+	if (!tmp_buf) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	if (num >= xu->desc.bControlSize)
+		memset(tmp_buf + xu->desc.bControlSize, 0,
+		       (num - xu->desc.bControlSize) * sizeof(u8));
+
+	xu->desc.bmControls = tmp_buf;
+	xu->desc.bControlSize = num;
+	xu->desc.bLength = 24 + xu->desc.bNrInPins + xu->desc.bControlSize;
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	return ret;
+}
+
+UVCG_EXTENSION_ATTR(b_control_size, bControlSize);
+
+static ssize_t uvcg_extension_guid_extension_code_show(struct config_item *item,
+						       char *page)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+	memcpy(page, xu->desc.guidExtensionCode, sizeof(xu->desc.guidExtensionCode));
+	mutex_unlock(&opts->lock);
+
+	return sizeof(xu->desc.guidExtensionCode);
+}
+
+static ssize_t uvcg_extension_guid_extension_code_store(struct config_item *item,
+							const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	int ret;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+	memcpy(xu->desc.guidExtensionCode, page,
+	       min(sizeof(xu->desc.guidExtensionCode), len));
+	mutex_unlock(&opts->lock);
+
+	ret = sizeof(xu->desc.guidExtensionCode);
+
+	return ret;
+}
+
+UVC_ATTR(uvcg_extension_, guid_extension_code, guidExtensionCode);
+
+static ssize_t uvcg_extension_ba_source_id_show(struct config_item *item,
+						char *page)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	char *pg = page;
+	int ret, i;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+	for (ret = 0, i = 0; i < xu->desc.bNrInPins; ++i) {
+		ret += sprintf(pg, "%u\n", xu->desc.baSourceID[i]);
+		pg = page + ret;
+	}
+	mutex_unlock(&opts->lock);
+
+	return ret;
+}
+
+static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
+						 const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	u8 *source_ids, *tmp;
+	int ret, n = 0;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+
+	ret = __uvcg_iter_item_entries_u8(page, len, __uvcg_count_item_entries, &n);
+	if (ret)
+		goto unlock;
+
+	tmp = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
+	if (!source_ids) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	ret = __uvcg_iter_item_entries_u8(page, len, __uvcg_fill_item_entries_u8, &tmp);
+	if (ret) {
+		kfree(source_ids);
+		goto unlock;
+	}
+
+	kfree(xu->desc.baSourceID);
+	xu->desc.baSourceID = source_ids;
+	xu->desc.bNrInPins = n;
+	xu->desc.bLength = 24 + xu->desc.bNrInPins + xu->desc.bControlSize;
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	return ret;
+}
+UVC_ATTR(uvcg_extension_, ba_source_id, baSourceID);
+
+static ssize_t uvcg_extension_bm_controls_show(struct config_item *item,
+					       char *page)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	char *pg = page;
+	int ret, i;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+	for (ret = 0, i = 0; i < xu->desc.bControlSize; ++i) {
+		ret += sprintf(pg, "0x%02x\n", xu->desc.bmControls[i]);
+		pg = page + ret;
+	}
+	mutex_unlock(&opts->lock);
+
+	return ret;
+}
+
+static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
+						const char *page, size_t len)
+{
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	u8 *bm_controls, *tmp;
+	int ret, n = 0;
+
+	opts_item = item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+
+	ret = __uvcg_iter_item_entries_u8(page, len, __uvcg_count_item_entries, &n);
+	if (ret)
+		goto unlock;
+
+	tmp = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
+	if (!bm_controls) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	ret = __uvcg_iter_item_entries_u8(page, len, __uvcg_fill_item_entries_u8, &tmp);
+	if (ret) {
+		kfree(bm_controls);
+		goto unlock;
+	}
+
+	kfree(xu->desc.bmControls);
+	xu->desc.bmControls = bm_controls;
+	xu->desc.bControlSize = n;
+	xu->desc.bLength = 24 + xu->desc.bNrInPins + xu->desc.bControlSize;
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	return ret;
+}
+
+UVC_ATTR(uvcg_extension_, bm_controls, bmControls);
+
+static struct configfs_attribute *uvcg_extension_attrs[] = {
+	&uvcg_extension_attr_b_length,
+	&uvcg_extension_attr_b_unit_id,
+	&uvcg_extension_attr_b_num_controls,
+	&uvcg_extension_attr_b_nr_in_pins,
+	&uvcg_extension_attr_b_control_size,
+	&uvcg_extension_attr_guid_extension_code,
+	&uvcg_extension_attr_ba_source_id,
+	&uvcg_extension_attr_bm_controls,
+	NULL,
+};
+
+static const struct config_item_type uvcg_extension_type = {
+	.ct_item_ops	= &uvcg_config_item_ops,
+	.ct_attrs	= uvcg_extension_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static void uvcg_extension_drop(struct config_group *group, struct config_item *item)
+{
+	struct uvcg_extension *xu = container_of(item, struct uvcg_extension, item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+
+	opts_item = group->cg_item.ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+
+	config_item_put(item);
+	list_del(&xu->list);
+	kfree(xu->desc.baSourceID);
+	kfree(xu->desc.bmControls);
+	kfree(xu);
+
+	mutex_unlock(&opts->lock);
+}
+
+static struct config_item *uvcg_extension_make(struct config_group *group, const char *name)
+{
+	struct config_item *opts_item;
+	struct uvcg_extension *xu;
+	struct f_uvc_opts *opts;
+
+	opts_item = group->cg_item.ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+
+	xu = kzalloc(sizeof(*xu), GFP_KERNEL);
+	if (!xu)
+		return ERR_PTR(-ENOMEM);
+
+	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(0, 0);
+	xu->desc.bDescriptorType = USB_DT_CS_INTERFACE;
+	xu->desc.bDescriptorSubType = UVC_VC_EXTENSION_UNIT;
+	xu->desc.bUnitID = ++opts->last_unit_id;
+	xu->desc.bNumControls = 0;
+	xu->desc.bNrInPins = 0;
+	xu->desc.baSourceID = NULL;
+	xu->desc.bControlSize = 0;
+	xu->desc.bmControls = NULL;
+
+	config_item_init_type_name(&xu->item, name, &uvcg_extension_type);
+	list_add_tail(&xu->list, &opts->extension_units);
+
+	mutex_unlock(&opts->lock);
+
+	return &xu->item;
+}
+
+static struct configfs_group_operations uvcg_extensions_grp_ops = {
+	.make_item	= uvcg_extension_make,
+	.drop_item	= uvcg_extension_drop,
+};
+
+static const struct uvcg_config_group_type uvcg_extensions_grp_type = {
+	.type = {
+		.ct_item_ops	= &uvcg_config_item_ops,
+		.ct_group_ops	= &uvcg_extensions_grp_ops,
+		.ct_owner	= THIS_MODULE,
+	},
+	.name = "extensions",
+};
+
 /* -----------------------------------------------------------------------------
  * control/class/{fs|ss}
  */
@@ -844,6 +1255,7 @@ static const struct uvcg_config_group_type uvcg_control_grp_type = {
 		&uvcg_processing_grp_type,
 		&uvcg_terminal_grp_type,
 		&uvcg_control_class_grp_type,
+		&uvcg_extensions_grp_type,
 		NULL,
 	},
 };
diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
index ad2ec8c4c78c..c9a4182fb26f 100644
--- a/drivers/usb/gadget/function/uvc_configfs.h
+++ b/drivers/usb/gadget/function/uvc_configfs.h
@@ -132,6 +132,35 @@ static inline struct uvcg_mjpeg *to_uvcg_mjpeg(struct config_item *item)
 	return container_of(to_uvcg_format(item), struct uvcg_mjpeg, fmt);
 }
 
+/* -----------------------------------------------------------------------------
+ * control/extensions/<NAME>
+ */
+
+struct uvcg_extension_unit_descriptor {
+	u8 bLength;
+	u8 bDescriptorType;
+	u8 bDescriptorSubType;
+	u8 bUnitID;
+	u8 guidExtensionCode[16];
+	u8 bNumControls;
+	u8 bNrInPins;
+	u8 *baSourceID;
+	u8 bControlSize;
+	u8 *bmControls;
+	u8 iExtension;
+} __packed;
+
+struct uvcg_extension {
+	struct config_item item;
+	struct list_head list;
+	struct uvcg_extension_unit_descriptor desc;
+};
+
+static inline struct uvcg_extension *to_uvcg_extension(struct config_item *item)
+{
+	return container_of(item, struct uvcg_extension, item);
+}
+
 int uvcg_attach_configfs(struct f_uvc_opts *opts);
 
 #endif /* UVC_CONFIGFS_H */
-- 
2.34.1

[PATCH 4/4] usb: gadget: uvc: Copy XU descriptors during .bind()

[Daniel Scally]

Now that extension unit support is available through configfs we need
to copy the descriptors for the XUs during uvc_function_bind() so that
they're exposed to the usb subsystem.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---

I don't especially like UVC_COPY_XU_DESCRIPTOR(), but the need to vary the array
size for baSourceID and bmControls plus the requirement for the struct to be
copied to consecutive bytes of memory constrained it a bit. An alternative might
be to replace baSourceID, bControlSize, bmControls and iExtension in struct
uvcg_extension_unit_descriptor with a single flexible array member (called data[]
or something). That would allow the copy to be a much more straight forward
memcpy(mem, desc, desc->bLength); - but the cost would be reallocating the entire
struct each time the baSourceID or bmControls attributes was changed. That might
be a better method, but I thought I'd stick with this for this submission at least
on the grounds that it's less confusing.

 drivers/usb/gadget/function/f_uvc.c | 35 +++++++++++++++++++++++++++++
 drivers/usb/gadget/function/uvc.h   |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index eca5f36dfa74..e0a308f1355c 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -464,6 +464,25 @@ uvc_register_video(struct uvc_device *uvc)
 		} \
 	} while (0)
 
+#define UVC_COPY_XU_DESCRIPTOR(mem, dst, desc)					\
+	do {									\
+		*(dst)++ = mem;							\
+		memcpy(mem, desc, 22); /* bLength to bNrInPins */		\
+		mem += 22;							\
+										\
+		memcpy(mem, desc->baSourceID, desc->bNrInPins);			\
+		mem += desc->bNrInPins;						\
+										\
+		memcpy(mem, &desc->bControlSize, 1);				\
+		mem++;								\
+										\
+		memcpy(mem, desc->bmControls, desc->bControlSize);		\
+		mem += desc->bControlSize;					\
+										\
+		memcpy(mem, &desc->iExtension, 1);				\
+		mem++;								\
+	} while (0)
+
 static struct usb_descriptor_header **
 uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 {
@@ -475,6 +494,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	const struct usb_descriptor_header * const *src;
 	struct usb_descriptor_header **dst;
 	struct usb_descriptor_header **hdr;
+	struct uvcg_extension *xu;
 	unsigned int control_size;
 	unsigned int streaming_size;
 	unsigned int n_desc;
@@ -539,6 +559,13 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 		bytes += (*src)->bLength;
 		n_desc++;
 	}
+
+	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		control_size += xu->desc.bLength;
+		bytes += xu->desc.bLength;
+		n_desc++;
+	}
+
 	for (src = (const struct usb_descriptor_header **)uvc_streaming_cls;
 	     *src; ++src) {
 		streaming_size += (*src)->bLength;
@@ -565,6 +592,13 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	uvc_control_header = mem;
 	UVC_COPY_DESCRIPTORS(mem, dst,
 		(const struct usb_descriptor_header **)uvc_control_desc);
+
+	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		struct uvcg_extension_unit_descriptor *desc = &xu->desc;
+
+		UVC_COPY_XU_DESCRIPTOR(mem, dst, desc);
+	}
+
 	uvc_control_header->wTotalLength = cpu_to_le16(control_size);
 	uvc_control_header->bInCollection = 1;
 	uvc_control_header->baInterfaceNr[0] = uvc->streaming_intf;
@@ -988,6 +1022,7 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	uvc->desc.fs_streaming = opts->fs_streaming;
 	uvc->desc.hs_streaming = opts->hs_streaming;
 	uvc->desc.ss_streaming = opts->ss_streaming;
+	uvc->desc.extension_units = &opts->extension_units;
 
 	streaming = config_group_find_item(&opts->func_inst.group, "streaming");
 	if (!streaming)
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e14..f1a016d20bb6 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -143,6 +143,7 @@ struct uvc_device {
 		const struct uvc_descriptor_header * const *fs_streaming;
 		const struct uvc_descriptor_header * const *hs_streaming;
 		const struct uvc_descriptor_header * const *ss_streaming;
+		struct list_head *extension_units;
 	} desc;
 
 	unsigned int control_intf;
-- 
2.34.1

Re: [PATCH 3/4] usb: gadget: uvc: Allow definition of XUs in configfs

[Dan Scally]

Morning all

On 02/11/2022 15:17, Daniel Scally wrote:
> +
> +static struct configfs_attribute *uvcg_extension_attrs[] = {
> +	&uvcg_extension_attr_b_length,
> +	&uvcg_extension_attr_b_unit_id,
> +	&uvcg_extension_attr_b_num_controls,
> +	&uvcg_extension_attr_b_nr_in_pins,
> +	&uvcg_extension_attr_b_control_size,
> +	&uvcg_extension_attr_guid_extension_code,
> +	&uvcg_extension_attr_ba_source_id,
> +	&uvcg_extension_attr_bm_controls,
> +	NULL,
> +};
> +
> +static const struct config_item_type uvcg_extension_type = {
> +	.ct_item_ops	= &uvcg_config_item_ops,
> +	.ct_attrs	= uvcg_extension_attrs,
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +static void uvcg_extension_drop(struct config_group *group, struct config_item *item)
> +{
> +	struct uvcg_extension *xu = container_of(item, struct uvcg_extension, item);
> +	struct config_item *opts_item;
> +	struct f_uvc_opts *opts;
> +
> +	opts_item = group->cg_item.ci_parent->ci_parent;
> +	opts = to_f_uvc_opts(opts_item);
> +
> +	mutex_lock(&opts->lock);
> +
> +	config_item_put(item);
> +	list_del(&xu->list);
> +	kfree(xu->desc.baSourceID);
> +	kfree(xu->desc.bmControls);
> +	kfree(xu);


This should actually have gone in the .release() callback for the item - 
I'll fix that in v2.

> +
> +	mutex_unlock(&opts->lock);
> +}
> +
> +static struct config_item *uvcg_extension_make(struct config_group *group, const char *name)
> +{
> +	struct config_item *opts_item;
> +	struct uvcg_extension *xu;
> +	struct f_uvc_opts *opts;
> +
> +	opts_item = group->cg_item.ci_parent->ci_parent;
> +	opts = to_f_uvc_opts(opts_item);
> +
> +	mutex_lock(&opts->lock);
> +
> +	xu = kzalloc(sizeof(*xu), GFP_KERNEL);
> +	if (!xu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(0, 0);
> +	xu->desc.bDescriptorType = USB_DT_CS_INTERFACE;
> +	xu->desc.bDescriptorSubType = UVC_VC_EXTENSION_UNIT;
> +	xu->desc.bUnitID = ++opts->last_unit_id;
> +	xu->desc.bNumControls = 0;
> +	xu->desc.bNrInPins = 0;
> +	xu->desc.baSourceID = NULL;
> +	xu->desc.bControlSize = 0;
> +	xu->desc.bmControls = NULL;
> +
> +	config_item_init_type_name(&xu->item, name, &uvcg_extension_type);
> +	list_add_tail(&xu->list, &opts->extension_units);
> +
> +	mutex_unlock(&opts->lock);
> +
> +	return &xu->item;
> +}
> +
> +static struct configfs_group_operations uvcg_extensions_grp_ops = {
> +	.make_item	= uvcg_extension_make,
> +	.drop_item	= uvcg_extension_drop,
> +};
> +
> +static const struct uvcg_config_group_type uvcg_extensions_grp_type = {
> +	.type = {
> +		.ct_item_ops	= &uvcg_config_item_ops,
> +		.ct_group_ops	= &uvcg_extensions_grp_ops,
> +		.ct_owner	= THIS_MODULE,
> +	},
> +	.name = "extensions",
> +};
> +
>   /* -----------------------------------------------------------------------------
>    * control/class/{fs|ss}
>    */
> @@ -844,6 +1255,7 @@ static const struct uvcg_config_group_type uvcg_control_grp_type = {
>   		&uvcg_processing_grp_type,
>   		&uvcg_terminal_grp_type,
>   		&uvcg_control_class_grp_type,
> +		&uvcg_extensions_grp_type,
>   		NULL,
>   	},
>   };
> diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
> index ad2ec8c4c78c..c9a4182fb26f 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.h
> +++ b/drivers/usb/gadget/function/uvc_configfs.h
> @@ -132,6 +132,35 @@ static inline struct uvcg_mjpeg *to_uvcg_mjpeg(struct config_item *item)
>   	return container_of(to_uvcg_format(item), struct uvcg_mjpeg, fmt);
>   }
>   
> +/* -----------------------------------------------------------------------------
> + * control/extensions/<NAME>
> + */
> +
> +struct uvcg_extension_unit_descriptor {
> +	u8 bLength;
> +	u8 bDescriptorType;
> +	u8 bDescriptorSubType;
> +	u8 bUnitID;
> +	u8 guidExtensionCode[16];
> +	u8 bNumControls;
> +	u8 bNrInPins;
> +	u8 *baSourceID;
> +	u8 bControlSize;
> +	u8 *bmControls;
> +	u8 iExtension;
> +} __packed;
> +
> +struct uvcg_extension {
> +	struct config_item item;
> +	struct list_head list;
> +	struct uvcg_extension_unit_descriptor desc;
> +};
> +
> +static inline struct uvcg_extension *to_uvcg_extension(struct config_item *item)
> +{
> +	return container_of(item, struct uvcg_extension, item);
> +}
> +
>   int uvcg_attach_configfs(struct f_uvc_opts *opts);
>   
>   #endif /* UVC_CONFIGFS_H */

Re: [PATCH 1/4] usb: gadget: uvc: Make bSourceID read/write

[Greg KH]

On Wed, Nov 02, 2022 at 03:17:52PM +0000, Daniel Scally wrote:
> At the moment, the UVC function graph is hardcoded IT -> PU -> OT.
> To add XU support we need the ability to insert the XU descriptors
> into the chain. To facilitate that, make the output terminal's
> bSourceID attribute writeable so that we can configure its source.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  drivers/usb/gadget/function/uvc_configfs.c | 59 +++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)

Isn't there a documentation update needed here too?

thanks,

greg k-h

Re: [PATCH 1/4] usb: gadget: uvc: Make bSourceID read/write

[Dan Scally]

Morning

On 09/11/2022 10:20, Greg KH wrote:
> On Wed, Nov 02, 2022 at 03:17:52PM +0000, Daniel Scally wrote:
>> At the moment, the UVC function graph is hardcoded IT -> PU -> OT.
>> To add XU support we need the ability to insert the XU descriptors
>> into the chain. To facilitate that, make the output terminal's
>> bSourceID attribute writeable so that we can configure its source.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>>   drivers/usb/gadget/function/uvc_configfs.c | 59 +++++++++++++++++++++-
>>   1 file changed, 58 insertions(+), 1 deletion(-)
> Isn't there a documentation update needed here too?


Yes - sorry, I forgot documentation for the whole patchset. My bad, 
it'll be in the v2.

>
> thanks,
>
> greg k-h