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