Hi Laurent,
On Monday 09 November 2015 16:25:02 Markus Pargmann wrote:
> On Monday 09 November 2015 15:22:06 Laurent Pinchart wrote:
[...]
> >
> > Please use proper controls names.
>
> Sorry I don't really know what you mean? For me these are proper names.
Could you give me a hint how these names should look like?
Thanks,
Markus
>
> >
> > > + .min = 1,
> > > + .max = 64,
> > > + .step = 1,
> > > + .def = 58,
> > > + .flags = 0,
> > > +};
> > > +
> > > +static const struct v4l2_ctrl_config mt9v032_aec_lpf = {
> > > + .ops = &mt9v032_ctrl_ops,
> > > + .id = V4L2_CID_AEC_LPF,
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .name = "aec_lpf",
> > > + .min = 0,
> > > + .max = 2,
> > > + .step = 1,
> > > + .def = 0,
> > > + .flags = 0,
> > > +};
> > > +
> > > +static const struct v4l2_ctrl_config mt9v032_agc_lpf = {
> > > + .ops = &mt9v032_ctrl_ops,
> > > + .id = V4L2_CID_AGC_LPF,
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .name = "agc_lpf",
> > > + .min = 0,
> > > + .max = 2,
> > > + .step = 1,
> > > + .def = 2,
> > > + .flags = 0,
> > > +};
> > > +
> > > +static const struct v4l2_ctrl_config mt9v032_aec_update_interval = {
> > > + .ops = &mt9v032_ctrl_ops,
> > > + .id = V4L2_CID_AEC_UPDATE_INTERVAL,
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .name = "aec_update_interval",
> > > + .min = 0,
> > > + .max = 16,
> > > + .step = 1,
> > > + .def = 2,
> > > + .flags = 0,
> > > +};
> > > +
> > > +static const struct v4l2_ctrl_config mt9v032_agc_update_interval = {
> > > + .ops = &mt9v032_ctrl_ops,
> > > + .id = V4L2_CID_AGC_UPDATE_INTERVAL,
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .name = "agc_update_interval",
> > > + .min = 0,
> > > + .max = 16,
> > > + .step = 1,
> > > + .def = 2,
> > > + .flags = 0,
> > > +};
> > > +
> > > +static const struct v4l2_ctrl_config mt9v032_aec_max_shutter_width = {
> > > + .ops = &mt9v032_ctrl_ops,
> > > + .id = V4L2_CID_AEC_MAX_SHUTTER_WIDTH,
> > > + .type = V4L2_CTRL_TYPE_INTEGER,
> > > + .name = "aec_max_shutter_width",
> > > + .min = 1,
> > > + .max = MT9V034_TOTAL_SHUTTER_WIDTH_MAX,
> >
> > Isn't the maximum value 2047 for the MT9V0[23]2 ?
>
> Oh right, these differ by 2. Not really much but will fix it.
>
> >
> > > + .step = 1,
> > > + .def = MT9V032_TOTAL_SHUTTER_WIDTH_DEF,
> > > + .flags = 0,
> > > +};
> > > +
> > > /*
> > > ---------------------------------------------------------------------------
> > > -- * V4L2 subdev core operations
> > > */
> > > @@ -1010,6 +1147,22 @@ static int mt9v032_probe(struct i2c_client *client,
> > > mt9v032_test_pattern_menu);
> > > mt9v032->test_pattern_color = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > &mt9v032_test_pattern_color, NULL);
> > > + mt9v032->desired_bin = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_desired_bin,
> > > + NULL);
> > > + mt9v032->aec_lpf = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_aec_lpf, NULL);
> > > + mt9v032->agc_lpf = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_agc_lpf, NULL);
> > > + mt9v032->aec_update_interval = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_aec_update_interval,
> > > + NULL);
> > > + mt9v032->agc_update_interval = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_agc_update_interval,
> > > + NULL);
> > > + mt9v032->aec_max_shutter_width = v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > > + &mt9v032_aec_max_shutter_width,
> > > + NULL);
> >
> > As there's no need to store the control pointers I would create an array of
> > struct v4l2_ctrl_config above instead of defining one variable per control,
> > and then loop over the array here.
> >
> > for (i = 0; i < ARRAY_SIZE(mt9v032_aegc_controls); ++i)
> > v4l2_ctrl_new_custom(&mt9v032->ctrls,
> > &mt9v032_aegc_controls[i]);
> >
> > You should also update the above v4l2_ctrl_handler_init() call to take the new
> > controls into account, as that will improve performances of the control
> > framework.
> >
> > v4l2_ctrl_handler_init(&mt9v032->ctrls,
> > 10 + ARRAY_SIZE(mt9v032_aegc_controls));
>
> Fixed as well. Will send a new version as soon as the proper naming is clear to
> me.
>
> Thanks,
>
> Markus
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: This is a digitally signed message part.
