Re: [PATCH v2 02/12] media: ov5640: Add light frequency control

[Maxime Ripard <maxime.ripard@bootlin.com>]

Hi Laurent,

On Thu, Apr 19, 2018 at 12:44:18PM +0300, Laurent Pinchart wrote:
> On Monday, 16 April 2018 15:36:51 EEST Maxime Ripard wrote:
> > From: Mylène Josserand <mylene.josserand@bootlin.com>
> > 
> > Add the light frequency control to be able to set the frequency
> > to manual (50Hz or 60Hz) or auto.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/media/i2c/ov5640.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index a33e45f8e2b0..28122341fc35 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -189,6 +189,7 @@ struct ov5640_ctrls {
> >  	};
> >  	struct v4l2_ctrl *auto_focus;
> >  	struct v4l2_ctrl *brightness;
> > +	struct v4l2_ctrl *light_freq;
> >  	struct v4l2_ctrl *saturation;
> >  	struct v4l2_ctrl *contrast;
> >  	struct v4l2_ctrl *hue;
> > @@ -2163,6 +2164,21 @@ static int ov5640_set_ctrl_focus(struct ov5640_dev
> > *sensor, int value) BIT(1), value ? BIT(1) : 0);
> >  }
> > 
> > +static int ov5640_set_ctl_light_freq(struct ov5640_dev *sensor, int value)
> 
> To stay consistent with the other functions, I propose calling this 
> ov5640_set_ctrl_light_freq().
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Consider it fixed in the next iteration, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com