Hi,
On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> map->get() gets a value from an uvc_control in "UVC format" and converts
> it to a value that can be consumed by v4l2.
>
> Instead of using a special get function for V4L2_CTRL_TYPE_MENU, we
> were converting from uvc_get_le_value in two different places.
>
> Move the conversion to uvc_get_le_value().
>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 77 +++++++++++++++++-----------------------
> 1 file changed, 32 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index bab9fdac98e6..77f7058ec966 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -862,6 +862,25 @@ static inline void uvc_clear_bit(u8 *data, int bit)
> data[bit >> 3] &= ~(1 << (bit & 7));
> }
>
> +static s32 uvc_menu_to_v4l2_menu(struct uvc_control_mapping *mapping, s32 val)
> +{
> + unsigned int i;
> +
> + for (i = 0; BIT(i) <= mapping->menu_mask; ++i) {
> + u32 menu_value;
> +
> + if (!test_bit(i, &mapping->menu_mask))
> + continue;
> +
> + menu_value = uvc_mapping_get_menu_value(mapping, i);
> +
> + if (menu_value == val)
> + return i;
> + }
> +
> + return val;
> +}
> +
> /*
> * Extract the bit string specified by mapping->offset and mapping->size
> * from the little-endian data stored at 'data' and return the result as
> @@ -896,6 +915,16 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
> value |= -(value & (1 << (mapping->size - 1)));
>
> + /* If it is a menu, convert from uvc to v4l2. */
> + if (mapping->v4l2_type != V4L2_CTRL_TYPE_MENU)
> + return value;
> +
> + switch (query) {
> + case UVC_GET_CUR:
> + case UVC_GET_DEF:
> + return uvc_menu_to_v4l2_menu(mapping, value);
> + }
> +
> return value;
> }
>
> @@ -1060,32 +1089,6 @@ static int uvc_ctrl_populate_cache(struct uvc_video_chain *chain,
> return 0;
> }
>
> -static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
> - const u8 *data)
> -{
> - s32 value = mapping->get(mapping, UVC_GET_CUR, data);
> -
> - if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) {
> - unsigned int i;
> -
> - for (i = 0; BIT(i) <= mapping->menu_mask; ++i) {
> - u32 menu_value;
> -
> - if (!test_bit(i, &mapping->menu_mask))
> - continue;
> -
> - menu_value = uvc_mapping_get_menu_value(mapping, i);
> -
> - if (menu_value == value) {
> - value = i;
> - break;
> - }
> - }
> - }
> -
> - return value;
> -}
> -
> static int __uvc_ctrl_load_cur(struct uvc_video_chain *chain,
> struct uvc_control *ctrl)
> {
> @@ -1136,8 +1139,8 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
> if (ret < 0)
> return ret;
>
> - *value = __uvc_ctrl_get_value(mapping,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> + *value = mapping->get(mapping, UVC_GET_CUR,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
>
> return 0;
> }
> @@ -1287,7 +1290,6 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> {
> struct uvc_control_mapping *master_map = NULL;
> struct uvc_control *master_ctrl = NULL;
> - unsigned int i;
>
> memset(v4l2_ctrl, 0, sizeof(*v4l2_ctrl));
> v4l2_ctrl->id = mapping->id;
> @@ -1330,21 +1332,6 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> v4l2_ctrl->minimum = ffs(mapping->menu_mask) - 1;
> v4l2_ctrl->maximum = fls(mapping->menu_mask) - 1;
> v4l2_ctrl->step = 1;
> -
> - for (i = 0; BIT(i) <= mapping->menu_mask; ++i) {
> - u32 menu_value;
> -
> - if (!test_bit(i, &mapping->menu_mask))
> - continue;
> -
> - menu_value = uvc_mapping_get_menu_value(mapping, i);
> -
> - if (menu_value == v4l2_ctrl->default_value) {
> - v4l2_ctrl->default_value = i;
> - break;
> - }
> - }
> -
> return 0;
>
> case V4L2_CTRL_TYPE_BOOLEAN:
> @@ -1592,7 +1579,7 @@ void uvc_ctrl_status_event(struct uvc_video_chain *chain,
> ctrl->handle = NULL;
>
> list_for_each_entry(mapping, &ctrl->info.mappings, list) {
> - s32 value = __uvc_ctrl_get_value(mapping, data);
> + s32 value = mapping->get(mapping, UVC_GET_CUR, data);
>
> /*
> * handle may be NULL here if the device sends auto-update
>
