[PATCH 0/4] ddbridge-0.9.33 fixes and improvements

[PATCH 0/4] ddbridge-0.9.33 fixes and improvements

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

Four post-ddbridge-0.9.33 patches fixing and improving a few things:

Patch 1 adds protection into the new ddbridge-mci code against an
out-of-bounds array write access as reported by CoverityScan and brought
to attention by Colin Ian King.

Patch 2 adds missing function argument identifiers to ddb_mci_attach() to
silence checkpatch. It still complains about the **fn_set_input which IS
the identifier but doesn't seem to be caught by checkpatch properly.

Patches 3 and 4 enable the higher speed TS mode on stv0910-equipped cards
and thus enables for higher bitrate transponders when the detected card
firmware permits this. This is basically what was removed in the initial
ddbridge-0.9.33 patch series but enhanced to be only enabled when the
FPGA firmware permits this.

Please merge, this should go in alongside the already merged ddbridge
0.9.33 patches. Thanks.

Daniel Scheller (4):
  [media] ddbridge/mci: protect against out-of-bounds array access in
    stop()
  [media] ddbridge/mci: add identifiers to function definition arguments
  [media] dvb-frontends/stv0910: make TS speed configurable
  [media] ddbridge: conditionally enable fast TS for stv0910-equipped
    bridges

 drivers/media/dvb-frontends/stv0910.c      |  5 ++---
 drivers/media/dvb-frontends/stv0910.h      |  1 +
 drivers/media/pci/ddbridge/ddbridge-core.c | 35 +++++++++++++++++++++++++-----
 drivers/media/pci/ddbridge/ddbridge-mci.c  | 23 ++++++++++----------
 drivers/media/pci/ddbridge/ddbridge-mci.h  |  6 ++++-
 drivers/media/pci/ngene/ngene-cards.c      |  1 +
 6 files changed, 51 insertions(+), 20 deletions(-)

-- 
2.16.1

[PATCH 1/4] [media] ddbridge/mci: protect against out-of-bounds array access in stop()

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

In stop(), an (unlikely) out-of-bounds write error can occur when setting
the demod_in_use element indexed by state->demod to zero, as state->demod
isn't checked for being in the range of the array size of demod_in_use, and
state->demod maybe carrying the magic 0xff (demod unused) value. Prevent
this by checking state->demod not exceeding the array size before setting
the element value. To make the code a bit easier to read, replace the magic
value and the number of array elements with defines, and use them at a few
more places.

Detected by CoverityScan, CID#1468550 ("Out-of-bounds write")

Thanks to Colin for reporting the problem and providing an initial patch.

Fixes: daeeb1319e6f ("media: ddbridge: initial support for MCI-based MaxSX8 cards")
Reported-by: Colin Ian King <colin.king@canonical.com>
Cc: Ralph Metzler <rjkm@metzlerbros.de>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-mci.c | 21 +++++++++++----------
 drivers/media/pci/ddbridge/ddbridge-mci.h |  4 ++++
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-mci.c b/drivers/media/pci/ddbridge/ddbridge-mci.c
index a85ff3e6b919..8d9592e75ad5 100644
--- a/drivers/media/pci/ddbridge/ddbridge-mci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-mci.c
@@ -38,10 +38,10 @@ struct mci_base {
 	struct mutex         mci_lock; /* concurrent MCI access lock */
 	int                  count;
 
-	u8                   tuner_use_count[4];
-	u8                   assigned_demod[8];
-	u32                  used_ldpc_bitrate[8];
-	u8                   demod_in_use[8];
+	u8                   tuner_use_count[MCI_TUNER_MAX];
+	u8                   assigned_demod[MCI_DEMOD_MAX];
+	u32                  used_ldpc_bitrate[MCI_DEMOD_MAX];
+	u8                   demod_in_use[MCI_DEMOD_MAX];
 	u32                  iq_mode;
 };
 
@@ -193,7 +193,7 @@ static int stop(struct dvb_frontend *fe)
 	u32 input = state->tuner;
 
 	memset(&cmd, 0, sizeof(cmd));
-	if (state->demod != 0xff) {
+	if (state->demod != DEMOD_UNUSED) {
 		cmd.command = MCI_CMD_STOP;
 		cmd.demod = state->demod;
 		mci_cmd(state, &cmd, NULL);
@@ -209,10 +209,11 @@ static int stop(struct dvb_frontend *fe)
 	state->base->tuner_use_count[input]--;
 	if (!state->base->tuner_use_count[input])
 		mci_set_tuner(fe, input, 0);
-	state->base->demod_in_use[state->demod] = 0;
+	if (state->demod < MCI_DEMOD_MAX)
+		state->base->demod_in_use[state->demod] = 0;
 	state->base->used_ldpc_bitrate[state->nr] = 0;
-	state->demod = 0xff;
-	state->base->assigned_demod[state->nr] = 0xff;
+	state->demod = DEMOD_UNUSED;
+	state->base->assigned_demod[state->nr] = DEMOD_UNUSED;
 	state->base->iq_mode = 0;
 	mutex_unlock(&state->base->tuner_lock);
 	state->started = 0;
@@ -250,7 +251,7 @@ static int start(struct dvb_frontend *fe, u32 flags, u32 modmask, u32 ts_config)
 		stat = -EBUSY;
 		goto unlock;
 	}
-	for (i = 0; i < 8; i++) {
+	for (i = 0; i < MCI_DEMOD_MAX; i++) {
 		used_ldpc_bitrate += state->base->used_ldpc_bitrate[i];
 		if (state->base->demod_in_use[i])
 			used_demods++;
@@ -342,7 +343,7 @@ static int start_iq(struct dvb_frontend *fe, u32 ts_config)
 		stat = -EBUSY;
 		goto unlock;
 	}
-	for (i = 0; i < 8; i++)
+	for (i = 0; i < MCI_DEMOD_MAX; i++)
 		if (state->base->demod_in_use[i])
 			used_demods++;
 	if (used_demods > 0) {
diff --git a/drivers/media/pci/ddbridge/ddbridge-mci.h b/drivers/media/pci/ddbridge/ddbridge-mci.h
index c4193c5ee095..453dcb9f8208 100644
--- a/drivers/media/pci/ddbridge/ddbridge-mci.h
+++ b/drivers/media/pci/ddbridge/ddbridge-mci.h
@@ -19,6 +19,10 @@
 #ifndef _DDBRIDGE_MCI_H_
 #define _DDBRIDGE_MCI_H_
 
+#define MCI_DEMOD_MAX                       8
+#define MCI_TUNER_MAX                       4
+#define DEMOD_UNUSED                        (0xFF)
+
 #define MCI_CONTROL                         (0x500)
 #define MCI_COMMAND                         (0x600)
 #define MCI_RESULT                          (0x680)
-- 
2.16.1

[PATCH 2/4] [media] ddbridge/mci: add identifiers to function definition arguments

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

Fixes two checkpatch warnings

  WARNING: function definition argument 'xxx' should also have an identifier name

in the ddb_mci_attach() prototype definition. checkpatch keeps complaining
on the "int (**fn_set_input)" as it seems to have issues with the
ptr-to-ptr, though this probably needs fixing in checkpatch.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-mci.c | 2 +-
 drivers/media/pci/ddbridge/ddbridge-mci.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-mci.c b/drivers/media/pci/ddbridge/ddbridge-mci.c
index 8d9592e75ad5..4ac634fc96e4 100644
--- a/drivers/media/pci/ddbridge/ddbridge-mci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-mci.c
@@ -500,7 +500,7 @@ static int probe(struct mci *state)
 struct dvb_frontend
 *ddb_mci_attach(struct ddb_input *input,
 		int mci_type, int nr,
-		int (**fn_set_input)(struct dvb_frontend *, int))
+		int (**fn_set_input)(struct dvb_frontend *fe, int input))
 {
 	struct ddb_port *port = input->port;
 	struct ddb *dev = port->dev;
diff --git a/drivers/media/pci/ddbridge/ddbridge-mci.h b/drivers/media/pci/ddbridge/ddbridge-mci.h
index 453dcb9f8208..209cc2b92dff 100644
--- a/drivers/media/pci/ddbridge/ddbridge-mci.h
+++ b/drivers/media/pci/ddbridge/ddbridge-mci.h
@@ -151,6 +151,6 @@ struct mci_result {
 struct dvb_frontend
 *ddb_mci_attach(struct ddb_input *input,
 		int mci_type, int nr,
-		int (**fn_set_input)(struct dvb_frontend *, int));
+		int (**fn_set_input)(struct dvb_frontend *fe, int input));
 
 #endif /* _DDBRIDGE_MCI_H_ */
-- 
2.16.1

[PATCH 3/4] [media] dvb-frontends/stv0910: make TS speed configurable

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

Add a tsspeed config option to struct stv0910_cfg which can be used by
users of the driver to set the (parallel) TS speed (higher speeds enable
support for higher bitrate transponders). If tsspeed isn't set in the
config, it'll default to a sane value.

This commit also updates the two consumers of the stv0910 driver (ngene
and ddbridge) to have a default tsspeed in their stv0910_cfg templates.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
Tested-by: Richard Scobie <rascobie@slingshot.co.nz>
Tested-by: Helmut Auer <post@helmutauer.de>
---
 drivers/media/dvb-frontends/stv0910.c      | 5 ++---
 drivers/media/dvb-frontends/stv0910.h      | 1 +
 drivers/media/pci/ddbridge/ddbridge-core.c | 1 +
 drivers/media/pci/ngene/ngene-cards.c      | 1 +
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c
index 7e9b016b3b28..41444fa1c0bb 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1200,7 +1200,6 @@ static int probe(struct stv *state)
 	write_reg(state, RSTV0910_P1_TSCFGM, 0xC0); /* Manual speed */
 	write_reg(state, RSTV0910_P1_TSCFGL, 0x20);
 
-	/* Speed = 67.5 MHz */
 	write_reg(state, RSTV0910_P1_TSSPEED, state->tsspeed);
 
 	write_reg(state, RSTV0910_P2_TSCFGH, state->tscfgh | 0x01);
@@ -1208,7 +1207,6 @@ static int probe(struct stv *state)
 	write_reg(state, RSTV0910_P2_TSCFGM, 0xC0); /* Manual speed */
 	write_reg(state, RSTV0910_P2_TSCFGL, 0x20);
 
-	/* Speed = 67.5 MHz */
 	write_reg(state, RSTV0910_P2_TSSPEED, state->tsspeed);
 
 	/* Reset stream merger */
@@ -1790,7 +1788,8 @@ struct dvb_frontend *stv0910_attach(struct i2c_adapter *i2c,
 	state->tscfgh = 0x20 | (cfg->parallel ? 0 : 0x40);
 	state->tsgeneral = (cfg->parallel == 2) ? 0x02 : 0x00;
 	state->i2crpt = 0x0A | ((cfg->rptlvl & 0x07) << 4);
-	state->tsspeed = 0x28;
+	/* use safe tsspeed value if unspecified through stv0910_cfg */
+	state->tsspeed = (cfg->tsspeed ? cfg->tsspeed : 0x28);
 	state->nr = nr;
 	state->regoff = state->nr ? 0 : 0x200;
 	state->search_range = 16000000;
diff --git a/drivers/media/dvb-frontends/stv0910.h b/drivers/media/dvb-frontends/stv0910.h
index fccd8d9b665f..f37171b7a2de 100644
--- a/drivers/media/dvb-frontends/stv0910.h
+++ b/drivers/media/dvb-frontends/stv0910.h
@@ -10,6 +10,7 @@ struct stv0910_cfg {
 	u8  parallel;
 	u8  rptlvl;
 	u8  single;
+	u8  tsspeed;
 };
 
 #if IS_REACHABLE(CONFIG_DVB_STV0910)
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 377269c64449..6c2341642017 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1183,6 +1183,7 @@ static const struct stv0910_cfg stv0910_p = {
 	.parallel = 1,
 	.rptlvl   = 4,
 	.clk      = 30000000,
+	.tsspeed  = 0x28,
 };
 
 static const struct lnbh25_config lnbh25_cfg = {
diff --git a/drivers/media/pci/ngene/ngene-cards.c b/drivers/media/pci/ngene/ngene-cards.c
index 7738565193d6..7a106bc11a2b 100644
--- a/drivers/media/pci/ngene/ngene-cards.c
+++ b/drivers/media/pci/ngene/ngene-cards.c
@@ -327,6 +327,7 @@ static struct stv0910_cfg stv0910_p = {
 	.parallel = 1,
 	.rptlvl   = 4,
 	.clk      = 30000000,
+	.tsspeed  = 0x28,
 };
 
 static struct lnbh25_config lnbh25_cfg = {
-- 
2.16.1

[PATCH 4/4] [media] ddbridge: conditionally enable fast TS for stv0910-equipped bridges

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

CineS2 V7(A) and Octopus CI S2 Pro/Advanced cards support faster TS speeds
on the card's contained stv0910 demodulator when their FPGA was updated
with a recent (>= 1.7, version number applies to all mentioned cards)
vendor firmware. Enable this faster TS speed on card port 0 (contained
demod) and parallel stv0910 connections when the card firmware is at least
1.7 or later.

Note: The mentioned cards and their demods are handled via the STV0910_PR
and STV0910_P tuner types. DuoFlex modules with such demodulators are
handled via the STV0910 (without suffix) types where such TS speed
increase doesn't technically make sense.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
Tested-by: Richard Scobie <rascobie@slingshot.co.nz>
Tested-by: Helmut Auer <post@helmutauer.de>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 34 +++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 6c2341642017..d5b0d1eaf3ad 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1191,7 +1191,7 @@ static const struct lnbh25_config lnbh25_cfg = {
 	.data2_config = LNBH25_TEN
 };
 
-static int demod_attach_stv0910(struct ddb_input *input, int type)
+static int demod_attach_stv0910(struct ddb_input *input, int type, int tsfast)
 {
 	struct i2c_adapter *i2c = &input->port->i2c->adap;
 	struct ddb_dvb *dvb = &input->port->dvb[input->nr & 1];
@@ -1204,6 +1204,12 @@ static int demod_attach_stv0910(struct ddb_input *input, int type)
 
 	if (type)
 		cfg.parallel = 2;
+
+	if (tsfast) {
+		dev_info(dev, "Enabling stv0910 higher speed TS\n");
+		cfg.tsspeed = 0x10;
+	}
+
 	dvb->fe = dvb_attach(stv0910_attach, i2c, &cfg, (input->nr & 1));
 	if (!dvb->fe) {
 		cfg.adr = 0x6c;
@@ -1439,7 +1445,25 @@ static int dvb_input_attach(struct ddb_input *input)
 	struct ddb_port *port = input->port;
 	struct dvb_adapter *adap = dvb->adap;
 	struct dvb_demux *dvbdemux = &dvb->demux;
-	int par = 0, osc24 = 0;
+	struct ddb_ids *devids = &input->port->dev->link[input->port->lnr].ids;
+	int par = 0, osc24 = 0, tsfast = 0;
+
+	/*
+	 * Determine if bridges with stv0910 demods can run with fast TS and
+	 * thus support high bandwidth transponders.
+	 * STV0910_PR and STV0910_P tuner types covers all relevant bridges,
+	 * namely the CineS2 V7(A) and the Octopus CI S2 Pro/Advanced. All
+	 * DuoFlex S2 V4(A) have type=DDB_TUNER_DVBS_STV0910 without any suffix
+	 * and are limited by the serial link to the bridge, thus won't work
+	 * in fast TS mode.
+	 */
+	if (port->nr == 0 &&
+	    (port->type == DDB_TUNER_DVBS_STV0910_PR ||
+	     port->type == DDB_TUNER_DVBS_STV0910_P)) {
+		/* fast TS on port 0 requires FPGA version >= 1.7 */
+		if ((devids->hwid & 0x00ffffff) >= 0x00010007)
+			tsfast = 1;
+	}
 
 	dvb->attached = 0x01;
 
@@ -1496,19 +1520,19 @@ static int dvb_input_attach(struct ddb_input *input)
 			goto err_tuner;
 		break;
 	case DDB_TUNER_DVBS_STV0910:
-		if (demod_attach_stv0910(input, 0) < 0)
+		if (demod_attach_stv0910(input, 0, tsfast) < 0)
 			goto err_detach;
 		if (tuner_attach_stv6111(input, 0) < 0)
 			goto err_tuner;
 		break;
 	case DDB_TUNER_DVBS_STV0910_PR:
-		if (demod_attach_stv0910(input, 1) < 0)
+		if (demod_attach_stv0910(input, 1, tsfast) < 0)
 			goto err_detach;
 		if (tuner_attach_stv6111(input, 1) < 0)
 			goto err_tuner;
 		break;
 	case DDB_TUNER_DVBS_STV0910_P:
-		if (demod_attach_stv0910(input, 0) < 0)
+		if (demod_attach_stv0910(input, 0, tsfast) < 0)
 			goto err_detach;
 		if (tuner_attach_stv6111(input, 1) < 0)
 			goto err_tuner;
-- 
2.16.1