Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[linux-dvb] Re: mt352 and SkyStar2 (FlexCop IIB) oddities



Bjorn writes:

> In mt352.c the read command is contains the following:
>       struct i2c_msg msg [] = { { .addr = I2C_MT352_ADDR,
>                                   .flags = I2C_M_NOSTART,
>                                   .buf = b0, .len = 1 },
>                                 { .addr = I2C_MT352_ADDR,
>                                   .flags = I2C_M_RD,
>                                   .buf = b1, .len = 1 } };
>
> as I2C_M_NOSTART isn't defined as 0 every read_register command will
> fail too.
> Changing I2C_M_NOSTART to 0 solves the problem and the module can read
> the mt352 registers successfully.
>
> Looking forward to hear some comments/explanations or so.

I have also been looking into this, as I'm developing for another card
which this also breaks on.  The reason this hasn't been spotted is because
the testers of the mt352 so far have been using bt878 cards with hardware
I2C support enabled.  The hardware i2c code ignores the I2C_M_NOSTART, so
it works on boards with hardware i2c enabled.  Changing the I2C_M_NOSTART
to 0, as you did, is the right thing to do here.

and Johannes writes:

> I think that parameters which are outside the DVB specification
> should be rejected (EINVAL). However, a driver might want to
> handle gracefully slightly wrong tuning parameters (e.g. a wrong FEC
> value) -- the application should be able to get correct values
> via FE_GET_FRONTEND if it is interested.

Attached is a patch against CVS that makes software I2C work, as well as
implementing the second policy.

Basically, if any parameter is set to _AUTO this enables automatic
acquisiton on the chip (for all parameters, as there is no way to specify
only a single one as auto), whilst "suggesting" good values for everything
that we do have values for.  So, if you do set a specific set of
parameters, we will stick to them.

This change also ignores a "FEC_NONE" low-priority coderate instead of
returning EINVAL, as we have no means to tell the mt352 this information.
In any case, we never ask the chip to decode the LP signal so this should
be safe.  The other "invalid" returns should be OK as they are invalid and
we don't report that we support anything other than what is explicitly
handled.

Finally, Dirk writes, on my previous patch:

> This also works for me and the sleep problem is gone. But I still need
> other parts of the diff I sent here before. Patch attached.

Dirk, I suspect that this will solve the tuning failures that you have
been seeing.  Can you confirm that you have FEC_NONE in the tzap lines
that have been failing for you and try this if so?  All of the other
changes, except for one, in your patch should be no-ops for valid tuning
requests.  The one change that isn't is a deviation from the recommended
settings which should only make a difference if a crystal oscillator in
your tuner is out of spec (the buf[7] = 0x06 instead of 0x05 slightly
adjusts a divisor in the chip).  If you need this change to get a lock,
you should probably get your board replaced.

Regards
Chris
-- 
Christopher Pascoe
IT Infrastructure Manager
School of Information Technology and Electrical Engineering
The University of Queensland   Brisbane  QLD  4072  Australia
Index: mt352.c
===================================================================
RCS file: /cvs/linuxtv/dvb-kernel/linux/drivers/media/dvb/frontends/mt352.c,v
retrieving revision 1.6
diff -u -p -r1.6 mt352.c
--- mt352.c	27 Jun 2004 21:02:30 -0000	1.6
+++ mt352.c	28 Jun 2004 23:20:49 -0000
@@ -83,7 +83,7 @@ int mt352_detect_avermedia_771(struct dv
 	{
 		{
 			.addr = 0x50,
-			.flags = I2C_M_NOSTART,
+			.flags = 0,
 			.buf = &reg,
 			.len = 1
 		},
@@ -165,11 +165,14 @@ int mt352_set_parameters (struct dvb_i2c
 {
 	unsigned char buf[14];
 	unsigned int tps = 0;
+	unsigned char acqctl = 0x0B;	/* suggest spectral inversion on, force mode, guard */
 	struct dvb_ofdm_parameters *op = &param->u.ofdm;
 	u32 freq = param->frequency / 1000;
 	uint16_t tmp;
 
 	switch (op->code_rate_HP) {
+		case FEC_1_2:
+			break;
 		case FEC_2_3:
 			tps = (1 << 7);
 			break;
@@ -182,14 +185,17 @@ int mt352_set_parameters (struct dvb_i2c
 		case FEC_7_8:
 			tps = (4 << 7);
 			break;
-		case FEC_1_2:
 		case FEC_AUTO:
+			acqctl |= (15 << 4);	/* all parameters auto */
 			break;
 		default:
 			return -EINVAL;
 	}
 
 	switch (op->code_rate_LP) {
+		case FEC_NONE:			/* "none" LP FEC not supported, so ignore */
+		case FEC_1_2:
+			break;
 		case FEC_2_3:
 			tps |= (1 << 4);
 			break;
@@ -202,8 +208,8 @@ int mt352_set_parameters (struct dvb_i2c
 		case FEC_7_8:
 			tps |= (4 << 4);
 			break;
-		case FEC_1_2:
 		case FEC_AUTO:
+			acqctl |= (15 << 4);	/* all parameters auto */
 			break;
 		default:
 			return -EINVAL;
@@ -212,31 +218,34 @@ int mt352_set_parameters (struct dvb_i2c
 	switch (op->constellation) {
 		case QPSK:
 			break;
-		case QAM_AUTO:
 		case QAM_16:
 			tps |= (1 << 13);
 			break;
 		case QAM_64:
 			tps |= (2 << 13);
 			break;
+		case QAM_AUTO:
+			acqctl |= (15 << 4);	/* all parameters auto */
+			break;
 		default:
 			return -EINVAL;
 	}
 
 	switch (op->transmission_mode) {
 		case TRANSMISSION_MODE_2K:
-		case TRANSMISSION_MODE_AUTO:
 			break;
 		case TRANSMISSION_MODE_8K:
 			tps |= (1 << 0);
 			break;
+		case TRANSMISSION_MODE_AUTO:
+			acqctl &= ~(1 << 0);	/* unforce mode */
+			break;
 		default:
 			return -EINVAL;
 	}
 
 	switch (op->guard_interval) {
 		case GUARD_INTERVAL_1_32:
-		case GUARD_INTERVAL_AUTO:
 			break;
 		case GUARD_INTERVAL_1_16:
 			tps |= (1 << 2);
@@ -247,12 +256,14 @@ int mt352_set_parameters (struct dvb_i2c
 		case GUARD_INTERVAL_1_4:
 			tps |= (3 << 2);
 			break;
+		case GUARD_INTERVAL_AUTO:
+			acqctl &= ~(1 << 1);	/* unforce guard */
+			break;
 		default:
 			return -EINVAL;
 	}
 
 	switch (op->hierarchy_information) {
-		case HIERARCHY_AUTO:
 		case HIERARCHY_NONE:
 			break;
 		case HIERARCHY_1:
@@ -264,6 +275,9 @@ int mt352_set_parameters (struct dvb_i2c
 		case HIERARCHY_4:
 			tps |= (3 << 10);
 			break;
+		case HIERARCHY_AUTO:
+			acqctl |= (15 << 4);	/* all parameters auto */
+			break;
 		default:
 			return -EINVAL;
 	}
@@ -274,7 +288,7 @@ int mt352_set_parameters (struct dvb_i2c
 	buf[1] = msb(tps);      /* TPS_GIVEN_(1|0) */
 	buf[2] = lsb(tps);
 
-	buf[3] = 0x50;
+	buf[3] = acqctl;        /* ACQ_CTL */
 
 	/**
 	 *  these settings assume 20.48MHz f_ADC, for other tuners you might
@@ -386,7 +400,7 @@ static u8 mt352_read_register (struct dv
 	u8 b0 [] = { reg };
 	u8 b1 [] = { 0 };
 	struct i2c_msg msg [] = { { .addr = I2C_MT352_ADDR,
-				    .flags = I2C_M_NOSTART,
+				    .flags = 0,
 				    .buf = b0, .len = 1 },
 				  { .addr = I2C_MT352_ADDR,
 				    .flags = I2C_M_RD,

Home | Main Index | Thread Index