Re: [PATCH v2 4/5] media: ov5640: fix auto controls values when switching to manual mode

Laurent Pinchart
2018-09-06T18:06:41Z

Hi Hugues,

Thank you for the patch.

On Monday, 13 August 2018 13:19:45 EEST Hugues Fruchet wrote:
> When switching from auto to manual mode, V4L2 core is calling
> g_volatile_ctrl() in manual mode in order to get the manual initial value.
> Remove the manual mode check/return to not break this behaviour.
>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
> drivers/media/i2c/ov5640.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 9fb17b5..c110a6a 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2277,16 +2277,12 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl
> *ctrl)
>
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> - if (!ctrl->val)
> - return 0;
> val = ov5640_get_gain(sensor);
> if (val < 0)
> return val;
> sensor->ctrls.gain->val = val;
> break;

What is this even supposed to do ? Only the V4L2_CID_GAIN and
V4L2_CID_EXPOSURE have the volatile flag set. Why can't this code be replaced
with

case V4L2_CID_GAIN:
val = ov5640_get_gain(sensor);
if (val < 0)
return val;
ctrl->val = val;
break;

> case V4L2_CID_EXPOSURE_AUTO:
> - if (ctrl->val == V4L2_EXPOSURE_MANUAL)
> - return 0;
> val = ov5640_get_exposure(sensor);
> if (val < 0)
> return val;

And same here.

--
Regards,

Laurent Pinchart