Hi Andy, On Thu, Jul 06, 2023 at 12:14:27PM +0300, Andy Shevchenko wrote: > On Thu, Jul 06, 2023 at 07:47:38AM +0000, Sakari Ailus wrote: > > On Wed, Jul 05, 2023 at 11:30:06PM +0200, Hans de Goede wrote: > > ... > > > > +static int dw9719_init_controls(struct dw9719_device *dw9719) > > > +{ > > > + const struct v4l2_ctrl_ops *ops = &dw9719_ctrl_ops; > > > + int ret; > > > + > > > + ret = v4l2_ctrl_handler_init(&dw9719->ctrls.handler, 1); > > > + if (ret) > > > + return ret; > > > > This check can be dropped. > > The obvious question why that API returns int instead of void? I guess it's for historical reasons. The control handler initialisation functions won't do anything if the handler is in error state. I guess this could be changed. Cc Hans. > > > > + dw9719->ctrls.focus = v4l2_ctrl_new_std(&dw9719->ctrls.handler, ops, > > > + V4L2_CID_FOCUS_ABSOLUTE, 0, > > > + DW9719_MAX_FOCUS_POS, 1, 0); > > > + > > > + if (dw9719->ctrls.handler.error) { > > > + dev_err(dw9719->dev, "Error initialising v4l2 ctrls\n"); > > > + ret = dw9719->ctrls.handler.error; > > > + goto err_free_handler; > > > + } > > > + > > > + dw9719->sd.ctrl_handler = &dw9719->ctrls.handler; > > > > + return ret; > > return 0; > > ? Makes sense. > > > > +err_free_handler: > > > + v4l2_ctrl_handler_free(&dw9719->ctrls.handler); > > > + return ret; > > > +} > -- Kind regards, Sakari Ailus