[PATCH 00/11] ov534: Fixes and updates

[PATCH 00/11] ov534: Fixes and updates

[Antonio Ospite]

Hi Jean-Francois,

will you review/apply these changes please?

Several fixes and updates:
 * Removed some controls because their state was not good enough
 * Added AEC
 * AGC and AWB enabled by default
 * Fixed setting/unsetting registers for
     - exposure
     - sharpness
     - hflip
     - vflip
 * Fixed coding style
 * Added Powerline Frequency control

The only big behavioural change should be AGC enabled by default, if
users want autogain disabled by default we can rediscuss this again.

Special thanks to Max Thrun and Mosalam Ebrahimi.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[PATCH 01/11] ov534: Remove ambiguous controls

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

Remove Blue/Red Channel Target Value, they are meant for Black Level
Calibration but it is not completely clear how to use them.

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |  100 +-------------------------------
 1 file changed, 6 insertions(+), 94 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,8 +60,6 @@
 	u8 contrast;
 	u8 gain;
 	u8 exposure;
-	u8 redblc;
-	u8 blueblc;
 	u8 hue;
 	u8 autogain;
 	u8 awb;
@@ -76,10 +74,6 @@
 static int sd_getgain(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setexposure(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getexposure(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setredblc(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getredblc(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setblueblc(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getblueblc(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getautogain(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val);
@@ -156,34 +150,6 @@
     },
     {							/* 4 */
 	{
-	    .id      = V4L2_CID_RED_BALANCE,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Red Balance",
-	    .minimum = 0,
-	    .maximum = 255,
-	    .step    = 1,
-#define RED_BALANCE_DEF 128
-	    .default_value = RED_BALANCE_DEF,
-	},
-	.set = sd_setredblc,
-	.get = sd_getredblc,
-    },
-    {							/* 5 */
-	{
-	    .id      = V4L2_CID_BLUE_BALANCE,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Blue Balance",
-	    .minimum = 0,
-	    .maximum = 255,
-	    .step    = 1,
-#define BLUE_BALANCE_DEF 128
-	    .default_value = BLUE_BALANCE_DEF,
-	},
-	.set = sd_setblueblc,
-	.get = sd_getblueblc,
-    },
-    {							/* 6 */
-	{
 		.id      = V4L2_CID_HUE,
 		.type    = V4L2_CTRL_TYPE_INTEGER,
 		.name    = "Hue",
@@ -196,7 +162,7 @@
 	.set = sd_sethue,
 	.get = sd_gethue,
     },
-    {							/* 7 */
+    {							/* 5 */
 	{
 	    .id      = V4L2_CID_AUTOGAIN,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -210,8 +176,8 @@
 	.set = sd_setautogain,
 	.get = sd_getautogain,
     },
-#define AWB_IDX 8
-    {							/* 8 */
+#define AWB_IDX 6
+    {							/* 6 */
 	{
 		.id      = V4L2_CID_AUTO_WHITE_BALANCE,
 		.type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -225,7 +191,7 @@
 	.set = sd_setawb,
 	.get = sd_getawb,
     },
-    {							/* 9 */
+    {							/* 7 */
 	{
 	    .id      = V4L2_CID_SHARPNESS,
 	    .type    = V4L2_CTRL_TYPE_INTEGER,
@@ -239,7 +205,7 @@
 	.set = sd_setsharpness,
 	.get = sd_getsharpness,
     },
-    {							/* 10 */
+    {							/* 8 */
 	{
 	    .id      = V4L2_CID_HFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -253,7 +219,7 @@
 	.set = sd_sethflip,
 	.get = sd_gethflip,
     },
-    {							/* 11 */
+    {							/* 9 */
 	{
 	    .id      = V4L2_CID_VFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -722,20 +688,6 @@
 	sccb_reg_write(gspca_dev, 0x10, val << 1);
 }
 
-static void setredblc(struct gspca_dev *gspca_dev)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sccb_reg_write(gspca_dev, 0x43, sd->redblc);
-}
-
-static void setblueblc(struct gspca_dev *gspca_dev)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sccb_reg_write(gspca_dev, 0x42, sd->blueblc);
-}
-
 static void sethue(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -825,8 +777,6 @@
 	sd->contrast = CONTRAST_DEF;
 	sd->gain = GAIN_DEF;
 	sd->exposure = EXPO_DEF;
-	sd->redblc = RED_BALANCE_DEF;
-	sd->blueblc = BLUE_BALANCE_DEF;
 	sd->hue = HUE_DEF;
 #if AUTOGAIN_DEF != 0
 	sd->autogain = AUTOGAIN_DEF;
@@ -907,8 +857,6 @@
 	setautogain(gspca_dev);
 	setawb(gspca_dev);
 	setgain(gspca_dev);
-	setredblc(gspca_dev);
-	setblueblc(gspca_dev);
 	sethue(gspca_dev);
 	setexposure(gspca_dev);
 	setbrightness(gspca_dev);
@@ -1092,42 +1040,6 @@
 	return 0;
 }
 
-static int sd_setredblc(struct gspca_dev *gspca_dev, __s32 val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sd->redblc = val;
-	if (gspca_dev->streaming)
-		setredblc(gspca_dev);
-	return 0;
-}
-
-static int sd_getredblc(struct gspca_dev *gspca_dev, __s32 *val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	*val = sd->redblc;
-	return 0;
-}
-
-static int sd_setblueblc(struct gspca_dev *gspca_dev, __s32 val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sd->blueblc = val;
-	if (gspca_dev->streaming)
-		setblueblc(gspca_dev);
-	return 0;
-}
-
-static int sd_getblueblc(struct gspca_dev *gspca_dev, __s32 *val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	*val = sd->blueblc;
-	return 0;
-}
-
 static int sd_sethue(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;

[PATCH 02/11] ov534: Remove hue control

[Antonio Ospite]

Hue control doesn't work and the sensor datasheet is not clear about how
to set hue properly.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   54 ++------------------------------
 1 file changed, 5 insertions(+), 49 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,7 +60,6 @@
 	u8 contrast;
 	u8 gain;
 	u8 exposure;
-	u8 hue;
 	u8 autogain;
 	u8 awb;
 	s8 sharpness;
@@ -82,8 +81,6 @@
 static int sd_gethflip(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setvflip(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getvflip(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_sethue(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_gethue(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setawb(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getawb(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setbrightness(struct gspca_dev *gspca_dev, __s32 val);
@@ -150,20 +147,6 @@
     },
     {							/* 4 */
 	{
-		.id      = V4L2_CID_HUE,
-		.type    = V4L2_CTRL_TYPE_INTEGER,
-		.name    = "Hue",
-		.minimum = 0,
-		.maximum = 255,
-		.step    = 1,
-#define HUE_DEF 143
-		.default_value = HUE_DEF,
-	},
-	.set = sd_sethue,
-	.get = sd_gethue,
-    },
-    {							/* 5 */
-	{
 	    .id      = V4L2_CID_AUTOGAIN,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
 	    .name    = "Autogain",
@@ -176,8 +159,8 @@
 	.set = sd_setautogain,
 	.get = sd_getautogain,
     },
-#define AWB_IDX 6
-    {							/* 6 */
+#define AWB_IDX 5
+    {							/* 5 */
 	{
 		.id      = V4L2_CID_AUTO_WHITE_BALANCE,
 		.type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -191,7 +174,7 @@
 	.set = sd_setawb,
 	.get = sd_getawb,
     },
-    {							/* 7 */
+    {							/* 6 */
 	{
 	    .id      = V4L2_CID_SHARPNESS,
 	    .type    = V4L2_CTRL_TYPE_INTEGER,
@@ -205,7 +188,7 @@
 	.set = sd_setsharpness,
 	.get = sd_getsharpness,
     },
-    {							/* 8 */
+    {							/* 7 */
 	{
 	    .id      = V4L2_CID_HFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -219,7 +202,7 @@
 	.set = sd_sethflip,
 	.get = sd_gethflip,
     },
-    {							/* 9 */
+    {							/* 8 */
 	{
 	    .id      = V4L2_CID_VFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -688,13 +671,6 @@
 	sccb_reg_write(gspca_dev, 0x10, val << 1);
 }
 
-static void sethue(struct gspca_dev *gspca_dev)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sccb_reg_write(gspca_dev, 0x01, sd->hue);
-}
-
 static void setautogain(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -777,7 +753,6 @@
 	sd->contrast = CONTRAST_DEF;
 	sd->gain = GAIN_DEF;
 	sd->exposure = EXPO_DEF;
-	sd->hue = HUE_DEF;
 #if AUTOGAIN_DEF != 0
 	sd->autogain = AUTOGAIN_DEF;
 #else
@@ -857,7 +832,6 @@
 	setautogain(gspca_dev);
 	setawb(gspca_dev);
 	setgain(gspca_dev);
-	sethue(gspca_dev);
 	setexposure(gspca_dev);
 	setbrightness(gspca_dev);
 	setcontrast(gspca_dev);
@@ -1040,24 +1014,6 @@
 	return 0;
 }
 
-static int sd_sethue(struct gspca_dev *gspca_dev, __s32 val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	sd->hue = val;
-	if (gspca_dev->streaming)
-		sethue(gspca_dev);
-	return 0;
-}
-
-static int sd_gethue(struct gspca_dev *gspca_dev, __s32 *val)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-
-	*val = sd->hue;
-	return 0;
-}
-
 static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;

[PATCH 03/11] ov534: Fix autogain control, enable it by default

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

  * Use 'agc' instead of 'autogain' in the code so to align the naming
    as in AEC/AWB.
  * Tweak brightness and contrast default values.
  * Fix setting/resetting registers values for AGC.
  * Set actual gain back when disabling AGC.
  * Skip setting GAIN register when AGC is enabled.
  * Enable AGC by default.

Note that as Auto Gain Control is now enabled by default, if you are
using the driver for visual computing applications you might need to
disable it explicitly in your software.

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   53 ++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 23 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -60,7 +60,7 @@
 	u8 contrast;
 	u8 gain;
 	u8 exposure;
-	u8 autogain;
+	u8 agc;
 	u8 awb;
 	s8 sharpness;
 	u8 hflip;
@@ -73,8 +73,8 @@
 static int sd_getgain(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setexposure(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getexposure(struct gspca_dev *gspca_dev, __s32 *val);
-static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val);
-static int sd_getautogain(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setagc(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getagc(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getsharpness(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_sethflip(struct gspca_dev *gspca_dev, __s32 val);
@@ -97,7 +97,7 @@
 		.minimum = 0,
 		.maximum = 255,
 		.step    = 1,
-#define BRIGHTNESS_DEF 20
+#define BRIGHTNESS_DEF 0
 		.default_value = BRIGHTNESS_DEF,
 	},
 	.set = sd_setbrightness,
@@ -111,7 +111,7 @@
 		.minimum = 0,
 		.maximum = 255,
 		.step    = 1,
-#define CONTRAST_DEF 37
+#define CONTRAST_DEF 32
 		.default_value = CONTRAST_DEF,
 	},
 	.set = sd_setcontrast,
@@ -149,15 +149,15 @@
 	{
 	    .id      = V4L2_CID_AUTOGAIN,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "Autogain",
+	    .name    = "Auto Gain",
 	    .minimum = 0,
 	    .maximum = 1,
 	    .step    = 1,
-#define AUTOGAIN_DEF 0
-	    .default_value = AUTOGAIN_DEF,
+#define AGC_DEF 1
+	    .default_value = AGC_DEF,
 	},
-	.set = sd_setautogain,
-	.get = sd_getautogain,
+	.set = sd_setagc,
+	.get = sd_getagc,
     },
 #define AWB_IDX 5
     {							/* 5 */
@@ -639,6 +639,9 @@
 	struct sd *sd = (struct sd *) gspca_dev;
 	u8 val;
 
+	if (sd->agc)
+		return;
+
 	val = sd->gain;
 	switch (val & 0x30) {
 	case 0x00:
@@ -671,18 +674,22 @@
 	sccb_reg_write(gspca_dev, 0x10, val << 1);
 }
 
-static void setautogain(struct gspca_dev *gspca_dev)
+static void setagc(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	if (sd->autogain) {
-		sccb_reg_write(gspca_dev, 0x13, 0xf7); /* AGC,AEC,AWB ON */
+	if (sd->agc) {
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) | 0x04);
 		sccb_reg_write(gspca_dev, 0x64,
 				sccb_reg_read(gspca_dev, 0x64) | 0x03);
 	} else {
-		sccb_reg_write(gspca_dev, 0x13, 0xf0); /* AGC,AEC,AWB OFF */
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) & ~0x04);
 		sccb_reg_write(gspca_dev, 0x64,
-				sccb_reg_read(gspca_dev, 0x64) & 0xfc);
+				sccb_reg_read(gspca_dev, 0x64) & ~0x03);
+
+		setgain(gspca_dev);
 	}
 }
 
@@ -753,8 +760,8 @@
 	sd->contrast = CONTRAST_DEF;
 	sd->gain = GAIN_DEF;
 	sd->exposure = EXPO_DEF;
-#if AUTOGAIN_DEF != 0
-	sd->autogain = AUTOGAIN_DEF;
+#if AGC_DEF != 0
+	sd->agc = AGC_DEF;
 #else
 	gspca_dev->ctrl_inac |= (1 << AWB_IDX);
 #endif
@@ -829,7 +836,7 @@
 	}
 	set_frame_rate(gspca_dev);
 
-	setautogain(gspca_dev);
+	setagc(gspca_dev);
 	setawb(gspca_dev);
 	setgain(gspca_dev);
 	setexposure(gspca_dev);
@@ -1014,11 +1021,11 @@
 	return 0;
 }
 
-static int sd_setautogain(struct gspca_dev *gspca_dev, __s32 val)
+static int sd_setagc(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	sd->autogain = val;
+	sd->agc = val;
 
 	if (gspca_dev->streaming) {
 
@@ -1028,16 +1035,16 @@
 			gspca_dev->ctrl_inac &= ~(1 << AWB_IDX);
 		else
 			gspca_dev->ctrl_inac |= (1 << AWB_IDX);
-		setautogain(gspca_dev);
+		setagc(gspca_dev);
 	}
 	return 0;
 }
 
-static int sd_getautogain(struct gspca_dev *gspca_dev, __s32 *val)
+static int sd_getagc(struct gspca_dev *gspca_dev, __s32 *val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	*val = sd->autogain;
+	*val = sd->agc;
 	return 0;
 }
 

[PATCH 04/11] ov534: Add Auto Exposure

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

This also makes manual exposure actually work: it never worked before
because AEC was always enabled.

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   55 ++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -62,6 +62,7 @@
 	u8 exposure;
 	u8 agc;
 	u8 awb;
+	u8 aec;
 	s8 sharpness;
 	u8 hflip;
 	u8 vflip;
@@ -83,6 +84,8 @@
 static int sd_getvflip(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setawb(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getawb(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setaec(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getaec(struct gspca_dev *gspca_dev, __s32 *val);
 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_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
@@ -176,6 +179,20 @@
     },
     {							/* 6 */
 	{
+		.id      = V4L2_CID_EXPOSURE_AUTO,
+		.type    = V4L2_CTRL_TYPE_BOOLEAN,
+		.name    = "Auto Exposure",
+		.minimum = 0,
+		.maximum = 1,
+		.step    = 1,
+#define AEC_DEF 1
+		.default_value = AEC_DEF,
+	},
+	.set = sd_setaec,
+	.get = sd_getaec,
+    },
+    {							/* 7 */
+	{
 	    .id      = V4L2_CID_SHARPNESS,
 	    .type    = V4L2_CTRL_TYPE_INTEGER,
 	    .name    = "Sharpness",
@@ -188,7 +205,7 @@
 	.set = sd_setsharpness,
 	.get = sd_getsharpness,
     },
-    {							/* 7 */
+    {							/* 8 */
 	{
 	    .id      = V4L2_CID_HFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -202,7 +219,7 @@
 	.set = sd_sethflip,
 	.get = sd_gethflip,
     },
-    {							/* 8 */
+    {							/* 9 */
 	{
 	    .id      = V4L2_CID_VFLIP,
 	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -703,6 +720,20 @@
 		sccb_reg_write(gspca_dev, 0x63, 0xaa);	/* AWB off */
 }
 
+static void setaec(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	if (sd->aec)
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) | 0x01);
+	else {
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) & ~0x01);
+		setexposure(gspca_dev);
+	}
+}
+
 static void setsharpness(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -768,6 +799,7 @@
 #if AWB_DEF != 0
 	sd->awb = AWB_DEF
 #endif
+	sd->aec = AEC_DEF;
 #if SHARPNESS_DEF != 0
 	sd->sharpness = SHARPNESS_DEF;
 #endif
@@ -838,6 +870,7 @@
 
 	setagc(gspca_dev);
 	setawb(gspca_dev);
+	setaec(gspca_dev);
 	setgain(gspca_dev);
 	setexposure(gspca_dev);
 	setbrightness(gspca_dev);
@@ -1066,6 +1099,24 @@
 	return 0;
 }
 
+static int sd_setaec(struct gspca_dev *gspca_dev, __s32 val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	sd->aec = val;
+	if (gspca_dev->streaming)
+		setaec(gspca_dev);
+	return 0;
+}
+
+static int sd_getaec(struct gspca_dev *gspca_dev, __s32 *val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	*val = sd->aec;
+	return 0;
+}
+
 static int sd_setsharpness(struct gspca_dev *gspca_dev, __s32 val)
 {
 	struct sd *sd = (struct sd *) gspca_dev;

[PATCH 05/11] ov534: Fix setting manual exposure

[Antonio Ospite]

Exposure is now a u16 value, both MSB and LSB are set, but values in the v4l2
control are limited to the interval [0,506] as 0x01fa (506) is the maximum
observed value with AEC enabled.

Skip setting exposure when AEC is enabled.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -59,7 +59,7 @@
 	u8 brightness;
 	u8 contrast;
 	u8 gain;
-	u8 exposure;
+	u16 exposure;
 	u8 agc;
 	u8 awb;
 	u8 aec;
@@ -140,7 +140,7 @@
 	    .type    = V4L2_CTRL_TYPE_INTEGER,
 	    .name    = "Exposure",
 	    .minimum = 0,
-	    .maximum = 255,
+	    .maximum = 506,
 	    .step    = 1,
 #define EXPO_DEF 120
 	    .default_value = EXPO_DEF,
@@ -684,11 +684,15 @@
 static void setexposure(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
-	u8 val;
+	u16 val;
+
+	if (sd->aec)
+		return;
 
 	val = sd->exposure;
-	sccb_reg_write(gspca_dev, 0x08, val >> 7);
-	sccb_reg_write(gspca_dev, 0x10, val << 1);
+	sccb_reg_write(gspca_dev, 0x08, val >> 8);
+	sccb_reg_write(gspca_dev, 0x10, val & 0xff);
+
 }
 
 static void setagc(struct gspca_dev *gspca_dev)

[PATCH 06/11] ov534: Fix Auto White Balance control

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

Set only the needed bits for AWB, and enable it by default.

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -171,7 +171,7 @@
 		.minimum = 0,
 		.maximum = 1,
 		.step    = 1,
-#define AWB_DEF 0
+#define AWB_DEF 1
 		.default_value = AWB_DEF,
 	},
 	.set = sd_setawb,
@@ -718,10 +718,17 @@
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	if (sd->awb)
-		sccb_reg_write(gspca_dev, 0x63, 0xe0);	/* AWB on */
-	else
-		sccb_reg_write(gspca_dev, 0x63, 0xaa);	/* AWB off */
+	if (sd->awb) {
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) | 0x02);
+		sccb_reg_write(gspca_dev, 0x63,
+				sccb_reg_read(gspca_dev, 0x63) | 0xc0);
+	} else {
+		sccb_reg_write(gspca_dev, 0x13,
+				sccb_reg_read(gspca_dev, 0x13) & ~0x02);
+		sccb_reg_write(gspca_dev, 0x63,
+				sccb_reg_read(gspca_dev, 0x63) & ~0xc0);
+	}
 }
 
 static void setaec(struct gspca_dev *gspca_dev)
@@ -800,9 +807,7 @@
 #else
 	gspca_dev->ctrl_inac |= (1 << AWB_IDX);
 #endif
-#if AWB_DEF != 0
-	sd->awb = AWB_DEF
-#endif
+	sd->awb = AWB_DEF;
 	sd->aec = AEC_DEF;
 #if SHARPNESS_DEF != 0
 	sd->sharpness = SHARPNESS_DEF;

[PATCH 07/11] ov534: Fixes for sharpness control

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

  * Adjust comments for sharpness control
  * Set default value unconditionally, for readability

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -751,8 +751,8 @@
 	u8 val;
 
 	val = sd->sharpness;
-	sccb_reg_write(gspca_dev, 0x91, val);	/* vga noise */
-	sccb_reg_write(gspca_dev, 0x8e, val);	/* qvga noise */
+	sccb_reg_write(gspca_dev, 0x91, val);	/* Auto de-noise threshold */
+	sccb_reg_write(gspca_dev, 0x8e, val);	/* De-noise threshold */
 }
 
 static void sethflip(struct gspca_dev *gspca_dev)
@@ -809,9 +809,7 @@
 #endif
 	sd->awb = AWB_DEF;
 	sd->aec = AEC_DEF;
-#if SHARPNESS_DEF != 0
 	sd->sharpness = SHARPNESS_DEF;
-#endif
 #if HFLIP_DEF != 0
 	sd->hflip = HFLIP_DEF;
 #endif

[PATCH 08/11] ov534: Fix unsetting hflip and vflip bits

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

Also set default values unconditionally, for readability.

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -764,7 +764,7 @@
 				sccb_reg_read(gspca_dev, 0x0c) | 0x40);
 	else
 		sccb_reg_write(gspca_dev, 0x0c,
-				sccb_reg_read(gspca_dev, 0x0c) & 0xbf);
+				sccb_reg_read(gspca_dev, 0x0c) & ~0x40);
 }
 
 static void setvflip(struct gspca_dev *gspca_dev)
@@ -776,7 +776,7 @@
 				sccb_reg_read(gspca_dev, 0x0c) | 0x80);
 	else
 		sccb_reg_write(gspca_dev, 0x0c,
-				sccb_reg_read(gspca_dev, 0x0c) & 0x7f);
+				sccb_reg_read(gspca_dev, 0x0c) & ~0x80);
 }
 
 /* this function is called at probe time */
@@ -810,12 +810,8 @@
 	sd->awb = AWB_DEF;
 	sd->aec = AEC_DEF;
 	sd->sharpness = SHARPNESS_DEF;
-#if HFLIP_DEF != 0
 	sd->hflip = HFLIP_DEF;
-#endif
-#if VFLIP_DEF != 0
 	sd->vflip = VFLIP_DEF;
-#endif
 
 	return 0;
 }

[PATCH 09/11] ov534: Cosmetics: fix indentation and hex digits

[Antonio Ospite]

  * Indent with tabs, not with spaces.
  * Less indentation for controls index comments.
  * Use lowercase hex digits.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |  128 ++++++++++++++++----------------
 1 file changed, 64 insertions(+), 64 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -92,7 +92,7 @@
 static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
 
 static const struct ctrl sd_ctrls[] = {
-    {							/* 0 */
+{	/* 0 */
 	{
 		.id      = V4L2_CID_BRIGHTNESS,
 		.type    = V4L2_CTRL_TYPE_INTEGER,
@@ -105,8 +105,8 @@
 	},
 	.set = sd_setbrightness,
 	.get = sd_getbrightness,
-    },
-    {							/* 1 */
+},
+{	/* 1 */
 	{
 		.id      = V4L2_CID_CONTRAST,
 		.type    = V4L2_CTRL_TYPE_INTEGER,
@@ -119,51 +119,51 @@
 	},
 	.set = sd_setcontrast,
 	.get = sd_getcontrast,
-    },
-    {							/* 2 */
+},
+{	/* 2 */
 	{
-	    .id      = V4L2_CID_GAIN,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Main Gain",
-	    .minimum = 0,
-	    .maximum = 63,
-	    .step    = 1,
+		.id      = V4L2_CID_GAIN,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Main Gain",
+		.minimum = 0,
+		.maximum = 63,
+		.step    = 1,
 #define GAIN_DEF 20
-	    .default_value = GAIN_DEF,
+		.default_value = GAIN_DEF,
 	},
 	.set = sd_setgain,
 	.get = sd_getgain,
-    },
-    {							/* 3 */
+},
+{	/* 3 */
 	{
-	    .id      = V4L2_CID_EXPOSURE,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Exposure",
-	    .minimum = 0,
-	    .maximum = 506,
-	    .step    = 1,
+		.id      = V4L2_CID_EXPOSURE,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Exposure",
+		.minimum = 0,
+		.maximum = 506,
+		.step    = 1,
 #define EXPO_DEF 120
-	    .default_value = EXPO_DEF,
+		.default_value = EXPO_DEF,
 	},
 	.set = sd_setexposure,
 	.get = sd_getexposure,
-    },
-    {							/* 4 */
+},
+{	/* 4 */
 	{
-	    .id      = V4L2_CID_AUTOGAIN,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "Auto Gain",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+		.id      = V4L2_CID_AUTOGAIN,
+		.type    = V4L2_CTRL_TYPE_BOOLEAN,
+		.name    = "Auto Gain",
+		.minimum = 0,
+		.maximum = 1,
+		.step    = 1,
 #define AGC_DEF 1
-	    .default_value = AGC_DEF,
+		.default_value = AGC_DEF,
 	},
 	.set = sd_setagc,
 	.get = sd_getagc,
-    },
+},
 #define AWB_IDX 5
-    {							/* 5 */
+{	/* 5 */
 	{
 		.id      = V4L2_CID_AUTO_WHITE_BALANCE,
 		.type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -176,8 +176,8 @@
 	},
 	.set = sd_setawb,
 	.get = sd_getawb,
-    },
-    {							/* 6 */
+},
+{	/* 6 */
 	{
 		.id      = V4L2_CID_EXPOSURE_AUTO,
 		.type    = V4L2_CTRL_TYPE_BOOLEAN,
@@ -190,49 +190,49 @@
 	},
 	.set = sd_setaec,
 	.get = sd_getaec,
-    },
-    {							/* 7 */
+},
+{	/* 7 */
 	{
-	    .id      = V4L2_CID_SHARPNESS,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Sharpness",
-	    .minimum = 0,
-	    .maximum = 63,
-	    .step    = 1,
+		.id      = V4L2_CID_SHARPNESS,
+		.type    = V4L2_CTRL_TYPE_INTEGER,
+		.name    = "Sharpness",
+		.minimum = 0,
+		.maximum = 63,
+		.step    = 1,
 #define SHARPNESS_DEF 0
-	    .default_value = SHARPNESS_DEF,
+		.default_value = SHARPNESS_DEF,
 	},
 	.set = sd_setsharpness,
 	.get = sd_getsharpness,
-    },
-    {							/* 8 */
+},
+{	/* 8 */
 	{
-	    .id      = V4L2_CID_HFLIP,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "HFlip",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+		.id      = V4L2_CID_HFLIP,
+		.type    = V4L2_CTRL_TYPE_BOOLEAN,
+		.name    = "HFlip",
+		.minimum = 0,
+		.maximum = 1,
+		.step    = 1,
 #define HFLIP_DEF 0
-	    .default_value = HFLIP_DEF,
+		.default_value = HFLIP_DEF,
 	},
 	.set = sd_sethflip,
 	.get = sd_gethflip,
-    },
-    {							/* 9 */
+},
+{	/* 9 */
 	{
-	    .id      = V4L2_CID_VFLIP,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "VFlip",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+		.id      = V4L2_CID_VFLIP,
+		.type    = V4L2_CTRL_TYPE_BOOLEAN,
+		.name    = "VFlip",
+		.minimum = 0,
+		.maximum = 1,
+		.step    = 1,
 #define VFLIP_DEF 0
-	    .default_value = VFLIP_DEF,
+		.default_value = VFLIP_DEF,
 	},
 	.set = sd_setvflip,
 	.get = sd_getvflip,
-    },
+},
 };
 
 static const struct v4l2_pix_format ov772x_mode[] = {
@@ -641,14 +641,14 @@
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	sccb_reg_write(gspca_dev, 0x9B, sd->brightness);
+	sccb_reg_write(gspca_dev, 0x9b, sd->brightness);
 }
 
 static void setcontrast(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	sccb_reg_write(gspca_dev, 0x9C, sd->contrast);
+	sccb_reg_write(gspca_dev, 0x9c, sd->contrast);
 }
 
 static void setgain(struct gspca_dev *gspca_dev)

[PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

From: Mosalam Ebrahimi <m.ebrahimi@ieee.org>

Note that setting this options to 50Hz can reduce the framerate, so the
default is still 60Hz.

Signed-off-by: Mosalam Ebrahimi <m.ebrahimi@ieee.org>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |   71 +++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -66,7 +66,7 @@
 	s8 sharpness;
 	u8 hflip;
 	u8 vflip;
-
+	u8 freqfltr;
 };
 
 /* V4L2 controls supported by the driver */
@@ -90,6 +90,10 @@
 static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+		struct v4l2_querymenu *menu);
 
 static const struct ctrl sd_ctrls[] = {
 {	/* 0 */
@@ -233,6 +237,20 @@
 	.set = sd_setvflip,
 	.get = sd_getvflip,
 },
+{	/* 10 */
+	{
+		.id      = V4L2_CID_POWER_LINE_FREQUENCY,
+		.type    = V4L2_CTRL_TYPE_MENU,
+		.name    = "Light Frequency Filter",
+		.minimum = 0,
+		.maximum = 1,
+		.step    = 1,
+#define FREQFLTR_DEF 1
+		.default_value = FREQFLTR_DEF,
+	},
+	.set = sd_setfreqfltr,
+	.get = sd_getfreqfltr,
+},
 };
 
 static const struct v4l2_pix_format ov772x_mode[] = {
@@ -779,6 +797,17 @@
 				sccb_reg_read(gspca_dev, 0x0c) & ~0x80);
 }
 
+static void setfreqfltr(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	if (sd->freqfltr == 0)
+		sccb_reg_write(gspca_dev, 0x2b, 0x9e);
+	else
+		sccb_reg_write(gspca_dev, 0x2b, 0x00);
+}
+
+
 /* this function is called at probe time */
 static int sd_config(struct gspca_dev *gspca_dev,
 		     const struct usb_device_id *id)
@@ -812,6 +841,7 @@
 	sd->sharpness = SHARPNESS_DEF;
 	sd->hflip = HFLIP_DEF;
 	sd->vflip = VFLIP_DEF;
+	sd->freqfltr = FREQFLTR_DEF;
 
 	return 0;
 }
@@ -881,6 +911,7 @@
 	setsharpness(gspca_dev);
 	setvflip(gspca_dev);
 	sethflip(gspca_dev);
+	setfreqfltr(gspca_dev);
 
 	ov534_set_led(gspca_dev, 1);
 	ov534_reg_write(gspca_dev, 0xe0, 0x00);
@@ -1174,6 +1205,43 @@
 	return 0;
 }
 
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	sd->freqfltr = val;
+	if (gspca_dev->streaming)
+		setfreqfltr(gspca_dev);
+	return 0;
+}
+
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	*val = sd->freqfltr;
+	return 0;
+}
+
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+		struct v4l2_querymenu *menu)
+{
+	switch (menu->id) {
+	case V4L2_CID_POWER_LINE_FREQUENCY:
+		switch (menu->index) {
+		case 0:         /* V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
+			strcpy((char *) menu->name, "50 Hz");
+			return 0;
+		case 1:         /* V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
+			strcpy((char *) menu->name, "60 Hz");
+			return 0;
+		}
+		break;
+	}
+
+	return -EINVAL;
+}
+
 /* get stream parameters (framerate) */
 static int sd_get_streamparm(struct gspca_dev *gspca_dev,
 			     struct v4l2_streamparm *parm)
@@ -1225,6 +1293,7 @@
 	.start    = sd_start,
 	.stopN    = sd_stopN,
 	.pkt_scan = sd_pkt_scan,
+	.querymenu = sd_querymenu,
 	.get_streamparm = sd_get_streamparm,
 	.set_streamparm = sd_set_streamparm,
 };

[PATCH 11/11] ov534: Update copyright info

[Antonio Ospite]

From: Max Thrun <bear24rw@gmail.com>

Signed-off-by: Max Thrun <bear24rw@gmail.com>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 linux/drivers/media/video/gspca/ov534.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -10,8 +10,8 @@
  * https://jim.sh/svn/jim/devl/playstation/ps3/eye/test/
  *
  * PS3 Eye camera enhanced by Richard Kaswy http://kaswy.free.fr
- * PS3 Eye camera, brightness, contrast, hue, AWB control added
- *	by Max Thrun <bear24rw@gmail.com>
+ * PS3 Eye camera - brightness, contrast, awb, agc, aec controls
+ *                  added by Max Thrun <bear24rw@gmail.com>
  *
  * 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

Re: [PATCH 05/11] ov534: Fix setting manual exposure

[Jean-Francois Moine]

On Sat, 27 Feb 2010 21:20:22 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Exposure is now a u16 value, both MSB and LSB are set, but values in
> the v4l2 control are limited to the interval [0,506] as 0x01fa (506)
> is the maximum observed value with AEC enabled.
	[snip]
>  	    .type    = V4L2_CTRL_TYPE_INTEGER,
>  	    .name    = "Exposure",
>  	    .minimum = 0,
> -	    .maximum = 255,
> +	    .maximum = 506,
>  	    .step    = 1,
>  #define EXPO_DEF 120
>  	    .default_value = EXPO_DEF,

Hi Antonio,

Do we need a so high precision for the exposure? Just setting the
maximum value to 253 should solve the problem.

Cheers.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 09/11] ov534: Cosmetics: fix indentation and hex digits

[Jean-Francois Moine]

On Sat, 27 Feb 2010 21:20:26 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

>   * Indent with tabs, not with spaces.
>   * Less indentation for controls index comments.
	[snip]
> -    },
> +},
>  };

I had preferred one more TAB for all controls.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jean-Francois Moine]

On Sat, 27 Feb 2010 21:20:27 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> +static int sd_querymenu(struct gspca_dev *gspca_dev,
> +		struct v4l2_querymenu *menu)
> +{
> +	switch (menu->id) {
> +	case V4L2_CID_POWER_LINE_FREQUENCY:
> +		switch (menu->index) {
> +		case 0:         /*
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
> +			strcpy((char *) menu->name, "50 Hz");
> +			return 0;
> +		case 1:         /*
> V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
> +			strcpy((char *) menu->name, "60 Hz");
> +			return 0;
> +		}
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}

In videodev2.h, there is:

V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
V4L2_CID_POWER_LINE_FREQUENCY_60HZ      = 2,

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 05/11] ov534: Fix setting manual exposure

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

On Sun, 28 Feb 2010 19:38:14 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Sat, 27 Feb 2010 21:20:22 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > Exposure is now a u16 value, both MSB and LSB are set, but values in
> > the v4l2 control are limited to the interval [0,506] as 0x01fa (506)
> > is the maximum observed value with AEC enabled.
> 	[snip]
> >  	    .type    = V4L2_CTRL_TYPE_INTEGER,
> >  	    .name    = "Exposure",
> >  	    .minimum = 0,
> > -	    .maximum = 255,
> > +	    .maximum = 506,
> >  	    .step    = 1,
> >  #define EXPO_DEF 120
> >  	    .default_value = EXPO_DEF,
> 
> Hi Antonio,
> 
> Do we need a so high precision for the exposure? Just setting the
> maximum value to 253 should solve the problem.
>

JF, the intent here is to cover all the range of values available in
Auto Exposure mode too, doesn't this make sense to you?

I could set .maximum to 253 to limit the "UI" control precision but then
I should use 2*value when setting the registers in order to cover the
actual max value, this looks a little unclean.

Anyhow, let me know what you prefer, I have no strong feelings on that.
If you want to save a byte, I'll agree.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 09/11] ov534: Cosmetics: fix indentation and hex digits

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Sun, 28 Feb 2010 19:46:36 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Sat, 27 Feb 2010 21:20:26 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> >   * Indent with tabs, not with spaces.
> >   * Less indentation for controls index comments.
> 	[snip]
> > -    },
> > +},
> >  };
> 
> I had preferred one more TAB for all controls.

I found it redundant, but I am preparing a v2 patch as per your request
now.

I'll also need to refresh patch 10, will send a v2 for it too after
discussing your comments on that one.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

On Sun, 28 Feb 2010 19:49:51 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Sat, 27 Feb 2010 21:20:27 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > +static int sd_querymenu(struct gspca_dev *gspca_dev,
> > +		struct v4l2_querymenu *menu)
> > +{
> > +	switch (menu->id) {
> > +	case V4L2_CID_POWER_LINE_FREQUENCY:
> > +		switch (menu->index) {
> > +		case 0:         /*
> > V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
> > +			strcpy((char *) menu->name, "50 Hz");
> > +			return 0;
> > +		case 1:         /*
> > V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
> > +			strcpy((char *) menu->name, "60 Hz");
> > +			return 0;
> > +		}
> > +		break;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> In videodev2.h, there is:
> 
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
> V4L2_CID_POWER_LINE_FREQUENCY_60HZ      = 2,
>

Maybe we could just use
	V4L2_CID_POWER_LINE_FREQUENCY_DISABLED	= 0,
	V4L2_CID_POWER_LINE_FREQUENCY_50HZ	= 1,

It looks like the code matches the DISABLED state (writing 0 to the
register). Mosalam?

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 05/11] ov534: Fix setting manual exposure

[Jean-Francois Moine]

On Sun, 28 Feb 2010 19:54:25 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> JF, the intent here is to cover all the range of values available in
> Auto Exposure mode too, doesn't this make sense to you?
> 
> I could set .maximum to 253 to limit the "UI" control precision but
> then I should use 2*value when setting the registers in order to
> cover the actual max value, this looks a little unclean.
> 
> Anyhow, let me know what you prefer, I have no strong feelings on
> that. If you want to save a byte, I'll agree.

Looking at the rare chip documents I got, I often saw internal control
values on 3 bytes (16 million values). Could anybody see the difference
between values v and (v+1)?

So, often, even when the internal controls are stored in only one byte,
the ms-win drivers propose only ranges as 0..30 or 0..100.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jean-Francois Moine]

On Sun, 28 Feb 2010 20:18:50 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Maybe we could just use
> 	V4L2_CID_POWER_LINE_FREQUENCY_DISABLED	= 0,
> 	V4L2_CID_POWER_LINE_FREQUENCY_50HZ	= 1,
> 
> It looks like the code matches the DISABLED state (writing 0 to the
> register). Mosalam?

I don't know the ov772x sensor. I think it should look like the ov7670
where there are 3 registers to control the light frequency: one
register tells if light frequency filter must be used, and which
frequency 50Hz or 60Hz; the two other ones give the filter values for
each frequency.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 05/11] ov534: Fix setting manual exposure

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On Sun, 28 Feb 2010 19:54:25 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Sun, 28 Feb 2010 19:38:14 +0100
> Jean-Francois Moine <moinejf@free.fr> wrote:
> 
> > On Sat, 27 Feb 2010 21:20:22 +0100
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > 
> > > Exposure is now a u16 value, both MSB and LSB are set, but values in
> > > the v4l2 control are limited to the interval [0,506] as 0x01fa (506)
> > > is the maximum observed value with AEC enabled.
> > 	[snip]
> > >  	    .type    = V4L2_CTRL_TYPE_INTEGER,
> > >  	    .name    = "Exposure",
> > >  	    .minimum = 0,
> > > -	    .maximum = 255,
> > > +	    .maximum = 506,
> > >  	    .step    = 1,
> > >  #define EXPO_DEF 120
> > >  	    .default_value = EXPO_DEF,
> > 
> > Hi Antonio,
> > 
> > Do we need a so high precision for the exposure? Just setting the
> > maximum value to 253 should solve the problem.
> >
> 
> JF, the intent here is to cover all the range of values available in
> Auto Exposure mode too, doesn't this make sense to you?
> 
> I could set .maximum to 253 to limit the "UI" control precision but then
> I should use 2*value when setting the registers in order to cover the
> actual max value, this looks a little unclean.
>

Ok, I now see that that's exactly what current code does, sorry for the
noise. The patch then degenerates to a simpler one with some
documentation added, so others don't overlook the code like I did.

Sending it in a min.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH v2 05/11] ov534: Fix and document setting manual exposure

[Antonio Ospite]

Document that even if the state is a u8 value, both MSB and LSB are set
as sd->exposure represents half of the value we are going to set into
registers.

Skip setting exposure when AEC is enabled.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
The code was already doing the right thing, I just overlooked it.

Regards,
   Antonio

 linux/drivers/media/video/gspca/ov534.c |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -686,6 +686,15 @@
 	struct sd *sd = (struct sd *) gspca_dev;
 	u8 val;
 
+	if (sd->aec)
+		return;
+
+	/* 'val' is one byte and represents half of the exposure value we are
+	 * going to set into registers, a two bytes value:
+	 * 
+	 *    MSB: ((u16) val << 1) >> 8   == val >> 7
+	 *    LSB: ((u16) val << 1) & 0xff == val << 1
+	 */
 	val = sd->exposure;
 	sccb_reg_write(gspca_dev, 0x08, val >> 7);
 	sccb_reg_write(gspca_dev, 0x10, val << 1);

[PATCH v2 09/11] ov534: Cosmetics: fix indentation and hex digits

[Antonio Ospite]

  * Indent with tabs, not with spaces, nor with mixed style.
  * Less indentation for controls index comments.
  * Use lowercase hex digits.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
Changes since v1:

Indent controls by one more level as JF requested.
Note that the controls "index comments" are still less indented than before,
it is more readable this way.

Thanks,
   Antonio

 linux/drivers/media/video/gspca/ov534.c |  260 ++++++++++++++++----------------
 1 file changed, 130 insertions(+), 130 deletions(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -92,147 +92,147 @@
 static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
 
 static const struct ctrl sd_ctrls[] = {
-    {							/* 0 */
-	{
-		.id      = V4L2_CID_BRIGHTNESS,
-		.type    = V4L2_CTRL_TYPE_INTEGER,
-		.name    = "Brightness",
-		.minimum = 0,
-		.maximum = 255,
-		.step    = 1,
+	{	/* 0 */
+		{
+			.id      = V4L2_CID_BRIGHTNESS,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Brightness",
+			.minimum = 0,
+			.maximum = 255,
+			.step    = 1,
 #define BRIGHTNESS_DEF 0
-		.default_value = BRIGHTNESS_DEF,
-	},
-	.set = sd_setbrightness,
-	.get = sd_getbrightness,
-    },
-    {							/* 1 */
-	{
-		.id      = V4L2_CID_CONTRAST,
-		.type    = V4L2_CTRL_TYPE_INTEGER,
-		.name    = "Contrast",
-		.minimum = 0,
-		.maximum = 255,
-		.step    = 1,
+			.default_value = BRIGHTNESS_DEF,
+		},
+		.set = sd_setbrightness,
+		.get = sd_getbrightness,
+	},
+	{	/* 1 */
+		{
+			.id      = V4L2_CID_CONTRAST,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Contrast",
+			.minimum = 0,
+			.maximum = 255,
+			.step    = 1,
 #define CONTRAST_DEF 32
-		.default_value = CONTRAST_DEF,
-	},
-	.set = sd_setcontrast,
-	.get = sd_getcontrast,
-    },
-    {							/* 2 */
-	{
-	    .id      = V4L2_CID_GAIN,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Main Gain",
-	    .minimum = 0,
-	    .maximum = 63,
-	    .step    = 1,
+			.default_value = CONTRAST_DEF,
+		},
+		.set = sd_setcontrast,
+		.get = sd_getcontrast,
+	},
+	{	/* 2 */
+		{
+			.id      = V4L2_CID_GAIN,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Main Gain",
+			.minimum = 0,
+			.maximum = 63,
+			.step    = 1,
 #define GAIN_DEF 20
-	    .default_value = GAIN_DEF,
-	},
-	.set = sd_setgain,
-	.get = sd_getgain,
-    },
-    {							/* 3 */
-	{
-	    .id      = V4L2_CID_EXPOSURE,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Exposure",
-	    .minimum = 0,
-	    .maximum = 255,
-	    .step    = 1,
+			.default_value = GAIN_DEF,
+		},
+		.set = sd_setgain,
+		.get = sd_getgain,
+	},
+	{	/* 3 */
+		{
+			.id      = V4L2_CID_EXPOSURE,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Exposure",
+			.minimum = 0,
+			.maximum = 255,
+			.step    = 1,
 #define EXPO_DEF 120
-	    .default_value = EXPO_DEF,
-	},
-	.set = sd_setexposure,
-	.get = sd_getexposure,
-    },
-    {							/* 4 */
-	{
-	    .id      = V4L2_CID_AUTOGAIN,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "Auto Gain",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+			.default_value = EXPO_DEF,
+		},
+		.set = sd_setexposure,
+		.get = sd_getexposure,
+	},
+	{	/* 4 */
+		{
+			.id      = V4L2_CID_AUTOGAIN,
+			.type    = V4L2_CTRL_TYPE_BOOLEAN,
+			.name    = "Auto Gain",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
 #define AGC_DEF 1
-	    .default_value = AGC_DEF,
+			.default_value = AGC_DEF,
+		},
+		.set = sd_setagc,
+		.get = sd_getagc,
 	},
-	.set = sd_setagc,
-	.get = sd_getagc,
-    },
 #define AWB_IDX 5
-    {							/* 5 */
-	{
-		.id      = V4L2_CID_AUTO_WHITE_BALANCE,
-		.type    = V4L2_CTRL_TYPE_BOOLEAN,
-		.name    = "Auto White Balance",
-		.minimum = 0,
-		.maximum = 1,
-		.step    = 1,
+	{	/* 5 */
+		{
+			.id      = V4L2_CID_AUTO_WHITE_BALANCE,
+			.type    = V4L2_CTRL_TYPE_BOOLEAN,
+			.name    = "Auto White Balance",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
 #define AWB_DEF 1
-		.default_value = AWB_DEF,
-	},
-	.set = sd_setawb,
-	.get = sd_getawb,
-    },
-    {							/* 6 */
-	{
-		.id      = V4L2_CID_EXPOSURE_AUTO,
-		.type    = V4L2_CTRL_TYPE_BOOLEAN,
-		.name    = "Auto Exposure",
-		.minimum = 0,
-		.maximum = 1,
-		.step    = 1,
+			.default_value = AWB_DEF,
+		},
+		.set = sd_setawb,
+		.get = sd_getawb,
+	},
+	{	/* 6 */
+		{
+			.id      = V4L2_CID_EXPOSURE_AUTO,
+			.type    = V4L2_CTRL_TYPE_BOOLEAN,
+			.name    = "Auto Exposure",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
 #define AEC_DEF 1
-		.default_value = AEC_DEF,
-	},
-	.set = sd_setaec,
-	.get = sd_getaec,
-    },
-    {							/* 7 */
-	{
-	    .id      = V4L2_CID_SHARPNESS,
-	    .type    = V4L2_CTRL_TYPE_INTEGER,
-	    .name    = "Sharpness",
-	    .minimum = 0,
-	    .maximum = 63,
-	    .step    = 1,
+			.default_value = AEC_DEF,
+		},
+		.set = sd_setaec,
+		.get = sd_getaec,
+	},
+	{	/* 7 */
+		{
+			.id      = V4L2_CID_SHARPNESS,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Sharpness",
+			.minimum = 0,
+			.maximum = 63,
+			.step    = 1,
 #define SHARPNESS_DEF 0
-	    .default_value = SHARPNESS_DEF,
-	},
-	.set = sd_setsharpness,
-	.get = sd_getsharpness,
-    },
-    {							/* 8 */
-	{
-	    .id      = V4L2_CID_HFLIP,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "HFlip",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+			.default_value = SHARPNESS_DEF,
+		},
+		.set = sd_setsharpness,
+		.get = sd_getsharpness,
+	},
+	{	/* 8 */
+		{
+			.id      = V4L2_CID_HFLIP,
+			.type    = V4L2_CTRL_TYPE_BOOLEAN,
+			.name    = "HFlip",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
 #define HFLIP_DEF 0
-	    .default_value = HFLIP_DEF,
-	},
-	.set = sd_sethflip,
-	.get = sd_gethflip,
-    },
-    {							/* 9 */
-	{
-	    .id      = V4L2_CID_VFLIP,
-	    .type    = V4L2_CTRL_TYPE_BOOLEAN,
-	    .name    = "VFlip",
-	    .minimum = 0,
-	    .maximum = 1,
-	    .step    = 1,
+			.default_value = HFLIP_DEF,
+		},
+		.set = sd_sethflip,
+		.get = sd_gethflip,
+	},
+	{	/* 9 */
+		{
+			.id      = V4L2_CID_VFLIP,
+			.type    = V4L2_CTRL_TYPE_BOOLEAN,
+			.name    = "VFlip",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
 #define VFLIP_DEF 0
-	    .default_value = VFLIP_DEF,
+			.default_value = VFLIP_DEF,
+		},
+		.set = sd_setvflip,
+		.get = sd_getvflip,
 	},
-	.set = sd_setvflip,
-	.get = sd_getvflip,
-    },
 };
 
 static const struct v4l2_pix_format ov772x_mode[] = {
@@ -641,14 +641,14 @@
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	sccb_reg_write(gspca_dev, 0x9B, sd->brightness);
+	sccb_reg_write(gspca_dev, 0x9b, sd->brightness);
 }
 
 static void setcontrast(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
 
-	sccb_reg_write(gspca_dev, 0x9C, sd->contrast);
+	sccb_reg_write(gspca_dev, 0x9c, sd->contrast);
 }
 
 static void setgain(struct gspca_dev *gspca_dev)

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[M.Ebrahimi]

On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Sun, 28 Feb 2010 20:18:50 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
>
>> Maybe we could just use
>>       V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
>>       V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
>>
>> It looks like the code matches the DISABLED state (writing 0 to the
>> register). Mosalam?
>
> I don't know the ov772x sensor. I think it should look like the ov7670
> where there are 3 registers to control the light frequency: one
> register tells if light frequency filter must be used, and which
> frequency 50Hz or 60Hz; the two other ones give the filter values for
> each frequency.
>

I think it's safe to go with disabled/50hz. Perhaps later if needed
can patch it to control the filter values. Since it seems there is no
flickering in the 60hz regions at available frame rates, and this
register almost perfectly removes light flickers in the 50hz regions
(by modifying exposure/frame rate).

Mosalam

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Tue, 2 Mar 2010 11:26:15 +0000
"M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:

> On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
> > On Sun, 28 Feb 2010 20:18:50 +0100
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> >
> >> Maybe we could just use
> >>       V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
> >>       V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
> >>
> >> It looks like the code matches the DISABLED state (writing 0 to the
> >> register). Mosalam?
> >
> > I don't know the ov772x sensor. I think it should look like the ov7670
> > where there are 3 registers to control the light frequency: one
> > register tells if light frequency filter must be used, and which
> > frequency 50Hz or 60Hz; the two other ones give the filter values for
> > each frequency.
> >
> 
> I think it's safe to go with disabled/50hz. Perhaps later if needed
> can patch it to control the filter values. Since it seems there is no
> flickering in the 60hz regions at available frame rates, and this
> register almost perfectly removes light flickers in the 50hz regions
> (by modifying exposure/frame rate).
>
> Mosalam
>

Mosalam did you spot the register from a PS3 usb dump or by looking at
the sensor datasheet?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[M.Ebrahimi]

On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
>
>
> On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> wrote:
>>
>> On Tue, 2 Mar 2010 11:26:15 +0000
>> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
>>
>> > On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
>> > > On Sun, 28 Feb 2010 20:18:50 +0100
>> > > Antonio Ospite <ospite@studenti.unina.it> wrote:
>> > >
>> > >> Maybe we could just use
>> > >>       V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
>> > >>       V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
>> > >>
>> > >> It looks like the code matches the DISABLED state (writing 0 to the
>> > >> register). Mosalam?
>> > >
>> > > I don't know the ov772x sensor. I think it should look like the ov7670
>> > > where there are 3 registers to control the light frequency: one
>> > > register tells if light frequency filter must be used, and which
>> > > frequency 50Hz or 60Hz; the two other ones give the filter values for
>> > > each frequency.
>> > >
>> >
>> > I think it's safe to go with disabled/50hz. Perhaps later if needed
>> > can patch it to control the filter values. Since it seems there is no
>> > flickering in the 60hz regions at available frame rates, and this
>> > register almost perfectly removes light flickers in the 50hz regions
>> > (by modifying exposure/frame rate).
>> >
>> > Mosalam
>> >
>>
>> Mosalam did you spot the register from a PS3 usb dump or by looking at
>> the sensor datasheet?

None, I got that register from sniffing a Windows driver for another
camera that turned out to be using ov7620 or something similar, though
I thought it has the same sensor. I double checked, this register is
for frame rate adjustment (decreasing frame rate / increasing
exposure) . And this has been used in some other drivers (e.g.
gspca_sonixb) to remove light flicker as well.

>>
>> --
>> Antonio Ospite
>> http://ao2.it
>>
>> PGP public key ID: 0x4553B001
>>
>> A: Because it messes up the order in which people normally read text.
>>   See http://en.wikipedia.org/wiki/Posting_style
>> Q: Why is top-posting such a bad thing?
>> A: Top-posting.
>> Q: What is the most annoying thing in e-mail?
>
> I'd also like to know where you got the 2b register from, cause someone else
> also said 2b was filtering but the datasheet says it LSB of dummy pixel...
>
>- Max Thrun

Definitely it is adjusting the frame rate (see the ov7620 DS for the
description how the register value is used, for instance). I have no
idea why the ov7720 datasheet says otherwise.

Since this patch does not use the banding filter registers mentioned
in the datasheet maybe should be discarded. I am working on 75 FPS at
VGA, when I get that working well I can get back to this again.

Thanks for the comments.
Mosalam

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[M.Ebrahimi]

On 3 March 2010 02:30, Max Thrun <bear24rw@gmail.com> wrote:
>
>
> On Tue, Mar 2, 2010 at 9:27 PM, M.Ebrahimi <m.ebrahimi@ieee.org> wrote:
>>
>> On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
>> >
>> >
>> > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite
>> > <ospite@studenti.unina.it>
>> > wrote:
>> >>
>> >> On Tue, 2 Mar 2010 11:26:15 +0000
>> >> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
>> >>
>> >> > On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr>
>> >> > wrote:
>> >> > > On Sun, 28 Feb 2010 20:18:50 +0100
>> >> > > Antonio Ospite <ospite@studenti.unina.it> wrote:
>> >> > >
>> >> > >> Maybe we could just use
>> >> > >>       V4L2_CID_POWER_LINE_FREQUENCY_DISABLED  = 0,
>> >> > >>       V4L2_CID_POWER_LINE_FREQUENCY_50HZ      = 1,
>> >> > >>
>> >> > >> It looks like the code matches the DISABLED state (writing 0 to
>> >> > >> the
>> >> > >> register). Mosalam?
>> >> > >
>> >> > > I don't know the ov772x sensor. I think it should look like the
>> >> > > ov7670
>> >> > > where there are 3 registers to control the light frequency: one
>> >> > > register tells if light frequency filter must be used, and which
>> >> > > frequency 50Hz or 60Hz; the two other ones give the filter values
>> >> > > for
>> >> > > each frequency.
>> >> > >
>> >> >
>> >> > I think it's safe to go with disabled/50hz. Perhaps later if needed
>> >> > can patch it to control the filter values. Since it seems there is no
>> >> > flickering in the 60hz regions at available frame rates, and this
>> >> > register almost perfectly removes light flickers in the 50hz regions
>> >> > (by modifying exposure/frame rate).
>> >> >
>> >> > Mosalam
>> >> >
>> >>
>> >> Mosalam did you spot the register from a PS3 usb dump or by looking at
>> >> the sensor datasheet?
>>
>> None, I got that register from sniffing a Windows driver for another
>> camera that turned out to be using ov7620 or something similar, though
>> I thought it has the same sensor. I double checked, this register is
>> for frame rate adjustment (decreasing frame rate / increasing
>> exposure) . And this has been used in some other drivers (e.g.
>> gspca_sonixb) to remove light flicker as well.
>>
>> >>
>> >> --
>> >> Antonio Ospite
>> >> http://ao2.it
>> >>
>> >> PGP public key ID: 0x4553B001
>> >>
>> >> A: Because it messes up the order in which people normally read text.
>> >>   See http://en.wikipedia.org/wiki/Posting_style
>> >> Q: Why is top-posting such a bad thing?
>> >> A: Top-posting.
>> >> Q: What is the most annoying thing in e-mail?
>> >
>> > I'd also like to know where you got the 2b register from, cause someone
>> > else
>> > also said 2b was filtering but the datasheet says it LSB of dummy
>> > pixel...
>> >
>> >- Max Thrun
>>
>> Definitely it is adjusting the frame rate (see the ov7620 DS for the
>> description how the register value is used, for instance). I have no
>> idea why the ov7720 datasheet says otherwise.
>>
>> Since this patch does not use the banding filter registers mentioned
>> in the datasheet maybe should be discarded. I am working on 75 FPS at
>> VGA, when I get that working well I can get back to this again.
>>
>> Thanks for the comments.
>> Mosalam
>
> Do you have a link by any chance to ov7620 DS? Also 75 FPS at VGA would be
> awesome (if its possible?). I managed to get QVGA up to 200 the other day..
>
> -- Max Thrun
>

http://mxhaard.free.fr/spca50x/Doc/Omnivision/OV7620.pdf

Mosalam

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

On Wed, 3 Mar 2010 02:27:38 +0000
"M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:

> On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
> >
> >
> > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> > wrote:
[...]
> >> Mosalam did you spot the register from a PS3 usb dump or by looking at
> >> the sensor datasheet?
> 
> None, I got that register from sniffing a Windows driver for another
> camera that turned out to be using ov7620 or something similar, though
> I thought it has the same sensor. I double checked, this register is
> for frame rate adjustment (decreasing frame rate / increasing
> exposure) . And this has been used in some other drivers (e.g.
> gspca_sonixb) to remove light flicker as well.
> 

I see. It would be interesting to see how Powerline Frequency filtering
is done on PS3. I added Jim Paris on CC.

> >
> > I'd also like to know where you got the 2b register from, cause someone else
> > also said 2b was filtering but the datasheet says it LSB of dummy pixel...
> >
> >- Max Thrun
> 
> Definitely it is adjusting the frame rate (see the ov7620 DS for the
> description how the register value is used, for instance). I have no
> idea why the ov7720 datasheet says otherwise.
> 
> Since this patch does not use the banding filter registers mentioned
> in the datasheet maybe should be discarded. I am working on 75 FPS at
> VGA, when I get that working well I can get back to this again.
> 
> Thanks for the comments.
> Mosalam
> 

Ok, so Jean-Francois can you apply the patches except 10/11, please?
We are keeping this one for another round.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jean-Francois Moine]

On Wed, 3 Mar 2010 09:00:08 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Ok, so Jean-Francois can you apply the patches except 10/11, please?
> We are keeping this one for another round.

Hello ov534 team,

Actually, I have problems with the mercurial tree. I will apply your
changes as soon as everything will be resolved..

Cheers.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 847 bytes --]

On Wed, 3 Mar 2010 09:37:44 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Wed, 3 Mar 2010 09:00:08 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > Ok, so Jean-Francois can you apply the patches except 10/11, please?
> > We are keeping this one for another round.
> 
> Hello ov534 team,
> 
> Actually, I have problems with the mercurial tree. I will apply your
> changes as soon as everything will be resolved..
>

No problem. Just a question, will you switch to git anytime soon?

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jean-Francois Moine]

On Wed, 3 Mar 2010 11:57:18 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:

> No problem. Just a question, will you switch to git anytime soon?

I don't look at this yet, but I think I will have to do it soon...

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jim Paris]

Antonio Ospite wrote:
> On Wed, 3 Mar 2010 02:27:38 +0000
> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
> 
> > On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
> > >
> > >
> > > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> > > wrote:
> [...]
> > >> Mosalam did you spot the register from a PS3 usb dump or by looking at
> > >> the sensor datasheet?
> > 
> > None, I got that register from sniffing a Windows driver for another
> > camera that turned out to be using ov7620 or something similar, though
> > I thought it has the same sensor. I double checked, this register is
> > for frame rate adjustment (decreasing frame rate / increasing
> > exposure) . And this has been used in some other drivers (e.g.
> > gspca_sonixb) to remove light flicker as well.
> > 
> 
> I see. It would be interesting to see how Powerline Frequency filtering
> is done on PS3. I added Jim Paris on CC.

Hi Antonio and Mosalam,

I tried, but I can't capture that.  My USB logger only does USB 1.1,
which is too slow for the camera to run normally, but good enough to
see the initialization sequence.  However, the 50/60Hz option only
appears later, once the PS3 is receiving good frame data.

I can open up the camera and sniff the I2C bus instead.  It'll take
a little longer.

Interesting side note, the only change in the initialization sequence
between PS3 firmware 1.93 and 3.15 is 0x0C bit 6 -- horizontal flip :)
So they haven't made any improvements that we can borrow.

-jim

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

On Wed, 3 Mar 2010 23:55:33 -0500
Jim Paris <jim@jtan.com> wrote:

> Antonio Ospite wrote:
[...]
> > 
> > I see. It would be interesting to see how Powerline Frequency filtering
> > is done on PS3. I added Jim Paris on CC.
> 
> Hi Antonio and Mosalam,
> 
> I tried, but I can't capture that.  My USB logger only does USB 1.1,
> which is too slow for the camera to run normally, but good enough to
> see the initialization sequence.  However, the 50/60Hz option only
> appears later, once the PS3 is receiving good frame data.
> 
> I can open up the camera and sniff the I2C bus instead.  It'll take
> a little longer.
>

Thanks for your time Jim.

> Interesting side note, the only change in the initialization sequence
> between PS3 firmware 1.93 and 3.15 is 0x0C bit 6 -- horizontal flip :)
> So they haven't made any improvements that we can borrow.
>
> -jim
> 

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jim Paris]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

Antonio Ospite wrote:
> On Wed, 3 Mar 2010 23:55:33 -0500
> Jim Paris <jim@jtan.com> wrote:
> 
> > Antonio Ospite wrote:
> [...]
> > > 
> > > I see. It would be interesting to see how Powerline Frequency filtering
> > > is done on PS3. I added Jim Paris on CC.
> > 
> > Hi Antonio and Mosalam,
> > 
> > I tried, but I can't capture that.  My USB logger only does USB 1.1,
> > which is too slow for the camera to run normally, but good enough to
> > see the initialization sequence.  However, the 50/60Hz option only
> > appears later, once the PS3 is receiving good frame data.
> > 
> > I can open up the camera and sniff the I2C bus instead.  It'll take
> > a little longer.
> >
> 
> Thanks for your time Jim.

No problem, glad to help.
Looks like Mosalam's patch is correct:

--- i2c-60hz.log	2010-03-04 15:09:23.000000000 -0500
+++ i2c-50hz.log	2010-03-04 15:09:27.000000000 -0500
@@ -69,7 +69,7 @@
 ov_write_verify 8C E8
 ov_write_verify 8D 20
 ov_write_verify 0C 90
-ov_write_verify 2B 00
+ov_write_verify 2B 9E
 ov_write_verify 22 7F
 ov_write_verify 23 03
 ov_write_verify 11 01

I'll attach the full logs.

-jim

[-- Attachment #2: i2c-60hz.log --]
[-- Type: text/plain, Size: 1917 bytes --]

ov_write 12 80
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 3D 03
ov_write_verify 17 26
ov_write_verify 18 A0
ov_write_verify 19 07
ov_write_verify 1A F0
ov_write_verify 32 00
ov_write_verify 29 A0
ov_write_verify 2C F0
ov_write_verify 65 20
ov_write_verify 11 01
ov_write_verify 42 7F
ov_write_verify 63 E0
ov_write_verify 64 FF
ov_write_verify 66 00
ov_write_verify 13 F0
ov_write_verify 0D 41
ov_write_verify 0F C5
ov_write_verify 14 11
ov_write_verify 22 7F
ov_write_verify 23 03
ov_write_verify 24 40
ov_write_verify 25 30
ov_write_verify 26 A1
ov_write_verify 2A 00
ov_write_verify 2B 00
ov_write_verify 6B AA
ov_write_verify 13 FF
ov_write_verify 90 05
ov_write_verify 91 01
ov_write_verify 92 03
ov_write_verify 93 00
ov_write_verify 94 60
ov_write_verify 95 3C
ov_write_verify 96 24
ov_write_verify 97 1E
ov_write_verify 98 62
ov_write_verify 99 80
ov_write_verify 9A 1E
ov_write_verify 9B 08
ov_write_verify 9C 20
ov_write_verify 9E 81
ov_write_verify A6 04
ov_write_verify 7E 0C
ov_write_verify 7F 16
ov_write_verify 80 2A
ov_write_verify 81 4E
ov_write_verify 82 61
ov_write_verify 83 6F
ov_write_verify 84 7B
ov_write_verify 85 86
ov_write_verify 86 8E
ov_write_verify 87 97
ov_write_verify 88 A4
ov_write_verify 89 AF
ov_write_verify 8A C5
ov_write_verify 8B D7
ov_write_verify 8C E8
ov_write_verify 8D 20
ov_write_verify 0C 90
ov_write_verify 2B 00
ov_write_verify 22 7F
ov_write_verify 23 03
ov_write_verify 11 01
ov_read 0C 90
ov_write_verify 0C 90
ov_read 64 FF
ov_write_verify 64 FF
ov_read 0D 41
ov_write_verify 0D 41
ov_read 14 11
ov_write_verify 14 41
ov_read 0E 79
ov_write_verify 0E CD
ov_read AC FF
ov_write_verify AC BF
ov_write_verify 8E 00
ov_read 0C 90
ov_write_verify 0C D0

[-- Attachment #3: i2c-50hz.log --]
[-- Type: text/plain, Size: 1917 bytes --]

ov_write 12 80
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 11 01
ov_write_verify 3D 03
ov_write_verify 17 26
ov_write_verify 18 A0
ov_write_verify 19 07
ov_write_verify 1A F0
ov_write_verify 32 00
ov_write_verify 29 A0
ov_write_verify 2C F0
ov_write_verify 65 20
ov_write_verify 11 01
ov_write_verify 42 7F
ov_write_verify 63 E0
ov_write_verify 64 FF
ov_write_verify 66 00
ov_write_verify 13 F0
ov_write_verify 0D 41
ov_write_verify 0F C5
ov_write_verify 14 11
ov_write_verify 22 7F
ov_write_verify 23 03
ov_write_verify 24 40
ov_write_verify 25 30
ov_write_verify 26 A1
ov_write_verify 2A 00
ov_write_verify 2B 00
ov_write_verify 6B AA
ov_write_verify 13 FF
ov_write_verify 90 05
ov_write_verify 91 01
ov_write_verify 92 03
ov_write_verify 93 00
ov_write_verify 94 60
ov_write_verify 95 3C
ov_write_verify 96 24
ov_write_verify 97 1E
ov_write_verify 98 62
ov_write_verify 99 80
ov_write_verify 9A 1E
ov_write_verify 9B 08
ov_write_verify 9C 20
ov_write_verify 9E 81
ov_write_verify A6 04
ov_write_verify 7E 0C
ov_write_verify 7F 16
ov_write_verify 80 2A
ov_write_verify 81 4E
ov_write_verify 82 61
ov_write_verify 83 6F
ov_write_verify 84 7B
ov_write_verify 85 86
ov_write_verify 86 8E
ov_write_verify 87 97
ov_write_verify 88 A4
ov_write_verify 89 AF
ov_write_verify 8A C5
ov_write_verify 8B D7
ov_write_verify 8C E8
ov_write_verify 8D 20
ov_write_verify 0C 90
ov_write_verify 2B 9E
ov_write_verify 22 7F
ov_write_verify 23 03
ov_write_verify 11 01
ov_read 0C 90
ov_write_verify 0C 90
ov_read 64 FF
ov_write_verify 64 FF
ov_read 0D 41
ov_write_verify 0D 41
ov_read 14 11
ov_write_verify 14 41
ov_read 0E 79
ov_write_verify 0E CD
ov_read AC FF
ov_write_verify AC BF
ov_write_verify 8E 00
ov_read 0C 90
ov_write_verify 0C D0

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Jim Paris]

Max Thrun wrote:
> On Thu, Mar 4, 2010 at 3:14 PM, Jim Paris <jim@jtan.com> wrote:
> 
> > Antonio Ospite wrote:
> > > On Wed, 3 Mar 2010 23:55:33 -0500
> > > Jim Paris <jim@jtan.com> wrote:
> > >
> > > > Antonio Ospite wrote:
> > > [...]
> > > > >
> > > > > I see. It would be interesting to see how Powerline Frequency
> > filtering
> > > > > is done on PS3. I added Jim Paris on CC.
> > > >
> > > > Hi Antonio and Mosalam,
> > > >
> > > > I tried, but I can't capture that.  My USB logger only does USB 1.1,
> > > > which is too slow for the camera to run normally, but good enough to
> > > > see the initialization sequence.  However, the 50/60Hz option only
> > > > appears later, once the PS3 is receiving good frame data.
> > > >
> > > > I can open up the camera and sniff the I2C bus instead.  It'll take
> > > > a little longer.
> > > >
> > >
> > > Thanks for your time Jim.
> >
> > No problem, glad to help.
> > Looks like Mosalam's patch is correct:
> >
> > --- i2c-60hz.log        2010-03-04 15:09:23.000000000 -0500
> > +++ i2c-50hz.log        2010-03-04 15:09:27.000000000 -0500
> > @@ -69,7 +69,7 @@
> >  ov_write_verify 8C E8
> >  ov_write_verify 8D 20
> >  ov_write_verify 0C 90
> > -ov_write_verify 2B 00
> > +ov_write_verify 2B 9E
> >  ov_write_verify 22 7F
> >  ov_write_verify 23 03
> >  ov_write_verify 11 01
> >
> > I'll attach the full logs.
> >
> > -jim
> >
> 
> Jim, I'm really interested in how you went about sniffing the bus. Can you
> share some details about what you use to do it?

Sure.  I borrowed one of these from a friend:
  http://www.totalphase.com/products/beagle_ism/

and tapped the GND, SCL, and SDA pins off the camera EEPROM.
I exported a CSV from the Beagle software and used a quick Perl script
to parse it into a more useful read/write/write_verify format.

-jim

Re: [PATCH 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

On Thu, 4 Mar 2010 15:14:45 -0500
Jim Paris <jim@jtan.com> wrote:

> > On Wed, 3 Mar 2010 23:55:33 -0500
> > Jim Paris <jim@jtan.com> wrote:
> > 
> > > Antonio Ospite wrote:
[...]
> > > > It would be interesting to see how Powerline Frequency filtering
> > > > is done on PS3. I added Jim Paris on CC.
> > > 
> > > I can open up the camera and sniff the I2C bus instead.  It'll take
> > > a little longer.
> > >
[...]
> Looks like Mosalam's patch is correct:
> 
> --- i2c-60hz.log	2010-03-04 15:09:23.000000000 -0500
> +++ i2c-50hz.log	2010-03-04 15:09:27.000000000 -0500
> @@ -69,7 +69,7 @@
>  ov_write_verify 8C E8
>  ov_write_verify 8D 20
>  ov_write_verify 0C 90
> -ov_write_verify 2B 00
> +ov_write_verify 2B 9E
>  ov_write_verify 22 7F
>  ov_write_verify 23 03
>  ov_write_verify 11 01
> 

OK, thanks.
I am sending a v2 for patch 10/11 as well, using DISABLED/50Hz as
choices.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH v2 10/11] ov534: Add Powerline Frequency control

[Antonio Ospite]

From: Mosalam Ebrahimi <m.ebrahimi@ieee.org>

Note that setting this options to 50Hz can reduce the framerate, so it is
still disabled (60Hz) by default.

Signed-off-by: Mosalam Ebrahimi <m.ebrahimi@ieee.org>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
Changes since v1:

  * Using Disabled/50Hz instead of 50Hz/60Hz to match menu indices and
    V4L2_CID_POWER_LINE_FREQUENCY values definitions in v4l2 spec.

This addresses a comment by Jean-Francois.

Thanks,
   Antonio

 linux/drivers/media/video/gspca/ov534.c |   71 +++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

Index: gspca/linux/drivers/media/video/gspca/ov534.c
===================================================================
--- gspca.orig/linux/drivers/media/video/gspca/ov534.c
+++ gspca/linux/drivers/media/video/gspca/ov534.c
@@ -66,7 +66,7 @@
 	s8 sharpness;
 	u8 hflip;
 	u8 vflip;
-
+	u8 freqfltr;
 };
 
 /* V4L2 controls supported by the driver */
@@ -90,6 +90,10 @@
 static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
 static int sd_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
 static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+		struct v4l2_querymenu *menu);
 
 static const struct ctrl sd_ctrls[] = {
 	{	/* 0 */
@@ -233,6 +237,20 @@
 		.set = sd_setvflip,
 		.get = sd_getvflip,
 	},
+	{	/* 10 */
+		{
+			.id      = V4L2_CID_POWER_LINE_FREQUENCY,
+			.type    = V4L2_CTRL_TYPE_MENU,
+			.name    = "Light Frequency Filter",
+			.minimum = 0,
+			.maximum = 1,
+			.step    = 1,
+#define FREQFLTR_DEF 0
+			.default_value = FREQFLTR_DEF,
+		},
+		.set = sd_setfreqfltr,
+		.get = sd_getfreqfltr,
+	},
 };
 
 static const struct v4l2_pix_format ov772x_mode[] = {
@@ -784,6 +802,17 @@
 				sccb_reg_read(gspca_dev, 0x0c) & ~0x80);
 }
 
+static void setfreqfltr(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	if (sd->freqfltr == 0)
+		sccb_reg_write(gspca_dev, 0x2b, 0x00);
+	else
+		sccb_reg_write(gspca_dev, 0x2b, 0x9e);
+}
+
+
 /* this function is called at probe time */
 static int sd_config(struct gspca_dev *gspca_dev,
 		     const struct usb_device_id *id)
@@ -817,6 +846,7 @@
 	sd->sharpness = SHARPNESS_DEF;
 	sd->hflip = HFLIP_DEF;
 	sd->vflip = VFLIP_DEF;
+	sd->freqfltr = FREQFLTR_DEF;
 
 	return 0;
 }
@@ -886,6 +916,7 @@
 	setsharpness(gspca_dev);
 	setvflip(gspca_dev);
 	sethflip(gspca_dev);
+	setfreqfltr(gspca_dev);
 
 	ov534_set_led(gspca_dev, 1);
 	ov534_reg_write(gspca_dev, 0xe0, 0x00);
@@ -1179,6 +1210,43 @@
 	return 0;
 }
 
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	sd->freqfltr = val;
+	if (gspca_dev->streaming)
+		setfreqfltr(gspca_dev);
+	return 0;
+}
+
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+
+	*val = sd->freqfltr;
+	return 0;
+}
+
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+		struct v4l2_querymenu *menu)
+{
+	switch (menu->id) {
+	case V4L2_CID_POWER_LINE_FREQUENCY:
+		switch (menu->index) {
+		case 0:         /* V4L2_CID_POWER_LINE_FREQUENCY_DISABLED */
+			strcpy((char *) menu->name, "Disabled");
+			return 0;
+		case 1:         /* V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
+			strcpy((char *) menu->name, "50 Hz");
+			return 0;
+		}
+		break;
+	}
+
+	return -EINVAL;
+}
+
 /* get stream parameters (framerate) */
 static int sd_get_streamparm(struct gspca_dev *gspca_dev,
 			     struct v4l2_streamparm *parm)
@@ -1230,6 +1298,7 @@
 	.start    = sd_start,
 	.stopN    = sd_stopN,
 	.pkt_scan = sd_pkt_scan,
+	.querymenu = sd_querymenu,
 	.get_streamparm = sd_get_streamparm,
 	.set_streamparm = sd_set_streamparm,
 };