Re: [PATCH 2/3] usb: gadget: uvc: Add new disable_interrupt_ep attribute

Laurent Pinchart
2022-12-28T02:02:15Z

Hi Dan,

Thank you for the patch.

On Mon, Dec 05, 2022 at 02:37:57PM +0000, Daniel Scally wrote:
> Add a new attribute to the default control config group that allows
> users to specify whether they want to disable the default interrupt
> endpoint for the VideoControl interface.
>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>

Apart from the missing documentation, this looks good to me.

> ---
> drivers/usb/gadget/function/u_uvc.h | 2 +
> drivers/usb/gadget/function/uvc_configfs.c | 53 ++++++++++++++++++++++
> 2 files changed, 55 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
> index 24b8681b0d6f..7e6d31a8fad7 100644
> --- a/drivers/usb/gadget/function/u_uvc.h
> +++ b/drivers/usb/gadget/function/u_uvc.h
> @@ -29,6 +29,8 @@ struct f_uvc_opts {
> unsigned int streaming_interface;
> char function_name[32];
>
> + bool disable_interrupt_ep;
> +
> /*
> * 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 4303a3283ba0..644d87eee164 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -716,8 +716,61 @@ static ssize_t uvcg_default_control_b_interface_number_show(
>
> UVC_ATTR_RO(uvcg_default_control_, b_interface_number, bInterfaceNumber);
>
> +static ssize_t uvcg_default_control_disable_interrupt_ep_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->disable_interrupt_ep);
> + mutex_unlock(&opts->lock);
> +
> + mutex_unlock(su_mutex);
> +
> + return result;
> +}
> +
> +static ssize_t uvcg_default_control_disable_interrupt_ep_store(
> + struct config_item *item, const char *page, size_t len)
> +{
> + 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;
> + ssize_t ret;
> + u8 num;
> +
> + ret = kstrtou8(page, 0, &num);
> + if (ret)
> + return ret;
> +
> + mutex_lock(su_mutex); /* for navigating configfs hierarchy */
> +
> + opts_item = item->ci_parent;
> + opts = to_f_uvc_opts(opts_item);
> +
> + mutex_lock(&opts->lock);
> + opts->disable_interrupt_ep = num;
> + mutex_unlock(&opts->lock);
> +
> + mutex_unlock(su_mutex);
> +
> + return len;
> +}
> +UVC_ATTR(uvcg_default_control_, disable_interrupt_ep, disable_interrupt_ep);
> +
> static struct configfs_attribute *uvcg_default_control_attrs[] = {
> &uvcg_default_control_attr_b_interface_number,
> + &uvcg_default_control_attr_disable_interrupt_ep,
> NULL,
> };
>

--
Regards,

Laurent Pinchart