Skip to contentSkip to conversation

fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134

V4L Maciej Szmigiero 2 participants 3 messages Oct 25, 2010 – Nov 9, 2010 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

[V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134

When Medion 7134 analog+DVB-T card is cold (after powerup) the tda9887 analog demodulator won't show on i2c bus.
This results in no signal on analog TV. After loading driver for second time eeprom (required for tuner autodetection) read is corrupted, but tda9987 is detected properly and analog TV works when tuner model is forced.

Fix tda9887 problem by moving its detect code after tuner setup which unhides it. The eeprom read issue is fixed by opening i2c gate in DVB-T demodulator.

Tested on Medion 7134 and also tested for reference on Typhoon Cardbus Hybrid (which also uses saa7134 driver).

drivers/media/video/saa7134/saa7134-cards.c +35 -5
@@ -7249,12 +7249,37 @@
 		break;
 	case SAA7134_BOARD_MD7134:
 	{
-		u8 subaddr;
+		u8 subaddr, dmdregval;
 		u8 data[3];
 		int ret, tuner_t;
+		struct i2c_msg i2cgatemsg_r[] = { {.addr = 0x08, .flags = 0,
+						.buf = &subaddr, .len = 1},
+						{.addr = 0x08,
+							.flags = I2C_M_RD,
+						.buf = &dmdregval, .len = 1} };
+		struct i2c_msg i2cgatemsg_w[] = { {.addr = 0x08, .flags = 0,
+						.buf = data, .len = 2} };
 		struct i2c_msg msg[] = {{.addr=0x50, .flags=0, .buf=&subaddr, .len = 1},
 					{.addr=0x50, .flags=I2C_M_RD, .buf=data, .len = 3}};
 
+		/* open i2c gate in DVB-T demod */
+		/* so eeprom read won't be corrupted */
+		subaddr = 0x7;
+		ret = i2c_transfer(&dev->i2c_adap, i2cgatemsg_r, 2);
+		if ((ret == 2) && (dmdregval & 0x2)) {
+			printk(KERN_NOTICE "%s DVB-T demod i2c gate was left"
+						    " closed\n", dev->name);
+			printk(KERN_NOTICE "%s previous informational"
+					    " EEPROM read might have been"
+					    " corrupted\n", dev->name);
+
+			data[0] = 0x7;
+			data[1] = (dmdregval & ~0x2);
+			if (i2c_transfer(&dev->i2c_adap, i2cgatemsg_w, 1) != 1)
+				printk(KERN_ERR "early i2c gate"
+						" open failure\n");
+		}
+
 		subaddr= 0x14;
 		tuner_t = 0;
 
@@ -7522,10 +7547,6 @@
 			v4l2_i2c_new_subdev(&dev->v4l2_dev,
 				&dev->i2c_adap, "tuner", "tuner",
 				dev->radio_addr, NULL);
-		if (has_demod)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev,
-				&dev->i2c_adap, "tuner", "tuner",
-				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
 		if (dev->tuner_addr == ADDR_UNSET) {
 			enum v4l2_i2c_tuner_type type =
 				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
@@ -7542,6 +7563,15 @@
 
 	saa7134_tuner_setup(dev);
 
+	/* some cards (Medion 7134 for example) needs tuner to be setup */
+	/* before tda9887 shows itself on i2c bus */
+	if ((TUNER_ABSENT != dev->tuner_type)
+			&& (dev->tda9887_conf & TDA9887_PRESENT)) {
+		v4l2_i2c_new_subdev(&dev->v4l2_dev,
+			&dev->i2c_adap, "tuner", "tuner",
+			0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
+	}
+
 	switch (dev->board) {
 	case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM:
 	case SAA7134_BOARD_AVERMEDIA_CARDBUS_501:
#1 lore raw

Em 25-10-2010 15:59, Maciej Szmigiero escreveu:

Quoted 36 lines

[V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134

When Medion 7134 analog+DVB-T card is cold (after powerup) the tda9887 analog demodulator won't show on i2c bus.
This results in no signal on analog TV. After loading driver for second time eeprom (required for tuner autodetection) read is corrupted, but tda9987 is detected properly and analog TV works when tuner model is forced.

Fix tda9887 problem by moving its detect code after tuner setup which unhides it. The eeprom read issue is fixed by opening i2c gate in DVB-T demodulator.

Tested on Medion 7134 and also tested for reference on Typhoon Cardbus Hybrid (which also uses saa7134 driver).

drivers/media/video/saa7134/saa7134-cards.c +8 -1
@@ -7249,12 +7249,37 @@
 		break;
 	case SAA7134_BOARD_MD7134:
 	{
-		u8 subaddr;
+		u8 subaddr, dmdregval;
 		u8 data[3];
 		int ret, tuner_t;
+		struct i2c_msg i2cgatemsg_r[] = { {.addr = 0x08, .flags = 0,
+						.buf = &subaddr, .len = 1},
+						{.addr = 0x08,
+							.flags = I2C_M_RD,
+						.buf = &dmdregval, .len = 1} };
+		struct i2c_msg i2cgatemsg_w[] = { {.addr = 0x08, .flags = 0,
+						.buf = data, .len = 2} };
 		struct i2c_msg msg[] = {{.addr=0x50, .flags=0, .buf=&subaddr, .len = 1},
 					{.addr=0x50, .flags=I2C_M_RD, .buf=data, .len = 3}};
 

drivers/media/video/saa7134/saa7134-cards.c line 7274

Quoted 10 lines
drivers/media/video/saa7134/saa7134-cards.c +10 -0
+		/* open i2c gate in DVB-T demod */
+		/* so eeprom read won't be corrupted */
+		subaddr = 0x7;
+		ret = i2c_transfer(&dev->i2c_adap, i2cgatemsg_r, 2);
+		if ((ret == 2) && (dmdregval & 0x2)) {
+			printk(KERN_NOTICE "%s DVB-T demod i2c gate was left"
+						    " closed\n", dev->name);
+			printk(KERN_NOTICE "%s previous informational"
+					    " EEPROM read might have been"
+					    " corrupted\n", dev->name);

hmm... I don't think we need those debug messages on normal cases. Also, we want the log shown as:

	saa7134: <foo>
and not as:
	saa7134 <foo>

drivers/media/video/saa7134/saa7134-cards.c line 7280

Quoted 6 lines
drivers/media/video/saa7134/saa7134-cards.c +6 -0
+
+			data[0] = 0x7;
+			data[1] = (dmdregval & ~0x2);
+			if (i2c_transfer(&dev->i2c_adap, i2cgatemsg_w, 1) != 1)
+				printk(KERN_ERR "early i2c gate"
+						" open failure\n");

Gah. A message like that should really specify the dev->name, and the board.

Quoted 22 lines
drivers/media/video/saa7134/saa7134-cards.c +4 -4
+		}
+
 		subaddr= 0x14;
 		tuner_t = 0;
 
@@ -7522,10 +7547,6 @@
 			v4l2_i2c_new_subdev(&dev->v4l2_dev,
 				&dev->i2c_adap, "tuner", "tuner",
 				dev->radio_addr, NULL);
-		if (has_demod)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev,
-				&dev->i2c_adap, "tuner", "tuner",
-				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
 		if (dev->tuner_addr == ADDR_UNSET) {
 			enum v4l2_i2c_tuner_type type =
 				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
@@ -7542,6 +7563,15 @@
 
 	saa7134_tuner_setup(dev);
 
+	/* some cards (Medion 7134 for example) needs tuner to be setup */
+	/* before tda9887 shows itself on i2c bus */

drivers/media/video/saa7134/saa7134-cards.c line 7577

Quoted 10 lines
drivers/media/video/saa7134/saa7134-cards.c +7 -0
+	if ((TUNER_ABSENT != dev->tuner_type)
+			&& (dev->tda9887_conf & TDA9887_PRESENT)) {
+		v4l2_i2c_new_subdev(&dev->v4l2_dev,
+			&dev->i2c_adap, "tuner", "tuner",
+			0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
+	}
+
 	switch (dev->board) {
 	case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM:
 	case SAA7134_BOARD_AVERMEDIA_CARDBUS_501:

The order change for the demod probe will likely break support for other boards. If the problem is specific to Medion 7134, what you should do, instead, is to change the order just for MD7134 (so, inside the switch(dev->board)).

Cheers,
Mauro.

#2 lore raw

W dniu 09.11.2010 11:53, Mauro Carvalho Chehab pisze:

Quoted 8 lines

Em 25-10-2010 15:59, Maciej Szmigiero escreveu:

drivers/media/video/saa7134/saa7134-cards.c line 7274

Quoted 5 lines
drivers/media/video/saa7134/saa7134-cards.c +5 -0
+			printk(KERN_NOTICE "%s DVB-T demod i2c gate was left"
+						    " closed\n", dev->name);
+			printk(KERN_NOTICE "%s previous informational"
+					    " EEPROM read might have been"
+					    " corrupted\n", dev->name);

hmm... I don't think we need those debug messages on normal cases.

I added this message because when the gate was left closed the eeprom content (printed out unconditionally in saa7134_i2c_eeprom) looks garbled in dmesg, so it's better to inform user that he (or she) shouldn't be worried about this.
The eeprom dump is called from saa7134_i2c_register, before card-specific code has opportunity to run.

Quoted 18 lines
Quoted 4 lines
drivers/media/video/saa7134/saa7134-cards.c +2 -0
 	saa7134_tuner_setup(dev);
 
+	/* some cards (Medion 7134 for example) needs tuner to be setup */
+	/* before tda9887 shows itself on i2c bus */

drivers/media/video/saa7134/saa7134-cards.c line 7577

Quoted 10 lines
drivers/media/video/saa7134/saa7134-cards.c +7 -0
+	if ((TUNER_ABSENT != dev->tuner_type)
+			&& (dev->tda9887_conf & TDA9887_PRESENT)) {
+		v4l2_i2c_new_subdev(&dev->v4l2_dev,
+			&dev->i2c_adap, "tuner", "tuner",
+			0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
+	}
+
 	switch (dev->board) {
 	case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM:
 	case SAA7134_BOARD_AVERMEDIA_CARDBUS_501:

The order change for the demod probe will likely break support for other boards. If the problem is specific to Medion 7134, what you should do, instead, is to change the order just for MD7134 (so, inside the switch(dev->board)).

It was done in that order (tuner first then tda9887) for a long time before it was changed.
For example ( http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/media/video/saa7134/saa7134-core.c;hb=7a766f9ddd74b50d6069f054a3004ece0439f5c1 ) it used to look like that:
942 /* load i2c helpers */
943 if (TUNER_ABSENT != dev->tuner_type)
944 request_module("tuner");
945 if (dev->tda9887_conf)
946 request_module("tda9887");
947 if (card_is_empress(dev)) {
948 request_module("saa6752hs");
949 request_module_depend("saa7134-empress",&need_empress);
950 }

But then the code for tda9887 was integrated into tuner module and later split out again, this time reversing the detection order (by accident I suppose).

Quoted 3 lines

Cheers,
Mauro.

Best regards,
Maciej Szmigiero