| Message ID | 20180801002909.2890-4-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: gadget: uvc: Improve configfs support | expand |
Hi Laurent, Thank you for the patch, On 01/08/18 01:29, Laurent Pinchart wrote: > The UVC configfs implementation creates all groups as global static > variables. This prevents creationg of multiple UVC function instances, /creationg/creation/ > as they would all require their own configfs group instances. > > Fix this by allocating all groups dynamically. To avoid duplicating code > around, extend the config_item_type structure with group name and > children, and implement helper functions to create children > automatically for most groups. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'm struggling to see what paths free all of the dynamically created children in this patch. Is this already supported by the config_group framework? I see a reference to config_group_put(&opts->func_inst.group); in one error path - but that's about it. Am I missing something nice and obvious? (or is it already handled by framework code not in this patch) In fact, I can't see how it could be handled by core - as the children are added to a new structure you have created. I'll let you look into this :) > --- > drivers/usb/gadget/function/f_uvc.c | 8 +- > drivers/usb/gadget/function/uvc_configfs.c | 480 +++++++++++++++++------------ > 2 files changed, 282 insertions(+), 206 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c > index d8ce7868fe22..95cb1b5f5ffe 100644 > --- a/drivers/usb/gadget/function/f_uvc.c > +++ b/drivers/usb/gadget/function/f_uvc.c > @@ -792,6 +792,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) > struct uvc_output_terminal_descriptor *od; > struct uvc_color_matching_descriptor *md; > struct uvc_descriptor_header **ctl_cls; > + int ret; > > opts = kzalloc(sizeof(*opts), GFP_KERNEL); > if (!opts) > @@ -868,7 +869,12 @@ static struct usb_function_instance *uvc_alloc_inst(void) > opts->streaming_interval = 1; > opts->streaming_maxpacket = 1024; > > - uvcg_attach_configfs(opts); > + ret = uvcg_attach_configfs(opts); > + if (ret < 0) { > + kfree(opts); > + return ERR_PTR(ret); > + } > + > return &opts->func_inst; > } > > diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c > index dbc95c9558de..e019ea317c7a 100644 > --- a/drivers/usb/gadget/function/uvc_configfs.c > +++ b/drivers/usb/gadget/function/uvc_configfs.c > @@ -41,6 +41,49 @@ static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item) > func_inst.group); > } > > +struct uvcg_config_group_type { > + struct config_item_type type; > + const char *name; > + const struct uvcg_config_group_type **children; > + int (*create_children)(struct config_group *group); > +}; > + > +static int uvcg_config_create_group(struct config_group *parent, > + const struct uvcg_config_group_type *type); > + > +static int uvcg_config_create_children(struct config_group *group, > + const struct uvcg_config_group_type *type) > +{ > + const struct uvcg_config_group_type **child; > + int ret; > + > + if (type->create_children) > + return type->create_children(group); > + > + for (child = type->children; child && *child; ++child) { > + ret = uvcg_config_create_group(group, *child); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +static int uvcg_config_create_group(struct config_group *parent, > + const struct uvcg_config_group_type *type) > +{ > + struct config_group *group; > + > + group = kzalloc(sizeof(*group), GFP_KERNEL); > + if (!group) > + return -ENOMEM; > + > + config_group_init_type_name(group, type->name, &type->type); > + configfs_add_default_group(group, parent); > + > + return uvcg_config_create_children(group, type); > +} > + > /* ----------------------------------------------------------------------------- > * control/header/<NAME> > * control/header > @@ -169,24 +212,23 @@ static void uvcg_control_header_drop(struct config_group *group, > kfree(h); > } > > -static struct config_group uvcg_control_header_grp; > - > static struct configfs_group_operations uvcg_control_header_grp_ops = { > .make_item = uvcg_control_header_make, > .drop_item = uvcg_control_header_drop, > }; > > -static const struct config_item_type uvcg_control_header_grp_type = { > - .ct_group_ops = &uvcg_control_header_grp_ops, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_control_header_grp_type = { > + .type = { > + .ct_group_ops = &uvcg_control_header_grp_ops, > + .ct_owner = THIS_MODULE, > + }, > + .name = "header", > }; > > /* ----------------------------------------------------------------------------- > * control/processing/default > */ > > -static struct config_group uvcg_default_processing_grp; > - > #define UVCG_DEFAULT_PROCESSING_ATTR(cname, aname, conv) \ > static ssize_t uvcg_default_processing_##cname##_show( \ > struct config_item *item, char *page) \ > @@ -265,27 +307,33 @@ static struct configfs_attribute *uvcg_default_processing_attrs[] = { > NULL, > }; > > -static const struct config_item_type uvcg_default_processing_type = { > - .ct_attrs = uvcg_default_processing_attrs, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_default_processing_type = { > + .type = { > + .ct_attrs = uvcg_default_processing_attrs, > + .ct_owner = THIS_MODULE, > + }, > + .name = "default", > }; > > /* ----------------------------------------------------------------------------- > * control/processing > */ > > -static struct config_group uvcg_processing_grp; > - > -static const struct config_item_type uvcg_processing_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_processing_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "processing", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_default_processing_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > * control/terminal/camera/default > */ > > -static struct config_group uvcg_default_camera_grp; > - > #define UVCG_DEFAULT_CAMERA_ATTR(cname, aname, conv) \ > static ssize_t uvcg_default_camera_##cname##_show( \ > struct config_item *item, char *page) \ > @@ -375,27 +423,33 @@ static struct configfs_attribute *uvcg_default_camera_attrs[] = { > NULL, > }; > > -static const struct config_item_type uvcg_default_camera_type = { > - .ct_attrs = uvcg_default_camera_attrs, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_default_camera_type = { > + .type = { > + .ct_attrs = uvcg_default_camera_attrs, > + .ct_owner = THIS_MODULE, > + }, > + .name = "default", > }; > > /* ----------------------------------------------------------------------------- > * control/terminal/camera > */ > > -static struct config_group uvcg_camera_grp; > - > -static const struct config_item_type uvcg_camera_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_camera_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "camera", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_default_camera_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > * control/terminal/output/default > */ > > -static struct config_group uvcg_default_output_grp; > - > #define UVCG_DEFAULT_OUTPUT_ATTR(cname, aname, conv) \ > static ssize_t uvcg_default_output_##cname##_show( \ > struct config_item *item, char *page) \ > @@ -446,47 +500,65 @@ static struct configfs_attribute *uvcg_default_output_attrs[] = { > NULL, > }; > > -static const struct config_item_type uvcg_default_output_type = { > - .ct_attrs = uvcg_default_output_attrs, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_default_output_type = { > + .type = { > + .ct_attrs = uvcg_default_output_attrs, > + .ct_owner = THIS_MODULE, > + }, > + .name = "default", > }; > > /* ----------------------------------------------------------------------------- > * control/terminal/output > */ > > -static struct config_group uvcg_output_grp; > - > -static const struct config_item_type uvcg_output_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_output_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "output", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_default_output_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > * control/terminal > */ > > -static struct config_group uvcg_terminal_grp; > - > -static const struct config_item_type uvcg_terminal_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_terminal_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "terminal", > + .children = (const struct uvcg_config_group_type*[]) { Is this cast really needed? Or is it just to constify the array ? > + &uvcg_camera_grp_type, > + &uvcg_output_grp_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > * control/class/{fs|ss} > */ > > -static struct config_group uvcg_control_class_fs_grp; > -static struct config_group uvcg_control_class_ss_grp; > +struct uvcg_control_class_group { > + struct config_group group; > + const char *name; > +}; > > static inline struct uvc_descriptor_header > **uvcg_get_ctl_class_arr(struct config_item *i, struct f_uvc_opts *o) > { > - struct config_group *group = to_config_group(i); > + struct uvcg_control_class_group *group = > + container_of(i, struct uvcg_control_class_group, > + group.cg_item); > > - if (group == &uvcg_control_class_fs_grp) > + if (!strcmp(group->name, "fs")) > return o->uvc_fs_control_cls; > > - if (group == &uvcg_control_class_ss_grp) > + if (!strcmp(group->name, "ss")) > return o->uvc_ss_control_cls; > > return NULL; > @@ -581,20 +653,52 @@ static const struct config_item_type uvcg_control_class_type = { > * control/class > */ > > -static struct config_group uvcg_control_class_grp; > +static int uvcg_control_class_create_children(struct config_group *parent) > +{ > + static const char * const names[] = { "fs", "ss" }; > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(names); ++i) { > + struct uvcg_control_class_group *group; > + > + group = kzalloc(sizeof(*group), GFP_KERNEL); > + if (!group) > + return -ENOMEM; > + > + group->name = names[i]; > > -static const struct config_item_type uvcg_control_class_grp_type = { > - .ct_owner = THIS_MODULE, > + config_group_init_type_name(&group->group, group->name, > + &uvcg_control_class_type); > + configfs_add_default_group(&group->group, parent); > + } > + > + return 0; > +} > + > +static const struct uvcg_config_group_type uvcg_control_class_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "class", > + .create_children = uvcg_control_class_create_children, > }; > > /* ----------------------------------------------------------------------------- > * control > */ > > -static struct config_group uvcg_control_grp; > - > -static const struct config_item_type uvcg_control_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_control_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "control", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_control_header_grp_type, > + &uvcg_processing_grp_type, > + &uvcg_terminal_grp_type, > + &uvcg_control_class_grp_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > @@ -602,12 +706,9 @@ static const struct config_item_type uvcg_control_grp_type = { > * streaming/mjpeg > */ > > -static struct config_group uvcg_uncompressed_grp; > -static struct config_group uvcg_mjpeg_grp; > - > -static struct config_item *fmt_parent[] = { > - &uvcg_uncompressed_grp.cg_item, > - &uvcg_mjpeg_grp.cg_item, > +static const char * const uvcg_format_names[] = { > + "uncompressed", > + "mjpeg", > }; > > enum uvcg_format_type { > @@ -733,10 +834,22 @@ static int uvcg_streaming_header_allow_link(struct config_item *src, > goto out; > } > > - for (i = 0; i < ARRAY_SIZE(fmt_parent); ++i) > - if (target->ci_parent == fmt_parent[i]) > + /* > + * Linking is only allowed to direct children of the format nodes > + * (streaming/uncompressed or streaming/mjpeg nodes). First check that > + * the grand-parent of the target matches the grand-parent of the source > + * (the streaming node), and then verify that the target parent is a > + * format node. > + */ > + if (src->ci_parent->ci_parent != target->ci_parent->ci_parent) > + goto out; > + > + for (i = 0; i < ARRAY_SIZE(uvcg_format_names); ++i) { > + if (!strcmp(target->ci_parent->ci_name, uvcg_format_names[i])) > break; > - if (i == ARRAY_SIZE(fmt_parent)) > + } > + > + if (i == ARRAY_SIZE(uvcg_format_names)) > goto out; > > target_fmt = container_of(to_config_group(target), struct uvcg_format, > @@ -881,16 +994,17 @@ static void uvcg_streaming_header_drop(struct config_group *group, > kfree(h); > } > > -static struct config_group uvcg_streaming_header_grp; > - > static struct configfs_group_operations uvcg_streaming_header_grp_ops = { > .make_item = uvcg_streaming_header_make, > .drop_item = uvcg_streaming_header_drop, > }; > > -static const struct config_item_type uvcg_streaming_header_grp_type = { > - .ct_group_ops = &uvcg_streaming_header_grp_ops, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_streaming_header_grp_type = { > + .type = { > + .ct_group_ops = &uvcg_streaming_header_grp_ops, > + .ct_owner = THIS_MODULE, > + }, > + .name = "header", > }; > > /* ----------------------------------------------------------------------------- > @@ -1462,9 +1576,12 @@ static struct configfs_group_operations uvcg_uncompressed_grp_ops = { > .drop_item = uvcg_uncompressed_drop, > }; > > -static const struct config_item_type uvcg_uncompressed_grp_type = { > - .ct_group_ops = &uvcg_uncompressed_grp_ops, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_uncompressed_grp_type = { > + .type = { > + .ct_group_ops = &uvcg_uncompressed_grp_ops, > + .ct_owner = THIS_MODULE, > + }, > + .name = "uncompressed", > }; > > /* ----------------------------------------------------------------------------- > @@ -1659,17 +1776,18 @@ static struct configfs_group_operations uvcg_mjpeg_grp_ops = { > .drop_item = uvcg_mjpeg_drop, > }; > > -static const struct config_item_type uvcg_mjpeg_grp_type = { > - .ct_group_ops = &uvcg_mjpeg_grp_ops, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_mjpeg_grp_type = { > + .type = { > + .ct_group_ops = &uvcg_mjpeg_grp_ops, > + .ct_owner = THIS_MODULE, > + }, > + .name = "mjpeg", > }; > > /* ----------------------------------------------------------------------------- > * streaming/color_matching/default > */ > > -static struct config_group uvcg_default_color_matching_grp; > - > #define UVCG_DEFAULT_COLOR_MATCHING_ATTR(cname, aname, conv) \ > static ssize_t uvcg_default_color_matching_##cname##_show( \ > struct config_item *item, char *page) \ > @@ -1717,41 +1835,52 @@ static struct configfs_attribute *uvcg_default_color_matching_attrs[] = { > NULL, > }; > > -static const struct config_item_type uvcg_default_color_matching_type = { > - .ct_attrs = uvcg_default_color_matching_attrs, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_default_color_matching_type = { > + .type = { > + .ct_attrs = uvcg_default_color_matching_attrs, > + .ct_owner = THIS_MODULE, > + }, > + .name = "default", > }; > > /* ----------------------------------------------------------------------------- > * streaming/color_matching > */ > > -static struct config_group uvcg_color_matching_grp; > - > -static const struct config_item_type uvcg_color_matching_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_color_matching_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "color_matching", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_default_color_matching_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > * streaming/class/{fs|hs|ss} > */ > > -static struct config_group uvcg_streaming_class_fs_grp; > -static struct config_group uvcg_streaming_class_hs_grp; > -static struct config_group uvcg_streaming_class_ss_grp; > +struct uvcg_streaming_class_group { > + struct config_group group; > + const char *name; > +}; > > static inline struct uvc_descriptor_header > ***__uvcg_get_stream_class_arr(struct config_item *i, struct f_uvc_opts *o) > { > - struct config_group *group = to_config_group(i); > + struct uvcg_streaming_class_group *group = > + container_of(i, struct uvcg_streaming_class_group, > + group.cg_item); > > - if (group == &uvcg_streaming_class_fs_grp) > + if (!strcmp(group->name, "fs")) > return &o->uvc_fs_streaming_cls; > > - if (group == &uvcg_streaming_class_hs_grp) > + if (!strcmp(group->name, "hs")) > return &o->uvc_hs_streaming_cls; > > - if (group == &uvcg_streaming_class_ss_grp) > + if (!strcmp(group->name, "ss")) > return &o->uvc_ss_streaming_cls; > > return NULL; > @@ -2083,20 +2212,53 @@ static const struct config_item_type uvcg_streaming_class_type = { > * streaming/class > */ > > -static struct config_group uvcg_streaming_class_grp; > +static int uvcg_streaming_class_create_children(struct config_group *parent) > +{ > + static const char * const names[] = { "fs", "hs", "ss" }; > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(names); ++i) { > + struct uvcg_streaming_class_group *group; > + > + group = kzalloc(sizeof(*group), GFP_KERNEL); > + if (!group) > + return -ENOMEM; > + > + group->name = names[i]; > + > + config_group_init_type_name(&group->group, group->name, > + &uvcg_streaming_class_type); > + configfs_add_default_group(&group->group, parent); > + } > + > + return 0; > +} > > -static const struct config_item_type uvcg_streaming_class_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_streaming_class_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "class", > + .create_children = uvcg_streaming_class_create_children, > }; > > /* ----------------------------------------------------------------------------- > * streaming > */ > > -static struct config_group uvcg_streaming_grp; > - > -static const struct config_item_type uvcg_streaming_grp_type = { > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvcg_streaming_grp_type = { > + .type = { > + .ct_owner = THIS_MODULE, > + }, > + .name = "streaming", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_streaming_header_grp_type, > + &uvcg_uncompressed_grp_type, > + &uvcg_mjpeg_grp_type, > + &uvcg_color_matching_grp_type, > + &uvcg_streaming_class_grp_type, > + NULL, > + }, > }; > > /* ----------------------------------------------------------------------------- > @@ -2179,123 +2341,31 @@ static struct configfs_attribute *uvc_attrs[] = { > NULL, > }; > > -static const struct config_item_type uvc_func_type = { > - .ct_item_ops = &uvc_item_ops, > - .ct_attrs = uvc_attrs, > - .ct_owner = THIS_MODULE, > +static const struct uvcg_config_group_type uvc_func_type = { > + .type = { > + .ct_item_ops = &uvc_item_ops, > + .ct_attrs = uvc_attrs, > + .ct_owner = THIS_MODULE, > + }, > + .name = "", > + .children = (const struct uvcg_config_group_type*[]) { > + &uvcg_control_grp_type, > + &uvcg_streaming_grp_type, > + NULL, > + }, > }; > > int uvcg_attach_configfs(struct f_uvc_opts *opts) > { > - config_group_init_type_name(&uvcg_control_header_grp, > - "header", > - &uvcg_control_header_grp_type); > - > - config_group_init_type_name(&uvcg_default_processing_grp, > - "default", &uvcg_default_processing_type); > - config_group_init_type_name(&uvcg_processing_grp, > - "processing", &uvcg_processing_grp_type); > - configfs_add_default_group(&uvcg_default_processing_grp, > - &uvcg_processing_grp); > - > - config_group_init_type_name(&uvcg_default_camera_grp, > - "default", &uvcg_default_camera_type); > - config_group_init_type_name(&uvcg_camera_grp, > - "camera", &uvcg_camera_grp_type); > - configfs_add_default_group(&uvcg_default_camera_grp, > - &uvcg_camera_grp); > - > - config_group_init_type_name(&uvcg_default_output_grp, > - "default", &uvcg_default_output_type); > - config_group_init_type_name(&uvcg_output_grp, > - "output", &uvcg_output_grp_type); > - configfs_add_default_group(&uvcg_default_output_grp, > - &uvcg_output_grp); > - > - config_group_init_type_name(&uvcg_terminal_grp, > - "terminal", &uvcg_terminal_grp_type); > - configfs_add_default_group(&uvcg_camera_grp, > - &uvcg_terminal_grp); > - configfs_add_default_group(&uvcg_output_grp, > - &uvcg_terminal_grp); > - > - config_group_init_type_name(&uvcg_control_class_fs_grp, > - "fs", &uvcg_control_class_type); > - config_group_init_type_name(&uvcg_control_class_ss_grp, > - "ss", &uvcg_control_class_type); > - config_group_init_type_name(&uvcg_control_class_grp, > - "class", > - &uvcg_control_class_grp_type); > - configfs_add_default_group(&uvcg_control_class_fs_grp, > - &uvcg_control_class_grp); > - configfs_add_default_group(&uvcg_control_class_ss_grp, > - &uvcg_control_class_grp); > - > - config_group_init_type_name(&uvcg_control_grp, > - "control", > - &uvcg_control_grp_type); > - configfs_add_default_group(&uvcg_control_header_grp, > - &uvcg_control_grp); > - configfs_add_default_group(&uvcg_processing_grp, > - &uvcg_control_grp); > - configfs_add_default_group(&uvcg_terminal_grp, > - &uvcg_control_grp); > - configfs_add_default_group(&uvcg_control_class_grp, > - &uvcg_control_grp); > - > - config_group_init_type_name(&uvcg_streaming_header_grp, > - "header", > - &uvcg_streaming_header_grp_type); > - config_group_init_type_name(&uvcg_uncompressed_grp, > - "uncompressed", > - &uvcg_uncompressed_grp_type); > - config_group_init_type_name(&uvcg_mjpeg_grp, > - "mjpeg", > - &uvcg_mjpeg_grp_type); > - config_group_init_type_name(&uvcg_default_color_matching_grp, > - "default", > - &uvcg_default_color_matching_type); > - config_group_init_type_name(&uvcg_color_matching_grp, > - "color_matching", > - &uvcg_color_matching_grp_type); > - configfs_add_default_group(&uvcg_default_color_matching_grp, > - &uvcg_color_matching_grp); > - > - config_group_init_type_name(&uvcg_streaming_class_fs_grp, > - "fs", &uvcg_streaming_class_type); > - config_group_init_type_name(&uvcg_streaming_class_hs_grp, > - "hs", &uvcg_streaming_class_type); > - config_group_init_type_name(&uvcg_streaming_class_ss_grp, > - "ss", &uvcg_streaming_class_type); > - config_group_init_type_name(&uvcg_streaming_class_grp, > - "class", &uvcg_streaming_class_grp_type); > - configfs_add_default_group(&uvcg_streaming_class_fs_grp, > - &uvcg_streaming_class_grp); > - configfs_add_default_group(&uvcg_streaming_class_hs_grp, > - &uvcg_streaming_class_grp); > - configfs_add_default_group(&uvcg_streaming_class_ss_grp, > - &uvcg_streaming_class_grp); > - > - config_group_init_type_name(&uvcg_streaming_grp, > - "streaming", &uvcg_streaming_grp_type); > - configfs_add_default_group(&uvcg_streaming_header_grp, > - &uvcg_streaming_grp); > - configfs_add_default_group(&uvcg_uncompressed_grp, > - &uvcg_streaming_grp); > - configfs_add_default_group(&uvcg_mjpeg_grp, > - &uvcg_streaming_grp); > - configfs_add_default_group(&uvcg_color_matching_grp, > - &uvcg_streaming_grp); > - configfs_add_default_group(&uvcg_streaming_class_grp, > - &uvcg_streaming_grp); > - > - config_group_init_type_name(&opts->func_inst.group, > - "", > - &uvc_func_type); > - configfs_add_default_group(&uvcg_control_grp, > - &opts->func_inst.group); > - configfs_add_default_group(&uvcg_streaming_grp, > - &opts->func_inst.group); > + int ret; > > - return 0; > + config_group_init_type_name(&opts->func_inst.group, uvc_func_type.name, > + &uvc_func_type.type); > + > + ret = uvcg_config_create_children(&opts->func_inst.group, > + &uvc_func_type); > + if (ret < 0) > + config_group_put(&opts->func_inst.group); > + > + return ret; > } >
Hi Kieran, On Wednesday, 1 August 2018 12:58:40 EEST Kieran Bingham wrote: > On 01/08/18 01:29, Laurent Pinchart wrote: > > The UVC configfs implementation creates all groups as global static > > variables. This prevents creationg of multiple UVC function instances, > > /creationg/creation/ > > > as they would all require their own configfs group instances. > > > > Fix this by allocating all groups dynamically. To avoid duplicating code > > around, extend the config_item_type structure with group name and > > children, and implement helper functions to create children > > automatically for most groups. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'm struggling to see what paths free all of the dynamically created > children in this patch. > > Is this already supported by the config_group framework? > > I see a reference to config_group_put(&opts->func_inst.group); in one > error path - but that's about it. > > Am I missing something nice and obvious? (or is it already handled by > framework code not in this patch) > > > In fact, I can't see how it could be handled by core - as the children > are added to a new structure you have created. > > I'll let you look into this :) I did, for a whole day :-S It wasn't easy as the configfs documentation is quite terse, and doesn't clearly explain when and how references should be acquired and released. I'll post a v2 shortly. > > --- > > > > drivers/usb/gadget/function/f_uvc.c | 8 +- > > drivers/usb/gadget/function/uvc_configfs.c | 480 ++++++++++++++---------- > > 2 files changed, 282 insertions(+), 206 deletions(-) [snip] > > diff --git a/drivers/usb/gadget/function/uvc_configfs.c > > b/drivers/usb/gadget/function/uvc_configfs.c index > > dbc95c9558de..e019ea317c7a 100644 > > --- a/drivers/usb/gadget/function/uvc_configfs.c > > +++ b/drivers/usb/gadget/function/uvc_configfs.c [snip] > > -static struct config_group uvcg_terminal_grp; > > - > > -static const struct config_item_type uvcg_terminal_grp_type = { > > - .ct_owner = THIS_MODULE, > > +static const struct uvcg_config_group_type uvcg_terminal_grp_type = { > > + .type = { > > + .ct_owner = THIS_MODULE, > > + }, > > + .name = "terminal", > > + .children = (const struct uvcg_config_group_type*[]) { > > Is this cast really needed? Or is it just to constify the array ? It's needed to make the expression within the curly braces an array that is then turned into a pointer to initialize the children field, which is defined as const struct uvcg_config_group_type **children; Without that you would get the following compilation errors. drivers/usb/gadget/function/uvc_configfs.c:557:2: error: braces around scalar initializer [-Werror] drivers/usb/gadget/function/uvc_configfs.c:557:2: error: (near initialization for ‘uvcg_terminal_grp_type.children’) [-Werror] drivers/usb/gadget/function/uvc_configfs.c:558:3: error: initialization from incompatible pointer type [-Werror] drivers/usb/gadget/function/uvc_configfs.c:558:3: error: (near initialization for ‘uvcg_terminal_grp_type.children’) [-Werror] drivers/usb/gadget/function/uvc_configfs.c:559:3: error: excess elements in scalar initializer [-Werror] drivers/usb/gadget/function/uvc_configfs.c:559:3: error: (near initialization for ‘uvcg_terminal_grp_type.children’) [-Werror] drivers/usb/gadget/function/uvc_configfs.c:560:3: error: excess elements in scalar initializer [-Werror] drivers/usb/gadget/function/uvc_configfs.c:560:3: error: (near initialization for ‘uvcg_terminal_grp_type.children’) [-Werror] Such a syntax removes the need to declare the array externally to uvcg_terminal_grp_type as static const struct uvcg_config_group_type *uvcg_terminal_grp_children[] = { &uvcg_camera_grp_type, &uvcg_output_grp_type, NULL, }; static const struct uvcg_config_group_type uvcg_terminal_grp_type = { .type = { .ct_owner = THIS_MODULE, }, .name = "terminal", .children = uvcg_terminal_grp_children, }; > > + &uvcg_camera_grp_type, > > + &uvcg_output_grp_type, > > + NULL, > > + }, > > }; [snip]
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index d8ce7868fe22..95cb1b5f5ffe 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -792,6 +792,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) struct uvc_output_terminal_descriptor *od; struct uvc_color_matching_descriptor *md; struct uvc_descriptor_header **ctl_cls; + int ret; opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) @@ -868,7 +869,12 @@ static struct usb_function_instance *uvc_alloc_inst(void) opts->streaming_interval = 1; opts->streaming_maxpacket = 1024; - uvcg_attach_configfs(opts); + ret = uvcg_attach_configfs(opts); + if (ret < 0) { + kfree(opts); + return ERR_PTR(ret); + } + return &opts->func_inst; } diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index dbc95c9558de..e019ea317c7a 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -41,6 +41,49 @@ static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item) func_inst.group); } +struct uvcg_config_group_type { + struct config_item_type type; + const char *name; + const struct uvcg_config_group_type **children; + int (*create_children)(struct config_group *group); +}; + +static int uvcg_config_create_group(struct config_group *parent, + const struct uvcg_config_group_type *type); + +static int uvcg_config_create_children(struct config_group *group, + const struct uvcg_config_group_type *type) +{ + const struct uvcg_config_group_type **child; + int ret; + + if (type->create_children) + return type->create_children(group); + + for (child = type->children; child && *child; ++child) { + ret = uvcg_config_create_group(group, *child); + if (ret < 0) + return ret; + } + + return 0; +} + +static int uvcg_config_create_group(struct config_group *parent, + const struct uvcg_config_group_type *type) +{ + struct config_group *group; + + group = kzalloc(sizeof(*group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + config_group_init_type_name(group, type->name, &type->type); + configfs_add_default_group(group, parent); + + return uvcg_config_create_children(group, type); +} + /* ----------------------------------------------------------------------------- * control/header/<NAME> * control/header @@ -169,24 +212,23 @@ static void uvcg_control_header_drop(struct config_group *group, kfree(h); } -static struct config_group uvcg_control_header_grp; - static struct configfs_group_operations uvcg_control_header_grp_ops = { .make_item = uvcg_control_header_make, .drop_item = uvcg_control_header_drop, }; -static const struct config_item_type uvcg_control_header_grp_type = { - .ct_group_ops = &uvcg_control_header_grp_ops, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_control_header_grp_type = { + .type = { + .ct_group_ops = &uvcg_control_header_grp_ops, + .ct_owner = THIS_MODULE, + }, + .name = "header", }; /* ----------------------------------------------------------------------------- * control/processing/default */ -static struct config_group uvcg_default_processing_grp; - #define UVCG_DEFAULT_PROCESSING_ATTR(cname, aname, conv) \ static ssize_t uvcg_default_processing_##cname##_show( \ struct config_item *item, char *page) \ @@ -265,27 +307,33 @@ static struct configfs_attribute *uvcg_default_processing_attrs[] = { NULL, }; -static const struct config_item_type uvcg_default_processing_type = { - .ct_attrs = uvcg_default_processing_attrs, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_default_processing_type = { + .type = { + .ct_attrs = uvcg_default_processing_attrs, + .ct_owner = THIS_MODULE, + }, + .name = "default", }; /* ----------------------------------------------------------------------------- * control/processing */ -static struct config_group uvcg_processing_grp; - -static const struct config_item_type uvcg_processing_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_processing_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "processing", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_default_processing_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- * control/terminal/camera/default */ -static struct config_group uvcg_default_camera_grp; - #define UVCG_DEFAULT_CAMERA_ATTR(cname, aname, conv) \ static ssize_t uvcg_default_camera_##cname##_show( \ struct config_item *item, char *page) \ @@ -375,27 +423,33 @@ static struct configfs_attribute *uvcg_default_camera_attrs[] = { NULL, }; -static const struct config_item_type uvcg_default_camera_type = { - .ct_attrs = uvcg_default_camera_attrs, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_default_camera_type = { + .type = { + .ct_attrs = uvcg_default_camera_attrs, + .ct_owner = THIS_MODULE, + }, + .name = "default", }; /* ----------------------------------------------------------------------------- * control/terminal/camera */ -static struct config_group uvcg_camera_grp; - -static const struct config_item_type uvcg_camera_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_camera_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "camera", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_default_camera_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- * control/terminal/output/default */ -static struct config_group uvcg_default_output_grp; - #define UVCG_DEFAULT_OUTPUT_ATTR(cname, aname, conv) \ static ssize_t uvcg_default_output_##cname##_show( \ struct config_item *item, char *page) \ @@ -446,47 +500,65 @@ static struct configfs_attribute *uvcg_default_output_attrs[] = { NULL, }; -static const struct config_item_type uvcg_default_output_type = { - .ct_attrs = uvcg_default_output_attrs, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_default_output_type = { + .type = { + .ct_attrs = uvcg_default_output_attrs, + .ct_owner = THIS_MODULE, + }, + .name = "default", }; /* ----------------------------------------------------------------------------- * control/terminal/output */ -static struct config_group uvcg_output_grp; - -static const struct config_item_type uvcg_output_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_output_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "output", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_default_output_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- * control/terminal */ -static struct config_group uvcg_terminal_grp; - -static const struct config_item_type uvcg_terminal_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_terminal_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "terminal", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_camera_grp_type, + &uvcg_output_grp_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- * control/class/{fs|ss} */ -static struct config_group uvcg_control_class_fs_grp; -static struct config_group uvcg_control_class_ss_grp; +struct uvcg_control_class_group { + struct config_group group; + const char *name; +}; static inline struct uvc_descriptor_header **uvcg_get_ctl_class_arr(struct config_item *i, struct f_uvc_opts *o) { - struct config_group *group = to_config_group(i); + struct uvcg_control_class_group *group = + container_of(i, struct uvcg_control_class_group, + group.cg_item); - if (group == &uvcg_control_class_fs_grp) + if (!strcmp(group->name, "fs")) return o->uvc_fs_control_cls; - if (group == &uvcg_control_class_ss_grp) + if (!strcmp(group->name, "ss")) return o->uvc_ss_control_cls; return NULL; @@ -581,20 +653,52 @@ static const struct config_item_type uvcg_control_class_type = { * control/class */ -static struct config_group uvcg_control_class_grp; +static int uvcg_control_class_create_children(struct config_group *parent) +{ + static const char * const names[] = { "fs", "ss" }; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(names); ++i) { + struct uvcg_control_class_group *group; + + group = kzalloc(sizeof(*group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + group->name = names[i]; -static const struct config_item_type uvcg_control_class_grp_type = { - .ct_owner = THIS_MODULE, + config_group_init_type_name(&group->group, group->name, + &uvcg_control_class_type); + configfs_add_default_group(&group->group, parent); + } + + return 0; +} + +static const struct uvcg_config_group_type uvcg_control_class_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "class", + .create_children = uvcg_control_class_create_children, }; /* ----------------------------------------------------------------------------- * control */ -static struct config_group uvcg_control_grp; - -static const struct config_item_type uvcg_control_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_control_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "control", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_control_header_grp_type, + &uvcg_processing_grp_type, + &uvcg_terminal_grp_type, + &uvcg_control_class_grp_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- @@ -602,12 +706,9 @@ static const struct config_item_type uvcg_control_grp_type = { * streaming/mjpeg */ -static struct config_group uvcg_uncompressed_grp; -static struct config_group uvcg_mjpeg_grp; - -static struct config_item *fmt_parent[] = { - &uvcg_uncompressed_grp.cg_item, - &uvcg_mjpeg_grp.cg_item, +static const char * const uvcg_format_names[] = { + "uncompressed", + "mjpeg", }; enum uvcg_format_type { @@ -733,10 +834,22 @@ static int uvcg_streaming_header_allow_link(struct config_item *src, goto out; } - for (i = 0; i < ARRAY_SIZE(fmt_parent); ++i) - if (target->ci_parent == fmt_parent[i]) + /* + * Linking is only allowed to direct children of the format nodes + * (streaming/uncompressed or streaming/mjpeg nodes). First check that + * the grand-parent of the target matches the grand-parent of the source + * (the streaming node), and then verify that the target parent is a + * format node. + */ + if (src->ci_parent->ci_parent != target->ci_parent->ci_parent) + goto out; + + for (i = 0; i < ARRAY_SIZE(uvcg_format_names); ++i) { + if (!strcmp(target->ci_parent->ci_name, uvcg_format_names[i])) break; - if (i == ARRAY_SIZE(fmt_parent)) + } + + if (i == ARRAY_SIZE(uvcg_format_names)) goto out; target_fmt = container_of(to_config_group(target), struct uvcg_format, @@ -881,16 +994,17 @@ static void uvcg_streaming_header_drop(struct config_group *group, kfree(h); } -static struct config_group uvcg_streaming_header_grp; - static struct configfs_group_operations uvcg_streaming_header_grp_ops = { .make_item = uvcg_streaming_header_make, .drop_item = uvcg_streaming_header_drop, }; -static const struct config_item_type uvcg_streaming_header_grp_type = { - .ct_group_ops = &uvcg_streaming_header_grp_ops, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_streaming_header_grp_type = { + .type = { + .ct_group_ops = &uvcg_streaming_header_grp_ops, + .ct_owner = THIS_MODULE, + }, + .name = "header", }; /* ----------------------------------------------------------------------------- @@ -1462,9 +1576,12 @@ static struct configfs_group_operations uvcg_uncompressed_grp_ops = { .drop_item = uvcg_uncompressed_drop, }; -static const struct config_item_type uvcg_uncompressed_grp_type = { - .ct_group_ops = &uvcg_uncompressed_grp_ops, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_uncompressed_grp_type = { + .type = { + .ct_group_ops = &uvcg_uncompressed_grp_ops, + .ct_owner = THIS_MODULE, + }, + .name = "uncompressed", }; /* ----------------------------------------------------------------------------- @@ -1659,17 +1776,18 @@ static struct configfs_group_operations uvcg_mjpeg_grp_ops = { .drop_item = uvcg_mjpeg_drop, }; -static const struct config_item_type uvcg_mjpeg_grp_type = { - .ct_group_ops = &uvcg_mjpeg_grp_ops, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_mjpeg_grp_type = { + .type = { + .ct_group_ops = &uvcg_mjpeg_grp_ops, + .ct_owner = THIS_MODULE, + }, + .name = "mjpeg", }; /* ----------------------------------------------------------------------------- * streaming/color_matching/default */ -static struct config_group uvcg_default_color_matching_grp; - #define UVCG_DEFAULT_COLOR_MATCHING_ATTR(cname, aname, conv) \ static ssize_t uvcg_default_color_matching_##cname##_show( \ struct config_item *item, char *page) \ @@ -1717,41 +1835,52 @@ static struct configfs_attribute *uvcg_default_color_matching_attrs[] = { NULL, }; -static const struct config_item_type uvcg_default_color_matching_type = { - .ct_attrs = uvcg_default_color_matching_attrs, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_default_color_matching_type = { + .type = { + .ct_attrs = uvcg_default_color_matching_attrs, + .ct_owner = THIS_MODULE, + }, + .name = "default", }; /* ----------------------------------------------------------------------------- * streaming/color_matching */ -static struct config_group uvcg_color_matching_grp; - -static const struct config_item_type uvcg_color_matching_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_color_matching_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "color_matching", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_default_color_matching_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- * streaming/class/{fs|hs|ss} */ -static struct config_group uvcg_streaming_class_fs_grp; -static struct config_group uvcg_streaming_class_hs_grp; -static struct config_group uvcg_streaming_class_ss_grp; +struct uvcg_streaming_class_group { + struct config_group group; + const char *name; +}; static inline struct uvc_descriptor_header ***__uvcg_get_stream_class_arr(struct config_item *i, struct f_uvc_opts *o) { - struct config_group *group = to_config_group(i); + struct uvcg_streaming_class_group *group = + container_of(i, struct uvcg_streaming_class_group, + group.cg_item); - if (group == &uvcg_streaming_class_fs_grp) + if (!strcmp(group->name, "fs")) return &o->uvc_fs_streaming_cls; - if (group == &uvcg_streaming_class_hs_grp) + if (!strcmp(group->name, "hs")) return &o->uvc_hs_streaming_cls; - if (group == &uvcg_streaming_class_ss_grp) + if (!strcmp(group->name, "ss")) return &o->uvc_ss_streaming_cls; return NULL; @@ -2083,20 +2212,53 @@ static const struct config_item_type uvcg_streaming_class_type = { * streaming/class */ -static struct config_group uvcg_streaming_class_grp; +static int uvcg_streaming_class_create_children(struct config_group *parent) +{ + static const char * const names[] = { "fs", "hs", "ss" }; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(names); ++i) { + struct uvcg_streaming_class_group *group; + + group = kzalloc(sizeof(*group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + group->name = names[i]; + + config_group_init_type_name(&group->group, group->name, + &uvcg_streaming_class_type); + configfs_add_default_group(&group->group, parent); + } + + return 0; +} -static const struct config_item_type uvcg_streaming_class_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_streaming_class_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "class", + .create_children = uvcg_streaming_class_create_children, }; /* ----------------------------------------------------------------------------- * streaming */ -static struct config_group uvcg_streaming_grp; - -static const struct config_item_type uvcg_streaming_grp_type = { - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvcg_streaming_grp_type = { + .type = { + .ct_owner = THIS_MODULE, + }, + .name = "streaming", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_streaming_header_grp_type, + &uvcg_uncompressed_grp_type, + &uvcg_mjpeg_grp_type, + &uvcg_color_matching_grp_type, + &uvcg_streaming_class_grp_type, + NULL, + }, }; /* ----------------------------------------------------------------------------- @@ -2179,123 +2341,31 @@ static struct configfs_attribute *uvc_attrs[] = { NULL, }; -static const struct config_item_type uvc_func_type = { - .ct_item_ops = &uvc_item_ops, - .ct_attrs = uvc_attrs, - .ct_owner = THIS_MODULE, +static const struct uvcg_config_group_type uvc_func_type = { + .type = { + .ct_item_ops = &uvc_item_ops, + .ct_attrs = uvc_attrs, + .ct_owner = THIS_MODULE, + }, + .name = "", + .children = (const struct uvcg_config_group_type*[]) { + &uvcg_control_grp_type, + &uvcg_streaming_grp_type, + NULL, + }, }; int uvcg_attach_configfs(struct f_uvc_opts *opts) { - config_group_init_type_name(&uvcg_control_header_grp, - "header", - &uvcg_control_header_grp_type); - - config_group_init_type_name(&uvcg_default_processing_grp, - "default", &uvcg_default_processing_type); - config_group_init_type_name(&uvcg_processing_grp, - "processing", &uvcg_processing_grp_type); - configfs_add_default_group(&uvcg_default_processing_grp, - &uvcg_processing_grp); - - config_group_init_type_name(&uvcg_default_camera_grp, - "default", &uvcg_default_camera_type); - config_group_init_type_name(&uvcg_camera_grp, - "camera", &uvcg_camera_grp_type); - configfs_add_default_group(&uvcg_default_camera_grp, - &uvcg_camera_grp); - - config_group_init_type_name(&uvcg_default_output_grp, - "default", &uvcg_default_output_type); - config_group_init_type_name(&uvcg_output_grp, - "output", &uvcg_output_grp_type); - configfs_add_default_group(&uvcg_default_output_grp, - &uvcg_output_grp); - - config_group_init_type_name(&uvcg_terminal_grp, - "terminal", &uvcg_terminal_grp_type); - configfs_add_default_group(&uvcg_camera_grp, - &uvcg_terminal_grp); - configfs_add_default_group(&uvcg_output_grp, - &uvcg_terminal_grp); - - config_group_init_type_name(&uvcg_control_class_fs_grp, - "fs", &uvcg_control_class_type); - config_group_init_type_name(&uvcg_control_class_ss_grp, - "ss", &uvcg_control_class_type); - config_group_init_type_name(&uvcg_control_class_grp, - "class", - &uvcg_control_class_grp_type); - configfs_add_default_group(&uvcg_control_class_fs_grp, - &uvcg_control_class_grp); - configfs_add_default_group(&uvcg_control_class_ss_grp, - &uvcg_control_class_grp); - - config_group_init_type_name(&uvcg_control_grp, - "control", - &uvcg_control_grp_type); - configfs_add_default_group(&uvcg_control_header_grp, - &uvcg_control_grp); - configfs_add_default_group(&uvcg_processing_grp, - &uvcg_control_grp); - configfs_add_default_group(&uvcg_terminal_grp, - &uvcg_control_grp); - configfs_add_default_group(&uvcg_control_class_grp, - &uvcg_control_grp); - - config_group_init_type_name(&uvcg_streaming_header_grp, - "header", - &uvcg_streaming_header_grp_type); - config_group_init_type_name(&uvcg_uncompressed_grp, - "uncompressed", - &uvcg_uncompressed_grp_type); - config_group_init_type_name(&uvcg_mjpeg_grp, - "mjpeg", - &uvcg_mjpeg_grp_type); - config_group_init_type_name(&uvcg_default_color_matching_grp, - "default", - &uvcg_default_color_matching_type); - config_group_init_type_name(&uvcg_color_matching_grp, - "color_matching", - &uvcg_color_matching_grp_type); - configfs_add_default_group(&uvcg_default_color_matching_grp, - &uvcg_color_matching_grp); - - config_group_init_type_name(&uvcg_streaming_class_fs_grp, - "fs", &uvcg_streaming_class_type); - config_group_init_type_name(&uvcg_streaming_class_hs_grp, - "hs", &uvcg_streaming_class_type); - config_group_init_type_name(&uvcg_streaming_class_ss_grp, - "ss", &uvcg_streaming_class_type); - config_group_init_type_name(&uvcg_streaming_class_grp, - "class", &uvcg_streaming_class_grp_type); - configfs_add_default_group(&uvcg_streaming_class_fs_grp, - &uvcg_streaming_class_grp); - configfs_add_default_group(&uvcg_streaming_class_hs_grp, - &uvcg_streaming_class_grp); - configfs_add_default_group(&uvcg_streaming_class_ss_grp, - &uvcg_streaming_class_grp); - - config_group_init_type_name(&uvcg_streaming_grp, - "streaming", &uvcg_streaming_grp_type); - configfs_add_default_group(&uvcg_streaming_header_grp, - &uvcg_streaming_grp); - configfs_add_default_group(&uvcg_uncompressed_grp, - &uvcg_streaming_grp); - configfs_add_default_group(&uvcg_mjpeg_grp, - &uvcg_streaming_grp); - configfs_add_default_group(&uvcg_color_matching_grp, - &uvcg_streaming_grp); - configfs_add_default_group(&uvcg_streaming_class_grp, - &uvcg_streaming_grp); - - config_group_init_type_name(&opts->func_inst.group, - "", - &uvc_func_type); - configfs_add_default_group(&uvcg_control_grp, - &opts->func_inst.group); - configfs_add_default_group(&uvcg_streaming_grp, - &opts->func_inst.group); + int ret; - return 0; + config_group_init_type_name(&opts->func_inst.group, uvc_func_type.name, + &uvc_func_type.type); + + ret = uvcg_config_create_children(&opts->func_inst.group, + &uvc_func_type); + if (ret < 0) + config_group_put(&opts->func_inst.group); + + return ret; }
The UVC configfs implementation creates all groups as global static variables. This prevents creationg of multiple UVC function instances, as they would all require their own configfs group instances. Fix this by allocating all groups dynamically. To avoid duplicating code around, extend the config_item_type structure with group name and children, and implement helper functions to create children automatically for most groups. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/usb/gadget/function/f_uvc.c | 8 +- drivers/usb/gadget/function/uvc_configfs.c | 480 +++++++++++++++++------------ 2 files changed, 282 insertions(+), 206 deletions(-)