Hi Laurent, Thank you for the patch,
On 01/08/18 01:29, Laurent Pinchart wrote: > The video control and video streaming interface numbers are needed in > the UVC gadget userspace stack to reply to UVC requests. They are > hardcoded to fixed values at the moment, preventing configurations with > multiple functions. > > To fix this, make them dynamically discoverable by userspace through > read-only configfs attributes in <function>/control/bInterfaceNumber and > <function>/streaming/bInterfaceNumber respectively. > > Signed-off-by: Laurent Pinchart <[email protected]> > --- > drivers/usb/gadget/function/f_uvc.c | 2 + > drivers/usb/gadget/function/u_uvc.h | 3 ++ > drivers/usb/gadget/function/uvc_configfs.c | 62 > ++++++++++++++++++++++++++++++ It sounds like this is extending the userspace ABI. Do we need to document this anywhere? Documentation/ABI/testing/configfs-usb-gadget-uvc ? > 3 files changed, 67 insertions(+) > > diff --git a/drivers/usb/gadget/function/f_uvc.c > b/drivers/usb/gadget/function/f_uvc.c > index 95cb1b5f5ffe..4ea987741e6e 100644 > --- a/drivers/usb/gadget/function/f_uvc.c > +++ b/drivers/usb/gadget/function/f_uvc.c > @@ -699,12 +699,14 @@ uvc_function_bind(struct usb_configuration *c, struct > usb_function *f) > uvc_iad.bFirstInterface = ret; > uvc_control_intf.bInterfaceNumber = ret; > uvc->control_intf = ret; > + opts->control_interface = ret; > > if ((ret = usb_interface_id(c, f)) < 0) > goto error; > uvc_streaming_intf_alt0.bInterfaceNumber = ret; > uvc_streaming_intf_alt1.bInterfaceNumber = ret; > uvc->streaming_intf = ret; > + opts->streaming_interface = ret; > > /* Copy descriptors */ > f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL); > diff --git a/drivers/usb/gadget/function/u_uvc.h > b/drivers/usb/gadget/function/u_uvc.h > index 2ed292e94fbc..5242d489e20a 100644 > --- a/drivers/usb/gadget/function/u_uvc.h > +++ b/drivers/usb/gadget/function/u_uvc.h > @@ -25,6 +25,9 @@ struct f_uvc_opts { > unsigned int streaming_maxpacket; > unsigned int streaming_maxburst; > > + unsigned int control_interface; > + unsigned int streaming_interface; > + > /* > * Control descriptors array pointers for full-/high-speed and > * super-speed. They point by default to the uvc_fs_control_cls and > diff --git a/drivers/usb/gadget/function/uvc_configfs.c > b/drivers/usb/gadget/function/uvc_configfs.c > index e019ea317c7a..801117686108 100644 > --- a/drivers/usb/gadget/function/uvc_configfs.c > +++ b/drivers/usb/gadget/function/uvc_configfs.c > @@ -687,8 +687,39 @@ static const struct uvcg_config_group_type > uvcg_control_class_grp_type = { > * control > */ > > +static ssize_t uvcg_default_control_b_interface_number_show( > + struct config_item *item, char *page) > +{ > + struct config_group *group = to_config_group(item); > + struct mutex *su_mutex = &group->cg_subsys->su_mutex; > + struct config_item *opts_item; > + struct f_uvc_opts *opts; > + int result = 0; > + > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */ > + > + opts_item = item->ci_parent; > + opts = to_f_uvc_opts(opts_item); > + > + mutex_lock(&opts->lock); > + result += sprintf(page, "%u\n", opts->control_interface); > + mutex_unlock(&opts->lock); > + > + mutex_unlock(su_mutex); > + > + return result; > +} > + > +UVC_ATTR_RO(uvcg_default_control_, b_interface_number, bInterfaceNumber); > + > +static struct configfs_attribute *uvcg_default_control_attrs[] = { > + &uvcg_default_control_attr_b_interface_number, > + NULL, > +}; > + > static const struct uvcg_config_group_type uvcg_control_grp_type = { > .type = { > + .ct_attrs = uvcg_default_control_attrs, > .ct_owner = THIS_MODULE, > }, > .name = "control", > @@ -2246,8 +2277,39 @@ static const struct uvcg_config_group_type > uvcg_streaming_class_grp_type = { > * streaming > */ > > +static ssize_t uvcg_default_streaming_b_interface_number_show( > + struct config_item *item, char *page) > +{ > + struct config_group *group = to_config_group(item); > + struct mutex *su_mutex = &group->cg_subsys->su_mutex; > + struct config_item *opts_item; > + struct f_uvc_opts *opts; > + int result = 0; > + > + mutex_lock(su_mutex); /* for navigating configfs hierarchy */ > + > + opts_item = item->ci_parent; > + opts = to_f_uvc_opts(opts_item); > + > + mutex_lock(&opts->lock); > + result += sprintf(page, "%u\n", opts->streaming_interface); > + mutex_unlock(&opts->lock); > + > + mutex_unlock(su_mutex); > + > + return result; > +} That looks like a lot of common boilerplate code copied for each file which prints a single %u. Perhaps we should convert those to a macro - but for now they look fine. > + > +UVC_ATTR_RO(uvcg_default_streaming_, b_interface_number, bInterfaceNumber);> > + > +static struct configfs_attribute *uvcg_default_streaming_attrs[] = { > + &uvcg_default_streaming_attr_b_interface_number, > + NULL, > +}; > + > static const struct uvcg_config_group_type uvcg_streaming_grp_type = { > .type = { > + .ct_attrs = uvcg_default_streaming_attrs, > .ct_owner = THIS_MODULE, > }, > .name = "streaming", > -- Regards -- Kieran -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
