2015-05-26 15:04:08

by David Howells

[permalink] [raw]
Subject: [PATCH 1/2] TS2020: Calculate tuner gain correctly

The TS2020 and TS2022 tuners take an input from the demodulator indicating the
AGC setting on that component that is then used to influence the tuner's own
gain. This should be taken into account when calculating the gain and signal
strength.

Further, the existing TS2020 driver miscalculates the signal strength as the
result of its calculations can exceed the storage capacity of the 16-bit word
used to return it to userspace.

To this end:

(1) Add a callback function (->get_agc_pwm()) in the ts2020_config struct that
the tuner can call to get the AGC PWM value from the demodulator.

(2) Modify the TS2020 driver to calculate the gain according to Montage's
specification with the adjustment that we produce a negative value and
scale it to 0.001dB units (which is what the DVBv5 API will require):

(a) Callback to the demodulator to retrieve the AGC PWM value and then
turn that into Vagc for incorporation in the calculations. If the
callback is unset, assume a Vagc of 0.

(b) Calculate the tuner gain from a combination of Vagc and the tuner's RF
gain and baseband gain settings.

(3) Turn this into a percentage signal strength as per Montage's
specification for return to userspace with the DVBv3 API.

(4) Provide a function in the M88DS3103 demodulator driver that can be used to
get the AGC PWM value on behalf of the tuner.

(5) The ts2020_config.get_agc_pwm function should be set by the code that
stitches together the drivers for each card.

For the DVBSky cards that use the M88DS3103 with the TS2020 or the TS2022,
set the get_agc_pwm function to point to m88ds3103_get_agc_pwm.

I have tested this with a DVBSky S952 card which has an M88DS3103 and a TS2022.

Thanks to Montage for providing access to information about the workings of
these parts.

Signed-off-by: David Howells <[email protected]>
---

drivers/media/dvb-frontends/m88ds3103.c | 16 ++++
drivers/media/dvb-frontends/m88ds3103.h | 2
drivers/media/dvb-frontends/ts2020.c | 138 +++++++++++++++++++++++++++----
drivers/media/dvb-frontends/ts2020.h | 5 +
drivers/media/pci/cx23885/cx23885-dvb.c | 3 +
drivers/media/usb/dvb-usb-v2/dvbsky.c | 2
6 files changed, 148 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 50ede6a..0703316 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -52,6 +52,22 @@ err:
return ret;
}

+/*
+ * Get the demodulator AGC PWM voltage setting supplied to the tuner.
+ */
+int m88ds3103_get_agc_pwm(struct dvb_frontend *fe, u8 *_agc_pwm)
+{
+ struct m88ds3103_dev *dev = fe->demodulator_priv;
+ unsigned tmp;
+ int ret;
+
+ ret = regmap_read(dev->regmap, 0x3f, &tmp);
+ if (ret == 0)
+ *_agc_pwm = tmp;
+ return ret;
+}
+EXPORT_SYMBOL(m88ds3103_get_agc_pwm);
+
static int m88ds3103_read_status(struct dvb_frontend *fe, fe_status_t *status)
{
struct m88ds3103_dev *dev = fe->demodulator_priv;
diff --git a/drivers/media/dvb-frontends/m88ds3103.h b/drivers/media/dvb-frontends/m88ds3103.h
index ff03905..04b355a 100644
--- a/drivers/media/dvb-frontends/m88ds3103.h
+++ b/drivers/media/dvb-frontends/m88ds3103.h
@@ -176,6 +176,7 @@ extern struct dvb_frontend *m88ds3103_attach(
const struct m88ds3103_config *config,
struct i2c_adapter *i2c,
struct i2c_adapter **tuner_i2c);
+extern int m88ds3103_get_agc_pwm(struct dvb_frontend *fe, u8 *_agc_pwm);
#else
static inline struct dvb_frontend *m88ds3103_attach(
const struct m88ds3103_config *config,
@@ -185,6 +186,7 @@ static inline struct dvb_frontend *m88ds3103_attach(
pr_warn("%s: driver disabled by Kconfig\n", __func__);
return NULL;
}
+#define m88ds3103_get_agc_pwm NULL
#endif

#endif
diff --git a/drivers/media/dvb-frontends/ts2020.c b/drivers/media/dvb-frontends/ts2020.c
index f674717..277e1cf 100644
--- a/drivers/media/dvb-frontends/ts2020.c
+++ b/drivers/media/dvb-frontends/ts2020.c
@@ -32,6 +32,7 @@ struct ts2020_priv {
struct regmap_config regmap_config;
struct regmap *regmap;
struct dvb_frontend *fe;
+ int (*get_agc_pwm)(struct dvb_frontend *fe, u8 *_agc_pwm);
/* i2c details */
int i2c_address;
struct i2c_adapter *i2c;
@@ -313,32 +314,132 @@ static int ts2020_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
return 0;
}

-/* read TS2020 signal strength */
-static int ts2020_read_signal_strength(struct dvb_frontend *fe,
- u16 *signal_strength)
+/*
+ * Get the tuner gain.
+ * @fe: The front end for which we're determining the gain
+ * @v_agc: The voltage of the AGC from the demodulator (0-2600mV)
+ * @_gain: Where to store the gain (in 0.001dB units)
+ *
+ * Returns 0 or a negative error code.
+ */
+static int ts2020_read_tuner_gain(struct dvb_frontend *fe, unsigned v_agc,
+ __s64 *_gain)
{
struct ts2020_priv *priv = fe->tuner_priv;
- unsigned int utmp;
- u16 sig_reading, sig_strength;
- u8 rfgain, bbgain;
+ unsigned long gain1, gain2, gain3;
+ unsigned utmp;
+ int ret;
+
+ /* Read the RF gain */
+ ret = regmap_read(priv->regmap, 0x3d, &utmp);
+ if (ret < 0)
+ return ret;
+ gain1 = utmp & 0x1f;
+
+ /* Read the baseband gain */
+ ret = regmap_read(priv->regmap, 0x21, &utmp);
+ if (ret < 0)
+ return ret;
+ gain2 = utmp & 0x1f;
+
+ switch (priv->tuner) {
+ case TS2020_M88TS2020:
+ gain1 = clamp_t(long, gain1, 0, 15);
+ gain2 = clamp_t(long, gain2, 0, 13);
+ v_agc = clamp_t(long, v_agc, 400, 1100);
+
+ *_gain = -(gain1 * 2330 +
+ gain2 * 3500 +
+ v_agc * 24 / 10 * 10 +
+ 10000);
+ /* gain in range -19600 to -116850 in units of 0.001dB */
+ break;
+
+ case TS2020_M88TS2022:
+ ret = regmap_read(priv->regmap, 0x66, &utmp);
+ if (ret < 0)
+ return ret;
+ gain3 = (utmp >> 3) & 0x07;
+
+ gain1 = clamp_t(long, gain1, 0, 15);
+ gain2 = clamp_t(long, gain2, 2, 16);
+ gain3 = clamp_t(long, gain3, 0, 6);
+ v_agc = clamp_t(long, v_agc, 600, 1600);
+
+ *_gain = -(gain1 * 2650 +
+ gain2 * 3380 +
+ gain3 * 2850 +
+ v_agc * 176 / 100 * 10 -
+ 30000);
+ /* gain in range -47320 to -158950 in units of 0.001dB */
+ break;
+ }
+
+ return 0;
+}
+
+/*
+ * Get the AGC information from the demodulator and use that to calculate the
+ * tuner gain.
+ */
+static int ts2020_get_tuner_gain(struct dvb_frontend *fe, __s64 *_gain)
+{
+ struct ts2020_priv *priv = fe->tuner_priv;
+ int v_agc = 0, ret;
+ u8 agc_pwm;

- regmap_read(priv->regmap, 0x3d, &utmp);
- rfgain = utmp & 0x1f;
- regmap_read(priv->regmap, 0x21, &utmp);
- bbgain = utmp & 0x1f;
+ /* Read the AGC PWM rate from the demodulator */
+ if (priv->get_agc_pwm) {
+ ret = priv->get_agc_pwm(fe, &agc_pwm);
+ if (ret < 0)
+ return ret;

- if (rfgain > 15)
- rfgain = 15;
- if (bbgain > 13)
- bbgain = 13;
+ switch (priv->tuner) {
+ case TS2020_M88TS2020:
+ v_agc = (int)agc_pwm * 20 - 1166;
+ break;
+ case TS2020_M88TS2022:
+ v_agc = (int)agc_pwm * 16 - 670;
+ break;
+ }

- sig_reading = rfgain * 2 + bbgain * 3;
+ if (v_agc < 0)
+ v_agc = 0;
+ }

- sig_strength = 40 + (64 - sig_reading) * 50 / 64 ;
+ return ts2020_read_tuner_gain(fe, v_agc, _gain);
+}

- /* cook the value to be suitable for szap-s2 human readable output */
- *signal_strength = sig_strength * 1000;
+/*
+ * Read TS2020 signal strength in v3 format.
+ */
+static int ts2020_read_signal_strength(struct dvb_frontend *fe,
+ u16 *signal_strength)
+{
+ unsigned strength;
+ __s64 gain;
+ int ret;
+
+ /* Determine the total gain of the tuner */
+ ret = ts2020_get_tuner_gain(fe, &gain);
+ if (ret < 0)
+ return ret;
+
+ /* Calculate the signal strength based on the total gain of the tuner */
+ if (gain < -85000)
+ /* 0%: no signal or weak signal */
+ strength = 0;
+ else if (gain < -65000)
+ /* 0% - 60%: weak signal */
+ strength = 0 + (85000 + gain) * 3 / 1000;
+ else if (gain < -45000)
+ /* 60% - 90%: normal signal */
+ strength = 60 + (65000 + gain) * 3 / 2000;
+ else
+ /* 90% - 99%: strong signal */
+ strength = 90 + (45000 + gain) / 5000;

+ *signal_strength = strength * 65535 / 100;
return 0;
}

@@ -442,6 +543,7 @@ static int ts2020_probe(struct i2c_client *client,
dev->clk_out_div = pdata->clk_out_div;
dev->frequency_div = pdata->frequency_div;
dev->fe = fe;
+ dev->get_agc_pwm = pdata->get_agc_pwm;
fe->tuner_priv = dev;
dev->client = client;

diff --git a/drivers/media/dvb-frontends/ts2020.h b/drivers/media/dvb-frontends/ts2020.h
index 8247ba5..1d7c55e 100644
--- a/drivers/media/dvb-frontends/ts2020.h
+++ b/drivers/media/dvb-frontends/ts2020.h
@@ -57,6 +57,11 @@ struct ts2020_config {
* driver private, do not set value
*/
u8 attach_in_use:1;
+
+ /* Operation to be called by the ts2020 driver to get the value of the
+ * AGC PWM tuner input as theoretically output by the demodulator.
+ */
+ int (*get_agc_pwm)(struct dvb_frontend *fe, u8 *_agc_pwm);
};

/* Do not add new ts2020_attach() users! Use I2C bindings instead. */
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 745caab..f5e3392 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -1857,6 +1857,7 @@ static int dvb_register(struct cx23885_tsport *port)
/* attach tuner */
memset(&ts2020_config, 0, sizeof(ts2020_config));
ts2020_config.fe = fe0->dvb.frontend;
+ ts2020_config.get_agc_pwm = m88ds3103_get_agc_pwm;
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "ts2020", I2C_NAME_SIZE);
info.addr = 0x60;
@@ -1986,6 +1987,7 @@ static int dvb_register(struct cx23885_tsport *port)
/* attach tuner */
memset(&ts2020_config, 0, sizeof(ts2020_config));
ts2020_config.fe = fe0->dvb.frontend;
+ ts2020_config.get_agc_pwm = m88ds3103_get_agc_pwm;
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "ts2020", I2C_NAME_SIZE);
info.addr = 0x60;
@@ -2031,6 +2033,7 @@ static int dvb_register(struct cx23885_tsport *port)
/* attach tuner */
memset(&ts2020_config, 0, sizeof(ts2020_config));
ts2020_config.fe = fe0->dvb.frontend;
+ ts2020_config.get_agc_pwm = m88ds3103_get_agc_pwm;
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "ts2020", I2C_NAME_SIZE);
info.addr = 0x60;
diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index cdf59bc..6b1bc24 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -331,6 +331,7 @@ static int dvbsky_s960_attach(struct dvb_usb_adapter *adap)

/* attach tuner */
ts2020_config.fe = adap->fe[0];
+ ts2020_config.get_agc_pwm = m88ds3103_get_agc_pwm;
strlcpy(info.type, "ts2020", I2C_NAME_SIZE);
info.addr = 0x60;
info.platform_data = &ts2020_config;
@@ -453,6 +454,7 @@ static int dvbsky_s960c_attach(struct dvb_usb_adapter *adap)

/* attach tuner */
ts2020_config.fe = adap->fe[0];
+ ts2020_config.get_agc_pwm = m88ds3103_get_agc_pwm;
strlcpy(info.type, "ts2020", I2C_NAME_SIZE);
info.addr = 0x60;
info.platform_data = &ts2020_config;


2015-05-26 15:04:25

by David Howells

[permalink] [raw]
Subject: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

Provide a DVBv5 API signal strength. This is in units of 0.001 dBm rather
than a percentage.

>From Antti Palosaari's testing with a signal generator, it appears that the
gain calculated according to Montage's specification if negated is a
reasonable representation of the signal strength of the generator.

To this end:

(1) Polled statistic gathering needed to be implemented in the TS2020 driver.
This is done in the ts2020_stat_work() function.

(2) The calculated gain is placed as the signal strength in the
dtv_property_cache associated with the front end with the scale set to
FE_SCALE_DECIBEL.

(3) The DVBv3 format signal strength then needed to be calculated from the
signal strength stored in the dtv_property_cache rather than accessing
the value when ts2020_read_signal_strength() is called.

Signed-off-by: David Howells <[email protected]>
---

drivers/media/dvb-frontends/ts2020.c | 62 +++++++++++++++++++++++++++++-----
1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb-frontends/ts2020.c b/drivers/media/dvb-frontends/ts2020.c
index 277e1cf..80ae039 100644
--- a/drivers/media/dvb-frontends/ts2020.c
+++ b/drivers/media/dvb-frontends/ts2020.c
@@ -32,10 +32,11 @@ struct ts2020_priv {
struct regmap_config regmap_config;
struct regmap *regmap;
struct dvb_frontend *fe;
+ struct delayed_work stat_work;
int (*get_agc_pwm)(struct dvb_frontend *fe, u8 *_agc_pwm);
/* i2c details */
- int i2c_address;
struct i2c_adapter *i2c;
+ int i2c_address;
u8 clk_out:2;
u8 clk_out_div:5;
u32 frequency_div; /* LO output divider switch frequency */
@@ -65,6 +66,7 @@ static int ts2020_release(struct dvb_frontend *fe)
static int ts2020_sleep(struct dvb_frontend *fe)
{
struct ts2020_priv *priv = fe->tuner_priv;
+ int ret;
u8 u8tmp;

if (priv->tuner == TS2020_M88TS2020)
@@ -72,11 +74,18 @@ static int ts2020_sleep(struct dvb_frontend *fe)
else
u8tmp = 0x00;

- return regmap_write(priv->regmap, u8tmp, 0x00);
+ ret = regmap_write(priv->regmap, u8tmp, 0x00);
+ if (ret < 0)
+ return ret;
+
+ /* stop statistics polling */
+ cancel_delayed_work_sync(&priv->stat_work);
+ return 0;
}

static int ts2020_init(struct dvb_frontend *fe)
{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
struct ts2020_priv *priv = fe->tuner_priv;
int i;
u8 u8tmp;
@@ -138,6 +147,13 @@ static int ts2020_init(struct dvb_frontend *fe)
reg_vals[i].val);
}

+ /* Initialise v5 stats here */
+ c->strength.len = 1;
+ c->strength.stat[0].scale = FE_SCALE_DECIBEL;
+ c->strength.stat[0].uvalue = 0;
+
+ /* Start statistics polling */
+ schedule_delayed_work(&priv->stat_work, 0);
return 0;
}

@@ -411,19 +427,46 @@ static int ts2020_get_tuner_gain(struct dvb_frontend *fe, __s64 *_gain)
}

/*
+ * Gather statistics on a regular basis
+ */
+static void ts2020_stat_work(struct work_struct *work)
+{
+ struct ts2020_priv *priv = container_of(work, struct ts2020_priv,
+ stat_work.work);
+ struct i2c_client *client = priv->client;
+ struct dtv_frontend_properties *c = &priv->fe->dtv_property_cache;
+ int ret;
+
+ dev_dbg(&client->dev, "\n");
+
+ ret = ts2020_get_tuner_gain(priv->fe, &c->strength.stat[0].svalue);
+ if (ret < 0)
+ goto err;
+
+ c->strength.stat[0].scale = FE_SCALE_DECIBEL;
+
+ schedule_delayed_work(&priv->stat_work, msecs_to_jiffies(2000));
+ return;
+err:
+ dev_dbg(&client->dev, "failed=%d\n", ret);
+}
+
+/*
* Read TS2020 signal strength in v3 format.
*/
static int ts2020_read_signal_strength(struct dvb_frontend *fe,
- u16 *signal_strength)
+ u16 *_signal_strength)
{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
unsigned strength;
__s64 gain;
- int ret;

- /* Determine the total gain of the tuner */
- ret = ts2020_get_tuner_gain(fe, &gain);
- if (ret < 0)
- return ret;
+ if (c->strength.stat[0].scale == FE_SCALE_NOT_AVAILABLE) {
+ *_signal_strength = 0;
+ return 0;
+ }
+
+ gain = c->strength.stat[0].svalue;

/* Calculate the signal strength based on the total gain of the tuner */
if (gain < -85000)
@@ -439,7 +482,7 @@ static int ts2020_read_signal_strength(struct dvb_frontend *fe,
/* 90% - 99%: strong signal */
strength = 90 + (45000 + gain) / 5000;

- *signal_strength = strength * 65535 / 100;
+ *_signal_strength = strength * 65535 / 100;
return 0;
}

@@ -546,6 +589,7 @@ static int ts2020_probe(struct i2c_client *client,
dev->get_agc_pwm = pdata->get_agc_pwm;
fe->tuner_priv = dev;
dev->client = client;
+ INIT_DELAYED_WORK(&dev->stat_work, ts2020_stat_work);

/* check if the tuner is there */
ret = regmap_read(dev->regmap, 0x00, &utmp);

2015-05-26 18:59:00

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

On 26/05/15 16:04, David Howells wrote:
> Provide a DVBv5 API signal strength. This is in units of 0.001 dBm rather
> than a percentage.
>
>>From Antti Palosaari's testing with a signal generator, it appears that the
> gain calculated according to Montage's specification if negated is a
> reasonable representation of the signal strength of the generator.
>
> To this end:
>
> (1) Polled statistic gathering needed to be implemented in the TS2020 driver.
> This is done in the ts2020_stat_work() function.
>
> (2) The calculated gain is placed as the signal strength in the
> dtv_property_cache associated with the front end with the scale set to
> FE_SCALE_DECIBEL.
>
> (3) The DVBv3 format signal strength then needed to be calculated from the
> signal strength stored in the dtv_property_cache rather than accessing
> the value when ts2020_read_signal_strength() is called.
>
> Signed-off-by: David Howells <[email protected]>
> ---
>
> drivers/media/dvb-frontends/ts2020.c | 62 +++++++++++++++++++++++++++++-----
> 1 file changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/ts2020.c b/drivers/media/dvb-frontends/ts2020.c
> index 277e1cf..80ae039 100644
> --- a/drivers/media/dvb-frontends/ts2020.c
> +++ b/drivers/media/dvb-frontends/ts2020.c
> @@ -32,10 +32,11 @@ struct ts2020_priv {
> struct regmap_config regmap_config;
> struct regmap *regmap;
> struct dvb_frontend *fe;
> + struct delayed_work stat_work;
> int (*get_agc_pwm)(struct dvb_frontend *fe, u8 *_agc_pwm);
> /* i2c details */
> - int i2c_address;
> struct i2c_adapter *i2c;
> + int i2c_address;
> u8 clk_out:2;
> u8 clk_out_div:5;
> u32 frequency_div; /* LO output divider switch frequency */
> @@ -65,6 +66,7 @@ static int ts2020_release(struct dvb_frontend *fe)
> static int ts2020_sleep(struct dvb_frontend *fe)
> {
> struct ts2020_priv *priv = fe->tuner_priv;
> + int ret;
> u8 u8tmp;
>
> if (priv->tuner == TS2020_M88TS2020)
> @@ -72,11 +74,18 @@ static int ts2020_sleep(struct dvb_frontend *fe)
> else
> u8tmp = 0x00;
>
> - return regmap_write(priv->regmap, u8tmp, 0x00);
> + ret = regmap_write(priv->regmap, u8tmp, 0x00);
> + if (ret < 0)
> + return ret;
> +
> + /* stop statistics polling */
> + cancel_delayed_work_sync(&priv->stat_work);
> + return 0;
> }
>
> static int ts2020_init(struct dvb_frontend *fe)
> {
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> struct ts2020_priv *priv = fe->tuner_priv;
> int i;
> u8 u8tmp;
> @@ -138,6 +147,13 @@ static int ts2020_init(struct dvb_frontend *fe)
> reg_vals[i].val);
> }
>
> + /* Initialise v5 stats here */
> + c->strength.len = 1;
> + c->strength.stat[0].scale = FE_SCALE_DECIBEL;
> + c->strength.stat[0].uvalue = 0;
> +
> + /* Start statistics polling */
> + schedule_delayed_work(&priv->stat_work, 0);
> return 0;
> }
>

Hi David

Statistics polling can not be done by lmedm04 driver's implementation of
M88RS2000/TS2020 because I2C messages stop the devices demuxer.

So any polling must be a config option for this driver.

Regards

Malcolm




2015-06-03 10:18:04

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

Malcolm Priestley <[email protected]> wrote:

> Yes, also, the workqueue appears not to be initialized when using the dvb
> attached method.

I'm not sure what you're referring to. It's initialised in ts2020_probe()
just after the ts2020_priv struct is allocated - the only place it is
allocated.

David

2015-06-03 11:13:38

by Antti Palosaari

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

On 05/28/2015 11:07 PM, Malcolm Priestley wrote:
> On 28/05/15 11:08, David Howells wrote:
>> Malcolm Priestley <[email protected]> wrote:
>>
>>> Statistics polling can not be done by lmedm04 driver's implementation of
>>> M88RS2000/TS2020 because I2C messages stop the devices demuxer.

I did make tests (using that same lme2510 + rs2000 device) and didn't
saw the issue TS was lost. Could test and and tell me how to reproduce it?
Signal strength returned was quite boring though, about same value all
the time, but it is different issue...

>>>
>>> So any polling must be a config option for this driver.
>>
>> Ummm... I presume a runtime config option is okay.
>
> Yes, also, the workqueue appears not to be initialized when using the
> dvb attached method.
>
>>
>> Also, does that mean that the lmedm04 driver can't be made compatible
>> with the
>> DVBv5 API?
>
> No, the driver will have to implement its own version. It doesn't need a
> polling thread it simply gets it directly from its interrupt urb buffer.

I assume lme2510 firmware will read signal strength from rs2000 and it
is returned then directly by USB interface.

regards
Antti

--
http://palosaari.fi/

2015-06-03 11:35:14

by David Howells

[permalink] [raw]
Subject: [PATCH] ts2020: Allow stats polling to be suppressed

Statistics polling can not be done by lmedm04 driver's implementation of
M88RS2000/TS2020 because I2C messages stop the device's demuxer, so allow
polling for statistics to be suppressed in the ts2020 driver by setting
dont_poll in the ts2020_config struct.

Reported-by: Malcolm Priestley <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Malcolm Priestley <[email protected]>
---

drivers/media/dvb-frontends/ts2020.c | 18 ++++++++++++++----
drivers/media/dvb-frontends/ts2020.h | 3 +++
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/ts2020.c b/drivers/media/dvb-frontends/ts2020.c
index 8c997d0..946d8e950 100644
--- a/drivers/media/dvb-frontends/ts2020.c
+++ b/drivers/media/dvb-frontends/ts2020.c
@@ -40,6 +40,7 @@ struct ts2020_priv {
bool loop_through:1;
u8 clk_out:2;
u8 clk_out_div:5;
+ bool dont_poll:1;
u32 frequency_div; /* LO output divider switch frequency */
u32 frequency_khz; /* actual used LO frequency */
#define TS2020_M88TS2020 0
@@ -52,6 +53,8 @@ struct ts2020_reg_val {
u8 val;
};

+static void ts2020_stat_work(struct work_struct *work);
+
static int ts2020_release(struct dvb_frontend *fe)
{
struct ts2020_priv *priv = fe->tuner_priv;
@@ -79,7 +82,8 @@ static int ts2020_sleep(struct dvb_frontend *fe)
return ret;

/* stop statistics polling */
- cancel_delayed_work_sync(&priv->stat_work);
+ if (!priv->dont_poll)
+ cancel_delayed_work_sync(&priv->stat_work);
return 0;
}

@@ -152,8 +156,8 @@ static int ts2020_init(struct dvb_frontend *fe)
c->strength.stat[0].scale = FE_SCALE_DECIBEL;
c->strength.stat[0].uvalue = 0;

- /* Start statistics polling */
- schedule_delayed_work(&priv->stat_work, 0);
+ /* Start statistics polling by invoking the work function */
+ ts2020_stat_work(&priv->stat_work.work);
return 0;
}

@@ -445,7 +449,8 @@ static void ts2020_stat_work(struct work_struct *work)

c->strength.stat[0].scale = FE_SCALE_DECIBEL;

- schedule_delayed_work(&priv->stat_work, msecs_to_jiffies(2000));
+ if (!priv->dont_poll)
+ schedule_delayed_work(&priv->stat_work, msecs_to_jiffies(2000));
return;
err:
dev_dbg(&client->dev, "failed=%d\n", ret);
@@ -458,9 +463,13 @@ static int ts2020_read_signal_strength(struct dvb_frontend *fe,
u16 *_signal_strength)
{
struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+ struct ts2020_priv *priv = fe->tuner_priv;
unsigned strength;
__s64 gain;

+ if (priv->dont_poll)
+ ts2020_stat_work(&priv->stat_work.work);
+
if (c->strength.stat[0].scale == FE_SCALE_NOT_AVAILABLE) {
*_signal_strength = 0;
return 0;
@@ -585,6 +594,7 @@ static int ts2020_probe(struct i2c_client *client,
dev->loop_through = pdata->loop_through;
dev->clk_out = pdata->clk_out;
dev->clk_out_div = pdata->clk_out_div;
+ dev->dont_poll = pdata->dont_poll;
dev->frequency_div = pdata->frequency_div;
dev->fe = fe;
dev->get_agc_pwm = pdata->get_agc_pwm;
diff --git a/drivers/media/dvb-frontends/ts2020.h b/drivers/media/dvb-frontends/ts2020.h
index 002bc0a..9220e5c 100644
--- a/drivers/media/dvb-frontends/ts2020.h
+++ b/drivers/media/dvb-frontends/ts2020.h
@@ -48,6 +48,9 @@ struct ts2020_config {
*/
u8 clk_out_div:5;

+ /* Set to true to suppress stat polling */
+ bool dont_poll:1;
+
/*
* pointer to DVB frontend
*/

2015-06-03 15:15:27

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength



On 03/06/15 11:17, David Howells wrote:
> Malcolm Priestley <[email protected]> wrote:
>
>> Yes, also, the workqueue appears not to be initialized when using the dvb
>> attached method.
>
> I'm not sure what you're referring to. It's initialised in ts2020_probe()
> just after the ts2020_priv struct is allocated - the only place it is
> allocated.
>
ts2020_probe() isn't touched by devices not converted to I2C binding.

2015-06-03 15:57:22

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength



On 03/06/15 12:13, Antti Palosaari wrote:
> On 05/28/2015 11:07 PM, Malcolm Priestley wrote:
>> On 28/05/15 11:08, David Howells wrote:
>>> Malcolm Priestley <[email protected]> wrote:
>>>
>>>> Statistics polling can not be done by lmedm04 driver's
>>>> implementation of
>>>> M88RS2000/TS2020 because I2C messages stop the devices demuxer.
>
> I did make tests (using that same lme2510 + rs2000 device) and didn't
> saw the issue TS was lost. Could test and and tell me how to reproduce it?
> Signal strength returned was quite boring though, about same value all
> the time, but it is different issue...
Hi Antti

The workqueue is not working because ts2020_probe() isn't called.

I am thinking that other drivers that still use dvb_attach may be broken.

It will become an issue when the driver is converted to I2C binding.

Regards


Malcolm

2015-06-03 16:37:31

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

Malcolm Priestley <[email protected]> wrote:

> >> Yes, also, the workqueue appears not to be initialized when using the dvb
> >> attached method.
> >
> > I'm not sure what you're referring to. It's initialised in ts2020_probe()
> > just after the ts2020_priv struct is allocated - the only place it is
> > allocated.
> >
> ts2020_probe() isn't touched by devices not converted to I2C binding.

Hmmm... Doesn't that expose a larger problem? The only place the ts2020_priv
struct is allocated is in ts2020_probe() within ts2020.c and the struct
definition is private to that file and so it can't be allocated from outside.
So if you don't pass through ts2020_probe(), fe->tuner_priv will remain NULL
and the driver will crash.

David

2015-06-03 16:43:43

by Antti Palosaari

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

On 06/03/2015 07:37 PM, David Howells wrote:
> Malcolm Priestley <[email protected]> wrote:
>
>>>> Yes, also, the workqueue appears not to be initialized when using the dvb
>>>> attached method.
>>>
>>> I'm not sure what you're referring to. It's initialised in ts2020_probe()
>>> just after the ts2020_priv struct is allocated - the only place it is
>>> allocated.
>>>
>> ts2020_probe() isn't touched by devices not converted to I2C binding.
>
> Hmmm... Doesn't that expose a larger problem? The only place the ts2020_priv
> struct is allocated is in ts2020_probe() within ts2020.c and the struct
> definition is private to that file and so it can't be allocated from outside.
> So if you don't pass through ts2020_probe(), fe->tuner_priv will remain NULL
> and the driver will crash.

Malcolm misses some pending patches where attach() is wrapped to I2C
model probe().
http://git.linuxtv.org/cgit.cgi/anttip/media_tree.git/log/?h=ts2020

regards
Antti

--
http://palosaari.fi/

2015-06-03 16:44:56

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength



On 03/06/15 17:37, David Howells wrote:
> Malcolm Priestley <[email protected]> wrote:
>
>>>> Yes, also, the workqueue appears not to be initialized when using the dvb
>>>> attached method.
>>>
>>> I'm not sure what you're referring to. It's initialised in ts2020_probe()
>>> just after the ts2020_priv struct is allocated - the only place it is
>>> allocated.
>>>
>> ts2020_probe() isn't touched by devices not converted to I2C binding.
>
> Hmmm... Doesn't that expose a larger problem? The only place the ts2020_priv
> struct is allocated is in ts2020_probe() within ts2020.c and the struct
> definition is private to that file and so it can't be allocated from outside.
> So if you don't pass through ts2020_probe(), fe->tuner_priv will remain NULL
> and the driver will crash.
>

No, it is also allocated in ts2020_attach.

2015-06-03 17:04:34

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength



On 03/06/15 17:43, Antti Palosaari wrote:
> On 06/03/2015 07:37 PM, David Howells wrote:
>> Malcolm Priestley <[email protected]> wrote:
>>
>>>>> Yes, also, the workqueue appears not to be initialized when using
>>>>> the dvb
>>>>> attached method.
>>>>
>>>> I'm not sure what you're referring to. It's initialised in
>>>> ts2020_probe()
>>>> just after the ts2020_priv struct is allocated - the only place it is
>>>> allocated.
>>>>
>>> ts2020_probe() isn't touched by devices not converted to I2C binding.
>>
>> Hmmm... Doesn't that expose a larger problem? The only place the
>> ts2020_priv
>> struct is allocated is in ts2020_probe() within ts2020.c and the struct
>> definition is private to that file and so it can't be allocated from
>> outside.
>> So if you don't pass through ts2020_probe(), fe->tuner_priv will
>> remain NULL
>> and the driver will crash.
>
> Malcolm misses some pending patches where attach() is wrapped to I2C
> model probe().
> http://git.linuxtv.org/cgit.cgi/anttip/media_tree.git/log/?h=ts2020
>

Hmmm... Yes, I am indeed missing those patches.

I will test.

2015-06-03 17:16:08

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 2/2] ts2020: Provide DVBv5 API signal strength

Antti Palosaari <[email protected]> wrote:

> Malcolm misses some pending patches where attach() is wrapped to I2C model
> probe().
> http://git.linuxtv.org/cgit.cgi/anttip/media_tree.git/log/?h=ts2020

Aha! That explains it.

ts2020: register I2C driver from legacy media attach

removes the allocation from attach() in the branch I'm working on top of.

David

2015-06-03 18:33:38

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [PATCH] ts2020: Allow stats polling to be suppressed



On 03/06/15 12:35, David Howells wrote:
> Statistics polling can not be done by lmedm04 driver's implementation of
> M88RS2000/TS2020 because I2C messages stop the device's demuxer, so allow
> polling for statistics to be suppressed in the ts2020 driver by setting
> dont_poll in the ts2020_config struct.

Hi David

As expected the lmedm04 driver needs this patch along with another patch
to enable it for the driver which I will post shortly.

Otherwise everything is working fine on Antti's ts2020 branch

Regards


Malcolm