[4/4] mn88472: implemented ber reporting
Commit Message
Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
drivers/staging/media/mn88472/mn88472.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
Comments
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
@@ -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,