[linux-dvb] [PATCH] Garbage diseqc messages emitted by bt8xx/dst module

Yeasah Pell yeasah at schwide.com
Fri Jun 9 23:45:37 CEST 2006


Gnome42 Gnome42 wrote:

>> Here's the latest patch I've got. List of changes:
>>
>> * 5-byte diseqc for supporting cards (USALS support)
>> * Fixes garbage commands caused by tone/power and tuner command buffer
>> conflict.
>> * Mini-DiSEqC commands corrected based on observations with a scope.
>> * More reliable about changing desired LNB voltage (though it still
>> won't switch it until tuning)
>> * Error reporting is much more thorough -- any failed commands should
>> now return a failure code for the ioctl. If you see applications
>> erroring out with failed ioctls, this is why. But it's much better this
>> way -- the command would have failed either way, but now the application
>> is told about it, and can retry, etc. -- or even better, we can actually
>> fix any problems that are revealed.
>> * Doesn't update internal state on unsuccessful commands to avoid cached
>> state diverging from real state.
>> * Avoids resending redundant tone/power commands.
>
>
> It applied cleanly but it didn't work, maybe it never turned the power 
> on?
>
> I took a guess and made these changes, now it is working again...
>
> --- dst.c.yeasah_jun9   2006-06-09 17:03:33.000000000 -0400
> +++ dst.c       2006-06-09 17:03:56.000000000 -0400
> @@ -1243,11 +1243,11 @@
>        case SEC_VOLTAGE_18:
>        case SEC_VOLTAGE_OFF:
>                dst_set_polarization(state);
> +               state->voltage = voltage;
>                retval = dst_tone_power_cmd(state);
> -               if(retval == 0)
> +               if (retval == 0)
>                {
> -                       state->voltage = voltage;
> -                       if(voltage == SEC_VOLTAGE_OFF)
> +                       if (voltage != SEC_VOLTAGE_OFF)
>                                state->diseq_flags |= HAS_POWER;
>                        else
>                                state->diseq_flags &= ~(HAS_POWER |
> HAS_LOCK | ATTEMPT_TUNE);

Oops, you're right, that's not going to work. Sorry, I didn't test that 
last minute state change. The change you made to get it to work is as at 
least as good as it was before, but it still has the problem where it 
the state can get wedged if the tone/power command should fail for any 
reason. I'm betting you don't have a universal LNB or you would be 
complaining that tone control isn't working either. :-)

Actually, it occurred to me that there's not really any way to know 
whether a failed command actually worked or not -- it just means there 
was some kind of communcations failure. So instead of not changing the 
parameter in the case of an error, it should just reset it to an invalid 
value.

Try this patch. I even tested it this time. :-)

-------------- next part --------------
diff -r b6b2d7591d38 linux/drivers/media/dvb/bt8xx/dst.c
--- a/linux/drivers/media/dvb/bt8xx/dst.c	Thu Jun 08 18:00:33 2006 -0300
+++ b/linux/drivers/media/dvb/bt8xx/dst.c	Fri Jun 09 17:35:45 2006 -0400
@@ -374,7 +374,7 @@ static int dst_set_bandwidth(struct dst_
 	state->bandwidth = bandwidth;
 
 	if (state->dst_type != DST_TYPE_IS_TERR)
-		return 0;
+		return -EOPNOTSUPP;
 
 	switch (bandwidth) {
 	case BANDWIDTH_6_MHZ:
@@ -441,10 +441,10 @@ static int dst_set_symbolrate(struct dst
 	u32 symcalc;
 	u64 sval;
 
+	if (state->dst_type == DST_TYPE_IS_TERR)
+		return -EOPNOTSUPP;
+
 	state->symbol_rate = srate;
-	if (state->dst_type == DST_TYPE_IS_TERR) {
-		return 0;
-	}
 	dprintk(verbose, DST_INFO, 1, "set symrate %u", srate);
 	srate /= 1000;
 	if (state->type_flags & DST_TYPE_HAS_SYMDIV) {
@@ -473,7 +473,7 @@ static int dst_set_modulation(struct dst
 static int dst_set_modulation(struct dst_state *state, fe_modulation_t modulation)
 {
 	if (state->dst_type != DST_TYPE_IS_CABLE)
-		return 0;
+		return -EOPNOTSUPP;
 
 	state->modulation = modulation;
 	switch (modulation) {
@@ -968,7 +968,7 @@ int dst_command(struct dst_state *state,
 		goto error;
 	}
 	if (write_dst(state, data, len)) {
-		dprintk(verbose, DST_INFO, 1, "Tring to recover.. ");
+		dprintk(verbose, DST_INFO, 1, "Trying to recover.. ");
 		if ((dst_error_recovery(state)) < 0) {
 			dprintk(verbose, DST_ERROR, 1, "Recovery Failed.");
 			goto error;
@@ -1058,15 +1058,42 @@ static int dst_tone_power_cmd(struct dst
 {
 	u8 paket[8] = { 0x00, 0x09, 0xff, 0xff, 0x01, 0x00, 0x00, 0x00 };
 
-	if (state->dst_type == DST_TYPE_IS_TERR)
-		return 0;
-	paket[4] = state->tx_tuna[4];
-	paket[2] = state->tx_tuna[2];
-	paket[3] = state->tx_tuna[3];
+	if (state->dst_type != DST_TYPE_IS_SAT)
+		return -EOPNOTSUPP;
+
+	switch (state->tone) {
+	case SEC_TONE_OFF:
+		if (state->type_flags & DST_TYPE_HAS_OBS_REGS)
+			paket[2] = 0x00;
+		else
+			paket[2] = 0xff;
+		break;
+	case SEC_TONE_ON:
+		paket[2] = 0x02;
+		break;
+	}
+
+	switch (state->minicmd) {
+	case SEC_MINI_A:
+		paket[3] = 0x00;
+		break;
+	case SEC_MINI_B:
+		paket[3] = 0x01;
+		break;
+	}
+	state->minicmd = -1;
+
+	switch (state->voltage) {
+	case SEC_VOLTAGE_OFF:
+		paket[4] = 0x00;
+		break;
+	default:
+		paket[4] = 0x01;
+		break;
+	}
+
 	paket[7] = dst_check_sum (paket, 7);
-	dst_command(state, paket, 8);
-
-	return 0;
+	return dst_command(state, paket, 8);
 }
 
 static int dst_get_tuna(struct dst_state *state)
@@ -1188,94 +1215,91 @@ static int dst_set_diseqc(struct dvb_fro
 	u8 paket[8] = { 0x00, 0x08, 0x04, 0xe0, 0x10, 0x38, 0xf0, 0xec };
 
 	if (state->dst_type != DST_TYPE_IS_SAT)
+		return -EOPNOTSUPP;
+
+	if (cmd->msg_len > 0 && cmd->msg_len < 5)
+		memcpy(&paket[3], cmd->msg, cmd->msg_len);
+	else if (cmd->msg_len == 5 && state->dst_hw_cap & DST_TYPE_HAS_DISEQC5)
+		memcpy(&paket[2], cmd->msg, cmd->msg_len);
+	else
+		return -EINVAL;
+
+	paket[7] = dst_check_sum(&paket[0], 7);
+	return dst_command(state, paket, 8);
+}
+
+static int dst_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage)
+{
+	struct dst_state *state = fe->demodulator_priv;
+	int retval;
+
+	if (state->dst_type != DST_TYPE_IS_SAT)
+		return -EOPNOTSUPP;
+        if (voltage == state->voltage)
 		return 0;
-	if (cmd->msg_len == 0 || cmd->msg_len > 4)
-		return -EINVAL;
-	memcpy(&paket[3], cmd->msg, cmd->msg_len);
-	paket[7] = dst_check_sum(&paket[0], 7);
-	dst_command(state, paket, 8);
-	return 0;
-}
-
-static int dst_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage)
-{
-	int need_cmd;
-	struct dst_state *state = fe->demodulator_priv;
-
-	state->voltage = voltage;
-	if (state->dst_type != DST_TYPE_IS_SAT)
-		return 0;
-
-	need_cmd = 0;
 
 	switch (voltage) {
 	case SEC_VOLTAGE_13:
 	case SEC_VOLTAGE_18:
-		if ((state->diseq_flags & HAS_POWER) == 0)
-			need_cmd = 1;
 		state->diseq_flags |= HAS_POWER;
-		state->tx_tuna[4] = 0x01;
 		break;
 	case SEC_VOLTAGE_OFF:
-		need_cmd = 1;
 		state->diseq_flags &= ~(HAS_POWER | HAS_LOCK | ATTEMPT_TUNE);
-		state->tx_tuna[4] = 0x00;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (need_cmd)
-		dst_tone_power_cmd(state);
-
-	return 0;
+	state->voltage = voltage;
+	dst_set_polarization(state);
+	retval = dst_tone_power_cmd(state);
+	if (retval != 0)
+		state->voltage = -1;
+	return retval;
 }
 
 static int dst_set_tone(struct dvb_frontend *fe, fe_sec_tone_mode_t tone)
 {
 	struct dst_state *state = fe->demodulator_priv;
-
-	state->tone = tone;
+	int retval;
+
 	if (state->dst_type != DST_TYPE_IS_SAT)
+		return -EOPNOTSUPP;
+        if (tone == state->tone)
 		return 0;
 
 	switch (tone) {
 	case SEC_TONE_OFF:
-		if (state->type_flags & DST_TYPE_HAS_OBS_REGS)
-		    state->tx_tuna[2] = 0x00;
-		else
-		    state->tx_tuna[2] = 0xff;
-		break;
-
 	case SEC_TONE_ON:
-		state->tx_tuna[2] = 0x02;
+		state->tone = tone;
+		retval = dst_tone_power_cmd(state);
+		if (retval == 0)
+			state->tone = -1;
+		break;
+	default:
+		retval = -EINVAL;
+		break;
+	}
+
+	return retval;
+}
+
+static int dst_send_burst(struct dvb_frontend *fe, fe_sec_mini_cmd_t minicmd)
+{
+	struct dst_state *state = fe->demodulator_priv;
+
+	if (state->dst_type != DST_TYPE_IS_SAT)
+		return -EOPNOTSUPP;
+
+	switch (minicmd) {
+	case SEC_MINI_A:
+	case SEC_MINI_B:
+		state->minicmd = minicmd;
+		return dst_tone_power_cmd(state);
 		break;
 	default:
 		return -EINVAL;
 	}
-	dst_tone_power_cmd(state);
-
-	return 0;
-}
-
-static int dst_send_burst(struct dvb_frontend *fe, fe_sec_mini_cmd_t minicmd)
-{
-	struct dst_state *state = fe->demodulator_priv;
-
-	if (state->dst_type != DST_TYPE_IS_SAT)
-		return 0;
-	state->minicmd = minicmd;
-	switch (minicmd) {
-	case SEC_MINI_A:
-		state->tx_tuna[3] = 0x02;
-		break;
-	case SEC_MINI_B:
-		state->tx_tuna[3] = 0xff;
-		break;
-	}
-	dst_tone_power_cmd(state);
-
-	return 0;
 }
 
 
@@ -1291,8 +1315,9 @@ static int dst_init(struct dvb_frontend 
 	static u8 cab_tuna_188[] = { 0x09, 0x00, 0x03, 0xb6, 0x01, 0x07, 0x00, 0x00, 0x00, 0x00 };
 
 	state->inversion = INVERSION_OFF;
-	state->voltage = SEC_VOLTAGE_13;
-	state->tone = SEC_TONE_OFF;
+	state->voltage = -1;
+	state->tone = -1;
+	state->minicmd = -1;
 	state->diseq_flags = 0;
 	state->k22 = 0x02;
 	state->bandwidth = BANDWIDTH_7_MHZ;
@@ -1325,20 +1350,20 @@ static int dst_read_signal_strength(stru
 {
 	struct dst_state *state = fe->demodulator_priv;
 
-	dst_get_signal(state);
+	int retval = dst_get_signal(state);
 	*strength = state->decode_strength;
 
-	return 0;
+	return retval;
 }
 
 static int dst_read_snr(struct dvb_frontend *fe, u16 *snr)
 {
 	struct dst_state *state = fe->demodulator_priv;
 
-	dst_get_signal(state);
+	int retval = dst_get_signal(state);
 	*snr = state->decode_snr;
 
-	return 0;
+	return retval;
 }
 
 static int dst_set_frontend(struct dvb_frontend* fe,
@@ -1347,10 +1372,12 @@ static int dst_set_frontend(struct dvb_f
 			    int *delay,
 			    fe_status_t *status)
 {
+	int retval = -EINVAL;
 	struct dst_state *state = fe->demodulator_priv;
 
 	if (p != NULL) {
-		dst_set_freq(state, p->frequency);
+		if ((retval = dst_set_freq(state, p->frequency)) != 0)
+			return retval;
 		dprintk(verbose, DST_DEBUG, 1, "Set Frequency=[%d]", p->frequency);
 
 		if (state->dst_type == DST_TYPE_IS_SAT) {
@@ -1368,14 +1395,15 @@ static int dst_set_frontend(struct dvb_f
 			dst_set_symbolrate(state, p->u.qam.symbol_rate);
 			dst_set_modulation(state, p->u.qam.modulation);
 		}
-		dst_write_tuna(fe);
+		if (retval == 0)
+			retval = dst_write_tuna(fe);
 	}
 
 	if (!(mode_flags & FE_TUNE_MODE_ONESHOT))
 		dst_read_status(fe, status);
 
 	*delay = HZ/10;
-	return 0;
+	return retval;
 }
 
 static int dst_get_frontend(struct dvb_frontend *fe, struct dvb_frontend_parameters *p)


More information about the linux-dvb mailing list