V4L/DVB (13137): gspca_mr97310a: Add controls for vga cams with sensor type 0

This patch adds controls for vga cams with sensor type 0, in order to
correctly report the present controls, the probing of the sensor type
has been moved from sd_start to sd_config, since this made the sensor
type probing unreliable the detection method was changed.

Note this requires the camera to enter streaming mode, so sd_config now
briefly makes the camera stream.

Signed-off-by: Theodore Kilgore <kilgota@banach.math.auburn.edu>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

1 files changed, 291 insertions, 215 deletions

diff --git a/drivers/media/video/gspca/mr97310a.c b/drivers/media/video/gspca/mr97310a.c
index f8328b9efae5..f4c83b367900 100644
--- a/drivers/media/video/gspca/mr97310a.c
+++ b/drivers/media/video/gspca/mr97310a.c
@@ -1,23 +1,28 @@
 /*
  * Mars MR97310A library
  *
+ * The original mr97310a driver, which supported the Aiptek Pencam VGA+, is
  * Copyright (C) 2009 Kyle Guinn <elyk03@gmail.com>
  *
  * Support for the MR97310A cameras in addition to the Aiptek Pencam VGA+
  * and for the routines for detecting and classifying these various cameras,
+ * is Copyright (C) 2009 Theodore Kilgore <kilgota@auburn.edu>
  *
+ * Support for the control settings for the CIF cameras is
+ * Copyright (C) 2009 Hans de Goede <hdgoede@redhat.com> and
+ * Thomas Kaiser <thomas@kaiser-linux.li>
+ *
+ * Support for the control settings for the VGA cameras is
  * Copyright (C) 2009 Theodore Kilgore <kilgota@auburn.edu>
  *
- * Acknowledgements:
+ * Several previously unsupported cameras are owned and have been tested by
+ * Hans de Goede <hdgoede@redhat.com> and
+ * Thomas Kaiser <thomas@kaiser-linux.li> and
+ * Theodore Kilgore <kilgota@auburn.edu>
  *
  * The MR97311A support in gspca/mars.c has been helpful in understanding some
  * of the registers in these cameras.
  *
- * Hans de Goede <hdgoede@redhat.com> and
- * Thomas Kaiser <thomas@kaiser-linux.li>
- * have assisted with their experience. Each of them has also helped by
- * testing a previously unsupported camera.
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -40,11 +45,9 @@
 #define CAM_TYPE_CIF                    0
 #define CAM_TYPE_VGA                    1
 
-#define MR97310A_BRIGHTNESS_MIN         -254
-#define MR97310A_BRIGHTNESS_MAX         255
 #define MR97310A_BRIGHTNESS_DEFAULT     0
 
-#define MR97310A_EXPOSURE_MIN           300
+#define MR97310A_EXPOSURE_MIN           0
 #define MR97310A_EXPOSURE_MAX           4095
 #define MR97310A_EXPOSURE_DEFAULT       1000
 
@@ -82,6 +85,7 @@ struct sensor_w_data {
         int len;
 };
 
+static void sd_stopN(struct gspca_dev *gspca_dev);
 static int sd_setbrightness(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setexposure(struct gspca_dev *gspca_dev, __s32 val);
@@ -94,14 +98,16 @@ static void setgain(struct gspca_dev *gspca_dev);
 
 /* V4L2 controls supported by the driver */
 static struct ctrl sd_ctrls[] = {
+/* Seprate brightness control description for Argus QuickClix as it has
+   different limits from to other mr97310a camera's */
         {
-#define BRIGHTNESS_IDX 0
+#define NORM_BRIGHTNESS_IDX 0
                 {
                         .id = V4L2_CID_BRIGHTNESS,
                         .type = V4L2_CTRL_TYPE_INTEGER,
                         .name = "Brightness",
-                        .minimum = MR97310A_BRIGHTNESS_MIN,
-                        .maximum = MR97310A_BRIGHTNESS_MAX,
+                        .minimum = -254,
+                        .maximum = 255,
                         .step = 1,
                         .default_value = MR97310A_BRIGHTNESS_DEFAULT,
                         .flags = 0,
@@ -110,7 +116,22 @@ static struct ctrl sd_ctrls[] = {
                 .get = sd_getbrightness,
         },
         {
-#define EXPOSURE_IDX 1
+#define ARGUS_QC_BRIGHTNESS_IDX 1
+                {
+                        .id = V4L2_CID_BRIGHTNESS,
+                        .type = V4L2_CTRL_TYPE_INTEGER,
+                        .name = "Brightness",
+                        .minimum = 0,
+                        .maximum = 15,
+                        .step = 1,
+                        .default_value = MR97310A_BRIGHTNESS_DEFAULT,
+                        .flags = 0,
+                },
+                .set = sd_setbrightness,
+                .get = sd_getbrightness,
+        },
+        {
+#define EXPOSURE_IDX 2
                 {
                         .id = V4L2_CID_EXPOSURE,
                         .type = V4L2_CTRL_TYPE_INTEGER,
@@ -125,7 +146,7 @@ static struct ctrl sd_ctrls[] = {
                 .get = sd_getexposure,
         },
         {
-#define GAIN_IDX 2
+#define GAIN_IDX 3
                 {
                         .id = V4L2_CID_GAIN,
                         .type = V4L2_CTRL_TYPE_INTEGER,
@@ -230,12 +251,17 @@ static int sensor_write1(struct gspca_dev *gspca_dev, u8 reg, u8 data)
         int rc;
 
         buf = data;
-        rc = sensor_write_reg(gspca_dev, reg, 0x01, &buf, 1);
+        if (sd->cam_type == CAM_TYPE_CIF) {
+                rc = sensor_write_reg(gspca_dev, reg, 0x01, &buf, 1);
+                confirm_reg = sd->sensor_type ? 0x13 : 0x11;
+        } else {
+                rc = sensor_write_reg(gspca_dev, reg, 0x00, &buf, 1);
+                confirm_reg = 0x11;
+        }
         if (rc < 0)
                 return rc;
 
         buf = 0x01;
-        confirm_reg = sd->sensor_type ? 0x13 : 0x11;
         rc = sensor_write_reg(gspca_dev, confirm_reg, 0x00, &buf, 1);
         if (rc < 0)
                 return rc;
@@ -243,18 +269,26 @@ static int sensor_write1(struct gspca_dev *gspca_dev, u8 reg, u8 data)
         return 0;
 }
 
-static int cam_get_response16(struct gspca_dev *gspca_dev)
+static int cam_get_response16(struct gspca_dev *gspca_dev, u8 reg, int verbose)
 {
-        __u8 *data = gspca_dev->usb_buf;
         int err_code;
 
-        data[0] = 0x21;
+        gspca_dev->usb_buf[0] = reg;
         err_code = mr_write(gspca_dev, 1);
         if (err_code < 0)
                 return err_code;
 
         err_code = mr_read(gspca_dev, 16);
-        return err_code;
+        if (err_code < 0)
+                return err_code;
+
+        if (verbose)
+                PDEBUG(D_PROBE, "Register: %02x reads %02x%02x%02x", reg,
+                       gspca_dev->usb_buf[0],
+                       gspca_dev->usb_buf[1],
+                       gspca_dev->usb_buf[2]);
+
+        return 0;
 }
 
 static int zero_the_pointer(struct gspca_dev *gspca_dev)
@@ -264,7 +298,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
         u8 status = 0;
         int tries = 0;
 
-        err_code = cam_get_response16(gspca_dev);
+        err_code = cam_get_response16(gspca_dev, 0x21, 0);
         if (err_code < 0)
                 return err_code;
 
@@ -275,7 +309,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
         if (err_code < 0)
                 return err_code;
 
-        err_code = cam_get_response16(gspca_dev);
+        err_code = cam_get_response16(gspca_dev, 0x21, 0);
         if (err_code < 0)
                 return err_code;
 
@@ -285,7 +319,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
         if (err_code < 0)
                 return err_code;
 
-        err_code = cam_get_response16(gspca_dev);
+        err_code = cam_get_response16(gspca_dev, 0x21, 0);
         if (err_code < 0)
                 return err_code;
 
@@ -295,7 +329,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
         if (err_code < 0)
                 return err_code;
 
-        err_code = cam_get_response16(gspca_dev);
+        err_code = cam_get_response16(gspca_dev, 0x21, 0);
         if (err_code < 0)
                 return err_code;
 
@@ -306,7 +340,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
                 return err_code;
 
         while (status != 0x0a && tries < 256) {
-                err_code = cam_get_response16(gspca_dev);
+                err_code = cam_get_response16(gspca_dev, 0x21, 0);
                 status = data[0];
                 tries++;
                 if (err_code < 0)
@@ -323,7 +357,7 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
                 if (err_code < 0)
                         return err_code;
 
-                err_code = cam_get_response16(gspca_dev);
+                err_code = cam_get_response16(gspca_dev, 0x21, 0);
                 status = data[0];
                 tries++;
                 if (err_code < 0)
@@ -342,22 +376,34 @@ static int zero_the_pointer(struct gspca_dev *gspca_dev)
         return 0;
 }
 
-static u8 get_sensor_id(struct gspca_dev *gspca_dev)
+static int stream_start(struct gspca_dev *gspca_dev)
 {
-        int err_code;
-
-        gspca_dev->usb_buf[0] = 0x1e;
-        err_code = mr_write(gspca_dev, 1);
-        if (err_code < 0)
-                return err_code;
+        gspca_dev->usb_buf[0] = 0x01;
+        gspca_dev->usb_buf[1] = 0x01;
+        return mr_write(gspca_dev, 2);
+}
 
-        err_code = mr_read(gspca_dev, 16);
-        if (err_code < 0)
-                return err_code;
+static void stream_stop(struct gspca_dev *gspca_dev)
+{
+        gspca_dev->usb_buf[0] = 0x01;
+        gspca_dev->usb_buf[1] = 0x00;
+        if (mr_write(gspca_dev, 2) < 0)
+                PDEBUG(D_ERR, "Stream Stop failed");
+}
 
-        PDEBUG(D_PROBE, "Byte zero reported is %01x", gspca_dev->usb_buf[0]);
+static void lcd_stop(struct gspca_dev *gspca_dev)
+{
+        gspca_dev->usb_buf[0] = 0x19;
+        gspca_dev->usb_buf[1] = 0x54;
+        if (mr_write(gspca_dev, 2) < 0)
+                PDEBUG(D_ERR, "LCD Stop failed");
+}
 
-        return gspca_dev->usb_buf[0];
+static int isoc_enable(struct gspca_dev *gspca_dev)
+{
+        gspca_dev->usb_buf[0] = 0x00;
+        gspca_dev->usb_buf[1] = 0x4d;  /* ISOC transfering enable... */
+        return mr_write(gspca_dev, 2);
 }
 
 /* this function is called at probe time */
@@ -366,60 +412,172 @@ static int sd_config(struct gspca_dev *gspca_dev,
 {
         struct sd *sd = (struct sd *) gspca_dev;
         struct cam *cam;
-        __u8 *data = gspca_dev->usb_buf;
         int err_code;
 
         cam = &gspca_dev->cam;
         cam->cam_mode = vga_mode;
         cam->nmodes = ARRAY_SIZE(vga_mode);
+        sd->do_lcd_stop = 0;
+
+        /* Now, logical layout of the driver must fall sacrifice to the
+         * realities of the hardware supported. We have to sort out several
+         * cameras which share the USB ID but are in fact different inside.
+         * We need to start the initialization process for the cameras in
+         * order to classify them. Some of the supported cameras require the
+         * memory pointer to be set to 0 as the very first item of business
+         * or else they will not stream. So we do that immediately.
+         */
+        err_code = zero_the_pointer(gspca_dev);
+        if (err_code < 0)
+                return err_code;
 
         if (id->idProduct == 0x010e) {
                 sd->cam_type = CAM_TYPE_CIF;
                 cam->nmodes--;
-
-                data[0] = 0x01;
-                data[1] = 0x01;
-                err_code = mr_write(gspca_dev, 2);
+                err_code = stream_start(gspca_dev);
+                if (err_code < 0)
+                        return err_code;
+                err_code = cam_get_response16(gspca_dev, 0x06, 1);
                 if (err_code < 0)
                         return err_code;
-
-                msleep(200);
-                data[0] = get_sensor_id(gspca_dev);
                 /*
-                 * Known CIF cameras. If you have another to report, please do
+                 * The various CIF cameras share the same USB ID but use
+                 * different init routines and different controls. We need to
+                 * detect which one is connected!
                  *
-                 * Name                 byte just read          sd->sensor_type
-                 *                                      reported by
-                 * Sakar Spy-shot       0x28            T. Kilgore      0
-                 * Innovage             0xf5 (unstable) T. Kilgore      0
-                 * Vivitar Mini         0x53            H. De Goede     0
-                 * Vivitar Mini         0x04 / 0x24     E. Rodriguez    0
-                 * Vivitar Mini         0x08            T. Kilgore      1
-                 * Elta-Media 8212dc    0x23            T. Kaiser       1
-                 * Philips dig. keych.  0x37            T. Kilgore      1
+                 * A list of known CIF cameras follows. They all report either
+                 * 0002 for type 0 or 0003 for type 1.
+                 * If you have another to report, please do
+                 *
+                 * Name         sd->sensor_type         reported by
+                 *
+                 * Sakar Spy-shot       0               T. Kilgore
+                 * Innovage             0               T. Kilgore
+                 * Vivitar Mini         0               H. De Goede
+                 * Vivitar Mini         0               E. Rodriguez
+                 * Vivitar Mini         1               T. Kilgore
+                 * Elta-Media 8212dc    1               T. Kaiser
+                 * Philips dig. keych.  1               T. Kilgore
                  */
-                if ((data[0] & 0x78) == 8 ||
-                    ((data[0] & 0x2) == 0x2 && data[0] != 0x53))
-                        sd->sensor_type = 1;
-                else
+                switch (gspca_dev->usb_buf[1]) {
+                case 2:
                         sd->sensor_type = 0;
-
+                        break;
+                case 3:
+                        sd->sensor_type = 1;
+                        break;
+                default:
+                        PDEBUG(D_ERR, "Unknown CIF Sensor id : %02x",
+                               gspca_dev->usb_buf[1]);
+                        return -ENODEV;
+                }
                 PDEBUG(D_PROBE, "MR97310A CIF camera detected, sensor: %d",
                        sd->sensor_type);
+        } else {
+                sd->cam_type = CAM_TYPE_VGA;
 
-                if (force_sensor_type != -1) {
-                        sd->sensor_type = !! force_sensor_type;
-                        PDEBUG(D_PROBE, "Forcing sensor type to: %d",
-                               sd->sensor_type);
+                /*
+                 * VGA cams also have two different sensor types. Detection
+                 * requires a two-step process.
+                 *
+                 * Here is a report on the result of the first test for the
+                 * known MR97310a VGA cameras. If you have another to report,
+                 * please do.
+                 *
+                 * Name         byte just read                  sd->sensor_type
+                 *                              sd->do_lcd_stop
+                 * Aiptek Pencam VGA+   0x31            0       1
+                 * ION digital          0x31            0       1
+                 * Sakar Digital 77379  0x31            0       1
+                 * Argus DC-1620        0x30            1       0
+                 * Argus QuickClix      0x30            1       1 (see note)
+                 * Note that this test fails to distinguish sd->sensor_type
+                 * for the two cameras which have reported 0x30.
+                 * Another test will be run on them.
+                 * But the sd->do_lcd_stop setting is needed, too.
+                 */
+
+                err_code = cam_get_response16(gspca_dev, 0x20, 1);
+                if (err_code < 0)
+                        return err_code;
+                sd->sensor_type = gspca_dev->usb_buf[0] & 1;
+                sd->do_lcd_stop = (~gspca_dev->usb_buf[0]) & 1;
+                err_code = stream_start(gspca_dev);
+                if (err_code < 0)
+                        return err_code;
+
+                /*
+                 * A second test can now resolve any remaining ambiguity in the
+                 * identification of the camera's sensor type. Specifically,
+                 * it now gives the correct sensor_type for the Argus DC-1620
+                 * and the Argus QuickClix.
+                 *
+                 * This second test is only run if needed,
+                 * but additional results from testing some other cameras
+                 * are recorded here, too:
+                 *
+                 * Name                 gspca_dev->usb_buf[]    sd->sensor_type
+                 *
+                 * Aiptek Pencam VGA+   0300    (test not needed)       1
+                 * ION digital          0350    (test not needed)       1
+                 * Argus DC-1620        0450    (remains as type 0)     0
+                 * Argus QuickClix      0420    (corrected to type 1)   1
+                 *
+                 * This test even seems able to distinguish one VGA cam from
+                 * another which may be useful. However, the CIF type 1 cameras
+                 * do not like it.
+                 */
+
+                if (!sd->sensor_type) {
+                        err_code = cam_get_response16(gspca_dev, 0x07, 1);
+                        if (err_code < 0)
+                                return err_code;
+
+                        switch (gspca_dev->usb_buf[1]) {
+                        case 0x50:
+                                break;
+                        case 0x20:
+                                sd->sensor_type = 1;
+                                PDEBUG(D_PROBE, "sensor_type corrected to 1");
+                                break;
+                        default:
+                                PDEBUG(D_ERR, "Unknown VGA Sensor id : %02x",
+                                       gspca_dev->usb_buf[1]);
+                                return -ENODEV;
+                        }
                 }
+                PDEBUG(D_PROBE, "MR97310A VGA camera detected, sensor: %d",
+                       sd->sensor_type);
+        }
+        /* Stop streaming as we've started it to probe the sensor type. */
+        sd_stopN(gspca_dev);
 
+        if (force_sensor_type != -1) {
+                sd->sensor_type = !!force_sensor_type;
+                PDEBUG(D_PROBE, "Forcing sensor type to: %d",
+                       sd->sensor_type);
+        }
+
+        /* Setup controls depending on camera type */
+        if (sd->cam_type == CAM_TYPE_CIF) {
+                /* No brightness for sensor_type 0 */
                 if (sd->sensor_type == 0)
-                        gspca_dev->ctrl_dis = (1 << BRIGHTNESS_IDX);
+                        gspca_dev->ctrl_dis = (1 << NORM_BRIGHTNESS_IDX) |
+                                              (1 << ARGUS_QC_BRIGHTNESS_IDX);
+                else
+                        gspca_dev->ctrl_dis = (1 << ARGUS_QC_BRIGHTNESS_IDX);
         } else {
-                sd->cam_type = CAM_TYPE_VGA;
-                PDEBUG(D_PROBE, "MR97310A VGA camera detected");
-                gspca_dev->ctrl_dis = (1 << BRIGHTNESS_IDX) |
-                                      (1 << EXPOSURE_IDX) | (1 << GAIN_IDX);
+                /* All controls need to be disabled if VGA sensor_type is 0 */
+                if (sd->sensor_type == 0)
+                        gspca_dev->ctrl_dis = (1 << NORM_BRIGHTNESS_IDX) |
+                                              (1 << ARGUS_QC_BRIGHTNESS_IDX) |
+                                              (1 << EXPOSURE_IDX) |
+                                              (1 << GAIN_IDX);
+                else if (sd->do_lcd_stop)
+                        /* Argus QuickClix has different brightness limits */
+                        gspca_dev->ctrl_dis = (1 << NORM_BRIGHTNESS_IDX);
+                else
+                        gspca_dev->ctrl_dis = (1 << ARGUS_QC_BRIGHTNESS_IDX);
         }
 
         sd->brightness = MR97310A_BRIGHTNESS_DEFAULT;
@@ -455,11 +613,6 @@ static int start_cif_cam(struct gspca_dev *gspca_dev)
         };
 
         /* Note: Some of the above descriptions guessed from MR97113A driver */
-        data[0] = 0x01;
-        data[1] = 0x01;
-        err_code = mr_write(gspca_dev, 2);
-        if (err_code < 0)
-                return err_code;
 
         memcpy(data, startup_string, 11);
         if (sd->sensor_type)
@@ -533,22 +686,7 @@ static int start_cif_cam(struct gspca_dev *gspca_dev)
                 err_code = sensor_write_regs(gspca_dev, cif_sensor1_init_data,
                                          ARRAY_SIZE(cif_sensor1_init_data));
         }
-        if (err_code < 0)
-                return err_code;
-
-        setbrightness(gspca_dev);
-        setexposure(gspca_dev);
-        setgain(gspca_dev);
-
-        msleep(200);
-
-        data[0] = 0x00;
-        data[1] = 0x4d;  /* ISOC transfering enable... */
-        err_code = mr_write(gspca_dev, 2);
-        if (err_code < 0)
-                return err_code;
-
-        return 0;
+        return err_code;
 }
 
 static int start_vga_cam(struct gspca_dev *gspca_dev)
@@ -558,84 +696,8 @@ static int start_vga_cam(struct gspca_dev *gspca_dev)
         int err_code;
         const __u8 startup_string[] = {0x00, 0x0d, 0x01, 0x00, 0x00, 0x2b,
                                        0x00, 0x00, 0x00, 0x50, 0xc0};
-
         /* What some of these mean is explained in start_cif_cam(), above */
-        sd->sof_read = 0;
-
-        /*
-         * We have to know which camera we have, because the register writes
-         * depend upon the camera. This test, run before we actually enter
-         * the initialization routine, distinguishes most of the cameras, If
-         * needed, another routine is done later, too.
-         */
-        memset(data, 0, 16);
-        data[0] = 0x20;
-        err_code = mr_write(gspca_dev, 1);
-        if (err_code < 0)
-                return err_code;
-
-        err_code = mr_read(gspca_dev, 16);
-        if (err_code < 0)
-                return err_code;
-
-        PDEBUG(D_PROBE, "Byte reported is %02x", data[0]);
-
-        msleep(200);
-        /*
-         * Known VGA cameras. If you have another to report, please do
-         *
-         * Name                 byte just read          sd->sensor_type
-         *                              sd->do_lcd_stop
-         * Aiptek Pencam VGA+   0x31            0       1
-         * ION digital          0x31            0       1
-         * Argus DC-1620        0x30            1       0
-         * Argus QuickClix      0x30            1       1 (not caught here)
-         */
-        sd->sensor_type = data[0] & 1;
-        sd->do_lcd_stop = (~data[0]) & 1;
-
-
-
-        /* Streaming setup begins here. */
-
-
-        data[0] = 0x01;
-        data[1] = 0x01;
-        err_code = mr_write(gspca_dev, 2);
-        if (err_code < 0)
-                return err_code;
-
-        /*
-         * A second test can now resolve any remaining ambiguity in the
-         * identification of the camera type,
-         */
-        if (!sd->sensor_type) {
-                data[0] = get_sensor_id(gspca_dev);
-                if (data[0] == 0x7f) {
-                        sd->sensor_type = 1;
-                        PDEBUG(D_PROBE, "sensor_type corrected to 1");
-                }
-                msleep(200);
-        }
-
-        if (force_sensor_type != -1) {
-                sd->sensor_type = !! force_sensor_type;
-                PDEBUG(D_PROBE, "Forcing sensor type to: %d",
-                       sd->sensor_type);
-        }
 
-        /*
-         * Known VGA cameras.
-         * This test is only run if the previous test returned 0x30, but
-         * here is the information for all others, too, just for reference.
-         *
-         * Name                 byte just read          sd->sensor_type
-         *
-         * Aiptek Pencam VGA+   0xfb    (this test not run)     1
-         * ION digital          0xbd    (this test not run)     1
-         * Argus DC-1620        0xe5    (no change)             0
-         * Argus QuickClix      0x7f    (reclassified)          1
-         */
         memcpy(data, startup_string, 11);
         if (!sd->sensor_type) {
                 data[5]  = 0x00;
@@ -704,14 +766,6 @@ static int start_vga_cam(struct gspca_dev *gspca_dev)
                 err_code = sensor_write_regs(gspca_dev, vga_sensor1_init_data,
                                          ARRAY_SIZE(vga_sensor1_init_data));
         }
-        if (err_code < 0)
-                return err_code;
-
-        msleep(200);
-        data[0] = 0x00;
-        data[1] = 0x4d;  /* ISOC transfering enable... */
-        err_code = mr_write(gspca_dev, 2);
-
         return err_code;
 }
 
@@ -719,82 +773,101 @@ static int sd_start(struct gspca_dev *gspca_dev)
 {
         struct sd *sd = (struct sd *) gspca_dev;
         int err_code;
-        struct cam *cam;
 
-        cam = &gspca_dev->cam;
         sd->sof_read = 0;
-        /*
-         * Some of the supported cameras require the memory pointer to be
-         * set to 0, or else they will not stream.
-         */
-        zero_the_pointer(gspca_dev);
-        msleep(200);
+
+        /* Some of the VGA cameras require the memory pointer
+         * to be set to 0 again. We have been forced to start the
+         * stream somewhere else to detect the hardware, and closed it,
+         * and now since we are restarting the stream we need to do a
+         * completely fresh and clean start. */
+        err_code = zero_the_pointer(gspca_dev);
+        if (err_code < 0)
+                return err_code;
+
+        err_code = stream_start(gspca_dev);
+        if (err_code < 0)
+                return err_code;
+
         if (sd->cam_type == CAM_TYPE_CIF) {
                 err_code = start_cif_cam(gspca_dev);
         } else {
                 err_code = start_vga_cam(gspca_dev);
         }
-        return err_code;
+        if (err_code < 0)
+                return err_code;
+
+        setbrightness(gspca_dev);
+        setexposure(gspca_dev);
+        setgain(gspca_dev);
+
+        return isoc_enable(gspca_dev);
 }
 
 static void sd_stopN(struct gspca_dev *gspca_dev)
 {
         struct sd *sd = (struct sd *) gspca_dev;
-        int result;
-
-        gspca_dev->usb_buf[0] = 1;
-        gspca_dev->usb_buf[1] = 0;
-        result = mr_write(gspca_dev, 2);
-        if (result < 0)
-                PDEBUG(D_ERR, "Camera Stop failed");
 
+        stream_stop(gspca_dev);
         /* Not all the cams need this, but even if not, probably a good idea */
         zero_the_pointer(gspca_dev);
-        if (sd->do_lcd_stop) {
-                gspca_dev->usb_buf[0] = 0x19;
-                gspca_dev->usb_buf[1] = 0x54;
-                result = mr_write(gspca_dev, 2);
-                if (result < 0)
-                        PDEBUG(D_ERR, "Camera Stop failed");
-        }
+        if (sd->do_lcd_stop)
+                lcd_stop(gspca_dev);
 }
 
 static void setbrightness(struct gspca_dev *gspca_dev)
 {
         struct sd *sd = (struct sd *) gspca_dev;
         u8 val;
-
-        if (gspca_dev->ctrl_dis & (1 << BRIGHTNESS_IDX))
+        u8 sign_reg = 7;  /* This reg and the next one used on CIF cams. */
+        u8 value_reg = 8; /* VGA cams seem to use regs 0x0b and 0x0c */
+        const u8 quick_clix_table[] =
+        /*        0  1  2   3  4  5  6  7  8  9  10  11  12  13  14  15 */
+                { 0, 4, 8, 12, 1, 2, 3, 5, 6, 9,  7, 10, 13, 11, 14, 15};
+        /*
+         * This control is disabled for CIF type 1 and VGA type 0 cameras.
+         * It does not quite act linearly for the Argus QuickClix camera,
+         * but it does control brightness. The values are 0 - 15 only, and
+         * the table above makes them act consecutively.
+         */
+        if ((gspca_dev->ctrl_dis & (1 << NORM_BRIGHTNESS_IDX)) &&
+            (gspca_dev->ctrl_dis & (1 << ARGUS_QC_BRIGHTNESS_IDX)))
                 return;
 
+        if (sd->cam_type == CAM_TYPE_VGA) {
+                sign_reg += 4;
+                value_reg += 4;
+        }
+
         /* Note register 7 is also seen as 0x8x or 0xCx in dumps */
         if (sd->brightness > 0) {
-                sensor_write1(gspca_dev, 7, 0x00);
+                sensor_write1(gspca_dev, sign_reg, 0x00);
                 val = sd->brightness;
         } else {
-                sensor_write1(gspca_dev, 7, 0x01);
-                val = 257 - sd->brightness;
+                sensor_write1(gspca_dev, sign_reg, 0x01);
+                val = (257 - sd->brightness);
         }
-        sensor_write1(gspca_dev, 8, val);
+        /* Use lookup table for funky Argus QuickClix brightness */
+        if (sd->do_lcd_stop)
+                val = quick_clix_table[val];
+
+        sensor_write1(gspca_dev, value_reg, val);
 }
 
 static void setexposure(struct gspca_dev *gspca_dev)
 {
         struct sd *sd = (struct sd *) gspca_dev;
-        u8 val;
+        int exposure;
 
         if (gspca_dev->ctrl_dis & (1 << EXPOSURE_IDX))
                 return;
 
-        if (sd->sensor_type) {
-                val = sd->exposure >> 4;
-                sensor_write1(gspca_dev, 3, val);
-                val = sd->exposure & 0xf;
-                sensor_write1(gspca_dev, 4, val);
+        if (sd->cam_type == CAM_TYPE_CIF && sd->sensor_type == 1) {
+                /* This cam does not like very low exposure settings */
+                exposure = (sd->exposure < 300) ? 300 : sd->exposure;
+                sensor_write1(gspca_dev, 3, exposure >> 4);
+                sensor_write1(gspca_dev, 4, exposure & 0x0f);
         } else {
-                u8 clockdiv;
-                int exposure;
-
                 /* We have both a clock divider and an exposure register.
                    We first calculate the clock divider, as that determines
                    the maximum exposure and then we calculayte the exposure
@@ -802,7 +875,7 @@ static void setexposure(struct gspca_dev *gspca_dev)
 
                    Note our 0 - 4095 exposure is mapped to 0 - 511
                    milliseconds exposure time */
-                clockdiv = (60 * sd->exposure + 7999) / 8000;
+                u8 clockdiv = (60 * sd->exposure + 7999) / 8000;
 
                 /* Limit framerate to not exceed usb bandwidth */
                 if (clockdiv < 3 && gspca_dev->width >= 320)
@@ -810,6 +883,9 @@ static void setexposure(struct gspca_dev *gspca_dev)
                 else if (clockdiv < 2)
                         clockdiv = 2;
 
+                if (sd->cam_type == CAM_TYPE_VGA && clockdiv < 4)
+                        clockdiv = 4;
+
                 /* Frame exposure time in ms = 1000 * clockdiv / 60 ->
                 exposure = (sd->exposure / 8) * 511 / (1000 * clockdiv / 60) */
                 exposure = (60 * 511 * sd->exposure) / (8000 * clockdiv);
@@ -832,7 +908,7 @@ static void setgain(struct gspca_dev *gspca_dev)
         if (gspca_dev->ctrl_dis & (1 << GAIN_IDX))
                 return;
 
-        if (sd->sensor_type) {
+        if (sd->cam_type == CAM_TYPE_CIF && sd->sensor_type == 1) {
                 sensor_write1(gspca_dev, 0x0e, sd->gain);
         } else {
                 sensor_write1(gspca_dev, 0x10, sd->gain);