| Message ID | 20220401160447.5919-1-w36195@motorola.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 324e4f85070f89b58c5f9926370ad19dea907dc7 |
| Headers | show |
| Series | [v2] usb: gadget: uvc: allow changing interface name via configfs | expand |
On Fri, Apr 01, 2022 at 11:04:45AM -0500, Dan Vacura wrote: > Add a configfs entry, "function_name", to change the iInterface field > for VideoControl. This name is used on host devices for user selection, > useful when multiple cameras are present. The default will remain "UVC > Camera". > diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c > index 71bb5e477dba..50e6e7a58b41 100644 > --- a/drivers/usb/gadget/function/f_uvc.c > +++ b/drivers/usb/gadget/function/f_uvc.c > @@ -44,7 +44,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > #define UVC_STRING_STREAMING_IDX 1 > > static struct usb_string uvc_en_us_strings[] = { > - [UVC_STRING_CONTROL_IDX].s = "UVC Camera", > + /* [UVC_STRING_CONTROL_IDX].s = DYNAMIC, */ > [UVC_STRING_STREAMING_IDX].s = "Video Streaming", > { } > }; > @@ -676,6 +676,7 @@ 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; > > + 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)); > if (IS_ERR(us)) { > @@ -866,6 +867,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) > > opts->streaming_interval = 1; > opts->streaming_maxpacket = 1024; > + snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera"); This only allows a single language to be specified. I know that's what the existing string uses, but for other strings which can be set by userspace multiple languages are supported. Should we be making USB_CONFIG_STRINGS_LANG more generic so that it can be used by functions as well as the core configfs code?
On 01/04/22 23.04, Dan Vacura wrote: > Add a configfs entry, "function_name", to change the iInterface field > for VideoControl. This name is used on host devices for user selection, > useful when multiple cameras are present. The default will remain "UVC > Camera". > > Signed-off-by: Dan Vacura <w36195@motorola.com> > --- > Changes in v2: > - remove stable cc > > .../ABI/testing/configfs-usb-gadget-uvc | 1 + > Documentation/usb/gadget-testing.rst | 1 + > drivers/usb/gadget/function/f_uvc.c | 4 +- > drivers/usb/gadget/function/u_uvc.h | 1 + > drivers/usb/gadget/function/uvc_configfs.c | 41 +++++++++++++++++++ > 5 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc > index 889ed45be4ca..611b23e6488d 100644 > --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc > @@ -7,6 +7,7 @@ Description: UVC function directory > streaming_maxburst 0..15 (ss only) > streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss) > streaming_interval 1..16 > + function_name string [32] > =================== ============================= Since you mention that default function_name is "UVC Camera", why don't you mention it in the documentation?
On Sun, Apr 03, 2022 at 01:13:02PM +0700, Bagas Sanjaya wrote: > On 01/04/22 23.04, Dan Vacura wrote: > > Add a configfs entry, "function_name", to change the iInterface field > > for VideoControl. This name is used on host devices for user selection, > > useful when multiple cameras are present. The default will remain "UVC > > Camera". > > > > Signed-off-by: Dan Vacura <w36195@motorola.com> > > --- > > Changes in v2: > > - remove stable cc > > > > .../ABI/testing/configfs-usb-gadget-uvc | 1 + > > Documentation/usb/gadget-testing.rst | 1 + > > drivers/usb/gadget/function/f_uvc.c | 4 +- > > drivers/usb/gadget/function/u_uvc.h | 1 + > > drivers/usb/gadget/function/uvc_configfs.c | 41 +++++++++++++++++++ > > 5 files changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc > > index 889ed45be4ca..611b23e6488d 100644 > > --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc > > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc > > @@ -7,6 +7,7 @@ Description: UVC function directory > > streaming_maxburst 0..15 (ss only) > > streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss) > > streaming_interval 1..16 > > + function_name string [32] > > =================== ============================= > > Since you mention that default function_name is "UVC Camera", why don't you > mention it in the documentation? Good question. The rest of the file didn't contain any default values and I wasn't sure what the process is for keeping things like this in sync between source and documentation. Looking through most of the other usb function files I don't see any mention of the hardcoded default values either, so maybe it's intentional to not include. > > -- > An old man doll... just what I always wanted! - Clara
On Sat, Apr 02, 2022 at 12:24:23PM +0100, John Keeping wrote: > On Fri, Apr 01, 2022 at 11:04:45AM -0500, Dan Vacura wrote: > > Add a configfs entry, "function_name", to change the iInterface field > > for VideoControl. This name is used on host devices for user selection, > > useful when multiple cameras are present. The default will remain "UVC > > Camera". > > > diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c > > index 71bb5e477dba..50e6e7a58b41 100644 > > --- a/drivers/usb/gadget/function/f_uvc.c > > +++ b/drivers/usb/gadget/function/f_uvc.c > > @@ -44,7 +44,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > > #define UVC_STRING_STREAMING_IDX 1 > > > > static struct usb_string uvc_en_us_strings[] = { > > - [UVC_STRING_CONTROL_IDX].s = "UVC Camera", > > + /* [UVC_STRING_CONTROL_IDX].s = DYNAMIC, */ > > [UVC_STRING_STREAMING_IDX].s = "Video Streaming", > > { } > > }; > > @@ -676,6 +676,7 @@ 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; > > > > + 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)); > > if (IS_ERR(us)) { > > @@ -866,6 +867,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) > > > > opts->streaming_interval = 1; > > opts->streaming_maxpacket = 1024; > > + snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera"); > > This only allows a single language to be specified. I know that's what > the existing string uses, but for other strings which can be set by > userspace multiple languages are supported. > > Should we be making USB_CONFIG_STRINGS_LANG more generic so that it can > be used by functions as well as the core configfs code? Agree that adding support for more than one language would be ideal. Looking through the gadget functions, most seem to be hardcoded to en-us locale and don't provide a way to change the exposed names. Recently this was just accepted, which I modeled my change after: https://lore.kernel.org/all/20220122112446.1415547-2-t123yh.xyz@gmail.com/ so at least making USB_CONFIG_STRINGS_LANG more generic would benefit the uac and uvc gadgets.
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 889ed45be4ca..611b23e6488d 100644 --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -7,6 +7,7 @@ Description: UVC function directory streaming_maxburst 0..15 (ss only) streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss) streaming_interval 1..16 + function_name string [32] =================== ============================= What: /config/usb-gadget/gadget/functions/uvc.name/control diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst index c6d034abce3a..1c37159fa171 100644 --- a/Documentation/usb/gadget-testing.rst +++ b/Documentation/usb/gadget-testing.rst @@ -787,6 +787,7 @@ The uvc function provides these attributes in its function directory: streaming_maxpacket maximum packet size this endpoint is capable of sending or receiving when this configuration is selected + function_name name of the interface =================== ================================================ There are also "control" and "streaming" subdirectories, each of which contain diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 71bb5e477dba..50e6e7a58b41 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -44,7 +44,7 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); #define UVC_STRING_STREAMING_IDX 1 static struct usb_string uvc_en_us_strings[] = { - [UVC_STRING_CONTROL_IDX].s = "UVC Camera", + /* [UVC_STRING_CONTROL_IDX].s = DYNAMIC, */ [UVC_STRING_STREAMING_IDX].s = "Video Streaming", { } }; @@ -676,6 +676,7 @@ 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; + 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)); if (IS_ERR(us)) { @@ -866,6 +867,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) opts->streaming_interval = 1; opts->streaming_maxpacket = 1024; + snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera"); ret = uvcg_attach_configfs(opts); if (ret < 0) { diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h index 9a01a7d4f17f..24b8681b0d6f 100644 --- a/drivers/usb/gadget/function/u_uvc.h +++ b/drivers/usb/gadget/function/u_uvc.h @@ -27,6 +27,7 @@ struct f_uvc_opts { unsigned int control_interface; unsigned int streaming_interface; + char function_name[32]; /* * Control descriptors array pointers for full-/high-speed and diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 77d64031aa9c..63b8d3758b38 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -2425,10 +2425,51 @@ UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15); #undef UVCG_OPTS_ATTR +#define UVCG_OPTS_STRING_ATTR(cname, aname) \ +static ssize_t f_uvc_opts_string_##cname##_show(struct config_item *item,\ + char *page) \ +{ \ + struct f_uvc_opts *opts = to_f_uvc_opts(item); \ + int result; \ + \ + mutex_lock(&opts->lock); \ + result = snprintf(page, sizeof(opts->aname), "%s", opts->aname);\ + mutex_unlock(&opts->lock); \ + \ + return result; \ +} \ + \ +static ssize_t f_uvc_opts_string_##cname##_store(struct config_item *item,\ + const char *page, size_t len) \ +{ \ + struct f_uvc_opts *opts = to_f_uvc_opts(item); \ + int ret = 0; \ + \ + mutex_lock(&opts->lock); \ + if (opts->refcnt) { \ + ret = -EBUSY; \ + goto end; \ + } \ + \ + ret = snprintf(opts->aname, min(sizeof(opts->aname), len), \ + "%s", page); \ + \ +end: \ + mutex_unlock(&opts->lock); \ + return ret; \ +} \ + \ +UVC_ATTR(f_uvc_opts_string_, cname, aname) + +UVCG_OPTS_STRING_ATTR(function_name, function_name); + +#undef UVCG_OPTS_STRING_ATTR + static struct configfs_attribute *uvc_attrs[] = { &f_uvc_opts_attr_streaming_interval, &f_uvc_opts_attr_streaming_maxpacket, &f_uvc_opts_attr_streaming_maxburst, + &f_uvc_opts_string_attr_function_name, NULL, };
Add a configfs entry, "function_name", to change the iInterface field for VideoControl. This name is used on host devices for user selection, useful when multiple cameras are present. The default will remain "UVC Camera". Signed-off-by: Dan Vacura <w36195@motorola.com> --- Changes in v2: - remove stable cc .../ABI/testing/configfs-usb-gadget-uvc | 1 + Documentation/usb/gadget-testing.rst | 1 + drivers/usb/gadget/function/f_uvc.c | 4 +- drivers/usb/gadget/function/u_uvc.h | 1 + drivers/usb/gadget/function/uvc_configfs.c | 41 +++++++++++++++++++ 5 files changed, 47 insertions(+), 1 deletion(-)