diff mbox series

media: uvcvideo: Filter hw errors while enumerating controls

Message ID 20241213-uvc-eaccess-v1-1-62e0b4fcc634@chromium.org
State New
Headers show
Series media: uvcvideo: Filter hw errors while enumerating controls | expand

Commit Message

Ricardo Ribalda Dec. 13, 2024, 11:21 a.m. UTC
To implement VIDIOC_QUERYCTRL, we need to read from the hardware all the
values that were not cached previously. If that read fails, we used to
report back the error to the user.

Unfortunately this does not play nice with userspace. When they are
enumerating the contols, the only expect an error when there are no
"next" control.

This is probably a corner case, and could be handled in userspace, but
both v4l2-ctl and yavta fail to enumerate all the controls if we return
then -EIO during VIDIOC_QUERYCTRL. I suspect that there are tons of
userspace apps handling this wrongly as well.

This patch get around this issue by ignoring the hardware errors and
always returning 0 if a control exists.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Hi 2*Hans and Laurent!

I came around a device that was listing just a couple of controls when
it should be listing much more.

Some debugging latter I found that the device was returning -EIO when
all the focal controls were read.

This could be solved in userspace.. but I suspect that a lot of people
has copied the implementation of v4l-utils or yavta.

What do you think of this workaround?

Without this patch:
$ v4l2-ctl --list-ctrls
                  auto_exposure 0x009a0901 (menu)   : min=0 max=3 default=3 value=3 (Aperture Priority Mode)
         exposure_time_absolute 0x009a0902 (int)    : min=50 max=10000 step=1 default=166 value=166 flags=inactive
     exposure_dynamic_framerate 0x009a0903 (bool)   : default=0 value=0
region_of_interest_auto_control 0x009a1902 (bitmask): max=0x00000001 default=0x00000001 value=1

With this patch:
$ v4l2-ctl --list-ctrls
                  auto_exposure 0x009a0901 (menu)   : min=0 max=3 default=3 value=3 (Aperture Priority Mode)
         exposure_time_absolute 0x009a0902 (int)    : min=50 max=10000 step=1 default=166 value=166 flags=inactive
     exposure_dynamic_framerate 0x009a0903 (bool)   : default=0 value=0
error 5 getting ext_ctrl Focus, Absolute
error 5 getting ext_ctrl Focus, Automatic Continuous
   region_of_interest_rectangle 0x009a1901 (unknown): type=107 value=unsupported payload type flags=has-payload
region_of_interest_auto_control 0x009a1902 (bitmask): max=0x00000001 default=0x00000001 value=1

--
---
 drivers/media/usb/uvc/uvc_ctrl.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)


---
base-commit: a7f5b36b34824415c28875d615c49a3cf5070615
change-id: 20241213-uvc-eaccess-755cc061a360

Best regards,
diff mbox series

Patch

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 4fe26e82e3d1..a11048c9a105 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1283,7 +1283,8 @@  static u32 uvc_get_ctrl_bitmap(struct uvc_control *ctrl,
 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 v4l2_queryctrl *v4l2_ctrl,
+	bool can_fail)
 {
 	struct uvc_control_mapping *master_map = NULL;
 	struct uvc_control *master_ctrl = NULL;
@@ -1307,17 +1308,28 @@  static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
 		s32 val;
 		int 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 (ret < 0) {
+			dev_warn_ratelimited(&chain->dev->udev->dev,
+					     "UVC non compliance: Error %d querying master control %x\n",
+					      ret, master_map->id);
+			if (can_fail)
+				return ret;
+		} else if (val != mapping->master_manual) {
+			v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+		}
 	}
 
 	if (!ctrl->cached) {
 		int ret = uvc_ctrl_populate_cache(chain, ctrl);
-		if (ret < 0)
-			return ret;
+
+		if (ret < 0) {
+			dev_warn_ratelimited(&chain->dev->udev->dev,
+					     "UVC non compliance: Error %d populating cache of control %x\n",
+					     ret, mapping->id);
+			if (can_fail)
+				return ret;
+		}
 	}
 
 	if (ctrl->info.flags & UVC_CTRL_FLAG_GET_DEF) {
@@ -1420,7 +1432,8 @@  int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 			goto done;
 	}
 
-	ret = __uvc_query_v4l2_ctrl(chain, ctrl, mapping, v4l2_ctrl);
+	ret = __uvc_query_v4l2_ctrl(chain, ctrl, mapping, v4l2_ctrl,
+				    !(v4l2_ctrl->id & V4L2_CTRL_FLAG_NEXT_CTRL));
 done:
 	mutex_unlock(&chain->ctrl_mutex);
 	return ret;
@@ -1513,7 +1526,7 @@  static void uvc_ctrl_fill_event(struct uvc_video_chain *chain,
 {
 	struct v4l2_queryctrl v4l2_ctrl;
 
-	__uvc_query_v4l2_ctrl(chain, ctrl, mapping, &v4l2_ctrl);
+	__uvc_query_v4l2_ctrl(chain, ctrl, mapping, &v4l2_ctrl, true);
 
 	memset(ev, 0, sizeof(*ev));
 	ev->type = V4L2_EVENT_CTRL;