| Message ID | 1418429925-16342-4-git-send-email-benjamin@southpole.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/13/2014 02:18 AM, Benjamin Larsson wrote: > Signed-off-by: Benjamin Larsson <benjamin@southpole.se> Reviewed-by: Antti Palosaari <crope@iki.fi> Even I could accept that, as a staging driver, I see there some issues: * missing commit message (ok, it is trivial and patch subject says) * it is legacy DVBv3 API BER reporting, whilst driver is DVBv5 mostly due to DVB-T2... So DVBv5 statistics are preferred. * dynamic debugs has unneded __func__, see Documentation/dynamic-debug-howto.txt * there should be spaces used around binary and ternary calculation operators, see Documentation/CodingStyle for more info how it should be. Could you read overall these two docs before make new patches: Documentation/CodingStyle Documentation/dynamic-debug-howto.txt also use scripts/checkpatch.pl to verify patch, like that git diff | ./scripts/checkpatch.pl - regards Antti > --- > drivers/staging/media/mn88472/mn88472.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c > index 746cc94..8b35639 100644 > --- a/drivers/staging/media/mn88472/mn88472.c > +++ b/drivers/staging/media/mn88472/mn88472.c > @@ -392,6 +392,36 @@ err: > return ret; > } > > +static int mn88472_read_ber(struct dvb_frontend *fe, u32 *ber) > +{ > + struct i2c_client *client = fe->demodulator_priv; > + struct mn88472_dev *dev = i2c_get_clientdata(client); > + int ret, err, len; > + u8 data[3]; > + > + dev_dbg(&client->dev, "%s:\n", __func__); > + > + ret = regmap_bulk_read(dev->regmap[0], 0x9F , data, 3); > + if (ret) > + goto err; > + err = data[0]<<16 | data[1]<<8 | data[2]; > + > + ret = regmap_bulk_read(dev->regmap[0], 0xA2 , data, 2); > + if (ret) > + goto err; > + len = data[0]<<8 | data[1]; > + > + if (len) > + *ber = (err*100)/len; > + else > + *ber = 0; > + > + return 0; > +err: > + dev_dbg(&client->dev, "%s: failed=%d\n", __func__, ret); > + return ret; > +} > + > static struct dvb_frontend_ops mn88472_ops = { > .delsys = {SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A}, > .info = { > @@ -425,6 +455,7 @@ static struct dvb_frontend_ops mn88472_ops = { > .set_frontend = mn88472_set_frontend, > > .read_status = mn88472_read_status, > + .read_ber = mn88472_read_ber, > }; > > static int mn88472_probe(struct i2c_client *client, >
On 12/13/2014 05:15 AM, Antti Palosaari wrote: > On 12/13/2014 02:18 AM, Benjamin Larsson wrote: >> Signed-off-by: Benjamin Larsson <benjamin@southpole.se> > > Reviewed-by: Antti Palosaari <crope@iki.fi> > > > Even I could accept that, as a staging driver, I see there some issues: > > * missing commit message (ok, it is trivial and patch subject says) > > * it is legacy DVBv3 API BER reporting, whilst driver is DVBv5 mostly > due to DVB-T2... So DVBv5 statistics are preferred. > > * dynamic debugs has unneded __func__, see > Documentation/dynamic-debug-howto.txt > > * there should be spaces used around binary and ternary calculation > operators, see Documentation/CodingStyle for more info how it should be. > > > Could you read overall these two docs before make new patches: > Documentation/CodingStyle > Documentation/dynamic-debug-howto.txt > > also use scripts/checkpatch.pl to verify patch, like that > git diff | ./scripts/checkpatch.pl - > > regards > Antti I will read those. Can you recommend a driver as template for DVBv5 statistics ? MvH Benjamin Larsson -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/13/2014 01:12 PM, Benjamin Larsson wrote: > On 12/13/2014 05:15 AM, Antti Palosaari wrote: >> On 12/13/2014 02:18 AM, Benjamin Larsson wrote: >>> Signed-off-by: Benjamin Larsson <benjamin@southpole.se> >> >> Reviewed-by: Antti Palosaari <crope@iki.fi> >> >> >> Even I could accept that, as a staging driver, I see there some issues: >> >> * missing commit message (ok, it is trivial and patch subject says) >> >> * it is legacy DVBv3 API BER reporting, whilst driver is DVBv5 mostly >> due to DVB-T2... So DVBv5 statistics are preferred. >> >> * dynamic debugs has unneded __func__, see >> Documentation/dynamic-debug-howto.txt >> >> * there should be spaces used around binary and ternary calculation >> operators, see Documentation/CodingStyle for more info how it should be. >> >> >> Could you read overall these two docs before make new patches: >> Documentation/CodingStyle >> Documentation/dynamic-debug-howto.txt >> >> also use scripts/checkpatch.pl to verify patch, like that >> git diff | ./scripts/checkpatch.pl - >> >> regards >> Antti > > I will read those. Can you recommend a driver as template for DVBv5 > statistics ? I just posted set of rtl2830 driver patches where is one example. Another example is af9035 and si2168. DVBv5 stats works so that you periodically update values in property cache, which are then returned to application if app request. Values are updated to cache even none is using those. regards Antti
diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c index 746cc94..8b35639 100644 --- a/drivers/staging/media/mn88472/mn88472.c +++ b/drivers/staging/media/mn88472/mn88472.c @@ -392,6 +392,36 @@ err: return ret; } +static int mn88472_read_ber(struct dvb_frontend *fe, u32 *ber) +{ + struct i2c_client *client = fe->demodulator_priv; + struct mn88472_dev *dev = i2c_get_clientdata(client); + int ret, err, len; + u8 data[3]; + + dev_dbg(&client->dev, "%s:\n", __func__); + + ret = regmap_bulk_read(dev->regmap[0], 0x9F , data, 3); + if (ret) + goto err; + err = data[0]<<16 | data[1]<<8 | data[2]; + + ret = regmap_bulk_read(dev->regmap[0], 0xA2 , data, 2); + if (ret) + goto err; + len = data[0]<<8 | data[1]; + + if (len) + *ber = (err*100)/len; + else + *ber = 0; + + return 0; +err: + dev_dbg(&client->dev, "%s: failed=%d\n", __func__, ret); + return ret; +} + static struct dvb_frontend_ops mn88472_ops = { .delsys = {SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A}, .info = { @@ -425,6 +455,7 @@ static struct dvb_frontend_ops mn88472_ops = { .set_frontend = mn88472_set_frontend, .read_status = mn88472_read_status, + .read_ber = mn88472_read_ber, }; static int mn88472_probe(struct i2c_client *client,
Signed-off-by: Benjamin Larsson <benjamin@southpole.se> --- drivers/staging/media/mn88472/mn88472.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)