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);