[RFC,v1,17/19] media: renesas: vsp1: Initialize control handler after subdev

Message ID 20231122043009.2741-18-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Headers
Series media: renesas: vsp1: Conversion to subdev active state |

Commit Message

Laurent Pinchart Nov. 22, 2023, 4:30 a.m. UTC
  Some VSP modules initialize their control handler after initializing the
subdev, while some initialize it before. This makes the code
inconsistent and more error prone. Standardize on control initialization
after initializing the subdev.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/platform/renesas/vsp1/vsp1_hgo.c    | 20 +++++++++----------
 .../media/platform/renesas/vsp1/vsp1_hgt.c    | 12 +++++------
 2 files changed, 16 insertions(+), 16 deletions(-)
  

Comments

Jacopo Mondi June 18, 2024, 4:56 p.m. UTC | #1
Hi Laurent

On Wed, Nov 22, 2023 at 06:30:07AM GMT, Laurent Pinchart wrote:
> Some VSP modules initialize their control handler after initializing the
> subdev, while some initialize it before. This makes the code
> inconsistent and more error prone. Standardize on control initialization
> after initializing the subdev.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The existing code doesn't check for errors after having created
controls. Anyway, if this will ever be done on top, it will be enough
to call vsp1_entity_destroy() in case of errors as it clears up the
control handler already.

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../media/platform/renesas/vsp1/vsp1_hgo.c    | 20 +++++++++----------
>  .../media/platform/renesas/vsp1/vsp1_hgt.c    | 12 +++++------
>  2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
> index 237dc4c7c5ed..21cffe6947a2 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
> @@ -192,6 +192,16 @@ struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
>  	if (hgo == NULL)
>  		return ERR_PTR(-ENOMEM);
>
> +	/* Initialize the video device and queue for statistics data. */
> +	ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
> +				  &hgo_entity_ops, hgo_mbus_formats,
> +				  ARRAY_SIZE(hgo_mbus_formats),
> +				  HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
> +	if (ret < 0) {
> +		vsp1_entity_destroy(&hgo->histo.entity);
> +		return ERR_PTR(ret);
> +	}
> +
>  	/* Initialize the control handler. */
>  	v4l2_ctrl_handler_init(&hgo->ctrls.handler,
>  			       vsp1->info->gen >= 3 ? 2 : 1);
> @@ -207,15 +217,5 @@ struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
>
>  	hgo->histo.entity.subdev.ctrl_handler = &hgo->ctrls.handler;
>
> -	/* Initialize the video device and queue for statistics data. */
> -	ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
> -				  &hgo_entity_ops, hgo_mbus_formats,
> -				  ARRAY_SIZE(hgo_mbus_formats),
> -				  HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
> -	if (ret < 0) {
> -		vsp1_entity_destroy(&hgo->histo.entity);
> -		return ERR_PTR(ret);
> -	}
> -
>  	return hgo;
>  }
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
> index b73eac676ef0..a447ed1c59c3 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
> @@ -191,12 +191,6 @@ struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
>  	if (hgt == NULL)
>  		return ERR_PTR(-ENOMEM);
>
> -	/* Initialize the control handler. */
> -	v4l2_ctrl_handler_init(&hgt->ctrls, 1);
> -	v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
> -
> -	hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
> -
>  	/* Initialize the video device and queue for statistics data. */
>  	ret = vsp1_histogram_init(vsp1, &hgt->histo, VSP1_ENTITY_HGT, "hgt",
>  				  &hgt_entity_ops, hgt_mbus_formats,
> @@ -207,6 +201,12 @@ struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
>  		return ERR_PTR(ret);
>  	}
>
> +	/* Initialize the control handler. */
> +	v4l2_ctrl_handler_init(&hgt->ctrls, 1);
> +	v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
> +
> +	hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
> +
>  	v4l2_ctrl_handler_setup(&hgt->ctrls);
>
>  	return hgt;
> --
> Regards,
>
> Laurent Pinchart
>
>
  

Patch

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
index 237dc4c7c5ed..21cffe6947a2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgo.c
@@ -192,6 +192,16 @@  struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
 	if (hgo == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	/* Initialize the video device and queue for statistics data. */
+	ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
+				  &hgo_entity_ops, hgo_mbus_formats,
+				  ARRAY_SIZE(hgo_mbus_formats),
+				  HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
+	if (ret < 0) {
+		vsp1_entity_destroy(&hgo->histo.entity);
+		return ERR_PTR(ret);
+	}
+
 	/* Initialize the control handler. */
 	v4l2_ctrl_handler_init(&hgo->ctrls.handler,
 			       vsp1->info->gen >= 3 ? 2 : 1);
@@ -207,15 +217,5 @@  struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1)
 
 	hgo->histo.entity.subdev.ctrl_handler = &hgo->ctrls.handler;
 
-	/* Initialize the video device and queue for statistics data. */
-	ret = vsp1_histogram_init(vsp1, &hgo->histo, VSP1_ENTITY_HGO, "hgo",
-				  &hgo_entity_ops, hgo_mbus_formats,
-				  ARRAY_SIZE(hgo_mbus_formats),
-				  HGO_DATA_SIZE, V4L2_META_FMT_VSP1_HGO);
-	if (ret < 0) {
-		vsp1_entity_destroy(&hgo->histo.entity);
-		return ERR_PTR(ret);
-	}
-
 	return hgo;
 }
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
index b73eac676ef0..a447ed1c59c3 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hgt.c
@@ -191,12 +191,6 @@  struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
 	if (hgt == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	/* Initialize the control handler. */
-	v4l2_ctrl_handler_init(&hgt->ctrls, 1);
-	v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
-
-	hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
-
 	/* Initialize the video device and queue for statistics data. */
 	ret = vsp1_histogram_init(vsp1, &hgt->histo, VSP1_ENTITY_HGT, "hgt",
 				  &hgt_entity_ops, hgt_mbus_formats,
@@ -207,6 +201,12 @@  struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1)
 		return ERR_PTR(ret);
 	}
 
+	/* Initialize the control handler. */
+	v4l2_ctrl_handler_init(&hgt->ctrls, 1);
+	v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL);
+
+	hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls;
+
 	v4l2_ctrl_handler_setup(&hgt->ctrls);
 
 	return hgt;