Hi Laurent,
On 09/06/2018 03:31 PM, Laurent Pinchart wrote:
> 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@xxxxxx>
>> ---
>> 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
This is because V4L2_CID_AUTOGAIN & V4L2_CID_GAIN are declared as
auto-cluster:
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,
0, 1023, 1, 0);
[...]
v4l2_ctrl_auto_cluster(2, &ctrls->auto_gain, 0, true);
By checking many other drivers that are using clustered auto controls,
they are all doing that way:
ctrls->auto_x = v4l2_ctrl_new_std(CID_X_AUTO..
ctrls->x = v4l2_ctrl_new_std(CID_X..
v4l2_ctrl_auto_cluster(2, &ctrls->auto, 0, true);
g_volatile_ctrl(ctrl)
switch (ctrl->id) {
case CID_X_AUTO:
ctrls->x->val = READ_REG()
>
> 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.
>
Best regards,
Hugues.
