Skip to contentSkip to conversation

gspca zc3xx: signal when unknown packet received

PATCH NAK Németh Márton 2 participants 3 messages Jan 29, 2010 Media (V4L/DVB)

lore
Share & tools
lore

Németh Márton

author
#0 lore raw

PATCH 1/1 gspca zc3xx: signal when unknown packet received

Signed-off-by Márton Németh <[email protected]>

---

linux/drivers/media/video/gspca/zc3xx.c +4 -1
diff -r 95d3956ea3e5 linux/drivers/media/video/gspca/zc3xx.c
@@ -7213,14 +7213,17 @@
 			u8 *data,		/* interrupt packet data */
 			int len)		/* interrput packet length */
 {
+	int ret = -EINVAL;
+
 	if (len == 8 && data[4] == 1) {
 		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1);
 		input_sync(gspca_dev->input_dev);
 		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0);
 		input_sync(gspca_dev->input_dev);
+		ret = 0;
 	}
 
-	return 0;
+	return ret;
 }
 #endif
NAK
#1 lore raw

Hi,

Nack!

Németh I know you mean well, but please don't go making semi random behavior changes to code you don't have hardware to test with.

There is a good reason this code is written the way it is.

Jean-Francois,

If you wonder what this is all about, this is a patch on top of one of my trees which no one else has yet as I have not send any pull request yet, see:
http://linuxtv.org/hg/~hgoede/gspca_jf

So back to the reason why this code is written the way it is, the zc3xx sends a steady stream of interrupt packets consisting of usually 8 0 byes, we definitely do not want to print out an error message every time such a packet is received.

On some cams when they are just plugged in the 6th byte (data[5]) becomes 1 a couple of times, probably a floating pin.

And on all cams with a button, pressing that will make the 5th byte (data[4]) 1. As said these cam sends a steady stream of interrupt packets, reporting I guess the status of 8 gpio pins independent on whether this status has changed since the last packet or not.

I've tested this with the following cams:

Logitech QuickCam IM/Connect    046d:08d9       zc3xx   HV7131R
Logitech QuickCam E2500         046d:089d       zc3xx   MC501CB
Labtec notebook cam             046d:08aa       zc3xx   PAS202B
Creative WebCam Notebook        041e:401f       zc3xx   TAS5130C
Creative Live! Cam Video IM     041e:4053       zc3xx   TAS5130-VF250
Philips SPC 200NC               0471:0325       zc3xx   PAS106
Creative WebCam NX Pro          041e:401e       zc3xx   HV7131B
No brand                        0ac8:307b       zc3xx   ADCM2700

Regards,

Hans

Quoted 25 lines · Németh Márton
Signed-off-by Márton Németh<[email protected]>

---
diff -r 95d3956ea3e5 linux/drivers/media/video/gspca/zc3xx.c

linux/drivers/media/video/gspca/zc3xx.c +4 -1
@@ -7213,14 +7213,17 @@
  			u8 *data,		/* interrupt packet data */
  			int len)		/* interrput packet length */
  {
+	int ret = -EINVAL;
+
  	if (len == 8&&  data[4] == 1) {
  		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1);
  		input_sync(gspca_dev->input_dev);
  		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0);
  		input_sync(gspca_dev->input_dev);
+		ret = 0;
  	}
 
-	return 0;
+	return ret;
  }
  #endif
author
#2 lore raw
Quoted 7 lines · Hans de Goede

Hi,

Nack!

Németh I know you mean well, but please don't go making semi random behavior changes to code you don't have hardware to test with.

I thought it is easier to write the patch I was thinking of than trying to describe it in words. Maybe it was not the best idea, sorry about that.

Quoted 14 lines

There is a good reason this code is written the way it is.

Jean-Francois,

If you wonder what this is all about, this is a patch on top of one of my trees which no one else has yet as I have not send any pull request yet, see:
http://linuxtv.org/hg/~hgoede/gspca_jf

So back to the reason why this code is written the way it is, the zc3xx sends a steady stream of interrupt packets consisting of usually 8 0 byes, we definitely do not want to print out an error message every time such a packet is received.

Quoted 8 lines

On some cams when they are just plugged in the 6th byte (data[5]) becomes 1 a couple of times, probably a floating pin.

And on all cams with a button, pressing that will make the 5th byte (data[4]) 1. As said these cam sends a steady stream of interrupt packets, reporting I guess the status of 8 gpio pins independent on whether this status has changed since the last packet or not.

Based on your description the following two lines could be separated:
input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1); input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0);

The first line should go when we detect a 0->1 transient (button push), the second one when there is an 1->0 transient (button release).

In case of pac7302 there was only one event at button push. So there was a need to simulate push and release. The camera haven't sent anything when the button was released.

Quoted 43 lines

I've tested this with the following cams:

Logitech QuickCam IM/Connect    046d:08d9       zc3xx   HV7131R
Logitech QuickCam E2500         046d:089d       zc3xx   MC501CB
Labtec notebook cam             046d:08aa       zc3xx   PAS202B
Creative WebCam Notebook        041e:401f       zc3xx   TAS5130C
Creative Live! Cam Video IM     041e:4053       zc3xx   TAS5130-VF250
Philips SPC 200NC               0471:0325       zc3xx   PAS106
Creative WebCam NX Pro          041e:401e       zc3xx   HV7131B
No brand                        0ac8:307b       zc3xx   ADCM2700

Regards,

Hans

Quoted 24 lines · Németh Márton
Signed-off-by Márton Németh<[email protected]>

---
diff -r 95d3956ea3e5 linux/drivers/media/video/gspca/zc3xx.c

linux/drivers/media/video/gspca/zc3xx.c +4 -1
@@ -7213,14 +7213,17 @@
  			u8 *data,		/* interrupt packet data */
  			int len)		/* interrput packet length */
  {
+	int ret = -EINVAL;
+
  	if (len == 8&&  data[4] == 1) {
  		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1);
  		input_sync(gspca_dev->input_dev);
  		input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0);
  		input_sync(gspca_dev->input_dev);
+		ret = 0;
  	}
 
-	return 0;
+	return ret;
  }
  #endif