[4/8] usb: gadget: uvc: configfs: Add interface number attributes

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Kieran,

On Wednesday, 1 August 2018 13:07:20 EEST Kieran Bingham wrote:
> 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 <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  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 ?

You're right, I'll document it.

> >  3 files changed, 67 insertions(+)

[snip]

> > 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,
> > +};

[snip]

> > +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.

Feel free to submit patches :-) Helper functions would be event better than 
macros to decrease the object size.

> > +
> > +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,
> > +};

[snip]