Hi Hans,
On 28 September 2012 10:23, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> On Fri September 28 2012 09:48:01 Javier Martin wrote:
>> static const struct v4l2_subdev_core_ops ov7670_core_ops = {
>> .g_chip_ident = ov7670_g_chip_ident,
>> - .g_ctrl = ov7670_g_ctrl,
>> - .s_ctrl = ov7670_s_ctrl,
>> - .queryctrl = ov7670_queryctrl,
>> + .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
>> + .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
>> + .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
>> + .g_ctrl = v4l2_subdev_g_ctrl,
>> + .s_ctrl = v4l2_subdev_s_ctrl,
>> + .queryctrl = v4l2_subdev_queryctrl,
>> + .querymenu = v4l2_subdev_querymenu,
>
> Can you make a fourth patch removing these lines again after mcam-core and
> via-camera are converted to the control framework? These control ops are
> only needed if there are bridge drivers using this sensor driver that are
> not yet converted to the control framework. Since that limitation is now
> lifted, these ops can be removed as well.
Fine, I will do that.
>> .reset = ov7670_reset,
>> .init = ov7670_init,
>> #ifdef CONFIG_VIDEO_ADV_DEBUG
>> @@ -1732,7 +1630,6 @@ static int ov7670_probe(struct i2c_client *client,
>>
>> info->devtype = &ov7670_devdata[id->driver_data];
>> info->fmt = &ov7670_formats[0];
>> - info->sat = 128; /* Review this */
>> info->clkrc = 0;
>>
>> /* Set default frame rate to 30 fps */
>> @@ -1743,6 +1640,42 @@ static int ov7670_probe(struct i2c_client *client,
>> if (info->pclk_hb_disable)
>> ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
>>
>> + v4l2_ctrl_handler_init(&info->hdl, 10);
>> + v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
>> + v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_CONTRAST, 0, 127, 1, 64);
>> + v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_VFLIP, 0, 1, 1, 0);
>> + v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_HFLIP, 0, 1, 1, 0);
>> + info->saturation = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_SATURATION, 0, 256, 1, 128);
>> + info->hue = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_HUE, -180, 180, 5, 0);
>> + info->gain = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_GAIN, 0, 255, 1, 128);
>> + info->auto_gain = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
>> + info->exposure = v4l2_ctrl_new_std(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_EXPOSURE, 0, 65535, 1, 500);
>> + info->auto_exposure = v4l2_ctrl_new_std_menu(&info->hdl, &ov7670_ctrl_ops,
>> + V4L2_CID_EXPOSURE_AUTO, 1, 0, 0);
>> + sd->ctrl_handler = &info->hdl;
>> + if (info->hdl.error) {
>> + int err = info->hdl.error;
>> +
>> + v4l2_ctrl_handler_free(&info->hdl);
>> + kfree(info);
>> + return err;
>> + }
>> + info->gain->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> + info->exposure->flags |= V4L2_CTRL_FLAG_VOLATILE;
>> + v4l2_ctrl_cluster(2, &info->auto_gain);
>> + v4l2_ctrl_cluster(2, &info->auto_exposure);
>
> You need to use v4l2_ctrl_auto_cluster for these two. Not having that function
> at the time was the reason my work on this driver stalled. The auto cluster
> functionality takes care of most of the nitty gritty details of handling
> these situations.
OK, let me take a look.
Regards.
--
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
