On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
+This is just duplicating the cached value in the control framework. I think this can be dropped.
+ /* cached control settings */
+ int ctrl_cache[OV5640_MAX_CONTROLS];
+This should use v4l2_ctrl_new_std() instead of this array.
+static struct ov5640_control ov5640_ctrls[] = {
+ {
+ .set = ov5640_set_agc,
+ .ctrl = {
+ .id = V4L2_CID_AUTOGAIN,
+ .name = "Auto Gain/Exposure Control",
+ .minimum = 0,
+ .maximum = 1,
+ .step = 1,
+ .default_value = 1,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ },
+ }, {
+ .set = ov5640_set_exposure,
+ .ctrl = {
+ .id = V4L2_CID_EXPOSURE,
+ .name = "Exposure",
+ .minimum = 0,
+ .maximum = 65535,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_gain,
+ .ctrl = {
+ .id = V4L2_CID_GAIN,
+ .name = "Gain",
+ .minimum = 0,
+ .maximum = 1023,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_hue,
+ .ctrl = {
+ .id = V4L2_CID_HUE,
+ .name = "Hue",
+ .minimum = 0,
+ .maximum = 359,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_contrast,
+ .ctrl = {
+ .id = V4L2_CID_CONTRAST,
+ .name = "Contrast",
+ .minimum = 0,
+ .maximum = 255,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_saturation,
+ .ctrl = {
+ .id = V4L2_CID_SATURATION,
+ .name = "Saturation",
+ .minimum = 0,
+ .maximum = 255,
+ .step = 1,
+ .default_value = 64,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_awb,
+ .ctrl = {
+ .id = V4L2_CID_AUTO_WHITE_BALANCE,
+ .name = "Auto White Balance",
+ .minimum = 0,
+ .maximum = 1,
+ .step = 1,
+ .default_value = 1,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ },
+ }, {
+ .set = ov5640_set_red_balance,
+ .ctrl = {
+ .id = V4L2_CID_RED_BALANCE,
+ .name = "Red Balance",
+ .minimum = 0,
+ .maximum = 4095,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ }, {
+ .set = ov5640_set_blue_balance,
+ .ctrl = {
+ .id = V4L2_CID_BLUE_BALANCE,
+ .name = "Blue Balance",
+ .minimum = 0,
+ .maximum = 4095,
+ .step = 1,
+ .default_value = 0,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ },
+ },
+};
+#define OV5640_NUM_CONTROLS ARRAY_SIZE(ov5640_ctrls)
Just put a switch on ctrl->id in s_ctrl, and each case calls the corresponding
set function.
+This does the same as v4l2_ctrl_handler_setup() if I understand the code correctly.
+static int ov5640_restore_ctrls(struct ov5640_dev *sensor)
+{
+ struct ov5640_control *c;
+ int i;
+
+ for (i = 0; i < OV5640_NUM_CONTROLS; i++) {
+ c = &ov5640_ctrls[i];
+ c->set(sensor, sensor->ctrl_cache[i]);
+ }
+
+ return 0;
+}
+As mentioned, just drop the ov5640_ctrls array and call v4l2_ctr_new_std for each
+static int ov5640_init_controls(struct ov5640_dev *sensor)
+{
+ struct ov5640_control *c;
+ int i;
+
+ v4l2_ctrl_handler_init(&sensor->ctrl_hdl, OV5640_NUM_CONTROLS);
+
+ for (i = 0; i < OV5640_NUM_CONTROLS; i++) {
+ c = &ov5640_ctrls[i];
+
+ v4l2_ctrl_new_std(&sensor->ctrl_hdl, &ov5640_ctrl_ops,
+ c->ctrl.id, c->ctrl.minimum, c->ctrl.maximum,
+ c->ctrl.step, c->ctrl.default_value);
+ }
control you're adding.
+Same comments apply to the next patch, so I won't repeat them.
+module_i2c_driver(ov5640_i2c_driver);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>");
+MODULE_DESCRIPTION("OV5640 MIPI Camera Subdev Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");