usb: gadget: uvc: configfs: Allocate groups dynamically
PATCH 3/8 usb: gadget: uvc: configfs: Allocate groups dynamically
Hi Kieran,
Quoted 23 lines · On Wednesday, 1 August 2018 12:58:40 EEST Kieran Bingham wrote:
Quoted 2 lines · Laurent Pinchart
The UVC configfs implementation creates all groups as global static variables. This prevents creationg of multiple UVC function instances,
/creationg/creation/
Quoted 8 lines
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_typestructure with group name and children, and implement helper functions to create children automatically for most groups.Signed-off-by Laurent Pinchart <[email protected]>I'm struggling to see what paths free all of the dynamically created children in this patch.
Is this already supported by the
config_groupframework?I see a reference to
config_group_put(&opts->func_inst.group); in one error path - but that's about it.
Quoted 8 lines
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.
Quoted 5 lines
Quoted 5 lines
---
drivers/usb/gadget/function/f_uvc.c | 8 +- drivers/usb/gadget/function/uvc_configfs.c | 480 ++++++++++++++---------- 2 files changed, 282 insertions(+), 206 deletions(-)
[snip]
Quoted 5 lines
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/usb/gadget/function/uvc_configfs.cindexdbc95c9558de..e019ea317c7a100644drivers/usb/gadget/function/uvc_configfs.c +0
-0
[snip]
Quoted 4 lines
Quoted 4 lines
drivers/usb/gadget/function/uvc_configfs.c +0
-4-static struct config_group uvcg_terminal_grp; - -static const struct config_item_type uvcg_terminal_grp_type = { - .ct_owner =THIS_MODULE ,
Quoted 8 lines
drivers/usb/gadget/function/uvc_configfs.c
Quoted 6 lines
drivers/usb/gadget/function/uvc_configfs.c +6
-0+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,
};Quoted 5 lines
Quoted 5 lines
drivers/usb/gadget/function/uvc_configfs.c +4
-0+ &uvcg_camera_grp_type, + &uvcg_output_grp_type, + NULL, + }, };
[snip]