Re: [PATCH 4/4] mn88472: implemented ber reporting

2014-12-14 Thread Antti Palosaari

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 benja...@southpole.se


Reviewed-by: Antti Palosaari cr...@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

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] mn88472: implemented ber reporting

2014-12-13 Thread Benjamin Larsson

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 benja...@southpole.se


Reviewed-by: Antti Palosaari cr...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] mn88472: implemented ber reporting

2014-12-12 Thread Antti Palosaari

On 12/13/2014 02:18 AM, Benjamin Larsson wrote:

Signed-off-by: Benjamin Larsson benja...@southpole.se


Reviewed-by: Antti Palosaari cr...@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,



--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html