[PATCH] usb: gadget: f_midi: allow customizing the USB MIDI interface string through configfs

Victor Krawiec posted 1 patch 1 week ago
drivers/usb/gadget/function/f_midi.c | 108 +++++++++++++++------------
drivers/usb/gadget/function/u_midi.h |   8 +-
2 files changed, 66 insertions(+), 50 deletions(-)
[PATCH] usb: gadget: f_midi: allow customizing the USB MIDI interface string through configfs
Posted by Victor Krawiec 1 week ago
When using f_midi from configfs the USB MIDI interface string is hardcoded
to 'MIDI function'.

This USB string descriptor is used by some third-party OS or software to
display the name of the MIDI device

Since we add an additional string option a new macro block was created to
factorize declarations

Signed-off-by: Victor Krawiec <victor.krawiec@arturia.com>
---
 drivers/usb/gadget/function/f_midi.c | 108 +++++++++++++++------------
 drivers/usb/gadget/function/u_midi.h |   8 +-
 2 files changed, 66 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index da82598fcef8..0a8af7d507d9 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -875,6 +875,7 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
 	struct usb_composite_dev *cdev = c->cdev;
 	struct f_midi *midi = func_to_midi(f);
 	struct usb_string *us;
+	struct f_midi_opts *opts;
 	int status, n, jack = 1, i = 0, endpoint_descriptor_index = 0;
 
 	midi->gadget = cdev->gadget;
@@ -883,6 +884,10 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
 	if (status < 0)
 		goto fail_register;
 
+	opts = container_of(f->fi, struct f_midi_opts, func_inst);
+	if (opts->interface_string_allocated && opts->interface_string)
+		midi_string_defs[STRING_FUNC_IDX].s = opts->interface_string;
+
 	/* maybe allocate device-global string ID */
 	us = usb_gstrings_attach(c->cdev, midi_strings,
 				 ARRAY_SIZE(midi_string_defs));
@@ -1178,59 +1183,62 @@ end:									\
 									\
 CONFIGFS_ATTR(f_midi_opts_, name);
 
+#define F_MIDI_OPT_STRING(name)						\
+static ssize_t f_midi_opts_##name##_show(struct config_item *item, char *page) \
+{									\
+	struct f_midi_opts *opts = to_f_midi_opts(item);		\
+	ssize_t result;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->name) {						\
+		result = strscpy(page, opts->name, PAGE_SIZE);		\
+	} else {							\
+		page[0] = 0;						\
+		result = 0;						\
+	}								\
+									\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t f_midi_opts_##name##_store(struct config_item *item,	\
+					 const char *page, size_t len)	\
+{									\
+	struct f_midi_opts *opts = to_f_midi_opts(item);		\
+	int ret;							\
+	char *c;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt > 1) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	c = kstrndup(page, len, GFP_KERNEL);				\
+	if (!c) {							\
+		ret = -ENOMEM;						\
+		goto end;						\
+	}								\
+	if (opts->name##_allocated)					\
+		kfree(opts->name);					\
+	opts->name = c;							\
+	opts->name##_allocated = true;					\
+	ret = len;							\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+CONFIGFS_ATTR(f_midi_opts_, name);
+
 F_MIDI_OPT_SIGNED(index, true, SNDRV_CARDS);
 F_MIDI_OPT(buflen, false, 0);
 F_MIDI_OPT(qlen, false, 0);
 F_MIDI_OPT(in_ports, true, MAX_PORTS);
 F_MIDI_OPT(out_ports, true, MAX_PORTS);
-
-static ssize_t f_midi_opts_id_show(struct config_item *item, char *page)
-{
-	struct f_midi_opts *opts = to_f_midi_opts(item);
-	ssize_t result;
-
-	mutex_lock(&opts->lock);
-	if (opts->id) {
-		result = strscpy(page, opts->id, PAGE_SIZE);
-	} else {
-		page[0] = 0;
-		result = 0;
-	}
-
-	mutex_unlock(&opts->lock);
-
-	return result;
-}
-
-static ssize_t f_midi_opts_id_store(struct config_item *item,
-				    const char *page, size_t len)
-{
-	struct f_midi_opts *opts = to_f_midi_opts(item);
-	int ret;
-	char *c;
-
-	mutex_lock(&opts->lock);
-	if (opts->refcnt > 1) {
-		ret = -EBUSY;
-		goto end;
-	}
-
-	c = kstrndup(page, len, GFP_KERNEL);
-	if (!c) {
-		ret = -ENOMEM;
-		goto end;
-	}
-	if (opts->id_allocated)
-		kfree(opts->id);
-	opts->id = c;
-	opts->id_allocated = true;
-	ret = len;
-end:
-	mutex_unlock(&opts->lock);
-	return ret;
-}
-
-CONFIGFS_ATTR(f_midi_opts_, id);
+F_MIDI_OPT_STRING(id);
+F_MIDI_OPT_STRING(interface_string);
 
 static struct configfs_attribute *midi_attrs[] = {
 	&f_midi_opts_attr_index,
@@ -1239,6 +1247,7 @@ static struct configfs_attribute *midi_attrs[] = {
 	&f_midi_opts_attr_in_ports,
 	&f_midi_opts_attr_out_ports,
 	&f_midi_opts_attr_id,
+	&f_midi_opts_attr_interface_string,
 	NULL,
 };
 
@@ -1264,6 +1273,8 @@ static void f_midi_free_inst(struct usb_function_instance *f)
 	if (free) {
 		if (opts->id_allocated)
 			kfree(opts->id);
+		if (opts->interface_string_allocated)
+			kfree(opts->interface_string);
 		kfree(opts);
 	}
 }
@@ -1280,6 +1291,7 @@ static struct usb_function_instance *f_midi_alloc_inst(void)
 	opts->func_inst.free_func_inst = f_midi_free_inst;
 	opts->index = SNDRV_DEFAULT_IDX1;
 	opts->id = SNDRV_DEFAULT_STR1;
+	opts->interface_string = SNDRV_DEFAULT_STR1;
 	opts->buflen = 512;
 	opts->qlen = 32;
 	opts->in_ports = 1;
diff --git a/drivers/usb/gadget/function/u_midi.h b/drivers/usb/gadget/function/u_midi.h
index 2e400b495cb8..c9c396301a43 100644
--- a/drivers/usb/gadget/function/u_midi.h
+++ b/drivers/usb/gadget/function/u_midi.h
@@ -15,11 +15,15 @@
 
 #include <linux/usb/composite.h>
 
+#define F_MIDI_OPT_STRING_DECLARE(name) \
+	char *name; \
+	bool name##_allocated; \
+
 struct f_midi_opts {
 	struct usb_function_instance	func_inst;
 	int				index;
-	char				*id;
-	bool				id_allocated;
+	F_MIDI_OPT_STRING_DECLARE(id);
+	F_MIDI_OPT_STRING_DECLARE(interface_string);
 	unsigned int			in_ports;
 	unsigned int			out_ports;
 	unsigned int			buflen;

base-commit: f83ec76bf285bea5727f478a68b894f5543ca76e
-- 
2.43.0
Re: [PATCH] usb: gadget: f_midi: allow customizing the USB MIDI interface string through configfs
Posted by Takashi Iwai 4 days, 15 hours ago
On Wed, 24 Sep 2025 17:48:21 +0200,
Victor Krawiec wrote:
> 
> When using f_midi from configfs the USB MIDI interface string is hardcoded
> to 'MIDI function'.
> 
> This USB string descriptor is used by some third-party OS or software to
> display the name of the MIDI device
> 
> Since we add an additional string option a new macro block was created to
> factorize declarations
> 
> Signed-off-by: Victor Krawiec <victor.krawiec@arturia.com>
> ---
>  drivers/usb/gadget/function/f_midi.c | 108 +++++++++++++++------------
>  drivers/usb/gadget/function/u_midi.h |   8 +-
>  2 files changed, 66 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index da82598fcef8..0a8af7d507d9 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -875,6 +875,7 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
>  	struct usb_composite_dev *cdev = c->cdev;
>  	struct f_midi *midi = func_to_midi(f);
>  	struct usb_string *us;
> +	struct f_midi_opts *opts;
>  	int status, n, jack = 1, i = 0, endpoint_descriptor_index = 0;
>  
>  	midi->gadget = cdev->gadget;
> @@ -883,6 +884,10 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f)
>  	if (status < 0)
>  		goto fail_register;
>  
> +	opts = container_of(f->fi, struct f_midi_opts, func_inst);
> +	if (opts->interface_string_allocated && opts->interface_string)
> +		midi_string_defs[STRING_FUNC_IDX].s = opts->interface_string;
> +
>  	/* maybe allocate device-global string ID */
>  	us = usb_gstrings_attach(c->cdev, midi_strings,
>  				 ARRAY_SIZE(midi_string_defs));
> @@ -1178,59 +1183,62 @@ end:									\
>  									\
>  CONFIGFS_ATTR(f_midi_opts_, name);
>  
> +#define F_MIDI_OPT_STRING(name)						\
> +static ssize_t f_midi_opts_##name##_show(struct config_item *item, char *page) \
> +{									\
> +	struct f_midi_opts *opts = to_f_midi_opts(item);		\
> +	ssize_t result;							\
> +									\
> +	mutex_lock(&opts->lock);					\
> +	if (opts->name) {						\
> +		result = strscpy(page, opts->name, PAGE_SIZE);		\
> +	} else {							\
> +		page[0] = 0;						\
> +		result = 0;						\
> +	}								\
> +									\
> +	mutex_unlock(&opts->lock);					\
> +									\
> +	return result;							\
> +}									\
> +									\
> +static ssize_t f_midi_opts_##name##_store(struct config_item *item,	\
> +					 const char *page, size_t len)	\
> +{									\
> +	struct f_midi_opts *opts = to_f_midi_opts(item);		\
> +	int ret;							\
> +	char *c;							\
> +									\
> +	mutex_lock(&opts->lock);					\
> +	if (opts->refcnt > 1) {						\
> +		ret = -EBUSY;						\
> +		goto end;						\
> +	}								\
> +									\
> +	c = kstrndup(page, len, GFP_KERNEL);				\
> +	if (!c) {							\
> +		ret = -ENOMEM;						\
> +		goto end;						\
> +	}								\
> +	if (opts->name##_allocated)					\
> +		kfree(opts->name);					\
> +	opts->name = c;							\
> +	opts->name##_allocated = true;					\
> +	ret = len;							\
> +end:									\
> +	mutex_unlock(&opts->lock);					\
> +	return ret;							\
> +}									\
> +									\
> +CONFIGFS_ATTR(f_midi_opts_, name);
> +
>  F_MIDI_OPT_SIGNED(index, true, SNDRV_CARDS);
>  F_MIDI_OPT(buflen, false, 0);
>  F_MIDI_OPT(qlen, false, 0);
>  F_MIDI_OPT(in_ports, true, MAX_PORTS);
>  F_MIDI_OPT(out_ports, true, MAX_PORTS);
> -
> -static ssize_t f_midi_opts_id_show(struct config_item *item, char *page)
> -{
> -	struct f_midi_opts *opts = to_f_midi_opts(item);
> -	ssize_t result;
> -
> -	mutex_lock(&opts->lock);
> -	if (opts->id) {
> -		result = strscpy(page, opts->id, PAGE_SIZE);
> -	} else {
> -		page[0] = 0;
> -		result = 0;
> -	}
> -
> -	mutex_unlock(&opts->lock);
> -
> -	return result;
> -}
> -
> -static ssize_t f_midi_opts_id_store(struct config_item *item,
> -				    const char *page, size_t len)
> -{
> -	struct f_midi_opts *opts = to_f_midi_opts(item);
> -	int ret;
> -	char *c;
> -
> -	mutex_lock(&opts->lock);
> -	if (opts->refcnt > 1) {
> -		ret = -EBUSY;
> -		goto end;
> -	}
> -
> -	c = kstrndup(page, len, GFP_KERNEL);
> -	if (!c) {
> -		ret = -ENOMEM;
> -		goto end;
> -	}
> -	if (opts->id_allocated)
> -		kfree(opts->id);
> -	opts->id = c;
> -	opts->id_allocated = true;
> -	ret = len;
> -end:
> -	mutex_unlock(&opts->lock);
> -	return ret;
> -}
> -
> -CONFIGFS_ATTR(f_midi_opts_, id);
> +F_MIDI_OPT_STRING(id);
> +F_MIDI_OPT_STRING(interface_string);
>  
>  static struct configfs_attribute *midi_attrs[] = {
>  	&f_midi_opts_attr_index,
> @@ -1239,6 +1247,7 @@ static struct configfs_attribute *midi_attrs[] = {
>  	&f_midi_opts_attr_in_ports,
>  	&f_midi_opts_attr_out_ports,
>  	&f_midi_opts_attr_id,
> +	&f_midi_opts_attr_interface_string,
>  	NULL,
>  };
>  
> @@ -1264,6 +1273,8 @@ static void f_midi_free_inst(struct usb_function_instance *f)
>  	if (free) {
>  		if (opts->id_allocated)
>  			kfree(opts->id);
> +		if (opts->interface_string_allocated)
> +			kfree(opts->interface_string);
>  		kfree(opts);
>  	}
>  }
> @@ -1280,6 +1291,7 @@ static struct usb_function_instance *f_midi_alloc_inst(void)
>  	opts->func_inst.free_func_inst = f_midi_free_inst;
>  	opts->index = SNDRV_DEFAULT_IDX1;
>  	opts->id = SNDRV_DEFAULT_STR1;
> +	opts->interface_string = SNDRV_DEFAULT_STR1;

SNDRV_DEFAULT_STR1 is NULL, and this fact can simplify things a lot.

So, it's better to initialize explicitly with NULL instead of
SNDRV_DEFAULT_STR1, then you can just do NULL-check of the pointer
without the use of *_allocated field (that can be dropped).

Other than that, the code change looks good.


thanks,

Takashi