[GIT,PULL,FOR,3.5] AF9035/AF9033/TUA9001 => TerraTec Cinergy T Stick [0ccd:0093]

Message ID 20120401151153.637d2393@milhouse (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Michael Büsch April 1, 2012, 1:11 p.m. UTC
  On Sun, 01 Apr 2012 15:29:07 +0300
Antti Palosaari <crope@iki.fi> wrote:
> buf[1] = msg[0].addr << 1;
> Maybe you have given I2C address as a "8bit" format?

Uhh, the address is leftshifted by one.
So I changed the i2c address from 0xC0 to 0x60.

The i2c write seems to work now. At least it doesn't complain anymore
and it sorta seems to tune to the right frequency.
But i2c read may be broken.
I had to enable the commented read code, but it still fails to read
the VCO calibration value:

[ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)

It doesn't run into this check on the other af903x driver.
So I suspect an i2c read issue here.

Attached: The patches.
  

Comments

Antti Palosaari April 1, 2012, 1:19 p.m. UTC | #1
On 01.04.2012 16:11, Michael Büsch wrote:
> On Sun, 01 Apr 2012 15:29:07 +0300
> Antti Palosaari<crope@iki.fi>  wrote:
>> buf[1] = msg[0].addr<<  1;
>> Maybe you have given I2C address as a "8bit" format?
>
> Uhh, the address is leftshifted by one.
> So I changed the i2c address from 0xC0 to 0x60.

That's a very common mistake, I2C addresses are 7 bit and LSB is 
direction. But it is very common to see used it as a "8bit" format, 0xC0 
write address and 0xc1 as a read address. Even some data-sheets have 
that "wrong" naming.
There is many drivers that is wrong and causing confusion, even my old 
AF9015... :]

> The i2c write seems to work now. At least it doesn't complain anymore
> and it sorta seems to tune to the right frequency.
> But i2c read may be broken.
> I had to enable the commented read code, but it still fails to read
> the VCO calibration value:
>
> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
>
> It doesn't run into this check on the other af903x driver.
> So I suspect an i2c read issue here.

That could be I2C adapter issue too, TUA9001 does not use it and thus 
not tested at all... But for my eyes it looks logically correct still.

> Attached: The patches.

Lets see.

regards
Antti
  
Hans-Frieder Vogt April 1, 2012, 2:42 p.m. UTC | #2
Michael,

Am Sonntag, 1. April 2012 schrieb Michael Büsch:
> On Sun, 01 Apr 2012 15:29:07 +0300
> 
> Antti Palosaari <crope@iki.fi> wrote:
> > buf[1] = msg[0].addr << 1;
> > Maybe you have given I2C address as a "8bit" format?
> 
> Uhh, the address is leftshifted by one.
> So I changed the i2c address from 0xC0 to 0x60.
> 
> The i2c write seems to work now. At least it doesn't complain anymore
> and it sorta seems to tune to the right frequency.
> But i2c read may be broken.
> I had to enable the commented read code, but it still fails to read
> the VCO calibration value:
> 
> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
> 
> It doesn't run into this check on the other af903x driver.
> So I suspect an i2c read issue here.

I would first uncomment the i2c read functionality in Antti's driver!

> 
> Attached: The patches.

Cheers,

Hans-Frieder Vogt                       e-mail: hfvogt <at> gmx .dot. net
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Michael Büsch April 1, 2012, 2:56 p.m. UTC | #3
On Sun, 1 Apr 2012 16:42:34 +0200
"Hans-Frieder Vogt" <hfvogt@gmx.net> wrote:
> > [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
> > 
> > It doesn't run into this check on the other af903x driver.
> > So I suspect an i2c read issue here.
> 
> I would first uncomment the i2c read functionality in Antti's driver!

I did this.

> > Attached: The patches.

See the patches.
  
Michael Büsch April 1, 2012, 4:15 p.m. UTC | #4
On Sun, 1 Apr 2012 15:11:53 +0200
Michael Büsch <m@bues.ch> wrote:

> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)

Ok, it turns out that it doesn't fail all the time, but only sporadically.
So increasing the number of retries fixes (or at least works around) it.

No idea why this behaves differently from fc0011 on Hans' driver, though.
  
Antti Palosaari April 1, 2012, 4:20 p.m. UTC | #5
On 01.04.2012 19:15, Michael Büsch wrote:
> On Sun, 1 Apr 2012 15:11:53 +0200
> Michael Büsch<m@bues.ch>  wrote:
>
>> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
>
> Ok, it turns out that it doesn't fail all the time, but only sporadically.
> So increasing the number of retries fixes (or at least works around) it.

OK, feel free to add ~3 retries inside af9035_ctrl_msg() i think.

You didn't mention if error is coming from af9035 firmware or from USB 
stack. Just for the interest...

> No idea why this behaves differently from fc0011 on Hans' driver, though.

Maybe some delay or there is retry on error logic.

regards
Antti
  
Hans-Frieder Vogt April 1, 2012, 4:24 p.m. UTC | #6
Am Sonntag, 1. April 2012 schrieb Michael Büsch:
> On Sun, 1 Apr 2012 16:42:34 +0200
> 
> "Hans-Frieder Vogt" <hfvogt@gmx.net> wrote:
> > > [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
> > > 
> > > It doesn't run into this check on the other af903x driver.
> > > So I suspect an i2c read issue here.
> > 
> > I would first uncomment the i2c read functionality in Antti's driver!
> 
> I did this.

sorry. I didn't check your patches. However, I found the problem: the buffer 
length needs to be msg[1].len, see below. For my mxl5007t based device it 
worked.

 --- old/af9035.c 2012-04-01 16:41:53.694103691 +0200
+++ new/af9035.c    2012-04-01 18:22:25.026930784 +0200
@@ -209,24 +209,15 @@
                                        msg[1].len);
                } else {
                        /* I2C */
-#if 0
-                       /*
-                        * FIXME: Keep that code. It should work but as it is
-                        * not tested I left it disabled and return -
EOPNOTSUPP
-                        * for the sure.
-                        */
                        u8 buf[4 + msg[0].len];
                        struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
                                        buf, msg[1].len, msg[1].buf };
-                       buf[0] = msg[0].len;
+                       buf[0] = msg[1].len;
                        buf[1] = msg[0].addr << 1;
                        buf[2] = 0x01;
                        buf[3] = 0x00;
                        memcpy(&buf[4], msg[0].buf, msg[0].len);
                        ret = af9035_ctrl_msg(d->udev, &req);
-#endif
-                       pr_debug("%s: I2C operation not supported\n", 
__func__);
-                       ret = -EOPNOTSUPP;
                }
        } else if (num == 1 && !(msg[0].flags & I2C_M_RD)) {
                if (msg[0].len > 40) {


> > > Attached: The patches.
> 
> See the patches.

cheers,

Hans-Frieder Vogt                       e-mail: hfvogt <at> gmx .dot. net
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Michael Büsch April 1, 2012, 4:32 p.m. UTC | #7
On Sun, 01 Apr 2012 19:20:21 +0300
Antti Palosaari <crope@iki.fi> wrote:

> On 01.04.2012 19:15, Michael Büsch wrote:
> > On Sun, 1 Apr 2012 15:11:53 +0200
> > Michael Büsch<m@bues.ch>  wrote:
> >
> >> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
> >
> > Ok, it turns out that it doesn't fail all the time, but only sporadically.
> > So increasing the number of retries fixes (or at least works around) it.
> 
> OK, feel free to add ~3 retries inside af9035_ctrl_msg() i think.

Well I didn't retry at that level, but at the fc0011 driver level.
It does already retry once in fc0011 (with complete tuner reset).
I increased it to 6 times (3 was not enough).

I think we can't retry at af9035_ctrl_msg() level, because the
actual i2c/usb transfer does not fail. The received packet checksum even
is ok (although we currently don't check it. I'll send a patch for that later).

> You didn't mention if error is coming from af9035 firmware or from USB 
> stack. Just for the interest...

I don't know how much the firmware is involved in this, but _maybe_ this
glitch is caused by it.
  
Michael Büsch April 1, 2012, 4:36 p.m. UTC | #8
On Sun, 1 Apr 2012 18:24:09 +0200
"Hans-Frieder Vogt" <hfvogt@gmx.net> wrote:
> However, I found the problem: the buffer 
> length needs to be msg[1].len, see below.

I spotted this bug, too, but it didn't fix the problem, as len is 1 for
both packets in my specific case. So it doesn't make a difference in this case.

But as I said in the previous mail: I did more debugging and I'm pretty sure that
the actual i2c reads and writes work as expected. The problem is at another level.

>  --- old/af9035.c 2012-04-01 16:41:53.694103691 +0200
> +++ new/af9035.c    2012-04-01 18:22:25.026930784 +0200
> @@ -209,24 +209,15 @@
>                                         msg[1].len);
>                 } else {
>                         /* I2C */
> -#if 0
> -                       /*
> -                        * FIXME: Keep that code. It should work but as it is
> -                        * not tested I left it disabled and return -
> EOPNOTSUPP
> -                        * for the sure.
> -                        */
>                         u8 buf[4 + msg[0].len];
>                         struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
>                                         buf, msg[1].len, msg[1].buf };
> -                       buf[0] = msg[0].len;
> +                       buf[0] = msg[1].len;
>                         buf[1] = msg[0].addr << 1;
>                         buf[2] = 0x01;
>                         buf[3] = 0x00;
>                         memcpy(&buf[4], msg[0].buf, msg[0].len);
>                         ret = af9035_ctrl_msg(d->udev, &req);
> -#endif
> -                       pr_debug("%s: I2C operation not supported\n", 
> __func__);
> -                       ret = -EOPNOTSUPP;
>                 }
>         } else if (num == 1 && !(msg[0].flags & I2C_M_RD)) {
>                 if (msg[0].len > 40) {
  
Antti Palosaari April 1, 2012, 4:39 p.m. UTC | #9
On 01.04.2012 19:32, Michael Büsch wrote:
> On Sun, 01 Apr 2012 19:20:21 +0300
> Antti Palosaari<crope@iki.fi>  wrote:
>
>> On 01.04.2012 19:15, Michael Büsch wrote:
>>> On Sun, 1 Apr 2012 15:11:53 +0200
>>> Michael Büsch<m@bues.ch>   wrote:
>>>
>>>> [ 3101.940765] i2c i2c-8: Failed to read VCO calibration value (got 20)
>>>
>>> Ok, it turns out that it doesn't fail all the time, but only sporadically.
>>> So increasing the number of retries fixes (or at least works around) it.
>>
>> OK, feel free to add ~3 retries inside af9035_ctrl_msg() i think.
>
> Well I didn't retry at that level, but at the fc0011 driver level.
> It does already retry once in fc0011 (with complete tuner reset).
> I increased it to 6 times (3 was not enough).

Maybe some delay is needed in order to wait tuner wakes up after the 
reset. Reason it does not occur the other driver is likely there is some 
delay somewhere...

> I think we can't retry at af9035_ctrl_msg() level, because the
> actual i2c/usb transfer does not fail. The received packet checksum even
> is ok (although we currently don't check it. I'll send a patch for that later).
>
>> You didn't mention if error is coming from af9035 firmware or from USB
>> stack. Just for the interest...
>
> I don't know how much the firmware is involved in this, but _maybe_ this
> glitch is caused by it.

Indeed, you are correct, no changes for af9035_ctrl_msg() are not wanted 
as error is not coming from af9035. Likely some small delay for tuner in 
order to wake up it from the reset.

regards
Antti
  
Michael Büsch April 1, 2012, 4:44 p.m. UTC | #10
On Sun, 01 Apr 2012 19:39:56 +0300
Antti Palosaari <crope@iki.fi> wrote:

> > Well I didn't retry at that level, but at the fc0011 driver level.
> > It does already retry once in fc0011 (with complete tuner reset).
> > I increased it to 6 times (3 was not enough).
> 
> Maybe some delay is needed in order to wait tuner wakes up after the 
> reset. Reason it does not occur the other driver is likely there is some 
> delay somewhere...

Yep, I suspect that, too. However, I already tried lots of things that didn't
really work. Such as adding 100ms delay before and after all i2c reads
and writes inside of the fc0011 driver.
I'll investigate further...
  

Patch

Index: linux/drivers/media/dvb/dvb-usb/af9035.c
===================================================================
--- linux.orig/drivers/media/dvb/dvb-usb/af9035.c	2012-04-01 11:42:38.867573221 +0200
+++ linux/drivers/media/dvb/dvb-usb/af9035.c	2012-04-01 14:44:20.848955668 +0200
@@ -22,6 +22,7 @@ 
 #include "af9035.h"
 #include "af9033.h"
 #include "tua9001.h"
+#include "fc0011.h"
 
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 static DEFINE_MUTEX(af9035_usb_mutex);
@@ -209,12 +210,6 @@ 
 					msg[1].len);
 		} else {
 			/* I2C */
-#if 0
-			/*
-			 * FIXME: Keep that code. It should work but as it is
-			 * not tested I left it disabled and return -EOPNOTSUPP
-			 * for the sure.
-			 */
 			u8 buf[4 + msg[0].len];
 			struct usb_req req = { CMD_I2C_RD, 0, sizeof(buf),
 					buf, msg[1].len, msg[1].buf };
@@ -224,9 +219,6 @@ 
 			buf[3] = 0x00;
 			memcpy(&buf[4], msg[0].buf, msg[0].len);
 			ret = af9035_ctrl_msg(d->udev, &req);
-#endif
-			pr_debug("%s: I2C operation not supported\n", __func__);
-			ret = -EOPNOTSUPP;
 		}
 	} else if (num == 1 && !(msg[0].flags & I2C_M_RD)) {
 		if (msg[0].len > 40) {
@@ -480,6 +472,7 @@ 
 
 		switch (tmp) {
 		case AF9033_TUNER_TUA9001:
+		case AF9033_TUNER_FC0011:
 			af9035_af9033_config[i].spec_inv = 1;
 			break;
 		default:
@@ -524,6 +517,83 @@ 
 	return ret;
 }
 
+static int af9035_fc0011_tuner_callback(struct dvb_usb_device *d,
+					int cmd, int arg)
+{
+	int err;
+
+	switch (cmd) {
+	case FC0011_FE_CALLBACK_POWER:
+		/* Tuner enable */
+		err = af9035_wr_reg_mask(d, 0xd8eb, 1, 1);
+		if (err)
+			return err;
+		err = af9035_wr_reg_mask(d, 0xd8ec, 1, 1);
+		if (err)
+			return err;
+		err = af9035_wr_reg_mask(d, 0xd8ed, 1, 1);
+		if (err)
+			return err;
+		/* LED */
+		err = af9035_wr_reg_mask(d, 0xd8d0, 1, 1);
+		if (err)
+			return err;
+		err = af9035_wr_reg_mask(d, 0xd8d1, 1, 1);
+		if (err)
+			return err;
+		msleep(10);
+		break;
+	case FC0011_FE_CALLBACK_RESET:
+		err = af9035_wr_reg(d, 0xd8e9, 1);
+		if (err)
+			return err;
+		err = af9035_wr_reg(d, 0xd8e8, 1);
+		if (err)
+			return err;
+		err = af9035_wr_reg(d, 0xd8e7, 1);
+		if (err)
+			return err;
+		msleep(10);
+		err = af9035_wr_reg(d, 0xd8e7, 0);
+		if (err)
+			return err;
+		msleep(10);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int af9035_tuner_callback(struct dvb_usb_device *d, int cmd, int arg)
+{
+	switch (af9035_af9033_config[0].tuner) {
+	case AF9033_TUNER_FC0011:
+		return af9035_fc0011_tuner_callback(d, cmd, arg);
+	default:
+		break;
+	}
+
+	return -ENODEV;
+}
+
+static int af9035_frontend_callback(void *adapter_priv, int component,
+				    int cmd, int arg)
+{
+	struct i2c_adapter *adap = adapter_priv;
+	struct dvb_usb_device *d = i2c_get_adapdata(adap);
+
+	switch (component) {
+	case DVB_FRONTEND_COMPONENT_TUNER:
+		return af9035_tuner_callback(d, cmd, arg);
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
 static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
 {
 	int ret;
@@ -552,6 +622,7 @@ 
 		ret = -ENODEV;
 		goto err;
 	}
+	adap->fe_adap[0].fe->callback = af9035_frontend_callback;
 
 	return 0;
 
@@ -565,6 +636,10 @@ 
 	.i2c_addr = 0x60,
 };
 
+static const struct fc0011_config af9035_fc0011_config = {
+	.i2c_address = 0x60,
+};
+
 static int af9035_tuner_attach(struct dvb_usb_adapter *adap)
 {
 	int ret;
@@ -613,6 +688,10 @@ 
 		fe = dvb_attach(tua9001_attach, adap->fe_adap[0].fe,
 				&adap->dev->i2c_adap, &af9035_tua9001_config);
 		break;
+	case AF9033_TUNER_FC0011:
+		fe = dvb_attach(fc0011_attach, adap->fe_adap[0].fe,
+				&adap->dev->i2c_adap, &af9035_fc0011_config);
+		break;
 	default:
 		fe = NULL;
 	}
Index: linux/drivers/media/dvb/frontends/af9033.c
===================================================================
--- linux.orig/drivers/media/dvb/frontends/af9033.c	2012-04-01 11:42:36.147526208 +0200
+++ linux/drivers/media/dvb/frontends/af9033.c	2012-04-01 11:42:38.919574120 +0200
@@ -297,6 +297,10 @@ 
 		len = ARRAY_SIZE(tuner_init_tua9001);
 		init = tuner_init_tua9001;
 		break;
+	case AF9033_TUNER_FC0011:
+		len = ARRAY_SIZE(tuner_init_fc0011);
+		init = tuner_init_fc0011;
+		break;
 	default:
 		pr_debug("%s: unsupported tuner ID=%d\n", __func__,
 				state->cfg.tuner);
Index: linux/drivers/media/dvb/frontends/af9033_priv.h
===================================================================
--- linux.orig/drivers/media/dvb/frontends/af9033_priv.h	2012-04-01 11:42:36.147526208 +0200
+++ linux/drivers/media/dvb/frontends/af9033_priv.h	2012-04-01 11:42:38.919574120 +0200
@@ -238,5 +238,66 @@ 
 	{ 0x80f1e6, 0x00 },
 };
 
+/* Fitipower fc0011 tuner init
+   AF9033_TUNER_FC0011    = 0x28 */
+static const struct reg_val tuner_init_fc0011[] = {
+	{ 0x800046, AF9033_TUNER_FC0011 },
+	{ 0x800057, 0x00 },
+	{ 0x800058, 0x01 },
+	{ 0x80005f, 0x00 },
+	{ 0x800060, 0x00 },
+	{ 0x800068, 0xa5 },
+	{ 0x80006e, 0x01 },
+	{ 0x800071, 0x0A },
+	{ 0x800072, 0x02 },
+	{ 0x800074, 0x01 },
+	{ 0x800079, 0x01 },
+	{ 0x800093, 0x00 },
+	{ 0x800094, 0x00 },
+	{ 0x800095, 0x00 },
+	{ 0x800096, 0x00 },
+	{ 0x80009b, 0x2D },
+	{ 0x80009c, 0x60 },
+	{ 0x80009d, 0x23 },
+	{ 0x8000a4, 0x50 },
+	{ 0x8000ad, 0x50 },
+	{ 0x8000b3, 0x01 },
+	{ 0x8000b7, 0x88 },
+	{ 0x8000b8, 0xa6 },
+	{ 0x8000c3, 0x01 },
+	{ 0x8000c4, 0x01 },
+	{ 0x8000c7, 0x69 },
+	{ 0x80F007, 0x00 },
+	{ 0x80F00A, 0x1B },
+	{ 0x80F00B, 0x1B },
+	{ 0x80F00C, 0x1B },
+	{ 0x80F00D, 0x1B },
+	{ 0x80F00E, 0xFF },
+	{ 0x80F00F, 0x01 },
+	{ 0x80F010, 0x00 },
+	{ 0x80F011, 0x02 },
+	{ 0x80F012, 0xFF },
+	{ 0x80F013, 0x01 },
+	{ 0x80F014, 0x00 },
+	{ 0x80F015, 0x02 },
+	{ 0x80F01B, 0xEF },
+	{ 0x80F01C, 0x01 },
+	{ 0x80F01D, 0x0f },
+	{ 0x80F01E, 0x02 },
+	{ 0x80F01F, 0x6E },
+	{ 0x80F020, 0x00 },
+	{ 0x80F025, 0xDE },
+	{ 0x80F026, 0x00 },
+	{ 0x80F027, 0x0A },
+	{ 0x80F028, 0x03 },
+	{ 0x80F029, 0x6E },
+	{ 0x80F02A, 0x00 },
+	{ 0x80F047, 0x00 },
+	{ 0x80F054, 0x00 },
+	{ 0x80F055, 0x00 },
+	{ 0x80F077, 0x01 },
+	{ 0x80F1E6, 0x00 },
+};
+
 #endif /* AF9033_PRIV_H */