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 ? That sounds like a nice cleanup. I'll do that for v2. Regards, Hans > >> + >> ctrls->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; >> ctrls->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; >> ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; >