Re: [PATCH v15 12/19] media: uvcvideo: Factor out query_boundaries from query_ctrl

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> Split the function in two parts. queryctrl_boundaries will be used in
> future patches.
> 
> No functional change expected from this patch.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 106 ++++++++++++++++++++++-----------------
>  1 file changed, 60 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e51cd0a2228a..b591d7fddc37 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1367,53 +1367,11 @@ static u32 uvc_get_ctrl_bitmap(struct uvc_control *ctrl,
>  	return ~0;
>  }
>  
> -static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> -	struct uvc_control *ctrl,
> -	struct uvc_control_mapping *mapping,
> -	struct v4l2_queryctrl *v4l2_ctrl)
> +static int __uvc_queryctrl_boundaries(struct uvc_video_chain *chain,
> +				      struct uvc_control *ctrl,
> +				      struct uvc_control_mapping *mapping,
> +				      struct v4l2_queryctrl *v4l2_ctrl)
>  {
> -	struct uvc_control_mapping *master_map = NULL;
> -	struct uvc_control *master_ctrl = NULL;
> -
> -	memset(v4l2_ctrl, 0, sizeof(*v4l2_ctrl));
> -	v4l2_ctrl->id = mapping->id;
> -	v4l2_ctrl->type = mapping->v4l2_type;
> -	strscpy(v4l2_ctrl->name, uvc_map_get_name(mapping),
> -		sizeof(v4l2_ctrl->name));
> -	v4l2_ctrl->flags = 0;
> -
> -	if (!(ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR))
> -		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> -	if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> -		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> -
> -	if (mapping->master_id)
> -		__uvc_find_control(ctrl->entity, mapping->master_id,
> -				   &master_map, &master_ctrl, 0, 0);
> -	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
> -		s32 val;
> -		int ret;
> -
> -		if (WARN_ON(uvc_ctrl_mapping_is_compound(master_map)))
> -			return -EIO;
> -
> -		ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
> -		if (ret < 0)
> -			return ret;
> -
> -		if (val != mapping->master_manual)
> -				v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> -	}
> -
> -	if (v4l2_ctrl->type >= V4L2_CTRL_COMPOUND_TYPES) {
> -		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_HAS_PAYLOAD;
> -		v4l2_ctrl->default_value = 0;
> -		v4l2_ctrl->minimum = 0;
> -		v4l2_ctrl->maximum = 0;
> -		v4l2_ctrl->step = 0;
> -		return 0;
> -	}
> -
>  	if (!ctrl->cached) {
>  		int ret = uvc_ctrl_populate_cache(chain, ctrl);
>  		if (ret < 0)
> @@ -1456,18 +1414,74 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>  	if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)
>  		v4l2_ctrl->minimum = uvc_mapping_get_s32(mapping, UVC_GET_MIN,
>  				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> +	else
> +		v4l2_ctrl->minimum = 0;
>  
>  	if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)
>  		v4l2_ctrl->maximum = uvc_mapping_get_s32(mapping, UVC_GET_MAX,
>  				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> +	else
> +		v4l2_ctrl->maximum = 0;
>  
>  	if (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES)
>  		v4l2_ctrl->step = uvc_mapping_get_s32(mapping, UVC_GET_RES,
>  				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> +	else
> +		v4l2_ctrl->step = 0;
>  
>  	return 0;
>  }
>  
> +static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> +				 struct uvc_control *ctrl,
> +				 struct uvc_control_mapping *mapping,
> +				 struct v4l2_queryctrl *v4l2_ctrl)
> +{
> +	struct uvc_control_mapping *master_map = NULL;
> +	struct uvc_control *master_ctrl = NULL;
> +
> +	memset(v4l2_ctrl, 0, sizeof(*v4l2_ctrl));
> +	v4l2_ctrl->id = mapping->id;
> +	v4l2_ctrl->type = mapping->v4l2_type;
> +	strscpy(v4l2_ctrl->name, uvc_map_get_name(mapping),
> +		sizeof(v4l2_ctrl->name));
> +	v4l2_ctrl->flags = 0;
> +
> +	if (!(ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR))
> +		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> +	if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> +		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> +	if (mapping->master_id)
> +		__uvc_find_control(ctrl->entity, mapping->master_id,
> +				   &master_map, &master_ctrl, 0, 0);
> +	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
> +		s32 val;
> +		int ret;
> +
> +		if (WARN_ON(uvc_ctrl_mapping_is_compound(master_map)))
> +			return -EIO;
> +
> +		ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (val != mapping->master_manual)
> +			v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> +	}
> +
> +	if (v4l2_ctrl->type >= V4L2_CTRL_COMPOUND_TYPES) {
> +		v4l2_ctrl->flags |= V4L2_CTRL_FLAG_HAS_PAYLOAD;
> +		v4l2_ctrl->default_value = 0;
> +		v4l2_ctrl->minimum = 0;
> +		v4l2_ctrl->maximum = 0;
> +		v4l2_ctrl->step = 0;
> +		return 0;
> +	}
> +
> +	return __uvc_queryctrl_boundaries(chain, ctrl, mapping, v4l2_ctrl);
> +}
> +
>  int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>  	struct v4l2_queryctrl *v4l2_ctrl)
>  {
>