Linux-Media Archive on lore.kernel.org
 help / Atom feed
* [PATCH 00/13] si2157: Analog tuning and optimizations
@ 2018-12-29 17:51 Brad Love
  2018-12-29 17:51 ` [PATCH 01/13] si2157: Enable tuner status flags Brad Love
                   ` (13 more replies)
  0 siblings, 14 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: 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
- Some si2141 init
- 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 ?


Brad Love (13):
  si2157: Enable tuner status flags
  si2157: Check error status bit on cmd execute
  si2157: Better check for running tuner in init
  si2157: Add clock and pin setup for si2141
  cx25840: Register labeling, chip specific correction
  si2157: Add analog tuning related functions
  si2157: Briefly wait for tuning operation to complete
  cx23885: Add analog tuner support to Hauppauge QuadHD
  cx23885: Add analog tuner to 1265_K4
  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

 drivers/media/dvb-frontends/lgdt3306a.c    |  14 +
 drivers/media/i2c/cx25840/cx25840-core.c   |  33 ++-
 drivers/media/pci/cx23885/cx23885-cards.c  |  39 ++-
 drivers/media/pci/cx23885/cx23885-dvb.c    |  25 ++
 drivers/media/pci/cx23885/cx23885-video.c  |  68 ++++-
 drivers/media/tuners/si2157.c              | 436 ++++++++++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h         |   2 +
 drivers/media/usb/cx231xx/cx231xx-avcore.c |  35 ++-
 drivers/media/usb/cx231xx/cx231xx-video.c  |  57 +++-
 9 files changed, 651 insertions(+), 58 deletions(-)

-- 
2.7.4


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

* [PATCH 01/13] si2157: Enable tuner status flags
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 02/13] si2157: Check error status bit on cmd execute Brad Love
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

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

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 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 d389f1f..4855448 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.7.4


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

* [PATCH 02/13] si2157: Check error status bit on cmd execute
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
  2018-12-29 17:51 ` [PATCH 01/13] si2157: Enable tuner status flags Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2019-01-09 18:01   ` Antti Palosaari
  2018-12-29 17:51 ` [PATCH 03/13] si2157: Better check for running tuner in init Brad Love
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: 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>
---
 drivers/media/tuners/si2157.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 4855448..3924c42 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -56,14 +56,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",
+		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
 				jiffies_to_msecs(jiffies) -
-				(jiffies_to_msecs(timeout) - TIMEOUT));
+				(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);
@@ -477,7 +483,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.7.4


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

* [PATCH 03/13] si2157: Better check for running tuner in init
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
  2018-12-29 17:51 ` [PATCH 01/13] si2157: Enable tuner status flags Brad Love
  2018-12-29 17:51 ` [PATCH 02/13] si2157: Check error status bit on cmd execute Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 04/13] si2157: Add clock and pin setup for si2141 Brad Love
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: 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>
---
 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 3924c42..f3a60a1 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -90,24 +90,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.7.4


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

* [PATCH 04/13] si2157: Add clock and pin setup for si2141
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (2 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 03/13] si2157: Better check for running tuner in init Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2019-01-20 17:17   ` Antti Palosaari
  2018-12-29 17:51 ` [PATCH 05/13] cx25840: Register labeling, chip specific correction Brad Love
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Include some missing setup for si2141

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/tuners/si2157.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index f3a60a1..1ad2d42 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -236,6 +236,23 @@ 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]);
 
+	if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
+		/* set clock */
+		memcpy(cmd.args, "\xc0\x00\x0d", 3);
+		cmd.wlen = 3;
+		cmd.rlen = 1;
+		ret = si2157_cmd_execute(client, &cmd);
+		if (ret)
+			goto err;
+		/* setup PIN */
+		memcpy(cmd.args, "\x12\x80\x80\x85\x00\x81\x00", 7);
+		cmd.wlen = 7;
+		cmd.rlen = 7;
+		ret = si2157_cmd_execute(client, &cmd);
+		if (ret)
+			goto err;
+	}
+
 	/* enable tuner status flags */
 	memcpy(cmd.args, "\x14\x00\x01\x05\x01\x00", 6);
 	cmd.wlen = 6;
-- 
2.7.4


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

* [PATCH 05/13] cx25840: Register labeling, chip specific correction
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (3 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 04/13] si2157: Add clock and pin setup for si2141 Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 06/13] si2157: Add analog tuning related functions Brad Love
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Remove vbi_regs_offset from a group of registers that are 888 specific,
include those registers names.

Add labels to some undocumented registers.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/i2c/cx25840/cx25840-core.c | 33 ++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index b168bf3..b23d8e4 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -684,14 +684,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
@@ -1127,16 +1127,24 @@ static int set_input(struct i2c_client *client, enum cx25840_video_input vid_inp
 			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);
@@ -1743,6 +1751,7 @@ static int cx25840_s_stream(struct v4l2_subdev *sd, int enable)
 	if (is_cx2388x(state) || is_cx231xx(state))
 		return 0;
 
+	/* PIN_CTRL1 */
 	if (enable) {
 		v = cx25840_read(client, 0x115) | 0x0c;
 		cx25840_write(client, 0x115, v);
-- 
2.7.4


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

* [PATCH 06/13] si2157: Add analog tuning related functions
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (4 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 05/13] cx25840: Register labeling, chip specific correction Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 07/13] si2157: Briefly wait for tuning operation to complete Brad Love
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: 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>
---
 drivers/media/tuners/si2157.c      | 245 ++++++++++++++++++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h |   2 +
 2 files changed, 244 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 1ad2d42..ff462ba 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -388,7 +388,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;
@@ -402,7 +402,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;
@@ -414,18 +414,254 @@ 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, "%s: Analog tuning not supported for chiptype=%u\n",
+				__func__, 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;
+	cmd.args[5] = (dev->if_port == 1)?2:1; /* Analog AGC assumed external */
+	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;
+#if 1 /* can use dev->inversion if assumed it applies to both digital/analog */
+	if (invert_analog)
+		cmd.args[5] |= 0x02;
+#else
+	if (dev->inversion)
+		cmd.args[5] |= 0x02;
+#endif
+	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;
+
+#if 0 /* testing */
+	/* get tuner status, RSSI values */
+	memcpy(cmd.args, "\x42\x01", 2);
+	cmd.wlen = 2;
+	cmd.rlen = 12;
+	ret = si2157_cmd_execute(client, &cmd);
+
+	dev_info(&client->dev, "%s: tuner status: ret=%d rssi=%d mode=%x freq=%d\n",
+		__func__, ret, cmd.args[3], cmd.args[8],
+		(cmd.args[7]<<24 | cmd.args[6]<<16 |
+		cmd.args[5]<<8 | cmd.args[4]));
+#endif
+	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, "%s: freq=%u\n", __func__, 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, "%s: bandwidth=%u\n", __func__, 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, "%s: if_frequency=%u\n", __func__, *frequency);
 	return 0;
 }
 
@@ -439,6 +675,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 50f8630..1e5ce5b 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -37,6 +37,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.7.4


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

* [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (5 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 06/13] si2157: Add analog tuning related functions Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2019-04-05 10:29   ` Sean Young
  2018-12-29 17:51 ` [PATCH 08/13] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Some software reports no signal found on a frequency due to immediately
checking for lock as soon as set_params returns. This waits up 40ms for
tuning operation, then from 30-150ms (dig/analog) for signal lock before
returning from set_params and set_analog_params.

Tuning typically completes in 20-30ms. Digital tuning will additionally
wait depending on signal characteristics. Analog tuning will wait the
full timeout in case of no signal.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/tuners/si2157.c | 87 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index ff462ba..1737007 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -318,6 +318,89 @@ static int si2157_sleep(struct dvb_frontend *fe)
 	return ret;
 }
 
+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);
+
+	/* 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 we tuned ok, wait a bit for tuner lock */
+	if ((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;
+	}
+
+	mutex_unlock(&dev->i2c_mutex);
+	return 0;
+
+err_mutex_unlock:
+	mutex_unlock(&dev->i2c_mutex);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
+	return ret;
+}
+
 static int si2157_set_params(struct dvb_frontend *fe)
 {
 	struct i2c_client *client = fe->tuner_priv;
@@ -417,6 +500,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	dev->bandwidth = bandwidth;
 	dev->frequency = c->frequency;
 
+	si2157_tune_wait(client, 1); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
@@ -626,6 +711,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
 #endif
 	dev->bandwidth = bandwidth;
 
+	si2157_tune_wait(client, 0); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
-- 
2.7.4


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

* [PATCH 08/13] cx23885: Add analog tuner support to Hauppauge QuadHD
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (6 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 07/13] si2157: Briefly wait for tuning operation to complete Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 09/13] cx23885: Add analog tuner to 1265_K4 Brad Love
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Add analog tuner frontend to 888 Hauppauge QuadHD boards

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/pci/cx23885/cx23885-cards.c | 32 +++++++++++++++++++++++++++----
 drivers/media/pci/cx23885/cx23885-dvb.c   | 20 +++++++++++++++++++
 drivers/media/pci/cx23885/cx23885-video.c |  8 +++++++-
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index ed3210d..774c6ea 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -767,24 +767,48 @@ 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,
+		.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_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 0d0929c..ee85c55 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2382,6 +2382,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 */
@@ -2471,6 +2481,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 168178c..ed4ca1e 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -264,6 +264,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) ||
@@ -1012,7 +1014,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) {
@@ -1043,6 +1047,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.7.4


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

* [PATCH 09/13] cx23885: Add analog tuner to 1265_K4
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (7 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 08/13] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 11/13] cx23885: Add i2c device analog tuner support Brad Love
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

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

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/pci/cx23885/cx23885-cards.c | 7 +++++++
 drivers/media/pci/cx23885/cx23885-dvb.c   | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 774c6ea..ca345cb 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -817,6 +817,13 @@ struct cx23885_board cx23885_boards[] = {
 		.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 ee85c55..0366c4d 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2562,6 +2562,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.7.4


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

* [PATCH 11/13] cx23885: Add i2c device analog tuner support
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (8 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 09/13] cx23885: Add analog tuner to 1265_K4 Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2018-12-29 17:51 ` [PATCH 10/13] cx231xx: " Brad Love
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Hauppauge QuadHD 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.

Models with analog tuner inputs enabled:
- CX23885_BOARD_HAUPPAUGE_HVR1265_K4 (tested)
- CX23885_BOARD_HAUPPAUGE_QUADHD_DVB (tested)
- CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC (tested)

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/pci/cx23885/cx23885-video.c | 60 +++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index ed4ca1e..6d6e7fb 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -646,8 +646,17 @@ static int vidioc_querycap(struct file *file, void  *priv,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCIe:%s", pci_name(dev->pci));
 	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING | V4L2_CAP_AUDIO;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	}
 	if (vdev->vfl_type == VFL_TYPE_VBI)
 		cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
 	else
@@ -901,8 +910,16 @@ 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_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;
 
@@ -917,8 +934,16 @@ 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_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 */
@@ -932,9 +957,16 @@ 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_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;
 
@@ -948,8 +980,16 @@ 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_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;
 
-- 
2.7.4


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

* [PATCH 10/13] cx231xx: Add i2c device analog tuner support
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (9 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 11/13] cx23885: Add i2c device analog tuner support Brad Love
@ 2018-12-29 17:51 ` " Brad Love
  2018-12-29 17:51 ` [PATCH 12/13] si2157: add on-demand rf strength func Brad Love
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: 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>
---
 drivers/media/usb/cx231xx/cx231xx-avcore.c | 35 ++++++++++++++----
 drivers/media/usb/cx231xx/cx231xx-video.c  | 57 ++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-avcore.c b/drivers/media/usb/cx231xx/cx231xx-avcore.c
index fdd3c22..a6202a4 100644
--- a/drivers/media/usb/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/usb/cx231xx/cx231xx-avcore.c
@@ -599,14 +599,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:
@@ -1205,12 +1218,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 0d451c4..d5e51a5 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1293,7 +1293,7 @@ int cx231xx_s_frequency(struct file *file, void *priv,
 	struct cx231xx_fh *fh = priv;
 	struct cx231xx *dev = fh->dev;
 	struct v4l2_frequency new_freq = *f;
-	int rc;
+	int rc, need_if_freq = 0;
 	u32 if_frequency = 5400000;
 
 	dev_dbg(dev->dev,
@@ -1310,14 +1310,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_err("%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)
@@ -1584,8 +1600,19 @@ int cx231xx_querycap(struct file *file, void *priv,
 		else
 			cap->device_caps |= 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:
 		cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	}
 	cap->capabilities = cap->device_caps | V4L2_CAP_READWRITE |
 		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_STREAMING | V4L2_CAP_DEVICE_CAPS;
@@ -2191,10 +2218,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;
+		}
 	}
 }
 
-- 
2.7.4


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

* [PATCH 12/13] si2157: add on-demand rf strength func
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (10 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 10/13] cx231xx: " Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2019-01-20 19:09   ` Antti Palosaari
  2018-12-29 17:51 ` [PATCH 13/13] lgdt3306a: Add CNR v5 stat Brad Love
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
  13 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

Add get_rf_strength callback to get RSSI from the tuner. DVBv5
stat cache is updated.

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 drivers/media/tuners/si2157.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 1737007..f28bf7f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -752,6 +752,40 @@ 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;
+
+	strength = (s8)cmd.args[3];
+	strength = (strength > -80) ? (u16)(strength + 100) : 0;
+	strength = strength > 80 ? 100 : strength;
+
+	*rssi = (u16)(strength * 0xffff / 100);
+	dev_dbg(&client->dev, "%s: strength=%d rssi=%u\n",
+		__func__, (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",
@@ -765,7 +799,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.7.4


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

* [PATCH 13/13] lgdt3306a: Add CNR v5 stat
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (11 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 12/13] si2157: add on-demand rf strength func Brad Love
@ 2018-12-29 17:51 ` Brad Love
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
  13 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2018-12-29 17:51 UTC (permalink / raw)
  To: linux-media, mchehab; +Cc: Brad Love

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

Signed-off-by: Brad Love <brad@nextdimension.cc>
---
 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 cee9c83..8b24748 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -855,6 +855,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;
 
@@ -1006,6 +1007,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;
 }
@@ -1606,6 +1610,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;
 
@@ -1646,6 +1651,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.7.4


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

* Re: [PATCH 02/13] si2157: Check error status bit on cmd execute
  2018-12-29 17:51 ` [PATCH 02/13] si2157: Check error status bit on cmd execute Brad Love
@ 2019-01-09 18:01   ` Antti Palosaari
  2019-01-15 17:28     ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Antti Palosaari @ 2019-01-09 18:01 UTC (permalink / raw)
  To: Brad Love, linux-media, mchehab

On 12/29/18 7:51 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.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>   drivers/media/tuners/si2157.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 4855448..3924c42 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -56,14 +56,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",
> +		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
>   				jiffies_to_msecs(jiffies) -
> -				(jiffies_to_msecs(timeout) - TIMEOUT));
> +				(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);
> @@ -477,7 +483,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));
> 

So you added check if firmware returns error during command execution, 
but that error is still skipped during probe, which does not feel 
correct. Chip should work during probe and ideally driver should ensure 
it is correct chip. At least you should read some property value or 
execute some other command without failure.

regards
Antti

-- 
http://palosaari.fi/

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

* Re: [PATCH 02/13] si2157: Check error status bit on cmd execute
  2019-01-09 18:01   ` Antti Palosaari
@ 2019-01-15 17:28     ` Brad Love
  0 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-01-15 17:28 UTC (permalink / raw)
  To: Antti Palosaari, Brad Love, linux-media, mchehab

Hi Antti,


On 09/01/2019 12.01, Antti Palosaari wrote:
> On 12/29/18 7:51 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.
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>>   drivers/media/tuners/si2157.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/tuners/si2157.c
>> b/drivers/media/tuners/si2157.c
>> index 4855448..3924c42 100644
>> --- a/drivers/media/tuners/si2157.c
>> +++ b/drivers/media/tuners/si2157.c
>> @@ -56,14 +56,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",
>> +        dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
>>                   jiffies_to_msecs(jiffies) -
>> -                (jiffies_to_msecs(timeout) - TIMEOUT));
>> +                (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);
>> @@ -477,7 +483,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));
>>
>
> So you added check if firmware returns error during command execution,
> but that error is still skipped during probe, which does not feel
> correct. Chip should work during probe and ideally driver should
> ensure it is correct chip. At least you should read some property
> value or execute some other command without failure.
>

The error status flags have not been enabled yet. The bits will be set
and the cmd_execute will return -EAGAIN even if the chip is there, until
the status flags are on. Probe currently only contains a sanity check.
The chip identification happens in si2157_init, some code could (and I
think should) be moved from init into probe, but that is reorganization
unrelated to this series. I'll think about that and probably submit a
patch to address it.

Regards,

Brad


> regards
> Antti
>

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

* Re: [PATCH 04/13] si2157: Add clock and pin setup for si2141
  2018-12-29 17:51 ` [PATCH 04/13] si2157: Add clock and pin setup for si2141 Brad Love
@ 2019-01-20 17:17   ` Antti Palosaari
  2019-01-22 17:10     ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Antti Palosaari @ 2019-01-20 17:17 UTC (permalink / raw)
  To: Brad Love, linux-media, mchehab


On 12/29/18 7:51 PM, Brad Love wrote:
> Include some missing setup for si2141
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>   drivers/media/tuners/si2157.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index f3a60a1..1ad2d42 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -236,6 +236,23 @@ 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]);
>   
> +	if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> +		/* set clock */
> +		memcpy(cmd.args, "\xc0\x00\x0d", 3);
> +		cmd.wlen = 3;
> +		cmd.rlen = 1;
> +		ret = si2157_cmd_execute(client, &cmd);
> +		if (ret)
> +			goto err;
> +		/* setup PIN */
> +		memcpy(cmd.args, "\x12\x80\x80\x85\x00\x81\x00", 7);
> +		cmd.wlen = 7;
> +		cmd.rlen = 7;
> +		ret = si2157_cmd_execute(client, &cmd);
> +		if (ret)
> +			goto err;
> +	}
> +
>   	/* enable tuner status flags */
>   	memcpy(cmd.args, "\x14\x00\x01\x05\x01\x00", 6);
>   	cmd.wlen = 6;
> 

Si2141 is working in my understanding, why these are required?


regards
Antti



-- 
http://palosaari.fi/

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

* Re: [PATCH 12/13] si2157: add on-demand rf strength func
  2018-12-29 17:51 ` [PATCH 12/13] si2157: add on-demand rf strength func Brad Love
@ 2019-01-20 19:09   ` Antti Palosaari
  0 siblings, 0 replies; 39+ messages in thread
From: Antti Palosaari @ 2019-01-20 19:09 UTC (permalink / raw)
  To: Brad Love, linux-media, mchehab

On 12/29/18 7:51 PM, Brad Love wrote:
> Add get_rf_strength callback to get RSSI from the tuner. DVBv5
> stat cache is updated.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>


> ---
>   drivers/media/tuners/si2157.c | 38 +++++++++++++++++++++++++++++++++++++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 1737007..f28bf7f 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -752,6 +752,40 @@ 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;
> +
> +	strength = (s8)cmd.args[3];
> +	strength = (strength > -80) ? (u16)(strength + 100) : 0;
> +	strength = strength > 80 ? 100 : strength;
> +
> +	*rssi = (u16)(strength * 0xffff / 100);
> +	dev_dbg(&client->dev, "%s: strength=%d rssi=%u\n",
> +		__func__, (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",
> @@ -765,7 +799,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)
> 

Where that is called from?

It is also hard to read how you convert dBm RSSI value to some other 
scale. There is various clamp() macros for limiting value to desired range.

__func__ should not be passed to dev_ macros, check some manual how to use.

Driver already polls rssi for digital tv, but I assume that is somehow 
related to analog.


regards
Antti

-- 
http://palosaari.fi/

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

* Re: [PATCH 04/13] si2157: Add clock and pin setup for si2141
  2019-01-20 17:17   ` Antti Palosaari
@ 2019-01-22 17:10     ` Brad Love
  0 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-01-22 17:10 UTC (permalink / raw)
  To: Antti Palosaari, Brad Love, linux-media, mchehab

Hi Antti,


On 20/01/2019 11.17, Antti Palosaari wrote:
>
> On 12/29/18 7:51 PM, Brad Love wrote:
>> Include some missing setup for si2141
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>>   drivers/media/tuners/si2157.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/media/tuners/si2157.c
>> b/drivers/media/tuners/si2157.c
>> index f3a60a1..1ad2d42 100644
>> --- a/drivers/media/tuners/si2157.c
>> +++ b/drivers/media/tuners/si2157.c
>> @@ -236,6 +236,23 @@ 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]);
>>   +    if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
>> +        /* set clock */
>> +        memcpy(cmd.args, "\xc0\x00\x0d", 3);
>> +        cmd.wlen = 3;
>> +        cmd.rlen = 1;
>> +        ret = si2157_cmd_execute(client, &cmd);
>> +        if (ret)
>> +            goto err;
>> +        /* setup PIN */
>> +        memcpy(cmd.args, "\x12\x80\x80\x85\x00\x81\x00", 7);
>> +        cmd.wlen = 7;
>> +        cmd.rlen = 7;
>> +        ret = si2157_cmd_execute(client, &cmd);
>> +        if (ret)
>> +            goto err;
>> +    }
>> +
>>       /* enable tuner status flags */
>>       memcpy(cmd.args, "\x14\x00\x01\x05\x01\x00", 6);
>>       cmd.wlen = 6;
>>
>
> Si2141 is working in my understanding, why these are required?
>

Apologies, this setting was necessary on a "dvbsky device" that
hauppauge was considering supporting. It never reached production in the
end, but this artifact stayed around anyways. I will drop this patch
from a v2 series.

Regards,

Brad






>
> regards
> Antti
>
>
>

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

* [PATCH v2 00/13] si2157: Analog tuning and optimizations
  2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
                   ` (12 preceding siblings ...)
  2018-12-29 17:51 ` [PATCH 13/13] lgdt3306a: Add CNR v5 stat Brad Love
@ 2019-02-27 18:27 ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 01/12] si2157: Enable tuner status flags Brad Love
                     ` (11 more replies)
  13 siblings, 12 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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 ?


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


Brad Love (13):
  si2157: Enable tuner status flags
  si2157: Check error status bit on cmd execute
  si2157: Better check for running tuner in init
  si2157: Add clock and pin setup for si2141
  cx25840: Register labeling, chip specific correction
  si2157: Add analog tuning related functions
  si2157: Briefly wait for tuning operation to complete
  cx23885: Add analog tuner support to Hauppauge QuadHD
  cx23885: Add analog tuner to 1265_K4
  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

 drivers/media/dvb-frontends/lgdt3306a.c    |  14 +
 drivers/media/i2c/cx25840/cx25840-core.c   |  33 ++-
 drivers/media/pci/cx23885/cx23885-cards.c  |  39 ++-
 drivers/media/pci/cx23885/cx23885-dvb.c    |  25 ++
 drivers/media/pci/cx23885/cx23885-video.c  |  68 ++++-
 drivers/media/tuners/si2157.c              | 436 ++++++++++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h         |   2 +
 drivers/media/usb/cx231xx/cx231xx-avcore.c |  35 ++-
 drivers/media/usb/cx231xx/cx231xx-video.c  |  57 +++-
 9 files changed, 651 insertions(+), 58 deletions(-)

-- 
2.7.4


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

* [PATCH v2 01/12] si2157: Enable tuner status flags
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 02/12] si2157: Check error status bit on cmd execute Brad Love
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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 d389f1f..4855448 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.7.4


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

* [PATCH v2 02/12] si2157: Check error status bit on cmd execute
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
  2019-02-27 18:27   ` [PATCH v2 01/12] si2157: Enable tuner status flags Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 03/12] si2157: Better check for running tuner in init Brad Love
                     ` (9 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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>
---
No changes

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

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 4855448..3924c42 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -56,14 +56,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",
+		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
 				jiffies_to_msecs(jiffies) -
-				(jiffies_to_msecs(timeout) - TIMEOUT));
+				(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);
@@ -477,7 +483,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.7.4


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

* [PATCH v2 03/12] si2157: Better check for running tuner in init
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
  2019-02-27 18:27   ` [PATCH v2 01/12] si2157: Enable tuner status flags Brad Love
  2019-02-27 18:27   ` [PATCH v2 02/12] si2157: Check error status bit on cmd execute Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 04/12] cx25840: Register labeling, chip specific correction Brad Love
                     ` (8 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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 3924c42..f3a60a1 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -90,24 +90,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.7.4


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

* [PATCH v2 04/12] cx25840: Register labeling, chip specific correction
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (2 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 03/12] si2157: Better check for running tuner in init Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 05/12] si2157: Add analog tuning related functions Brad Love
                     ` (7 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: Brad Love

Remove vbi_regs_offset from a group of registers that are 888 specific,
include those registers names.

Add labels to some undocumented registers.

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

 drivers/media/i2c/cx25840/cx25840-core.c | 33 ++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index 8b0b8b5a..7416b29 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -684,14 +684,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
@@ -1127,16 +1127,24 @@ static int set_input(struct i2c_client *client, enum cx25840_video_input vid_inp
 			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);
@@ -1743,6 +1751,7 @@ static int cx25840_s_stream(struct v4l2_subdev *sd, int enable)
 	if (is_cx2388x(state) || is_cx231xx(state))
 		return 0;
 
+	/* PIN_CTRL1 */
 	if (enable) {
 		v = cx25840_read(client, 0x115) | 0x0c;
 		cx25840_write(client, 0x115, v);
-- 
2.7.4


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

* [PATCH v2 05/12] si2157: Add analog tuning related functions
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (3 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 04/12] cx25840: Register labeling, chip specific correction Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-04-05 13:24     ` Sean Young
  2019-02-27 18:27   ` [PATCH v2 06/12] si2157: Briefly wait for tuning operation to complete Brad Love
                     ` (6 subsequent siblings)
  11 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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      | 245 ++++++++++++++++++++++++++++++++++++-
 drivers/media/tuners/si2157_priv.h |   2 +
 2 files changed, 244 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index f3a60a1..1006172 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,254 @@ 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;
+	cmd.args[5] = (dev->if_port == 1)?2:1; /* Analog AGC assumed external */
+	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;
+#if 1 /* can use dev->inversion if assumed it applies to both digital/analog */
+	if (invert_analog)
+		cmd.args[5] |= 0x02;
+#else
+	if (dev->inversion)
+		cmd.args[5] |= 0x02;
+#endif
+	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;
+
+#if 0 /* testing */
+	/* get tuner status, RSSI values */
+	memcpy(cmd.args, "\x42\x01", 2);
+	cmd.wlen = 2;
+	cmd.rlen = 12;
+	ret = si2157_cmd_execute(client, &cmd);
+
+	dev_info(&client->dev, "tuner status: ret=%d rssi=%d mode=%x freq=%d\n",
+		ret, cmd.args[3], cmd.args[8],
+		(cmd.args[7]<<24 | cmd.args[6]<<16 |
+		cmd.args[5]<<8 | cmd.args[4]));
+#endif
+	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 +658,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 50f8630..1e5ce5b 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -37,6 +37,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.7.4


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

* [PATCH v2 06/12] si2157: Briefly wait for tuning operation to complete
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (4 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 05/12] si2157: Add analog tuning related functions Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 07/12] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
                     ` (5 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: Brad Love

Some software reports no signal found on a frequency due to immediately
checking for lock as soon as set_params returns. This waits up 40ms for
tuning operation, then from 30-150ms (dig/analog) for signal lock before
returning from set_params and set_analog_params.

Tuning typically completes in 20-30ms. Digital tuning will additionally
wait depending on signal characteristics. Analog tuning will wait the
full timeout in case of no signal.

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

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

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 1006172..36b2ea8 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -301,6 +301,89 @@ static int si2157_sleep(struct dvb_frontend *fe)
 	return ret;
 }
 
+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);
+
+	/* 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 we tuned ok, wait a bit for tuner lock */
+	if ((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;
+	}
+
+	mutex_unlock(&dev->i2c_mutex);
+	return 0;
+
+err_mutex_unlock:
+	mutex_unlock(&dev->i2c_mutex);
+	dev_dbg(&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 +483,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	dev->bandwidth = bandwidth;
 	dev->frequency = c->frequency;
 
+	si2157_tune_wait(client, 1); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
@@ -609,6 +694,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
 #endif
 	dev->bandwidth = bandwidth;
 
+	si2157_tune_wait(client, 0); /* wait to complete, ignore any errors */
+
 	return 0;
 err:
 	dev->bandwidth = 0;
-- 
2.7.4


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

* [PATCH v2 07/12] cx23885: Add analog tuner support to Hauppauge QuadHD
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (5 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 06/12] si2157: Briefly wait for tuning operation to complete Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 08/12] cx23885: Add analog frontend to 1265_K4 Brad Love
                     ` (4 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: Brad Love

Add analog tuner frontend to 888 Hauppauge QuadHD boards

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

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

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index ed3210d..774c6ea 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -767,24 +767,48 @@ 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,
+		.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_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 0d0929c..ee85c55 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2382,6 +2382,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 */
@@ -2471,6 +2481,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 168178c..ed4ca1e 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -264,6 +264,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) ||
@@ -1012,7 +1014,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) {
@@ -1043,6 +1047,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.7.4


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

* [PATCH v2 08/12] cx23885: Add analog frontend to 1265_K4
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (6 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 07/12] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 09/12] cx23885: Add i2c device analog tuner support Brad Love
                     ` (3 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: Brad Love

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

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

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

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 774c6ea..ca345cb 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -817,6 +817,13 @@ struct cx23885_board cx23885_boards[] = {
 		.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 ee85c55..0366c4d 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2562,6 +2562,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.7.4


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

* [PATCH v2 09/12] cx23885: Add i2c device analog tuner support
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (7 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 08/12] cx23885: Add analog frontend to 1265_K4 Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 10/12] cx231xx: " Brad Love
                     ` (2 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: Brad Love

Hauppauge QuadHD 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.

Models with analog tuner inputs enabled:
- CX23885_BOARD_HAUPPAUGE_HVR1265_K4 (tested)
- CX23885_BOARD_HAUPPAUGE_QUADHD_DVB (tested)
- CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC (tested)

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

 drivers/media/pci/cx23885/cx23885-video.c | 60 +++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index ed4ca1e..6d6e7fb 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -646,8 +646,17 @@ static int vidioc_querycap(struct file *file, void  *priv,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCIe:%s", pci_name(dev->pci));
 	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING | V4L2_CAP_AUDIO;
-	if (dev->tuner_type != TUNER_ABSENT)
+	switch (dev->board) { /* i2c device tuners */
+	case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+	case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
 		cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	}
 	if (vdev->vfl_type == VFL_TYPE_VBI)
 		cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
 	else
@@ -901,8 +910,16 @@ 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_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;
 
@@ -917,8 +934,16 @@ 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_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 */
@@ -932,9 +957,16 @@ 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_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;
 
@@ -948,8 +980,16 @@ 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_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;
 
-- 
2.7.4


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

* [PATCH v2 10/12] cx231xx: Add i2c device analog tuner support
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (8 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 09/12] cx23885: Add i2c device analog tuner support Brad Love
@ 2019-02-27 18:27   ` " Brad Love
  2019-02-27 18:27   ` [PATCH v2 11/12] si2157: add on-demand rf strength func Brad Love
  2019-02-27 18:27   ` [PATCH v2 12/12] lgdt3306a: Add CNR v5 stat Brad Love
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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>
---
Changes since v1:
- remove __func__ from dev_dbg macros

 drivers/media/usb/cx231xx/cx231xx-avcore.c | 35 ++++++++++++++----
 drivers/media/usb/cx231xx/cx231xx-video.c  | 57 ++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-avcore.c b/drivers/media/usb/cx231xx/cx231xx-avcore.c
index fdd3c22..a6202a4 100644
--- a/drivers/media/usb/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/usb/cx231xx/cx231xx-avcore.c
@@ -599,14 +599,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:
@@ -1205,12 +1218,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 aebbaf9..b64ab53 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1293,7 +1293,7 @@ int cx231xx_s_frequency(struct file *file, void *priv,
 	struct cx231xx_fh *fh = priv;
 	struct cx231xx *dev = fh->dev;
 	struct v4l2_frequency new_freq = *f;
-	int rc;
+	int rc, need_if_freq = 0;
 	u32 if_frequency = 5400000;
 
 	dev_dbg(dev->dev,
@@ -1310,14 +1310,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)
@@ -1584,8 +1600,19 @@ int cx231xx_querycap(struct file *file, void *priv,
 		else
 			cap->device_caps |= 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:
 		cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	default:
+		if (dev->tuner_type != TUNER_ABSENT)
+			cap->device_caps |= V4L2_CAP_TUNER;
+		break;
+	}
 	cap->capabilities = cap->device_caps | V4L2_CAP_READWRITE |
 		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_VIDEO_CAPTURE |
 		V4L2_CAP_STREAMING | V4L2_CAP_DEVICE_CAPS;
@@ -2191,10 +2218,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;
+		}
 	}
 }
 
-- 
2.7.4


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

* [PATCH v2 11/12] si2157: add on-demand rf strength func
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (9 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 10/12] cx231xx: " Brad Love
@ 2019-02-27 18:27   ` Brad Love
  2019-02-27 18:27   ` [PATCH v2 12/12] lgdt3306a: Add CNR v5 stat Brad Love
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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 36b2ea8..5ce7562 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -735,6 +735,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",
@@ -748,7 +784,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.7.4


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

* [PATCH v2 12/12] lgdt3306a: Add CNR v5 stat
  2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
                     ` (10 preceding siblings ...)
  2019-02-27 18:27   ` [PATCH v2 11/12] si2157: add on-demand rf strength func Brad Love
@ 2019-02-27 18:27   ` Brad Love
  11 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-02-27 18:27 UTC (permalink / raw)
  To: linux-media; +Cc: 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 99c6289..ceaf617 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -855,6 +855,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;
 
@@ -1006,6 +1007,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;
 }
@@ -1606,6 +1610,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;
 
@@ -1646,6 +1651,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.7.4


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

* Re: [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2018-12-29 17:51 ` [PATCH 07/13] si2157: Briefly wait for tuning operation to complete Brad Love
@ 2019-04-05 10:29   ` Sean Young
  2019-04-08 18:14     ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Sean Young @ 2019-04-05 10:29 UTC (permalink / raw)
  To: Brad Love; +Cc: linux-media, mchehab

On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote:
> Some software reports no signal found on a frequency due to immediately
> checking for lock as soon as set_params returns. This waits up 40ms for
> tuning operation, then from 30-150ms (dig/analog) for signal lock before
> returning from set_params and set_analog_params.
> 
> Tuning typically completes in 20-30ms. Digital tuning will additionally
> wait depending on signal characteristics. Analog tuning will wait the
> full timeout in case of no signal.

This looks like a workaround for broken userspace. Very possibly this
is software was tested on a different frontend which does wait for tuning
to complete.

This is a change in uapi and will have to be done for all frontends, and
documented. However I doubt such a change is acceptable.

What software are you refering to?


Sean

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>  drivers/media/tuners/si2157.c | 87 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index ff462ba..1737007 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -318,6 +318,89 @@ static int si2157_sleep(struct dvb_frontend *fe)
>  	return ret;
>  }
>  
> +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);
> +
> +	/* 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 we tuned ok, wait a bit for tuner lock */
> +	if ((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;
> +	}
> +
> +	mutex_unlock(&dev->i2c_mutex);
> +	return 0;
> +
> +err_mutex_unlock:
> +	mutex_unlock(&dev->i2c_mutex);
> +	dev_dbg(&client->dev, "failed=%d\n", ret);
> +	return ret;
> +}
> +
>  static int si2157_set_params(struct dvb_frontend *fe)
>  {
>  	struct i2c_client *client = fe->tuner_priv;
> @@ -417,6 +500,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
>  	dev->bandwidth = bandwidth;
>  	dev->frequency = c->frequency;
>  
> +	si2157_tune_wait(client, 1); /* wait to complete, ignore any errors */
> +
>  	return 0;
>  err:
>  	dev->bandwidth = 0;
> @@ -626,6 +711,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
>  #endif
>  	dev->bandwidth = bandwidth;
>  
> +	si2157_tune_wait(client, 0); /* wait to complete, ignore any errors */
> +
>  	return 0;
>  err:
>  	dev->bandwidth = 0;
> -- 
> 2.7.4

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

* Re: [PATCH v2 05/12] si2157: Add analog tuning related functions
  2019-02-27 18:27   ` [PATCH v2 05/12] si2157: Add analog tuning related functions Brad Love
@ 2019-04-05 13:24     ` Sean Young
  2019-04-08 18:18       ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Sean Young @ 2019-04-05 13:24 UTC (permalink / raw)
  To: Brad Love; +Cc: linux-media

On Wed, Feb 27, 2019 at 12:27:39PM -0600, 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.

There are quite a few warnings from checkpatch on this patch. Please
run patches through "./scripts/checkpatch.pl --strict" before submitting.

Generally there should be no output from checkpatch if possible.

Thanks

Sean

> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> Changes since v1:
> - remove __func__ from dev_dbg macros
> 
>  drivers/media/tuners/si2157.c      | 245 ++++++++++++++++++++++++++++++++++++-
>  drivers/media/tuners/si2157_priv.h |   2 +
>  2 files changed, 244 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index f3a60a1..1006172 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,254 @@ 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;
> +	cmd.args[5] = (dev->if_port == 1)?2:1; /* Analog AGC assumed external */
> +	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;
> +#if 1 /* can use dev->inversion if assumed it applies to both digital/analog */
> +	if (invert_analog)
> +		cmd.args[5] |= 0x02;
> +#else
> +	if (dev->inversion)
> +		cmd.args[5] |= 0x02;
> +#endif
> +	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;
> +
> +#if 0 /* testing */
> +	/* get tuner status, RSSI values */
> +	memcpy(cmd.args, "\x42\x01", 2);
> +	cmd.wlen = 2;
> +	cmd.rlen = 12;
> +	ret = si2157_cmd_execute(client, &cmd);
> +
> +	dev_info(&client->dev, "tuner status: ret=%d rssi=%d mode=%x freq=%d\n",
> +		ret, cmd.args[3], cmd.args[8],
> +		(cmd.args[7]<<24 | cmd.args[6]<<16 |
> +		cmd.args[5]<<8 | cmd.args[4]));
> +#endif
> +	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 +658,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 50f8630..1e5ce5b 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -37,6 +37,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.7.4

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

* Re: [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2019-04-05 10:29   ` Sean Young
@ 2019-04-08 18:14     ` Brad Love
  2019-04-08 20:55       ` Sean Young
  0 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2019-04-08 18:14 UTC (permalink / raw)
  To: Sean Young, Brad Love; +Cc: linux-media, mchehab


On 05/04/2019 05.29, Sean Young wrote:
> On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote:
>> Some software reports no signal found on a frequency due to immediately
>> checking for lock as soon as set_params returns. This waits up 40ms for
>> tuning operation, then from 30-150ms (dig/analog) for signal lock before
>> returning from set_params and set_analog_params.
>>
>> Tuning typically completes in 20-30ms. Digital tuning will additionally
>> wait depending on signal characteristics. Analog tuning will wait the
>> full timeout in case of no signal.
> This looks like a workaround for broken userspace. Very possibly this
> is software was tested on a different frontend which does wait for tuning
> to complete.
>
> This is a change in uapi and will have to be done for all frontends, and
> documented. However I doubt such a change is acceptable.
>
> What software are you refering to?


Hi Sean,

I would have to check with support to find out the various software that
expect when a tune command returns for that tuning operation to have
actually completed. In the current state when you tune si2157 it
immediately returns, no care of the tuning operation completion, no care
of the pll lock success. This is correct? Not so according to Silicon
Labs documentation, which suggests a brief wait. I just took a look at
other drivers, sampling only those with set_analog_params, all but two
have similar code in place to actually allow the tune operation time to
complete (both digital and analog) before returning to userland. The
other drivers just insert arbitrary time delays to accommodate the
operations completion time. At least with my patch I am monitoring the
hardware to know when the operation actually completes.

I see in tuners (not frontends):

mt2063 - waits up to 100ms for lock status and return
r820t - sleep 100ms for pll lock status and return
tda827x - 170-500ms wait after tune before checking status and return
tda8290 - sleep 100ms up to 3x while checking tune status and return
tuner-xc2028 - sleep for 110ms awaiting lock status and return
xc4000 - 10ms sleep after tune, unless debug, then 100ms and return
xc5000 - 20ms sleep after tune,  if pll not locked, re-tune and sleep
again, repeat until success and return

There's also other arbitrary sleeps peppered throughout the operations.

Then you have si2157 that fires off the tuning commands and goes right
back to userland immediately, when with instrumented testing, the
operation takes time to complete and lock. The operation does not happen
instantaneously. Software that expects clocks to be locked when the
function returns determine this is an error. They query the tuner
immediately when tune returns and they output tuning failed.

Please explain why awaiting the hardware to say the "tuning operation
you requested is done and clocks are locked" is not ok. If it's not ok,
fine, but then a lot of other drivers are currently "not ok" as well.

Regards,

Brad


>
>
> Sean
>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>>  drivers/media/tuners/si2157.c | 87 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 87 insertions(+)
>>
>> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
>> index ff462ba..1737007 100644
>> --- a/drivers/media/tuners/si2157.c
>> +++ b/drivers/media/tuners/si2157.c
>> @@ -318,6 +318,89 @@ static int si2157_sleep(struct dvb_frontend *fe)
>>  	return ret;
>>  }
>>  
>> +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);
>> +
>> +	/* 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 we tuned ok, wait a bit for tuner lock */
>> +	if ((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;
>> +	}
>> +
>> +	mutex_unlock(&dev->i2c_mutex);
>> +	return 0;
>> +
>> +err_mutex_unlock:
>> +	mutex_unlock(&dev->i2c_mutex);
>> +	dev_dbg(&client->dev, "failed=%d\n", ret);
>> +	return ret;
>> +}
>> +
>>  static int si2157_set_params(struct dvb_frontend *fe)
>>  {
>>  	struct i2c_client *client = fe->tuner_priv;
>> @@ -417,6 +500,8 @@ static int si2157_set_params(struct dvb_frontend *fe)
>>  	dev->bandwidth = bandwidth;
>>  	dev->frequency = c->frequency;
>>  
>> +	si2157_tune_wait(client, 1); /* wait to complete, ignore any errors */
>> +
>>  	return 0;
>>  err:
>>  	dev->bandwidth = 0;
>> @@ -626,6 +711,8 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
>>  #endif
>>  	dev->bandwidth = bandwidth;
>>  
>> +	si2157_tune_wait(client, 0); /* wait to complete, ignore any errors */
>> +
>>  	return 0;
>>  err:
>>  	dev->bandwidth = 0;
>> -- 
>> 2.7.4


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

* Re: [PATCH v2 05/12] si2157: Add analog tuning related functions
  2019-04-05 13:24     ` Sean Young
@ 2019-04-08 18:18       ` Brad Love
  0 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-04-08 18:18 UTC (permalink / raw)
  To: Sean Young, Brad Love; +Cc: linux-media


On 05/04/2019 08.24, Sean Young wrote:
> On Wed, Feb 27, 2019 at 12:27:39PM -0600, 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.
> There are quite a few warnings from checkpatch on this patch. Please
> run patches through "./scripts/checkpatch.pl --strict" before submitting.
>
> Generally there should be no output from checkpatch if possible.

Hi Sean,

Kernel patch guidelines suggest ./scripts/checkpatch.pl without
--strict, so that is what I've always used. I have no problems switching
to strict from now on.

Regards,

Brad


> Thanks
>
> Sean
>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> Changes since v1:
>> - remove __func__ from dev_dbg macros
>>
>>  drivers/media/tuners/si2157.c      | 245 ++++++++++++++++++++++++++++++++++++-
>>  drivers/media/tuners/si2157_priv.h |   2 +
>>  2 files changed, 244 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
>> index f3a60a1..1006172 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,254 @@ 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;
>> +	cmd.args[5] = (dev->if_port == 1)?2:1; /* Analog AGC assumed external */
>> +	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;
>> +#if 1 /* can use dev->inversion if assumed it applies to both digital/analog */
>> +	if (invert_analog)
>> +		cmd.args[5] |= 0x02;
>> +#else
>> +	if (dev->inversion)
>> +		cmd.args[5] |= 0x02;
>> +#endif
>> +	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;
>> +
>> +#if 0 /* testing */
>> +	/* get tuner status, RSSI values */
>> +	memcpy(cmd.args, "\x42\x01", 2);
>> +	cmd.wlen = 2;
>> +	cmd.rlen = 12;
>> +	ret = si2157_cmd_execute(client, &cmd);
>> +
>> +	dev_info(&client->dev, "tuner status: ret=%d rssi=%d mode=%x freq=%d\n",
>> +		ret, cmd.args[3], cmd.args[8],
>> +		(cmd.args[7]<<24 | cmd.args[6]<<16 |
>> +		cmd.args[5]<<8 | cmd.args[4]));
>> +#endif
>> +	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 +658,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 50f8630..1e5ce5b 100644
>> --- a/drivers/media/tuners/si2157_priv.h
>> +++ b/drivers/media/tuners/si2157_priv.h
>> @@ -37,6 +37,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.7.4

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

* Re: [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2019-04-08 18:14     ` Brad Love
@ 2019-04-08 20:55       ` Sean Young
  2019-04-08 21:37         ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Sean Young @ 2019-04-08 20:55 UTC (permalink / raw)
  To: Brad Love; +Cc: linux-media, mchehab

On Mon, Apr 08, 2019 at 01:14:08PM -0500, Brad Love wrote:
> 
> On 05/04/2019 05.29, Sean Young wrote:
> > On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote:
> >> Some software reports no signal found on a frequency due to immediately
> >> checking for lock as soon as set_params returns. This waits up 40ms for
> >> tuning operation, then from 30-150ms (dig/analog) for signal lock before
> >> returning from set_params and set_analog_params.
> >>
> >> Tuning typically completes in 20-30ms. Digital tuning will additionally
> >> wait depending on signal characteristics. Analog tuning will wait the
> >> full timeout in case of no signal.
> > This looks like a workaround for broken userspace. Very possibly this
> > is software was tested on a different frontend which does wait for tuning
> > to complete.
> >
> > This is a change in uapi and will have to be done for all frontends, and
> > documented. However I doubt such a change is acceptable.
> >
> > What software are you refering to?
> 
> 
> Hi Sean,
> 
> I would have to check with support to find out the various software that
> expect when a tune command returns for that tuning operation to have
> actually completed. In the current state when you tune si2157 it
> immediately returns, no care of the tuning operation completion, no care
> of the pll lock success. This is correct? Not so according to Silicon
> Labs documentation, which suggests a brief wait. I just took a look at
> other drivers, sampling only those with set_analog_params, all but two
> have similar code in place to actually allow the tune operation time to
> complete (both digital and analog) before returning to userland. The
> other drivers just insert arbitrary time delays to accommodate the
> operations completion time. At least with my patch I am monitoring the
> hardware to know when the operation actually completes.
> 
> I see in tuners (not frontends):
> 
> mt2063 - waits up to 100ms for lock status and return
> r820t - sleep 100ms for pll lock status and return
> tda827x - 170-500ms wait after tune before checking status and return
> tda8290 - sleep 100ms up to 3x while checking tune status and return
> tuner-xc2028 - sleep for 110ms awaiting lock status and return
> xc4000 - 10ms sleep after tune, unless debug, then 100ms and return
> xc5000 - 20ms sleep after tune,  if pll not locked, re-tune and sleep
> again, repeat until success and return
> 
> There's also other arbitrary sleeps peppered throughout the operations.
> 
> Then you have si2157 that fires off the tuning commands and goes right
> back to userland immediately, when with instrumented testing, the
> operation takes time to complete and lock. The operation does not happen
> instantaneously. Software that expects clocks to be locked when the
> function returns determine this is an error. They query the tuner
> immediately when tune returns and they output tuning failed.
> 
> Please explain why awaiting the hardware to say the "tuning operation
> you requested is done and clocks are locked" is not ok. If it's not ok,
> fine, but then a lot of other drivers are currently "not ok" as well.

If you read the dvb userspace api, there is nothing in the DTV_TUNE
property that says it will block until a lock has been made.

https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe_property_parameters.html#dtv-tune

The locking status can be queried using read status.

https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe-read-status.html#fe-read-status

Using this mechanism, there is a minimum of sleeping in the kernel which helps
interactivity. (uninterruptable) sleeping in drivers something that really
should be avoided if at all possible. Userspace can't do anything during that
time.

So if you look at how dvbv5-zap finds a lock, you can see that it sets
DTV_TUNE (amongst others) and then uses the frontend read_status() to query
for locking until either timeout or a lock is found (not sure why it polls
one per second though, seems a bit overly conservative).

https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-zap.c#n494

Now, if other drivers have a sleep to wait for tuning lock in them and whether
they should be removed, is another question. Looking at the tda8290 driver
it needs to try various things in order to make a lock, so there is (currently)
no way to avoid this. It would be nice if it could be changed to interruptable
sleeps though, so that dvb userspace does not "hang" until a lock is made
or failed.


Thanks,

Sean

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

* Re: [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2019-04-08 20:55       ` Sean Young
@ 2019-04-08 21:37         ` Brad Love
  2019-04-10 14:48           ` Brad Love
  0 siblings, 1 reply; 39+ messages in thread
From: Brad Love @ 2019-04-08 21:37 UTC (permalink / raw)
  To: Sean Young, Brad Love; +Cc: linux-media, mchehab

Hi Sean,


On 08/04/2019 15.55, Sean Young wrote:
> On Mon, Apr 08, 2019 at 01:14:08PM -0500, Brad Love wrote:
>> On 05/04/2019 05.29, Sean Young wrote:
>>> On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote:
>>>> Some software reports no signal found on a frequency due to immediately
>>>> checking for lock as soon as set_params returns. This waits up 40ms for
>>>> tuning operation, then from 30-150ms (dig/analog) for signal lock before
>>>> returning from set_params and set_analog_params.
>>>>
>>>> Tuning typically completes in 20-30ms. Digital tuning will additionally
>>>> wait depending on signal characteristics. Analog tuning will wait the
>>>> full timeout in case of no signal.
>>> This looks like a workaround for broken userspace. Very possibly this
>>> is software was tested on a different frontend which does wait for tuning
>>> to complete.
>>>
>>> This is a change in uapi and will have to be done for all frontends, and
>>> documented. However I doubt such a change is acceptable.
>>>
>>> What software are you refering to?
>>
>> Hi Sean,
>>
>> I would have to check with support to find out the various software that
>> expect when a tune command returns for that tuning operation to have
>> actually completed. In the current state when you tune si2157 it
>> immediately returns, no care of the tuning operation completion, no care
>> of the pll lock success. This is correct? Not so according to Silicon
>> Labs documentation, which suggests a brief wait. I just took a look at
>> other drivers, sampling only those with set_analog_params, all but two
>> have similar code in place to actually allow the tune operation time to
>> complete (both digital and analog) before returning to userland. The
>> other drivers just insert arbitrary time delays to accommodate the
>> operations completion time. At least with my patch I am monitoring the
>> hardware to know when the operation actually completes.
>>
>> I see in tuners (not frontends):
>>
>> mt2063 - waits up to 100ms for lock status and return
>> r820t - sleep 100ms for pll lock status and return
>> tda827x - 170-500ms wait after tune before checking status and return
>> tda8290 - sleep 100ms up to 3x while checking tune status and return
>> tuner-xc2028 - sleep for 110ms awaiting lock status and return
>> xc4000 - 10ms sleep after tune, unless debug, then 100ms and return
>> xc5000 - 20ms sleep after tune,  if pll not locked, re-tune and sleep
>> again, repeat until success and return
>>
>> There's also other arbitrary sleeps peppered throughout the operations.
>>
>> Then you have si2157 that fires off the tuning commands and goes right
>> back to userland immediately, when with instrumented testing, the
>> operation takes time to complete and lock. The operation does not happen
>> instantaneously. Software that expects clocks to be locked when the
>> function returns determine this is an error. They query the tuner
>> immediately when tune returns and they output tuning failed.
>>
>> Please explain why awaiting the hardware to say the "tuning operation
>> you requested is done and clocks are locked" is not ok. If it's not ok,
>> fine, but then a lot of other drivers are currently "not ok" as well.
> If you read the dvb userspace api, there is nothing in the DTV_TUNE
> property that says it will block until a lock has been made.
>
> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe_property_parameters.html#dtv-tune
>
> The locking status can be queried using read status.
>
> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe-read-status.html#fe-read-status
>
> Using this mechanism, there is a minimum of sleeping in the kernel which helps
> interactivity. (uninterruptable) sleeping in drivers something that really
> should be avoided if at all possible. Userspace can't do anything during that
> time.


This is not dealing with signal lock. As far as I understand the
documentation I have, this is hardware tuning completion, aka the pll's
and whatever else are properly configured and the tuning operation
itself is finished successfully. The byte read is looking for
TUNINT/ATVINT/DTVINT bits set, meaning the operation is complete. Right
now I ignore errors, which probably should be returned to userland, as
if a tune fails the userland has no way to determine this fact. This
process comes directly from a flowchart in the SL guide.

Signal lock is determined later, using the methods you describe. No
property registers are being queried here, the success of the previous
(tuning) operation is.



> So if you look at how dvbv5-zap finds a lock, you can see that it sets
> DTV_TUNE (amongst others) and then uses the frontend read_status() to query
> for locking until either timeout or a lock is found (not sure why it polls
> one per second though, seems a bit overly conservative).
>
> https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-zap.c#n494
>
> Now, if other drivers have a sleep to wait for tuning lock in them and whether
> they should be removed, is another question. Looking at the tda8290 driver
> it needs to try various things in order to make a lock, so there is (currently)
> no way to avoid this. It would be nice if it could be changed to interruptable
> sleeps though, so that dvb userspace does not "hang" until a lock is made
> or failed.


I indeed noticed none of the other drivers use usleep_range, which is
interruptible, correct?

Regards,

Brad


>
>
> Thanks,
>
> Sean


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

* Re: [PATCH 07/13] si2157: Briefly wait for tuning operation to complete
  2019-04-08 21:37         ` Brad Love
@ 2019-04-10 14:48           ` Brad Love
  0 siblings, 0 replies; 39+ messages in thread
From: Brad Love @ 2019-04-10 14:48 UTC (permalink / raw)
  To: Brad Love, Sean Young; +Cc: linux-media, mchehab

Hi Sean,


On 08/04/2019 16.37, Brad Love wrote:
> Hi Sean,
>
>
> On 08/04/2019 15.55, Sean Young wrote:
>> On Mon, Apr 08, 2019 at 01:14:08PM -0500, Brad Love wrote:
>>> On 05/04/2019 05.29, Sean Young wrote:
>>>> On Sat, Dec 29, 2018 at 11:51:16AM -0600, Brad Love wrote:
>>>>> Some software reports no signal found on a frequency due to immediately
>>>>> checking for lock as soon as set_params returns. This waits up 40ms for
>>>>> tuning operation, then from 30-150ms (dig/analog) for signal lock before
>>>>> returning from set_params and set_analog_params.
>>>>>
>>>>> Tuning typically completes in 20-30ms. Digital tuning will additionally
>>>>> wait depending on signal characteristics. Analog tuning will wait the
>>>>> full timeout in case of no signal.
>>>> This looks like a workaround for broken userspace. Very possibly this
>>>> is software was tested on a different frontend which does wait for tuning
>>>> to complete.
>>>>
>>>> This is a change in uapi and will have to be done for all frontends, and
>>>> documented. However I doubt such a change is acceptable.
>>>>
>>>> What software are you refering to?
>>> Hi Sean,
>>>
>>> I would have to check with support to find out the various software that
>>> expect when a tune command returns for that tuning operation to have
>>> actually completed. In the current state when you tune si2157 it
>>> immediately returns, no care of the tuning operation completion, no care
>>> of the pll lock success. This is correct? Not so according to Silicon
>>> Labs documentation, which suggests a brief wait. I just took a look at
>>> other drivers, sampling only those with set_analog_params, all but two
>>> have similar code in place to actually allow the tune operation time to
>>> complete (both digital and analog) before returning to userland. The
>>> other drivers just insert arbitrary time delays to accommodate the
>>> operations completion time. At least with my patch I am monitoring the
>>> hardware to know when the operation actually completes.
>>>
>>> I see in tuners (not frontends):
>>>
>>> mt2063 - waits up to 100ms for lock status and return
>>> r820t - sleep 100ms for pll lock status and return
>>> tda827x - 170-500ms wait after tune before checking status and return
>>> tda8290 - sleep 100ms up to 3x while checking tune status and return
>>> tuner-xc2028 - sleep for 110ms awaiting lock status and return
>>> xc4000 - 10ms sleep after tune, unless debug, then 100ms and return
>>> xc5000 - 20ms sleep after tune,  if pll not locked, re-tune and sleep
>>> again, repeat until success and return
>>>
>>> There's also other arbitrary sleeps peppered throughout the operations.
>>>
>>> Then you have si2157 that fires off the tuning commands and goes right
>>> back to userland immediately, when with instrumented testing, the
>>> operation takes time to complete and lock. The operation does not happen
>>> instantaneously. Software that expects clocks to be locked when the
>>> function returns determine this is an error. They query the tuner
>>> immediately when tune returns and they output tuning failed.
>>>
>>> Please explain why awaiting the hardware to say the "tuning operation
>>> you requested is done and clocks are locked" is not ok. If it's not ok,
>>> fine, but then a lot of other drivers are currently "not ok" as well.
>> If you read the dvb userspace api, there is nothing in the DTV_TUNE
>> property that says it will block until a lock has been made.
>>
>> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe_property_parameters.html#dtv-tune
>>
>> The locking status can be queried using read status.
>>
>> https://www.kernel.org/doc/html/latest/media/uapi/dvb/fe-read-status.html#fe-read-status
>>
>> Using this mechanism, there is a minimum of sleeping in the kernel which helps
>> interactivity. (uninterruptable) sleeping in drivers something that really
>> should be avoided if at all possible. Userspace can't do anything during that
>> time.
>
> This is not dealing with signal lock. As far as I understand the
> documentation I have, this is hardware tuning completion, aka the pll's
> and whatever else are properly configured and the tuning operation
> itself is finished successfully. The byte read is looking for
> TUNINT/ATVINT/DTVINT bits set, meaning the operation is complete. Right
> now I ignore errors, which probably should be returned to userland, as
> if a tune fails the userland has no way to determine this fact. This
> process comes directly from a flowchart in the SL guide.
>
> Signal lock is determined later, using the methods you describe. No
> property registers are being queried here, the success of the previous
> (tuning) operation is.


Hi Sean,

Just noticing now that my commit message does not agree with what I'm
saying. I will need to do some testing to see which is correct. I've
been basing my statements here off the SL reference I have, but I wrote
the commit message over a year ago probably. I'll need to do some
testing, I seem to recall the timing being consistent whether a signal
is found or not. I might have mis-wrote the message long ago when I was
splitting this up.

Regards,

Brad




>
>
>> So if you look at how dvbv5-zap finds a lock, you can see that it sets
>> DTV_TUNE (amongst others) and then uses the frontend read_status() to query
>> for locking until either timeout or a lock is found (not sure why it polls
>> one per second though, seems a bit overly conservative).
>>
>> https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-zap.c#n494
>>
>> Now, if other drivers have a sleep to wait for tuning lock in them and whether
>> they should be removed, is another question. Looking at the tda8290 driver
>> it needs to try various things in order to make a lock, so there is (currently)
>> no way to avoid this. It would be nice if it could be changed to interruptable
>> sleeps though, so that dvb userspace does not "hang" until a lock is made
>> or failed.
>
> I indeed noticed none of the other drivers use usleep_range, which is
> interruptible, correct?
>
> Regards,
>
> Brad
>
>
>>
>> Thanks,
>>
>> Sean

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

end of thread, back to index

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-29 17:51 [PATCH 00/13] si2157: Analog tuning and optimizations Brad Love
2018-12-29 17:51 ` [PATCH 01/13] si2157: Enable tuner status flags Brad Love
2018-12-29 17:51 ` [PATCH 02/13] si2157: Check error status bit on cmd execute Brad Love
2019-01-09 18:01   ` Antti Palosaari
2019-01-15 17:28     ` Brad Love
2018-12-29 17:51 ` [PATCH 03/13] si2157: Better check for running tuner in init Brad Love
2018-12-29 17:51 ` [PATCH 04/13] si2157: Add clock and pin setup for si2141 Brad Love
2019-01-20 17:17   ` Antti Palosaari
2019-01-22 17:10     ` Brad Love
2018-12-29 17:51 ` [PATCH 05/13] cx25840: Register labeling, chip specific correction Brad Love
2018-12-29 17:51 ` [PATCH 06/13] si2157: Add analog tuning related functions Brad Love
2018-12-29 17:51 ` [PATCH 07/13] si2157: Briefly wait for tuning operation to complete Brad Love
2019-04-05 10:29   ` Sean Young
2019-04-08 18:14     ` Brad Love
2019-04-08 20:55       ` Sean Young
2019-04-08 21:37         ` Brad Love
2019-04-10 14:48           ` Brad Love
2018-12-29 17:51 ` [PATCH 08/13] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
2018-12-29 17:51 ` [PATCH 09/13] cx23885: Add analog tuner to 1265_K4 Brad Love
2018-12-29 17:51 ` [PATCH 11/13] cx23885: Add i2c device analog tuner support Brad Love
2018-12-29 17:51 ` [PATCH 10/13] cx231xx: " Brad Love
2018-12-29 17:51 ` [PATCH 12/13] si2157: add on-demand rf strength func Brad Love
2019-01-20 19:09   ` Antti Palosaari
2018-12-29 17:51 ` [PATCH 13/13] lgdt3306a: Add CNR v5 stat Brad Love
2019-02-27 18:27 ` [PATCH v2 00/13] si2157: Analog tuning and optimizations Brad Love
2019-02-27 18:27   ` [PATCH v2 01/12] si2157: Enable tuner status flags Brad Love
2019-02-27 18:27   ` [PATCH v2 02/12] si2157: Check error status bit on cmd execute Brad Love
2019-02-27 18:27   ` [PATCH v2 03/12] si2157: Better check for running tuner in init Brad Love
2019-02-27 18:27   ` [PATCH v2 04/12] cx25840: Register labeling, chip specific correction Brad Love
2019-02-27 18:27   ` [PATCH v2 05/12] si2157: Add analog tuning related functions Brad Love
2019-04-05 13:24     ` Sean Young
2019-04-08 18:18       ` Brad Love
2019-02-27 18:27   ` [PATCH v2 06/12] si2157: Briefly wait for tuning operation to complete Brad Love
2019-02-27 18:27   ` [PATCH v2 07/12] cx23885: Add analog tuner support to Hauppauge QuadHD Brad Love
2019-02-27 18:27   ` [PATCH v2 08/12] cx23885: Add analog frontend to 1265_K4 Brad Love
2019-02-27 18:27   ` [PATCH v2 09/12] cx23885: Add i2c device analog tuner support Brad Love
2019-02-27 18:27   ` [PATCH v2 10/12] cx231xx: " Brad Love
2019-02-27 18:27   ` [PATCH v2 11/12] si2157: add on-demand rf strength func Brad Love
2019-02-27 18:27   ` [PATCH v2 12/12] lgdt3306a: Add CNR v5 stat Brad Love

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox