All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.