Hi Dan,
Thank you for the patch.
On Mon, Nov 21, 2022 at 09:25:14AM +0000, Daniel Scally wrote:
> Add .allow_link() and .drop_link() callbacks to allow users to link
> an extension unit descriptor to a string descriptor.
A link seems weird to me for this. Wouldn't it be better to store the
name in uvcg_extension in a similar way that device or config strings
are handled in drievrs/usb/gadget/configfs.c ?
> Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> ---
> Changes in v2:
>
> - New patch
>
> drivers/usb/gadget/function/uvc_configfs.c | 60 ++++++++++++++++++++++
> drivers/usb/gadget/function/uvc_configfs.h | 1 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index da2f70036993..5c51862150c4 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -1053,8 +1053,68 @@ 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 *strings;
> + struct uvcg_string *string;
> + struct config_item *opts_item;
> + struct f_uvc_opts *opts;
> + int ret = 0;
> +
> + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> +
> + /* Validate that the target of the link is an entry in strings/<langid> */
> + opts_item = src->ci_parent->ci_parent->ci_parent;
> +
> + strings = config_group_find_item(to_config_group(opts_item), "strings");
> + if (!strings || tgt->ci_parent->ci_parent != strings) {
> + ret = -EINVAL;
> + goto put_strings;
> + }
> +
> + string = to_uvcg_string(tgt);
> + opts = to_f_uvc_opts(opts_item);
> +
> + mutex_lock(&opts->lock);
> +
> + xu->string_descriptor_index = string->usb_string.id;
> +
> + mutex_unlock(&opts->lock);
> +
> +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 a714426a174a..e1308026aed6 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.h
> +++ b/drivers/usb/gadget/function/uvc_configfs.h
> @@ -183,6 +183,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;
> };
>
--
Regards,
Laurent Pinchart
