linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [media] drxk: Minor cleanup (basically logging related).
@ 2012-06-28 21:20 Martin Blumenstingl
  2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
  2012-06-28 21:20 ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
  0 siblings, 2 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-28 21:20 UTC (permalink / raw)
  To: linux-media

Hi,

this little patch series cleans up the dmesg when using the drxk driver.
The first patch fixes a warning regarding the wrong QAM demodulator
command (this depends on the drxk's firmware). These warnings were
spamming my dmesg.
The second patch just fixes some partially incorrect dprintk's/printk's.

Regards,
Martin


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable.
  2012-06-28 21:20 [media] drxk: Minor cleanup (basically logging related) Martin Blumenstingl
@ 2012-06-28 21:20 ` Martin Blumenstingl
  2012-06-29  8:31   ` Ralph Metzler
                     ` (2 more replies)
  2012-06-28 21:20 ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
  1 sibling, 3 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-28 21:20 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

Currently there are two different commands: the old command which takes
4 parameters, and a newer one with just takes 2 parameters.
The driver shows an error in dmesg When using the old command with a new
firmware. Unfortunately this happens every time when chaning the
frequency (on DVB-C).

This patch simply makes configurable, whether the old or the new command
will be used.
All existing drxk_config instances using the "drxk_a3.mc" are updated to
make sure that these won't break (as this is the only firmware that uses
the old command).

I also removed a few lines of duplicated code.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/ddbridge/ddbridge-core.c |  1 +
 drivers/media/dvb/frontends/drxk.h         |  6 +++-
 drivers/media/dvb/frontends/drxk_hard.c    | 44 ++++++++++++++++++------------
 drivers/media/dvb/frontends/drxk_hard.h    |  3 +-
 drivers/media/dvb/ngene/ngene-cards.c      |  1 +
 5 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c
index 131b938..80985bb 100644
--- a/drivers/media/dvb/ddbridge/ddbridge-core.c
+++ b/drivers/media/dvb/ddbridge/ddbridge-core.c
@@ -578,6 +578,7 @@ static int demod_attach_drxk(struct ddb_input *input)
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.old_qam_demod_cmd = true;
 	config.adr = 0x29 + (input->nr & 1);
 
 	fe = input->fe = dvb_attach(drxk_attach, &config, i2c);
diff --git a/drivers/media/dvb/frontends/drxk.h b/drivers/media/dvb/frontends/drxk.h
index 9d64e4f..c718530 100644
--- a/drivers/media/dvb/frontends/drxk.h
+++ b/drivers/media/dvb/frontends/drxk.h
@@ -20,6 +20,9 @@
  *			means that 1=DVBC, 0 = DVBT. Zero means the opposite.
  * @mpeg_out_clk_strength: DRXK Mpeg output clock drive strength.
  * @microcode_name:	Name of the firmware file with the microcode
+ * @old_qam_demod_cmd:	True if the firmware uses the old (4-parameter)
+ * 			version of the command to set the demodulator params.
+ * 			Only the initial firmware (drxk_a3.mc) use this.
  *
  * On the *_gpio vars, bit 0 is UIO-1, bit 1 is UIO-2 and bit 2 is
  * UIO-3.
@@ -38,7 +41,8 @@ struct drxk_config {
 	u8	mpeg_out_clk_strength;
 	int	chunk_size;
 
-	const char *microcode_name;
+	const char	*microcode_name;
+	bool		old_qam_demod_cmd;
 };
 
 #if defined(CONFIG_DVB_DRXK) || (defined(CONFIG_DVB_DRXK_MODULE) \
diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 60b868f..9b4d28c 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -5445,34 +5445,43 @@ static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
 	}
 	if (status < 0)
 		goto error;
+
 	setParamParameters[0] = state->m_Constellation;	/* modulation     */
 	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
-	if (state->m_OperationMode == OM_QAM_ITU_C)
-		setParamParameters[2] = QAM_TOP_ANNEX_C;
-	else
-		setParamParameters[2] = QAM_TOP_ANNEX_A;
-	setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
-	/* Env parameters */
-	/* check for LOCKRANGE Extented */
-	/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
 
-	status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 4, setParamParameters, 1, &cmdResult);
-	if (status < 0) {
-		/* Fall-back to the simpler call */
+	if (state->old_qam_demod_cmd) {
 		if (state->m_OperationMode == OM_QAM_ITU_C)
-			setParamParameters[0] = QAM_TOP_ANNEX_C;
+			setParamParameters[2] = QAM_TOP_ANNEX_C;
 		else
-			setParamParameters[0] = QAM_TOP_ANNEX_A;
-		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setParamParameters, 1, &cmdResult);
+			setParamParameters[2] = QAM_TOP_ANNEX_A;
+		setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
+		/* Env parameters */
+		/* check for LOCKRANGE Extented */
+		/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
+
+		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 4, setParamParameters, 1, &cmdResult);
+	} else {
+		u16 setEnvParameters[1];
+
+		if (state->m_OperationMode == OM_QAM_ITU_C)
+			setEnvParameters[0] = QAM_TOP_ANNEX_C;
+		else
+			setEnvParameters[0] = QAM_TOP_ANNEX_A;
+
+		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult);
 		if (status < 0)
 			goto error;
 
-		setParamParameters[0] = state->m_Constellation; /* modulation     */
-		setParamParameters[1] = DRXK_QAM_I12_J17;       /* interleave mode   */
 		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 2, setParamParameters, 1, &cmdResult);
 	}
-	if (status < 0)
+
+	if (status < 0) {
+		dprintk(1, "Could not set demodulator parameters. Make "
+			"sure old_qam_demod_cmd (%d) is correct for your "
+			"firmware (%s).\n",
+			state->old_qam_demod_cmd, state->microcode_name);
 		goto error;
+	}
 
 	/*
 	 * STEP 3: enable the system in a mode where the ADC provides valid
@@ -6385,6 +6394,7 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 	state->demod_address = adr;
 	state->single_master = config->single_master;
 	state->microcode_name = config->microcode_name;
+	state->old_qam_demod_cmd = config->old_qam_demod_cmd;
 	state->no_i2c_bridge = config->no_i2c_bridge;
 	state->antenna_gpio = config->antenna_gpio;
 	state->antenna_dvbt = config->antenna_dvbt;
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index 4bbf841..9c84197 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -338,7 +338,8 @@ struct drxk_state {
 	bool	antenna_dvbt;
 	u16	antenna_gpio;
 
-	const char *microcode_name;
+	const char    *microcode_name;
+	bool           old_qam_demod_cmd;
 };
 
 #define NEVER_LOCK 0
diff --git a/drivers/media/dvb/ngene/ngene-cards.c b/drivers/media/dvb/ngene/ngene-cards.c
index 7539a5d..ad45570 100644
--- a/drivers/media/dvb/ngene/ngene-cards.c
+++ b/drivers/media/dvb/ngene/ngene-cards.c
@@ -217,6 +217,7 @@ static int demod_attach_drxk(struct ngene_channel *chan,
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.old_qam_demod_cmd = true;
 	config.adr = 0x29 + (chan->number ^ 2);
 
 	chan->fe = dvb_attach(drxk_attach, &config, i2c);
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] [media] drxk: Improve logging.
  2012-06-28 21:20 [media] drxk: Minor cleanup (basically logging related) Martin Blumenstingl
  2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
@ 2012-06-28 21:20 ` Martin Blumenstingl
  2012-06-30 22:42   ` [2/2,media] " Martin Blumenstingl
  2012-06-30 22:42   ` [PATCH 2/2] [media] " Martin Blumenstingl
  1 sibling, 2 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-28 21:20 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

This patch simply fixes some logging calls:
- Use KERN_INFO when printing the chip status.
- Add a missing space when logging the drxk_gate_ctrl call.
- Use the same logging text as always if the scu_command in GetQAMLockStatus fails.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/frontends/drxk_hard.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 9b4d28c..e62d39c 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -932,7 +932,7 @@ static int GetDeviceCapabilities(struct drxk_state *state)
 	if (status < 0)
 		goto error;
 
-printk(KERN_ERR "drxk: status = 0x%08x\n", sioTopJtagidLo);
+	printk(KERN_INFO "drxk: status = 0x%08x\n", sioTopJtagidLo);
 
 	/* driver 0.9.0 */
 	switch ((sioTopJtagidLo >> 29) & 0xF) {
@@ -5361,7 +5361,7 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
 			SCU_RAM_COMMAND_CMD_DEMOD_GET_LOCK, 0, NULL, 2,
 			Result);
 	if (status < 0)
-		printk(KERN_ERR "drxk: %s status = %08x\n", __func__, status);
+		printk(KERN_ERR "drxk: Error %d on %s\n", status, __func__);
 
 	if (Result[1] < SCU_RAM_QAM_LOCKED_LOCKED_DEMOD_LOCKED) {
 		/* 0x0000 NOT LOCKED */
@@ -6205,7 +6205,7 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 {
 	struct drxk_state *state = fe->demodulator_priv;
 
-	dprintk(1, "%s\n", enable ? "enable" : "disable");
+	dprintk(1, ": %s\n", enable ? "enable" : "disable");
 	return ConfigureI2CBridge(state, enable ? true : false);
 }
 
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable.
  2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
@ 2012-06-29  8:31   ` Ralph Metzler
  2012-06-29 15:58     ` Martin Blumenstingl
  2012-06-30 22:32   ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable Martin Blumenstingl
  2012-06-30 22:32   ` Martin Blumenstingl
  2 siblings, 1 reply; 16+ messages in thread
From: Ralph Metzler @ 2012-06-29  8:31 UTC (permalink / raw)
  To: Martin Blumenstingl; +Cc: linux-media

Martin Blumenstingl writes:
 > Currently there are two different commands: the old command which takes
 > 4 parameters, and a newer one with just takes 2 parameters.

Hi,

are you sure about this?
>From what I have been told, the 2 parameter command is in the 
firmware ROM and older loadable/patch firmwares.
Newer firmwares provided the 4 parameter command.

Regards,
Ralph


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable.
  2012-06-29  8:31   ` Ralph Metzler
@ 2012-06-29 15:58     ` Martin Blumenstingl
  2012-06-29 16:13       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-29 15:58 UTC (permalink / raw)
  To: Ralph Metzler; +Cc: linux-media

Hi Ralph,

> are you sure about this?
> From what I have been told, the 2 parameter command is in the
> firmware ROM and older loadable/patch firmwares.
> Newer firmwares provided the 4 parameter command.
The firmwares in the ROM are a good point.

I discussed with  Mauro Carvalho Chehab before I started writing my patch,
and he told me that the only (loadable) firmware that uses the old command
is the "drxk_a3.mc" one.
But you are right, there is some firmware (for DVB-C, afaik it's NOT for DVB-T)
stored in the ROM.

If  I find out that the ROM uses the "old" command then I'll probably try
making this smart:
old_qam_demod_cmd will be an int with the following possible values:
* -1: unknown - trial and error approach will be used
(afterwards this will be updated to either 0 or 1)
* 0: use the 2-parameter command
* 1: use the 4-parameter command

I'll also try to guess a smart default value:
-1 will be used if no firmware was given.
Otherwise 0 will be the default.
The remaining two drxk_config instances that are still using the old
firmware will be set to 1 (like in my first patch).

If everything goes right then I'll be able to test and update my patch tonight.

Regards,
Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable.
  2012-06-29 15:58     ` Martin Blumenstingl
@ 2012-06-29 16:13       ` Mauro Carvalho Chehab
  2012-06-29 16:32         ` Martin Blumenstingl
  0 siblings, 1 reply; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 16:13 UTC (permalink / raw)
  To: Martin Blumenstingl; +Cc: Ralph Metzler, linux-media

Em 29-06-2012 12:58, Martin Blumenstingl escreveu:
> Hi Ralph,
> 
>> are you sure about this?
>>  From what I have been told, the 2 parameter command is in the
>> firmware ROM and older loadable/patch firmwares.
>> Newer firmwares provided the 4 parameter command.
> The firmwares in the ROM are a good point.
> 
> I discussed with  Mauro Carvalho Chehab before I started writing my patch,
> and he told me that the only (loadable) firmware that uses the old command
> is the "drxk_a3.mc" one.
>
> But you are right, there is some firmware (for DVB-C, afaik it's NOT for DVB-T)
> stored in the ROM.
> 
> If  I find out that the ROM uses the "old" command then I'll probably try
> making this smart:
> old_qam_demod_cmd will be an int with the following possible values:
> * -1: unknown - trial and error approach will be used
> (afterwards this will be updated to either 0 or 1)
> * 0: use the 2-parameter command
> * 1: use the 4-parameter command
> 
> I'll also try to guess a smart default value:
> -1 will be used if no firmware was given.
> Otherwise 0 will be the default.
> The remaining two drxk_config instances that are still using the old
> firmware will be set to 1 (like in my first patch).

I didn't tell "old command", or at least not in the sense of old firmware. I told
that the first drivers (ddbridge and mantis), based on drxk_ac3.mc firmware, use the
4-parameters variant, while the other drivers use the 2-parameters variant.

Anyway, using the name "old" for such parameter is not a good idea. IMHO, you
should use something like qam_demod_needs_4_parameters for this config data,
or, maybe "number_of_qam_demod_parameters".

If number_of_qam_demod_parameters is not 2 or 4, try both ways. So, a device driver
that won't specify it will be auto-probed.

> 
> If everything goes right then I'll be able to test and update my patch tonight.
> 
> Regards,
> Martin
> --
> 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
> 

Regards,
Mauro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable.
  2012-06-29 16:13       ` Mauro Carvalho Chehab
@ 2012-06-29 16:32         ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-29 16:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Ralph Metzler, linux-media

Hi,

> I didn't tell "old command", or at least not in the sense of old firmware. I told
> that the first drivers (ddbridge and mantis), based on drxk_ac3.mc firmware, use the
> 4-parameters variant, while the other drivers use the 2-parameters variant.
Oh sorry, I must have gotten that wrong.

> Anyway, using the name "old" for such parameter is not a good idea. IMHO, you
> should use something like qam_demod_needs_4_parameters for this config data,
> or, maybe "number_of_qam_demod_parameters".
Sounds good.

> If number_of_qam_demod_parameters is not 2 or 4, try both ways. So, a device driver
> that won't specify it will be auto-probed.
Ok, I'll keep the existing order:
- first try the 4-parameter one
- then try the 2-parameter one
And I'll update the variable then to make sure we're not doing that
trial-and-error thing
twice.

I'll also update all drxk_config instances where I'm sure that they're
using the (newer)
2-parameter method.

Thanks you two for you feedback!

Regards,
Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable.
  2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
  2012-06-29  8:31   ` Ralph Metzler
@ 2012-06-30 22:32   ` Martin Blumenstingl
  2012-06-30 22:32   ` Martin Blumenstingl
  2 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-30 22:32 UTC (permalink / raw)
  To: linux-media

Hi,

this is an updated version of my patch.
Thanks to Mauro Carvalho Chehab and Ralph Metzler for the suggestions.

Regards,
Martin


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable.
  2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
  2012-06-29  8:31   ` Ralph Metzler
  2012-06-30 22:32   ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable Martin Blumenstingl
@ 2012-06-30 22:32   ` Martin Blumenstingl
  2012-07-01  2:46     ` Mauro Carvalho Chehab
  2012-07-04 21:36     ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable. [v3] Martin Blumenstingl
  2 siblings, 2 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-30 22:32 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

Currently there are two different implementations (in the firmware) for
the QAM demodulator command: one takes 4 and the other takes 2 parameters.
The driver shows an error in dmesg When using the 4-parameter command with
firmware that implements the 2-parameter command.
Unfortunately this happens every time when chaning the frequency (on DVB-C).

This patch simply makes configurable, how many command parameters will be
used.
All existing drxk_config instances using the "drxk_a3.mc" were updated
because this firmware is the only loadable firmware where the QAM
demodulator command takes 4 parameters. Some firmwares in the ROM
might also use it.
The drxk instances in the em28xx-dvb driver were also updated to
silence the warnings.

If no qam_demod_parameter_count is given in the drxk_config struct,
then the correct number of parameters will be auto-detected.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/ddbridge/ddbridge-core.c |   1 +
 drivers/media/dvb/frontends/drxk.h         |  11 ++-
 drivers/media/dvb/frontends/drxk_hard.c    | 105 ++++++++++++++++++++++-------
 drivers/media/dvb/frontends/drxk_hard.h    |   1 +
 drivers/media/dvb/ngene/ngene-cards.c      |   1 +
 drivers/media/video/em28xx/em28xx-dvb.c    |   4 ++
 6 files changed, 96 insertions(+), 27 deletions(-)

diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c
index 131b938..ebf3f05 100644
--- a/drivers/media/dvb/ddbridge/ddbridge-core.c
+++ b/drivers/media/dvb/ddbridge/ddbridge-core.c
@@ -578,6 +578,7 @@ static int demod_attach_drxk(struct ddb_input *input)
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.qam_demod_parameter_count = 4;
 	config.adr = 0x29 + (input->nr & 1);
 
 	fe = input->fe = dvb_attach(drxk_attach, &config, i2c);
diff --git a/drivers/media/dvb/frontends/drxk.h b/drivers/media/dvb/frontends/drxk.h
index 9d64e4f..19a8114 100644
--- a/drivers/media/dvb/frontends/drxk.h
+++ b/drivers/media/dvb/frontends/drxk.h
@@ -20,6 +20,14 @@
  *			means that 1=DVBC, 0 = DVBT. Zero means the opposite.
  * @mpeg_out_clk_strength: DRXK Mpeg output clock drive strength.
  * @microcode_name:	Name of the firmware file with the microcode
+ * @qam_demod_parameter_count:	The number of parameters used for the command
+ * 				to set the demodulator parameters. All
+ * 				firmwares are using the 2-parameter commmand.
+ * 				An exception is the "drxk_a3.mc" firmware,
+ * 				which uses the 4-parameter command.
+ * 				A value of 0 (default) or lower indicates that
+ * 				the correct number of parameters will be
+ * 				automatically detected.
  *
  * On the *_gpio vars, bit 0 is UIO-1, bit 1 is UIO-2 and bit 2 is
  * UIO-3.
@@ -38,7 +46,8 @@ struct drxk_config {
 	u8	mpeg_out_clk_strength;
 	int	chunk_size;
 
-	const char *microcode_name;
+	const char	*microcode_name;
+	int		 qam_demod_parameter_count;
 };
 
 #if defined(CONFIG_DVB_DRXK) || (defined(CONFIG_DVB_DRXK_MODULE) \
diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 8fa28bb..ef146ca 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -5415,12 +5415,59 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
 #define QAM_LOCKRANGE__M      0x10
 #define QAM_LOCKRANGE_NORMAL  0x10
 
+static int QAMDemodulatorCommand(struct drxk_state *state, int numberOfParameters)
+{
+	int status;
+	u16 cmdResult;
+	u16 setParamParameters[4] = { 0, 0, 0, 0 };
+
+	setParamParameters[0] = state->m_Constellation;	/* modulation     */
+	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
+
+	if (numberOfParameters == 2) {
+		u16 setEnvParameters[1] = { 0 };
+
+		if (state->m_OperationMode == OM_QAM_ITU_C)
+			setEnvParameters[0] = QAM_TOP_ANNEX_C;
+		else
+			setEnvParameters[0] = QAM_TOP_ANNEX_A;
+
+		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult);
+		if (status < 0)
+			goto error;
+
+		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
+				     numberOfParameters, setParamParameters, 1, &cmdResult);
+	} else if (numberOfParameters == 4) {
+		if (state->m_OperationMode == OM_QAM_ITU_C)
+			setParamParameters[2] = QAM_TOP_ANNEX_C;
+		else
+			setParamParameters[2] = QAM_TOP_ANNEX_A;
+
+		setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
+		/* Env parameters */
+		/* check for LOCKRANGE Extented */
+		/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
+
+		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
+				     numberOfParameters, setParamParameters, 1, &cmdResult);
+	} else {
+		printk(KERN_WARNING "drxk: Unknown QAM demodulator parameter "
+			"count %d\n", numberOfParameters);
+	}
+
+error:
+	if (status < 0)
+		printk(KERN_WARNING "drxk: Warning %d on %s\n", status, __func__);
+	return status;
+}
+
 static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
 		  s32 tunerFreqOffset)
 {
 	int status;
-	u16 setParamParameters[4] = { 0, 0, 0, 0 };
 	u16 cmdResult;
+	int qamDemodParamCount = state->qam_demod_parameter_count;
 
 	dprintk(1, "\n");
 	/*
@@ -5472,34 +5519,39 @@ static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
 	}
 	if (status < 0)
 		goto error;
-	setParamParameters[0] = state->m_Constellation;	/* modulation     */
-	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
-	if (state->m_OperationMode == OM_QAM_ITU_C)
-		setParamParameters[2] = QAM_TOP_ANNEX_C;
-	else
-		setParamParameters[2] = QAM_TOP_ANNEX_A;
-	setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
-	/* Env parameters */
-	/* check for LOCKRANGE Extented */
-	/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
 
-	status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 4, setParamParameters, 1, &cmdResult);
-	if (status < 0) {
-		/* Fall-back to the simpler call */
-		if (state->m_OperationMode == OM_QAM_ITU_C)
-			setParamParameters[0] = QAM_TOP_ANNEX_C;
-		else
-			setParamParameters[0] = QAM_TOP_ANNEX_A;
-		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setParamParameters, 1, &cmdResult);
-		if (status < 0)
-			goto error;
+	// Use the 4-parameter if it's requested or we're probing for
+	// the correct command.
+	if (state->qam_demod_parameter_count == 4
+		|| !state->qam_demod_parameter_count) {
+		qamDemodParamCount = 4;
+		status = QAMDemodulatorCommand(state, qamDemodParamCount);
+	}
 
-		setParamParameters[0] = state->m_Constellation; /* modulation     */
-		setParamParameters[1] = DRXK_QAM_I12_J17;       /* interleave mode   */
-		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 2, setParamParameters, 1, &cmdResult);
+	// Use the 2-parameter command if it was requested or if we're
+	// probing for the correct command and the 4-parameter command
+	// failed.
+	if (state->qam_demod_parameter_count == 2
+		|| (!state->qam_demod_parameter_count && status < 0)) {
+		qamDemodParamCount = 2;
+		status = QAMDemodulatorCommand(state, qamDemodParamCount);
+	}
+
+	if (status < 0) {
+		dprintk(1, "Could not set demodulator parameters. Make "
+			"sure qam_demod_parameter_count (%d) is correct for "
+			"your firmware (%s).\n",
+			state->qam_demod_parameter_count, state->microcode_name);
+		goto error;
+	} else if (!state->qam_demod_parameter_count) {
+		dprintk(1, "Auto-probing the correct QAM demodulator command "
+			"parameters was successful - using %d parameters.\n",
+			qamDemodParamCount);
+
+		// One of our commands was successful. We don't need to 
+		// auto-probe anymore, now that we got the correct command.
+		state->qam_demod_parameter_count = qamDemodParamCount;
 	}
-	if (status < 0)
-		goto error;
 
 	/*
 	 * STEP 3: enable the system in a mode where the ADC provides valid
@@ -6502,6 +6554,7 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 	state->demod_address = adr;
 	state->single_master = config->single_master;
 	state->microcode_name = config->microcode_name;
+	state->qam_demod_parameter_count = config->qam_demod_parameter_count;
 	state->no_i2c_bridge = config->no_i2c_bridge;
 	state->antenna_gpio = config->antenna_gpio;
 	state->antenna_dvbt = config->antenna_dvbt;
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index f417797..6bb9fc4 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -353,6 +353,7 @@ struct drxk_state {
 	const char *microcode_name;
 	struct completion fw_wait_load;
 	const struct firmware *fw;
+	int qam_demod_parameter_count;
 };
 
 #define NEVER_LOCK 0
diff --git a/drivers/media/dvb/ngene/ngene-cards.c b/drivers/media/dvb/ngene/ngene-cards.c
index 7539a5d..72ee8de 100644
--- a/drivers/media/dvb/ngene/ngene-cards.c
+++ b/drivers/media/dvb/ngene/ngene-cards.c
@@ -217,6 +217,7 @@ static int demod_attach_drxk(struct ngene_channel *chan,
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.qam_demod_parameter_count = 4;
 	config.adr = 0x29 + (chan->number ^ 2);
 
 	chan->fe = dvb_attach(drxk_attach, &config, i2c);
diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index f8ffe10..a16531f 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -315,6 +315,7 @@ static struct drxk_config terratec_h5_drxk = {
 	.single_master = 1,
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-terratec-h5-drxk.fw",
+	.qam_demod_parameter_count = 2,
 };
 
 static struct drxk_config hauppauge_930c_drxk = {
@@ -323,6 +324,7 @@ static struct drxk_config hauppauge_930c_drxk = {
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-hauppauge-hvr930c-drxk.fw",
 	.chunk_size = 56,
+	.qam_demod_parameter_count = 2,
 };
 
 struct drxk_config terratec_htc_stick_drxk = {
@@ -331,6 +333,7 @@ struct drxk_config terratec_htc_stick_drxk = {
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-terratec-htc-stick-drxk.fw",
 	.chunk_size = 54,
+	.qam_demod_parameter_count = 2,
 	/* Required for the antenna_gpio to disable LNA. */
 	.antenna_dvbt = true,
 	/* The windows driver uses the same. This will disable LNA. */
@@ -347,6 +350,7 @@ static struct drxk_config pctv_520e_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.microcode_name = "dvb-demod-drxk-pctv.fw",
+	.qam_demod_parameter_count = 2,
 	.chunk_size = 58,
 	.antenna_dvbt = true, /* disable LNA */
 	.antenna_gpio = (1 << 2), /* disable LNA */
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [2/2,media] drxk: Improve logging.
  2012-06-28 21:20 ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
@ 2012-06-30 22:42   ` Martin Blumenstingl
  2012-06-30 22:42   ` [PATCH 2/2] [media] " Martin Blumenstingl
  1 sibling, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-30 22:42 UTC (permalink / raw)
  To: linux-media

there are no functional changes in this update.
This patch is just rebased for my other patch
("[1/2,media] drxk: Make the QAM demodulator command parameters configurable.").

Regards,
Martin


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/2] [media] drxk: Improve logging.
  2012-06-28 21:20 ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
  2012-06-30 22:42   ` [2/2,media] " Martin Blumenstingl
@ 2012-06-30 22:42   ` Martin Blumenstingl
  2012-07-04 21:38     ` [PATCH 2/2] [media] drxk: Improve logging. [v3] Martin Blumenstingl
  1 sibling, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2012-06-30 22:42 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

This patch simply fixes some logging calls:
- Use KERN_INFO when printing the chip status.
- Add a missing space when logging the drxk_gate_ctrl call.
- Use the same logging text as always if the scu_command in GetQAMLockStatus fails.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/frontends/drxk_hard.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index ef146ca..128a4e6 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -959,7 +959,7 @@ static int GetDeviceCapabilities(struct drxk_state *state)
 	if (status < 0)
 		goto error;
 
-printk(KERN_ERR "drxk: status = 0x%08x\n", sioTopJtagidLo);
+	printk(KERN_INFO "drxk: status = 0x%08x\n", sioTopJtagidLo);
 
 	/* driver 0.9.0 */
 	switch ((sioTopJtagidLo >> 29) & 0xF) {
@@ -5388,7 +5388,7 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
 			SCU_RAM_COMMAND_CMD_DEMOD_GET_LOCK, 0, NULL, 2,
 			Result);
 	if (status < 0)
-		printk(KERN_ERR "drxk: %s status = %08x\n", __func__, status);
+		printk(KERN_ERR "drxk: Error %d on %s\n", status, __func__);
 
 	if (Result[1] < SCU_RAM_QAM_LOCKED_LOCKED_DEMOD_LOCKED) {
 		/* 0x0000 NOT LOCKED */
@@ -6318,7 +6318,7 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 {
 	struct drxk_state *state = fe->demodulator_priv;
 
-	dprintk(1, "%s\n", enable ? "enable" : "disable");
+	dprintk(1, ": %s\n", enable ? "enable" : "disable");
 
 	if (state->m_DrxkState == DRXK_NO_DEV)
 		return -ENODEV;
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable.
  2012-06-30 22:32   ` Martin Blumenstingl
@ 2012-07-01  2:46     ` Mauro Carvalho Chehab
  2012-07-04 21:36     ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable. [v3] Martin Blumenstingl
  1 sibling, 0 replies; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2012-07-01  2:46 UTC (permalink / raw)
  To: Martin Blumenstingl; +Cc: linux-media

Em 30-06-2012 19:32, Martin Blumenstingl escreveu:
> Currently there are two different implementations (in the firmware) for
> the QAM demodulator command: one takes 4 and the other takes 2 parameters.
> The driver shows an error in dmesg When using the 4-parameter command with
> firmware that implements the 2-parameter command.
> Unfortunately this happens every time when chaning the frequency (on DVB-C).
> 
> This patch simply makes configurable, how many command parameters will be
> used.
> All existing drxk_config instances using the "drxk_a3.mc" were updated
> because this firmware is the only loadable firmware where the QAM
> demodulator command takes 4 parameters. Some firmwares in the ROM
> might also use it.
> The drxk instances in the em28xx-dvb driver were also updated to
> silence the warnings.
> 
> If no qam_demod_parameter_count is given in the drxk_config struct,
> then the correct number of parameters will be auto-detected.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   drivers/media/dvb/ddbridge/ddbridge-core.c |   1 +
>   drivers/media/dvb/frontends/drxk.h         |  11 ++-
>   drivers/media/dvb/frontends/drxk_hard.c    | 105 ++++++++++++++++++++++-------
>   drivers/media/dvb/frontends/drxk_hard.h    |   1 +
>   drivers/media/dvb/ngene/ngene-cards.c      |   1 +
>   drivers/media/video/em28xx/em28xx-dvb.c    |   4 ++
>   6 files changed, 96 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c
> index 131b938..ebf3f05 100644
> --- a/drivers/media/dvb/ddbridge/ddbridge-core.c
> +++ b/drivers/media/dvb/ddbridge/ddbridge-core.c
> @@ -578,6 +578,7 @@ static int demod_attach_drxk(struct ddb_input *input)
>   
>   	memset(&config, 0, sizeof(config));
>   	config.microcode_name = "drxk_a3.mc";
> +	config.qam_demod_parameter_count = 4;
>   	config.adr = 0x29 + (input->nr & 1);
>   
>   	fe = input->fe = dvb_attach(drxk_attach, &config, i2c);
> diff --git a/drivers/media/dvb/frontends/drxk.h b/drivers/media/dvb/frontends/drxk.h
> index 9d64e4f..19a8114 100644
> --- a/drivers/media/dvb/frontends/drxk.h
> +++ b/drivers/media/dvb/frontends/drxk.h
> @@ -20,6 +20,14 @@
>    *			means that 1=DVBC, 0 = DVBT. Zero means the opposite.
>    * @mpeg_out_clk_strength: DRXK Mpeg output clock drive strength.
>    * @microcode_name:	Name of the firmware file with the microcode
> + * @qam_demod_parameter_count:	The number of parameters used for the command
> + * 				to set the demodulator parameters. All
> + * 				firmwares are using the 2-parameter commmand.
> + * 				An exception is the "drxk_a3.mc" firmware,
> + * 				which uses the 4-parameter command.
> + * 				A value of 0 (default) or lower indicates that
> + * 				the correct number of parameters will be
> + * 				automatically detected.
>    *
>    * On the *_gpio vars, bit 0 is UIO-1, bit 1 is UIO-2 and bit 2 is
>    * UIO-3.
> @@ -38,7 +46,8 @@ struct drxk_config {
>   	u8	mpeg_out_clk_strength;
>   	int	chunk_size;
>   
> -	const char *microcode_name;
> +	const char	*microcode_name;
> +	int		 qam_demod_parameter_count;
>   };
>   
>   #if defined(CONFIG_DVB_DRXK) || (defined(CONFIG_DVB_DRXK_MODULE) \
> diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
> index 8fa28bb..ef146ca 100644
> --- a/drivers/media/dvb/frontends/drxk_hard.c
> +++ b/drivers/media/dvb/frontends/drxk_hard.c
> @@ -5415,12 +5415,59 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
>   #define QAM_LOCKRANGE__M      0x10
>   #define QAM_LOCKRANGE_NORMAL  0x10
>   
> +static int QAMDemodulatorCommand(struct drxk_state *state, int numberOfParameters)
> +{
> +	int status;
> +	u16 cmdResult;
> +	u16 setParamParameters[4] = { 0, 0, 0, 0 };
> +
> +	setParamParameters[0] = state->m_Constellation;	/* modulation     */
> +	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
> +
> +	if (numberOfParameters == 2) {
> +		u16 setEnvParameters[1] = { 0 };
> +
> +		if (state->m_OperationMode == OM_QAM_ITU_C)
> +			setEnvParameters[0] = QAM_TOP_ANNEX_C;
> +		else
> +			setEnvParameters[0] = QAM_TOP_ANNEX_A;
> +
> +		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult);
> +		if (status < 0)
> +			goto error;
> +
> +		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
> +				     numberOfParameters, setParamParameters, 1, &cmdResult);
> +	} else if (numberOfParameters == 4) {
> +		if (state->m_OperationMode == OM_QAM_ITU_C)
> +			setParamParameters[2] = QAM_TOP_ANNEX_C;
> +		else
> +			setParamParameters[2] = QAM_TOP_ANNEX_A;
> +
> +		setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
> +		/* Env parameters */
> +		/* check for LOCKRANGE Extented */
> +		/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
> +
> +		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
> +				     numberOfParameters, setParamParameters, 1, &cmdResult);
> +	} else {
> +		printk(KERN_WARNING "drxk: Unknown QAM demodulator parameter "
> +			"count %d\n", numberOfParameters);
> +	}
> +
> +error:
> +	if (status < 0)
> +		printk(KERN_WARNING "drxk: Warning %d on %s\n", status, __func__);
> +	return status;
> +}
> +
>   static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
>   		  s32 tunerFreqOffset)
>   {
>   	int status;
> -	u16 setParamParameters[4] = { 0, 0, 0, 0 };
>   	u16 cmdResult;
> +	int qamDemodParamCount = state->qam_demod_parameter_count;
>   
>   	dprintk(1, "\n");
>   	/*
> @@ -5472,34 +5519,39 @@ static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
>   	}
>   	if (status < 0)
>   		goto error;
> -	setParamParameters[0] = state->m_Constellation;	/* modulation     */
> -	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
> -	if (state->m_OperationMode == OM_QAM_ITU_C)
> -		setParamParameters[2] = QAM_TOP_ANNEX_C;
> -	else
> -		setParamParameters[2] = QAM_TOP_ANNEX_A;
> -	setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
> -	/* Env parameters */
> -	/* check for LOCKRANGE Extented */
> -	/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
>   
> -	status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 4, setParamParameters, 1, &cmdResult);
> -	if (status < 0) {
> -		/* Fall-back to the simpler call */
> -		if (state->m_OperationMode == OM_QAM_ITU_C)
> -			setParamParameters[0] = QAM_TOP_ANNEX_C;
> -		else
> -			setParamParameters[0] = QAM_TOP_ANNEX_A;
> -		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setParamParameters, 1, &cmdResult);
> -		if (status < 0)
> -			goto error;
> +	// Use the 4-parameter if it's requested or we're probing for
> +	// the correct command.

Please, don't use C99 comments. They're forbidden at the Linux Kernel.

Also, please check your patches using scripts/checkpatch.pl. It helps
to identify issues like that.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable. [v3]
  2012-06-30 22:32   ` Martin Blumenstingl
  2012-07-01  2:46     ` Mauro Carvalho Chehab
@ 2012-07-04 21:36     ` Martin Blumenstingl
  2012-07-04 21:36       ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable Martin Blumenstingl
  1 sibling, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2012-07-04 21:36 UTC (permalink / raw)
  To: linux-media

Hi,

here's an updated patch which fixes the style issues (sorry for that).

Regards,
Martin


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable.
  2012-07-04 21:36     ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable. [v3] Martin Blumenstingl
@ 2012-07-04 21:36       ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-07-04 21:36 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

Currently there are two different implementations (in the firmware) for
the QAM demodulator command: one takes 4 and the other takes 2 parameters.
The driver shows an error in dmesg When using the 4-parameter command with
firmware that implements the 2-parameter command.
Unfortunately this happens every time when chaning the frequency (on DVB-C).

This patch simply makes configurable, how many command parameters will be
used.
All existing drxk_config instances using the "drxk_a3.mc" were updated
because this firmware is the only loadable firmware where the QAM
demodulator command takes 4 parameters. Some firmwares in the ROM
might also use it.
The drxk instances in the em28xx-dvb driver were also updated to
silence the warnings.

If no qam_demod_parameter_count is given in the drxk_config struct,
then the correct number of parameters will be auto-detected.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/ddbridge/ddbridge-core.c |   1 +
 drivers/media/dvb/frontends/drxk.h         |  11 ++-
 drivers/media/dvb/frontends/drxk_hard.c    | 112 ++++++++++++++++++++++-------
 drivers/media/dvb/frontends/drxk_hard.h    |   1 +
 drivers/media/dvb/ngene/ngene-cards.c      |   1 +
 drivers/media/video/em28xx/em28xx-dvb.c    |   4 ++
 6 files changed, 104 insertions(+), 26 deletions(-)

diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c
index 131b938..ebf3f05 100644
--- a/drivers/media/dvb/ddbridge/ddbridge-core.c
+++ b/drivers/media/dvb/ddbridge/ddbridge-core.c
@@ -578,6 +578,7 @@ static int demod_attach_drxk(struct ddb_input *input)
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.qam_demod_parameter_count = 4;
 	config.adr = 0x29 + (input->nr & 1);
 
 	fe = input->fe = dvb_attach(drxk_attach, &config, i2c);
diff --git a/drivers/media/dvb/frontends/drxk.h b/drivers/media/dvb/frontends/drxk.h
index 9d64e4f..d615d7d 100644
--- a/drivers/media/dvb/frontends/drxk.h
+++ b/drivers/media/dvb/frontends/drxk.h
@@ -20,6 +20,14 @@
  *			means that 1=DVBC, 0 = DVBT. Zero means the opposite.
  * @mpeg_out_clk_strength: DRXK Mpeg output clock drive strength.
  * @microcode_name:	Name of the firmware file with the microcode
+ * @qam_demod_parameter_count:	The number of parameters used for the command
+ *				to set the demodulator parameters. All
+ *				firmwares are using the 2-parameter commmand.
+ *				An exception is the "drxk_a3.mc" firmware,
+ *				which uses the 4-parameter command.
+ *				A value of 0 (default) or lower indicates that
+ *				the correct number of parameters will be
+ *				automatically detected.
  *
  * On the *_gpio vars, bit 0 is UIO-1, bit 1 is UIO-2 and bit 2 is
  * UIO-3.
@@ -38,7 +46,8 @@ struct drxk_config {
 	u8	mpeg_out_clk_strength;
 	int	chunk_size;
 
-	const char *microcode_name;
+	const char	*microcode_name;
+	int		 qam_demod_parameter_count;
 };
 
 #if defined(CONFIG_DVB_DRXK) || (defined(CONFIG_DVB_DRXK_MODULE) \
diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 8fa28bb..07b362f 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -5415,12 +5415,67 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
 #define QAM_LOCKRANGE__M      0x10
 #define QAM_LOCKRANGE_NORMAL  0x10
 
+static int QAMDemodulatorCommand(struct drxk_state *state,
+				 int numberOfParameters)
+{
+	int status;
+	u16 cmdResult;
+	u16 setParamParameters[4] = { 0, 0, 0, 0 };
+
+	setParamParameters[0] = state->m_Constellation;	/* modulation     */
+	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
+
+	if (numberOfParameters == 2) {
+		u16 setEnvParameters[1] = { 0 };
+
+		if (state->m_OperationMode == OM_QAM_ITU_C)
+			setEnvParameters[0] = QAM_TOP_ANNEX_C;
+		else
+			setEnvParameters[0] = QAM_TOP_ANNEX_A;
+
+		status = scu_command(state,
+				     SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV,
+				     1, setEnvParameters, 1, &cmdResult);
+		if (status < 0)
+			goto error;
+
+		status = scu_command(state,
+				     SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
+				     numberOfParameters, setParamParameters,
+				     1, &cmdResult);
+	} else if (numberOfParameters == 4) {
+		if (state->m_OperationMode == OM_QAM_ITU_C)
+			setParamParameters[2] = QAM_TOP_ANNEX_C;
+		else
+			setParamParameters[2] = QAM_TOP_ANNEX_A;
+
+		setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
+		/* Env parameters */
+		/* check for LOCKRANGE Extented */
+		/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
+
+		status = scu_command(state,
+				     SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM,
+				     numberOfParameters, setParamParameters,
+				     1, &cmdResult);
+	} else {
+		printk(KERN_WARNING "drxk: Unknown QAM demodulator parameter "
+			"count %d\n", numberOfParameters);
+	}
+
+error:
+	if (status < 0)
+		printk(KERN_WARNING "drxk: Warning %d on %s\n",
+		       status, __func__);
+	return status;
+}
+
 static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
 		  s32 tunerFreqOffset)
 {
 	int status;
-	u16 setParamParameters[4] = { 0, 0, 0, 0 };
 	u16 cmdResult;
+	int qamDemodParamCount = state->qam_demod_parameter_count;
 
 	dprintk(1, "\n");
 	/*
@@ -5472,34 +5527,40 @@ static int SetQAM(struct drxk_state *state, u16 IntermediateFreqkHz,
 	}
 	if (status < 0)
 		goto error;
-	setParamParameters[0] = state->m_Constellation;	/* modulation     */
-	setParamParameters[1] = DRXK_QAM_I12_J17;	/* interleave mode   */
-	if (state->m_OperationMode == OM_QAM_ITU_C)
-		setParamParameters[2] = QAM_TOP_ANNEX_C;
-	else
-		setParamParameters[2] = QAM_TOP_ANNEX_A;
-	setParamParameters[3] |= (QAM_MIRROR_AUTO_ON);
-	/* Env parameters */
-	/* check for LOCKRANGE Extented */
-	/* setParamParameters[3] |= QAM_LOCKRANGE_NORMAL; */
 
-	status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 4, setParamParameters, 1, &cmdResult);
-	if (status < 0) {
-		/* Fall-back to the simpler call */
-		if (state->m_OperationMode == OM_QAM_ITU_C)
-			setParamParameters[0] = QAM_TOP_ANNEX_C;
-		else
-			setParamParameters[0] = QAM_TOP_ANNEX_A;
-		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setParamParameters, 1, &cmdResult);
-		if (status < 0)
-			goto error;
+	/* Use the 4-parameter if it's requested or we're probing for
+	 * the correct command. */
+	if (state->qam_demod_parameter_count == 4
+		|| !state->qam_demod_parameter_count) {
+		qamDemodParamCount = 4;
+		status = QAMDemodulatorCommand(state, qamDemodParamCount);
+	}
 
-		setParamParameters[0] = state->m_Constellation; /* modulation     */
-		setParamParameters[1] = DRXK_QAM_I12_J17;       /* interleave mode   */
-		status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM, 2, setParamParameters, 1, &cmdResult);
+	/* Use the 2-parameter command if it was requested or if we're
+	 * probing for the correct command and the 4-parameter command
+	 * failed. */
+	if (state->qam_demod_parameter_count == 2
+		|| (!state->qam_demod_parameter_count && status < 0)) {
+		qamDemodParamCount = 2;
+		status = QAMDemodulatorCommand(state, qamDemodParamCount);
 	}
-	if (status < 0)
+
+	if (status < 0) {
+		dprintk(1, "Could not set demodulator parameters. Make "
+			"sure qam_demod_parameter_count (%d) is correct for "
+			"your firmware (%s).\n",
+			state->qam_demod_parameter_count,
+			state->microcode_name);
 		goto error;
+	} else if (!state->qam_demod_parameter_count) {
+		dprintk(1, "Auto-probing the correct QAM demodulator command "
+			"parameters was successful - using %d parameters.\n",
+			qamDemodParamCount);
+
+		/* One of our commands was successful. We don't need to
+		/* auto-probe anymore, now that we got the correct command. */
+		state->qam_demod_parameter_count = qamDemodParamCount;
+	}
 
 	/*
 	 * STEP 3: enable the system in a mode where the ADC provides valid
@@ -6502,6 +6563,7 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 	state->demod_address = adr;
 	state->single_master = config->single_master;
 	state->microcode_name = config->microcode_name;
+	state->qam_demod_parameter_count = config->qam_demod_parameter_count;
 	state->no_i2c_bridge = config->no_i2c_bridge;
 	state->antenna_gpio = config->antenna_gpio;
 	state->antenna_dvbt = config->antenna_dvbt;
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index f417797..6bb9fc4 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -353,6 +353,7 @@ struct drxk_state {
 	const char *microcode_name;
 	struct completion fw_wait_load;
 	const struct firmware *fw;
+	int qam_demod_parameter_count;
 };
 
 #define NEVER_LOCK 0
diff --git a/drivers/media/dvb/ngene/ngene-cards.c b/drivers/media/dvb/ngene/ngene-cards.c
index 7539a5d..72ee8de 100644
--- a/drivers/media/dvb/ngene/ngene-cards.c
+++ b/drivers/media/dvb/ngene/ngene-cards.c
@@ -217,6 +217,7 @@ static int demod_attach_drxk(struct ngene_channel *chan,
 
 	memset(&config, 0, sizeof(config));
 	config.microcode_name = "drxk_a3.mc";
+	config.qam_demod_parameter_count = 4;
 	config.adr = 0x29 + (chan->number ^ 2);
 
 	chan->fe = dvb_attach(drxk_attach, &config, i2c);
diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index f8ffe10..a16531f 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -315,6 +315,7 @@ static struct drxk_config terratec_h5_drxk = {
 	.single_master = 1,
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-terratec-h5-drxk.fw",
+	.qam_demod_parameter_count = 2,
 };
 
 static struct drxk_config hauppauge_930c_drxk = {
@@ -323,6 +324,7 @@ static struct drxk_config hauppauge_930c_drxk = {
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-hauppauge-hvr930c-drxk.fw",
 	.chunk_size = 56,
+	.qam_demod_parameter_count = 2,
 };
 
 struct drxk_config terratec_htc_stick_drxk = {
@@ -331,6 +333,7 @@ struct drxk_config terratec_htc_stick_drxk = {
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-terratec-htc-stick-drxk.fw",
 	.chunk_size = 54,
+	.qam_demod_parameter_count = 2,
 	/* Required for the antenna_gpio to disable LNA. */
 	.antenna_dvbt = true,
 	/* The windows driver uses the same. This will disable LNA. */
@@ -347,6 +350,7 @@ static struct drxk_config pctv_520e_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.microcode_name = "dvb-demod-drxk-pctv.fw",
+	.qam_demod_parameter_count = 2,
 	.chunk_size = 58,
 	.antenna_dvbt = true, /* disable LNA */
 	.antenna_gpio = (1 << 2), /* disable LNA */
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] [media] drxk: Improve logging. [v3]
  2012-06-30 22:42   ` [PATCH 2/2] [media] " Martin Blumenstingl
@ 2012-07-04 21:38     ` Martin Blumenstingl
  2012-07-04 21:38       ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2012-07-04 21:38 UTC (permalink / raw)
  To: linux-media

Hi,

here's an updated patch, rebased for the changes in patch 1.

Regards,
Martin


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/2] [media] drxk: Improve logging.
  2012-07-04 21:38     ` [PATCH 2/2] [media] drxk: Improve logging. [v3] Martin Blumenstingl
@ 2012-07-04 21:38       ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2012-07-04 21:38 UTC (permalink / raw)
  To: linux-media; +Cc: Martin Blumenstingl

This patch simply fixes some logging calls:
- Use KERN_INFO when printing the chip status.
- Add a missing space when logging the drxk_gate_ctrl call.
- Use the same logging text as always if the scu_command in GetQAMLockStatus fails.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/media/dvb/frontends/drxk_hard.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 07b362f..1e4d3d5 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -959,7 +959,7 @@ static int GetDeviceCapabilities(struct drxk_state *state)
 	if (status < 0)
 		goto error;
 
-printk(KERN_ERR "drxk: status = 0x%08x\n", sioTopJtagidLo);
+	printk(KERN_INFO "drxk: status = 0x%08x\n", sioTopJtagidLo);
 
 	/* driver 0.9.0 */
 	switch ((sioTopJtagidLo >> 29) & 0xF) {
@@ -5388,7 +5388,7 @@ static int GetQAMLockStatus(struct drxk_state *state, u32 *pLockStatus)
 			SCU_RAM_COMMAND_CMD_DEMOD_GET_LOCK, 0, NULL, 2,
 			Result);
 	if (status < 0)
-		printk(KERN_ERR "drxk: %s status = %08x\n", __func__, status);
+		printk(KERN_ERR "drxk: Error %d on %s\n", status, __func__);
 
 	if (Result[1] < SCU_RAM_QAM_LOCKED_LOCKED_DEMOD_LOCKED) {
 		/* 0x0000 NOT LOCKED */
@@ -6327,7 +6327,7 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 {
 	struct drxk_state *state = fe->demodulator_priv;
 
-	dprintk(1, "%s\n", enable ? "enable" : "disable");
+	dprintk(1, ": %s\n", enable ? "enable" : "disable");
 
 	if (state->m_DrxkState == DRXK_NO_DEV)
 		return -ENODEV;
-- 
1.7.11.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-07-04 21:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-28 21:20 [media] drxk: Minor cleanup (basically logging related) Martin Blumenstingl
2012-06-28 21:20 ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command configurable Martin Blumenstingl
2012-06-29  8:31   ` Ralph Metzler
2012-06-29 15:58     ` Martin Blumenstingl
2012-06-29 16:13       ` Mauro Carvalho Chehab
2012-06-29 16:32         ` Martin Blumenstingl
2012-06-30 22:32   ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable Martin Blumenstingl
2012-06-30 22:32   ` Martin Blumenstingl
2012-07-01  2:46     ` Mauro Carvalho Chehab
2012-07-04 21:36     ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable. [v3] Martin Blumenstingl
2012-07-04 21:36       ` [PATCH 1/2] [media] drxk: Make the QAM demodulator command parameters configurable Martin Blumenstingl
2012-06-28 21:20 ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl
2012-06-30 22:42   ` [2/2,media] " Martin Blumenstingl
2012-06-30 22:42   ` [PATCH 2/2] [media] " Martin Blumenstingl
2012-07-04 21:38     ` [PATCH 2/2] [media] drxk: Improve logging. [v3] Martin Blumenstingl
2012-07-04 21:38       ` [PATCH 2/2] [media] drxk: Improve logging Martin Blumenstingl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).