Hi Niklas, On 04/07/2019 02:58, Niklas Söderlund wrote: > Now that both Gen2 (device centric) and Gen3 (media device centric) s/Gen2 (device centric)/Gen2 (video device centric)/ ? (only if you feel it helps clarify the distinction). > modes of this driver have controls it make sens to call s/sens/sense/ > v4l2_ctrl_handler_setup() unconditionally when opening the video device. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 30 ++++++++++----------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c > index f8b6ec4408b2f5fa..cbf5d8cd6db32d77 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -789,26 +789,26 @@ static int rvin_open(struct file *file) > if (ret) > goto err_unlock; > > - if (vin->info->use_mc) { > + if (vin->info->use_mc) > ret = v4l2_pipeline_pm_use(&vin->vdev.entity, 1); > - if (ret < 0) > - goto err_open; > - } else { > - if (v4l2_fh_is_singular_file(file)) { > - ret = rvin_power_parallel(vin, true); > - if (ret < 0) > - goto err_open; > + else if (v4l2_fh_is_singular_file(file)) > + ret = rvin_power_parallel(vin, true); > + > + if (ret < 0) > + goto err_open; > + > + ret = v4l2_ctrl_handler_setup(&vin->ctrl_handler); > + if (ret) > + goto err_power; > > - ret = v4l2_ctrl_handler_setup(&vin->ctrl_handler); > - if (ret) > - goto err_parallel; > - } > - } > mutex_unlock(&vin->lock); > > return 0; It was already here before this patch, but a new line would be nice here to separate the error paths... Otherwise, Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > -err_parallel: > - rvin_power_parallel(vin, false); > +err_power: > + if (vin->info->use_mc) > + v4l2_pipeline_pm_use(&vin->vdev.entity, 0); > + else if (v4l2_fh_is_singular_file(file)) > + rvin_power_parallel(vin, false); > err_open: > v4l2_fh_release(file); > err_unlock: >