ddbridge-0.9.33 fixes and improvements
Subsystems (3)
cover ddbridge-0.9.33 fixes and improvements
From: Daniel Scheller <[email protected]>
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(-)
Signature · 1 line
2.16.1
PATCH 1/4 ddbridge/mci: protect against out-of-bounds array access in stop()
From: Daniel Scheller <[email protected]>
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.
--- drivers/media/pci/ddbridge/ddbridge-mci.c | 21 +++++++++++---------- drivers/media/pci/ddbridge/ddbridge-mci.h | 4 ++++ 2 files changed, 15 insertions(+), 10 deletions(-)
drivers/media/pci/ddbridge/ddbridge-mci.c +11 -10
@@ -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 ) {
drivers/media/pci/ddbridge/ddbridge-mci.h +4 -0
@@ -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)
Signature · 1 line
2.16.1
PATCH 2/4 ddbridge/mci: add identifiers to function definition arguments
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
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.
--- drivers/media/pci/ddbridge/ddbridge-mci.c | 2 +- drivers/media/pci/ddbridge/ddbridge-mci.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/media/pci/ddbridge/ddbridge-mci.c +1 -1
@@ -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;
drivers/media/pci/ddbridge/ddbridge-mci.h +1 -1
@@ -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_ */
Signature · 1 line
2.16.1
PATCH 3/4 dvb-frontends/stv0910: make TS speed configurable
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
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.
--- 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(-)
drivers/media/dvb-frontends/stv0910.c +2 -3
@@ -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 ;
drivers/media/dvb-frontends/stv0910.h +1 -0
@@ -10,6 +10,7 @@ struct stv0910_cfg {u8 parallel;u8 rptlvl;u8 single; +u8 tsspeed; };#if IS_REACHABLE (CONFIG_DVB_STV0910 )
drivers/media/pci/ddbridge/ddbridge-core.c +1 -0
@@ -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 = {
drivers/media/pci/ngene/ngene-cards.c +1 -0
@@ -327,6 +327,7 @@ static struct stv0910_cfg stv0910_p = { .parallel = 1, .rptlvl = 4, .clk = 30000000, + .tsspeed = 0x28, };static struct lnbh25_config lnbh25_cfg = {
Signature · 1 line
2.16.1
PATCH 4/4 ddbridge: conditionally enable fast TS for stv0910-equipped bridges
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
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.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 34 +++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +29 -5
@@ -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;
Signature · 1 line
2.16.1