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

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

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