Hi Laurent On Wed, Nov 23, 2022 at 11:54:07AM +0200, Laurent Pinchart wrote: > From: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx> > > The ov5640 driver incorrectly uses V4L2_CID_GAIN for the analogue gain. > V4L2 has a specific control for analogue gain, V4L2_CID_ANALOGUE_GAIN. > Use it. > > Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> It's a bit of a shame we cannot report here your analysis of the gain control handling on ov5640, so I'll link it here for reference https://lists.libcamera.org/pipermail/libcamera-devel/2022-November/035655.html I agree with the above conclusions, until proven differently we can consider 0x350a/b to control the analogue gain, as there's a separate digital gain register, hence the below change makes sense to me. I've expanded the cc-list to who has been recently involved in ov5640 developments. If required by any user, we should try to map CID_GAIN on CID_ANALOGUE_GAIN for compatibility. >From my side it's good the way it is: Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > --- > drivers/media/i2c/ov5640.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 2d740397a5d4..c65c391bc1eb 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -3458,7 +3458,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor) > /* Auto/manual gain */ > ctrls->auto_gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTOGAIN, > 0, 1, 1, 1); > - ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN, > + ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, > 0, 1023, 1, 0); > > ctrls->saturation = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, > -- > Regards, > > Laurent Pinchart >
