[PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls

Ricardo Ribalda posted 4 patches 1 month, 2 weeks ago
[PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls
Posted by Ricardo Ribalda 1 month, 2 weeks ago
The function uvc_ctrl_init_ctrl() is called for every control for every
entity, but it exits early if the entity is a extension unit. The comment
claims that this is done to avoid querying XU controls during probe.

We only query a control if its entity GUIDs and index matches the
uvc_ctrls list. There are only controls for the following GUIDs:
UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
UVC_GUID_EXT_GPIO_CONTROLLER.

In other words, XU controls will not be queried even without this
condition.

In future patches we want to add ChromeOS XU controls that need to the
initialized. We will make sure that all cameras with ChromeOS XU can
be queried at probe time.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 {
 	unsigned int i;
 
-	/*
-	 * XU controls initialization requires querying the device for control
-	 * information. As some buggy UVC devices will crash when queried
-	 * repeatedly in a tight loop, delay XU controls initialization until
-	 * first use.
-	 */
-	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
 		const struct uvc_control_info *info = &uvc_ctrls[i];
 

-- 
2.51.0.rc1.167.g924127e9c0-goog
Re: [PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls
Posted by Laurent Pinchart 2 weeks, 6 days ago
Hi Ricardo,

Thank you for the patch.

On Mon, Aug 18, 2025 at 08:15:38PM +0000, Ricardo Ribalda wrote:
> The function uvc_ctrl_init_ctrl() is called for every control for every
> entity, but it exits early if the entity is a extension unit. The comment
> claims that this is done to avoid querying XU controls during probe.
> 
> We only query a control if its entity GUIDs and index matches the
> uvc_ctrls list. There are only controls for the following GUIDs:
> UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
> UVC_GUID_EXT_GPIO_CONTROLLER.
> 
> In other words, XU controls will not be queried even without this
> condition.

Looking at the commit that introduced this code

commit 52c58ad6f95ff60343bf0c517182d5f649ca0403
Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date:   Wed Sep 29 16:03:03 2010 -0300

    [media] uvcvideo: Delay initialization of XU controls

we see it contains the following change

-       /* Query XU controls for control information */
-       if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT) {
-               struct uvc_control_info info;
-               int ret;
-
-               ret = uvc_ctrl_fill_xu_info(dev, ctrl, &info);
-               if (ret < 0)
-                       return;
-
-               ret = uvc_ctrl_add_info(dev, ctrl, &info);
-               if (ret < 0) {
-                       /* Skip the control */
-                       uvc_trace(UVC_TRACE_CONTROL, "Failed to initialize "
-                               "control %pUl/%u on device %s entity %u\n",
-                               info.entity, info.selector, dev->udev->devpath,
-                               ctrl->entity->id);
-                       memset(ctrl, 0, sizeof(*ctrl));
-               }
+       /* XU controls initialization requires querying the device for control
+        * information. As some buggy UVC devices will crash when queried
+        * repeatedly in a tight loop, delay XU controls initialization until
+        * first use.
+        */
+       if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
                return;
-       }


This is followed by

        for (; info < iend; ++info) {
                if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
                    ctrl->index == info->index) {
                        uvc_ctrl_add_info(dev, ctrl, info);
                        break;
                 }
        }

        if (!ctrl->initialized)
                return;

(this loops over uvc_ctrls). As uvc_ctrls doesn't contain any non-XU
control, you're right that the function never calls uvc_ctrl_add_info()
for XU controls. The ctrl->initialized check then causes the function to
return, as uvc_ctrl_add_info() wasn't called and initialized is false.
The entity type check was therefore not needed, and can be removed.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> In future patches we want to add ChromeOS XU controls that need to the
> initialized. We will make sure that all cameras with ChromeOS XU can
> be queried at probe time.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  {
>  	unsigned int i;
>  
> -	/*
> -	 * XU controls initialization requires querying the device for control
> -	 * information. As some buggy UVC devices will crash when queried
> -	 * repeatedly in a tight loop, delay XU controls initialization until
> -	 * first use.
> -	 */
> -	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> -		return;
> -
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
>  		const struct uvc_control_info *info = &uvc_ctrls[i];
>  

-- 
Regards,

Laurent Pinchart
Re: [PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls
Posted by Hans de Goede 3 weeks, 4 days ago
Hi,

On 18-Aug-25 22:15, Ricardo Ribalda wrote:
> The function uvc_ctrl_init_ctrl() is called for every control for every
> entity, but it exits early if the entity is a extension unit. The comment
> claims that this is done to avoid querying XU controls during probe.
> 
> We only query a control if its entity GUIDs and index matches the
> uvc_ctrls list. There are only controls for the following GUIDs:
> UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
> UVC_GUID_EXT_GPIO_CONTROLLER.
> 
> In other words, XU controls will not be queried even without this
> condition.
> 
> In future patches we want to add ChromeOS XU controls that need to the
> initialized. We will make sure that all cameras with ChromeOS XU can
> be queried at probe time.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  {
>  	unsigned int i;
>  
> -	/*
> -	 * XU controls initialization requires querying the device for control
> -	 * information. As some buggy UVC devices will crash when queried
> -	 * repeatedly in a tight loop, delay XU controls initialization until
> -	 * first use.
> -	 */
> -	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> -		return;
> -
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
>  		const struct uvc_control_info *info = &uvc_ctrls[i];
>  
>