Hi Sakari, Thank you for the patch. On Tue, Apr 16, 2024 at 10:33:11PM +0300, Sakari Ailus wrote: > Remove two variables in ov2740_init_control() that are used as a shorthand > for where the information is really located. Make the code more readable > by removing them. Dropping size is nice. I don't know if removing cur_mode makes the code more readable, but if it does for you, Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > --- > drivers/media/i2c/ov2740.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c > index 57906df7be4e..196a111516b0 100644 > --- a/drivers/media/i2c/ov2740.c > +++ b/drivers/media/i2c/ov2740.c > @@ -754,10 +754,8 @@ static const struct v4l2_ctrl_ops ov2740_ctrl_ops = { > static int ov2740_init_controls(struct ov2740 *ov2740) > { > struct v4l2_ctrl_handler *ctrl_hdlr; > - const struct ov2740_mode *cur_mode; > s64 exposure_max, h_blank, pixel_rate; > u32 vblank_min, vblank_max, vblank_default; > - int size; > int ret; > > ctrl_hdlr = &ov2740->ctrl_handler; > @@ -765,12 +763,10 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > if (ret) > return ret; > > - cur_mode = ov2740->cur_mode; > - size = ARRAY_SIZE(link_freq_menu_items); > - > ov2740->link_freq = > v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov2740_ctrl_ops, > - V4L2_CID_LINK_FREQ, size - 1, > + V4L2_CID_LINK_FREQ, > + ARRAY_SIZE(link_freq_menu_items) - 1, > ov2740->supported_modes->link_freq_index, > link_freq_menu_items); > if (ov2740->link_freq) > @@ -781,14 +777,14 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > V4L2_CID_PIXEL_RATE, 0, > pixel_rate, 1, pixel_rate); > > - vblank_min = cur_mode->vts_min - cur_mode->height; > - vblank_max = cur_mode->vts_max - cur_mode->height; > - vblank_default = cur_mode->vts_def - cur_mode->height; > + vblank_min = ov2740->cur_mode->vts_min - ov2740->cur_mode->height; > + vblank_max = ov2740->cur_mode->vts_max - ov2740->cur_mode->height; > + vblank_default = ov2740->cur_mode->vts_def - ov2740->cur_mode->height; > ov2740->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops, > V4L2_CID_VBLANK, vblank_min, > vblank_max, 1, vblank_default); > > - h_blank = cur_mode->hts - cur_mode->width; > + h_blank = ov2740->cur_mode->hts - ov2740->cur_mode->width; > ov2740->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops, > V4L2_CID_HBLANK, h_blank, h_blank, 1, > h_blank); > @@ -801,7 +797,7 @@ static int ov2740_init_controls(struct ov2740 *ov2740) > v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops, V4L2_CID_DIGITAL_GAIN, > OV2740_DGTL_GAIN_MIN, OV2740_DGTL_GAIN_MAX, > OV2740_DGTL_GAIN_STEP, OV2740_DGTL_GAIN_DEFAULT); > - exposure_max = cur_mode->vts_def - OV2740_EXPOSURE_MAX_MARGIN; > + exposure_max = ov2740->cur_mode->vts_def - OV2740_EXPOSURE_MAX_MARGIN; > ov2740->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops, > V4L2_CID_EXPOSURE, > OV2740_EXPOSURE_MIN, exposure_max, -- Regards, Laurent Pinchart