Hi,
On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> Be consistent with uvc_get_le_value() and do the menu translation there.
>
> Note that in this case, the refactor does not provide much... but
> consistency is a nice feature.
>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 77f7058ec966..c975e0ab479b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -939,6 +939,8 @@ static void uvc_set_le_value(struct uvc_control_mapping *mapping,
> int offset = mapping->offset;
> u8 mask;
>
> + if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU)
> + value = uvc_mapping_get_menu_value(mapping, value);
> /*
> * According to the v4l2 spec, writing any value to a button control
> * should result in the action belonging to the button control being
There is a:
if (mapping->v4l2_type == V4L2_CTRL_TYPE_BUTTON)
below this comment block, maybe put the if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) below
that as "else if (mapping->v4l2_type == V4L2_CTRL_TYPE_BUTTON) ... "
Or maybe make this a switch-case on mapping->v4l2_type right away for future
special handling of other types ?
Otherwise this looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> @@ -1988,23 +1990,23 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> if (!test_bit(xctrl->value, &mapping->menu_mask))
> return -EINVAL;
>
> - value = uvc_mapping_get_menu_value(mapping, xctrl->value);
> -
> /*
> * Valid menu indices are reported by the GET_RES request for
> * UVC controls that support it.
> */
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) {
> + int val = uvc_mapping_get_menu_value(mapping,
> + xctrl->value);
> if (!ctrl->cached) {
> ret = uvc_ctrl_populate_cache(chain, ctrl);
> if (ret < 0)
> return ret;
> }
>
> - if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & value))
> + if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & val))
> return -EINVAL;
> }
> -
> + value = xctrl->value;
> break;
>
> default:
>
