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 <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
