Move the logic to a separated function. Do not expect any change.
This is a preparation for supporting compound controls.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 82 +++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 38 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 6d5167eb368d..893d12cd3f90 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2002,28 +2002,17 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
return -EINVAL;
}
-int uvc_ctrl_set(struct uvc_fh *handle,
- struct v4l2_ext_control *xctrl)
+static int uvc_ctrl_clamp(struct uvc_video_chain *chain,
+ struct uvc_control *ctrl,
+ struct uvc_control_mapping *mapping,
+ s32 *value_in_out)
{
- struct uvc_video_chain *chain = handle->chain;
- struct uvc_control *ctrl;
- struct uvc_control_mapping *mapping;
- s32 value;
+ s32 value = *value_in_out;
u32 step;
s32 min;
s32 max;
int ret;
- if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
- return -EACCES;
-
- ctrl = uvc_find_control(chain, xctrl->id, &mapping);
- if (ctrl == NULL)
- return -EINVAL;
- if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
- return -EACCES;
-
- /* Clamp out of range values. */
switch (mapping->v4l2_type) {
case V4L2_CTRL_TYPE_INTEGER:
if (!ctrl->cached) {
@@ -2041,14 +2030,13 @@ int uvc_ctrl_set(struct uvc_fh *handle,
if (step == 0)
step = 1;
- xctrl->value = min + DIV_ROUND_CLOSEST((u32)(xctrl->value - min),
- step) * step;
+ value = min + DIV_ROUND_CLOSEST((u32)(value - min), step) * step;
if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
- xctrl->value = clamp(xctrl->value, min, max);
+ value = clamp(value, min, max);
else
- xctrl->value = clamp_t(u32, xctrl->value, min, max);
- value = xctrl->value;
- break;
+ value = clamp_t(u32, value, min, max);
+ *value_in_out = value;
+ return 0;
case V4L2_CTRL_TYPE_BITMASK:
if (!ctrl->cached) {
@@ -2057,21 +2045,20 @@ int uvc_ctrl_set(struct uvc_fh *handle,
return ret;
}
- xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping);
- value = xctrl->value;
- break;
+ value &= uvc_get_ctrl_bitmap(ctrl, mapping);
+ *value_in_out = value;
+ return 0;
case V4L2_CTRL_TYPE_BOOLEAN:
- xctrl->value = clamp(xctrl->value, 0, 1);
- value = xctrl->value;
- break;
+ *value_in_out = clamp(value, 0, 1);
+ return 0;
case V4L2_CTRL_TYPE_MENU:
- if (xctrl->value < (ffs(mapping->menu_mask) - 1) ||
- xctrl->value > (fls(mapping->menu_mask) - 1))
+ if (value < (ffs(mapping->menu_mask) - 1) ||
+ value > (fls(mapping->menu_mask) - 1))
return -ERANGE;
- if (!test_bit(xctrl->value, &mapping->menu_mask))
+ if (!test_bit(value, &mapping->menu_mask))
return -EINVAL;
/*
@@ -2079,8 +2066,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
* UVC controls that support it.
*/
if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) {
- int val = uvc_mapping_get_menu_value(mapping,
- xctrl->value);
+ int val = uvc_mapping_get_menu_value(mapping, value);
if (!ctrl->cached) {
ret = uvc_ctrl_populate_cache(chain, ctrl);
if (ret < 0)
@@ -2090,14 +2076,34 @@ int uvc_ctrl_set(struct uvc_fh *handle,
if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & val))
return -EINVAL;
}
- value = xctrl->value;
- break;
+ return 0;
default:
- value = xctrl->value;
- break;
+ return 0;
}
+ return 0;
+}
+
+int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
+{
+ struct uvc_video_chain *chain = handle->chain;
+ struct uvc_control_mapping *mapping;
+ struct uvc_control *ctrl;
+ int ret;
+
+ if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
+ return -EACCES;
+
+ ctrl = uvc_find_control(chain, xctrl->id, &mapping);
+ if (!ctrl)
+ return -EINVAL;
+ if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
+ return -EACCES;
+
+ ret = uvc_ctrl_clamp(chain, ctrl, mapping, &xctrl->value);
+ if (ret)
+ return ret;
/*
* If the mapping doesn't span the whole UVC control, the current value
* needs to be loaded from the device to perform the read-modify-write
@@ -2116,7 +2122,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
ctrl->info.size);
}
- uvc_mapping_set_s32(mapping, value,
+ uvc_mapping_set_s32(mapping, xctrl->value,
uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
--
2.47.0.338.g60cca15819-googHi,
On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> Move the logic to a separated function. Do not expect any change.
> This is a preparation for supporting compound controls.
>
> 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 | 82 +++++++++++++++++++++-------------------
> 1 file changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 6d5167eb368d..893d12cd3f90 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2002,28 +2002,17 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
> return -EINVAL;
> }
>
> -int uvc_ctrl_set(struct uvc_fh *handle,
> - struct v4l2_ext_control *xctrl)
> +static int uvc_ctrl_clamp(struct uvc_video_chain *chain,
> + struct uvc_control *ctrl,
> + struct uvc_control_mapping *mapping,
> + s32 *value_in_out)
> {
> - struct uvc_video_chain *chain = handle->chain;
> - struct uvc_control *ctrl;
> - struct uvc_control_mapping *mapping;
> - s32 value;
> + s32 value = *value_in_out;
> u32 step;
> s32 min;
> s32 max;
> int ret;
>
> - if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
> - return -EACCES;
> -
> - ctrl = uvc_find_control(chain, xctrl->id, &mapping);
> - if (ctrl == NULL)
> - return -EINVAL;
> - if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> - return -EACCES;
> -
> - /* Clamp out of range values. */
> switch (mapping->v4l2_type) {
> case V4L2_CTRL_TYPE_INTEGER:
> if (!ctrl->cached) {
> @@ -2041,14 +2030,13 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> if (step == 0)
> step = 1;
>
> - xctrl->value = min + DIV_ROUND_CLOSEST((u32)(xctrl->value - min),
> - step) * step;
> + value = min + DIV_ROUND_CLOSEST((u32)(value - min), step) * step;
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
> - xctrl->value = clamp(xctrl->value, min, max);
> + value = clamp(value, min, max);
> else
> - xctrl->value = clamp_t(u32, xctrl->value, min, max);
> - value = xctrl->value;
> - break;
> + value = clamp_t(u32, value, min, max);
> + *value_in_out = value;
> + return 0;
>
> case V4L2_CTRL_TYPE_BITMASK:
> if (!ctrl->cached) {
> @@ -2057,21 +2045,20 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> return ret;
> }
>
> - xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping);
> - value = xctrl->value;
> - break;
> + value &= uvc_get_ctrl_bitmap(ctrl, mapping);
> + *value_in_out = value;
> + return 0;
>
> case V4L2_CTRL_TYPE_BOOLEAN:
> - xctrl->value = clamp(xctrl->value, 0, 1);
> - value = xctrl->value;
> - break;
> + *value_in_out = clamp(value, 0, 1);
> + return 0;
>
> case V4L2_CTRL_TYPE_MENU:
> - if (xctrl->value < (ffs(mapping->menu_mask) - 1) ||
> - xctrl->value > (fls(mapping->menu_mask) - 1))
> + if (value < (ffs(mapping->menu_mask) - 1) ||
> + value > (fls(mapping->menu_mask) - 1))
> return -ERANGE;
>
> - if (!test_bit(xctrl->value, &mapping->menu_mask))
> + if (!test_bit(value, &mapping->menu_mask))
> return -EINVAL;
>
> /*
> @@ -2079,8 +2066,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> * UVC controls that support it.
> */
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) {
> - int val = uvc_mapping_get_menu_value(mapping,
> - xctrl->value);
> + int val = uvc_mapping_get_menu_value(mapping, value);
> if (!ctrl->cached) {
> ret = uvc_ctrl_populate_cache(chain, ctrl);
> if (ret < 0)
> @@ -2090,14 +2076,34 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & val))
> return -EINVAL;
> }
> - value = xctrl->value;
> - break;
> + return 0;
>
> default:
> - value = xctrl->value;
> - break;
> + return 0;
> }
>
> + return 0;
> +}
> +
> +int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
> +{
> + struct uvc_video_chain *chain = handle->chain;
> + struct uvc_control_mapping *mapping;
> + struct uvc_control *ctrl;
> + int ret;
> +
> + if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
> + return -EACCES;
> +
> + ctrl = uvc_find_control(chain, xctrl->id, &mapping);
> + if (!ctrl)
> + return -EINVAL;
> + if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> + return -EACCES;
> +
> + ret = uvc_ctrl_clamp(chain, ctrl, mapping, &xctrl->value);
> + if (ret)
> + return ret;
> /*
> * If the mapping doesn't span the whole UVC control, the current value
> * needs to be loaded from the device to perform the read-modify-write
> @@ -2116,7 +2122,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> ctrl->info.size);
> }
>
> - uvc_mapping_set_s32(mapping, value,
> + uvc_mapping_set_s32(mapping, xctrl->value,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
>
> if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
>
Hi Ricardo,
This patch looks good to me.
Reviewed-by: Yunke Cao <yunkec@google.com>
Thanks,
Yunke
On Fri, Nov 15, 2024 at 4:10 AM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Move the logic to a separated function. Do not expect any change.
> This is a preparation for supporting compound controls.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 82 +++++++++++++++++++++-------------------
> 1 file changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 6d5167eb368d..893d12cd3f90 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2002,28 +2002,17 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
> return -EINVAL;
> }
>
> -int uvc_ctrl_set(struct uvc_fh *handle,
> - struct v4l2_ext_control *xctrl)
> +static int uvc_ctrl_clamp(struct uvc_video_chain *chain,
> + struct uvc_control *ctrl,
> + struct uvc_control_mapping *mapping,
> + s32 *value_in_out)
> {
> - struct uvc_video_chain *chain = handle->chain;
> - struct uvc_control *ctrl;
> - struct uvc_control_mapping *mapping;
> - s32 value;
> + s32 value = *value_in_out;
> u32 step;
> s32 min;
> s32 max;
> int ret;
>
> - if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
> - return -EACCES;
> -
> - ctrl = uvc_find_control(chain, xctrl->id, &mapping);
> - if (ctrl == NULL)
> - return -EINVAL;
> - if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> - return -EACCES;
> -
> - /* Clamp out of range values. */
> switch (mapping->v4l2_type) {
> case V4L2_CTRL_TYPE_INTEGER:
> if (!ctrl->cached) {
> @@ -2041,14 +2030,13 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> if (step == 0)
> step = 1;
>
> - xctrl->value = min + DIV_ROUND_CLOSEST((u32)(xctrl->value - min),
> - step) * step;
> + value = min + DIV_ROUND_CLOSEST((u32)(value - min), step) * step;
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
> - xctrl->value = clamp(xctrl->value, min, max);
> + value = clamp(value, min, max);
> else
> - xctrl->value = clamp_t(u32, xctrl->value, min, max);
> - value = xctrl->value;
> - break;
> + value = clamp_t(u32, value, min, max);
> + *value_in_out = value;
> + return 0;
>
> case V4L2_CTRL_TYPE_BITMASK:
> if (!ctrl->cached) {
> @@ -2057,21 +2045,20 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> return ret;
> }
>
> - xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping);
> - value = xctrl->value;
> - break;
> + value &= uvc_get_ctrl_bitmap(ctrl, mapping);
> + *value_in_out = value;
> + return 0;
>
> case V4L2_CTRL_TYPE_BOOLEAN:
> - xctrl->value = clamp(xctrl->value, 0, 1);
> - value = xctrl->value;
> - break;
> + *value_in_out = clamp(value, 0, 1);
> + return 0;
>
> case V4L2_CTRL_TYPE_MENU:
> - if (xctrl->value < (ffs(mapping->menu_mask) - 1) ||
> - xctrl->value > (fls(mapping->menu_mask) - 1))
> + if (value < (ffs(mapping->menu_mask) - 1) ||
> + value > (fls(mapping->menu_mask) - 1))
> return -ERANGE;
>
> - if (!test_bit(xctrl->value, &mapping->menu_mask))
> + if (!test_bit(value, &mapping->menu_mask))
> return -EINVAL;
>
> /*
> @@ -2079,8 +2066,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> * UVC controls that support it.
> */
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) {
> - int val = uvc_mapping_get_menu_value(mapping,
> - xctrl->value);
> + int val = uvc_mapping_get_menu_value(mapping, value);
> if (!ctrl->cached) {
> ret = uvc_ctrl_populate_cache(chain, ctrl);
> if (ret < 0)
> @@ -2090,14 +2076,34 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & val))
> return -EINVAL;
> }
> - value = xctrl->value;
> - break;
> + return 0;
>
> default:
> - value = xctrl->value;
> - break;
> + return 0;
> }
>
> + return 0;
> +}
> +
> +int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
> +{
> + struct uvc_video_chain *chain = handle->chain;
> + struct uvc_control_mapping *mapping;
> + struct uvc_control *ctrl;
> + int ret;
> +
> + if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
> + return -EACCES;
> +
> + ctrl = uvc_find_control(chain, xctrl->id, &mapping);
> + if (!ctrl)
> + return -EINVAL;
> + if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> + return -EACCES;
> +
> + ret = uvc_ctrl_clamp(chain, ctrl, mapping, &xctrl->value);
> + if (ret)
> + return ret;
> /*
> * If the mapping doesn't span the whole UVC control, the current value
> * needs to be loaded from the device to perform the read-modify-write
> @@ -2116,7 +2122,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> ctrl->info.size);
> }
>
> - uvc_mapping_set_s32(mapping, value,
> + uvc_mapping_set_s32(mapping, xctrl->value,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
>
> if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
>
> --
> 2.47.0.338.g60cca15819-goog
>
© 2016 - 2026 Red Hat, Inc.