Hi Hans,
On Mon, Apr 15, 2024 at 01:58:53PM +0200, Hans de Goede wrote:
> Hi Sakari,
>
> On 4/15/24 1:36 PM, Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Mon, Apr 15, 2024 at 11:37:04AM +0200, Hans de Goede wrote:
> >> Add camera orientation and sensor rotation controls using
> >> the v4l2_fwnode_device_parse() and v4l2_ctrl_new_fwnode_properties()
> >> helpers.
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> >> ---
> >> drivers/media/i2c/ov2680.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> >> index 14a5ac2bbf8b..840e6b63ad19 100644
> >> --- a/drivers/media/i2c/ov2680.c
> >> +++ b/drivers/media/i2c/ov2680.c
> >> @@ -946,6 +946,7 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor)
> >> const struct v4l2_ctrl_ops *ops = &ov2680_ctrl_ops;
> >> struct ov2680_ctrls *ctrls = &sensor->ctrls;
> >> struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> >> + struct v4l2_fwnode_device_properties props;
> >> int def, max, ret = 0;
> >>
> >> v4l2_i2c_subdev_init(&sensor->sd, client, &ov2680_subdev_ops);
> >> @@ -1000,6 +1001,14 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor)
> >> goto cleanup_entity;
> >> }
> >>
> >> + ret = v4l2_fwnode_device_parse(sensor->dev, &props);
> >> + if (ret)
> >> + goto cleanup_entity;
> >> +
> >> + ret = v4l2_ctrl_new_fwnode_properties(hdl, ops, &props);
> >> + if (ret)
> >> + goto cleanup_entity;
> >
> > As discussed with Umang recently, you can postpone
> > v4l2_ctrl_new_fwnode_properties() error check until the control handler. Up
> > to you.
>
> So you mean move this up to above the
>
> if (hdl->error) {
> ret = hdl->error;
> goto cleanup_entity;
> }
>
> check and then drop error handling for the v4l2_ctrl_new_fwnode_properties()
> call since any errors there will set hdl->error, right ?
Correct.
>
> That sounds like a nice cleanup. I'll do that for v2.
Thanks!
--
Regards,
Sakari Ailus
