diff mbox series

[v6,09/17] media: uvcvideo: refactor __uvc_ctrl_add_mapping

Message ID 20210317164511.39967-10-ribalda@chromium.org
State Superseded
Headers show
Series uvcvideo: Fix v4l2-compliance errors | expand

Commit Message

Ricardo Ribalda March 17, 2021, 4:45 p.m. UTC
Pass the chain instead of the device. We want to keed the reference to
the chain that controls belong to.

We need to delay the initialization of the controls after the chains
have been initialized.

This is a cleanup needed for the next patches.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c   | 41 ++++++++++++++++++++----------
 drivers/media/usb/uvc/uvc_driver.c |  8 +++---
 2 files changed, 32 insertions(+), 17 deletions(-)

Comments

Hans Verkuil March 18, 2021, 7:23 a.m. UTC | #1
On 17/03/2021 17:45, Ricardo Ribalda wrote:
> Pass the chain instead of the device. We want to keed the reference to

keed -> keep

With that typo fixed you can add my:

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Thanks!

	Hans

> the chain that controls belong to.
> 
> We need to delay the initialization of the controls after the chains
> have been initialized.
> 
> This is a cleanup needed for the next patches.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 41 ++++++++++++++++++++----------
>  drivers/media/usb/uvc/uvc_driver.c |  8 +++---
>  2 files changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b3dde98499f4..b75da65115ef 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2057,7 +2057,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, struct uvc_control *ctrl,
>  /*
>   * Add a control mapping to a given control.
>   */
> -static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
> +static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
>  {
>  	struct uvc_control_mapping *map;
> @@ -2086,7 +2086,7 @@ static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
>  		map->set = uvc_set_le_value;
>  
>  	list_add_tail(&map->list, &ctrl->info.mappings);
> -	uvc_dbg(dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> +	uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
>  		map->name, ctrl->info.entity, ctrl->info.selector);
>  
>  	return 0;
> @@ -2168,7 +2168,7 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  		goto done;
>  	}
>  
> -	ret = __uvc_ctrl_add_mapping(dev, ctrl, mapping);
> +	ret = __uvc_ctrl_add_mapping(chain, ctrl, mapping);
>  	if (ret < 0)
>  		atomic_dec(&dev->nmappings);
>  
> @@ -2244,7 +2244,8 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
>   * Add control information and hardcoded stock control mappings to the given
>   * device.
>   */
> -static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
> +static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> +			       struct uvc_control *ctrl)
>  {
>  	const struct uvc_control_info *info = uvc_ctrls;
>  	const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> @@ -2263,14 +2264,14 @@ static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
>  	for (; info < iend; ++info) {
>  		if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
>  		    ctrl->index == info->index) {
> -			uvc_ctrl_add_info(dev, ctrl, info);
> +			uvc_ctrl_add_info(chain->dev, ctrl, info);
>  			/*
>  			 * Retrieve control flags from the device. Ignore errors
>  			 * and work with default flag values from the uvc_ctrl
>  			 * array when the device doesn't properly implement
>  			 * GET_INFO on standard controls.
>  			 */
> -			uvc_ctrl_get_flags(dev, ctrl, &ctrl->info);
> +			uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
>  			break;
>  		 }
>  	}
> @@ -2281,22 +2282,20 @@ static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
>  	for (; mapping < mend; ++mapping) {
>  		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
>  		    ctrl->info.selector == mapping->selector)
> -			__uvc_ctrl_add_mapping(dev, ctrl, mapping);
> +			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
>  	}
>  }
>  
>  /*
>   * Initialize device controls.
>   */
> -int uvc_ctrl_init_device(struct uvc_device *dev)
> +static int uvc_ctrl_init_chain(struct uvc_video_chain *chain)
>  {
>  	struct uvc_entity *entity;
>  	unsigned int i;
>  
> -	INIT_WORK(&dev->async_ctrl.work, uvc_ctrl_status_event_work);
> -
>  	/* Walk the entities list and instantiate controls */
> -	list_for_each_entry(entity, &dev->entities, list) {
> +	list_for_each_entry(entity, &chain->entities, chain) {
>  		struct uvc_control *ctrl;
>  		unsigned int bControlSize = 0, ncontrols;
>  		u8 *bmControls = NULL;
> @@ -2316,7 +2315,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>  		}
>  
>  		/* Remove bogus/blacklisted controls */
> -		uvc_ctrl_prune_entity(dev, entity);
> +		uvc_ctrl_prune_entity(chain->dev, entity);
>  
>  		/* Count supported controls and allocate the controls array */
>  		ncontrols = memweight(bmControls, bControlSize);
> @@ -2338,7 +2337,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>  			ctrl->entity = entity;
>  			ctrl->index = i;
>  
> -			uvc_ctrl_init_ctrl(dev, ctrl);
> +			uvc_ctrl_init_ctrl(chain, ctrl);
>  			ctrl++;
>  		}
>  	}
> @@ -2346,6 +2345,22 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
>  	return 0;
>  }
>  
> +int uvc_ctrl_init_device(struct uvc_device *dev)
> +{
> +	struct uvc_video_chain *chain;
> +	int ret;
> +
> +	INIT_WORK(&dev->async_ctrl.work, uvc_ctrl_status_event_work);
> +
> +	list_for_each_entry(chain, &dev->chains, list) {
> +		ret = uvc_ctrl_init_chain(chain);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Cleanup device controls.
>   */
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 30ef2a3110f7..35873cf2773d 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2423,14 +2423,14 @@ static int uvc_probe(struct usb_interface *intf,
>  	if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
>  		goto error;
>  
> -	/* Initialize controls. */
> -	if (uvc_ctrl_init_device(dev) < 0)
> -		goto error;
> -
>  	/* Scan the device for video chains. */
>  	if (uvc_scan_device(dev) < 0)
>  		goto error;
>  
> +	/* Initialize controls. */
> +	if (uvc_ctrl_init_device(dev) < 0)
> +		goto error;
> +
>  	/* Register video device nodes. */
>  	if (uvc_register_chains(dev) < 0)
>  		goto error;
>
diff mbox series

Patch

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b3dde98499f4..b75da65115ef 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2057,7 +2057,7 @@  static int uvc_ctrl_add_info(struct uvc_device *dev, struct uvc_control *ctrl,
 /*
  * Add a control mapping to a given control.
  */
-static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
+static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
 {
 	struct uvc_control_mapping *map;
@@ -2086,7 +2086,7 @@  static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
 		map->set = uvc_set_le_value;
 
 	list_add_tail(&map->list, &ctrl->info.mappings);
-	uvc_dbg(dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
+	uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
 		map->name, ctrl->info.entity, ctrl->info.selector);
 
 	return 0;
@@ -2168,7 +2168,7 @@  int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 		goto done;
 	}
 
-	ret = __uvc_ctrl_add_mapping(dev, ctrl, mapping);
+	ret = __uvc_ctrl_add_mapping(chain, ctrl, mapping);
 	if (ret < 0)
 		atomic_dec(&dev->nmappings);
 
@@ -2244,7 +2244,8 @@  static void uvc_ctrl_prune_entity(struct uvc_device *dev,
  * Add control information and hardcoded stock control mappings to the given
  * device.
  */
-static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
+static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
+			       struct uvc_control *ctrl)
 {
 	const struct uvc_control_info *info = uvc_ctrls;
 	const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
@@ -2263,14 +2264,14 @@  static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
 	for (; info < iend; ++info) {
 		if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
 		    ctrl->index == info->index) {
-			uvc_ctrl_add_info(dev, ctrl, info);
+			uvc_ctrl_add_info(chain->dev, ctrl, info);
 			/*
 			 * Retrieve control flags from the device. Ignore errors
 			 * and work with default flag values from the uvc_ctrl
 			 * array when the device doesn't properly implement
 			 * GET_INFO on standard controls.
 			 */
-			uvc_ctrl_get_flags(dev, ctrl, &ctrl->info);
+			uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
 			break;
 		 }
 	}
@@ -2281,22 +2282,20 @@  static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control *ctrl)
 	for (; mapping < mend; ++mapping) {
 		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
 		    ctrl->info.selector == mapping->selector)
-			__uvc_ctrl_add_mapping(dev, ctrl, mapping);
+			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
 	}
 }
 
 /*
  * Initialize device controls.
  */
-int uvc_ctrl_init_device(struct uvc_device *dev)
+static int uvc_ctrl_init_chain(struct uvc_video_chain *chain)
 {
 	struct uvc_entity *entity;
 	unsigned int i;
 
-	INIT_WORK(&dev->async_ctrl.work, uvc_ctrl_status_event_work);
-
 	/* Walk the entities list and instantiate controls */
-	list_for_each_entry(entity, &dev->entities, list) {
+	list_for_each_entry(entity, &chain->entities, chain) {
 		struct uvc_control *ctrl;
 		unsigned int bControlSize = 0, ncontrols;
 		u8 *bmControls = NULL;
@@ -2316,7 +2315,7 @@  int uvc_ctrl_init_device(struct uvc_device *dev)
 		}
 
 		/* Remove bogus/blacklisted controls */
-		uvc_ctrl_prune_entity(dev, entity);
+		uvc_ctrl_prune_entity(chain->dev, entity);
 
 		/* Count supported controls and allocate the controls array */
 		ncontrols = memweight(bmControls, bControlSize);
@@ -2338,7 +2337,7 @@  int uvc_ctrl_init_device(struct uvc_device *dev)
 			ctrl->entity = entity;
 			ctrl->index = i;
 
-			uvc_ctrl_init_ctrl(dev, ctrl);
+			uvc_ctrl_init_ctrl(chain, ctrl);
 			ctrl++;
 		}
 	}
@@ -2346,6 +2345,22 @@  int uvc_ctrl_init_device(struct uvc_device *dev)
 	return 0;
 }
 
+int uvc_ctrl_init_device(struct uvc_device *dev)
+{
+	struct uvc_video_chain *chain;
+	int ret;
+
+	INIT_WORK(&dev->async_ctrl.work, uvc_ctrl_status_event_work);
+
+	list_for_each_entry(chain, &dev->chains, list) {
+		ret = uvc_ctrl_init_chain(chain);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Cleanup device controls.
  */
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 30ef2a3110f7..35873cf2773d 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2423,14 +2423,14 @@  static int uvc_probe(struct usb_interface *intf,
 	if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
 		goto error;
 
-	/* Initialize controls. */
-	if (uvc_ctrl_init_device(dev) < 0)
-		goto error;
-
 	/* Scan the device for video chains. */
 	if (uvc_scan_device(dev) < 0)
 		goto error;
 
+	/* Initialize controls. */
+	if (uvc_ctrl_init_device(dev) < 0)
+		goto error;
+
 	/* Register video device nodes. */
 	if (uvc_register_chains(dev) < 0)
 		goto error;