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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxx>
> ---
>  .../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
>
>




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux