public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Hugues Fruchet <hugues.fruchet@st.com>
Cc: Steve Longerbeam <slongerbeam@gmail.com>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	Jacopo Mondi <jacopo@jmondi.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>
Subject: Re: [PATCH v2 4/5] media: ov5640: fix auto controls values when switching to manual mode
Date: Thu, 06 Sep 2018 16:31:09 +0300	[thread overview]
Message-ID: <1747395.R2Yra0TKY1@avalon> (raw)
In-Reply-To: <1534155586-26974-5-git-send-email-hugues.fruchet@st.com>

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

  reply	other threads:[~2018-09-06 18:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-13 10:19 [PATCH v2 0/5] Fix OV5640 exposure & gain Hugues Fruchet
2018-08-13 10:19 ` [PATCH v2 1/5] media: ov5640: fix exposure regression Hugues Fruchet
2018-09-06 13:31   ` Laurent Pinchart
2018-08-13 10:19 ` [PATCH v2 2/5] media: ov5640: fix auto gain & exposure when changing mode Hugues Fruchet
2018-08-13 10:19 ` [PATCH v2 3/5] media: ov5640: fix wrong binning value in exposure calculation Hugues Fruchet
2018-09-06 13:23   ` Laurent Pinchart
2018-08-13 10:19 ` [PATCH v2 4/5] media: ov5640: fix auto controls values when switching to manual mode Hugues Fruchet
2018-09-06 13:31   ` Laurent Pinchart [this message]
2018-09-10 10:23     ` Hugues FRUCHET
2018-09-10 10:46       ` Laurent Pinchart
2018-09-10 14:43         ` Hugues FRUCHET
2018-09-10 20:35           ` Laurent Pinchart
2018-08-13 10:19 ` [PATCH v2 5/5] media: ov5640: fix restore of last mode set Hugues Fruchet
2018-08-16 10:10   ` jacopo mondi
2018-08-16 15:07     ` Hugues FRUCHET
2018-08-16 19:53       ` jacopo mondi
2018-09-07 14:18       ` Laurent Pinchart
2018-09-10 15:14         ` Hugues FRUCHET
2018-09-10 20:56           ` Laurent Pinchart
2018-09-11  8:26             ` Hugues FRUCHET
2018-08-25 14:53   ` jacopo mondi
2018-09-11  7:32     ` Hugues FRUCHET

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1747395.R2Yra0TKY1@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=benjamin.gaignard@linaro.org \
    --cc=hugues.fruchet@st.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jacopo@jmondi.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@iki.fi \
    --cc=slongerbeam@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox