Re: [PATCH v3 14/18] media: i2c: Add driver for DW9719 VCM

Sakari Ailus
2023-07-06T09:30:20Z

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