All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more)
@ 2015-05-21 19:22 Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready Antti Palosaari
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

v2
Meld some commits together.

Antti Palosaari (5):
  m88ds3103: do not return error from get_frontend() when not ready
  m88ds3103: implement DVBv5 CNR statistics
  m88ds3103: implement DVBv5 BER
  m88ds3103: use jiffies when polling DiSEqC TX ready
  m88ds3103: add I2C client binding

 drivers/media/dvb-frontends/m88ds3103.c      | 642 ++++++++++++++++-----------
 drivers/media/dvb-frontends/m88ds3103.h      |  63 ++-
 drivers/media/dvb-frontends/m88ds3103_priv.h |   6 +-
 3 files changed, 456 insertions(+), 255 deletions(-)

-- 
http://palosaari.fi/


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

* [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready
  2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
@ 2015-05-21 19:22 ` Antti Palosaari
  2015-05-30 15:02   ` Mauro Carvalho Chehab
  2015-05-21 19:22 ` [PATCHv2 2/5] m88ds3103: implement DVBv5 CNR statistics Antti Palosaari
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Do not return error from get_frontend() when status is queried, but
device is not ready. dvbv5-zap has habit to call that IOCTL before
device is tuned and it also refuses to use DVBv5 statistic after
that...

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/m88ds3103.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index d3d928e..03dceb5 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -742,7 +742,7 @@ static int m88ds3103_get_frontend(struct dvb_frontend *fe)
 	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
 
 	if (!priv->warm || !(priv->fe_status & FE_HAS_LOCK)) {
-		ret = -EAGAIN;
+		ret = 0;
 		goto err;
 	}
 
-- 
http://palosaari.fi/


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

* [PATCHv2 2/5] m88ds3103: implement DVBv5 CNR statistics
  2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready Antti Palosaari
@ 2015-05-21 19:22 ` Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 3/5] m88ds3103: implement DVBv5 BER Antti Palosaari
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Implement DVBv5 CNR statistics.
Wrap legacy DVBv3 SNR to DVBv5 CNR.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/m88ds3103.c | 154 +++++++++++++++++---------------
 1 file changed, 82 insertions(+), 72 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 03dceb5..381a8ad 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -190,8 +190,9 @@ static int m88ds3103_read_status(struct dvb_frontend *fe, fe_status_t *status)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-	int ret;
+	int ret, i, itmp;
 	u8 u8tmp;
+	u8 buf[3];
 
 	*status = 0;
 
@@ -233,6 +234,77 @@ static int m88ds3103_read_status(struct dvb_frontend *fe, fe_status_t *status)
 	dev_dbg(&priv->i2c->dev, "%s: lock=%02x status=%02x\n",
 			__func__, u8tmp, *status);
 
+	/* CNR */
+	if (priv->fe_status & FE_HAS_VITERBI) {
+		unsigned int cnr, noise, signal, noise_tot, signal_tot;
+
+		cnr = 0;
+		/* more iterations for more accurate estimation */
+		#define M88DS3103_SNR_ITERATIONS 3
+
+		switch (c->delivery_system) {
+		case SYS_DVBS:
+			itmp = 0;
+
+			for (i = 0; i < M88DS3103_SNR_ITERATIONS; i++) {
+				ret = m88ds3103_rd_reg(priv, 0xff, &buf[0]);
+				if (ret)
+					goto err;
+
+				itmp += buf[0];
+			}
+
+			/* use of single register limits max value to 15 dB */
+			/* SNR(X) dB = 10 * ln(X) / ln(10) dB */
+			itmp = DIV_ROUND_CLOSEST(itmp, 8 * M88DS3103_SNR_ITERATIONS);
+			if (itmp)
+				cnr = div_u64((u64) 10000 * intlog2(itmp), intlog2(10));
+			break;
+		case SYS_DVBS2:
+			noise_tot = 0;
+			signal_tot = 0;
+
+			for (i = 0; i < M88DS3103_SNR_ITERATIONS; i++) {
+				ret = m88ds3103_rd_regs(priv, 0x8c, buf, 3);
+				if (ret)
+					goto err;
+
+				noise = buf[1] << 6;    /* [13:6] */
+				noise |= buf[0] & 0x3f; /*  [5:0] */
+				noise >>= 2;
+				signal = buf[2] * buf[2];
+				signal >>= 1;
+
+				noise_tot += noise;
+				signal_tot += signal;
+			}
+
+			noise = noise_tot / M88DS3103_SNR_ITERATIONS;
+			signal = signal_tot / M88DS3103_SNR_ITERATIONS;
+
+			/* SNR(X) dB = 10 * log10(X) dB */
+			if (signal > noise) {
+				itmp = signal / noise;
+				cnr = div_u64((u64) 10000 * intlog10(itmp), (1 << 24));
+			}
+			break;
+		default:
+			dev_dbg(&priv->i2c->dev,
+				"%s: invalid delivery_system\n", __func__);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		if (cnr) {
+			c->cnr.stat[0].scale = FE_SCALE_DECIBEL;
+			c->cnr.stat[0].svalue = cnr;
+		} else {
+			c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		}
+	} else {
+		c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	}
+
 	return 0;
 err:
 	dev_dbg(&priv->i2c->dev, "%s: failed=%d\n", __func__, ret);
@@ -577,6 +649,7 @@ err:
 static int m88ds3103_init(struct dvb_frontend *fe)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret, len, remaining;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
@@ -684,7 +757,9 @@ static int m88ds3103_init(struct dvb_frontend *fe)
 skip_fw_download:
 	/* warm state */
 	priv->warm = true;
-
+	/* init stats here in order signal app which stats are supported */
+	c->cnr.len = 1;
+	c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 	return 0;
 
 error_fw_release:
@@ -702,6 +777,7 @@ static int m88ds3103_sleep(struct dvb_frontend *fe)
 
 	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
 
+	priv->fe_status = 0;
 	priv->delivery_system = SYS_UNDEFINED;
 
 	/* TS Hi-Z */
@@ -908,80 +984,14 @@ err:
 
 static int m88ds3103_read_snr(struct dvb_frontend *fe, u16 *snr)
 {
-	struct m88ds3103_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-	int ret, i, tmp;
-	u8 buf[3];
-	u16 noise, signal;
-	u32 noise_tot, signal_tot;
 
-	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
-	/* reports SNR in resolution of 0.1 dB */
-
-	/* more iterations for more accurate estimation */
-	#define M88DS3103_SNR_ITERATIONS 3
-
-	switch (c->delivery_system) {
-	case SYS_DVBS:
-		tmp = 0;
-
-		for (i = 0; i < M88DS3103_SNR_ITERATIONS; i++) {
-			ret = m88ds3103_rd_reg(priv, 0xff, &buf[0]);
-			if (ret)
-				goto err;
-
-			tmp += buf[0];
-		}
-
-		/* use of one register limits max value to 15 dB */
-		/* SNR(X) dB = 10 * ln(X) / ln(10) dB */
-		tmp = DIV_ROUND_CLOSEST(tmp, 8 * M88DS3103_SNR_ITERATIONS);
-		if (tmp)
-			*snr = div_u64((u64) 100 * intlog2(tmp), intlog2(10));
-		else
-			*snr = 0;
-		break;
-	case SYS_DVBS2:
-		noise_tot = 0;
-		signal_tot = 0;
-
-		for (i = 0; i < M88DS3103_SNR_ITERATIONS; i++) {
-			ret = m88ds3103_rd_regs(priv, 0x8c, buf, 3);
-			if (ret)
-				goto err;
-
-			noise = buf[1] << 6;    /* [13:6] */
-			noise |= buf[0] & 0x3f; /*  [5:0] */
-			noise >>= 2;
-			signal = buf[2] * buf[2];
-			signal >>= 1;
-
-			noise_tot += noise;
-			signal_tot += signal;
-		}
-
-		noise = noise_tot / M88DS3103_SNR_ITERATIONS;
-		signal = signal_tot / M88DS3103_SNR_ITERATIONS;
-
-		/* SNR(X) dB = 10 * log10(X) dB */
-		if (signal > noise) {
-			tmp = signal / noise;
-			*snr = div_u64((u64) 100 * intlog10(tmp), (1 << 24));
-		} else {
-			*snr = 0;
-		}
-		break;
-	default:
-		dev_dbg(&priv->i2c->dev, "%s: invalid delivery_system\n",
-				__func__);
-		ret = -EINVAL;
-		goto err;
-	}
+	if (c->cnr.stat[0].scale == FE_SCALE_DECIBEL)
+		*snr = div_s64(c->cnr.stat[0].svalue, 100);
+	else
+		*snr = 0;
 
 	return 0;
-err:
-	dev_dbg(&priv->i2c->dev, "%s: failed=%d\n", __func__, ret);
-	return ret;
 }
 
 static int m88ds3103_read_ber(struct dvb_frontend *fe, u32 *ber)
-- 
http://palosaari.fi/


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

* [PATCHv2 3/5] m88ds3103: implement DVBv5 BER
  2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 2/5] m88ds3103: implement DVBv5 CNR statistics Antti Palosaari
@ 2015-05-21 19:22 ` Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 4/5] m88ds3103: use jiffies when polling DiSEqC TX ready Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 5/5] m88ds3103: add I2C client binding Antti Palosaari
  4 siblings, 0 replies; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Implement DVBv5 BER statistics.
Wrap legacy DVBv3 BER to DVBv5 BER.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/m88ds3103.c      | 165 +++++++++++++++------------
 drivers/media/dvb-frontends/m88ds3103_priv.h |   4 +-
 2 files changed, 94 insertions(+), 75 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 381a8ad..33d8c19 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -305,6 +305,92 @@ static int m88ds3103_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 	}
 
+	/* BER */
+	if (priv->fe_status & FE_HAS_LOCK) {
+		unsigned int utmp, post_bit_error, post_bit_count;
+
+		switch (c->delivery_system) {
+		case SYS_DVBS:
+			ret = m88ds3103_wr_reg(priv, 0xf9, 0x04);
+			if (ret)
+				goto err;
+
+			ret = m88ds3103_rd_reg(priv, 0xf8, &u8tmp);
+			if (ret)
+				goto err;
+
+			/* measurement ready? */
+			if (!(u8tmp & 0x10)) {
+				ret = m88ds3103_rd_regs(priv, 0xf6, buf, 2);
+				if (ret)
+					goto err;
+
+				post_bit_error = buf[1] << 8 | buf[0] << 0;
+				post_bit_count = 0x800000;
+				priv->post_bit_error += post_bit_error;
+				priv->post_bit_count += post_bit_count;
+				priv->dvbv3_ber = post_bit_error;
+
+				/* restart measurement */
+				u8tmp |= 0x10;
+				ret = m88ds3103_wr_reg(priv, 0xf8, u8tmp);
+				if (ret)
+					goto err;
+			}
+			break;
+		case SYS_DVBS2:
+			ret = m88ds3103_rd_regs(priv, 0xd5, buf, 3);
+			if (ret)
+				goto err;
+
+			utmp = buf[2] << 16 | buf[1] << 8 | buf[0] << 0;
+
+			/* enough data? */
+			if (utmp > 4000) {
+				ret = m88ds3103_rd_regs(priv, 0xf7, buf, 2);
+				if (ret)
+					goto err;
+
+				post_bit_error = buf[1] << 8 | buf[0] << 0;
+				post_bit_count = 32 * utmp; /* TODO: FEC */
+				priv->post_bit_error += post_bit_error;
+				priv->post_bit_count += post_bit_count;
+				priv->dvbv3_ber = post_bit_error;
+
+				/* restart measurement */
+				ret = m88ds3103_wr_reg(priv, 0xd1, 0x01);
+				if (ret)
+					goto err;
+
+				ret = m88ds3103_wr_reg(priv, 0xf9, 0x01);
+				if (ret)
+					goto err;
+
+				ret = m88ds3103_wr_reg(priv, 0xf9, 0x00);
+				if (ret)
+					goto err;
+
+				ret = m88ds3103_wr_reg(priv, 0xd1, 0x00);
+				if (ret)
+					goto err;
+			}
+			break;
+		default:
+			dev_dbg(&priv->i2c->dev,
+				"%s: invalid delivery_system\n", __func__);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		c->post_bit_error.stat[0].scale = FE_SCALE_COUNTER;
+		c->post_bit_error.stat[0].uvalue = priv->post_bit_error;
+		c->post_bit_count.stat[0].scale = FE_SCALE_COUNTER;
+		c->post_bit_count.stat[0].uvalue = priv->post_bit_count;
+	} else {
+		c->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+		c->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	}
+
 	return 0;
 err:
 	dev_dbg(&priv->i2c->dev, "%s: failed=%d\n", __func__, ret);
@@ -760,6 +846,10 @@ skip_fw_download:
 	/* init stats here in order signal app which stats are supported */
 	c->cnr.len = 1;
 	c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	c->post_bit_error.len = 1;
+	c->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	c->post_bit_count.len = 1;
+	c->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 	return 0;
 
 error_fw_release:
@@ -997,83 +1087,10 @@ static int m88ds3103_read_snr(struct dvb_frontend *fe, u16 *snr)
 static int m88ds3103_read_ber(struct dvb_frontend *fe, u32 *ber)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
-	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-	int ret;
-	unsigned int utmp;
-	u8 buf[3], u8tmp;
 
-	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
-
-	switch (c->delivery_system) {
-	case SYS_DVBS:
-		ret = m88ds3103_wr_reg(priv, 0xf9, 0x04);
-		if (ret)
-			goto err;
-
-		ret = m88ds3103_rd_reg(priv, 0xf8, &u8tmp);
-		if (ret)
-			goto err;
-
-		if (!(u8tmp & 0x10)) {
-			u8tmp |= 0x10;
-
-			ret = m88ds3103_rd_regs(priv, 0xf6, buf, 2);
-			if (ret)
-				goto err;
-
-			priv->ber = (buf[1] << 8) | (buf[0] << 0);
-
-			/* restart counters */
-			ret = m88ds3103_wr_reg(priv, 0xf8, u8tmp);
-			if (ret)
-				goto err;
-		}
-		break;
-	case SYS_DVBS2:
-		ret = m88ds3103_rd_regs(priv, 0xd5, buf, 3);
-		if (ret)
-			goto err;
-
-		utmp = (buf[2] << 16) | (buf[1] << 8) | (buf[0] << 0);
-
-		if (utmp > 3000) {
-			ret = m88ds3103_rd_regs(priv, 0xf7, buf, 2);
-			if (ret)
-				goto err;
-
-			priv->ber = (buf[1] << 8) | (buf[0] << 0);
-
-			/* restart counters */
-			ret = m88ds3103_wr_reg(priv, 0xd1, 0x01);
-			if (ret)
-				goto err;
-
-			ret = m88ds3103_wr_reg(priv, 0xf9, 0x01);
-			if (ret)
-				goto err;
-
-			ret = m88ds3103_wr_reg(priv, 0xf9, 0x00);
-			if (ret)
-				goto err;
-
-			ret = m88ds3103_wr_reg(priv, 0xd1, 0x00);
-			if (ret)
-				goto err;
-		}
-		break;
-	default:
-		dev_dbg(&priv->i2c->dev, "%s: invalid delivery_system\n",
-				__func__);
-		ret = -EINVAL;
-		goto err;
-	}
-
-	*ber = priv->ber;
+	*ber = priv->dvbv3_ber;
 
 	return 0;
-err:
-	dev_dbg(&priv->i2c->dev, "%s: failed=%d\n", __func__, ret);
-	return ret;
 }
 
 static int m88ds3103_set_tone(struct dvb_frontend *fe,
diff --git a/drivers/media/dvb-frontends/m88ds3103_priv.h b/drivers/media/dvb-frontends/m88ds3103_priv.h
index a2c0958..78e58e3 100644
--- a/drivers/media/dvb-frontends/m88ds3103_priv.h
+++ b/drivers/media/dvb-frontends/m88ds3103_priv.h
@@ -38,13 +38,15 @@ struct m88ds3103_priv {
 	struct dvb_frontend fe;
 	fe_delivery_system_t delivery_system;
 	fe_status_t fe_status;
-	u32 ber;
+	u32 dvbv3_ber; /* for old DVBv3 API read_ber */
 	bool warm; /* FW running */
 	struct i2c_adapter *i2c_adapter;
 	/* auto detect chip id to do different config */
 	u8 chip_id;
 	/* main mclk is calculated for M88RS6000 dynamically */
 	u32 mclk_khz;
+	u64 post_bit_error;
+	u64 post_bit_count;
 };
 
 struct m88ds3103_reg_val {
-- 
http://palosaari.fi/


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

* [PATCHv2 4/5] m88ds3103: use jiffies when polling DiSEqC TX ready
  2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
                   ` (2 preceding siblings ...)
  2015-05-21 19:22 ` [PATCHv2 3/5] m88ds3103: implement DVBv5 BER Antti Palosaari
@ 2015-05-21 19:22 ` Antti Palosaari
  2015-05-21 19:22 ` [PATCHv2 5/5] m88ds3103: add I2C client binding Antti Palosaari
  4 siblings, 0 replies; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Use jiffies to set timeout for DiSEqC TX ready polling. Using jiffies
is more elegant solution than looping N times with sleep.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/m88ds3103.c | 53 +++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index 33d8c19..e45641f 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -1195,7 +1195,8 @@ static int m88ds3103_diseqc_send_master_cmd(struct dvb_frontend *fe,
 		struct dvb_diseqc_master_cmd *diseqc_cmd)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
-	int ret, i;
+	int ret;
+	unsigned long timeout;
 	u8 u8tmp;
 
 	dev_dbg(&priv->i2c->dev, "%s: msg=%*ph\n", __func__,
@@ -1226,21 +1227,24 @@ static int m88ds3103_diseqc_send_master_cmd(struct dvb_frontend *fe,
 	if (ret)
 		goto err;
 
-	/* DiSEqC message typical period is 54 ms */
-	usleep_range(40000, 60000);
-
 	/* wait DiSEqC TX ready */
-	for (i = 20, u8tmp = 1; i && u8tmp; i--) {
-		usleep_range(5000, 10000);
+	#define SEND_MASTER_CMD_TIMEOUT 120
+	timeout = jiffies + msecs_to_jiffies(SEND_MASTER_CMD_TIMEOUT);
+
+	/* DiSEqC message typical period is 54 ms */
+	usleep_range(50000, 54000);
 
+	for (u8tmp = 1; !time_after(jiffies, timeout) && u8tmp;) {
 		ret = m88ds3103_rd_reg_mask(priv, 0xa1, &u8tmp, 0x40);
 		if (ret)
 			goto err;
 	}
 
-	dev_dbg(&priv->i2c->dev, "%s: loop=%d\n", __func__, i);
-
-	if (i == 0) {
+	if (u8tmp == 0) {
+		dev_dbg(&priv->i2c->dev, "%s: diseqc tx took %u ms\n", __func__,
+			jiffies_to_msecs(jiffies) -
+			(jiffies_to_msecs(timeout) - SEND_MASTER_CMD_TIMEOUT));
+	} else {
 		dev_dbg(&priv->i2c->dev, "%s: diseqc tx timeout\n", __func__);
 
 		ret = m88ds3103_wr_reg_mask(priv, 0xa1, 0x40, 0xc0);
@@ -1252,7 +1256,7 @@ static int m88ds3103_diseqc_send_master_cmd(struct dvb_frontend *fe,
 	if (ret)
 		goto err;
 
-	if (i == 0) {
+	if (u8tmp == 1) {
 		ret = -ETIMEDOUT;
 		goto err;
 	}
@@ -1267,7 +1271,8 @@ static int m88ds3103_diseqc_send_burst(struct dvb_frontend *fe,
 	fe_sec_mini_cmd_t fe_sec_mini_cmd)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
-	int ret, i;
+	int ret;
+	unsigned long timeout;
 	u8 u8tmp, burst;
 
 	dev_dbg(&priv->i2c->dev, "%s: fe_sec_mini_cmd=%d\n", __func__,
@@ -1301,26 +1306,36 @@ static int m88ds3103_diseqc_send_burst(struct dvb_frontend *fe,
 	if (ret)
 		goto err;
 
-	/* DiSEqC ToneBurst period is 12.5 ms */
-	usleep_range(11000, 20000);
-
 	/* wait DiSEqC TX ready */
-	for (i = 5, u8tmp = 1; i && u8tmp; i--) {
-		usleep_range(800, 2000);
+	#define SEND_BURST_TIMEOUT 40
+	timeout = jiffies + msecs_to_jiffies(SEND_BURST_TIMEOUT);
+
+	/* DiSEqC ToneBurst period is 12.5 ms */
+	usleep_range(8500, 12500);
 
+	for (u8tmp = 1; !time_after(jiffies, timeout) && u8tmp;) {
 		ret = m88ds3103_rd_reg_mask(priv, 0xa1, &u8tmp, 0x40);
 		if (ret)
 			goto err;
 	}
 
-	dev_dbg(&priv->i2c->dev, "%s: loop=%d\n", __func__, i);
+	if (u8tmp == 0) {
+		dev_dbg(&priv->i2c->dev, "%s: diseqc tx took %u ms\n", __func__,
+			jiffies_to_msecs(jiffies) -
+			(jiffies_to_msecs(timeout) - SEND_BURST_TIMEOUT));
+	} else {
+		dev_dbg(&priv->i2c->dev, "%s: diseqc tx timeout\n", __func__);
+
+		ret = m88ds3103_wr_reg_mask(priv, 0xa1, 0x40, 0xc0);
+		if (ret)
+			goto err;
+	}
 
 	ret = m88ds3103_wr_reg_mask(priv, 0xa2, 0x80, 0xc0);
 	if (ret)
 		goto err;
 
-	if (i == 0) {
-		dev_dbg(&priv->i2c->dev, "%s: diseqc tx timeout\n", __func__);
+	if (u8tmp == 1) {
 		ret = -ETIMEDOUT;
 		goto err;
 	}
-- 
http://palosaari.fi/


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

* [PATCHv2 5/5] m88ds3103: add I2C client binding
  2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
                   ` (3 preceding siblings ...)
  2015-05-21 19:22 ` [PATCHv2 4/5] m88ds3103: use jiffies when polling DiSEqC TX ready Antti Palosaari
@ 2015-05-21 19:22 ` Antti Palosaari
  4 siblings, 0 replies; 8+ messages in thread
From: Antti Palosaari @ 2015-05-21 19:22 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Implement I2C client device binding.
Wrap media attach to driver I2C probe.
Add wrapper from m88ds3103_attach() to m88ds3103_probe() via driver
core in order to provide proper I2C client for legacy media attach
binding.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/m88ds3103.c      | 268 +++++++++++++++++++--------
 drivers/media/dvb-frontends/m88ds3103.h      |  63 ++++++-
 drivers/media/dvb-frontends/m88ds3103_priv.h |   2 +
 3 files changed, 245 insertions(+), 88 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index e45641f..01b9ded 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -1357,9 +1357,9 @@ static int m88ds3103_get_tune_settings(struct dvb_frontend *fe,
 static void m88ds3103_release(struct dvb_frontend *fe)
 {
 	struct m88ds3103_priv *priv = fe->demodulator_priv;
+	struct i2c_client *client = priv->client;
 
-	i2c_del_mux_adapter(priv->i2c_adapter);
-	kfree(priv);
+	i2c_unregister_device(client);
 }
 
 static int m88ds3103_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
@@ -1401,43 +1401,158 @@ static int m88ds3103_deselect(struct i2c_adapter *adap, void *mux_priv,
 	return 0;
 }
 
+/*
+ * XXX: That is wrapper to m88ds3103_probe() via driver core in order to provide
+ * proper I2C client for legacy media attach binding.
+ * New users must use I2C client binding directly!
+ */
 struct dvb_frontend *m88ds3103_attach(const struct m88ds3103_config *cfg,
 		struct i2c_adapter *i2c, struct i2c_adapter **tuner_i2c_adapter)
 {
+	struct i2c_client *client;
+	struct i2c_board_info board_info;
+	struct m88ds3103_platform_data pdata;
+
+	pdata.clk = cfg->clock;
+	pdata.i2c_wr_max = cfg->i2c_wr_max;
+	pdata.ts_mode = cfg->ts_mode;
+	pdata.ts_clk = cfg->ts_clk;
+	pdata.ts_clk_pol = cfg->ts_clk_pol;
+	pdata.spec_inv = cfg->spec_inv;
+	pdata.agc = cfg->agc;
+	pdata.agc_inv = cfg->agc_inv;
+	pdata.clk_out = cfg->clock_out;
+	pdata.envelope_mode = cfg->envelope_mode;
+	pdata.lnb_hv_pol = cfg->lnb_hv_pol;
+	pdata.lnb_en_pol = cfg->lnb_en_pol;
+	pdata.attach_in_use = true;
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "m88ds3103", I2C_NAME_SIZE);
+	board_info.addr = cfg->i2c_addr;
+	board_info.platform_data = &pdata;
+	client = i2c_new_device(i2c, &board_info);
+	if (!client || !client->dev.driver)
+		return NULL;
+
+	*tuner_i2c_adapter = pdata.get_i2c_adapter(client);
+	return pdata.get_dvb_frontend(client);
+}
+EXPORT_SYMBOL(m88ds3103_attach);
+
+static struct dvb_frontend_ops m88ds3103_ops = {
+	.delsys = { SYS_DVBS, SYS_DVBS2 },
+	.info = {
+		.name = "Montage M88DS3103",
+		.frequency_min =  950000,
+		.frequency_max = 2150000,
+		.frequency_tolerance = 5000,
+		.symbol_rate_min =  1000000,
+		.symbol_rate_max = 45000000,
+		.caps = FE_CAN_INVERSION_AUTO |
+			FE_CAN_FEC_1_2 |
+			FE_CAN_FEC_2_3 |
+			FE_CAN_FEC_3_4 |
+			FE_CAN_FEC_4_5 |
+			FE_CAN_FEC_5_6 |
+			FE_CAN_FEC_6_7 |
+			FE_CAN_FEC_7_8 |
+			FE_CAN_FEC_8_9 |
+			FE_CAN_FEC_AUTO |
+			FE_CAN_QPSK |
+			FE_CAN_RECOVER |
+			FE_CAN_2G_MODULATION
+	},
+
+	.release = m88ds3103_release,
+
+	.get_tune_settings = m88ds3103_get_tune_settings,
+
+	.init = m88ds3103_init,
+	.sleep = m88ds3103_sleep,
+
+	.set_frontend = m88ds3103_set_frontend,
+	.get_frontend = m88ds3103_get_frontend,
+
+	.read_status = m88ds3103_read_status,
+	.read_snr = m88ds3103_read_snr,
+	.read_ber = m88ds3103_read_ber,
+
+	.diseqc_send_master_cmd = m88ds3103_diseqc_send_master_cmd,
+	.diseqc_send_burst = m88ds3103_diseqc_send_burst,
+
+	.set_tone = m88ds3103_set_tone,
+	.set_voltage = m88ds3103_set_voltage,
+};
+
+static struct dvb_frontend *m88ds3103_get_dvb_frontend(struct i2c_client *client)
+{
+	struct m88ds3103_priv *dev = i2c_get_clientdata(client);
+
+	dev_dbg(&client->dev, "\n");
+
+	return &dev->fe;
+}
+
+static struct i2c_adapter *m88ds3103_get_i2c_adapter(struct i2c_client *client)
+{
+	struct m88ds3103_priv *dev = i2c_get_clientdata(client);
+
+	dev_dbg(&client->dev, "\n");
+
+	return dev->i2c_adapter;
+}
+
+static int m88ds3103_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct m88ds3103_priv *dev;
+	struct m88ds3103_platform_data *pdata = client->dev.platform_data;
 	int ret;
-	struct m88ds3103_priv *priv;
 	u8 chip_id, u8tmp;
 
-	/* allocate memory for the internal priv */
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
 		ret = -ENOMEM;
-		dev_err(&i2c->dev, "%s: kzalloc() failed\n", KBUILD_MODNAME);
 		goto err;
 	}
 
-	priv->cfg = cfg;
-	priv->i2c = i2c;
-	mutex_init(&priv->i2c_mutex);
+	dev->client = client;
+	dev->i2c = client->adapter;
+	dev->config.i2c_addr = client->addr;
+	dev->config.clock = pdata->clk;
+	dev->config.i2c_wr_max = pdata->i2c_wr_max;
+	dev->config.ts_mode = pdata->ts_mode;
+	dev->config.ts_clk = pdata->ts_clk;
+	dev->config.ts_clk_pol = pdata->ts_clk_pol;
+	dev->config.spec_inv = pdata->spec_inv;
+	dev->config.agc_inv = pdata->agc_inv;
+	dev->config.clock_out = pdata->clk_out;
+	dev->config.envelope_mode = pdata->envelope_mode;
+	dev->config.agc = pdata->agc;
+	dev->config.lnb_hv_pol = pdata->lnb_hv_pol;
+	dev->config.lnb_en_pol = pdata->lnb_en_pol;
+	dev->cfg = &dev->config;
+	mutex_init(&dev->i2c_mutex);
 
 	/* 0x00: chip id[6:0], 0x01: chip ver[7:0], 0x02: chip ver[15:8] */
-	ret = m88ds3103_rd_reg(priv, 0x00, &chip_id);
+	ret = m88ds3103_rd_reg(dev, 0x00, &chip_id);
 	if (ret)
-		goto err;
+		goto err_kfree;
 
 	chip_id >>= 1;
-	dev_info(&priv->i2c->dev, "%s: chip_id=%02x\n", __func__, chip_id);
+	dev_dbg(&client->dev, "chip_id=%02x\n", chip_id);
 
 	switch (chip_id) {
 	case M88RS6000_CHIP_ID:
 	case M88DS3103_CHIP_ID:
 		break;
 	default:
-		goto err;
+		goto err_kfree;
 	}
-	priv->chip_id = chip_id;
+	dev->chip_id = chip_id;
 
-	switch (priv->cfg->clock_out) {
+	switch (dev->cfg->clock_out) {
 	case M88DS3103_CLOCK_OUT_DISABLED:
 		u8tmp = 0x80;
 		break;
@@ -1448,7 +1563,7 @@ struct dvb_frontend *m88ds3103_attach(const struct m88ds3103_config *cfg,
 		u8tmp = 0x10;
 		break;
 	default:
-		goto err;
+		goto err_kfree;
 	}
 
 	/* 0x29 register is defined differently for m88rs6000. */
@@ -1456,91 +1571,80 @@ struct dvb_frontend *m88ds3103_attach(const struct m88ds3103_config *cfg,
 	if (chip_id == M88RS6000_CHIP_ID)
 		u8tmp = 0x00;
 
-	ret = m88ds3103_wr_reg(priv, 0x29, u8tmp);
+	ret = m88ds3103_wr_reg(dev, 0x29, u8tmp);
 	if (ret)
-		goto err;
+		goto err_kfree;
 
 	/* sleep */
-	ret = m88ds3103_wr_reg_mask(priv, 0x08, 0x00, 0x01);
+	ret = m88ds3103_wr_reg_mask(dev, 0x08, 0x00, 0x01);
 	if (ret)
-		goto err;
-
-	ret = m88ds3103_wr_reg_mask(priv, 0x04, 0x01, 0x01);
+		goto err_kfree;
+	ret = m88ds3103_wr_reg_mask(dev, 0x04, 0x01, 0x01);
 	if (ret)
-		goto err;
-
-	ret = m88ds3103_wr_reg_mask(priv, 0x23, 0x10, 0x10);
+		goto err_kfree;
+	ret = m88ds3103_wr_reg_mask(dev, 0x23, 0x10, 0x10);
 	if (ret)
-		goto err;
+		goto err_kfree;
 
 	/* create mux i2c adapter for tuner */
-	priv->i2c_adapter = i2c_add_mux_adapter(i2c, &i2c->dev, priv, 0, 0, 0,
-			m88ds3103_select, m88ds3103_deselect);
-	if (priv->i2c_adapter == NULL)
-		goto err;
-
-	*tuner_i2c_adapter = priv->i2c_adapter;
+	dev->i2c_adapter = i2c_add_mux_adapter(client->adapter, &client->dev,
+					       dev, 0, 0, 0, m88ds3103_select,
+					       m88ds3103_deselect);
+	if (dev->i2c_adapter == NULL)
+		goto err_kfree;
 
 	/* create dvb_frontend */
-	memcpy(&priv->fe.ops, &m88ds3103_ops, sizeof(struct dvb_frontend_ops));
-	if (priv->chip_id == M88RS6000_CHIP_ID)
-		strncpy(priv->fe.ops.info.name,
-			"Montage M88RS6000", sizeof(priv->fe.ops.info.name));
-	priv->fe.demodulator_priv = priv;
-
-	return &priv->fe;
+	memcpy(&dev->fe.ops, &m88ds3103_ops, sizeof(struct dvb_frontend_ops));
+	if (dev->chip_id == M88RS6000_CHIP_ID)
+		strncpy(dev->fe.ops.info.name,
+			"Montage M88RS6000", sizeof(dev->fe.ops.info.name));
+	if (!pdata->attach_in_use)
+		dev->fe.ops.release = NULL;
+	dev->fe.demodulator_priv = dev;
+	i2c_set_clientdata(client, dev);
+
+	/* setup callbacks */
+	pdata->get_dvb_frontend = m88ds3103_get_dvb_frontend;
+	pdata->get_i2c_adapter = m88ds3103_get_i2c_adapter;
+	return 0;
+err_kfree:
+	kfree(dev);
 err:
-	dev_dbg(&i2c->dev, "%s: failed=%d\n", __func__, ret);
-	kfree(priv);
-	return NULL;
+	dev_dbg(&client->dev, "failed=%d\n", ret);
+	return ret;
 }
-EXPORT_SYMBOL(m88ds3103_attach);
-
-static struct dvb_frontend_ops m88ds3103_ops = {
-	.delsys = { SYS_DVBS, SYS_DVBS2 },
-	.info = {
-		.name = "Montage M88DS3103",
-		.frequency_min =  950000,
-		.frequency_max = 2150000,
-		.frequency_tolerance = 5000,
-		.symbol_rate_min =  1000000,
-		.symbol_rate_max = 45000000,
-		.caps = FE_CAN_INVERSION_AUTO |
-			FE_CAN_FEC_1_2 |
-			FE_CAN_FEC_2_3 |
-			FE_CAN_FEC_3_4 |
-			FE_CAN_FEC_4_5 |
-			FE_CAN_FEC_5_6 |
-			FE_CAN_FEC_6_7 |
-			FE_CAN_FEC_7_8 |
-			FE_CAN_FEC_8_9 |
-			FE_CAN_FEC_AUTO |
-			FE_CAN_QPSK |
-			FE_CAN_RECOVER |
-			FE_CAN_2G_MODULATION
-	},
 
-	.release = m88ds3103_release,
+static int m88ds3103_remove(struct i2c_client *client)
+{
+	struct m88ds3103_priv *dev = i2c_get_clientdata(client);
 
-	.get_tune_settings = m88ds3103_get_tune_settings,
+	dev_dbg(&client->dev, "\n");
 
-	.init = m88ds3103_init,
-	.sleep = m88ds3103_sleep,
+	i2c_del_mux_adapter(dev->i2c_adapter);
 
-	.set_frontend = m88ds3103_set_frontend,
-	.get_frontend = m88ds3103_get_frontend,
-
-	.read_status = m88ds3103_read_status,
-	.read_snr = m88ds3103_read_snr,
-	.read_ber = m88ds3103_read_ber,
+	kfree(dev);
+	return 0;
+}
 
-	.diseqc_send_master_cmd = m88ds3103_diseqc_send_master_cmd,
-	.diseqc_send_burst = m88ds3103_diseqc_send_burst,
+static const struct i2c_device_id m88ds3103_id_table[] = {
+	{"m88ds3103", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, m88ds3103_id_table);
 
-	.set_tone = m88ds3103_set_tone,
-	.set_voltage = m88ds3103_set_voltage,
+static struct i2c_driver m88ds3103_driver = {
+	.driver = {
+		.owner	= THIS_MODULE,
+		.name	= "m88ds3103",
+		.suppress_bind_attrs = true,
+	},
+	.probe		= m88ds3103_probe,
+	.remove		= m88ds3103_remove,
+	.id_table	= m88ds3103_id_table,
 };
 
+module_i2c_driver(m88ds3103_driver);
+
 MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
 MODULE_DESCRIPTION("Montage M88DS3103 DVB-S/S2 demodulator driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/m88ds3103.h b/drivers/media/dvb-frontends/m88ds3103.h
index 9b3b496..3811468 100644
--- a/drivers/media/dvb-frontends/m88ds3103.h
+++ b/drivers/media/dvb-frontends/m88ds3103.h
@@ -19,6 +19,63 @@
 
 #include <linux/dvb/frontend.h>
 
+/*
+ * I2C address
+ * 0x68,
+ */
+
+/**
+ * struct m88ds3103_platform_data - Platform data for the m88ds3103 driver
+ * @clk: Clock frequency.
+ * @i2c_wr_max: Max bytes I2C adapter can write at once.
+ * @ts_mode: TS mode.
+ * @ts_clk: TS clock (KHz).
+ * @ts_clk_pol: TS clk polarity. 1-active at falling edge; 0-active at rising
+ *  edge.
+ * @spec_inv: Input spectrum inversion.
+ * @agc: AGC configuration.
+ * @agc_inv: AGC polarity.
+ * @clk_out: Clock output.
+ * @envelope_mode: DiSEqC envelope mode.
+ * @lnb_hv_pol: LNB H/V pin polarity. 0: pin high set to VOLTAGE_18, pin low to
+ *  set VOLTAGE_13. 1: pin high set to VOLTAGE_13, pin low to set VOLTAGE_18.
+ * @lnb_en_pol: LNB enable pin polarity. 0: pin high to disable, pin low to
+ *  enable. 1: pin high to enable, pin low to disable.
+ * @get_dvb_frontend: Get DVB frontend.
+ * @get_i2c_adapter: Get I2C adapter.
+ */
+
+struct m88ds3103_platform_data {
+	u32 clk;
+	u16 i2c_wr_max;
+#define M88DS3103_TS_SERIAL             0 /* TS output pin D0, normal */
+#define M88DS3103_TS_SERIAL_D7          1 /* TS output pin D7 */
+#define M88DS3103_TS_PARALLEL           2 /* TS Parallel mode */
+#define M88DS3103_TS_CI                 3 /* TS CI Mode */
+	u8 ts_mode:2;
+	u32 ts_clk;
+	u8 ts_clk_pol:1;
+	u8 spec_inv:1;
+	u8 agc;
+	u8 agc_inv:1;
+#define M88DS3103_CLOCK_OUT_DISABLED        0
+#define M88DS3103_CLOCK_OUT_ENABLED         1
+#define M88DS3103_CLOCK_OUT_ENABLED_DIV2    2
+	u8 clk_out:2;
+	u8 envelope_mode:1;
+	u8 lnb_hv_pol:1;
+	u8 lnb_en_pol:1;
+
+	struct dvb_frontend* (*get_dvb_frontend)(struct i2c_client *);
+	struct i2c_adapter* (*get_i2c_adapter)(struct i2c_client *);
+
+/* private: For legacy media attach wrapper. Do not set value. */
+	u8 attach_in_use:1;
+};
+
+/*
+ * Do not add new m88ds3103_attach() users! Use I2C bindings instead.
+ */
 struct m88ds3103_config {
 	/*
 	 * I2C address
@@ -113,12 +170,6 @@ struct m88ds3103_config {
 	u8 lnb_en_pol:1;
 };
 
-/*
- * Driver implements own I2C-adapter for tuner I2C access. That's since chip
- * has I2C-gate control which closes gate automatically after I2C transfer.
- * Using own I2C adapter we can workaround that.
- */
-
 #if defined(CONFIG_DVB_M88DS3103) || \
 		(defined(CONFIG_DVB_M88DS3103_MODULE) && defined(MODULE))
 extern struct dvb_frontend *m88ds3103_attach(
diff --git a/drivers/media/dvb-frontends/m88ds3103_priv.h b/drivers/media/dvb-frontends/m88ds3103_priv.h
index 78e58e3..6217d92 100644
--- a/drivers/media/dvb-frontends/m88ds3103_priv.h
+++ b/drivers/media/dvb-frontends/m88ds3103_priv.h
@@ -32,8 +32,10 @@
 
 struct m88ds3103_priv {
 	struct i2c_adapter *i2c;
+	struct i2c_client *client;
 	/* mutex needed due to own tuner I2C adapter */
 	struct mutex i2c_mutex;
+	struct m88ds3103_config config;
 	const struct m88ds3103_config *cfg;
 	struct dvb_frontend fe;
 	fe_delivery_system_t delivery_system;
-- 
http://palosaari.fi/


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

* Re: [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready
  2015-05-21 19:22 ` [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready Antti Palosaari
@ 2015-05-30 15:02   ` Mauro Carvalho Chehab
  2015-05-30 15:10     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2015-05-30 15:02 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media

Em Thu, 21 May 2015 22:22:48 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> Do not return error from get_frontend() when status is queried, but
> device is not ready. dvbv5-zap has habit to call that IOCTL before
> device is tuned and it also refuses to use DVBv5 statistic after
> that...

This is actually an error at libdvbv5, that was solved by this patch:

commit bf028618f0a2f86f8515560865b8f8142eddb1d9
Author: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Date:   Tue Apr 14 11:47:57 2015 -0300

    libdvbv5: Retry FE_GET_PROPERTY ioctl if it returns EAGAIN
    
    Retry the FE_GET_PROPERTY ioctl used to determine if we have a DVBv5 device
    if it returns EAGAIN indicating the driver is currently locked by the kernel.
    
    Also skip over subsequent information gathering calls to FE_GET_PROPERTY
    that return EAGAIN.
    
    Signed-off-by: David Howells <dhowells@redhat.com>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Basically, -EAGAIN should be discarded.

That's said, see below.


> 
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/dvb-frontends/m88ds3103.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
> index d3d928e..03dceb5 100644
> --- a/drivers/media/dvb-frontends/m88ds3103.c
> +++ b/drivers/media/dvb-frontends/m88ds3103.c
> @@ -742,7 +742,7 @@ static int m88ds3103_get_frontend(struct dvb_frontend *fe)
>  	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
>  
>  	if (!priv->warm || !(priv->fe_status & FE_HAS_LOCK)) {
> -		ret = -EAGAIN;
> +		ret = 0;

Returning EAGAIN here doesn't seem right, as this ioctl didn't fail
due to mutex locked while calling an ioctl with non-block mode (that's
basically the usage of EAGAIN).

The proper behavior is to succeed the ioctl, keeping the cache untouched,
with all the DVBv5 available status with scale filled with
FE_SCALE_NOT_AVAILABLE.

That's said, I'm not seeing the part of the code on m88ds3103 that would
be filling the DVBv5 stats. It seems that this patch comment is bogus.

>  		goto err;
>  	}
>  

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

* Re: [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready
  2015-05-30 15:02   ` Mauro Carvalho Chehab
@ 2015-05-30 15:10     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2015-05-30 15:10 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media

Em Sat, 30 May 2015 12:02:14 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Thu, 21 May 2015 22:22:48 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
> 
> > Do not return error from get_frontend() when status is queried, but
> > device is not ready. dvbv5-zap has habit to call that IOCTL before
> > device is tuned and it also refuses to use DVBv5 statistic after
> > that...
> 
> This is actually an error at libdvbv5, that was solved by this patch:
> 
> commit bf028618f0a2f86f8515560865b8f8142eddb1d9
> Author: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Date:   Tue Apr 14 11:47:57 2015 -0300
> 
>     libdvbv5: Retry FE_GET_PROPERTY ioctl if it returns EAGAIN
>     
>     Retry the FE_GET_PROPERTY ioctl used to determine if we have a DVBv5 device
>     if it returns EAGAIN indicating the driver is currently locked by the kernel.
>     
>     Also skip over subsequent information gathering calls to FE_GET_PROPERTY
>     that return EAGAIN.
>     
>     Signed-off-by: David Howells <dhowells@redhat.com>
>     Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> Basically, -EAGAIN should be discarded.
> 
> That's said, see below.
> 
> 
> > 
> > Signed-off-by: Antti Palosaari <crope@iki.fi>
> > ---
> >  drivers/media/dvb-frontends/m88ds3103.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
> > index d3d928e..03dceb5 100644
> > --- a/drivers/media/dvb-frontends/m88ds3103.c
> > +++ b/drivers/media/dvb-frontends/m88ds3103.c
> > @@ -742,7 +742,7 @@ static int m88ds3103_get_frontend(struct dvb_frontend *fe)
> >  	dev_dbg(&priv->i2c->dev, "%s:\n", __func__);
> >  
> >  	if (!priv->warm || !(priv->fe_status & FE_HAS_LOCK)) {
> > -		ret = -EAGAIN;
> > +		ret = 0;
> 
> Returning EAGAIN here doesn't seem right, as this ioctl didn't fail
> due to mutex locked while calling an ioctl with non-block mode (that's
> basically the usage of EAGAIN).
> 
> The proper behavior is to succeed the ioctl, keeping the cache untouched,
> with all the DVBv5 available status with scale filled with
> FE_SCALE_NOT_AVAILABLE.
> 
> That's said, I'm not seeing the part of the code on m88ds3103 that would
> be filling the DVBv5 stats. It seems that this patch comment is bogus.

I ended by applying the patch together with the complete patch series
just fixing the patch description.

Regards,
Mauro

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

end of thread, other threads:[~2015-05-30 15:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-21 19:22 [PATCHv2 0/5] m88ds3103 improvements (DVBv5 stats and more) Antti Palosaari
2015-05-21 19:22 ` [PATCHv2 1/5] m88ds3103: do not return error from get_frontend() when not ready Antti Palosaari
2015-05-30 15:02   ` Mauro Carvalho Chehab
2015-05-30 15:10     ` Mauro Carvalho Chehab
2015-05-21 19:22 ` [PATCHv2 2/5] m88ds3103: implement DVBv5 CNR statistics Antti Palosaari
2015-05-21 19:22 ` [PATCHv2 3/5] m88ds3103: implement DVBv5 BER Antti Palosaari
2015-05-21 19:22 ` [PATCHv2 4/5] m88ds3103: use jiffies when polling DiSEqC TX ready Antti Palosaari
2015-05-21 19:22 ` [PATCHv2 5/5] m88ds3103: add I2C client binding Antti Palosaari

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.