From: Ricardo Ribalda <ribalda@chromium.org>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Subject: Re: [PATCH RESEND v2 1/2] media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU
Date: Fri, 2 Dec 2022 18:34:26 +0100 [thread overview]
Message-ID: <CANiDSCtzuOmSCPfYxm-uynfHRbr2AbvbqEGPniy+=vkKjdgadA@mail.gmail.com> (raw)
In-Reply-To: <20220920-standard-menues-v2-1-323be5539c22@chromium.org>
Please ignore this one. It is already part of the series Follow-up
patches for uvc v4l2-compliance
Sorry for sending it duplicated
On Fri, 2 Dec 2022 at 18:26, Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Replace the count with a mask field that let us choose not only the max
> value, but also the minimum value and what values are valid in between.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++----------
> drivers/media/usb/uvc/uvc_driver.c | 3 ++-
> drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
> drivers/media/usb/uvc/uvcvideo.h | 2 +-
> 4 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 8c208db9600b..13bc57e2a08d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -524,7 +524,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_BITMASK,
> .menu_info = exposure_auto_controls,
> - .menu_count = ARRAY_SIZE(exposure_auto_controls),
> + .menu_mask =
> + BIT_MASK(ARRAY_SIZE(exposure_auto_controls)),
> .slave_ids = { V4L2_CID_EXPOSURE_ABSOLUTE, },
> },
> {
> @@ -730,7 +731,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = {
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> .menu_info = power_line_frequency_controls,
> - .menu_count = ARRAY_SIZE(power_line_frequency_controls) - 1,
> + .menu_mask =
> + BIT_MASK(ARRAY_SIZE(power_line_frequency_controls) - 1),
> },
> };
>
> @@ -744,7 +746,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings_uvc15[] = {
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> .menu_info = power_line_frequency_controls,
> - .menu_count = ARRAY_SIZE(power_line_frequency_controls),
> + .menu_mask =
> + BIT_MASK(ARRAY_SIZE(power_line_frequency_controls)),
> },
> };
>
> @@ -974,7 +977,9 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
> const struct uvc_menu_info *menu = mapping->menu_info;
> unsigned int i;
>
> - for (i = 0; i < mapping->menu_count; ++i, ++menu) {
> + for (i = 1; BIT(i) <= mapping->menu_mask; ++i, ++menu) {
> + if (!test_bit(i, &mapping->menu_mask))
> + continue;
> if (menu->value == value) {
> value = i;
> break;
> @@ -1148,12 +1153,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>
> switch (mapping->v4l2_type) {
> case V4L2_CTRL_TYPE_MENU:
> - v4l2_ctrl->minimum = 0;
> - v4l2_ctrl->maximum = mapping->menu_count - 1;
> + v4l2_ctrl->minimum = ffs(mapping->menu_mask) - 1;
> + v4l2_ctrl->maximum = fls(mapping->menu_mask) - 1;
> v4l2_ctrl->step = 1;
>
> menu = mapping->menu_info;
> - for (i = 0; i < mapping->menu_count; ++i, ++menu) {
> + for (i = 1; BIT(i) <= mapping->menu_mask; ++i, ++menu) {
> + if (!test_bit(i, &mapping->menu_mask))
> + continue;
> if (menu->value == v4l2_ctrl->default_value) {
> v4l2_ctrl->default_value = i;
> break;
> @@ -1268,7 +1275,7 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> goto done;
> }
>
> - if (query_menu->index >= mapping->menu_count) {
> + if (!test_bit(query_menu->index, &mapping->menu_mask)) {
> ret = -EINVAL;
> goto done;
> }
> @@ -1776,8 +1783,13 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> break;
>
> case V4L2_CTRL_TYPE_MENU:
> - if (xctrl->value < 0 || xctrl->value >= mapping->menu_count)
> + if (xctrl->value < (ffs(mapping->menu_mask) - 1) ||
> + xctrl->value > (fls(mapping->menu_mask) - 1))
> return -ERANGE;
> +
> + if (!test_bit(xctrl->value, &mapping->menu_mask))
> + return -EINVAL;
> +
> value = mapping->menu_info[xctrl->value].value;
>
> /*
> @@ -2227,7 +2239,7 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>
> INIT_LIST_HEAD(&map->ev_subs);
>
> - size = sizeof(*mapping->menu_info) * mapping->menu_count;
> + size = sizeof(*mapping->menu_info) * fls(mapping->menu_mask);
> map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL);
> if (map->menu_info == NULL) {
> kfree(map->name);
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 9c05776f11d1..09cc459c1253 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2675,7 +2675,8 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> .menu_info = power_line_frequency_controls_limited,
> - .menu_count = ARRAY_SIZE(power_line_frequency_controls_limited),
> + .menu_mask =
> + BIT_MASK(ARRAY_SIZE(power_line_frequency_controls_limited)),
> };
>
> static const struct uvc_device_info uvc_ctrl_power_line_limited = {
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 4cc3fa6b8c98..07c7acce8025 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -80,7 +80,7 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain,
> goto free_map;
> }
>
> - map->menu_count = xmap->menu_count;
> + map->menu_mask = BIT_MASK(xmap->menu_count);
> break;
>
> default:
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 24c911aeebce..fde4e975334f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -255,7 +255,7 @@ struct uvc_control_mapping {
> u32 data_type;
>
> const struct uvc_menu_info *menu_info;
> - u32 menu_count;
> + unsigned long menu_mask;
>
> u32 master_id;
> s32 master_manual;
>
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog-b4-0.11.0-dev-696ae
--
Ricardo Ribalda
next prev parent reply other threads:[~2022-12-02 17:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-02 17:26 [PATCH RESEND v2 0/2] media: uvc: Get menu names from framework Ricardo Ribalda
2022-12-02 17:26 ` [PATCH RESEND v2 1/2] media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU Ricardo Ribalda
2022-12-02 17:34 ` Ricardo Ribalda [this message]
2022-12-02 17:26 ` [PATCH RESEND v2 2/2] media: uvcvideo: Use standard names for menus Ricardo Ribalda
2022-12-02 17:34 ` Ricardo Ribalda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANiDSCtzuOmSCPfYxm-uynfHRbr2AbvbqEGPniy+=vkKjdgadA@mail.gmail.com' \
--to=ribalda@chromium.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.