[PATCH v2 00/11] em28xx: i2c debugging cleanups and support for newer eeproms

[PATCH v2 00/11] em28xx: i2c debugging cleanups and support for newer eeproms

[Frank Schäfer]

The first 3 patches clean up / simplify the debugging and info messages a bit.

Patches 4, 5 and 6 fix two bugs I've noticed while working on the eeprom stuff.

Patches 7-10 add support for the newer eeproms with 16 bit address width.
This allows us to display the eeprom content, to calculate the eeprom hash for 
board hints for devices with generic USB IDs, to read the device configuration 
and to use tveeprom for Hauppauge devices just as with the old eeprom type.

The used eeprom type depends on the chip type/id (confirmed by the Empia datasheets).
For unknown chips, the old 8 bit eeprom type is assumed, which makes sure that the
eeprom content can't be damaged accidentally.
In video capturing/TV/DVB devices, the new eeprom still has the old eeprom content 
(device configuration + tveeprom structure for Hauppauge devices) embedded.
Camera devices (em25xx, em276x+) however are using a different data structure,
which isn't supported yet.

Patch 11 is a follow-up which enables tveeprom for the Hauppauge HVR-930C

All patches have been tested with the following devices:
- Hauppauge HVR-900 (normal 8 bit eeprom)
- Hauppauge HVR-930C (16 bit eeprom, tveeprom)
- SpeedLink VAD Laplace webcam (16 bit eeprom, no device config dataset)

I also checked the USB log of the MSI Digivox ATSC which confirms that 
non-Hauppauge devices are using the same eeprom format.

Changes since v1:
- fixed the sanity check in patch 8
- fixed a minor output format issue in patch 10

Frank Schäfer (11):
  em28xx-i2c: replace printk() with the corresponding em28xx macros
  em28xx-i2c: get rid of the dprintk2 macro
  em28xx-i2c: also print debug messages at debug level 1
  em28xx: do not interpret eeprom content if eeprom key is invalid
  em28xx: fix eeprom data endianess
  em28xx: make sure we are at i2c bus A when calling
    em28xx_i2c_register()
  em28xx: add basic support for eeproms with 16 bit address width
  em28xx: add helper function for reading data blocks from i2c clients
  em28xx: do not store eeprom content permanently
  em28xx: extract the device configuration dataset from eeproms with 16
    bit address width
  em28xx: enable tveeprom for device Hauppauge HVR-930C

 drivers/media/usb/em28xx/em28xx-cards.c |   45 +++--
 drivers/media/usb/em28xx/em28xx-i2c.c   |  284 ++++++++++++++++++++-----------
 drivers/media/usb/em28xx/em28xx.h       |   17 +-
 3 Dateien geändert, 225 Zeilen hinzugefügt(+), 121 Zeilen entfernt(-)

-- 
1.7.10.4

[PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros

[Frank Schäfer]

Reduces the number of characters/lines, unifies the code and improves readability.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |   55 ++++++++++++++-------------------
 1 Datei geändert, 24 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 8532c1d..8819b54 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -399,7 +399,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 	/* Check if board has eeprom */
 	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
 	if (err < 0) {
-		em28xx_errdev("board has no eeprom\n");
+		em28xx_info("board has no eeprom\n");
 		memset(eedata, 0, len);
 		return -ENODEV;
 	}
@@ -408,8 +408,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 
 	err = i2c_master_send(&dev->i2c_client, &buf, 1);
 	if (err != 1) {
-		printk(KERN_INFO "%s: Huh, no eeprom present (err=%d)?\n",
-		       dev->name, err);
+		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
 		return err;
 	}
 
@@ -426,9 +425,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 
 		if (block !=
 		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
-			printk(KERN_WARNING
-			       "%s: i2c eeprom read error (err=%d)\n",
-			       dev->name, err);
+			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
 			return err;
 		}
 		size -= block;
@@ -436,7 +433,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 	}
 	for (i = 0; i < len; i++) {
 		if (0 == (i % 16))
-			printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
+			em28xx_info("i2c eeprom %02x:", i);
 		printk(" %02x", eedata[i]);
 		if (15 == (i % 16))
 			printk("\n");
@@ -445,55 +442,51 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 	if (em_eeprom->id == 0x9567eb1a)
 		dev->hash = em28xx_hash_mem(eedata, len, 32);
 
-	printk(KERN_INFO "%s: EEPROM ID= 0x%08x, EEPROM hash = 0x%08lx\n",
-	       dev->name, em_eeprom->id, dev->hash);
+	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
+		    em_eeprom->id, dev->hash);
 
-	printk(KERN_INFO "%s: EEPROM info:\n", dev->name);
+	em28xx_info("EEPROM info:\n");
 
 	switch (em_eeprom->chip_conf >> 4 & 0x3) {
 	case 0:
-		printk(KERN_INFO "%s:\tNo audio on board.\n", dev->name);
+		em28xx_info("\tNo audio on board.\n");
 		break;
 	case 1:
-		printk(KERN_INFO "%s:\tAC97 audio (5 sample rates)\n",
-				 dev->name);
+		em28xx_info("\tAC97 audio (5 sample rates)\n");
 		break;
 	case 2:
-		printk(KERN_INFO "%s:\tI2S audio, sample rate=32k\n",
-				 dev->name);
+		em28xx_info("\tI2S audio, sample rate=32k\n");
 		break;
 	case 3:
-		printk(KERN_INFO "%s:\tI2S audio, 3 sample rates\n",
-				 dev->name);
+		em28xx_info("\tI2S audio, 3 sample rates\n");
 		break;
 	}
 
 	if (em_eeprom->chip_conf & 1 << 3)
-		printk(KERN_INFO "%s:\tUSB Remote wakeup capable\n", dev->name);
+		em28xx_info("\tUSB Remote wakeup capable\n");
 
 	if (em_eeprom->chip_conf & 1 << 2)
-		printk(KERN_INFO "%s:\tUSB Self power capable\n", dev->name);
+		em28xx_info("\tUSB Self power capable\n");
 
 	switch (em_eeprom->chip_conf & 0x3) {
 	case 0:
-		printk(KERN_INFO "%s:\t500mA max power\n", dev->name);
+		em28xx_info("\t500mA max power\n");
 		break;
 	case 1:
-		printk(KERN_INFO "%s:\t400mA max power\n", dev->name);
+		em28xx_info("\t400mA max power\n");
 		break;
 	case 2:
-		printk(KERN_INFO "%s:\t300mA max power\n", dev->name);
+		em28xx_info("\t300mA max power\n");
 		break;
 	case 3:
-		printk(KERN_INFO "%s:\t200mA max power\n", dev->name);
+		em28xx_info("\t200mA max power\n");
 		break;
 	}
-	printk(KERN_INFO "%s:\tTable at 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
-				dev->name,
-				em_eeprom->string_idx_table,
-				em_eeprom->string1,
-				em_eeprom->string2,
-				em_eeprom->string3);
+	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
+		    em_eeprom->string_idx_table,
+		    em_eeprom->string1,
+		    em_eeprom->string2,
+		    em_eeprom->string3);
 
 	return 0;
 }
@@ -570,8 +563,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
 		if (rc < 0)
 			continue;
 		i2c_devicelist[i] = i;
-		printk(KERN_INFO "%s: found i2c device @ 0x%x [%s]\n",
-		       dev->name, i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
+		em28xx_info("found i2c device @ 0x%x [%s]\n",
+			    i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
 	}
 
 	dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
-- 
1.7.10.4

[PATCH v2 02/11] em28xx-i2c: get rid of the dprintk2 macro

[Frank Schäfer]

There is only a single place where the dprintk2 macro is used, so get rid of it.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |   17 ++++++-----------
 1 Datei geändert, 6 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 8819b54..f970c29 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -41,14 +41,6 @@ static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
 MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
 
-#define dprintk2(lvl, fmt, args...)			\
-do {							\
-	if (i2c_debug >= lvl) {				\
-		printk(KERN_DEBUG "%s at %s: " fmt,	\
-		       dev->name, __func__ , ##args);	\
-      } 						\
-} while (0)
-
 /*
  * em2800_i2c_send_bytes()
  * send up to 4 bytes to the em2800 i2c device
@@ -295,9 +287,12 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 		return 0;
 	for (i = 0; i < num; i++) {
 		addr = msgs[i].addr << 1;
-		dprintk2(2, "%s %s addr=%x len=%d:",
-			 (msgs[i].flags & I2C_M_RD) ? "read" : "write",
-			 i == num - 1 ? "stop" : "nonstop", addr, msgs[i].len);
+		if (i2c_debug >= 2)
+			printk(KERN_DEBUG "%s at %s: %s %s addr=%02x len=%d:",
+			       dev->name, __func__ ,
+			       (msgs[i].flags & I2C_M_RD) ? "read" : "write",
+			       i == num - 1 ? "stop" : "nonstop",
+			       addr, msgs[i].len			     );
 		if (!msgs[i].len) { /* no len: check only for device presence */
 			if (dev->board.is_em2800)
 				rc = em2800_i2c_check_for_device(dev, addr);
-- 
1.7.10.4

[PATCH v2 03/11] em28xx-i2c: also print debug messages at debug level 1

[Frank Schäfer]

The current code uses only a single debug level and all debug messages are
printed for i2c_debug >= 2 only. So debug level 1 is actually the same as
level 0, which is odd.
Users expect debugging messages to become enabled for anything else than
debug level 0.

Fix it and simplify the code a bit by printing the debug messages also at debug
level 1;

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |   12 ++++++------
 1 Datei geändert, 6 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index f970c29..d765567 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -287,7 +287,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 		return 0;
 	for (i = 0; i < num; i++) {
 		addr = msgs[i].addr << 1;
-		if (i2c_debug >= 2)
+		if (i2c_debug)
 			printk(KERN_DEBUG "%s at %s: %s %s addr=%02x len=%d:",
 			       dev->name, __func__ ,
 			       (msgs[i].flags & I2C_M_RD) ? "read" : "write",
@@ -299,7 +299,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 			else
 				rc = em28xx_i2c_check_for_device(dev, addr);
 			if (rc == -ENODEV) {
-				if (i2c_debug >= 2)
+				if (i2c_debug)
 					printk(" no device\n");
 				return rc;
 			}
@@ -313,13 +313,13 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 				rc = em28xx_i2c_recv_bytes(dev, addr,
 							   msgs[i].buf,
 							   msgs[i].len);
-			if (i2c_debug >= 2) {
+			if (i2c_debug) {
 				for (byte = 0; byte < msgs[i].len; byte++)
 					printk(" %02x", msgs[i].buf[byte]);
 			}
 		} else {
 			/* write bytes */
-			if (i2c_debug >= 2) {
+			if (i2c_debug) {
 				for (byte = 0; byte < msgs[i].len; byte++)
 					printk(" %02x", msgs[i].buf[byte]);
 			}
@@ -334,11 +334,11 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 							   i == num - 1);
 		}
 		if (rc < 0) {
-			if (i2c_debug >= 2)
+			if (i2c_debug)
 				printk(" ERROR: %i\n", rc);
 			return rc;
 		}
-		if (i2c_debug >= 2)
+		if (i2c_debug)
 			printk("\n");
 	}
 
-- 
1.7.10.4

[PATCH v2 04/11] em28xx: do not interpret eeprom content if eeprom key is invalid

[Frank Schäfer]

If the eeprom key isn't valid, either a different (currently unknown) format
is used or the eeprom is corrupted.
In both cases it doesn't make sense to interpret the data.
Also print an error message.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |    8 ++++++--
 1 Datei geändert, 6 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index d765567..9612086 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -434,8 +434,12 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 			printk("\n");
 	}
 
-	if (em_eeprom->id == 0x9567eb1a)
-		dev->hash = em28xx_hash_mem(eedata, len, 32);
+	if (em_eeprom->id != 0x9567eb1a) {
+		em28xx_errdev("Unknown eeprom type or eeprom corrupted !");
+		return -ENODEV;
+	}
+
+	dev->hash = em28xx_hash_mem(eedata, len, 32);
 
 	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
 		    em_eeprom->id, dev->hash);
-- 
1.7.10.4

[PATCH v2 05/11] em28xx: fix eeprom data endianess

[Frank Schäfer]

The data is stored as little endian in the eeprom.
Hence the correct data types should be used and the data should be converted
to the machine endianess before using it.
The eeprom id (key) also isn't a 32 bit value but 4 separate bytes instead.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |   22 ++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h     |   12 ++++++------
 2 Dateien geändert, 18 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9612086..19f3e4f 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -434,19 +434,21 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 			printk("\n");
 	}
 
-	if (em_eeprom->id != 0x9567eb1a) {
+	if (em_eeprom->id[0] != 0x1a || em_eeprom->id[1] != 0xeb ||
+	    em_eeprom->id[2] != 0x67 || em_eeprom->id[3] != 0x95   ) {
 		em28xx_errdev("Unknown eeprom type or eeprom corrupted !");
 		return -ENODEV;
 	}
 
 	dev->hash = em28xx_hash_mem(eedata, len, 32);
 
-	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
-		    em_eeprom->id, dev->hash);
+	em28xx_info("EEPROM ID = %02x %02x %02x %02x, EEPROM hash = 0x%08lx\n",
+		    em_eeprom->id[0], em_eeprom->id[1],
+		    em_eeprom->id[2], em_eeprom->id[3], dev->hash);
 
 	em28xx_info("EEPROM info:\n");
 
-	switch (em_eeprom->chip_conf >> 4 & 0x3) {
+	switch (le16_to_cpu(em_eeprom->chip_conf) >> 4 & 0x3) {
 	case 0:
 		em28xx_info("\tNo audio on board.\n");
 		break;
@@ -461,13 +463,13 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 		break;
 	}
 
-	if (em_eeprom->chip_conf & 1 << 3)
+	if (le16_to_cpu(em_eeprom->chip_conf) & 1 << 3)
 		em28xx_info("\tUSB Remote wakeup capable\n");
 
-	if (em_eeprom->chip_conf & 1 << 2)
+	if (le16_to_cpu(em_eeprom->chip_conf) & 1 << 2)
 		em28xx_info("\tUSB Self power capable\n");
 
-	switch (em_eeprom->chip_conf & 0x3) {
+	switch (le16_to_cpu(em_eeprom->chip_conf) & 0x3) {
 	case 0:
 		em28xx_info("\t500mA max power\n");
 		break;
@@ -483,9 +485,9 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 	}
 	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
 		    em_eeprom->string_idx_table,
-		    em_eeprom->string1,
-		    em_eeprom->string2,
-		    em_eeprom->string3);
+		    le16_to_cpu(em_eeprom->string1),
+		    le16_to_cpu(em_eeprom->string2),
+		    le16_to_cpu(em_eeprom->string3));
 
 	return 0;
 }
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 4160a2a..90266a1 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -405,15 +405,15 @@ struct em28xx_board {
 };
 
 struct em28xx_eeprom {
-	u32 id;			/* 0x9567eb1a */
-	u16 vendor_ID;
-	u16 product_ID;
+	u8 id[4];			/* 1a eb 67 95 */
+	__le16 vendor_ID;
+	__le16 product_ID;
 
-	u16 chip_conf;
+	__le16 chip_conf;
 
-	u16 board_conf;
+	__le16 board_conf;
 
-	u16 string1, string2, string3;
+	__le16 string1, string2, string3;
 
 	u8 string_idx_table;
 };
-- 
1.7.10.4

[PATCH v2 06/11] em28xx: make sure we are at i2c bus A when calling em28xx_i2c_register()

[Frank Schäfer]

The current code first configures register EM28XX_R06_I2C_CLK, which includes
i2c speed, ack, wait and (on some devices) i2c bus selection.
The register value usually comes from the board definition.
em28xx_i2c_register() is called afterwards, which also tries to read the eeprom.
If the device uses i2c bus B, eeprom reading fails.

Fix the problem by selecting bus A before calling em28xx_i2c_register() and
apply the board settings for register EM28XX_R06_I2C_CLK afterwards.
I also noticed that this is what the Windows driver does.
To be sure the i2c bus scan works as expected/before, remove its call from
em28xx_i2c_register() and call it directly after the i2c bus has been configured.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |   31 ++++++++++++++++++++-----------
 drivers/media/usb/em28xx/em28xx-i2c.c   |    7 -------
 2 Dateien geändert, 20 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 9332d05..0d74734 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -66,6 +66,10 @@ module_param(usb_xfer_mode, int, 0444);
 MODULE_PARM_DESC(usb_xfer_mode,
 		 "USB transfer mode for frame data (-1 = auto, 0 = prefer isoc, 1 = prefer bulk)");
 
+static unsigned int i2c_scan;
+module_param(i2c_scan, int, 0444);
+MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
+
 
 /* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
 static unsigned long em28xx_devused;
@@ -3074,8 +3078,20 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 		snprintf(dev->name, sizeof(dev->name), "%s #%d", chip_name, dev->devno);
 	}
 
+	/* Select i2c bus A (if necessary) */
+	if (dev->chip_id == CHIP_ID_EM2874 ||
+	    dev->chip_id == CHIP_ID_EM28174 ||
+	    dev->chip_id == CHIP_ID_EM2884)
+		em28xx_write_reg_bits(dev, EM28XX_R06_I2C_CLK, 0, EM2874_I2C_SECONDARY_BUS_SELECT);
+	/* Register i2c bus */
+	retval = em28xx_i2c_register(dev);
+	if (retval < 0) {
+		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
+			__func__, retval);
+		return retval;
+	}
+	/* Configure i2c bus */
 	if (!dev->board.is_em2800) {
-		/* Resets I2C speed */
 		retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
 		if (retval < 0) {
 			em28xx_errdev("%s: em28xx_write_reg failed!"
@@ -3084,6 +3100,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 			return retval;
 		}
 	}
+	/* Scan i2c bus */
+	if (i2c_scan)
+		em28xx_do_i2c_scan(dev);
 
 	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
 	if (retval < 0) {
@@ -3094,14 +3113,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 	v4l2_ctrl_handler_init(hdl, 8);
 	dev->v4l2_dev.ctrl_handler = hdl;
 
-	/* register i2c bus */
-	retval = em28xx_i2c_register(dev);
-	if (retval < 0) {
-		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
-			__func__, retval);
-		goto unregister_dev;
-	}
-
 	/*
 	 * Default format, used for tvp5150 or saa711x output formats
 	 */
@@ -3173,8 +3184,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 fail:
 	em28xx_i2c_unregister(dev);
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-
-unregister_dev:
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return retval;
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 19f3e4f..ebe4b20 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -33,10 +33,6 @@
 
 /* ----------------------------------------------------------- */
 
-static unsigned int i2c_scan;
-module_param(i2c_scan, int, 0444);
-MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
-
 static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
 MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
@@ -606,9 +602,6 @@ int em28xx_i2c_register(struct em28xx *dev)
 		return retval;
 	}
 
-	if (i2c_scan)
-		em28xx_do_i2c_scan(dev);
-
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH v2 07/11] em28xx: add basic support for eeproms with 16 bit address width

[Frank Schäfer]

Newer devices (em2874, em2884, em28174, em25xx, em27[6,7,8]x) use eeproms with
16 bit instead of 8 bit address width.
The used eeprom type depends on the chip type, which makes sure eeproms can't
be damaged.

This patch adds basic support for 16 bit eeproms only, which includes
- reading the content
- calculating the eeprom hash
- displaying the content

The eeprom content uses a different format, for which support will be added with
subsequent patches.

Tested with the "Hauppauge HVR-930C" and the "Speedlink VAD Laplace webcam"
(with additional experimental patches).

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |    4 ++
 drivers/media/usb/em28xx/em28xx-i2c.c   |   69 ++++++++++++++++++++-----------
 drivers/media/usb/em28xx/em28xx.h       |    1 +
 3 Dateien geändert, 49 Zeilen hinzugefügt(+), 25 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 0d74734..fa51f81 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2187,6 +2187,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
 	{0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
 	{0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
 };
+/* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
 
 /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
 static unsigned short saa711x_addrs[] = {
@@ -3023,11 +3024,13 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 			chip_name = "em2874";
 			dev->reg_gpio_num = EM2874_R80_GPIO;
 			dev->wait_after_write = 0;
+			dev->eeprom_addrwidth_16bit = 1;
 			break;
 		case CHIP_ID_EM28174:
 			chip_name = "em28174";
 			dev->reg_gpio_num = EM2874_R80_GPIO;
 			dev->wait_after_write = 0;
+			dev->eeprom_addrwidth_16bit = 1;
 			break;
 		case CHIP_ID_EM2883:
 			chip_name = "em2882/3";
@@ -3037,6 +3040,7 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 			chip_name = "em2884";
 			dev->reg_gpio_num = EM2874_R80_GPIO;
 			dev->wait_after_write = 0;
+			dev->eeprom_addrwidth_16bit = 1;
 			break;
 		default:
 			printk(KERN_INFO DRIVER_NAME
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index ebe4b20..7185812 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -368,46 +368,34 @@ static inline unsigned long em28xx_hash_mem(char *buf, int length, int bits)
 
 static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 {
-	unsigned char buf, *p = eedata;
+	unsigned char buf[2], *p = eedata;
 	struct em28xx_eeprom *em_eeprom = (void *)eedata;
 	int i, err, size = len, block, block_max;
 
-	if (dev->chip_id == CHIP_ID_EM2874 ||
-	    dev->chip_id == CHIP_ID_EM28174 ||
-	    dev->chip_id == CHIP_ID_EM2884) {
-		/* Empia switched to a 16-bit addressable eeprom in newer
-		   devices.  While we could certainly write a routine to read
-		   the eeprom, there is nothing of use in there that cannot be
-		   accessed through registers, and there is the risk that we
-		   could corrupt the eeprom (since a 16-bit read call is
-		   interpreted as a write call by 8-bit eeproms).
-		*/
-		return 0;
-	}
-
 	dev->i2c_client.addr = 0xa0 >> 1;
 
 	/* Check if board has eeprom */
-	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
+	err = i2c_master_recv(&dev->i2c_client, buf, 0);
 	if (err < 0) {
 		em28xx_info("board has no eeprom\n");
 		memset(eedata, 0, len);
 		return -ENODEV;
 	}
 
-	buf = 0;
-
-	err = i2c_master_send(&dev->i2c_client, &buf, 1);
-	if (err != 1) {
+	/* Select address memory address 0x00(00) */
+	buf[0] = 0;
+	buf[1] = 0;
+	err = i2c_master_send(&dev->i2c_client, buf, 1 + dev->eeprom_addrwidth_16bit);
+	if (err != 1 + dev->eeprom_addrwidth_16bit) {
 		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
 		return err;
 	}
 
+	/* Read eeprom content */
 	if (dev->board.is_em2800)
 		block_max = 4;
 	else
 		block_max = 64;
-
 	while (size > 0) {
 		if (size > block_max)
 			block = block_max;
@@ -422,17 +410,48 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 		size -= block;
 		p += block;
 	}
+
+	/* Display eeprom content */
 	for (i = 0; i < len; i++) {
-		if (0 == (i % 16))
-			em28xx_info("i2c eeprom %02x:", i);
+		if (0 == (i % 16)) {
+			if (dev->eeprom_addrwidth_16bit)
+				em28xx_info("i2c eeprom %04x:", i);
+			else
+				em28xx_info("i2c eeprom %02x:", i);
+		}
 		printk(" %02x", eedata[i]);
 		if (15 == (i % 16))
 			printk("\n");
 	}
 
-	if (em_eeprom->id[0] != 0x1a || em_eeprom->id[1] != 0xeb ||
-	    em_eeprom->id[2] != 0x67 || em_eeprom->id[3] != 0x95   ) {
-		em28xx_errdev("Unknown eeprom type or eeprom corrupted !");
+	if (dev->eeprom_addrwidth_16bit &&
+	    eedata[0] == 0x26 && eedata[3] == 0x00) {
+		/* new eeprom format; size 4-64kb */
+		dev->hash = em28xx_hash_mem(eedata, len, 32);
+		em28xx_info("EEPROM hash = 0x%08lx\n", dev->hash);
+		em28xx_info("EEPROM info: boot page address = 0x%02x04, "
+			    "boot configuration = 0x%02x\n",
+			    eedata[1], eedata[2]);
+		/* boot configuration (address 0x0002):
+		 * [0]   microcode download speed: 1 = 400 kHz; 0 = 100 kHz
+		 * [1]   always selects 12 kb RAM
+		 * [2]   USB device speed: 1 = force Full Speed; 0 = auto detect
+		 * [4]   1 = force fast mode and no suspend for device testing
+		 * [5:7] USB PHY tuning registers; determined by device
+		 *       characterization
+		 */
+
+		/* FIXME:
+		 * - read more than 256 bytes / addresses above 0x00ff
+		 * - find offset for device config dataset and extract it
+		 * - decrypt eeprom data for camera bridges (em25xx, em276x+)
+		 * - use separate/different eeprom hashes (not yet used)
+		 */
+
+		return 0;
+	} else if (em_eeprom->id[0] != 0x1a || em_eeprom->id[1] != 0xeb ||
+		   em_eeprom->id[2] != 0x67 || em_eeprom->id[3] != 0x95   ) {
+		em28xx_info("unknown eeprom format or eeprom corrupted !\n");
 		return -ENODEV;
 	}
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 90266a1..139dfe5 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -510,6 +510,7 @@ struct em28xx {
 	/* i2c i/o */
 	struct i2c_adapter i2c_adap;
 	struct i2c_client i2c_client;
+	unsigned char eeprom_addrwidth_16bit:1;
 	/* video for linux */
 	int users;		/* user count for exclusive use */
 	int streaming_users;    /* Number of actively streaming users */
-- 
1.7.10.4

[PATCH v2 08/11] em28xx: add helper function for reading data blocks from i2c clients

[Frank Schäfer]

Add a helper function for reading data blocks from i2c devices with 8 or 16 bit
address width and 8 bit register width.
This allows us to reduce the size of new code added by the following patches.
Works only for devices with activated register auto incrementation.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |   74 ++++++++++++++++++++-------------
 1 Datei geändert, 46 Zeilen hinzugefügt(+), 28 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 7185812..a3e9547 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -366,51 +366,69 @@ static inline unsigned long em28xx_hash_mem(char *buf, int length, int bits)
 	return (hash >> (32 - bits)) & 0xffffffffUL;
 }
 
+/* Helper function to read data blocks from i2c clients with 8 or 16 bit
+ * address width, 8 bit register width and auto incrementation been activated */
+static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
+				 u16 len, u8 *data)
+{
+	int remain = len, rsize, rsize_max, ret;
+	u8 buf[2];
+
+	/* Sanity check */
+	if (addr + remain > (addr_w16 * 0xff00 + 0xff + 1))
+		return -EINVAL;
+	/* Select address */
+	buf[0] = addr >> 8;
+	buf[1] = addr & 0xff;
+	ret = i2c_master_send(&dev->i2c_client, buf + !addr_w16, 1 + addr_w16);
+	if (ret < 0)
+		return ret;
+	/* Read data */
+	if (dev->board.is_em2800)
+		rsize_max = 4;
+	else
+		rsize_max = 64;
+	while (remain > 0) {
+		if (remain > rsize_max)
+			rsize = rsize_max;
+		else
+			rsize = remain;
+
+		ret = i2c_master_recv(&dev->i2c_client, data, rsize);
+		if (ret < 0)
+			return ret;
+
+		remain -= rsize;
+		data += rsize;
+	}
+
+	return len;
+}
+
 static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 {
-	unsigned char buf[2], *p = eedata;
+	unsigned char buf, *p = eedata;
 	struct em28xx_eeprom *em_eeprom = (void *)eedata;
-	int i, err, size = len, block, block_max;
+	int i, err;
 
 	dev->i2c_client.addr = 0xa0 >> 1;
 
 	/* Check if board has eeprom */
-	err = i2c_master_recv(&dev->i2c_client, buf, 0);
+	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
 	if (err < 0) {
 		em28xx_info("board has no eeprom\n");
 		memset(eedata, 0, len);
 		return -ENODEV;
 	}
 
-	/* Select address memory address 0x00(00) */
-	buf[0] = 0;
-	buf[1] = 0;
-	err = i2c_master_send(&dev->i2c_client, buf, 1 + dev->eeprom_addrwidth_16bit);
-	if (err != 1 + dev->eeprom_addrwidth_16bit) {
+	/* Read EEPROM content */
+	err = em28xx_i2c_read_block(dev, 0x0000, dev->eeprom_addrwidth_16bit,
+				    len, p);
+	if (err != len) {
 		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
 		return err;
 	}
 
-	/* Read eeprom content */
-	if (dev->board.is_em2800)
-		block_max = 4;
-	else
-		block_max = 64;
-	while (size > 0) {
-		if (size > block_max)
-			block = block_max;
-		else
-			block = size;
-
-		if (block !=
-		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
-			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
-			return err;
-		}
-		size -= block;
-		p += block;
-	}
-
 	/* Display eeprom content */
 	for (i = 0; i < len; i++) {
 		if (0 == (i % 16)) {
-- 
1.7.10.4

[PATCH v2 09/11] em28xx: do not store eeprom content permanently

[Frank Schäfer]

We currently reserve an array of 256 bytes for the eeprom content in the device
struct. For eeproms with 16 bit address width it might even be necessary to
increase the buffer size further.

Having such a big chunk of memory reserved even if the device has no eeprom and
keeping it after it has already been processed seems to be a waste of memory.

Change the code to allocate + free the eeprom memory dynamically.
This also makes it possible to handle different dataset sizes depending on what
is stored/found in the eeprom.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |    9 +++++++-
 drivers/media/usb/em28xx/em28xx-i2c.c   |   35 +++++++++++++++++++------------
 drivers/media/usb/em28xx/em28xx.h       |    2 +-
 3 Dateien geändert, 31 Zeilen hinzugefügt(+), 15 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index fa51f81..2e3d3ad 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2743,6 +2743,9 @@ static void em28xx_card_setup(struct em28xx *dev)
 	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_950:
 	{
 		struct tveeprom tv;
+
+		if (dev->eedata == NULL)
+			break;
 #if defined(CONFIG_MODULES) && defined(MODULE)
 		request_module("tveeprom");
 #endif
@@ -2796,7 +2799,7 @@ static void em28xx_card_setup(struct em28xx *dev)
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
 		break;
 
-/*
+		/*
 		 * The Dikom DK300 is detected as an Kworld VS-DVB-T 323UR.
 		 *
 		 * This occurs because they share identical USB vendor and
@@ -2831,6 +2834,10 @@ static void em28xx_card_setup(struct em28xx *dev)
 				"addresses)\n\n");
 	}
 
+	/* Free eeprom data memory */
+	kfree(dev->eedata);
+	dev->eedata = NULL;
+
 	/* Allow override tuner type by a module parameter */
 	if (tuner >= 0)
 		dev->tuner_type = tuner;
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index a3e9547..dfbc22e 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -405,27 +405,33 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
 	return len;
 }
 
-static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
+static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len)
 {
-	unsigned char buf, *p = eedata;
-	struct em28xx_eeprom *em_eeprom = (void *)eedata;
+	u8 buf, *data;
+	struct em28xx_eeprom *em_eeprom;
 	int i, err;
 
+	*eedata = NULL;
+
 	dev->i2c_client.addr = 0xa0 >> 1;
 
 	/* Check if board has eeprom */
 	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
 	if (err < 0) {
 		em28xx_info("board has no eeprom\n");
-		memset(eedata, 0, len);
 		return -ENODEV;
 	}
 
+	data = kzalloc(len, GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
 	/* Read EEPROM content */
 	err = em28xx_i2c_read_block(dev, 0x0000, dev->eeprom_addrwidth_16bit,
-				    len, p);
+				    len, data);
 	if (err != len) {
 		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
+		kfree(data);
 		return err;
 	}
 
@@ -437,19 +443,19 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 			else
 				em28xx_info("i2c eeprom %02x:", i);
 		}
-		printk(" %02x", eedata[i]);
+		printk(" %02x", data[i]);
 		if (15 == (i % 16))
 			printk("\n");
 	}
 
 	if (dev->eeprom_addrwidth_16bit &&
-	    eedata[0] == 0x26 && eedata[3] == 0x00) {
+	    data[0] == 0x26 && data[3] == 0x00) {
 		/* new eeprom format; size 4-64kb */
-		dev->hash = em28xx_hash_mem(eedata, len, 32);
+		dev->hash = em28xx_hash_mem(data, len, 32);
 		em28xx_info("EEPROM hash = 0x%08lx\n", dev->hash);
 		em28xx_info("EEPROM info: boot page address = 0x%02x04, "
 			    "boot configuration = 0x%02x\n",
-			    eedata[1], eedata[2]);
+			    data[1], data[2]);
 		/* boot configuration (address 0x0002):
 		 * [0]   microcode download speed: 1 = 400 kHz; 0 = 100 kHz
 		 * [1]   always selects 12 kb RAM
@@ -467,13 +473,16 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
 		 */
 
 		return 0;
-	} else if (em_eeprom->id[0] != 0x1a || em_eeprom->id[1] != 0xeb ||
-		   em_eeprom->id[2] != 0x67 || em_eeprom->id[3] != 0x95   ) {
+	} else if (data[0] != 0x1a || data[1] != 0xeb ||
+		   data[2] != 0x67 || data[3] != 0x95   ) {
 		em28xx_info("unknown eeprom format or eeprom corrupted !\n");
 		return -ENODEV;
 	}
 
-	dev->hash = em28xx_hash_mem(eedata, len, 32);
+	*eedata = data;
+	em_eeprom = (void *)eedata;
+
+	dev->hash = em28xx_hash_mem(data, len, 32);
 
 	em28xx_info("EEPROM ID = %02x %02x %02x %02x, EEPROM hash = 0x%08lx\n",
 		    em_eeprom->id[0], em_eeprom->id[1],
@@ -631,7 +640,7 @@ int em28xx_i2c_register(struct em28xx *dev)
 	dev->i2c_client = em28xx_client_template;
 	dev->i2c_client.adapter = &dev->i2c_adap;
 
-	retval = em28xx_i2c_eeprom(dev, dev->eedata, sizeof(dev->eedata));
+	retval = em28xx_i2c_eeprom(dev, &dev->eedata, 256);
 	if ((retval < 0) && (retval != -ENODEV)) {
 		em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
 			__func__, retval);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 139dfe5..77f600d 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -562,7 +562,7 @@ struct em28xx {
 	/* resources in use */
 	unsigned int resources;
 
-	unsigned char eedata[256];
+	u8 *eedata;	/* currently always 256 bytes */
 
 	/* Isoc control struct */
 	struct em28xx_dmaqueue vidq;
-- 
1.7.10.4

[PATCH v2 10/11] em28xx: extract the device configuration dataset from eeproms with 16 bit address width

[Frank Schäfer]

The new eeproms with 16 address width still have the the device config dataset
(the content of the old 8 bit eeproms) embedded.
Hauppauge also continues to include the tveeprom data structure inside this
dataset in their devices.
The start address of the dataset depends on the start address of the microcode
and a variable additional offset.

It should be mentioned that Camera devices seem to use a different dataset type,
which is not yet supported.

Tested with devices "Hauppauge HVR-930C". I've also checked the USB-log from the
"MSI Digivox ATSC" and it works the same way.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c |  117 +++++++++++++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h     |    4 +-
 2 Dateien geändert, 85 Zeilen hinzugefügt(+), 36 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index dfbc22e..44bef43 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -405,13 +405,18 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
 	return len;
 }
 
-static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len)
+static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
 {
-	u8 buf, *data;
-	struct em28xx_eeprom *em_eeprom;
+	const u16 len = 256;
+	/* FIXME common length/size for bytes to read, to display, hash
+	 * calculation and returned device dataset. Simplifies the code a lot,
+	 * but we might have to deal with multiple sizes in the future !      */
 	int i, err;
+	struct em28xx_eeprom *dev_config;
+	u8 buf, *data;
 
 	*eedata = NULL;
+	*eedata_len = 0;
 
 	dev->i2c_client.addr = 0xa0 >> 1;
 
@@ -431,8 +436,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len
 				    len, data);
 	if (err != len) {
 		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
-		kfree(data);
-		return err;
+		goto error;
 	}
 
 	/* Display eeprom content */
@@ -447,15 +451,25 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len
 		if (15 == (i % 16))
 			printk("\n");
 	}
+	if (dev->eeprom_addrwidth_16bit)
+		em28xx_info("i2c eeprom %04x: ... (skipped)\n", i);
 
 	if (dev->eeprom_addrwidth_16bit &&
 	    data[0] == 0x26 && data[3] == 0x00) {
 		/* new eeprom format; size 4-64kb */
+		u16 mc_start;
+		u16 hwconf_offset;
+
 		dev->hash = em28xx_hash_mem(data, len, 32);
-		em28xx_info("EEPROM hash = 0x%08lx\n", dev->hash);
-		em28xx_info("EEPROM info: boot page address = 0x%02x04, "
+		mc_start = (data[1] << 8) + 4;	/* usually 0x0004 */
+
+		em28xx_info("EEPROM ID = %02x %02x %02x %02x, "
+			    "EEPROM hash = 0x%08lx\n",
+			    data[0], data[1], data[2], data[3], dev->hash);
+		em28xx_info("EEPROM info:\n");
+		em28xx_info("\tmicrocode start address = 0x%04x, "
 			    "boot configuration = 0x%02x\n",
-			    data[1], data[2]);
+			    mc_start, data[2]);
 		/* boot configuration (address 0x0002):
 		 * [0]   microcode download speed: 1 = 400 kHz; 0 = 100 kHz
 		 * [1]   always selects 12 kb RAM
@@ -465,32 +479,61 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len
 		 *       characterization
 		 */
 
-		/* FIXME:
-		 * - read more than 256 bytes / addresses above 0x00ff
-		 * - find offset for device config dataset and extract it
-		 * - decrypt eeprom data for camera bridges (em25xx, em276x+)
-		 * - use separate/different eeprom hashes (not yet used)
+		/* Read hardware config dataset offset from address
+		 * (microcode start + 46)			    */
+		err = em28xx_i2c_read_block(dev, mc_start + 46, 1, 2, data);
+		if (err != 2) {
+			em28xx_errdev("failed to read hardware configuration data from eeprom (err=%d)\n",
+				      err);
+			goto error;
+		}
+
+		/* Calculate hardware config dataset start address */
+		hwconf_offset = mc_start + data[0] + (data[1] << 8);
+
+		/* Read hardware config dataset */
+		/* NOTE: the microcode copy can be multiple pages long, but
+		 * we assume the hardware config dataset is the same as in
+		 * the old eeprom and not longer than 256 bytes.
+		 * tveeprom is currently also limited to 256 bytes.
 		 */
+		err = em28xx_i2c_read_block(dev, hwconf_offset, 1, len, data);
+		if (err != len) {
+			em28xx_errdev("failed to read hardware configuration data from eeprom (err=%d)\n",
+				      err);
+			goto error;
+		}
 
-		return 0;
-	} else if (data[0] != 0x1a || data[1] != 0xeb ||
-		   data[2] != 0x67 || data[3] != 0x95   ) {
+		/* Verify hardware config dataset */
+		/* NOTE: not all devices provide this type of dataset */
+		if (data[0] != 0x1a || data[1] != 0xeb ||
+		    data[2] != 0x67 || data[3] != 0x95    ) {
+			em28xx_info("\tno hardware configuration dataset found in eeprom\n");
+			kfree(data);
+			return 0;
+		}
+
+		/* TODO: decrypt eeprom data for camera bridges (em25xx, em276x+) */
+
+	} else if (!dev->eeprom_addrwidth_16bit &&
+		   data[0] == 0x1a && data[1] == 0xeb &&
+		   data[2] == 0x67 && data[3] == 0x95   ) {
+		dev->hash = em28xx_hash_mem(data, len, 32);
+		em28xx_info("EEPROM ID = %02x %02x %02x %02x, "
+			    "EEPROM hash = 0x%08lx\n",
+			    data[0], data[1], data[2], data[3], dev->hash);
+		em28xx_info("EEPROM info:\n");
+	} else {
 		em28xx_info("unknown eeprom format or eeprom corrupted !\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto error;
 	}
 
 	*eedata = data;
-	em_eeprom = (void *)eedata;
+	*eedata_len = len;
+	dev_config = (void *)eedata;
 
-	dev->hash = em28xx_hash_mem(data, len, 32);
-
-	em28xx_info("EEPROM ID = %02x %02x %02x %02x, EEPROM hash = 0x%08lx\n",
-		    em_eeprom->id[0], em_eeprom->id[1],
-		    em_eeprom->id[2], em_eeprom->id[3], dev->hash);
-
-	em28xx_info("EEPROM info:\n");
-
-	switch (le16_to_cpu(em_eeprom->chip_conf) >> 4 & 0x3) {
+	switch (le16_to_cpu(dev_config->chip_conf) >> 4 & 0x3) {
 	case 0:
 		em28xx_info("\tNo audio on board.\n");
 		break;
@@ -505,13 +548,13 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len
 		break;
 	}
 
-	if (le16_to_cpu(em_eeprom->chip_conf) & 1 << 3)
+	if (le16_to_cpu(dev_config->chip_conf) & 1 << 3)
 		em28xx_info("\tUSB Remote wakeup capable\n");
 
-	if (le16_to_cpu(em_eeprom->chip_conf) & 1 << 2)
+	if (le16_to_cpu(dev_config->chip_conf) & 1 << 2)
 		em28xx_info("\tUSB Self power capable\n");
 
-	switch (le16_to_cpu(em_eeprom->chip_conf) & 0x3) {
+	switch (le16_to_cpu(dev_config->chip_conf) & 0x3) {
 	case 0:
 		em28xx_info("\t500mA max power\n");
 		break;
@@ -526,12 +569,16 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char **eedata, int len
 		break;
 	}
 	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
-		    em_eeprom->string_idx_table,
-		    le16_to_cpu(em_eeprom->string1),
-		    le16_to_cpu(em_eeprom->string2),
-		    le16_to_cpu(em_eeprom->string3));
+		    dev_config->string_idx_table,
+		    le16_to_cpu(dev_config->string1),
+		    le16_to_cpu(dev_config->string2),
+		    le16_to_cpu(dev_config->string3));
 
 	return 0;
+
+error:
+	kfree(data);
+	return err;
 }
 
 /* ----------------------------------------------------------- */
@@ -640,7 +687,7 @@ int em28xx_i2c_register(struct em28xx *dev)
 	dev->i2c_client = em28xx_client_template;
 	dev->i2c_client.adapter = &dev->i2c_adap;
 
-	retval = em28xx_i2c_eeprom(dev, &dev->eedata, 256);
+	retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
 	if ((retval < 0) && (retval != -ENODEV)) {
 		em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
 			__func__, retval);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 77f600d..2d6d31a 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -562,7 +562,9 @@ struct em28xx {
 	/* resources in use */
 	unsigned int resources;
 
-	u8 *eedata;	/* currently always 256 bytes */
+	/* eeprom content */
+	u8 *eedata;
+	u16 eedata_len;
 
 	/* Isoc control struct */
 	struct em28xx_dmaqueue vidq;
-- 
1.7.10.4

[PATCH v2 11/11] em28xx: enable tveeprom for device Hauppauge HVR-930C

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |    1 +
 1 Datei geändert, 1 Zeile hinzugefügt(+)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 2e3d3ad..5a5b637 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2741,6 +2741,7 @@ static void em28xx_card_setup(struct em28xx *dev)
 	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900_R2:
 	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_850:
 	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_950:
+	case EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C:
 	{
 		struct tveeprom tv;
 
-- 
1.7.10.4

Re: [PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros

[Mauro Carvalho Chehab]

Em Sun,  3 Mar 2013 20:37:34 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Reduces the number of characters/lines, unifies the code and improves readability.

Had you actually test this patch? The reason why printk() is used on some
places is because dev->name is not available early.

That's said, it makes sense to replace all those em28xx-specific printk
functions by the standard pr_fmt-based ones (pr_err, pr_info, pr_debug, etc).

Regards,
Mauro

> 
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-i2c.c |   55 ++++++++++++++-------------------
>  1 Datei geändert, 24 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> index 8532c1d..8819b54 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -399,7 +399,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>  	/* Check if board has eeprom */
>  	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
>  	if (err < 0) {
> -		em28xx_errdev("board has no eeprom\n");
> +		em28xx_info("board has no eeprom\n");
>  		memset(eedata, 0, len);
>  		return -ENODEV;
>  	}
> @@ -408,8 +408,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>  
>  	err = i2c_master_send(&dev->i2c_client, &buf, 1);
>  	if (err != 1) {
> -		printk(KERN_INFO "%s: Huh, no eeprom present (err=%d)?\n",
> -		       dev->name, err);
> +		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
>  		return err;
>  	}
>  
> @@ -426,9 +425,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>  
>  		if (block !=
>  		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
> -			printk(KERN_WARNING
> -			       "%s: i2c eeprom read error (err=%d)\n",
> -			       dev->name, err);
> +			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
>  			return err;
>  		}
>  		size -= block;
> @@ -436,7 +433,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>  	}
>  	for (i = 0; i < len; i++) {
>  		if (0 == (i % 16))
> -			printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
> +			em28xx_info("i2c eeprom %02x:", i);
>  		printk(" %02x", eedata[i]);
>  		if (15 == (i % 16))
>  			printk("\n");
> @@ -445,55 +442,51 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>  	if (em_eeprom->id == 0x9567eb1a)
>  		dev->hash = em28xx_hash_mem(eedata, len, 32);
>  
> -	printk(KERN_INFO "%s: EEPROM ID= 0x%08x, EEPROM hash = 0x%08lx\n",
> -	       dev->name, em_eeprom->id, dev->hash);
> +	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
> +		    em_eeprom->id, dev->hash);
>  
> -	printk(KERN_INFO "%s: EEPROM info:\n", dev->name);
> +	em28xx_info("EEPROM info:\n");
>  
>  	switch (em_eeprom->chip_conf >> 4 & 0x3) {
>  	case 0:
> -		printk(KERN_INFO "%s:\tNo audio on board.\n", dev->name);
> +		em28xx_info("\tNo audio on board.\n");
>  		break;
>  	case 1:
> -		printk(KERN_INFO "%s:\tAC97 audio (5 sample rates)\n",
> -				 dev->name);
> +		em28xx_info("\tAC97 audio (5 sample rates)\n");
>  		break;
>  	case 2:
> -		printk(KERN_INFO "%s:\tI2S audio, sample rate=32k\n",
> -				 dev->name);
> +		em28xx_info("\tI2S audio, sample rate=32k\n");
>  		break;
>  	case 3:
> -		printk(KERN_INFO "%s:\tI2S audio, 3 sample rates\n",
> -				 dev->name);
> +		em28xx_info("\tI2S audio, 3 sample rates\n");
>  		break;
>  	}
>  
>  	if (em_eeprom->chip_conf & 1 << 3)
> -		printk(KERN_INFO "%s:\tUSB Remote wakeup capable\n", dev->name);
> +		em28xx_info("\tUSB Remote wakeup capable\n");
>  
>  	if (em_eeprom->chip_conf & 1 << 2)
> -		printk(KERN_INFO "%s:\tUSB Self power capable\n", dev->name);
> +		em28xx_info("\tUSB Self power capable\n");
>  
>  	switch (em_eeprom->chip_conf & 0x3) {
>  	case 0:
> -		printk(KERN_INFO "%s:\t500mA max power\n", dev->name);
> +		em28xx_info("\t500mA max power\n");
>  		break;
>  	case 1:
> -		printk(KERN_INFO "%s:\t400mA max power\n", dev->name);
> +		em28xx_info("\t400mA max power\n");
>  		break;
>  	case 2:
> -		printk(KERN_INFO "%s:\t300mA max power\n", dev->name);
> +		em28xx_info("\t300mA max power\n");
>  		break;
>  	case 3:
> -		printk(KERN_INFO "%s:\t200mA max power\n", dev->name);
> +		em28xx_info("\t200mA max power\n");
>  		break;
>  	}
> -	printk(KERN_INFO "%s:\tTable at 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
> -				dev->name,
> -				em_eeprom->string_idx_table,
> -				em_eeprom->string1,
> -				em_eeprom->string2,
> -				em_eeprom->string3);
> +	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
> +		    em_eeprom->string_idx_table,
> +		    em_eeprom->string1,
> +		    em_eeprom->string2,
> +		    em_eeprom->string3);
>  
>  	return 0;
>  }
> @@ -570,8 +563,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
>  		if (rc < 0)
>  			continue;
>  		i2c_devicelist[i] = i;
> -		printk(KERN_INFO "%s: found i2c device @ 0x%x [%s]\n",
> -		       dev->name, i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
> +		em28xx_info("found i2c device @ 0x%x [%s]\n",
> +			    i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
>  	}
>  
>  	dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,


-- 

Cheers,
Mauro

Re: [PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros

[Frank Schäfer]

Am 04.03.2013 19:09, schrieb Mauro Carvalho Chehab:
> Em Sun,  3 Mar 2013 20:37:34 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Reduces the number of characters/lines, unifies the code and improves readability.
> Had you actually test this patch? The reason why printk() is used on some
> places is because dev->name is not available early.

The em28xx-specific macros are using printk, too.
They are actually just an abbreviation of the current printks (saving us
the KERN_XY and the the dev->name parameter).
See em28xx.h.

> That's said, it makes sense to replace all those em28xx-specific printk
> functions by the standard pr_fmt-based ones (pr_err, pr_info, pr_debug, etc).

Yeah, I agree.
But that would be a separate patch series... ;)
I can do that (later) if you want.

Regards,
Frank

>
> Regards,
> Mauro
>
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-i2c.c |   55 ++++++++++++++-------------------
>>  1 Datei geändert, 24 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index 8532c1d..8819b54 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -399,7 +399,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	/* Check if board has eeprom */
>>  	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
>>  	if (err < 0) {
>> -		em28xx_errdev("board has no eeprom\n");
>> +		em28xx_info("board has no eeprom\n");
>>  		memset(eedata, 0, len);
>>  		return -ENODEV;
>>  	}
>> @@ -408,8 +408,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  
>>  	err = i2c_master_send(&dev->i2c_client, &buf, 1);
>>  	if (err != 1) {
>> -		printk(KERN_INFO "%s: Huh, no eeprom present (err=%d)?\n",
>> -		       dev->name, err);
>> +		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
>>  		return err;
>>  	}
>>  
>> @@ -426,9 +425,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  
>>  		if (block !=
>>  		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
>> -			printk(KERN_WARNING
>> -			       "%s: i2c eeprom read error (err=%d)\n",
>> -			       dev->name, err);
>> +			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
>>  			return err;
>>  		}
>>  		size -= block;
>> @@ -436,7 +433,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	}
>>  	for (i = 0; i < len; i++) {
>>  		if (0 == (i % 16))
>> -			printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
>> +			em28xx_info("i2c eeprom %02x:", i);
>>  		printk(" %02x", eedata[i]);
>>  		if (15 == (i % 16))
>>  			printk("\n");
>> @@ -445,55 +442,51 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	if (em_eeprom->id == 0x9567eb1a)
>>  		dev->hash = em28xx_hash_mem(eedata, len, 32);
>>  
>> -	printk(KERN_INFO "%s: EEPROM ID= 0x%08x, EEPROM hash = 0x%08lx\n",
>> -	       dev->name, em_eeprom->id, dev->hash);
>> +	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
>> +		    em_eeprom->id, dev->hash);
>>  
>> -	printk(KERN_INFO "%s: EEPROM info:\n", dev->name);
>> +	em28xx_info("EEPROM info:\n");
>>  
>>  	switch (em_eeprom->chip_conf >> 4 & 0x3) {
>>  	case 0:
>> -		printk(KERN_INFO "%s:\tNo audio on board.\n", dev->name);
>> +		em28xx_info("\tNo audio on board.\n");
>>  		break;
>>  	case 1:
>> -		printk(KERN_INFO "%s:\tAC97 audio (5 sample rates)\n",
>> -				 dev->name);
>> +		em28xx_info("\tAC97 audio (5 sample rates)\n");
>>  		break;
>>  	case 2:
>> -		printk(KERN_INFO "%s:\tI2S audio, sample rate=32k\n",
>> -				 dev->name);
>> +		em28xx_info("\tI2S audio, sample rate=32k\n");
>>  		break;
>>  	case 3:
>> -		printk(KERN_INFO "%s:\tI2S audio, 3 sample rates\n",
>> -				 dev->name);
>> +		em28xx_info("\tI2S audio, 3 sample rates\n");
>>  		break;
>>  	}
>>  
>>  	if (em_eeprom->chip_conf & 1 << 3)
>> -		printk(KERN_INFO "%s:\tUSB Remote wakeup capable\n", dev->name);
>> +		em28xx_info("\tUSB Remote wakeup capable\n");
>>  
>>  	if (em_eeprom->chip_conf & 1 << 2)
>> -		printk(KERN_INFO "%s:\tUSB Self power capable\n", dev->name);
>> +		em28xx_info("\tUSB Self power capable\n");
>>  
>>  	switch (em_eeprom->chip_conf & 0x3) {
>>  	case 0:
>> -		printk(KERN_INFO "%s:\t500mA max power\n", dev->name);
>> +		em28xx_info("\t500mA max power\n");
>>  		break;
>>  	case 1:
>> -		printk(KERN_INFO "%s:\t400mA max power\n", dev->name);
>> +		em28xx_info("\t400mA max power\n");
>>  		break;
>>  	case 2:
>> -		printk(KERN_INFO "%s:\t300mA max power\n", dev->name);
>> +		em28xx_info("\t300mA max power\n");
>>  		break;
>>  	case 3:
>> -		printk(KERN_INFO "%s:\t200mA max power\n", dev->name);
>> +		em28xx_info("\t200mA max power\n");
>>  		break;
>>  	}
>> -	printk(KERN_INFO "%s:\tTable at 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
>> -				dev->name,
>> -				em_eeprom->string_idx_table,
>> -				em_eeprom->string1,
>> -				em_eeprom->string2,
>> -				em_eeprom->string3);
>> +	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
>> +		    em_eeprom->string_idx_table,
>> +		    em_eeprom->string1,
>> +		    em_eeprom->string2,
>> +		    em_eeprom->string3);
>>  
>>  	return 0;
>>  }
>> @@ -570,8 +563,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
>>  		if (rc < 0)
>>  			continue;
>>  		i2c_devicelist[i] = i;
>> -		printk(KERN_INFO "%s: found i2c device @ 0x%x [%s]\n",
>> -		       dev->name, i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
>> +		em28xx_info("found i2c device @ 0x%x [%s]\n",
>> +			    i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
>>  	}
>>  
>>  	dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
>

Re: [PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros

[Mauro Carvalho Chehab]

Em Mon, 04 Mar 2013 19:27:12 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Am 04.03.2013 19:09, schrieb Mauro Carvalho Chehab:
> > Em Sun,  3 Mar 2013 20:37:34 +0100
> > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> >
> >> Reduces the number of characters/lines, unifies the code and improves readability.
> > Had you actually test this patch? The reason why printk() is used on some
> > places is because dev->name is not available early.
> 
> The em28xx-specific macros are using printk, too.
> They are actually just an abbreviation of the current printks (saving us
> the KERN_XY and the the dev->name parameter).
> See em28xx.h.

Hmm... em28xx dev->name used to be initialized after I2C register. 
With the current logic this patch makes sense ;)

Ok, I'm requeuing it.

> > That's said, it makes sense to replace all those em28xx-specific printk
> > functions by the standard pr_fmt-based ones (pr_err, pr_info, pr_debug, etc).
> 
> Yeah, I agree.
> But that would be a separate patch series... ;)
> I can do that (later) if you want.

IMHO, that would be a nice cleanup. Of course, there's no hush for such
cleanups ;)
> 
> Regards,
> Frank
> 
> >
> > Regards,
> > Mauro
> >
> >> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> >> ---
> >>  drivers/media/usb/em28xx/em28xx-i2c.c |   55 ++++++++++++++-------------------
> >>  1 Datei geändert, 24 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-)
> >>
> >> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> >> index 8532c1d..8819b54 100644
> >> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> >> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> >> @@ -399,7 +399,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
> >>  	/* Check if board has eeprom */
> >>  	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
> >>  	if (err < 0) {
> >> -		em28xx_errdev("board has no eeprom\n");
> >> +		em28xx_info("board has no eeprom\n");
> >>  		memset(eedata, 0, len);
> >>  		return -ENODEV;
> >>  	}
> >> @@ -408,8 +408,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
> >>  
> >>  	err = i2c_master_send(&dev->i2c_client, &buf, 1);
> >>  	if (err != 1) {
> >> -		printk(KERN_INFO "%s: Huh, no eeprom present (err=%d)?\n",
> >> -		       dev->name, err);
> >> +		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
> >>  		return err;
> >>  	}
> >>  
> >> @@ -426,9 +425,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
> >>  
> >>  		if (block !=
> >>  		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
> >> -			printk(KERN_WARNING
> >> -			       "%s: i2c eeprom read error (err=%d)\n",
> >> -			       dev->name, err);
> >> +			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
> >>  			return err;
> >>  		}
> >>  		size -= block;
> >> @@ -436,7 +433,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
> >>  	}
> >>  	for (i = 0; i < len; i++) {
> >>  		if (0 == (i % 16))
> >> -			printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
> >> +			em28xx_info("i2c eeprom %02x:", i);
> >>  		printk(" %02x", eedata[i]);
> >>  		if (15 == (i % 16))
> >>  			printk("\n");
> >> @@ -445,55 +442,51 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
> >>  	if (em_eeprom->id == 0x9567eb1a)
> >>  		dev->hash = em28xx_hash_mem(eedata, len, 32);
> >>  
> >> -	printk(KERN_INFO "%s: EEPROM ID= 0x%08x, EEPROM hash = 0x%08lx\n",
> >> -	       dev->name, em_eeprom->id, dev->hash);
> >> +	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
> >> +		    em_eeprom->id, dev->hash);
> >>  
> >> -	printk(KERN_INFO "%s: EEPROM info:\n", dev->name);
> >> +	em28xx_info("EEPROM info:\n");
> >>  
> >>  	switch (em_eeprom->chip_conf >> 4 & 0x3) {
> >>  	case 0:
> >> -		printk(KERN_INFO "%s:\tNo audio on board.\n", dev->name);
> >> +		em28xx_info("\tNo audio on board.\n");
> >>  		break;
> >>  	case 1:
> >> -		printk(KERN_INFO "%s:\tAC97 audio (5 sample rates)\n",
> >> -				 dev->name);
> >> +		em28xx_info("\tAC97 audio (5 sample rates)\n");
> >>  		break;
> >>  	case 2:
> >> -		printk(KERN_INFO "%s:\tI2S audio, sample rate=32k\n",
> >> -				 dev->name);
> >> +		em28xx_info("\tI2S audio, sample rate=32k\n");
> >>  		break;
> >>  	case 3:
> >> -		printk(KERN_INFO "%s:\tI2S audio, 3 sample rates\n",
> >> -				 dev->name);
> >> +		em28xx_info("\tI2S audio, 3 sample rates\n");
> >>  		break;
> >>  	}
> >>  
> >>  	if (em_eeprom->chip_conf & 1 << 3)
> >> -		printk(KERN_INFO "%s:\tUSB Remote wakeup capable\n", dev->name);
> >> +		em28xx_info("\tUSB Remote wakeup capable\n");
> >>  
> >>  	if (em_eeprom->chip_conf & 1 << 2)
> >> -		printk(KERN_INFO "%s:\tUSB Self power capable\n", dev->name);
> >> +		em28xx_info("\tUSB Self power capable\n");
> >>  
> >>  	switch (em_eeprom->chip_conf & 0x3) {
> >>  	case 0:
> >> -		printk(KERN_INFO "%s:\t500mA max power\n", dev->name);
> >> +		em28xx_info("\t500mA max power\n");
> >>  		break;
> >>  	case 1:
> >> -		printk(KERN_INFO "%s:\t400mA max power\n", dev->name);
> >> +		em28xx_info("\t400mA max power\n");
> >>  		break;
> >>  	case 2:
> >> -		printk(KERN_INFO "%s:\t300mA max power\n", dev->name);
> >> +		em28xx_info("\t300mA max power\n");
> >>  		break;
> >>  	case 3:
> >> -		printk(KERN_INFO "%s:\t200mA max power\n", dev->name);
> >> +		em28xx_info("\t200mA max power\n");
> >>  		break;
> >>  	}
> >> -	printk(KERN_INFO "%s:\tTable at 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
> >> -				dev->name,
> >> -				em_eeprom->string_idx_table,
> >> -				em_eeprom->string1,
> >> -				em_eeprom->string2,
> >> -				em_eeprom->string3);
> >> +	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
> >> +		    em_eeprom->string_idx_table,
> >> +		    em_eeprom->string1,
> >> +		    em_eeprom->string2,
> >> +		    em_eeprom->string3);
> >>  
> >>  	return 0;
> >>  }
> >> @@ -570,8 +563,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
> >>  		if (rc < 0)
> >>  			continue;
> >>  		i2c_devicelist[i] = i;
> >> -		printk(KERN_INFO "%s: found i2c device @ 0x%x [%s]\n",
> >> -		       dev->name, i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
> >> +		em28xx_info("found i2c device @ 0x%x [%s]\n",
> >> +			    i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
> >>  	}
> >>  
> >>  	dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
> >
> 


-- 

Cheers,
Mauro

Re: [PATCH v2 06/11] em28xx: make sure we are at i2c bus A when calling em28xx_i2c_register()

[Mauro Carvalho Chehab]

Em Sun,  3 Mar 2013 20:37:39 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> The current code first configures register EM28XX_R06_I2C_CLK, which includes
> i2c speed, ack, wait and (on some devices) i2c bus selection.
> The register value usually comes from the board definition.
> em28xx_i2c_register() is called afterwards, which also tries to read the eeprom.
> If the device uses i2c bus B, eeprom reading fails.
> 
> Fix the problem by selecting bus A before calling em28xx_i2c_register() and
> apply the board settings for register EM28XX_R06_I2C_CLK afterwards.
> I also noticed that this is what the Windows driver does.
> To be sure the i2c bus scan works as expected/before, remove its call from
> em28xx_i2c_register() and call it directly after the i2c bus has been configured.

This doesn't seen ok, for a few reasons:

	1) On all current em2874 (and upper) similar devices, the driver
doesn't even care for any devices at bus A, as the TV demods, tuners, etc
are at bus B. The only thing at bus A is the eeprom;

	2) It reduces the bus speed to the minimal rate, slowing down the
device probing, as it doesn't honor the I2C bus speed bits;

	3) It doesn't properly address the real issue: a separate I2C
register is needed for bus B.

Regards,
Mauro

> 
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-cards.c |   31 ++++++++++++++++++++-----------
>  drivers/media/usb/em28xx/em28xx-i2c.c   |    7 -------
>  2 Dateien geändert, 20 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 9332d05..0d74734 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -66,6 +66,10 @@ module_param(usb_xfer_mode, int, 0444);
>  MODULE_PARM_DESC(usb_xfer_mode,
>  		 "USB transfer mode for frame data (-1 = auto, 0 = prefer isoc, 1 = prefer bulk)");
>  
> +static unsigned int i2c_scan;
> +module_param(i2c_scan, int, 0444);
> +MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
> +
>  
>  /* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
>  static unsigned long em28xx_devused;
> @@ -3074,8 +3078,20 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  		snprintf(dev->name, sizeof(dev->name), "%s #%d", chip_name, dev->devno);
>  	}
>  
> +	/* Select i2c bus A (if necessary) */
> +	if (dev->chip_id == CHIP_ID_EM2874 ||
> +	    dev->chip_id == CHIP_ID_EM28174 ||
> +	    dev->chip_id == CHIP_ID_EM2884)
> +		em28xx_write_reg_bits(dev, EM28XX_R06_I2C_CLK, 0, EM2874_I2C_SECONDARY_BUS_SELECT);
> +	/* Register i2c bus */
> +	retval = em28xx_i2c_register(dev);
> +	if (retval < 0) {
> +		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
> +			__func__, retval);
> +		return retval;
> +	}
> +	/* Configure i2c bus */
>  	if (!dev->board.is_em2800) {
> -		/* Resets I2C speed */
>  		retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
>  		if (retval < 0) {
>  			em28xx_errdev("%s: em28xx_write_reg failed!"
> @@ -3084,6 +3100,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  			return retval;
>  		}
>  	}
> +	/* Scan i2c bus */
> +	if (i2c_scan)
> +		em28xx_do_i2c_scan(dev);
>  
>  	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
>  	if (retval < 0) {
> @@ -3094,14 +3113,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  	v4l2_ctrl_handler_init(hdl, 8);
>  	dev->v4l2_dev.ctrl_handler = hdl;
>  
> -	/* register i2c bus */
> -	retval = em28xx_i2c_register(dev);
> -	if (retval < 0) {
> -		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
> -			__func__, retval);
> -		goto unregister_dev;
> -	}
> -
>  	/*
>  	 * Default format, used for tvp5150 or saa711x output formats
>  	 */
> @@ -3173,8 +3184,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  fail:
>  	em28xx_i2c_unregister(dev);
>  	v4l2_ctrl_handler_free(&dev->ctrl_handler);
> -
> -unregister_dev:
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return retval;
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> index 19f3e4f..ebe4b20 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -33,10 +33,6 @@
>  
>  /* ----------------------------------------------------------- */
>  
> -static unsigned int i2c_scan;
> -module_param(i2c_scan, int, 0444);
> -MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
> -
>  static unsigned int i2c_debug;
>  module_param(i2c_debug, int, 0644);
>  MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
> @@ -606,9 +602,6 @@ int em28xx_i2c_register(struct em28xx *dev)
>  		return retval;
>  	}
>  
> -	if (i2c_scan)
> -		em28xx_do_i2c_scan(dev);
> -
>  	return 0;
>  }
>  


-- 

Cheers,
Mauro

Re: [PATCH v2 06/11] em28xx: make sure we are at i2c bus A when calling em28xx_i2c_register()

[Frank Schäfer]

Am 04.03.2013 20:14, schrieb Mauro Carvalho Chehab:
> Em Sun,  3 Mar 2013 20:37:39 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> The current code first configures register EM28XX_R06_I2C_CLK, which includes
>> i2c speed, ack, wait and (on some devices) i2c bus selection.
>> The register value usually comes from the board definition.
>> em28xx_i2c_register() is called afterwards, which also tries to read the eeprom.
>> If the device uses i2c bus B, eeprom reading fails.
>>
>> Fix the problem by selecting bus A before calling em28xx_i2c_register() and
>> apply the board settings for register EM28XX_R06_I2C_CLK afterwards.
>> I also noticed that this is what the Windows driver does.
>> To be sure the i2c bus scan works as expected/before, remove its call from
>> em28xx_i2c_register() and call it directly after the i2c bus has been configured.
> This doesn't seen ok, for a few reasons:
>
> 	1) On all current em2874 (and upper) similar devices, the driver
> doesn't even care for any devices at bus A, as the TV demods, tuners, etc
> are at bus B. The only thing at bus A is the eeprom;

Right, and the patch doesn't change that. The bus switch is still done
when configuring register 0x06.

> 	2) It reduces the bus speed to the minimal rate, slowing down the
> device probing, as it doesn't honor the I2C bus speed bits;

No, the device probing ist still done _after_ register 0x06 (i2c speed
etc.) is configured.
That's why I moved the call to em28xx_do_i2c_scan() from
em28xx_i2c_register() to em28xx_init_dev() (after the register 0x06
configuration).
See the commit message.

Old code:
(device seems to starts up with bus B selected)
- configure EM28XX_R06_I2C_CLK    <=== SWITCHES TO BUS B
- call em28xx_i2c_register()
        - calls i2c_add_adapter() /* adapter registration */
        - calls em28xx_i2c_eeprom() /* read eeprom, calculate hash,
print content */        <=== FAILS, EEPROM IS ON BUS A
        - calls em28xx_do_i2c_scan() /* probes for presence of devices
at known i2c addresses */

New code:
(device seems to starts up with bus B selected)
- clear bit EM2874_I2C_SECONDARY_BUS_SELECT of register
EM28XX_R06_I2C_CLK (only for bridges that support it)        <= SWITCHES
TO BUS A
- call em28xx_i2c_register()
        - calls i2c_add_adapter() /* adapter registration */
        - calls em28xx_i2c_eeprom() /* read eeprom, calculate hash,
print content */
- configure EM28XX_R06_I2C_CLK    <=== SWITCHES TO BUS B
- call em28xx_do_i2c_scan() /* probes for presence of devices at known
i2c addresses */


The new order now is also exactly the same as used by the Windows driver
(mentioned in the commit message, too).


> 	3) It doesn't properly address the real issue: a separate I2C
> register is needed for bus B.

Definitely. :(
We talked about that at the beginning.

I have spent several ours working on this, but finally gave it up (for
now), because
a) it is a very huge change. I started changing/fixing one thing, then I
noticed that this requires fixing 2 other issues first, which again made
it necessary to change others and so on...
The main problem isn't the i2c adapter/bus changes, it's the subdevice
handling/tracking...
b) the resulting code has a big overhead and I'm not sure if it is
justified as long as there is no device yet using both busses in parallel.

Sure, we all like clean code and sooner or later we will likely be
forced to do it properly.
Maybe I will come back to it when the webcam stuff is finished.
But for now (with regards to the eeprom reading), reordering the bus
setup/configuration calls seems to be the easiest and appropriate
solution to me.

Regards,
Frank

> Regards,
> Mauro
>
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-cards.c |   31 ++++++++++++++++++++-----------
>>  drivers/media/usb/em28xx/em28xx-i2c.c   |    7 -------
>>  2 Dateien geändert, 20 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
>> index 9332d05..0d74734 100644
>> --- a/drivers/media/usb/em28xx/em28xx-cards.c
>> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
>> @@ -66,6 +66,10 @@ module_param(usb_xfer_mode, int, 0444);
>>  MODULE_PARM_DESC(usb_xfer_mode,
>>  		 "USB transfer mode for frame data (-1 = auto, 0 = prefer isoc, 1 = prefer bulk)");
>>  
>> +static unsigned int i2c_scan;
>> +module_param(i2c_scan, int, 0444);
>> +MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
>> +
>>  
>>  /* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
>>  static unsigned long em28xx_devused;
>> @@ -3074,8 +3078,20 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>>  		snprintf(dev->name, sizeof(dev->name), "%s #%d", chip_name, dev->devno);
>>  	}
>>  
>> +	/* Select i2c bus A (if necessary) */
>> +	if (dev->chip_id == CHIP_ID_EM2874 ||
>> +	    dev->chip_id == CHIP_ID_EM28174 ||
>> +	    dev->chip_id == CHIP_ID_EM2884)
>> +		em28xx_write_reg_bits(dev, EM28XX_R06_I2C_CLK, 0, EM2874_I2C_SECONDARY_BUS_SELECT);
>> +	/* Register i2c bus */
>> +	retval = em28xx_i2c_register(dev);
>> +	if (retval < 0) {
>> +		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
>> +			__func__, retval);
>> +		return retval;
>> +	}
>> +	/* Configure i2c bus */
>>  	if (!dev->board.is_em2800) {
>> -		/* Resets I2C speed */
>>  		retval = em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
>>  		if (retval < 0) {
>>  			em28xx_errdev("%s: em28xx_write_reg failed!"
>> @@ -3084,6 +3100,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>>  			return retval;
>>  		}
>>  	}
>> +	/* Scan i2c bus */
>> +	if (i2c_scan)
>> +		em28xx_do_i2c_scan(dev);
>>  
>>  	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
>>  	if (retval < 0) {
>> @@ -3094,14 +3113,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>>  	v4l2_ctrl_handler_init(hdl, 8);
>>  	dev->v4l2_dev.ctrl_handler = hdl;
>>  
>> -	/* register i2c bus */
>> -	retval = em28xx_i2c_register(dev);
>> -	if (retval < 0) {
>> -		em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
>> -			__func__, retval);
>> -		goto unregister_dev;
>> -	}
>> -
>>  	/*
>>  	 * Default format, used for tvp5150 or saa711x output formats
>>  	 */
>> @@ -3173,8 +3184,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>>  fail:
>>  	em28xx_i2c_unregister(dev);
>>  	v4l2_ctrl_handler_free(&dev->ctrl_handler);
>> -
>> -unregister_dev:
>>  	v4l2_device_unregister(&dev->v4l2_dev);
>>  
>>  	return retval;
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index 19f3e4f..ebe4b20 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -33,10 +33,6 @@
>>  
>>  /* ----------------------------------------------------------- */
>>  
>> -static unsigned int i2c_scan;
>> -module_param(i2c_scan, int, 0444);
>> -MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
>> -
>>  static unsigned int i2c_debug;
>>  module_param(i2c_debug, int, 0644);
>>  MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
>> @@ -606,9 +602,6 @@ int em28xx_i2c_register(struct em28xx *dev)
>>  		return retval;
>>  	}
>>  
>> -	if (i2c_scan)
>> -		em28xx_do_i2c_scan(dev);
>> -
>>  	return 0;
>>  }
>>  

Re: [PATCH v2 06/11] em28xx: make sure we are at i2c bus A when calling em28xx_i2c_register()

[Mauro Carvalho Chehab]

Em Mon, 04 Mar 2013 22:24:43 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Am 04.03.2013 20:14, schrieb Mauro Carvalho Chehab:

> > 	3) It doesn't properly address the real issue: a separate I2C
> > register is needed for bus B.
> 
> Definitely. :(
> We talked about that at the beginning.
> 
> I have spent several ours working on this, but finally gave it up (for
> now), because
> a) it is a very huge change. I started changing/fixing one thing, then I
> noticed that this requires fixing 2 other issues first, which again made
> it necessary to change others and so on...
> The main problem isn't the i2c adapter/bus changes, it's the subdevice
> handling/tracking...
> b) the resulting code has a big overhead and I'm not sure if it is
> justified as long as there is no device yet using both busses in parallel.
> 
> Sure, we all like clean code and sooner or later we will likely be
> forced to do it properly.
> Maybe I will come back to it when the webcam stuff is finished.
> But for now (with regards to the eeprom reading), reordering the bus
> setup/configuration calls seems to be the easiest and appropriate
> solution to me.

Hmm... I found it easy to do it ;)

The fact that, currently, on all devices, The first bus has the eeprom[1]
and that all other devices are on the secondary bus makes easy to properly
implementing it with just a few changes. Ok, if/when we start to see
devices mixed, we'll need to add a more complex logic, but for now,
it can be simply done, and em28xx i2c scan can check both buses, with
may help future development.

I'll post some patches tomorrow after testing.

[1] That could be a hardware requirement, as it makes easier for the
device to seek for eeprom data.

Regards,
Mauro