Re: [PATCH v2 3/4] media: ov2685: add support for OV2685 sensor

[Shunqian Zheng <zhengsq@rock-chips.com>]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

Hi Sakari,


On 2018年01月03日 19:43, Sakari Ailus wrote:
>> +static int ov2685_s_stream(struct v4l2_subdev *sd, int on)
>> +{
>> +	struct ov2685 *ov2685 = to_ov2685(sd);
>> +	struct i2c_client *client = ov2685->client;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&ov2685->mutex);
>> +
>> +	on = !!on;
>> +	if (on == ov2685->streaming)
>> +		goto unlock_and_return;
>> +
>> +	if (on) {
>> +		/* In case these controls are set before streaming */
>> +		ov2685_set_exposure(ov2685, ov2685->exposure->val);
>> +		ov2685_set_gain(ov2685, ov2685->anal_gain->val);
>> +		ov2685_set_vts(ov2685, ov2685->vblank->val);
>> +		ov2685_enable_test_pattern(ov2685, ov2685->test_pattern->val);
> You should use __v4l2_ctrl_handler_setup() here. Or put that to the
> driver's runtime_resume function. That actually might be better.
The v3 put __v4l2_ctrl_handler_setup() to the runtime_resume callback. But
ov2685_s_stream()
    -> pm_runtime_get_sync()
        -> ov2685_runtime_resume()
             -> __v4l2_ctrl_handler_setup()
                 -> pm_runtime_get_if_in_use(), always <= 0 because 
dev->power.runtime_status != RPM_ACTIVE.

Seems like  __v4l2_ctrl_handler_setup() has to be in ov2685_s_stream().

Thanks
>> +
>> +		ret = ov2685_write_reg(client, REG_SC_CTRL_MODE,
>> +				OV2685_REG_VALUE_08BIT, SC_CTRL_MODE_STREAMING);
>> +		if (ret)
>> +			goto unlock_and_return;
>> +	} else {
>> +		ret = ov2685_write_reg(client, REG_SC_CTRL_MODE,
>> +				OV2685_REG_VALUE_08BIT, SC_CTRL_MODE_STANDBY);
>> +		if (ret)
>> +			goto unlock_and_return;
>> +	}
>> +
>> +	ov2685->streaming = on;
>> +
>> +unlock_and_return:
>> +	mutex_unlock(&ov2685->mutex);
>> +	return ret;
>> +}


[-- Attachment #2: Type: text/html, Size: 2485 bytes --]