[linux-dvb] [PATCH] Two tiny patches plus or51132 firmware loading change

Trent Piepho xyzzy at speakeasy.org
Tue Apr 4 00:12:21 CEST 2006


The first patch is a tiny spelling fix.
The second is small bit of cruft code removed from or51132 driver.

The third actually changes somthing.  The or51132 uses the same firmware for all
the QAM modulations it supports, QAM_64, QAM_256, and QAM_AUTO.  The driver
re-loads the firmware when changing between these modulations, which isn't
necessary.  Loading the firmware takes about 12 seconds and locks up the system
while it happens, corrupting any ongoing audio or video captures on other cards.
So, it is nice to avoid it when possible.  Which is what this patch does,
getting rid of some duplicated code in the process.

There is also a mishandled error condition in the same code.  It used to work
like this:
1. Load firmware file into kernel
2. Set clock mode in driver state
3. Upload firmware to demodulator  (*)
4. Set new modulation in driver state

Steps 1 and 3 can fail, in which case the modulation change is aborted and the
old modulation (as far as the driver knows) remains in effect.  The problem is
when step 3 fails.  The clock mode is changed, but the modulation isn't.  This
leaves the driver in an incorrect half-QAM/half-VSB setup.

I just exchanged steps 2 and 3.  Step 2 doesn't change the hardware, it just
sets some state in the driver which takes effect later when a DMA transfer is
started.

I've tested this with QAM-256 and everything works as it did before.  I don't
have access to QAM-64 to test that.  ATSC QAM-64 is a rare beast now, and I
doubt anyone has access to an or51132 and QAM-64.
-------------- next part --------------
# HG changeset patch
# User Trent Piepho <xyzzy at speakeasy.org>
# Node ID 4ef75c2f0d8c872e0efbb2650923bc628b850daa
# Parent  103dceb47d97714ce3dcf271a62b67efcd3eaba0
Fix spelling

From: Trent Piepho <xyzzy at speakeasy.org>

It's "Terrestrial"

Signed-off-by: Trent Piepho <xyzzy at speakeasy.org>

diff -r 103dceb47d97 -r 4ef75c2f0d8c linux/drivers/media/dvb/frontends/Kconfig
--- a/linux/drivers/media/dvb/frontends/Kconfig	Fri Mar 31 14:44:33 2006 -0800
+++ b/linux/drivers/media/dvb/frontends/Kconfig	Mon Apr  3 14:10:54 2006 -0700
@@ -157,7 +157,7 @@ config DVB_STV0297
 	help
 	  A DVB-C tuner module. Say Y when you want to support this frontend.
 
-comment "ATSC (North American/Korean Terresterial DTV) frontends"
+comment "ATSC (North American/Korean Terrestrial/Cable DTV) frontends"
 	depends on DVB_CORE
 
 config DVB_NXT200X
-------------- next part --------------
# HG changeset patch
# User Trent Piepho <xyzzy at speakeasy.org>
# Node ID c682dce90b13edbf9e429db01a39142d382c4e04
# Parent  baa25d300d8ece215f9d52a6b85b630d3d5d02cf
Remove a wee bit of cruft

From: Trent Piepho <xyzzy at speakeasy.org>

A few lines that do nothing in the or51132 frontend, removed.

Signed-off-by: Trent Piepho <xyzzy at speakeasy.org>

diff -r baa25d300d8e -r c682dce90b13 linux/drivers/media/dvb/frontends/or51132.c
--- a/linux/drivers/media/dvb/frontends/or51132.c	Mon Apr  3 14:30:01 2006 -0700
+++ b/linux/drivers/media/dvb/frontends/or51132.c	Mon Apr  3 14:32:59 2006 -0700
@@ -240,7 +240,7 @@ static int or51132_setmode(struct dvb_fr
 static int or51132_setmode(struct dvb_frontend* fe)
 {
 	struct or51132_state* state = fe->demodulator_priv;
-	unsigned char cmd_buf[4];
+	unsigned char cmd_buf[3];
 
 	dprintk("setmode %d\n",(int)state->current_modulation);
 	/* set operation mode in Receiver 1 register; */
@@ -260,7 +260,6 @@ static int or51132_setmode(struct dvb_fr
 	default:
 		printk("setmode:Modulation set to unsupported value\n");
 	};
-	cmd_buf[3] = 0x00;
 	if (i2c_writebytes(state,state->config->demod_address,
 			   cmd_buf,3)) {
 		printk(KERN_WARNING "or51132: set_mode error 1\n");
@@ -298,7 +297,6 @@ static int or51132_setmode(struct dvb_fr
 	default:
 		printk("setmode: Modulation set to unsupported value\n");
 	};
-	cmd_buf[3] = 0x00;
 	msleep(20); /* 20ms */
 	if (i2c_writebytes(state,state->config->demod_address,
 			   cmd_buf,3)) {
-------------- next part --------------
# HG changeset patch
# User Trent Piepho <xyzzy at speakeasy.org>
# Node ID baa25d300d8ece215f9d52a6b85b630d3d5d02cf
# Parent  4ef75c2f0d8c872e0efbb2650923bc628b850daa
Avoid unnecessary firmware re-loads in or51132 frontend

From: Trent Piepho <xyzzy at speakeasy.org>

As QAM_64, QAM_256, and QAM_AUTO all use the same firmware, switching
between these modulations doesn't require a firmware re-load.  This also 
fixes a mishandled error condition, in which the firmware file is loaded
into the kernel, the clock mode is changed, but then the firmware upload
to the device fails.  The modulation change is aborted, but the clock
mode would still be changed.

Signed-off-by: Trent Piepho <xyzzy at speakeasy.org>

diff -r 4ef75c2f0d8c -r baa25d300d8e linux/drivers/media/dvb/frontends/or51132.c
--- a/linux/drivers/media/dvb/frontends/or51132.c	Mon Apr  3 14:10:54 2006 -0700
+++ b/linux/drivers/media/dvb/frontends/or51132.c	Mon Apr  3 14:30:01 2006 -0700
@@ -310,6 +310,25 @@ static int or51132_setmode(struct dvb_fr
 	return 0;
 }
 
+/* Some modulations use the same firmware.  This classifies modulations
+   by the firmware they use. */
+#define MOD_FWCLASS_UNKNOWN	0
+#define MOD_FWCLASS_VSB		1
+#define MOD_FWCLASS_QAM		2
+static int modulation_fw_class(fe_modulation_t modulation)
+{
+	switch(modulation) {
+	case VSB_8:
+		return MOD_FWCLASS_VSB;
+	case QAM_AUTO:
+	case QAM_64:
+	case QAM_256:
+		return MOD_FWCLASS_QAM;
+	default:
+		return MOD_FWCLASS_UNKNOWN;
+	}
+}
+
 static int or51132_set_parameters(struct dvb_frontend* fe,
 				  struct dvb_frontend_parameters *param)
 {
@@ -317,45 +336,40 @@ static int or51132_set_parameters(struct
 	u8 buf[4];
 	struct or51132_state* state = fe->demodulator_priv;
 	const struct firmware *fw;
-
-	/* Change only if we are actually changing the modulation */
-	if (state->current_modulation != param->u.vsb.modulation) {
-		switch(param->u.vsb.modulation) {
-		case VSB_8:
+	const char *fwname;
+	int clock_mode;
+
+	/* Upload new firmware only if we need a different one */
+	if (modulation_fw_class(state->current_modulation) !=
+	    modulation_fw_class(param->u.vsb.modulation)) {
+		switch(modulation_fw_class(param->u.vsb.modulation)) {
+		case MOD_FWCLASS_VSB:
 			dprintk("set_parameters VSB MODE\n");
-			printk("or51132: Waiting for firmware upload(%s)...\n",
-			       OR51132_VSB_FIRMWARE);
-			ret = request_firmware(&fw, OR51132_VSB_FIRMWARE,
-					       &state->i2c->dev);
-			if (ret){
-				printk(KERN_WARNING "or51132: No firmware up"
-				       "loaded(timeout or file not found?)\n");
-				return ret;
-			}
+			fwname = OR51132_VSB_FIRMWARE;
+
 			/* Set non-punctured clock for VSB */
-			state->config->set_ts_params(fe, 0);
+			clock_mode = 0;
 			break;
-		case QAM_AUTO:
-		case QAM_64:
-		case QAM_256:
+		case MOD_FWCLASS_QAM:
 			dprintk("set_parameters QAM MODE\n");
-			printk("or51132: Waiting for firmware upload(%s)...\n",
-			       OR51132_QAM_FIRMWARE);
-			ret = request_firmware(&fw, OR51132_QAM_FIRMWARE,
-					       &state->i2c->dev);
-			if (ret){
-				printk(KERN_WARNING "or51132: No firmware up"
-				       "loaded(timeout or file not found?)\n");
-				return ret;
-			}
+			fwname = OR51132_QAM_FIRMWARE;
+
 			/* Set punctured clock for QAM */
-			state->config->set_ts_params(fe, 1);
+			clock_mode = 1;
 			break;
 		default:
-			printk("or51132:Modulation type(%d) UNSUPPORTED\n",
+			printk("or51132: Modulation type(%d) UNSUPPORTED\n",
 			       param->u.vsb.modulation);
 			return -1;
-		};
+		}
+		printk("or51132: Waiting for firmware upload(%s)...\n",
+		       fwname);
+		ret = request_firmware(&fw, fwname, &state->i2c->dev);
+		if (ret) {
+			printk(KERN_WARNING "or51132: No firmware up"
+			       "loaded(timeout or file not found?)\n");
+			return ret;
+		}
 		ret = or51132_load_firmware(fe, fw);
 		release_firmware(fw);
 		if (ret) {
@@ -364,7 +378,10 @@ static int or51132_set_parameters(struct
 			return ret;
 		}
 		printk("or51132: Firmware upload complete.\n");
-
+		state->config->set_ts_params(fe, clock_mode);
+	}
+	/* Change only if we are actually changing the modulation */
+	if (state->current_modulation != param->u.vsb.modulation) {
 		state->current_modulation = param->u.vsb.modulation;
 		or51132_setmode(fe);
 	}
@@ -373,7 +390,7 @@ static int or51132_set_parameters(struct
 			  param->frequency, 0);
 	dprintk("set_parameters tuner bytes: 0x%02x 0x%02x "
 		"0x%02x 0x%02x\n",buf[0],buf[1],buf[2],buf[3]);
-	if (i2c_writebytes(state, state->config->pll_address ,buf, 4))
+	if (i2c_writebytes(state, state->config->pll_address, buf, 4))
 		printk(KERN_WARNING "or51132: set_parameters error "
 		       "writing to tuner\n");
 


More information about the linux-dvb mailing list