All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/11] Add XU support to UVC Gadget
@ 2023-01-30  9:34 Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 01/11] usb: gadget: uvc: Make bSourceID read/write Daniel Scally
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, 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.

Series level changes:

  - Added patches 5-9 which additionally add the ability to create string
  descriptors through configfs and link them to the extension units as well as
  to override the default descriptors for the IAD and VC/VS interfaces

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

v2 of the series here: https://lore.kernel.org/linux-usb/0ae65812-c937-d071-455b-7c1d6418b080@ideasonboard.com/

Thanks
Dan

Daniel Scally (11):
  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()
  usb: gadget: configfs: Rename struct gadget_strings
  usb: gadget: configfs: Support arbitrary string descriptors
  usb: gadget: configfs: Attach arbitrary strings to cdev
  usb: gadget: uvc: Allow linking XUs to string descriptors
  usb: gadget: uvc: Pick up custom string descriptor IDs
  usb: gadget: uvc: Allow linking function to string descs
  usb: gadget: uvc: Use custom strings if available

 .../ABI/testing/configfs-usb-gadget-uvc       |  30 +-
 drivers/usb/gadget/configfs.c                 | 293 ++++++-
 drivers/usb/gadget/function/f_uvc.c           |  69 +-
 drivers/usb/gadget/function/u_uvc.h           |  15 +
 drivers/usb/gadget/function/uvc.h             |   1 +
 drivers/usb/gadget/function/uvc_configfs.c    | 771 ++++++++++++++++--
 drivers/usb/gadget/function/uvc_configfs.h    |  30 +
 include/linux/usb/composite.h                 |   1 +
 include/linux/usb/gadget.h                    |  11 +
 9 files changed, 1129 insertions(+), 92 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v3 01/11] usb: gadget: uvc: Make bSourceID read/write
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 02/11] usb: gadget: uvc: Generalise helper functions for reuse Daniel Scally
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, 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>
---
Changes in v3:

	- None 

Changes in v2:

	- Updated the ABI Documentation to reflect the change.

 .../ABI/testing/configfs-usb-gadget-uvc       |  2 +-
 drivers/usb/gadget/function/uvc_configfs.c    | 59 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index f00cff6d8c5c..c25cc2823fc8 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -52,7 +52,7 @@ Date:		Dec 2014
 KernelVersion:	4.0
 Description:	Default output terminal descriptors
 
-		All attributes read only:
+		All attributes read only except bSourceID:
 
 		==============	=============================================
 		iTerminal	index of string descriptor
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 76cb60d13049..0a3095c0450b 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 02/11] usb: gadget: uvc: Generalise helper functions for reuse
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 01/11] usb: gadget: uvc: Make bSourceID read/write Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs Daniel Scally
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

The __uvcg_*frm_intrv() helper functions can be helpful when adding
support for similar attributes. Generalise the functions and
move them higher in the file for better coverage.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v3 (Laurent):

	- Spelling
	- Renamed the "interv" variable to "values"
	- Switched to functions taking a size argument rather than a macro that
	duplicated the implementation.
	
Changes in v2:

	- none

 drivers/usb/gadget/function/uvc_configfs.c | 120 ++++++++++++---------
 1 file changed, 67 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 0a3095c0450b..cbb3a71a6351 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -46,6 +46,71 @@ static int uvcg_config_compare_u32(const void *l, const void *r)
 	return li < ri ? -1 : li == ri ? 0 : 1;
 }
 
+static inline int __uvcg_count_item_entries(char *buf, void *priv, unsigned int size)
+{
+	++*((int *)priv);
+	return 0;
+}
+
+static inline int __uvcg_fill_item_entries(char *buf, void *priv, unsigned int size)
+{
+	unsigned int num;
+	u8 **values;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &num);
+	if (ret)
+		return ret;
+
+	if (num != (num & GENMASK((size * 8) - 1, 0)))
+		return -ERANGE;
+
+	values = priv;
+	memcpy(*values, &num, size);
+	*values += size;
+
+	return 0;
+}
+
+static int __uvcg_iter_item_entries(const char *page, size_t len,
+				    int (*fun)(char *, void *, unsigned int),
+				    void *priv, unsigned int size)
+{
+	/* sign, base 2 representation, newline, terminator */
+	unsigned int bufsize = 1 + size * 8 + 1 + 1;
+	const char *pg = page;
+	int i, ret = 0;
+	char *buf;
+
+	if (!fun)
+		return -EINVAL;
+
+	buf = kzalloc(bufsize, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	while (pg - page < len) {
+		i = 0;
+		while (i < sizeof(buf) && (pg - page < len) &&
+				*pg != '\0' && *pg != '\n')
+			buf[i++] = *pg++;
+		if (i == sizeof(buf)) {
+			ret = -EINVAL;
+			goto out_free_buf;
+		}
+		while ((pg - page < len) && (*pg == '\0' || *pg == '\n'))
+			++pg;
+		buf[i] = '\0';
+		ret = fun(buf, priv, size);
+		if (ret)
+			goto out_free_buf;
+	}
+
+out_free_buf:
+	kfree(buf);
+	return ret;
+}
+
 struct uvcg_config_group_type {
 	struct config_item_type type;
 	const char *name;
@@ -1188,57 +1253,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 +1276,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(page, len, __uvcg_count_item_entries, &n, sizeof(u32));
 	if (ret)
 		goto end;
 
@@ -1272,7 +1286,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(page, len, __uvcg_fill_item_entries, &tmp, sizeof(u32));
 	if (ret) {
 		kfree(frm_intrv);
 		goto end;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 01/11] usb: gadget: uvc: Make bSourceID read/write Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 02/11] usb: gadget: uvc: Generalise helper functions for reuse Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30 20:13   ` kernel test robot
  2023-01-30  9:34 ` [PATCH v3 04/11] usb: gadget: uvc: Copy XU descriptors during .bind() Daniel Scally
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, 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>
---
Changes in v3 (Laurent):

	- Spelling
	- Passed __GFP_ZERO to krealloc_array() instead of memsetting the new
	entries to zero afterwards.
	- Switched to using the bLength calculation macro instead of open coding
	the same thing
	- Fixed a return without mutex_unlock()

Changes in v2:

	- Updated the ABI documentation with the new elements.
	- Locked the su_mutex when appropriate.

 .../ABI/testing/configfs-usb-gadget-uvc       |  28 +
 drivers/usb/gadget/function/f_uvc.c           |   9 +
 drivers/usb/gadget/function/u_uvc.h           |   7 +
 drivers/usb/gadget/function/uvc_configfs.c    | 480 ++++++++++++++++++
 drivers/usb/gadget/function/uvc_configfs.h    |  29 ++
 5 files changed, 553 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index c25cc2823fc8..31636695f2d0 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -111,6 +111,34 @@ Description:	Default processing unit descriptors
 		bUnitID		a non-zero id of this unit
 		===============	========================================
 
+What:		/config/usb-gadget/gadget/functions/uvc.name/control/extensions
+Date:		Nov 2022
+KernelVersion:	6.1
+Description:	Extension unit descriptors
+
+What:		/config/usb-gadget/gadget/functions/uvc.name/control/extensions/name
+Date:		Nov 2022
+KernelVersion:	6.1
+Description:	Extension Unit (XU) Descriptor
+
+		bLength, bUnitID and iExtension are read-only. All others are
+		read-write.
+
+		===============		========================================
+		bLength			size of the descriptor in bytes
+		bUnitID			non-zero ID of this unit
+		guidExtensionCode	Vendor-specific code identifying the XU
+		bNumControls		number of controls in this XU
+		bNrInPins		number of input pins for this unit
+		baSourceID		list of the IDs of the units or terminals
+					to which this XU is connected
+		bControlSize		size of the bmControls field in bytes
+		bmControls		list of bitmaps detailing which vendor
+					specific controls are supported
+		iExtension		index of a string descriptor that describes
+					this extension unit
+		===============		========================================
+
 What:		/config/usb-gadget/gadget/functions/uvc.name/control/header
 Date:		Dec 2014
 KernelVersion:	4.0
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 32f2c1645467..64a9e35ff075 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -852,6 +852,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;
@@ -880,6 +887,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 cbb3a71a6351..a81613e9652d 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -661,6 +661,485 @@ static const struct uvcg_config_group_type uvcg_terminal_grp_type = {
 	},
 };
 
+/* -----------------------------------------------------------------------------
+ * control/extensions
+ */
+
+#define UVCG_EXTENSION_ATTR(cname, aname, ro...)			\
+static ssize_t uvcg_extension_##cname##_show(struct config_item *item,	\
+					     char *page)		\
+{									\
+	struct config_group *group = to_config_group(item->ci_parent);	\
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;		\
+	struct uvcg_extension *xu = to_uvcg_extension(item);		\
+	struct config_item *opts_item;					\
+	struct f_uvc_opts *opts;					\
+	int ret;							\
+									\
+	mutex_lock(su_mutex);						\
+									\
+	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);					\
+									\
+	mutex_unlock(su_mutex);						\
+									\
+	return ret;							\
+}									\
+UVC_ATTR##ro(uvcg_extension_, cname, aname)
+
+UVCG_EXTENSION_ATTR(b_length, bLength, _RO);
+UVCG_EXTENSION_ATTR(b_unit_id, bUnitID, _RO);
+UVCG_EXTENSION_ATTR(i_extension, iExtension, _RO);
+
+static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
+						   const char *page, size_t len)
+{
+	struct config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	int ret;
+	u8 num;
+
+	mutex_lock(su_mutex);
+
+	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);
+
+	mutex_unlock(su_mutex);
+
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	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;
+
+	mutex_lock(su_mutex);
+
+	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 | __GFP_ZERO);
+	if (!tmp_buf) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	xu->desc.baSourceID = tmp_buf;
+	xu->desc.bNrInPins = num;
+	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+						      xu->desc.bControlSize);
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	mutex_unlock(su_mutex);
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	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;
+
+	mutex_lock(su_mutex);
+
+	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 | __GFP_ZERO);
+	if (!tmp_buf) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	xu->desc.bmControls = tmp_buf;
+	xu->desc.bControlSize = num;
+	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+						      xu->desc.bControlSize);
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	mutex_unlock(su_mutex);
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+
+	mutex_lock(su_mutex);
+
+	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);
+
+	mutex_unlock(su_mutex);
+
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	int ret;
+
+	mutex_lock(su_mutex);
+
+	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);
+
+	mutex_unlock(su_mutex);
+
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	char *pg = page;
+	int ret, i;
+
+	mutex_lock(su_mutex);
+
+	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);
+
+	mutex_unlock(su_mutex);
+
+	return ret;
+}
+
+static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
+						 const char *page, size_t len)
+{
+	struct config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	u8 *source_ids, *iter;
+	int ret, n = 0;
+
+	mutex_lock(su_mutex);
+
+	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(page, len, __uvcg_count_item_entries, &n,
+				       sizeof(u8));
+	if (ret)
+		goto unlock;
+
+	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
+	if (!source_ids) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	ret = __uvcg_iter_item_entries(page, len, __uvcg_fill_item_entries, &iter,
+				       sizeof(u8));
+	if (ret) {
+		kfree(source_ids);
+		goto unlock;
+	}
+
+	kfree(xu->desc.baSourceID);
+	xu->desc.baSourceID = source_ids;
+	xu->desc.bNrInPins = n;
+	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+						      xu->desc.bControlSize);
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	mutex_unlock(su_mutex);
+	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 config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(item);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+	char *pg = page;
+	int ret, i;
+
+	mutex_lock(su_mutex);
+
+	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);
+
+	mutex_unlock(su_mutex);
+
+	return ret;
+}
+
+static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
+						const char *page, size_t len)
+{
+	struct config_group *group = to_config_group(item->ci_parent);
+	struct mutex *su_mutex = &group->cg_subsys->su_mutex;
+	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;
+
+	mutex_lock(su_mutex);
+
+	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(page, len, __uvcg_count_item_entries, &n,
+				       sizeof(u8));
+	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(page, len, __uvcg_fill_item_entries, &tmp,
+				       sizeof(u8));
+	if (ret) {
+		kfree(bm_controls);
+		goto unlock;
+	}
+
+	kfree(xu->desc.bmControls);
+	xu->desc.bmControls = bm_controls;
+	xu->desc.bControlSize = n;
+	xu->desc.bLength = UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+						      xu->desc.bControlSize);
+
+	ret = len;
+
+unlock:
+	mutex_unlock(&opts->lock);
+	mutex_unlock(su_mutex);
+	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,
+	&uvcg_extension_attr_i_extension,
+	NULL,
+};
+
+static void uvcg_extension_release(struct config_item *item)
+{
+	struct uvcg_extension *xu = container_of(item, struct uvcg_extension, item);
+
+	kfree(xu);
+}
+
+static struct configfs_item_operations uvcg_extension_item_ops = {
+	.release	= uvcg_extension_release,
+};
+
+static const struct config_item_type uvcg_extension_type = {
+	.ct_item_ops	= &uvcg_extension_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);
+
+	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);
+
+	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.bNumControls = 0;
+	xu->desc.bNrInPins = 0;
+	xu->desc.baSourceID = NULL;
+	xu->desc.bControlSize = 0;
+	xu->desc.bmControls = NULL;
+
+	mutex_lock(&opts->lock);
+
+	xu->desc.bUnitID = ++opts->last_unit_id;
+
+	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}
  */
@@ -855,6 +1334,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


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 04/11] usb: gadget: uvc: Copy XU descriptors during .bind()
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (2 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 05/11] usb: gadget: configfs: Rename struct gadget_strings Daniel Scally
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, 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>
---
Laurent - I didn't add your tag because having switched it to a function I actually
decided I preferred it this way and switched it back; too many pointers to
pointers of things made it less easy to follow I think, but if you disagree let me
know and I'll go ahead.

Changes in v3:
	- Dropped a local variable.

Changes in v2:

	- none

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

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 64a9e35ff075..ca548974e5a0 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -474,6 +474,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)
 {
@@ -485,6 +504,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;
@@ -549,6 +569,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;
@@ -575,6 +602,10 @@ 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)
+		UVC_COPY_XU_DESCRIPTOR(mem, dst, &xu->desc);
+
 	uvc_control_header->wTotalLength = cpu_to_le16(control_size);
 	uvc_control_header->bInCollection = 1;
 	uvc_control_header->baInterfaceNr[0] = uvc->streaming_intf;
@@ -1020,6 +1051,8 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 		return ERR_PTR(-EBUSY);
 	}
 
+	uvc->desc.extension_units = &opts->extension_units;
+
 	++opts->refcnt;
 	mutex_unlock(&opts->lock);
 
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


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 05/11] usb: gadget: configfs: Rename struct gadget_strings
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (3 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 04/11] usb: gadget: uvc: Copy XU descriptors during .bind() Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

The struct gadget_strings really represents a single language in
configfs. Rename it to make that more clear.

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

	- New patch

 drivers/usb/gadget/configfs.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 0853536cbf2e..e0f93c42cde6 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -79,7 +79,7 @@ static inline struct gadget_info *cfg_to_gadget_info(struct config_usb_cfg *cfg)
 	return container_of(cfg->c.cdev, struct gadget_info, cdev);
 }
 
-struct gadget_strings {
+struct gadget_language {
 	struct usb_gadget_strings stringtab_dev;
 	struct usb_string strings[USB_GADGET_FIRST_AVAIL_IDX];
 	char *manufacturer;
@@ -365,9 +365,9 @@ static struct configfs_attribute *gadget_root_attrs[] = {
 	NULL,
 };
 
-static inline struct gadget_strings *to_gadget_strings(struct config_item *item)
+static inline struct gadget_language *to_gadget_language(struct config_item *item)
 {
-	return container_of(to_config_group(item), struct gadget_strings,
+	return container_of(to_config_group(item), struct gadget_language,
 			 group);
 }
 
@@ -755,20 +755,20 @@ static const struct config_item_type config_desc_type = {
 	.ct_owner       = THIS_MODULE,
 };
 
-GS_STRINGS_RW(gadget_strings, manufacturer);
-GS_STRINGS_RW(gadget_strings, product);
-GS_STRINGS_RW(gadget_strings, serialnumber);
+GS_STRINGS_RW(gadget_language, manufacturer);
+GS_STRINGS_RW(gadget_language, product);
+GS_STRINGS_RW(gadget_language, serialnumber);
 
-static struct configfs_attribute *gadget_strings_langid_attrs[] = {
-	&gadget_strings_attr_manufacturer,
-	&gadget_strings_attr_product,
-	&gadget_strings_attr_serialnumber,
+static struct configfs_attribute *gadget_language_langid_attrs[] = {
+	&gadget_language_attr_manufacturer,
+	&gadget_language_attr_product,
+	&gadget_language_attr_serialnumber,
 	NULL,
 };
 
-static void gadget_strings_attr_release(struct config_item *item)
+static void gadget_language_attr_release(struct config_item *item)
 {
-	struct gadget_strings *gs = to_gadget_strings(item);
+	struct gadget_language *gs = to_gadget_language(item);
 
 	kfree(gs->manufacturer);
 	kfree(gs->product);
@@ -778,8 +778,8 @@ static void gadget_strings_attr_release(struct config_item *item)
 	kfree(gs);
 }
 
-USB_CONFIG_STRING_RW_OPS(gadget_strings);
-USB_CONFIG_STRINGS_LANG(gadget_strings, gadget_info);
+USB_CONFIG_STRING_RW_OPS(gadget_language);
+USB_CONFIG_STRINGS_LANG(gadget_language, gadget_info);
 
 static inline struct gadget_info *os_desc_item_to_gadget_info(
 		struct config_item *item)
@@ -1316,7 +1316,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 
 	/* init all strings */
 	if (!list_empty(&gi->string_list)) {
-		struct gadget_strings *gs;
+		struct gadget_language *gs;
 
 		i = 0;
 		list_for_each_entry(gs, &gi->string_list, list) {
@@ -1598,7 +1598,7 @@ static struct config_group *gadgets_make(
 	configfs_add_default_group(&gi->configs_group, &gi->group);
 
 	config_group_init_type_name(&gi->strings_group, "strings",
-			&gadget_strings_strings_type);
+			&gadget_language_strings_type);
 	configfs_add_default_group(&gi->strings_group, &gi->group);
 
 	config_group_init_type_name(&gi->os_desc_group, "os_desc",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (4 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 05/11] usb: gadget: configfs: Rename struct gadget_strings Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30 14:24   ` kernel test robot
                     ` (2 more replies)
  2023-01-30  9:34 ` [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev Daniel Scally
                   ` (4 subsequent siblings)
  10 siblings, 3 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Add a framework to allow users to define arbitrary string descriptors
for a USB Gadget. This is modelled as a new type of config item rather
than as hardcoded attributes so as to be as flexible as possible.

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

	- Moved this functionality from the UVC function to usb gadget core.

Changes in v2:

	- New patch

 drivers/usb/gadget/configfs.c | 172 +++++++++++++++++++++++++++++++++-
 include/linux/usb/gadget.h    |  11 +++
 2 files changed, 181 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index e0f93c42cde6..7c8b8ab5dfa3 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -88,6 +88,8 @@ struct gadget_language {
 
 	struct config_group group;
 	struct list_head list;
+	struct list_head gadget_strings;
+	unsigned int nstrings;
 };
 
 struct gadget_config_name {
@@ -778,8 +780,174 @@ static void gadget_language_attr_release(struct config_item *item)
 	kfree(gs);
 }
 
-USB_CONFIG_STRING_RW_OPS(gadget_language);
-USB_CONFIG_STRINGS_LANG(gadget_language, gadget_info);
+static struct configfs_item_operations gadget_language_langid_item_ops = {
+	.release                = gadget_language_attr_release,
+};
+
+static ssize_t gadget_string_id_show(struct config_item *item, char *page)
+{
+	struct gadget_string *string = to_gadget_string(item);
+	int ret;
+
+	ret = sprintf(page, "%u\n", string->usb_string.id);
+	return ret;
+}
+CONFIGFS_ATTR_RO(gadget_string_, id);
+
+static ssize_t gadget_string_s_show(struct config_item *item, char *page)
+{
+	struct gadget_string *string = to_gadget_string(item);
+	int ret;
+
+	ret = snprintf(page, sizeof(string->string), "%s\n", string->string);
+	return ret;
+}
+
+static ssize_t gadget_string_s_store(struct config_item *item, const char *page,
+				     size_t len)
+{
+	struct gadget_string *string = to_gadget_string(item);
+	int size = min(sizeof(string->string), len + 1);
+	int ret;
+
+	if (len > USB_MAX_STRING_LEN)
+		return -EINVAL;
+
+	ret = strscpy(string->string, page, size);
+	return len;
+}
+CONFIGFS_ATTR(gadget_string_, s);
+
+static struct configfs_attribute *gadget_string_attrs[] = {
+	&gadget_string_attr_id,
+	&gadget_string_attr_s,
+	NULL,
+};
+
+static void gadget_string_release(struct config_item *item)
+{
+	struct gadget_string *string = to_gadget_string(item);
+
+	kfree(string);
+}
+
+static struct configfs_item_operations gadget_string_item_ops = {
+	.release	= gadget_string_release,
+};
+
+static const struct config_item_type gadget_string_type = {
+	.ct_item_ops	= &gadget_string_item_ops,
+	.ct_attrs	= gadget_string_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct config_item *gadget_language_string_make(struct config_group *group,
+						       const char *name)
+{
+	struct gadget_language *language;
+	struct gadget_string *string;
+
+	language = to_gadget_language(&group->cg_item);
+
+	string = kzalloc(sizeof(*string), GFP_KERNEL);
+	if (!string)
+		return ERR_PTR(-ENOMEM);
+
+	string->usb_string.id = language->nstrings++;
+	string->usb_string.s = string->string;
+	list_add_tail(&string->list, &language->gadget_strings);
+
+	config_item_init_type_name(&string->item, name, &gadget_string_type);
+
+	return &string->item;
+}
+
+static void gadget_language_string_drop(struct config_group *group,
+					struct config_item *item)
+{
+	struct gadget_language *language;
+	struct gadget_string *string;
+	unsigned int i = USB_GADGET_FIRST_AVAIL_IDX;
+
+	language = to_gadget_language(&group->cg_item);
+	string = to_gadget_string(item);
+
+	list_del(&string->list);
+	language->nstrings--;
+
+	/* Reset the ids for the language's strings to guarantee a continuous set */
+	list_for_each_entry(string, &language->gadget_strings, list)
+		string->usb_string.id = i++;
+}
+
+static struct configfs_group_operations gadget_language_langid_group_ops = {
+	.make_item		= gadget_language_string_make,
+	.drop_item		= gadget_language_string_drop,
+};
+
+static struct config_item_type gadget_language_type = {
+	.ct_item_ops	= &gadget_language_langid_item_ops,
+	.ct_group_ops	= &gadget_language_langid_group_ops,
+	.ct_attrs	= gadget_language_langid_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct config_group *gadget_language_make(struct config_group *group,
+						 const char *name)
+{
+	struct gadget_info *gi;
+	struct gadget_language *gs;
+	struct gadget_language *new;
+	int langs = 0;
+	int ret;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	ret = check_user_usb_string(name, &new->stringtab_dev);
+	if (ret)
+		goto err;
+	config_group_init_type_name(&new->group, name,
+				    &gadget_language_type);
+
+	gi = container_of(group, struct gadget_info, strings_group);
+	ret = -EEXIST;
+	list_for_each_entry(gs, &gi->string_list, list) {
+		if (gs->stringtab_dev.language == new->stringtab_dev.language)
+			goto err;
+		langs++;
+	}
+	ret = -EOVERFLOW;
+	if (langs >= MAX_USB_STRING_LANGS)
+		goto err;
+
+	list_add_tail(&new->list, &gi->string_list);
+	INIT_LIST_HEAD(&new->gadget_strings);
+
+	/* We have the default manufacturer, product and serialnumber strings */
+	new->nstrings = 3;
+	return &new->group;
+err:
+	kfree(new);
+	return ERR_PTR(ret);
+}
+
+static void gadget_language_drop(struct config_group *group,
+				 struct config_item *item)
+{
+	config_item_put(item);
+}
+
+static struct configfs_group_operations gadget_language_group_ops = {
+	.make_group     = &gadget_language_make,
+	.drop_item      = &gadget_language_drop,
+};
+
+static struct config_item_type gadget_language_strings_type = {
+	.ct_group_ops   = &gadget_language_group_ops,
+	.ct_owner       = THIS_MODULE,
+};
 
 static inline struct gadget_info *os_desc_item_to_gadget_info(
 		struct config_item *item)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index dc3092cea99e..00750f7020f3 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -15,6 +15,7 @@
 #ifndef __LINUX_USB_GADGET_H
 #define __LINUX_USB_GADGET_H
 
+#include <linux/configfs.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/init.h>
@@ -821,6 +822,16 @@ int usb_gadget_get_string(const struct usb_gadget_strings *table, int id, u8 *bu
 /* check if the given language identifier is valid */
 bool usb_validate_langid(u16 langid);
 
+struct gadget_string {
+	struct config_item item;
+	struct list_head list;
+	char string[USB_MAX_STRING_LEN];
+	struct usb_string usb_string;
+};
+
+#define to_gadget_string(str_item)\
+container_of(str_item, struct gadget_string, item)
+
 /*-------------------------------------------------------------------------*/
 
 /* utility to simplify managing config descriptors */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (5 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 08/11] usb: gadget: uvc: Allow linking XUs to string descriptors Daniel Scally
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Attach any arbitrary strings that are defined to the composite dev.
We handle the old-style manufacturer, product and serialnumbers
strings in the same function for simplicity.

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

	- Was 7/9 in version 2, moved the same functionality from the UVC
	function to usb gadget core.

Changes in v2:

	- New patch

 drivers/usb/gadget/configfs.c | 95 ++++++++++++++++++++++++++++-------
 include/linux/usb/composite.h |  1 +
 2 files changed, 79 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 7c8b8ab5dfa3..c2f23a63ab10 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1441,6 +1441,80 @@ static void purge_configs_funcs(struct gadget_info *gi)
 	}
 }
 
+static struct usb_string *
+configfs_attach_gadget_strings(struct gadget_info *gi)
+{
+	struct usb_gadget_strings **gadget_strings;
+	struct gadget_language *language;
+	struct gadget_string *string;
+	unsigned int nlangs = 0;
+	struct list_head *iter;
+	struct usb_string *us;
+	unsigned int i = 0;
+	int nstrings = -1;
+	unsigned int j;
+
+	list_for_each(iter, &gi->string_list)
+		nlangs++;
+
+	/* Bail out early if no languages are configured */
+	if (!nlangs)
+		return NULL;
+
+	gadget_strings = kcalloc(nlangs + 1, /* including NULL terminator */
+				 sizeof(struct usb_gadget_strings *), GFP_KERNEL);
+	if (!gadget_strings)
+		return ERR_PTR(-ENOMEM);
+
+	list_for_each_entry(language, &gi->string_list, list) {
+		struct usb_string *stringtab;
+
+		if (nstrings == -1) {
+			nstrings = language->nstrings;
+		} else if (nstrings != language->nstrings) {
+			pr_err("languages must contain the same number of strings\n");
+			us = ERR_PTR(-EINVAL);
+			goto cleanup;
+		}
+
+		stringtab = kcalloc(language->nstrings + 1, sizeof(struct usb_string),
+				    GFP_KERNEL);
+		if (!stringtab) {
+			us = ERR_PTR(-ENOMEM);
+			goto cleanup;
+		}
+
+		stringtab[USB_GADGET_MANUFACTURER_IDX].id = USB_GADGET_MANUFACTURER_IDX;
+		stringtab[USB_GADGET_MANUFACTURER_IDX].s = language->manufacturer;
+		stringtab[USB_GADGET_PRODUCT_IDX].id = USB_GADGET_PRODUCT_IDX;
+		stringtab[USB_GADGET_PRODUCT_IDX].s = language->product;
+		stringtab[USB_GADGET_SERIAL_IDX].id = USB_GADGET_SERIAL_IDX;
+		stringtab[USB_GADGET_SERIAL_IDX].s = language->serialnumber;
+
+		j = USB_GADGET_FIRST_AVAIL_IDX;
+		list_for_each_entry(string, &language->gadget_strings, list) {
+			memcpy(&stringtab[j], &string->usb_string, sizeof(struct usb_string));
+			j++;
+		}
+
+		language->stringtab_dev.strings = stringtab;
+		gadget_strings[i] = &language->stringtab_dev;
+		i++;
+	}
+
+	us = usb_gstrings_attach(&gi->cdev, gadget_strings, nstrings);
+
+cleanup:
+	list_for_each_entry(language, &gi->string_list, list) {
+		kfree(language->stringtab_dev.strings);
+		language->stringtab_dev.strings = NULL;
+	}
+
+	kfree(gadget_strings);
+
+	return us;
+}
+
 static int configfs_composite_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *gdriver)
 {
@@ -1484,23 +1558,8 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 
 	/* init all strings */
 	if (!list_empty(&gi->string_list)) {
-		struct gadget_language *gs;
-
-		i = 0;
-		list_for_each_entry(gs, &gi->string_list, list) {
-
-			gi->gstrings[i] = &gs->stringtab_dev;
-			gs->stringtab_dev.strings = gs->strings;
-			gs->strings[USB_GADGET_MANUFACTURER_IDX].s =
-				gs->manufacturer;
-			gs->strings[USB_GADGET_PRODUCT_IDX].s = gs->product;
-			gs->strings[USB_GADGET_SERIAL_IDX].s = gs->serialnumber;
-			i++;
-		}
-		gi->gstrings[i] = NULL;
-		s = usb_gstrings_attach(&gi->cdev, gi->gstrings,
-				USB_GADGET_FIRST_AVAIL_IDX);
-		if (IS_ERR(s)) {
+		s = configfs_attach_gadget_strings(gi);
+		if (IS_ERR_OR_NULL(s)) {
 			ret = PTR_ERR(s);
 			goto err_comp_cleanup;
 		}
@@ -1508,6 +1567,8 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
 		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
 		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
+
+		gi->cdev.usb_strings = s;
 	}
 
 	if (gi->use_os_desc) {
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 43ac3fa760db..85c7f6036933 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -483,6 +483,7 @@ struct usb_composite_dev {
 	struct usb_composite_driver	*driver;
 	u8				next_string_id;
 	char				*def_manufacturer;
+	struct usb_string		*usb_strings;
 
 	/* the gadget driver won't enable the data pullup
 	 * while the deactivation count is nonzero.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 08/11] usb: gadget: uvc: Allow linking XUs to string descriptors
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (6 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 09/11] usb: gadget: uvc: Pick up custom string descriptor IDs Daniel Scally
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Add .allow_link() and .drop_link() callbacks to allow users to link
an extension unit descriptor to a string descriptor.

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

	- Changed the target of the links to in <gadget root>/strings

Changes in v2:

	- New patch

 drivers/usb/gadget/function/uvc_configfs.c | 52 ++++++++++++++++++++++
 drivers/usb/gadget/function/uvc_configfs.h |  1 +
 2 files changed, 53 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index a81613e9652d..9da3b784f954 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1063,8 +1063,60 @@ static void uvcg_extension_release(struct config_item *item)
 	kfree(xu);
 }
 
+static int uvcg_extension_allow_link(struct config_item *src, struct config_item *tgt)
+{
+	struct mutex *su_mutex = &src->ci_group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(src);
+	struct config_item *gadget_item;
+	struct gadget_string *string;
+	struct config_item *strings;
+	int ret = 0;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	/* Validate that the target of the link is an entry in strings/<langid> */
+	gadget_item = src->ci_parent->ci_parent->ci_parent->ci_parent->ci_parent;
+	strings = config_group_find_item(to_config_group(gadget_item), "strings");
+	if (!strings || tgt->ci_parent->ci_parent != strings) {
+		ret = -EINVAL;
+		goto put_strings;
+	}
+
+	string = to_gadget_string(tgt);
+	xu->string_descriptor_index = string->usb_string.id;
+
+put_strings:
+	config_item_put(strings);
+	mutex_unlock(su_mutex);
+
+	return ret;
+}
+
+static void uvcg_extension_drop_link(struct config_item *src, struct config_item *tgt)
+{
+	struct mutex *su_mutex = &src->ci_group->cg_subsys->su_mutex;
+	struct uvcg_extension *xu = to_uvcg_extension(src);
+	struct config_item *opts_item;
+	struct f_uvc_opts *opts;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	opts_item = src->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+
+	xu->string_descriptor_index = 0;
+
+	mutex_unlock(&opts->lock);
+
+	mutex_unlock(su_mutex);
+}
+
 static struct configfs_item_operations uvcg_extension_item_ops = {
 	.release	= uvcg_extension_release,
+	.allow_link	= uvcg_extension_allow_link,
+	.drop_link	= uvcg_extension_drop_link,
 };
 
 static const struct config_item_type uvcg_extension_type = {
diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
index c9a4182fb26f..bf188080034c 100644
--- a/drivers/usb/gadget/function/uvc_configfs.h
+++ b/drivers/usb/gadget/function/uvc_configfs.h
@@ -153,6 +153,7 @@ struct uvcg_extension_unit_descriptor {
 struct uvcg_extension {
 	struct config_item item;
 	struct list_head list;
+	u8 string_descriptor_index;
 	struct uvcg_extension_unit_descriptor desc;
 };
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 09/11] usb: gadget: uvc: Pick up custom string descriptor IDs
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (7 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 08/11] usb: gadget: uvc: Allow linking XUs to string descriptors Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 10/11] usb: gadget: uvc: Allow linking function to string descs Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 11/11] usb: gadget: uvc: Use custom strings if available Daniel Scally
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

If any custom string descriptors have been linked to from the
extension unit, pick up the string ID that was returned when
the strings were attached to the composite dev and use it to
set the iExtension field of the Extension Unit Descriptor.

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

	- New patch

 drivers/usb/gadget/function/f_uvc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index ca548974e5a0..642ab210abb1 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -634,6 +634,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct usb_composite_dev *cdev = c->cdev;
 	struct uvc_device *uvc = to_uvc(f);
+	struct uvcg_extension *xu;
 	struct usb_string *us;
 	unsigned int max_packet_mult;
 	unsigned int max_packet_size;
@@ -722,6 +723,14 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
 
+	/*
+	 * XUs can have an arbitrary string descriptor describing them. If they
+	 * have one pick up the ID.
+	 */
+	list_for_each_entry(xu, &opts->extension_units, list)
+		if (xu->string_descriptor_index)
+			xu->desc.iExtension = cdev->usb_strings[xu->string_descriptor_index].id;
+
 	uvc_en_us_strings[UVC_STRING_CONTROL_IDX].s = opts->function_name;
 	us = usb_gstrings_attach(cdev, uvc_function_strings,
 				 ARRAY_SIZE(uvc_en_us_strings));
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 10/11] usb: gadget: uvc: Allow linking function to string descs
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (8 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 09/11] usb: gadget: uvc: Pick up custom string descriptor IDs Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  2023-01-30  9:34 ` [PATCH v3 11/11] usb: gadget: uvc: Use custom strings if available Daniel Scally
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Currently the string descriptors for the IAD and VideoStreaming
Interfaces are hardcoded into f_uvc. Now that we can create arbitrary
string descriptors, add a mechanism to define string descriptors for
the IAD, VC and VS interfaces by linking to the appropriate directory
at function level.

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

	- Changed target of link to be within <gadget root>/strings
	- Dropped the VideoControl description since there's an attribute for
	that now

Changes in v2:

	- New patch

 drivers/usb/gadget/function/u_uvc.h        |  8 +++
 drivers/usb/gadget/function/uvc_configfs.c | 60 ++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 5119cfe5ee4e..565c14fb3d73 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -81,6 +81,14 @@ struct f_uvc_opts {
 	struct uvc_descriptor_header			**uvc_hs_streaming_cls;
 	struct uvc_descriptor_header			**uvc_ss_streaming_cls;
 
+	/*
+	 * Indexes into the function's string descriptors allowing users to set
+	 * custom descriptions rather than the hard-coded defaults.
+	 */
+	u8						iad_index;
+	u8						vs0_index;
+	u8						vs1_index;
+
 	/*
 	 * Read/write access to configfs attributes is handled by configfs.
 	 *
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 9da3b784f954..ffa338643cc8 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2901,8 +2901,68 @@ static void uvc_func_item_release(struct config_item *item)
 	usb_put_function_instance(&opts->func_inst);
 }
 
+static int uvc_func_allow_link(struct config_item *src, struct config_item *tgt)
+{
+	struct mutex *su_mutex = &src->ci_group->cg_subsys->su_mutex;
+	struct gadget_string *string;
+	struct config_item *strings;
+	struct f_uvc_opts *opts;
+	int ret = 0;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	/* Validate that the target is an entry in strings/<langid> */
+	strings = config_group_find_item(to_config_group(src->ci_parent->ci_parent),
+					 "strings");
+	if (!strings || tgt->ci_parent->ci_parent != strings) {
+		ret = -EINVAL;
+		goto put_strings;
+	}
+
+	string = to_gadget_string(tgt);
+
+	opts = to_f_uvc_opts(src);
+	mutex_lock(&opts->lock);
+
+	if (!strcmp(tgt->ci_name, "iad_desc"))
+		opts->iad_index = string->usb_string.id;
+	else if (!strcmp(tgt->ci_name, "vs0_desc"))
+		opts->vs0_index = string->usb_string.id;
+	else if (!strcmp(tgt->ci_name, "vs1_desc"))
+		opts->vs1_index = string->usb_string.id;
+	else
+		ret = -EINVAL;
+
+	mutex_unlock(&opts->lock);
+
+put_strings:
+	config_item_put(strings);
+	mutex_unlock(su_mutex);
+
+	return ret;
+}
+
+static void uvc_func_drop_link(struct config_item *src, struct config_item *tgt)
+{
+	struct f_uvc_opts *opts;
+
+	opts = to_f_uvc_opts(src);
+	mutex_lock(&opts->lock);
+
+	if (!strcmp(tgt->ci_name, "iad_desc"))
+		opts->iad_index = 0;
+	else if (!strcmp(tgt->ci_name, "vs0_desc"))
+		opts->vs0_index = 0;
+	else if (!strcmp(tgt->ci_name, "vs1_desc"))
+		opts->vs1_index = 0;
+
+	mutex_unlock(&opts->lock);
+}
+
 static struct configfs_item_operations uvc_func_item_ops = {
 	.release	= uvc_func_item_release,
+	.allow_link	= uvc_func_allow_link,
+	.drop_link	= uvc_func_drop_link,
 };
 
 #define UVCG_OPTS_ATTR(cname, aname, limit)				\
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v3 11/11] usb: gadget: uvc: Use custom strings if available
  2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
                   ` (9 preceding siblings ...)
  2023-01-30  9:34 ` [PATCH v3 10/11] usb: gadget: uvc: Allow linking function to string descs Daniel Scally
@ 2023-01-30  9:34 ` Daniel Scally
  10 siblings, 0 replies; 20+ messages in thread
From: Daniel Scally @ 2023-01-30  9:34 UTC (permalink / raw)
  To: linux-usb, gregkh, laurent.pinchart
  Cc: mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

If the user has defined a custom string descriptor for the IAD or the
VideoStreaming interfaces then set their index field to point to the
custom descriptor instead of the hardcoded defaults. If no custom
descriptors have been linked to, then use the default ones.

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

	- Dropped the VideoControl description since there's an attribute for
	that now

Changes in v2:

	- New patch

 drivers/usb/gadget/function/f_uvc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 642ab210abb1..7bce5e5f226a 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -731,6 +731,10 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		if (xu->string_descriptor_index)
 			xu->desc.iExtension = cdev->usb_strings[xu->string_descriptor_index].id;
 
+	/*
+	 * We attach the hard-coded defaults incase the user does not provide
+	 * any more appropriate strings through configfs.
+	 */
 	uvc_en_us_strings[UVC_STRING_CONTROL_IDX].s = opts->function_name;
 	us = usb_gstrings_attach(cdev, uvc_function_strings,
 				 ARRAY_SIZE(uvc_en_us_strings));
@@ -738,11 +742,15 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ret = PTR_ERR(us);
 		goto error;
 	}
-	uvc_iad.iFunction = us[UVC_STRING_CONTROL_IDX].id;
-	uvc_control_intf.iInterface = us[UVC_STRING_CONTROL_IDX].id;
-	ret = us[UVC_STRING_STREAMING_IDX].id;
-	uvc_streaming_intf_alt0.iInterface = ret;
-	uvc_streaming_intf_alt1.iInterface = ret;
+
+	uvc_iad.iFunction = opts->iad_index ? cdev->usb_strings[opts->iad_index].id :
+			    us[UVC_STRING_CONTROL_IDX].id;
+	uvc_streaming_intf_alt0.iInterface = opts->vs0_index ?
+					     cdev->usb_strings[opts->vs0_index].id :
+					     us[UVC_STRING_STREAMING_IDX].id;
+	uvc_streaming_intf_alt1.iInterface = opts->vs1_index ?
+					     cdev->usb_strings[opts->vs1_index].id :
+					     us[UVC_STRING_STREAMING_IDX].id;
 
 	/* Allocate interface IDs. */
 	if ((ret = usb_interface_id(c, f)) < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
  2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
@ 2023-01-30 14:24   ` kernel test robot
  2023-01-30 16:07   ` kernel test robot
  2023-01-30 16:37   ` Alan Stern
  2 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-01-30 14:24 UTC (permalink / raw)
  To: Daniel Scally, linux-usb, gregkh, laurent.pinchart
  Cc: llvm, oe-kbuild-all, mgr, balbi, kieran.bingham, torleiv, stern,
	Daniel Scally

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-linus]
[also build test WARNING on westeri-thunderbolt/next linus/master v6.2-rc6 next-20230130]
[cannot apply to usb/usb-testing usb/usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-7-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
config: i386-randconfig-a013-20230130 (https://download.01.org/0day-ci/archive/20230130/202301302236.Ogdqs7GZ-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/452304e81fac1427f314a4b0beef8561ef4ebf17
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
        git checkout 452304e81fac1427f314a4b0beef8561ef4ebf17
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/gadget/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/gadget/configfs.c:811:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
           int ret;
               ^
   1 warning generated.


vim +/ret +811 drivers/usb/gadget/configfs.c

   805	
   806	static ssize_t gadget_string_s_store(struct config_item *item, const char *page,
   807					     size_t len)
   808	{
   809		struct gadget_string *string = to_gadget_string(item);
   810		int size = min(sizeof(string->string), len + 1);
 > 811		int ret;
   812	
   813		if (len > USB_MAX_STRING_LEN)
   814			return -EINVAL;
   815	
   816		ret = strscpy(string->string, page, size);
   817		return len;
   818	}
   819	CONFIGFS_ATTR(gadget_string_, s);
   820	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
  2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
  2023-01-30 14:24   ` kernel test robot
@ 2023-01-30 16:07   ` kernel test robot
  2023-01-30 16:37   ` Alan Stern
  2 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-01-30 16:07 UTC (permalink / raw)
  To: Daniel Scally, linux-usb, gregkh, laurent.pinchart
  Cc: oe-kbuild-all, mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-linus]
[also build test WARNING on westeri-thunderbolt/next linus/master v6.2-rc6 next-20230130]
[cannot apply to usb/usb-testing usb/usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-7-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230130/202301302344.mwzUOCim-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/452304e81fac1427f314a4b0beef8561ef4ebf17
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
        git checkout 452304e81fac1427f314a4b0beef8561ef4ebf17
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/usb/gadget/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/gadget/configfs.c: In function 'gadget_string_s_store':
>> drivers/usb/gadget/configfs.c:811:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     811 |         int ret;
         |             ^~~


vim +/ret +811 drivers/usb/gadget/configfs.c

   805	
   806	static ssize_t gadget_string_s_store(struct config_item *item, const char *page,
   807					     size_t len)
   808	{
   809		struct gadget_string *string = to_gadget_string(item);
   810		int size = min(sizeof(string->string), len + 1);
 > 811		int ret;
   812	
   813		if (len > USB_MAX_STRING_LEN)
   814			return -EINVAL;
   815	
   816		ret = strscpy(string->string, page, size);
   817		return len;
   818	}
   819	CONFIGFS_ATTR(gadget_string_, s);
   820	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
  2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
  2023-01-30 14:24   ` kernel test robot
  2023-01-30 16:07   ` kernel test robot
@ 2023-01-30 16:37   ` Alan Stern
  2023-02-02 13:06     ` Dan Scally
  2 siblings, 1 reply; 20+ messages in thread
From: Alan Stern @ 2023-01-30 16:37 UTC (permalink / raw)
  To: Daniel Scally
  Cc: linux-usb, gregkh, laurent.pinchart, mgr, balbi, kieran.bingham, torleiv

On Mon, Jan 30, 2023 at 09:34:38AM +0000, Daniel Scally wrote:
> Add a framework to allow users to define arbitrary string descriptors
> for a USB Gadget. This is modelled as a new type of config item rather
> than as hardcoded attributes so as to be as flexible as possible.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v3:
> 
> 	- Moved this functionality from the UVC function to usb gadget core.
> 
> Changes in v2:
> 
> 	- New patch
> 
>  drivers/usb/gadget/configfs.c | 172 +++++++++++++++++++++++++++++++++-
>  include/linux/usb/gadget.h    |  11 +++
>  2 files changed, 181 insertions(+), 2 deletions(-)

Shouldn't this patch also include an update to 
Documentation/usb/gadget_configfs.rst?

Alan Stern

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs
  2023-01-30  9:34 ` [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs Daniel Scally
@ 2023-01-30 20:13   ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-01-30 20:13 UTC (permalink / raw)
  To: Daniel Scally, linux-usb, gregkh, laurent.pinchart
  Cc: oe-kbuild-all, mgr, balbi, kieran.bingham, torleiv, stern, Daniel Scally

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-linus]
[also build test WARNING on westeri-thunderbolt/next linus/master v6.2-rc6 next-20230130]
[cannot apply to usb/usb-testing usb/usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-4-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs
reproduce:
        # https://github.com/intel-lab-lkp/linux/commit/c1a147d5dfd26ea76a1d054efc3a184dec36061b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
        git checkout c1a147d5dfd26ea76a1d054efc3a184dec36061b
        make menuconfig
        # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
        make htmldocs

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> Documentation/ABI/testing/configfs-usb-gadget-uvc:120: WARNING: Malformed table.

vim +120 Documentation/ABI/testing/configfs-usb-gadget-uvc

   118	
   119	What:		/config/usb-gadget/gadget/functions/uvc.name/control/extensions/name
 > 120	Date:		Nov 2022
   121	KernelVersion:	6.1
   122	Description:	Extension Unit (XU) Descriptor
   123	
   124			bLength, bUnitID and iExtension are read-only. All others are
   125			read-write.
   126	
   127			===============		========================================
   128			bLength			size of the descriptor in bytes
   129			bUnitID			non-zero ID of this unit
   130			guidExtensionCode	Vendor-specific code identifying the XU
   131			bNumControls		number of controls in this XU
   132			bNrInPins		number of input pins for this unit
   133			baSourceID		list of the IDs of the units or terminals
   134						to which this XU is connected
   135			bControlSize		size of the bmControls field in bytes
   136			bmControls		list of bitmaps detailing which vendor
   137						specific controls are supported
   138			iExtension		index of a string descriptor that describes
   139						this extension unit
   140			===============		========================================
   141	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
@ 2023-01-31  7:35 ` Dan Carpenter
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-01-31  7:09 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230130093443.25644-8-dan.scally@ideasonboard.com>
References: <20230130093443.25644-8-dan.scally@ideasonboard.com>
TO: Daniel Scally <dan.scally@ideasonboard.com>
TO: linux-usb@vger.kernel.org
TO: gregkh@linuxfoundation.org
TO: laurent.pinchart@ideasonboard.com
CC: mgr@pengutronix.de
CC: balbi@kernel.org
CC: kieran.bingham@ideasonboard.com
CC: torleiv@huddly.com
CC: stern@rowland.harvard.edu
CC: Daniel Scally <dan.scally@ideasonboard.com>

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-linus]
[also build test WARNING on westeri-thunderbolt/next linus/master v6.2-rc6 next-20230130]
[cannot apply to usb/usb-testing usb/usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-8-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
:::::: branch date: 21 hours ago
:::::: commit date: 21 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230131/202301311446.AqdvqXkI-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

New smatch warnings:
drivers/usb/gadget/configfs.c:1563 configfs_composite_bind() warn: passing zero to 'PTR_ERR'

Old smatch warnings:
drivers/usb/gadget/configfs.c:985 os_desc_b_vendor_code_show() warn: argument 3 to %02x specifier has type 'char'

vim +/PTR_ERR +1563 drivers/usb/gadget/configfs.c

0c252735443756 Daniel Scally             2023-01-30  1517  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1518  static int configfs_composite_bind(struct usb_gadget *gadget,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1519  		struct usb_gadget_driver *gdriver)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1520  {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1521  	struct usb_composite_driver     *composite = to_cdriver(gdriver);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1522  	struct gadget_info		*gi = container_of(composite,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1523  						struct gadget_info, composite);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1524  	struct usb_composite_dev	*cdev = &gi->cdev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1525  	struct usb_configuration	*c;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1526  	struct usb_string		*s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1527  	unsigned			i;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1528  	int				ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1529  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1530  	/* the gi->lock is hold by the caller */
1a1c851bbd706e Peter Chen                2019-08-26  1531  	gi->unbind = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1532  	cdev->gadget = gadget;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1533  	set_gadget_data(gadget, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1534  	ret = composite_dev_prepare(composite, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1535  	if (ret)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1536  		return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1537  	/* and now the gadget bind */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1538  	ret = -EINVAL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1539  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1540  	if (list_empty(&gi->cdev.configs)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1541  		pr_err("Need at least one configuration in %s.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1542  				gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1543  		goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1544  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1545  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1546  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1547  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1548  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1549  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1550  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1551  		if (list_empty(&cfg->func_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1552  			pr_err("Config %s/%d of %s needs at least one function.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1553  			      c->label, c->bConfigurationValue,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1554  			      gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1555  			goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1556  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1557  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1558  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1559  	/* init all strings */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1560  	if (!list_empty(&gi->string_list)) {
0c252735443756 Daniel Scally             2023-01-30  1561  		s = configfs_attach_gadget_strings(gi);
0c252735443756 Daniel Scally             2023-01-30  1562  		if (IS_ERR_OR_NULL(s)) {
fea77077d1623c Wei Yongjun               2013-05-07 @1563  			ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1564  			goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1565  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1566  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1567  		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1568  		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1569  		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
0c252735443756 Daniel Scally             2023-01-30  1570  
0c252735443756 Daniel Scally             2023-01-30  1571  		gi->cdev.usb_strings = s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1572  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1573  
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1574  	if (gi->use_os_desc) {
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1575  		cdev->use_os_string = true;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1576  		cdev->b_vendor_code = gi->b_vendor_code;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1577  		memcpy(cdev->qw_sign, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1578  	}
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1579  
41ce84c86d0a04 Li Jun                    2015-07-09  1580  	if (gadget_is_otg(gadget) && !otg_desc[0]) {
41ce84c86d0a04 Li Jun                    2015-07-09  1581  		struct usb_descriptor_header *usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1582  
41ce84c86d0a04 Li Jun                    2015-07-09  1583  		usb_desc = usb_otg_descriptor_alloc(gadget);
41ce84c86d0a04 Li Jun                    2015-07-09  1584  		if (!usb_desc) {
41ce84c86d0a04 Li Jun                    2015-07-09  1585  			ret = -ENOMEM;
41ce84c86d0a04 Li Jun                    2015-07-09  1586  			goto err_comp_cleanup;
41ce84c86d0a04 Li Jun                    2015-07-09  1587  		}
41ce84c86d0a04 Li Jun                    2015-07-09  1588  		usb_otg_descriptor_init(gadget, usb_desc);
41ce84c86d0a04 Li Jun                    2015-07-09  1589  		otg_desc[0] = usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1590  		otg_desc[1] = NULL;
41ce84c86d0a04 Li Jun                    2015-07-09  1591  	}
41ce84c86d0a04 Li Jun                    2015-07-09  1592  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1593  	/* Go through all configs, attach all functions */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1594  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1595  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1596  		struct usb_function *f;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1597  		struct usb_function *tmp;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1598  		struct gadget_config_name *cn;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1599  
41ce84c86d0a04 Li Jun                    2015-07-09  1600  		if (gadget_is_otg(gadget))
41ce84c86d0a04 Li Jun                    2015-07-09  1601  			c->descriptors = otg_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1602  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1603  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1604  		if (!list_empty(&cfg->string_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1605  			i = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1606  			list_for_each_entry(cn, &cfg->string_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1607  				cfg->gstrings[i] = &cn->stringtab_dev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1608  				cn->stringtab_dev.strings = &cn->strings;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1609  				cn->strings.s = cn->configuration;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1610  				i++;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1611  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1612  			cfg->gstrings[i] = NULL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1613  			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
fea77077d1623c Wei Yongjun               2013-05-07  1614  			if (IS_ERR(s)) {
fea77077d1623c Wei Yongjun               2013-05-07  1615  				ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1616  				goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1617  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1618  			c->iConfiguration = s[0].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1619  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1620  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1621  		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1622  			list_del(&f->list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1623  			ret = usb_add_function(c, f);
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1624  			if (ret) {
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1625  				list_add(&f->list, &cfg->func_list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1626  				goto err_purge_funcs;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1627  			}
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1628  		}
7adf9e3adc398e Wesley Cheng              2021-07-10  1629  		ret = usb_gadget_check_config(cdev->gadget);
7adf9e3adc398e Wesley Cheng              2021-07-10  1630  		if (ret)
7adf9e3adc398e Wesley Cheng              2021-07-10  1631  			goto err_purge_funcs;
7adf9e3adc398e Wesley Cheng              2021-07-10  1632  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1633  		usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1634  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1635  	if (cdev->use_os_string) {
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1636  		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1637  		if (ret)
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1638  			goto err_purge_funcs;
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1639  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1640  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1641  	usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1642  	return 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1643  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1644  err_purge_funcs:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1645  	purge_configs_funcs(gi);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1646  err_comp_cleanup:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1647  	composite_dev_cleanup(cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1648  	return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1649  }
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1650  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
@ 2023-01-31  7:35 ` Dan Carpenter
  0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2023-01-31  7:35 UTC (permalink / raw)
  To: oe-kbuild, Daniel Scally, linux-usb, gregkh, laurent.pinchart
  Cc: lkp, oe-kbuild-all, mgr, balbi, kieran.bingham, torleiv, stern,
	Daniel Scally

Hi Daniel,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
patch link:    https://lore.kernel.org/r/20230130093443.25644-8-dan.scally%40ideasonboard.com
patch subject: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230131/202301311446.AqdvqXkI-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

New smatch warnings:
drivers/usb/gadget/configfs.c:1563 configfs_composite_bind() warn: passing zero to 'PTR_ERR'

Old smatch warnings:
drivers/usb/gadget/configfs.c:985 os_desc_b_vendor_code_show() warn: argument 3 to %02x specifier has type 'char'

vim +/PTR_ERR +1563 drivers/usb/gadget/configfs.c

88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1518  static int configfs_composite_bind(struct usb_gadget *gadget,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1519  		struct usb_gadget_driver *gdriver)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1520  {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1521  	struct usb_composite_driver     *composite = to_cdriver(gdriver);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1522  	struct gadget_info		*gi = container_of(composite,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1523  						struct gadget_info, composite);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1524  	struct usb_composite_dev	*cdev = &gi->cdev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1525  	struct usb_configuration	*c;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1526  	struct usb_string		*s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1527  	unsigned			i;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1528  	int				ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1529  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1530  	/* the gi->lock is hold by the caller */
1a1c851bbd706e Peter Chen                2019-08-26  1531  	gi->unbind = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1532  	cdev->gadget = gadget;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1533  	set_gadget_data(gadget, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1534  	ret = composite_dev_prepare(composite, cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1535  	if (ret)
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1536  		return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1537  	/* and now the gadget bind */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1538  	ret = -EINVAL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1539  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1540  	if (list_empty(&gi->cdev.configs)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1541  		pr_err("Need at least one configuration in %s.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1542  				gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1543  		goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1544  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1545  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1546  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1547  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1548  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1549  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1550  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1551  		if (list_empty(&cfg->func_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1552  			pr_err("Config %s/%d of %s needs at least one function.\n",
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1553  			      c->label, c->bConfigurationValue,
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1554  			      gi->composite.name);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1555  			goto err_comp_cleanup;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1556  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1557  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1558  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1559  	/* init all strings */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1560  	if (!list_empty(&gi->string_list)) {
0c252735443756 Daniel Scally             2023-01-30  1561  		s = configfs_attach_gadget_strings(gi);
0c252735443756 Daniel Scally             2023-01-30  1562  		if (IS_ERR_OR_NULL(s)) {


Passing NULL to PTR_ERR(s) is not a bug, but this check has a probably
under 10% false positive rate because 90% of the time when people do
that it is wrong.

In this case configfs_attach_gadget_strings() cannot actually return
NULL so this could just be changed to if (IS_ERR(s)) {.

I would say that probably if it *did* return NULL then we should return
-EPROBE_DEFER.  It's an interesting philosophical debate how to handle
impossible things...

fea77077d1623c Wei Yongjun               2013-05-07 @1563  			ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1564  			goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1565  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1566  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1567  		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1568  		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1569  		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
0c252735443756 Daniel Scally             2023-01-30  1570  
0c252735443756 Daniel Scally             2023-01-30  1571  		gi->cdev.usb_strings = s;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1572  	}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1573  
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1574  	if (gi->use_os_desc) {
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1575  		cdev->use_os_string = true;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1576  		cdev->b_vendor_code = gi->b_vendor_code;
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1577  		memcpy(cdev->qw_sign, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1578  	}
87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1579  
41ce84c86d0a04 Li Jun                    2015-07-09  1580  	if (gadget_is_otg(gadget) && !otg_desc[0]) {
41ce84c86d0a04 Li Jun                    2015-07-09  1581  		struct usb_descriptor_header *usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1582  
41ce84c86d0a04 Li Jun                    2015-07-09  1583  		usb_desc = usb_otg_descriptor_alloc(gadget);
41ce84c86d0a04 Li Jun                    2015-07-09  1584  		if (!usb_desc) {
41ce84c86d0a04 Li Jun                    2015-07-09  1585  			ret = -ENOMEM;
41ce84c86d0a04 Li Jun                    2015-07-09  1586  			goto err_comp_cleanup;
41ce84c86d0a04 Li Jun                    2015-07-09  1587  		}
41ce84c86d0a04 Li Jun                    2015-07-09  1588  		usb_otg_descriptor_init(gadget, usb_desc);
41ce84c86d0a04 Li Jun                    2015-07-09  1589  		otg_desc[0] = usb_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1590  		otg_desc[1] = NULL;
41ce84c86d0a04 Li Jun                    2015-07-09  1591  	}
41ce84c86d0a04 Li Jun                    2015-07-09  1592  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1593  	/* Go through all configs, attach all functions */
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1594  	list_for_each_entry(c, &gi->cdev.configs, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1595  		struct config_usb_cfg *cfg;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1596  		struct usb_function *f;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1597  		struct usb_function *tmp;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1598  		struct gadget_config_name *cn;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1599  
41ce84c86d0a04 Li Jun                    2015-07-09  1600  		if (gadget_is_otg(gadget))
41ce84c86d0a04 Li Jun                    2015-07-09  1601  			c->descriptors = otg_desc;
41ce84c86d0a04 Li Jun                    2015-07-09  1602  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1603  		cfg = container_of(c, struct config_usb_cfg, c);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1604  		if (!list_empty(&cfg->string_list)) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1605  			i = 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1606  			list_for_each_entry(cn, &cfg->string_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1607  				cfg->gstrings[i] = &cn->stringtab_dev;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1608  				cn->stringtab_dev.strings = &cn->strings;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1609  				cn->strings.s = cn->configuration;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1610  				i++;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1611  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1612  			cfg->gstrings[i] = NULL;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1613  			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
fea77077d1623c Wei Yongjun               2013-05-07  1614  			if (IS_ERR(s)) {
fea77077d1623c Wei Yongjun               2013-05-07  1615  				ret = PTR_ERR(s);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1616  				goto err_comp_cleanup;
fea77077d1623c Wei Yongjun               2013-05-07  1617  			}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1618  			c->iConfiguration = s[0].id;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1619  		}
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1620  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1621  		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1622  			list_del(&f->list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1623  			ret = usb_add_function(c, f);
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1624  			if (ret) {
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1625  				list_add(&f->list, &cfg->func_list);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1626  				goto err_purge_funcs;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1627  			}
5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1628  		}
7adf9e3adc398e Wesley Cheng              2021-07-10  1629  		ret = usb_gadget_check_config(cdev->gadget);
7adf9e3adc398e Wesley Cheng              2021-07-10  1630  		if (ret)
7adf9e3adc398e Wesley Cheng              2021-07-10  1631  			goto err_purge_funcs;
7adf9e3adc398e Wesley Cheng              2021-07-10  1632  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1633  		usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1634  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1635  	if (cdev->use_os_string) {
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1636  		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1637  		if (ret)
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1638  			goto err_purge_funcs;
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1639  	}
da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1640  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1641  	usb_ep_autoconfig_reset(cdev->gadget);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1642  	return 0;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1643  
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1644  err_purge_funcs:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1645  	purge_configs_funcs(gi);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1646  err_comp_cleanup:
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1647  	composite_dev_cleanup(cdev);
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1648  	return ret;
88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1649  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
  2023-01-31  7:35 ` Dan Carpenter
  (?)
@ 2023-02-01  9:50 ` Dan Scally
  -1 siblings, 0 replies; 20+ messages in thread
From: Dan Scally @ 2023-02-01  9:50 UTC (permalink / raw)
  To: Dan Carpenter, oe-kbuild, linux-usb, gregkh, laurent.pinchart
  Cc: lkp, oe-kbuild-all, mgr, balbi, kieran.bingham, torleiv, stern

Morning Dan

On 31/01/2023 07:35, Dan Carpenter wrote:
> Hi Daniel,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/usb-gadget-uvc-Make-bSourceID-read-write/20230130-174215
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus
> patch link:    https://lore.kernel.org/r/20230130093443.25644-8-dan.scally%40ideasonboard.com
> patch subject: [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230131/202301311446.AqdvqXkI-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
>
> New smatch warnings:
> drivers/usb/gadget/configfs.c:1563 configfs_composite_bind() warn: passing zero to 'PTR_ERR'
>
> Old smatch warnings:
> drivers/usb/gadget/configfs.c:985 os_desc_b_vendor_code_show() warn: argument 3 to %02x specifier has type 'char'


Really must make time to look at Smatch...

>
> vim +/PTR_ERR +1563 drivers/usb/gadget/configfs.c
>
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1518  static int configfs_composite_bind(struct usb_gadget *gadget,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1519  		struct usb_gadget_driver *gdriver)
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1520  {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1521  	struct usb_composite_driver     *composite = to_cdriver(gdriver);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1522  	struct gadget_info		*gi = container_of(composite,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1523  						struct gadget_info, composite);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1524  	struct usb_composite_dev	*cdev = &gi->cdev;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1525  	struct usb_configuration	*c;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1526  	struct usb_string		*s;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1527  	unsigned			i;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1528  	int				ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1529
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1530  	/* the gi->lock is hold by the caller */
> 1a1c851bbd706e Peter Chen                2019-08-26  1531  	gi->unbind = 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1532  	cdev->gadget = gadget;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1533  	set_gadget_data(gadget, cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1534  	ret = composite_dev_prepare(composite, cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1535  	if (ret)
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1536  		return ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1537  	/* and now the gadget bind */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1538  	ret = -EINVAL;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1539
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1540  	if (list_empty(&gi->cdev.configs)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1541  		pr_err("Need at least one configuration in %s.\n",
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1542  				gi->composite.name);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1543  		goto err_comp_cleanup;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1544  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1545
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1546
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1547  	list_for_each_entry(c, &gi->cdev.configs, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1548  		struct config_usb_cfg *cfg;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1549
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1550  		cfg = container_of(c, struct config_usb_cfg, c);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1551  		if (list_empty(&cfg->func_list)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1552  			pr_err("Config %s/%d of %s needs at least one function.\n",
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1553  			      c->label, c->bConfigurationValue,
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1554  			      gi->composite.name);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1555  			goto err_comp_cleanup;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1556  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1557  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1558
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1559  	/* init all strings */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1560  	if (!list_empty(&gi->string_list)) {
> 0c252735443756 Daniel Scally             2023-01-30  1561  		s = configfs_attach_gadget_strings(gi);
> 0c252735443756 Daniel Scally             2023-01-30  1562  		if (IS_ERR_OR_NULL(s)) {
>
>
> Passing NULL to PTR_ERR(s) is not a bug, but this check has a probably
> under 10% false positive rate because 90% of the time when people do
> that it is wrong.
>
> In this case configfs_attach_gadget_strings() cannot actually return
> NULL so this could just be changed to if (IS_ERR(s)) {.


Yes good spot, thank you

>
> I would say that probably if it *did* return NULL then we should return
> -EPROBE_DEFER.  It's an interesting philosophical debate how to handle
> impossible things...


Well in this case it returns null when there's no languages defined, 
which isn't possible here since it's behind the if(!list_empty(...)) 
check but in principle I think is allowed - usb devices don't have to 
support any string descriptors as far as I know.

> fea77077d1623c Wei Yongjun               2013-05-07 @1563  			ret = PTR_ERR(s);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1564  			goto err_comp_cleanup;
> fea77077d1623c Wei Yongjun               2013-05-07  1565  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1566
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1567  		gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1568  		gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1569  		gi->cdev.desc.iSerialNumber = s[USB_GADGET_SERIAL_IDX].id;
> 0c252735443756 Daniel Scally             2023-01-30  1570
> 0c252735443756 Daniel Scally             2023-01-30  1571  		gi->cdev.usb_strings = s;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1572  	}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1573
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1574  	if (gi->use_os_desc) {
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1575  		cdev->use_os_string = true;
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1576  		cdev->b_vendor_code = gi->b_vendor_code;
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1577  		memcpy(cdev->qw_sign, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1578  	}
> 87213d388e927a Andrzej Pietrasiewicz     2014-05-08  1579
> 41ce84c86d0a04 Li Jun                    2015-07-09  1580  	if (gadget_is_otg(gadget) && !otg_desc[0]) {
> 41ce84c86d0a04 Li Jun                    2015-07-09  1581  		struct usb_descriptor_header *usb_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1582
> 41ce84c86d0a04 Li Jun                    2015-07-09  1583  		usb_desc = usb_otg_descriptor_alloc(gadget);
> 41ce84c86d0a04 Li Jun                    2015-07-09  1584  		if (!usb_desc) {
> 41ce84c86d0a04 Li Jun                    2015-07-09  1585  			ret = -ENOMEM;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1586  			goto err_comp_cleanup;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1587  		}
> 41ce84c86d0a04 Li Jun                    2015-07-09  1588  		usb_otg_descriptor_init(gadget, usb_desc);
> 41ce84c86d0a04 Li Jun                    2015-07-09  1589  		otg_desc[0] = usb_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1590  		otg_desc[1] = NULL;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1591  	}
> 41ce84c86d0a04 Li Jun                    2015-07-09  1592
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1593  	/* Go through all configs, attach all functions */
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1594  	list_for_each_entry(c, &gi->cdev.configs, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1595  		struct config_usb_cfg *cfg;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1596  		struct usb_function *f;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1597  		struct usb_function *tmp;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1598  		struct gadget_config_name *cn;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1599
> 41ce84c86d0a04 Li Jun                    2015-07-09  1600  		if (gadget_is_otg(gadget))
> 41ce84c86d0a04 Li Jun                    2015-07-09  1601  			c->descriptors = otg_desc;
> 41ce84c86d0a04 Li Jun                    2015-07-09  1602
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1603  		cfg = container_of(c, struct config_usb_cfg, c);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1604  		if (!list_empty(&cfg->string_list)) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1605  			i = 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1606  			list_for_each_entry(cn, &cfg->string_list, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1607  				cfg->gstrings[i] = &cn->stringtab_dev;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1608  				cn->stringtab_dev.strings = &cn->strings;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1609  				cn->strings.s = cn->configuration;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1610  				i++;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1611  			}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1612  			cfg->gstrings[i] = NULL;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1613  			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
> fea77077d1623c Wei Yongjun               2013-05-07  1614  			if (IS_ERR(s)) {
> fea77077d1623c Wei Yongjun               2013-05-07  1615  				ret = PTR_ERR(s);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1616  				goto err_comp_cleanup;
> fea77077d1623c Wei Yongjun               2013-05-07  1617  			}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1618  			c->iConfiguration = s[0].id;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1619  		}
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1620
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1621  		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1622  			list_del(&f->list);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1623  			ret = usb_add_function(c, f);
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1624  			if (ret) {
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1625  				list_add(&f->list, &cfg->func_list);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1626  				goto err_purge_funcs;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1627  			}
> 5a68e9b57b1c19 Andrzej Pietrasiewicz     2013-08-08  1628  		}
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1629  		ret = usb_gadget_check_config(cdev->gadget);
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1630  		if (ret)
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1631  			goto err_purge_funcs;
> 7adf9e3adc398e Wesley Cheng              2021-07-10  1632
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1633  		usb_ep_autoconfig_reset(cdev->gadget);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1634  	}
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1635  	if (cdev->use_os_string) {
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1636  		ret = composite_os_desc_req_prepare(cdev, gadget->ep0);
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1637  		if (ret)
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1638  			goto err_purge_funcs;
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1639  	}
> da4243145fb197 Andrzej Pietrasiewicz     2014-05-08  1640
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1641  	usb_ep_autoconfig_reset(cdev->gadget);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1642  	return 0;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1643
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1644  err_purge_funcs:
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1645  	purge_configs_funcs(gi);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1646  err_comp_cleanup:
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1647  	composite_dev_cleanup(cdev);
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1648  	return ret;
> 88af8bbe4ef781 Sebastian Andrzej Siewior 2012-12-23  1649  }
>

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors
  2023-01-30 16:37   ` Alan Stern
@ 2023-02-02 13:06     ` Dan Scally
  0 siblings, 0 replies; 20+ messages in thread
From: Dan Scally @ 2023-02-02 13:06 UTC (permalink / raw)
  To: Alan Stern
  Cc: linux-usb, gregkh, laurent.pinchart, mgr, balbi, kieran.bingham, torleiv

Hi Alan

On 30/01/2023 16:37, Alan Stern wrote:
> On Mon, Jan 30, 2023 at 09:34:38AM +0000, Daniel Scally wrote:
>> Add a framework to allow users to define arbitrary string descriptors
>> for a USB Gadget. This is modelled as a new type of config item rather
>> than as hardcoded attributes so as to be as flexible as possible.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v3:
>>
>> 	- Moved this functionality from the UVC function to usb gadget core.
>>
>> Changes in v2:
>>
>> 	- New patch
>>
>>   drivers/usb/gadget/configfs.c | 172 +++++++++++++++++++++++++++++++++-
>>   include/linux/usb/gadget.h    |  11 +++
>>   2 files changed, 181 insertions(+), 2 deletions(-)
> Shouldn't this patch also include an update to
> Documentation/usb/gadget_configfs.rst?


Yes; I'll update that before resubmitting.

> Alan Stern

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-02-02 13:06 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30  9:34 [PATCH v3 00/11] Add XU support to UVC Gadget Daniel Scally
2023-01-30  9:34 ` [PATCH v3 01/11] usb: gadget: uvc: Make bSourceID read/write Daniel Scally
2023-01-30  9:34 ` [PATCH v3 02/11] usb: gadget: uvc: Generalise helper functions for reuse Daniel Scally
2023-01-30  9:34 ` [PATCH v3 03/11] usb: gadget: uvc: Allow definition of XUs in configfs Daniel Scally
2023-01-30 20:13   ` kernel test robot
2023-01-30  9:34 ` [PATCH v3 04/11] usb: gadget: uvc: Copy XU descriptors during .bind() Daniel Scally
2023-01-30  9:34 ` [PATCH v3 05/11] usb: gadget: configfs: Rename struct gadget_strings Daniel Scally
2023-01-30  9:34 ` [PATCH v3 06/11] usb: gadget: configfs: Support arbitrary string descriptors Daniel Scally
2023-01-30 14:24   ` kernel test robot
2023-01-30 16:07   ` kernel test robot
2023-01-30 16:37   ` Alan Stern
2023-02-02 13:06     ` Dan Scally
2023-01-30  9:34 ` [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev Daniel Scally
2023-01-30  9:34 ` [PATCH v3 08/11] usb: gadget: uvc: Allow linking XUs to string descriptors Daniel Scally
2023-01-30  9:34 ` [PATCH v3 09/11] usb: gadget: uvc: Pick up custom string descriptor IDs Daniel Scally
2023-01-30  9:34 ` [PATCH v3 10/11] usb: gadget: uvc: Allow linking function to string descs Daniel Scally
2023-01-30  9:34 ` [PATCH v3 11/11] usb: gadget: uvc: Use custom strings if available Daniel Scally
2023-01-31  7:09 [PATCH v3 07/11] usb: gadget: configfs: Attach arbitrary strings to cdev kernel test robot
2023-01-31  7:35 ` Dan Carpenter
2023-02-01  9:50 ` Dan Scally

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.