[PATCH v3 00/14] si2157: Analog tuning and optimizations

[PATCH v3 00/14] si2157: Analog tuning and optimizations

[Brad Love]

This series mainly enables analog tuning in the si2157
driver. Some various optimizations are included as well,
along with the dots connected to allow devices with
TUNER_ABSENT to utilize an analog frontend on two different
bridges. Finally two missing statistics are added to get
signal strength and CNR on a tuner and demod respectively.

Summary:
- Enable tuner status flags
- Check tuner status flags
- String cleanup and register labeling
- The analog tuning functions in si2157
- A function to wait for set_*params to complete
- Enable analog fe on TUNER_ABSENT devices in cx231xx and cx23885
- Include some signal strength DVBv5 stats

Now the two patches that 'Add i2c device analog tuner support'
I would like comment on. It looks quite ugly to have big case
statements identifying the TUNER_ABSENT models that have analog.
There is nothing unique done in the blocks, mostly. Right now
there is only a few models, but the addition of more would become
a bit excessive.

Instead of the case statement should a board profile field be
added to the two affected drivers? Something like .has_i2c_analog_fe ?

===

I split up the tune completion and tune lock (5 & 6) patches. Patch 5
should be acceptable, patch 6 has been found to be very insightful in
debugging situations.

Changes since v2:
- Rebase
- Enable HVR5525 analog tuner
- Remove bff option from NTSC analog capable boards
- Split tune completion and tuner lock patches
- Fix si2141 init with error status flags enabled
- Device caps and capabilities fixes

Changes since v1:
- One unnecessary patch removed
- __func__ removed from dev_XXX macros
- normalization logic simplified and explained in rf strength calculation

Brad Love (14):
  si2157: Enable tuner status flags
  si2157: Check error status bit on cmd execute
  si2157: Better check for running tuner in init
  si2157: Add analog tuning related functions
  si2157: Briefly wait for tuning operation to complete
  si2157: module debug option to wait on signal lock
  cx23885: Add analog frontend to Hauppauge QuadHD
  cx23885: Add analog frontend to 1265_K4
  cx23885: Add analog frontend to HVR5525
  cx23885: Add i2c device analog tuner support
  cx231xx: Add i2c device analog tuner support
  si2157: add on-demand rf strength func
  lgdt3306a: Add CNR v5 stat
  cx25840: Register labeling, chip specific correction

 drivers/media/dvb-frontends/lgdt3306a.c    |  14 +
 drivers/media/i2c/cx25840/cx25840-core.c   |  40 +-
 drivers/media/pci/cx23885/cx23885-cards.c  |  51 ++-
 drivers/media/pci/cx23885/cx23885-dvb.c    |  31 ++
 drivers/media/pci/cx23885/cx23885-video.c  |  98 ++++-
 drivers/media/tuners/si2157.c              | 415 ++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h         |   2 +
 drivers/media/usb/cx231xx/cx231xx-avcore.c |  35 +-
 drivers/media/usb/cx231xx/cx231xx-video.c  |  85 ++++-
 9 files changed, 697 insertions(+), 74 deletions(-)

-- 
2.23.0

[PATCH v3 01/14] si2157: Enable tuner status flags

[Brad Love]

Enable flags to get status of commands sent to the tuner.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
No changes

 drivers/media/tuners/si2157.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 898e0f9f8b70..12f88304ac0f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -230,6 +230,28 @@ static int si2157_init(struct dvb_frontend *fe)
 
 	dev_info(&client->dev, "firmware version: %c.%c.%d\n",
 			cmd.args[6], cmd.args[7], cmd.args[8]);
+
+	/* enable tuner status flags */
+	memcpy(cmd.args, "\x14\x00\x01\x05\x01\x00", 6);
+	cmd.wlen = 6;
+	cmd.rlen = 1;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	memcpy(cmd.args, "\x14\x00\x01\x06\x01\x00", 6);
+	cmd.wlen = 6;
+	cmd.rlen = 1;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	memcpy(cmd.args, "\x14\x00\x01\x07\x01\x00", 6);
+	cmd.wlen = 6;
+	cmd.rlen = 1;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
 warm:
 	/* init statistics in order signal app which are supported */
 	c->strength.len = 1;
-- 
2.23.0

[PATCH v3 02/14] si2157: Check error status bit on cmd execute

[Brad Love]

Check error status bit on command execute, if error bit is
set return -EAGAIN. Ignore -EAGAIN in probe during device check.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Fix -EAGAIN returned by si2141 in si2157_init

 drivers/media/tuners/si2157.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 12f88304ac0f..69c625eaee25 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -47,14 +47,20 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
 				break;
 		}
 
-		dev_dbg(&client->dev, "cmd execution took %d ms\n",
-				jiffies_to_msecs(jiffies) -
-				(jiffies_to_msecs(timeout) - TIMEOUT));
+		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
+			jiffies_to_msecs(jiffies) -
+			(jiffies_to_msecs(timeout) - TIMEOUT),
+			cmd->args[0]);
 
 		if (!((cmd->args[0] >> 7) & 0x01)) {
 			ret = -ETIMEDOUT;
 			goto err_mutex_unlock;
 		}
+		/* check error status bit */
+		if (cmd->args[0] & 0x40) {
+			ret = -EAGAIN;
+			goto err_mutex_unlock;
+		}
 	}
 
 	mutex_unlock(&dev->i2c_mutex);
@@ -106,7 +112,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	}
 	cmd.rlen = 1;
 	ret = si2157_cmd_execute(client, &cmd);
-	if (ret)
+	if (ret && (dev->chiptype != SI2157_CHIPTYPE_SI2141 || ret != -EAGAIN))
 		goto err;
 
 	/* Si2141 needs a second command before it answers the revision query */
@@ -478,7 +484,7 @@ static int si2157_probe(struct i2c_client *client,
 	cmd.wlen = 0;
 	cmd.rlen = 1;
 	ret = si2157_cmd_execute(client, &cmd);
-	if (ret)
+	if (ret && ret != -EAGAIN)
 		goto err_kfree;
 
 	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
-- 
2.23.0

Re: [PATCH v3 02/14] si2157: Check error status bit on cmd execute

[Antti Palosaari]

Hello,

On 11/14/19 10:03 PM, Brad Love wrote:
> Check error status bit on command execute, if error bit is
> set return -EAGAIN. Ignore -EAGAIN in probe during device check.

You should open when and why this error condition happens and try to fix 
things on first hand that error situation never appears. As you added 
that kind of special error handling to does it mean it happens during 
probe? If yes, then something must be wrong before probe is called. Also 
succeeding probe on "error again" does not sound very correct.

Antti

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Since v2:
> - Fix -EAGAIN returned by si2141 in si2157_init
> 
>   drivers/media/tuners/si2157.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 12f88304ac0f..69c625eaee25 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -47,14 +47,20 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
>   				break;
>   		}
>   
> -		dev_dbg(&client->dev, "cmd execution took %d ms\n",
> -				jiffies_to_msecs(jiffies) -
> -				(jiffies_to_msecs(timeout) - TIMEOUT));
> +		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
> +			jiffies_to_msecs(jiffies) -
> +			(jiffies_to_msecs(timeout) - TIMEOUT),
> +			cmd->args[0]);
>   
>   		if (!((cmd->args[0] >> 7) & 0x01)) {
>   			ret = -ETIMEDOUT;
>   			goto err_mutex_unlock;
>   		}
> +		/* check error status bit */
> +		if (cmd->args[0] & 0x40) {
> +			ret = -EAGAIN;
> +			goto err_mutex_unlock;
> +		}
>   	}
>   
>   	mutex_unlock(&dev->i2c_mutex);
> @@ -106,7 +112,7 @@ static int si2157_init(struct dvb_frontend *fe)
>   	}
>   	cmd.rlen = 1;
>   	ret = si2157_cmd_execute(client, &cmd);
> -	if (ret)
> +	if (ret && (dev->chiptype != SI2157_CHIPTYPE_SI2141 || ret != -EAGAIN))
>   		goto err;
>   
>   	/* Si2141 needs a second command before it answers the revision query */
> @@ -478,7 +484,7 @@ static int si2157_probe(struct i2c_client *client,
>   	cmd.wlen = 0;
>   	cmd.rlen = 1;
>   	ret = si2157_cmd_execute(client, &cmd);
> -	if (ret)
> +	if (ret && ret != -EAGAIN)
>   		goto err_kfree;
>   
>   	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> 

-- 
http://palosaari.fi/

[PATCH v3 03/14] si2157: Better check for running tuner in init

[Brad Love]

Getting the Xtal trim property to check if running is less error prone.
Reset if_frequency if state is unknown.

Replaces the previous "garbage check".

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
No changes

 drivers/media/tuners/si2157.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 69c625eaee25..e1e23e78fd19 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -81,24 +81,23 @@ static int si2157_init(struct dvb_frontend *fe)
 	struct si2157_cmd cmd;
 	const struct firmware *fw;
 	const char *fw_name;
-	unsigned int uitmp, chip_id;
+	unsigned int chip_id, xtal_trim;
 
 	dev_dbg(&client->dev, "\n");
 
-	/* Returned IF frequency is garbage when firmware is not running */
-	memcpy(cmd.args, "\x15\x00\x06\x07", 4);
+	/* Try to get Xtal trim property, to verify tuner still running */
+	memcpy(cmd.args, "\x15\x00\x04\x02", 4);
 	cmd.wlen = 4;
 	cmd.rlen = 4;
 	ret = si2157_cmd_execute(client, &cmd);
-	if (ret)
-		goto err;
 
-	uitmp = cmd.args[2] << 0 | cmd.args[3] << 8;
-	dev_dbg(&client->dev, "if_frequency kHz=%u\n", uitmp);
+	xtal_trim = cmd.args[2] | (cmd.args[3] << 8);
 
-	if (uitmp == dev->if_frequency / 1000)
+	if (ret == 0 && xtal_trim < 16)
 		goto warm;
 
+	dev->if_frequency = 0; /* we no longer know current tuner state */
+
 	/* power up */
 	if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
 		memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
-- 
2.23.0

Re: [PATCH v3 03/14] si2157: Better check for running tuner in init

[Antti Palosaari]

On 11/14/19 10:03 PM, Brad Love wrote:
> Getting the Xtal trim property to check if running is less error prone.
> Reset if_frequency if state is unknown.
> 
> Replaces the previous "garbage check".
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> No changes
> 
>   drivers/media/tuners/si2157.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 69c625eaee25..e1e23e78fd19 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -81,24 +81,23 @@ static int si2157_init(struct dvb_frontend *fe)
>   	struct si2157_cmd cmd;
>   	const struct firmware *fw;
>   	const char *fw_name;
> -	unsigned int uitmp, chip_id;
> +	unsigned int chip_id, xtal_trim;
>   
>   	dev_dbg(&client->dev, "\n");
>   
> -	/* Returned IF frequency is garbage when firmware is not running */
> -	memcpy(cmd.args, "\x15\x00\x06\x07", 4);
> +	/* Try to get Xtal trim property, to verify tuner still running */
> +	memcpy(cmd.args, "\x15\x00\x04\x02", 4);
>   	cmd.wlen = 4;
>   	cmd.rlen = 4;
>   	ret = si2157_cmd_execute(client, &cmd);
> -	if (ret)
> -		goto err;
>   
> -	uitmp = cmd.args[2] << 0 | cmd.args[3] << 8;
> -	dev_dbg(&client->dev, "if_frequency kHz=%u\n", uitmp);
> +	xtal_trim = cmd.args[2] | (cmd.args[3] << 8);
>   
> -	if (uitmp == dev->if_frequency / 1000)
> +	if (ret == 0 && xtal_trim < 16)
>   		goto warm;

Defining new variable does not make code clearer, uitmp used earlier was 
just fine as value was used just next line or so. You just need to 
change two or three line or so here.

Checking xtal trim sounds still a bit elegant than old method.

>   
> +	dev->if_frequency = 0; /* we no longer know current tuner state */
> +

hmmm, what is idea of that?

>   	/* power up */
>   	if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
>   		memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> 

-- 
http://palosaari.fi/

[PATCH v3 04/14] si2157: Add analog tuning related functions

[Brad Love]

Include set_analog_params, get_frequency, and get_bandwidth.

Tested with NTSC and PAL standards via ch3/4 generator. Other standards
are included, but are untested due to lack of generator.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v1:
- remove __func__ from dev_dbg macros

 drivers/media/tuners/si2157.c      | 230 ++++++++++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h |   2 +
 2 files changed, 229 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e1e23e78fd19..78a855df30da 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -371,7 +371,7 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	if (ret)
 		goto err;
 
-	/* set if frequency if needed */
+	/* set digital if frequency if needed */
 	if (if_frequency != dev->if_frequency) {
 		memcpy(cmd.args, "\x14\x00\x06\x07", 4);
 		cmd.args[4] = (if_frequency / 1000) & 0xff;
@@ -385,7 +385,7 @@ static int si2157_set_params(struct dvb_frontend *fe)
 		dev->if_frequency = if_frequency;
 	}
 
-	/* set frequency */
+	/* set digital frequency */
 	memcpy(cmd.args, "\x41\x00\x00\x00\x00\x00\x00\x00", 8);
 	cmd.args[4] = (c->frequency >>  0) & 0xff;
 	cmd.args[5] = (c->frequency >>  8) & 0xff;
@@ -397,18 +397,239 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	if (ret)
 		goto err;
 
+	dev->bandwidth = bandwidth;
+	dev->frequency = c->frequency;
+
 	return 0;
 err:
+	dev->bandwidth = 0;
+	dev->frequency = 0;
+	dev->if_frequency = 0;
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
+static int si2157_set_analog_params(struct dvb_frontend *fe,
+				     struct analog_parameters *params)
+{
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
+	char *std; /* for debugging */
+	int ret;
+	struct si2157_cmd cmd;
+	u32 bandwidth = 0;
+	u32 if_frequency = 0;
+	u32 freq = 0;
+	u64 tmp_lval = 0;
+	u8 system = 0;
+	u8 color = 0;    /* 0=NTSC/PAL, 0x10=SECAM */
+	u8 invert_analog = 1; /* analog tuner spectrum; 0=normal, 1=inverted */
+
+	if (dev->chiptype != SI2157_CHIPTYPE_SI2157) {
+		dev_info(&client->dev, "Analog tuning not supported for chiptype=%u\n",
+			 dev->chiptype);
+		ret = -EINVAL;
+		goto err;
+	}
+
+	if (!dev->active)
+		si2157_init(fe);
+
+	if (!dev->active) {
+		ret = -EAGAIN;
+		goto err;
+	}
+	if (params->mode == V4L2_TUNER_RADIO) {
+	/*
+	 * std = "fm";
+	 * bandwidth = 1700000; //best can do for FM, AGC will be a mess though
+	 * if_frequency = 1250000;  //HVR-225x(saa7164), HVR-12xx(cx23885)
+	 * if_frequency = 6600000;  //HVR-9xx(cx231xx)
+	 * if_frequency = 5500000;  //HVR-19xx(pvrusb2)
+	 */
+		dev_err(&client->dev, "si2157 does not currently support FM radio\n");
+		ret = -EINVAL;
+		goto err;
+	}
+	tmp_lval = params->frequency * 625LL;
+	do_div(tmp_lval, 10); /* convert to HZ */
+	freq = (u32)tmp_lval;
+
+	if (freq < 1000000) /* is freq in KHz */
+		freq = freq * 1000;
+	dev->frequency = freq;
+
+	/* if_frequency values based on tda187271C2 */
+	if (params->std & (V4L2_STD_B | V4L2_STD_GH)) {
+		if (freq >= 470000000) {
+			std = "palGH";
+			bandwidth = 8000000;
+			if_frequency = 6000000;
+			system = 1;
+			if (params->std & (V4L2_STD_SECAM_G | V4L2_STD_SECAM_H)) {
+				std = "secamGH";
+				color = 0x10;
+			}
+		} else {
+			std = "palB";
+			bandwidth = 7000000;
+			if_frequency = 6000000;
+			system = 0;
+			if (params->std & V4L2_STD_SECAM_B) {
+				std = "secamB";
+				color = 0x10;
+			}
+		}
+	} else if (params->std & V4L2_STD_MN) {
+		std = "MN";
+		bandwidth = 6000000;
+		if_frequency = 5400000;
+		system = 2;
+	} else if (params->std & V4L2_STD_PAL_I) {
+		std = "palI";
+		bandwidth = 8000000;
+		if_frequency = 7250000; /* TODO: does not work yet */
+		system = 4;
+	} else if (params->std & V4L2_STD_DK) {
+		std = "palDK";
+		bandwidth = 8000000;
+		if_frequency = 6900000; /* TODO: does not work yet */
+		system = 5;
+		if (params->std & V4L2_STD_SECAM_DK) {
+			std = "secamDK";
+			color = 0x10;
+		}
+	} else if (params->std & V4L2_STD_SECAM_L) {
+		std = "secamL";
+		bandwidth = 8000000;
+		if_frequency = 6750000; /* TODO: untested */
+		system = 6;
+		color = 0x10;
+	} else if (params->std & V4L2_STD_SECAM_LC) {
+		std = "secamL'";
+		bandwidth = 7000000;
+		if_frequency = 1250000; /* TODO: untested */
+		system = 7;
+		color = 0x10;
+	} else {
+		std = "unknown";
+	}
+	/* calc channel center freq */
+	freq = freq - 1250000 + (bandwidth / 2);
+
+	dev_dbg(&client->dev,
+		"mode=%d system=%u std='%s' params->frequency=%u center freq=%u if=%u bandwidth=%u\n",
+		params->mode, system, std, params->frequency,
+		freq, if_frequency, bandwidth);
+
+	/* set analog IF port */
+	memcpy(cmd.args, "\x14\x00\x03\x06\x08\x02", 6);
+	/* in using dev->if_port, we assume analog and digital IF's */
+	/*   are always on different ports */
+	/* assumes if_port definition is 0 or 1 for digital out */
+	cmd.args[4] = (dev->if_port == 1) ? 8 : 10;
+	/* Analog AGC assumed external */
+	cmd.args[5] = (dev->if_port == 1) ? 2 : 1;
+	cmd.wlen = 6;
+	cmd.rlen = 4;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	/* set analog IF output config */
+	memcpy(cmd.args, "\x14\x00\x0d\x06\x94\x64", 6);
+	cmd.wlen = 6;
+	cmd.rlen = 4;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	/* make this distinct from a digital IF */
+	dev->if_frequency = if_frequency | 1;
+
+	/* calc and set tuner analog if center frequency */
+	if_frequency = if_frequency + 1250000 - (bandwidth / 2);
+	dev_dbg(&client->dev, "IF Ctr freq=%d\n", if_frequency);
+
+	memcpy(cmd.args, "\x14\x00\x0C\x06", 4);
+	cmd.args[4] = (if_frequency / 1000) & 0xff;
+	cmd.args[5] = ((if_frequency / 1000) >> 8) & 0xff;
+	cmd.wlen = 6;
+	cmd.rlen = 4;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	/* set analog AGC config */
+	memcpy(cmd.args, "\x14\x00\x07\x06\x32\xc8", 6);
+	cmd.wlen = 6;
+	cmd.rlen = 4;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	/* set analog video mode */
+	memcpy(cmd.args, "\x14\x00\x04\x06\x00\x00", 6);
+	cmd.args[4] = system | color;
+	/* can use dev->inversion if assumed applies to both digital/analog */
+	if (invert_analog)
+		cmd.args[5] |= 0x02;
+	cmd.wlen = 6;
+	cmd.rlen = 1;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	/* set analog frequency */
+	memcpy(cmd.args, "\x41\x01\x00\x00\x00\x00\x00\x00", 8);
+	cmd.args[4] = (freq >>  0) & 0xff;
+	cmd.args[5] = (freq >>  8) & 0xff;
+	cmd.args[6] = (freq >> 16) & 0xff;
+	cmd.args[7] = (freq >> 24) & 0xff;
+	cmd.wlen = 8;
+	cmd.rlen = 1;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	dev->bandwidth = bandwidth;
+
+	return 0;
+err:
+	dev->bandwidth = 0;
+	dev->frequency = 0;
+	dev->if_frequency = 0;
+	dev_dbg(&client->dev, "failed=%d\n", ret);
+	return ret;
+}
+
+static int si2157_get_frequency(struct dvb_frontend *fe, u32 *frequency)
+{
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
+
+	*frequency = dev->frequency;
+	dev_dbg(&client->dev, "freq=%u\n", dev->frequency);
+	return 0;
+}
+
+static int si2157_get_bandwidth(struct dvb_frontend *fe, u32 *bandwidth)
+{
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
+
+	*bandwidth = dev->bandwidth;
+	dev_dbg(&client->dev, "bandwidth=%u\n", dev->bandwidth);
+	return 0;
+}
+
 static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
 {
 	struct i2c_client *client = fe->tuner_priv;
 	struct si2157_dev *dev = i2c_get_clientdata(client);
 
-	*frequency = dev->if_frequency;
+	*frequency = dev->if_frequency & ~1; /* strip analog IF indicator bit */
+	dev_dbg(&client->dev, "if_frequency=%u\n", *frequency);
 	return 0;
 }
 
@@ -422,6 +643,9 @@ static const struct dvb_tuner_ops si2157_ops = {
 	.init = si2157_init,
 	.sleep = si2157_sleep,
 	.set_params = si2157_set_params,
+	.set_analog_params = si2157_set_analog_params,
+	.get_frequency     = si2157_get_frequency,
+	.get_bandwidth     = si2157_get_bandwidth,
 	.get_if_frequency = si2157_get_if_frequency,
 };
 
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 778f81b39996..ef4796098931 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -29,6 +29,8 @@ struct si2157_dev {
 	u8 chiptype;
 	u8 if_port;
 	u32 if_frequency;
+	u32 bandwidth;
+	u32 frequency;
 	struct delayed_work stat_work;
 
 #if defined(CONFIG_MEDIA_CONTROLLER)
-- 
2.23.0

Re: [PATCH v3 04/14] si2157: Add analog tuning related functions

[Antti Palosaari]

On 11/14/19 10:03 PM, Brad Love wrote:
> Include set_analog_params, get_frequency, and get_bandwidth.
> 
> Tested with NTSC and PAL standards via ch3/4 generator. Other standards
> are included, but are untested due to lack of generator.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Changes since v1:
> - remove __func__ from dev_dbg macros

After all it looks pretty simply, but implementation is not done that 
simply. Crazy RF/IF offsets, impossible values and so.

I think you need to study some tuner basics:
* what IF frequency is, why, and so
* IF vs. BW, what is relation, what are possible values
* Down conversion RF to IF. OK, *on that case* firmware covers PLL, but 
it is fundamental. So basics of integer-N and fractional-N PLL is always 
you must to know.
* Filtering. Especially IF filtering, which is generally low-pass 
filtering. Think possible filters when selecting IF.


regards
Antti


-- 
http://palosaari.fi/

Re: [PATCH v3 04/14] si2157: Add analog tuning related functions

[Mauro Carvalho Chehab]

Em Sun, 24 Nov 2019 07:09:07 +0200
Antti Palosaari <crope@iki.fi> escreveu:

> On 11/14/19 10:03 PM, Brad Love wrote:
> > Include set_analog_params, get_frequency, and get_bandwidth.
> > 
> > Tested with NTSC and PAL standards via ch3/4 generator. Other standards
> > are included, but are untested due to lack of generator.
> > 
> > Signed-off-by: Brad Love <brad@nextdimension.cc>
> > ---
> > Changes since v1:
> > - remove __func__ from dev_dbg macros  
> 
> After all it looks pretty simply, but implementation is not done that 
> simply. Crazy RF/IF offsets, impossible values and so.
> 
> I think you need to study some tuner basics:
> * what IF frequency is, why, and so
> * IF vs. BW, what is relation, what are possible values
> * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but 
> it is fundamental. So basics of integer-N and fractional-N PLL is always 
> you must to know.
> * Filtering. Especially IF filtering, which is generally low-pass 
> filtering. Think possible filters when selecting IF.

For me, the implementation seems to make sense. I mean, for analog TV, both
channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard
only.

So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it
may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is
a channel at VHF or at UHF range.

So, this part of the patch sounds OK to me.

The IF is actually a little trickier. Yet, if you take a lok on other
tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to
automatically set the IF that will work for each video standard:

static void tda827x_set_std(struct dvb_frontend *fe,
			    struct analog_parameters *params)
{
	struct tda827x_priv *priv = fe->tuner_priv;
	char *mode;

	priv->lpsel = 0;
	if (params->std & V4L2_STD_MN) {
		priv->sgIF = 92;
		priv->lpsel = 1;
		mode = "MN";
	} else if (params->std & V4L2_STD_B) {
		priv->sgIF = 108;
		mode = "B";
...

static int tda827xo_set_analog_params(struct dvb_frontend *fe,
				      struct analog_parameters *params)
{

...

	N = freq + priv->sgIF;

In other words, for analog TV, the tuner will always receive the 
channel central frequency, with may vary depending on the video
standard, and will adjust it to tune at the right channel, using the
per-standard IF (if needed), as, on most tuner drivers, the tunning
frequency should be either initial frequency or the main carrier
frequency, and not the center frequency.


Cheers,
Mauro

Re: [PATCH v3 04/14] si2157: Add analog tuning related functions

[Antti Palosaari]

On 12/19/19 3:13 PM, Mauro Carvalho Chehab wrote:
> Em Sun, 24 Nov 2019 07:09:07 +0200
> Antti Palosaari <crope@iki.fi> escreveu:
> 
>> On 11/14/19 10:03 PM, Brad Love wrote:
>>> Include set_analog_params, get_frequency, and get_bandwidth.
>>>
>>> Tested with NTSC and PAL standards via ch3/4 generator. Other standards
>>> are included, but are untested due to lack of generator.
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>> ---
>>> Changes since v1:
>>> - remove __func__ from dev_dbg macros
>>
>> After all it looks pretty simply, but implementation is not done that
>> simply. Crazy RF/IF offsets, impossible values and so.
>>
>> I think you need to study some tuner basics:
>> * what IF frequency is, why, and so
>> * IF vs. BW, what is relation, what are possible values
>> * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but
>> it is fundamental. So basics of integer-N and fractional-N PLL is always
>> you must to know.
>> * Filtering. Especially IF filtering, which is generally low-pass
>> filtering. Think possible filters when selecting IF.
> 
> For me, the implementation seems to make sense. I mean, for analog TV, both
> channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard
> only.
> 
> So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it
> may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is
> a channel at VHF or at UHF range.
> 
> So, this part of the patch sounds OK to me.
> 
> The IF is actually a little trickier. Yet, if you take a lok on other
> tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to
> automatically set the IF that will work for each video standard:
> 
> static void tda827x_set_std(struct dvb_frontend *fe,
> 			    struct analog_parameters *params)
> {
> 	struct tda827x_priv *priv = fe->tuner_priv;
> 	char *mode;
> 
> 	priv->lpsel = 0;
> 	if (params->std & V4L2_STD_MN) {
> 		priv->sgIF = 92;
> 		priv->lpsel = 1;
> 		mode = "MN";
> 	} else if (params->std & V4L2_STD_B) {
> 		priv->sgIF = 108;
> 		mode = "B";
> ...
> 
> static int tda827xo_set_analog_params(struct dvb_frontend *fe,
> 				      struct analog_parameters *params)
> {
> 
> ...
> 
> 	N = freq + priv->sgIF;
> 
> In other words, for analog TV, the tuner will always receive the
> channel central frequency, with may vary depending on the video
> standard, and will adjust it to tune at the right channel, using the
> per-standard IF (if needed), as, on most tuner drivers, the tunning
> frequency should be either initial frequency or the main carrier
> frequency, and not the center frequency.

How the carrier central frequency can be vary tings like video standard?
If you tune to some channel like 654.321MHz then that is central 
frequency no matter what kind of bandwidth is used.

And as I pointed those strange offset, please tell me what is that 
1250000 Hz offset? It simply looks nonsense. You first add 1250000 to RF 
frequency, then you compensate same from IF value - the carrier at IF 
will be just same.

Also, there was something like 6/7/8MHz wide channel dropped to IF only 
1.25MHz. How that wide channel could fit that small space?

regards
Antti




-- 
http://palosaari.fi/

Re: [PATCH v3 04/14] si2157: Add analog tuning related functions

[Brad Love]

Hi Mauro,


On 12/19/19 7:13 AM, Mauro Carvalho Chehab wrote:
> Em Sun, 24 Nov 2019 07:09:07 +0200
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> On 11/14/19 10:03 PM, Brad Love wrote:
>>> Include set_analog_params, get_frequency, and get_bandwidth.
>>>
>>> Tested with NTSC and PAL standards via ch3/4 generator. Other standards
>>> are included, but are untested due to lack of generator.
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>> ---
>>> Changes since v1:
>>> - remove __func__ from dev_dbg macros  
>> After all it looks pretty simply, but implementation is not done that 
>> simply. Crazy RF/IF offsets, impossible values and so.
>>
>> I think you need to study some tuner basics:
>> * what IF frequency is, why, and so
>> * IF vs. BW, what is relation, what are possible values
>> * Down conversion RF to IF. OK, *on that case* firmware covers PLL, but 
>> it is fundamental. So basics of integer-N and fractional-N PLL is always 
>> you must to know.
>> * Filtering. Especially IF filtering, which is generally low-pass 
>> filtering. Think possible filters when selecting IF.
> For me, the implementation seems to make sense. I mean, for analog TV, both
> channel bandwidth and chroma/audio sub-carrier IF depends on the TV standard
> only.
>
> So, for NTSC and PAL/M/N/N', bandwidth is always 6MHz. For other standards, it
> may be either 6MHz, 7MHz or 8MHz. the actual bandwidth depends if it is
> a channel at VHF or at UHF range.
>
> So, this part of the patch sounds OK to me.
>
> The IF is actually a little trickier. Yet, if you take a lok on other
> tuners, like drivers/media/tuners/tda827x.c, it is up tot he tuner to
> automatically set the IF that will work for each video standard:
>
> static void tda827x_set_std(struct dvb_frontend *fe,
> 			    struct analog_parameters *params)
> {
> 	struct tda827x_priv *priv = fe->tuner_priv;
> 	char *mode;
>
> 	priv->lpsel = 0;
> 	if (params->std & V4L2_STD_MN) {
> 		priv->sgIF = 92;
> 		priv->lpsel = 1;
> 		mode = "MN";
> 	} else if (params->std & V4L2_STD_B) {
> 		priv->sgIF = 108;
> 		mode = "B";
> ...
>
> static int tda827xo_set_analog_params(struct dvb_frontend *fe,
> 				      struct analog_parameters *params)
> {
>
> ...
>
> 	N = freq + priv->sgIF;
>
> In other words, for analog TV, the tuner will always receive the 
> channel central frequency, with may vary depending on the video
> standard, and will adjust it to tune at the right channel, using the
> per-standard IF (if needed), as, on most tuner drivers, the tunning
> frequency should be either initial frequency or the main carrier
> frequency, and not the center frequency.
>
>
> Cheers,
> Mauro


This code has been widely tested by multiple Hauppauge customers across
North America and Europe. This code has been in use by various parties
for years, with zero issues reported. I am merely working to upstream
all of the code we have generated over the years.

It took a while for me to get tester and test bench time, but included
below is results from using an analog generator and testing channels
across the entire frequency range for ATSC and PAL-I analog TV channels.
First lock and signal strength were verified, then video and audio
signal decoding was verified. Decoding is flawless on every channel tried.

Software used for validation is scantv, tvtime, vlc, and qv4l2.

Cheers,

Brad


$ #########################
$ # USA ATSC TESTING
$ #########################
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
5    ( 77.25 MHz): ???
[unknown (5)]
channel = 5

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
10   (193.25 MHz): ???
[unknown (10)]
channel = 10

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
17   (489.25 MHz): ???
[unknown (17)]
channel = 17

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
30   (567.25 MHz): ???
[unknown (30)]
channel = 30

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
40   (627.25 MHz): ???
[unknown (40)]
channel = 40

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
50   (687.25 MHz): ???
[unknown (50)]
channel = 50

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
60   (747.25 MHz): ???
[unknown (60)]
channel = 60

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
70   (807.25 MHz): ???
[unknown (70)]
channel = 70

<...>
$
$ scantv -n NTSC-M -f us-bcast
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = us-bcast

[defaults]
input = Television
norm = NTSC-M


scanning channel list us-bcast...
<...>
83   (885.25 MHz): ???
[unknown (83)]
channel = 83

$ #########################
$ # EUROPEAN PAL-I TESTING
$ #########################
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
21   (471.25 MHz): ???
[unknown (21)]
channel = 21

<...>
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
30   (543.25 MHz): ???
[unknown (30)]
channel = 30

<...>
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
40   (623.25 MHz): ???
[unknown (40)]
channel = 40

<...>
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
50   (703.25 MHz): ???
[unknown (50)]
channel = 50

<...>
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
60   (783.25 MHz): ???
[unknown (60)]
channel = 60

<...>
$
$ scantv -n PAL-I -f europe-west
vid-open-auto: using analog TV device /dev/video0
[global]
freqtab = europe-west

[defaults]
input = Television
norm = PAL-I


scanning channel list europe-west...
<...>
68   (847.25 MHz): ???
[unknown (68)]
channel = 68

<...>
$
$
$

[PATCH v3 05/14] si2157: Briefly wait for tuning operation to complete

[Brad Love]

To detect errors in the tuning operation, this waits up 40ms for operation
completion status. This allows for error detection and prevents issuing
additional commands to the tuner before it is finished.

Tuning typically completes in 20-30ms.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Split into two patches, tune completion and signal lock

 drivers/media/tuners/si2157.c | 52 +++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 78a855df30da..cac4870017f5 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -301,6 +301,54 @@ static int si2157_sleep(struct dvb_frontend *fe)
 	return ret;
 }
 
+static int si2157_tune_wait(struct i2c_client *client)
+{
+#define TUN_TIMEOUT 40
+	struct si2157_dev *dev = i2c_get_clientdata(client);
+	int ret;
+	unsigned long timeout;
+	unsigned long start_time;
+	u8 wait_status;
+
+	mutex_lock(&dev->i2c_mutex);
+
+	/* wait tuner command complete */
+	start_time = jiffies;
+	timeout = start_time + msecs_to_jiffies(TUN_TIMEOUT);
+	while (!time_after(jiffies, timeout)) {
+		ret = i2c_master_recv(client, &wait_status,
+				      sizeof(wait_status));
+		if (ret < 0) {
+			goto err_mutex_unlock;
+		} else if (ret != sizeof(wait_status)) {
+			ret = -EREMOTEIO;
+			goto err_mutex_unlock;
+		}
+
+		/* tuner done? */
+		if ((wait_status & 0x81) == 0x81)
+			break;
+		usleep_range(5000, 10000);
+	}
+
+	dev_dbg(&client->dev, "tuning took %d ms, status=0x%x\n",
+		jiffies_to_msecs(jiffies) - jiffies_to_msecs(start_time),
+		wait_status);
+
+	if ((wait_status & 0xc0) != 0x80) {
+		ret = -ETIMEDOUT;
+		goto err_mutex_unlock;
+	}
+
+	mutex_unlock(&dev->i2c_mutex);
+	return 0;
+
+err_mutex_unlock:
+	mutex_unlock(&dev->i2c_mutex);
+	dev_err(&client->dev, "failed=%d\n", ret);
+	return ret;
+}
+
 static int si2157_set_params(struct dvb_frontend *fe)
 {
 	struct i2c_client *client = fe->tuner_priv;
@@ -400,6 +448,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	dev->bandwidth = bandwidth;
 	dev->frequency = c->frequency;
 
+	si2157_tune_wait(client); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
@@ -594,6 +644,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
 
 	dev->bandwidth = bandwidth;
 
+	si2157_tune_wait(client); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
-- 
2.23.0

Re: [PATCH v3 05/14] si2157: Briefly wait for tuning operation to complete

[Antti Palosaari]

On 11/14/19 10:03 PM, Brad Love wrote:
> To detect errors in the tuning operation, this waits up 40ms for operation
> completion status. This allows for error detection and prevents issuing
> additional commands to the tuner before it is finished.
> 
> Tuning typically completes in 20-30ms.

So that adds some magic waiting at the end of both digital and analog 
tuning.

For digital side it surely is not needed, it has been ages without. 
Tuning on digital side works almost always on 3 phases, 1) program 
tuner, 2) program demod, 3) start waiting demod lock. Tuner is generally 
ready much faster than demod, it is usually just programming PLL and PLL 
locks very fast if it is inside supported frequency ranges and outside 
ranges it does not lock at all. Some tuners supports PLL lock reading, 
but generally there is no idea to read it on normal operation. So 
all-in-all, tuner is almost certainly first device which is ready on 
that chain and if something fails dvb-frontend does retuning.

For analog stuff you added I am not very familiar. Maybe you should 
check analog demod status somehow, is there such callback to read status 
as digital side is..?

Antti

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Since v2:
> - Split into two patches, tune completion and signal lock
> 
>   drivers/media/tuners/si2157.c | 52 +++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 78a855df30da..cac4870017f5 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -301,6 +301,54 @@ static int si2157_sleep(struct dvb_frontend *fe)
>   	return ret;
>   }
>   
> +static int si2157_tune_wait(struct i2c_client *client)
> +{
> +#define TUN_TIMEOUT 40
> +	struct si2157_dev *dev = i2c_get_clientdata(client);
> +	int ret;
> +	unsigned long timeout;
> +	unsigned long start_time;
> +	u8 wait_status;
> +
> +	mutex_lock(&dev->i2c_mutex);
> +
> +	/* wait tuner command complete */
> +	start_time = jiffies;
> +	timeout = start_time + msecs_to_jiffies(TUN_TIMEOUT);
> +	while (!time_after(jiffies, timeout)) {
> +		ret = i2c_master_recv(client, &wait_status,
> +				      sizeof(wait_status));
> +		if (ret < 0) {
> +			goto err_mutex_unlock;
> +		} else if (ret != sizeof(wait_status)) {
> +			ret = -EREMOTEIO;
> +			goto err_mutex_unlock;
> +		}
> +
> +		/* tuner done? */
> +		if ((wait_status & 0x81) == 0x81)
> +			break;
> +		usleep_range(5000, 10000);
> +	}
> +
> +	dev_dbg(&client->dev, "tuning took %d ms, status=0x%x\n",
> +		jiffies_to_msecs(jiffies) - jiffies_to_msecs(start_time),
> +		wait_status);
> +
> +	if ((wait_status & 0xc0) != 0x80) {
> +		ret = -ETIMEDOUT;
> +		goto err_mutex_unlock;
> +	}
> +
> +	mutex_unlock(&dev->i2c_mutex);
> +	return 0;
> +
> +err_mutex_unlock:
> +	mutex_unlock(&dev->i2c_mutex);
> +	dev_err(&client->dev, "failed=%d\n", ret);
> +	return ret;
> +}
> +
>   static int si2157_set_params(struct dvb_frontend *fe)
>   {
>   	struct i2c_client *client = fe->tuner_priv;
> @@ -400,6 +448,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
>   	dev->bandwidth = bandwidth;
>   	dev->frequency = c->frequency;
>   
> +	si2157_tune_wait(client); /* wait to complete, ignore any errors */
> +
>   	return 0;
>   err:
>   	dev->bandwidth = 0;
> @@ -594,6 +644,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
>   
>   	dev->bandwidth = bandwidth;
>   
> +	si2157_tune_wait(client); /* wait to complete, ignore any errors */
> +
>   	return 0;
>   err:
>   	dev->bandwidth = 0;
> 

-- 
http://palosaari.fi/

[PATCH v3 06/14] si2157: module debug option to wait on signal lock

[Brad Love]

In some debugging cases it is useful to know how long it took
signal lock to happen after tuning. This can help diagnose
line issues.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Split into two parts, tune completion and signal lock

 drivers/media/tuners/si2157.c | 46 ++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index cac4870017f5..16d7169f46fb 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -9,6 +9,10 @@
 
 static const struct dvb_tuner_ops si2157_ops;
 
+static int tuner_lock_debug;
+module_param(tuner_lock_debug, int, 0644);
+MODULE_PARM_DESC(tuner_lock_debug, "if set, signal lock is briefly waited on after setting params");
+
 /* execute firmware command */
 static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
 {
@@ -301,14 +305,22 @@ static int si2157_sleep(struct dvb_frontend *fe)
 	return ret;
 }
 
-static int si2157_tune_wait(struct i2c_client *client)
+static int si2157_tune_wait(struct i2c_client *client, u8 is_digital)
 {
 #define TUN_TIMEOUT 40
+#define DIG_TIMEOUT 30
+#define ANALOG_TIMEOUT 150
 	struct si2157_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	unsigned long timeout;
 	unsigned long start_time;
 	u8 wait_status;
+	u8  tune_lock_mask;
+
+	if (is_digital)
+		tune_lock_mask = 0x04;
+	else
+		tune_lock_mask = 0x02;
 
 	mutex_lock(&dev->i2c_mutex);
 
@@ -335,6 +347,34 @@ static int si2157_tune_wait(struct i2c_client *client)
 		jiffies_to_msecs(jiffies) - jiffies_to_msecs(start_time),
 		wait_status);
 
+	/* if we tuned ok, wait a bit for tuner lock */
+	if (tuner_lock_debug && (wait_status & 0x81) == 0x81) {
+		if (is_digital)
+			timeout = jiffies + msecs_to_jiffies(DIG_TIMEOUT);
+		else
+			timeout = jiffies + msecs_to_jiffies(ANALOG_TIMEOUT);
+
+		while (!time_after(jiffies, timeout)) {
+			ret = i2c_master_recv(client, &wait_status,
+					      sizeof(wait_status));
+			if (ret < 0) {
+				goto err_mutex_unlock;
+			} else if (ret != sizeof(wait_status)) {
+				ret = -EREMOTEIO;
+				goto err_mutex_unlock;
+			}
+
+			/* tuner locked? */
+			if (wait_status & tune_lock_mask)
+				break;
+			usleep_range(5000, 10000);
+		}
+
+		dev_dbg(&client->dev, "tuning+lock took %d ms, status=0x%x\n",
+			jiffies_to_msecs(jiffies) - jiffies_to_msecs(start_time),
+			wait_status);
+	}
+
 	if ((wait_status & 0xc0) != 0x80) {
 		ret = -ETIMEDOUT;
 		goto err_mutex_unlock;
@@ -448,7 +488,7 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	dev->bandwidth = bandwidth;
 	dev->frequency = c->frequency;
 
-	si2157_tune_wait(client); /* wait to complete, ignore any errors */
+	si2157_tune_wait(client, 1); /* wait to complete, ignore any errors */
 
 	return 0;
 err:
@@ -644,7 +684,7 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
 
 	dev->bandwidth = bandwidth;
 
-	si2157_tune_wait(client); /* wait to complete, ignore any errors */
+	si2157_tune_wait(client, 0); /* wait to complete, ignore any errors */
 
 	return 0;
 err:
-- 
2.23.0

[PATCH v3 07/14] cx23885: Add analog frontend to Hauppauge QuadHD

[Brad Love]

Add analog tuner frontend to 888 Hauppauge QuadHD boards

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Remove force_bff from ATSC QuadHD

 drivers/media/pci/cx23885/cx23885-cards.c | 31 ++++++++++++++++++++---
 drivers/media/pci/cx23885/cx23885-dvb.c   | 20 +++++++++++++++
 drivers/media/pci/cx23885/cx23885-video.c |  8 +++++-
 3 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 8644205d3cd3..95c9c7f74ba8 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -757,24 +757,47 @@ struct cx23885_board cx23885_boards[] = {
 		} },
 	},
 	[CX23885_BOARD_HAUPPAUGE_QUADHD_DVB] = {
-		.name        = "Hauppauge WinTV-QuadHD-DVB",
+		.name         = "Hauppauge WinTV-QuadHD-DVB",
+		.porta        = CX23885_ANALOG_VIDEO,
 		.portb        = CX23885_MPEG_DVB,
 		.portc        = CX23885_MPEG_DVB,
+		.tuner_type	= TUNER_ABSENT,
+		.force_bff	= 1,
+		.input          = {{
+			.type   = CX23885_VMUX_TELEVISION,
+			.vmux   =	CX25840_VIN7_CH3 |
+					CX25840_VIN5_CH2 |
+					CX25840_VIN2_CH1 |
+					CX25840_DIF_ON,
+			.amux   = CX25840_AUDIO8,
+		} },
 	},
 	[CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885] = {
-		.name       = "Hauppauge WinTV-QuadHD-DVB(885)",
+		.name         = "Hauppauge WinTV-QuadHD-DVB(885)",
 		.portb        = CX23885_MPEG_DVB,
 		.portc        = CX23885_MPEG_DVB,
+		.tuner_type   = TUNER_ABSENT,
 	},
 	[CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC] = {
-		.name        = "Hauppauge WinTV-QuadHD-ATSC",
+		.name         = "Hauppauge WinTV-QuadHD-ATSC",
+		.porta        = CX23885_ANALOG_VIDEO,
 		.portb        = CX23885_MPEG_DVB,
 		.portc        = CX23885_MPEG_DVB,
+		.tuner_type	= TUNER_ABSENT,
+		.input          = {{
+			.type   = CX23885_VMUX_TELEVISION,
+			.vmux   =	CX25840_VIN7_CH3 |
+					CX25840_VIN5_CH2 |
+					CX25840_VIN2_CH1 |
+					CX25840_DIF_ON,
+			.amux   = CX25840_AUDIO8,
+		} },
 	},
 	[CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885] = {
-		.name       = "Hauppauge WinTV-QuadHD-ATSC(885)",
+		.name         = "Hauppauge WinTV-QuadHD-ATSC(885)",
 		.portb        = CX23885_MPEG_DVB,
 		.portc        = CX23885_MPEG_DVB,
+		.tuner_type   = TUNER_ABSENT,
 	},
 	[CX23885_BOARD_HAUPPAUGE_HVR1265_K4] = {
 		.name		= "Hauppauge WinTV-HVR-1265(161111)",
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 4f386db33a11..590af4656828 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2373,6 +2373,16 @@ static int dvb_register(struct cx23885_tsport *port)
 				goto frontend_detach;
 			}
 			port->i2c_client_tuner = client_tuner;
+
+			/* we only attach tuner for analog on the 888 version */
+			if (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_DVB) {
+				pr_info("%s(): QUADHD_DVB analog setup\n",
+					__func__);
+				dev->ts1.analog_fe.tuner_priv = client_tuner;
+				memcpy(&dev->ts1.analog_fe.ops.tuner_ops,
+				       &fe0->dvb.frontend->ops.tuner_ops,
+				       sizeof(struct dvb_tuner_ops));
+			}
 			break;
 
 		/* port c - terrestrial/cable */
@@ -2462,6 +2472,16 @@ static int dvb_register(struct cx23885_tsport *port)
 				goto frontend_detach;
 			}
 			port->i2c_client_tuner = client_tuner;
+
+			/* we only attach tuner for analog on the 888 version */
+			if (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC) {
+				pr_info("%s(): QUADHD_ATSC analog setup\n",
+					__func__);
+				dev->ts1.analog_fe.tuner_priv = client_tuner;
+				memcpy(&dev->ts1.analog_fe.ops.tuner_ops,
+				       &fe0->dvb.frontend->ops.tuner_ops,
+				       sizeof(struct dvb_tuner_ops));
+			}
 			break;
 
 		/* port c - terrestrial/cable */
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 8098b15493de..3331d6833769 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -253,6 +253,8 @@ static int cx23885_video_mux(struct cx23885_dev *dev, unsigned int input)
 		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255) ||
 		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255_22111) ||
 		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1265_K4) ||
+		(dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC) ||
+		(dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_DVB) ||
 		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1850) ||
 		(dev->board == CX23885_BOARD_MYGICA_X8507) ||
 		(dev->board == CX23885_BOARD_AVERMEDIA_HC81R) ||
@@ -995,7 +997,9 @@ static int cx23885_set_freq_via_ops(struct cx23885_dev *dev,
 	if ((dev->board == CX23885_BOARD_HAUPPAUGE_HVR1850) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255_22111) ||
-	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1265_K4))
+	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1265_K4) ||
+	    (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_DVB) ||
+	    (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC))
 		fe = &dev->ts1.analog_fe;
 
 	if (fe && fe->ops.tuner_ops.set_analog_params) {
@@ -1026,6 +1030,8 @@ int cx23885_set_frequency(struct file *file, void *priv,
 	case CX23885_BOARD_HAUPPAUGE_HVR1255_22111:
 	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
 	case CX23885_BOARD_HAUPPAUGE_HVR1850:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		ret = cx23885_set_freq_via_ops(dev, f);
 		break;
 	default:
-- 
2.23.0

[PATCH v3 08/14] cx23885: Add analog frontend to 1265_K4

[Brad Love]

Enables the analog tuning frontend for Hauppauge HVR-1265_K4.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Remove force_bff from ATSC 1264_K4

 drivers/media/pci/cx23885/cx23885-cards.c | 8 +++++++-
 drivers/media/pci/cx23885/cx23885-dvb.c   | 5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 95c9c7f74ba8..916fa8987f8b 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -804,8 +804,14 @@ struct cx23885_board cx23885_boards[] = {
 		.porta          = CX23885_ANALOG_VIDEO,
 		.portc		= CX23885_MPEG_DVB,
 		.tuner_type     = TUNER_ABSENT,
-		.force_bff	= 1,
 		.input          = {{
+			.type   = CX23885_VMUX_TELEVISION,
+			.vmux   =	CX25840_VIN7_CH3 |
+					CX25840_VIN5_CH2 |
+					CX25840_VIN2_CH1 |
+					CX25840_DIF_ON,
+			.amux   = CX25840_AUDIO8,
+		}, {
 			.type   = CX23885_VMUX_COMPOSITE1,
 			.vmux   =	CX25840_VIN7_CH3 |
 					CX25840_VIN4_CH2 |
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 590af4656828..37dc13b052a3 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2553,6 +2553,11 @@ static int dvb_register(struct cx23885_tsport *port)
 				goto frontend_detach;
 			}
 			port->i2c_client_tuner = client_tuner;
+
+			dev->ts1.analog_fe.tuner_priv = client_tuner;
+			memcpy(&dev->ts1.analog_fe.ops.tuner_ops,
+			       &fe0->dvb.frontend->ops.tuner_ops,
+			       sizeof(struct dvb_tuner_ops));
 			break;
 		}
 		break;
-- 
2.23.0

[PATCH v3 09/14] cx23885: Add analog frontend to HVR5525

[Brad Love]

Enables the analog tuning frontend for Hauppauge HVR-5525.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
No changes

 drivers/media/pci/cx23885/cx23885-cards.c | 12 ++++++++++++
 drivers/media/pci/cx23885/cx23885-dvb.c   |  6 ++++++
 drivers/media/pci/cx23885/cx23885-video.c |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 916fa8987f8b..aa4b1150a860 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -703,8 +703,19 @@ struct cx23885_board cx23885_boards[] = {
 	},
 	[CX23885_BOARD_HAUPPAUGE_HVR5525] = {
 		.name		= "Hauppauge WinTV-HVR5525",
+		.porta		= CX23885_ANALOG_VIDEO,
 		.portb		= CX23885_MPEG_DVB,
 		.portc		= CX23885_MPEG_DVB,
+		.tuner_type	= TUNER_ABSENT,
+		.force_bff	= 1,
+		.input		= {{
+			.type	= CX23885_VMUX_TELEVISION,
+			.vmux	=	CX25840_VIN7_CH3 |
+					CX25840_VIN5_CH2 |
+					CX25840_VIN2_CH1 |
+					CX25840_DIF_ON,
+			.amux   = CX25840_AUDIO8,
+		} },
 	},
 	[CX23885_BOARD_VIEWCAST_260E] = {
 		.name		= "ViewCast 260e",
@@ -2356,6 +2367,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
 	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 	case CX23885_BOARD_HAUPPAUGE_HVR1270:
 	case CX23885_BOARD_HAUPPAUGE_HVR1850:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
 	case CX23885_BOARD_MYGICA_X8506:
 	case CX23885_BOARD_MAGICPRO_PROHDTVE2:
 	case CX23885_BOARD_HAUPPAUGE_HVR1290:
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 37dc13b052a3..e8812589a6a3 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2320,6 +2320,12 @@ static int dvb_register(struct cx23885_tsport *port)
 				goto frontend_detach;
 			}
 			port->i2c_client_tuner = client_tuner;
+
+			dev->ts1.analog_fe.tuner_priv = client_tuner;
+			memcpy(&dev->ts1.analog_fe.ops.tuner_ops,
+			       &fe0->dvb.frontend->ops.tuner_ops,
+			       sizeof(struct dvb_tuner_ops));
+
 			break;
 		}
 		break;
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 3331d6833769..e2062a0c4cf7 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -256,6 +256,7 @@ static int cx23885_video_mux(struct cx23885_dev *dev, unsigned int input)
 		(dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC) ||
 		(dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_DVB) ||
 		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1850) ||
+		(dev->board == CX23885_BOARD_HAUPPAUGE_HVR5525) ||
 		(dev->board == CX23885_BOARD_MYGICA_X8507) ||
 		(dev->board == CX23885_BOARD_AVERMEDIA_HC81R) ||
 		(dev->board == CX23885_BOARD_VIEWCAST_260E) ||
@@ -998,6 +999,7 @@ static int cx23885_set_freq_via_ops(struct cx23885_dev *dev,
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255_22111) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR1265_K4) ||
+	    (dev->board == CX23885_BOARD_HAUPPAUGE_HVR5525) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_DVB) ||
 	    (dev->board == CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC))
 		fe = &dev->ts1.analog_fe;
@@ -1030,6 +1032,7 @@ int cx23885_set_frequency(struct file *file, void *priv,
 	case CX23885_BOARD_HAUPPAUGE_HVR1255_22111:
 	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
 	case CX23885_BOARD_HAUPPAUGE_HVR1850:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
 	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
 	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		ret = cx23885_set_freq_via_ops(dev, f);
-- 
2.23.0

[PATCH v3 10/14] cx23885: Add i2c device analog tuner support

[Brad Love]

Hauppauge QuadHD/1265/5525 boards all use i2c device drivers and
have tuner_type equal TUNER_ABSENT. This means additional support
is required to enable the analog tuning capability, a case
statement is used to identify these models.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Fix capabilities and device caps

 drivers/media/pci/cx23885/cx23885-video.c | 87 +++++++++++++++++++----
 1 file changed, 75 insertions(+), 12 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index e2062a0c4cf7..feb083600b1c 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -638,8 +638,18 @@ static int vidioc_querycap(struct file *file, void  *priv,
 			    V4L2_CAP_AUDIO | V4L2_CAP_VBI_CAPTURE |
 			    V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VBI_CAPTURE |
 			    V4L2_CAP_DEVICE_CAPS;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		cap->capabilities |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->capabilities |= V4L2_CAP_TUNER;
+		break;
+	}
 	return 0;
 }
 
@@ -885,8 +895,17 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 {
 	struct cx23885_dev *dev = video_drvdata(file);
 
-	if (dev->tuner_type == TUNER_ABSENT)
-		return -EINVAL;
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+		break;
+	default:
+		if (dev->tuner_type == TUNER_ABSENT)
+			return -EINVAL;
+		break;
+	}
 	if (0 != t->index)
 		return -EINVAL;
 
@@ -901,8 +920,17 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 {
 	struct cx23885_dev *dev = video_drvdata(file);
 
-	if (dev->tuner_type == TUNER_ABSENT)
-		return -EINVAL;
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+		break;
+	default:
+		if (dev->tuner_type == TUNER_ABSENT)
+			return -EINVAL;
+		break;
+	}
 	if (0 != t->index)
 		return -EINVAL;
 	/* Update the A/V core */
@@ -916,9 +944,17 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 {
 	struct cx23885_dev *dev = video_drvdata(file);
 
-	if (dev->tuner_type == TUNER_ABSENT)
-		return -EINVAL;
-
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+		break;
+	default:
+		if (dev->tuner_type == TUNER_ABSENT)
+			return -EINVAL;
+		break;
+	}
 	f->type = V4L2_TUNER_ANALOG_TV;
 	f->frequency = dev->freq;
 
@@ -932,8 +968,17 @@ static int cx23885_set_freq(struct cx23885_dev *dev, const struct v4l2_frequency
 	struct v4l2_ctrl *mute;
 	int old_mute_val = 1;
 
-	if (dev->tuner_type == TUNER_ABSENT)
-		return -EINVAL;
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+		break;
+	default:
+		if (dev->tuner_type == TUNER_ABSENT)
+			return -EINVAL;
+		break;
+	}
 	if (unlikely(f->tuner != 0))
 		return -EINVAL;
 
@@ -1310,8 +1355,17 @@ int cx23885_video_register(struct cx23885_dev *dev)
 	dev->video_dev->queue = &dev->vb2_vidq;
 	dev->video_dev->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
 				      V4L2_CAP_AUDIO | V4L2_CAP_VIDEO_CAPTURE;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		dev->video_dev->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			dev->video_dev->device_caps |= V4L2_CAP_TUNER;
+	}
 	err = video_register_device(dev->video_dev, VFL_TYPE_GRABBER,
 				    video_nr[dev->nr]);
 	if (err < 0) {
@@ -1328,8 +1382,17 @@ int cx23885_video_register(struct cx23885_dev *dev)
 	dev->vbi_dev->queue = &dev->vb2_vbiq;
 	dev->vbi_dev->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
 				    V4L2_CAP_AUDIO | V4L2_CAP_VBI_CAPTURE;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_HVR5525:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		dev->vbi_dev->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			dev->vbi_dev->device_caps |= V4L2_CAP_TUNER;
+	}
 	err = video_register_device(dev->vbi_dev, VFL_TYPE_VBI,
 				    vbi_nr[dev->nr]);
 	if (err < 0) {
-- 
2.23.0

[PATCH v3 11/14] cx231xx: Add i2c device analog tuner support

[Brad Love]

The boards listed below use i2c device drivers and have
tuner_type equal TUNER_ABSENT. This means additional support
is required to enable the analog tuning capability, a case
statement is used to identify these models.

Models with analog tuning enabled:
- CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx (tested)
- CX231XX_BOARD_HAUPPAUGE_935C (tested)
- CX231XX_BOARD_HAUPPAUGE_955Q (tested)
- CX231XX_BOARD_HAUPPAUGE_975 (tested)
- CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD (untested)

The EvroMedia model was added, since it uses the si2157
tuner and the board profile claims it has analog inputs.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- Fix capabilities and device caps
Changes since v1:
- remove __func__ from dev_dbg macros

 drivers/media/usb/cx231xx/cx231xx-avcore.c | 35 +++++++--
 drivers/media/usb/cx231xx/cx231xx-video.c  | 85 ++++++++++++++++++----
 2 files changed, 101 insertions(+), 19 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-avcore.c b/drivers/media/usb/cx231xx/cx231xx-avcore.c
index 0974965e848f..d6117093491b 100644
--- a/drivers/media/usb/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/usb/cx231xx/cx231xx-avcore.c
@@ -587,14 +587,27 @@ int cx231xx_set_video_input_mux(struct cx231xx *dev, u8 input)
 				return status;
 			}
 		}
-		if (dev->tuner_type == TUNER_NXP_TDA18271)
+		switch (dev->model) { /* i2c device tuners */
+		case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+		case CX231XX_BOARD_HAUPPAUGE_935C:
+		case CX231XX_BOARD_HAUPPAUGE_955Q:
+		case CX231XX_BOARD_HAUPPAUGE_975:
+		case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
 			status = cx231xx_set_decoder_video_input(dev,
 							CX231XX_VMUX_TELEVISION,
 							INPUT(input)->vmux);
-		else
-			status = cx231xx_set_decoder_video_input(dev,
+			break;
+		default:
+			if (dev->tuner_type == TUNER_NXP_TDA18271)
+				status = cx231xx_set_decoder_video_input(dev,
+							CX231XX_VMUX_TELEVISION,
+							INPUT(input)->vmux);
+			else
+				status = cx231xx_set_decoder_video_input(dev,
 							CX231XX_VMUX_COMPOSITE1,
 							INPUT(input)->vmux);
+			break;
+		};
 
 		break;
 	default:
@@ -1193,12 +1206,22 @@ int cx231xx_set_audio_decoder_input(struct cx231xx *dev,
 					cx231xx_set_field(FLD_SIF_EN, 0));
 			break;
 		default:
+			switch (dev->model) { /* i2c device tuners */
+			case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+			case CX231XX_BOARD_HAUPPAUGE_935C:
+			case CX231XX_BOARD_HAUPPAUGE_955Q:
+			case CX231XX_BOARD_HAUPPAUGE_975:
+			case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+			/* TODO: Normal mode: SIF passthrough at 14.32 MHz?? */
+				break;
+			default:
 			/* This is just a casual suggestion to people adding
 			   new boards in case they use a tuner type we don't
 			   currently know about */
-			dev_info(dev->dev,
-				 "Unknown tuner type configuring SIF");
-			break;
+				dev_info(dev->dev,
+					 "Unknown tuner type configuring SIF");
+				break;
+			}
 		}
 		break;
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 69abafaebbf3..e6b89706a4df 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1129,7 +1129,7 @@ int cx231xx_s_frequency(struct file *file, void *priv,
 {
 	struct cx231xx *dev = video_drvdata(file);
 	struct v4l2_frequency new_freq = *f;
-	int rc;
+	int rc, need_if_freq = 0;
 	u32 if_frequency = 5400000;
 
 	dev_dbg(dev->dev,
@@ -1142,14 +1142,30 @@ int cx231xx_s_frequency(struct file *file, void *priv,
 	/* set pre channel change settings in DIF first */
 	rc = cx231xx_tuner_pre_channel_change(dev);
 
-	call_all(dev, tuner, s_frequency, f);
-	call_all(dev, tuner, g_frequency, &new_freq);
-	dev->ctl_freq = new_freq.frequency;
+	switch (dev->model) { /* i2c device tuners */
+	case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+	case CX231XX_BOARD_HAUPPAUGE_935C:
+	case CX231XX_BOARD_HAUPPAUGE_955Q:
+	case CX231XX_BOARD_HAUPPAUGE_975:
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+		if (dev->cx231xx_set_analog_freq)
+			dev->cx231xx_set_analog_freq(dev, f->frequency);
+		dev->ctl_freq = f->frequency;
+		need_if_freq = 1;
+		break;
+	default:
+		call_all(dev, tuner, s_frequency, f);
+		call_all(dev, tuner, g_frequency, &new_freq);
+		dev->ctl_freq = new_freq.frequency;
+		break;
+	}
+
+	pr_debug("%s() %u  :  %u\n", __func__, f->frequency, dev->ctl_freq);
 
 	/* set post channel change settings in DIF first */
 	rc = cx231xx_tuner_post_channel_change(dev);
 
-	if (dev->tuner_type == TUNER_NXP_TDA18271) {
+	if (need_if_freq || dev->tuner_type == TUNER_NXP_TDA18271) {
 		if (dev->norm & (V4L2_STD_MN | V4L2_STD_NTSC_443))
 			if_frequency = 5400000;  /*5.4MHz	*/
 		else if (dev->norm & V4L2_STD_B)
@@ -1362,9 +1378,20 @@ int cx231xx_querycap(struct file *file, void *priv,
 		V4L2_CAP_STREAMING | V4L2_CAP_DEVICE_CAPS;
 	if (video_is_registered(&dev->radio_dev))
 		cap->capabilities |= V4L2_CAP_RADIO;
-	if (dev->tuner_type != TUNER_ABSENT)
-		cap->capabilities |= V4L2_CAP_TUNER;
 
+	switch (dev->model) {
+	case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+	case CX231XX_BOARD_HAUPPAUGE_935C:
+	case CX231XX_BOARD_HAUPPAUGE_955Q:
+	case CX231XX_BOARD_HAUPPAUGE_975:
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+		cap->capabilities |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->capabilities |= V4L2_CAP_TUNER;
+		break;
+	}
 	return 0;
 }
 
@@ -1708,10 +1735,20 @@ static void cx231xx_vdev_init(struct cx231xx *dev,
 
 	video_set_drvdata(vfd, dev);
 	if (dev->tuner_type == TUNER_ABSENT) {
-		v4l2_disable_ioctl(vfd, VIDIOC_G_FREQUENCY);
-		v4l2_disable_ioctl(vfd, VIDIOC_S_FREQUENCY);
-		v4l2_disable_ioctl(vfd, VIDIOC_G_TUNER);
-		v4l2_disable_ioctl(vfd, VIDIOC_S_TUNER);
+		switch (dev->model) {
+		case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+		case CX231XX_BOARD_HAUPPAUGE_935C:
+		case CX231XX_BOARD_HAUPPAUGE_955Q:
+		case CX231XX_BOARD_HAUPPAUGE_975:
+		case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+			break;
+		default:
+			v4l2_disable_ioctl(vfd, VIDIOC_G_FREQUENCY);
+			v4l2_disable_ioctl(vfd, VIDIOC_S_FREQUENCY);
+			v4l2_disable_ioctl(vfd, VIDIOC_G_TUNER);
+			v4l2_disable_ioctl(vfd, VIDIOC_S_TUNER);
+			break;
+		}
 	}
 }
 
@@ -1781,8 +1818,20 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 	dev->vdev.queue = q;
 	dev->vdev.device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
 				V4L2_CAP_VIDEO_CAPTURE;
-	if (dev->tuner_type != TUNER_ABSENT)
+
+	switch (dev->model) { /* i2c device tuners */
+	case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+	case CX231XX_BOARD_HAUPPAUGE_935C:
+	case CX231XX_BOARD_HAUPPAUGE_955Q:
+	case CX231XX_BOARD_HAUPPAUGE_975:
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
 		dev->vdev.device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			dev->vdev.device_caps |= V4L2_CAP_TUNER;
+		break;
+	}
 
 	/* register v4l2 video video_device */
 	ret = video_register_device(&dev->vdev, VFL_TYPE_GRABBER,
@@ -1829,8 +1878,18 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 	dev->vbi_dev.queue = q;
 	dev->vbi_dev.device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING |
 				   V4L2_CAP_VBI_CAPTURE;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->model) { /* i2c device tuners */
+	case CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx:
+	case CX231XX_BOARD_HAUPPAUGE_935C:
+	case CX231XX_BOARD_HAUPPAUGE_955Q:
+	case CX231XX_BOARD_HAUPPAUGE_975:
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
 		dev->vbi_dev.device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			dev->vbi_dev.device_caps |= V4L2_CAP_TUNER;
+	}
 
 	/* register v4l2 vbi video_device */
 	ret = video_register_device(&dev->vbi_dev, VFL_TYPE_VBI,
-- 
2.23.0

[PATCH v3 12/14] si2157: add on-demand rf strength func

[Brad Love]

Add get_rf_strength callback to get RSSI from the tuner. DVBv5
stat cache is updated. get_rf_strength is called by tuner_core
for analog tuners and is also used by some bridge drivers to
obtain RSSI directly from the tuner.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v1:
- simplify normalization of signal strength calculation
- use clamp and add description of operation
- remove __func__ from dev_dbg macro

 drivers/media/tuners/si2157.c | 40 ++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 16d7169f46fb..aaf223b9b781 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -725,6 +725,42 @@ static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
 	return 0;
 }
 
+static int si2157_get_rf_strength(struct dvb_frontend *fe, u16 *rssi)
+{
+	struct i2c_client *client = fe->tuner_priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct si2157_cmd cmd;
+	int ret;
+	int strength;
+
+	dev_dbg(&client->dev, "\n");
+
+	memcpy(cmd.args, "\x42\x00", 2);
+	cmd.wlen = 2;
+	cmd.rlen = 12;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	c->strength.stat[0].scale = FE_SCALE_DECIBEL;
+	c->strength.stat[0].svalue = (s8)cmd.args[3] * 1000;
+
+	/* normalize values based on Silicon Labs reference
+	 * add 100, then anything > 80 is 100% signal
+	 */
+	strength = (s8)cmd.args[3] + 100;
+	strength = clamp_val(strength, 0, 80);
+	*rssi = (u16)(strength * 0xffff / 80);
+
+	dev_dbg(&client->dev, "strength=%d rssi=%u\n",
+		(s8)cmd.args[3], *rssi);
+
+	return 0;
+err:
+	dev_dbg(&client->dev, "failed=%d\n", ret);
+	return ret;
+}
+
 static const struct dvb_tuner_ops si2157_ops = {
 	.info = {
 		.name             = "Silicon Labs Si2141/Si2146/2147/2148/2157/2158",
@@ -738,7 +774,9 @@ static const struct dvb_tuner_ops si2157_ops = {
 	.set_analog_params = si2157_set_analog_params,
 	.get_frequency     = si2157_get_frequency,
 	.get_bandwidth     = si2157_get_bandwidth,
-	.get_if_frequency = si2157_get_if_frequency,
+	.get_if_frequency  = si2157_get_if_frequency,
+
+	.get_rf_strength   = si2157_get_rf_strength,
 };
 
 static void si2157_stat_work(struct work_struct *work)
-- 
2.23.0

Re: [PATCH v3 12/14] si2157: add on-demand rf strength func

[Antti Palosaari]

On 11/14/19 10:04 PM, Brad Love wrote:
> Add get_rf_strength callback to get RSSI from the tuner. DVBv5
> stat cache is updated. get_rf_strength is called by tuner_core
> for analog tuners and is also used by some bridge drivers to
> obtain RSSI directly from the tuner.

Driver is updating its signal strength already. Is there any reason you 
don't use existing value but add this new I/O to read it?

Antti

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Changes since v1:
> - simplify normalization of signal strength calculation
> - use clamp and add description of operation
> - remove __func__ from dev_dbg macro
> 
>   drivers/media/tuners/si2157.c | 40 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 16d7169f46fb..aaf223b9b781 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -725,6 +725,42 @@ static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
>   	return 0;
>   }
>   
> +static int si2157_get_rf_strength(struct dvb_frontend *fe, u16 *rssi)
> +{
> +	struct i2c_client *client = fe->tuner_priv;
> +	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	struct si2157_cmd cmd;
> +	int ret;
> +	int strength;
> +
> +	dev_dbg(&client->dev, "\n");
> +
> +	memcpy(cmd.args, "\x42\x00", 2);
> +	cmd.wlen = 2;
> +	cmd.rlen = 12;
> +	ret = si2157_cmd_execute(client, &cmd);
> +	if (ret)
> +		goto err;
> +
> +	c->strength.stat[0].scale = FE_SCALE_DECIBEL;
> +	c->strength.stat[0].svalue = (s8)cmd.args[3] * 1000;
> +
> +	/* normalize values based on Silicon Labs reference
> +	 * add 100, then anything > 80 is 100% signal
> +	 */
> +	strength = (s8)cmd.args[3] + 100;
> +	strength = clamp_val(strength, 0, 80);
> +	*rssi = (u16)(strength * 0xffff / 80);
> +
> +	dev_dbg(&client->dev, "strength=%d rssi=%u\n",
> +		(s8)cmd.args[3], *rssi);
> +
> +	return 0;
> +err:
> +	dev_dbg(&client->dev, "failed=%d\n", ret);
> +	return ret;
> +}
> +
>   static const struct dvb_tuner_ops si2157_ops = {
>   	.info = {
>   		.name             = "Silicon Labs Si2141/Si2146/2147/2148/2157/2158",
> @@ -738,7 +774,9 @@ static const struct dvb_tuner_ops si2157_ops = {
>   	.set_analog_params = si2157_set_analog_params,
>   	.get_frequency     = si2157_get_frequency,
>   	.get_bandwidth     = si2157_get_bandwidth,
> -	.get_if_frequency = si2157_get_if_frequency,
> +	.get_if_frequency  = si2157_get_if_frequency,
> +
> +	.get_rf_strength   = si2157_get_rf_strength,
>   };
>   
>   static void si2157_stat_work(struct work_struct *work)
> 

-- 
http://palosaari.fi/

[PATCH v3 13/14] lgdt3306a: Add CNR v5 stat

[Brad Love]

The CNR is already calculated, so populate DVBv5 CNR stat
during read_status.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
No changes

 drivers/media/dvb-frontends/lgdt3306a.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index 6c4adec58174..d3c330e035c4 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -846,6 +846,7 @@ static int lgdt3306a_fe_sleep(struct dvb_frontend *fe)
 static int lgdt3306a_init(struct dvb_frontend *fe)
 {
 	struct lgdt3306a_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	u8 val;
 	int ret;
 
@@ -997,6 +998,9 @@ static int lgdt3306a_init(struct dvb_frontend *fe)
 	ret = lgdt3306a_sleep(state);
 	lg_chkerr(ret);
 
+	c->cnr.len = 1;
+	c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+
 fail:
 	return ret;
 }
@@ -1597,6 +1601,7 @@ static int lgdt3306a_read_status(struct dvb_frontend *fe,
 				 enum fe_status *status)
 {
 	struct lgdt3306a_state *state = fe->demodulator_priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	u16 strength = 0;
 	int ret = 0;
 
@@ -1637,6 +1642,15 @@ static int lgdt3306a_read_status(struct dvb_frontend *fe,
 		default:
 			ret = -EINVAL;
 		}
+
+		if (*status & FE_HAS_SYNC) {
+			c->cnr.len = 1;
+			c->cnr.stat[0].scale = FE_SCALE_DECIBEL;
+			c->cnr.stat[0].svalue = lgdt3306a_calculate_snr_x100(state) * 10;
+		} else {
+			c->cnr.len = 1;
+			c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		}
 	}
 	return ret;
 }
-- 
2.23.0

[PATCH v3 14/14] cx25840: Register labeling, chip specific correction

[Brad Love]

Remove vbi_regs_offset from a group of registers that are 888 specific,
include those registers names. Sources used for reference are 885 and 888
datasheets.

Add labels to some undocumented registers.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Since v2:
- rebase

 drivers/media/i2c/cx25840/cx25840-core.c | 40 ++++++++++++------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index 0de946fe2109..e2e935f78986 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -997,14 +997,14 @@ static void cx23885_initialize(struct i2c_client *client)
 	 */
 	cx25840_write4(client, 0x404, 0x0010253e);
 
-	/* CC on  - Undocumented Register */
+	/* CC on  - VBI_LINE_CTRL3, FLD_VBI_MD_LINE12 */
 	cx25840_write(client, state->vbi_regs_offset + 0x42f, 0x66);
 
 	/* HVR-1250 / HVR1850 DIF related */
 	/* Power everything up */
 	cx25840_write4(client, 0x130, 0x0);
 
-	/* Undocumented */
+	/* SRC_COMB_CFG */
 	if (is_cx23888(state))
 		cx25840_write4(client, 0x454, 0x6628021F);
 	else
@@ -1486,24 +1486,24 @@ static int set_input(struct i2c_client *client,
 			cx25840_write4(client, 0x410, 0xffff0dbf);
 			cx25840_write4(client, 0x414, 0x00137d03);
 
-			cx25840_write4(client, state->vbi_regs_offset + 0x42c,
-				       0x42600000);
-			cx25840_write4(client, state->vbi_regs_offset + 0x430,
-				       0x0000039b);
-			cx25840_write4(client, state->vbi_regs_offset + 0x438,
-				       0x00000000);
-
-			cx25840_write4(client, state->vbi_regs_offset + 0x440,
-				       0xF8E3E824);
-			cx25840_write4(client, state->vbi_regs_offset + 0x444,
-				       0x401040dc);
-			cx25840_write4(client, state->vbi_regs_offset + 0x448,
-				       0xcd3f02a0);
-			cx25840_write4(client, state->vbi_regs_offset + 0x44c,
-				       0x161f1000);
-			cx25840_write4(client, state->vbi_regs_offset + 0x450,
-				       0x00000802);
-
+			if (is_cx23888(state)) {
+				/* 888 MISC_TIM_CTRL */
+				cx25840_write4(client, 0x42c, 0x42600000);
+				/* 888 FIELD_COUNT */
+				cx25840_write4(client, 0x430, 0x0000039b);
+				/* 888 VSCALE_CTRL */
+				cx25840_write4(client, 0x438, 0x00000000);
+				/* 888 DFE_CTRL1 */
+				cx25840_write4(client, 0x440, 0xF8E3E824);
+				/* 888 DFE_CTRL2 */
+				cx25840_write4(client, 0x444, 0x401040dc);
+				/* 888 DFE_CTRL3 */
+				cx25840_write4(client, 0x448, 0xcd3f02a0);
+				/* 888 PLL_CTRL */
+				cx25840_write4(client, 0x44c, 0x161f1000);
+				/* 888 HTL_CTRL */
+				cx25840_write4(client, 0x450, 0x00000802);
+			}
 			cx25840_write4(client, 0x91c, 0x01000000);
 			cx25840_write4(client, 0x8e0, 0x03063870);
 			cx25840_write4(client, 0x8d4, 0x7FFF0024);
-- 
2.23.0