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