[PATCH 1/4] mn88472: implement dvb-t signal lock

[PATCH 1/4] mn88472: implement dvb-t signal lock

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/staging/media/mn88472/mn88472.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
index 107552a..4d80046 100644
--- a/drivers/staging/media/mn88472/mn88472.c
+++ b/drivers/staging/media/mn88472/mn88472.c
@@ -238,6 +238,7 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	unsigned int utmp;
+	int lock = 0;
 
 	*status = 0;
 
@@ -248,6 +249,12 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
 
 	switch (c->delivery_system) {
 	case SYS_DVBT:
+		ret = regmap_read(dev->regmap[0], 0x7F, &utmp);
+		if (ret)
+			goto err;
+		if ((utmp&0xF) > 8)
+			lock = 1;
+		break;
 	case SYS_DVBT2:
 		/* FIXME: implement me */
 		utmp = 0x08; /* DVB-C lock value */
@@ -262,7 +269,7 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		goto err;
 	}
 
-	if (utmp == 0x08)
+	if (lock)
 		*status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI |
 				FE_HAS_SYNC | FE_HAS_LOCK;
 
-- 
1.9.1

[PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index ab48b5f..cdc342a 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -863,6 +863,7 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
 
 		/* attach slave demodulator */
 		if (priv->slave_demod == SLAVE_DEMOD_MN88472) {
+			struct dvb_frontend *tmp_fe;
 			struct mn88472_config mn88472_config = {};
 
 			mn88472_config.fe = &adap->fe[1];
@@ -887,6 +888,12 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
 			}
 
 			priv->i2c_client_slave_demod = client;
+
+			/* Swap frontend order */
+			tmp_fe = adap->fe[0];
+			adap->fe[0] = adap->fe[1];
+			adap->fe[1] = tmp_fe;
+
 		} else {
 			struct mn88473_config mn88473_config = {};
 
@@ -1373,7 +1380,7 @@ static int rtl2832u_frontend_ctrl(struct dvb_frontend *fe, int onoff)
 
 	/* bypass slave demod TS through master demod */
 	if (fe->id == 1 && onoff) {
-		ret = rtl2832_enable_external_ts_if(adap->fe[0]);
+		ret = rtl2832_enable_external_ts_if(adap->fe[1]);
 		if (ret)
 			goto err;
 	}
-- 
1.9.1

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Antti Palosaari]

I am not sure even idea of that. You didn't add even commit description, 
like all the other patches too :( You should really start adding commit 
messages explaining why and how commit is.

So the question is why that patch should be applied?

On the other-hand, how there is
if (fe->id == 1 && onoff) {
... as I don't remember any patch changing it to 0. I look my tree FE ID 
is 0. Do you have some unpublished hacks?

Antti


On 12/13/2014 02:18 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> index ab48b5f..cdc342a 100644
> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -863,6 +863,7 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
>
>   		/* attach slave demodulator */
>   		if (priv->slave_demod == SLAVE_DEMOD_MN88472) {
> +			struct dvb_frontend *tmp_fe;
>   			struct mn88472_config mn88472_config = {};
>
>   			mn88472_config.fe = &adap->fe[1];
> @@ -887,6 +888,12 @@ static int rtl2832u_frontend_attach(struct dvb_usb_adapter *adap)
>   			}
>
>   			priv->i2c_client_slave_demod = client;
> +
> +			/* Swap frontend order */
> +			tmp_fe = adap->fe[0];
> +			adap->fe[0] = adap->fe[1];
> +			adap->fe[1] = tmp_fe;
> +
>   		} else {
>   			struct mn88473_config mn88473_config = {};
>
> @@ -1373,7 +1380,7 @@ static int rtl2832u_frontend_ctrl(struct dvb_frontend *fe, int onoff)
>
>   	/* bypass slave demod TS through master demod */
>   	if (fe->id == 1 && onoff) {
> -		ret = rtl2832_enable_external_ts_if(adap->fe[0]);
> +		ret = rtl2832_enable_external_ts_if(adap->fe[1]);
>   		if (ret)
>   			goto err;
>   	}
>

-- 
http://palosaari.fi/

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Benjamin Larsson]

On 12/13/2014 05:02 AM, Antti Palosaari wrote:
> I am not sure even idea of that. You didn't add even commit 
> description, like all the other patches too :( You should really start 
> adding commit messages explaining why and how commit is.
>
> So the question is why that patch should be applied?

Lots of legacy applications doesn't set the frontend number and use 0 by 
default. For me to use w_scan I need this change. If that is reason good 
enough I can amend that to the commit message and resend?

>
> On the other-hand, how there is
> if (fe->id == 1 && onoff) {
> ... as I don't remember any patch changing it to 0. I look my tree FE 
> ID is 0. Do you have some unpublished hacks?

No hacks, it works for me that way.

>
> Antti
MvH
Benjamin Larsson

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Antti Palosaari]

On 12/13/2014 01:09 PM, Benjamin Larsson wrote:
> On 12/13/2014 05:02 AM, Antti Palosaari wrote:
>> I am not sure even idea of that. You didn't add even commit
>> description, like all the other patches too :( You should really start
>> adding commit messages explaining why and how commit is.
>>
>> So the question is why that patch should be applied?
>
> Lots of legacy applications doesn't set the frontend number and use 0 by
> default. For me to use w_scan I need this change. If that is reason good
> enough I can amend that to the commit message and resend?
>
>>
>> On the other-hand, how there is
>> if (fe->id == 1 && onoff) {
>> ... as I don't remember any patch changing it to 0. I look my tree FE
>> ID is 0. Do you have some unpublished hacks?
>
> No hacks, it works for me that way.

Do you understand that code at all?

Now it is:
FE0 == (fe->id == 0) == RTL2832
FE1 == (fe->id == 1) == MN88472

you changed it to:
FE0 == (fe->id == 0) == MN88472
FE1 == (fe->id == 1) == RTL2832

Then there is:

/* bypass slave demod TS through master demod */
if (fe->id == 1 && onoff) {
	ret = rtl2832_enable_external_ts_if(adap->fe[1]);
	if (ret)
		goto err;
}

After your change that code branch is taken when RTL2832 demod is 
activated / used. Shouldn't TS bypass enabled just opposite, when 
MN88472 is used....


Antti

-- 
http://palosaari.fi/

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Benjamin Larsson]

On 12/13/2014 02:35 PM, Antti Palosaari wrote:
>
>
> Do you understand that code at all?

No I can't really say I understand all the workings of the media api.

>
> Now it is:
> FE0 == (fe->id == 0) == RTL2832
> FE1 == (fe->id == 1) == MN88472
>
> you changed it to:
> FE0 == (fe->id == 0) == MN88472
> FE1 == (fe->id == 1) == RTL2832

I thought the rtl2832u_frontend_attach() actually attached the devices. 
Then the id's would have followed the frontend.


>
> Then there is:
>
> /* bypass slave demod TS through master demod */
> if (fe->id == 1 && onoff) {
>     ret = rtl2832_enable_external_ts_if(adap->fe[1]);
>     if (ret)
>         goto err;
> }
>
> After your change that code branch is taken when RTL2832 demod is 
> activated / used. Shouldn't TS bypass enabled just opposite, when 
> MN88472 is used....
>
>
> Antti
>

This intent of the patch was for better backwards compatibility with old 
software. This isn't strictly needed so consider the patch dropped.

MvH
Benjamin Larsson

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Antti Palosaari]

On 12/13/2014 08:52 PM, Benjamin Larsson wrote:
> On 12/13/2014 02:35 PM, Antti Palosaari wrote:
>>
>>
>> Do you understand that code at all?
>
> No I can't really say I understand all the workings of the media api.
>
>>
>> Now it is:
>> FE0 == (fe->id == 0) == RTL2832
>> FE1 == (fe->id == 1) == MN88472
>>
>> you changed it to:
>> FE0 == (fe->id == 0) == MN88472
>> FE1 == (fe->id == 1) == RTL2832
>
> I thought the rtl2832u_frontend_attach() actually attached the devices.
> Then the id's would have followed the frontend.
>
>
>>
>> Then there is:
>>
>> /* bypass slave demod TS through master demod */
>> if (fe->id == 1 && onoff) {
>>     ret = rtl2832_enable_external_ts_if(adap->fe[1]);
>>     if (ret)
>>         goto err;
>> }
>>
>> After your change that code branch is taken when RTL2832 demod is
>> activated / used. Shouldn't TS bypass enabled just opposite, when
>> MN88472 is used....
>>
>>
>> Antti
>>
>
> This intent of the patch was for better backwards compatibility with old
> software. This isn't strictly needed so consider the patch dropped.

I just tested that patch, and it behaves just like I expected - does not 
work at all (because RTL2832 TS bypass will not be enabled anymore).

Here is log, first with your patch, then I fixed it a little as diff 
shows, and after that scan works. I wonder what kind of test you did for 
it - or do you have some other hacks committed...

[crope@localhost linux]$ dvbv5-scan ~/.tzap/mux-Oulu-t-t2 
--file-freqs-only -a 0 -f 0
Scanning frequency #1 634000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #2 714000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #3 738000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #4 498000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #5 602000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #6 570000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #7 177500000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #8 205500000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #9 219500000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
[crope@localhost linux]$ git diff
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c 
b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 61a4a86..6902801 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1372,7 +1372,7 @@ static int rtl2832u_frontend_ctrl(struct 
dvb_frontend *fe, int onoff)
                 goto err;

         /* bypass slave demod TS through master demod */
-       if (fe->id == 1 && onoff) {
+       if (fe->id == 0 && onoff) {
                 ret = rtl2832_enable_external_ts_if(adap->fe[1]);
                 if (ret)
                         goto err;
[crope@localhost linux]$ dvbv5-scan ~/.tzap/mux-Oulu-t-t2 
--file-freqs-only -a 0 -f 0
Scanning frequency #1 634000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while reading the NIT table
Service Yle TV1, provider YLE: digital television
Service Yle TV2, provider YLE: digital television
Service Yle Fem, provider YLE: digital television
Service Yle Teema, provider YLE: digital television
Service AVA, provider MTV Oy: digital television
Service FOX, provider Fox International Channels Oy: digital television
Service Ohjelmistopäivitykset, provider Digita OY: data broadcast
Service Yle Puhe, provider YLE: digital radio
Service Yle Klassinen, provider YLE: digital radio
Service Yle Mondo, provider YLE: digital radio
Scanning frequency #2 714000000
Lock   (0x1f) Signal= 0.00%
Service 0700 11111 deitti, provider 0700 11111 deitti: digital television
Service MTV3, provider MTV Oy: digital television
Service Nelonen, provider Sanoma Television Oy: digital television
Service Sub, provider SubTV OY: digital television
Service Liv, provider Sanoma Television Oy: digital television
Service MTV MAX, provider MTV Oy: digital television
Service MTV Leffa, provider SubTV OY: digital television
Service MTV Juniori, provider SubTV OY: digital television
Service Ohjelmistopäivitykset, provider Digita Oy: data broadcast
Service Estradi, provider Digita Oy: digital television
Scanning frequency #3 738000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while reading the NIT table
Service Nelonen Pro 2, provider Sanoma Television Oy: digital television
Service TV5, provider SBS Finland: digital television
Service Nelonen Pro 1, provider Sanoma Television Oy: digital television
Service Disney Channel, provider CANAL+: digital television
Service C More First, provider C More: digital television
Service C More Series, provider C More: digital television
Service MTV Sport 1, provider C More: digital television
Service Hero, provider Sanoma: digital television
Service Iskelmä/Harju&Pöntinen, provider SBS Finland Oy / 
Etelä-Pohjanmaan Viestintä Oy: digital television
Service DIGIVIIHDE.fi, provider Telefirst Oy: digital television
Scanning frequency #4 498000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #5 602000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while reading the NIT table
Service Nelonen Prime, provider Sanoma Television Oy: digital television
Service Nelonen Nappula, provider Sanoma Television Oy: digital television
Service Nelonen Maailma, provider Sanoma Television Oy: digital television
Service Jim, provider Sanoma Television OY: digital television
Service Kutonen, provider SBS Finland: digital television
Service Discovery, provider Discovery Communications Europe: digital 
television
Service Eurosport, provider Eurosport SA: digital television
Service MTV, provider MTV Networks Europe: digital television
Service Nick Jr., provider Nickelodeon International Ltd.: digital 
television
Scanning frequency #6 570000000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #7 177500000
Lock   (0x1f) Signal= 0.00%
ERROR    dvb_read_sections: no data read on section filter
ERROR    error while waiting for PAT table
Scanning frequency #8 205500000
Lock   (0x1f) Signal= 0.00%
Service Viasat Hockey Finland, provider (null): reserved
Service Animal Planet, provider Telenor: reserved
Service Silver, provider Levira: reserved
Service Disney Junior, provider (null): reserved
Service Nelonen Pro 1 HD, provider (null): reserved
Service MTV Max HD, provider MTV OY: reserved
Service MTV Fakta, provider MTV OY: reserved
Service C More Hits, provider Telenor: reserved
Service C More First HD, provider Telenor: reserved
Service MTV Sport 2, provider MTV OY: reserved
Service Nelonen Pro 1 HD, provider (null): reserved
Service Viasat Xtra NHL 4, provider (null): reserved
Service Viasat Xtra NHL 5, provider (null): reserved
Service Disney XD, provider (null): reserved
Scanning frequency #9 219500000
Lock   (0x1f) Signal= 0.00%
Service Yle TV1 HD, provider Yle: reserved
Service Yle TV2 HD, provider Yle: reserved
Service Viasat Xtra NHL 1, provider (null): reserved
Service Viasat Xtra NHL 2, provider (null): reserved
Service Viasat Xtra NHL 3, provider (null): reserved
Service MTV3 HD, provider MTV OY: reserved
[crope@localhost linux]$



regards
Antti

-- 
http://palosaari.fi/

Re: [PATCH 2/4] rtl28xxu: swap frontend order for devices with slave demodulators

[Benjamin Larsson]

On 12/14/2014 09:05 AM, Antti Palosaari wrote:
> [...]
> I just tested that patch, and it behaves just like I expected - does 
> not work at all (because RTL2832 TS bypass will not be enabled anymore).
>
> Here is log, first with your patch, then I fixed it a little as diff 
> shows, and after that scan works. I wonder what kind of test you did 
> for it - or do you have some other hacks committed...

I tried running w_scan. I remember that I also tried with changing this:

case TUNER_RTL2832_R828D:
         fe = dvb_attach(r820t_attach, adap->fe[0],
                 priv->demod_i2c_adapter,
                 &rtl2832u_r828d_config);
         adap->fe[0]->ops.read_signal_strength =
                 adap->fe[0]->ops.tuner_ops.get_rf_strength;

         if (adap->fe[1]) {
             fe = dvb_attach(r820t_attach, adap->fe[1],
                     priv->demod_i2c_adapter,
                     &rtl2832u_r828d_config);
             adap->fe[1]->ops.read_signal_strength =
adap->fe[1]->ops.tuner_ops.get_rf_strength;
         }

to
case TUNER_RTL2832_R828D:
         if (adap->fe[1]) {
             fe = dvb_attach(r820t_attach, adap->fe[1],
                     priv->demod_i2c_adapter,
                     &rtl2832u_r828d_config);
             adap->fe[1]->ops.read_signal_strength =
adap->fe[1]->ops.tuner_ops.get_rf_strength;
         }

         fe = dvb_attach(r820t_attach, adap->fe[0],
                 priv->demod_i2c_adapter,
                 &rtl2832u_r828d_config);
         adap->fe[0]->ops.read_signal_strength =
                 adap->fe[0]->ops.tuner_ops.get_rf_strength;


I must have had that change still active in my tree. Does that make any 
sense?

MvH
Benjamin Larsson

[PATCH 3/4] mn88472: elaborate debug printout

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/staging/media/mn88472/mn88472.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
index 4d80046..746cc94 100644
--- a/drivers/staging/media/mn88472/mn88472.c
+++ b/drivers/staging/media/mn88472/mn88472.c
@@ -33,6 +33,7 @@ static int mn88472_set_frontend(struct dvb_frontend *fe)
 	u64 tmp;
 	u8 delivery_system_val, if_val[3], bw_val[7], bw_val2;
 
+	dev_dbg(&client->dev, "%s:\n", __func__);
 	dev_dbg(&client->dev,
 			"delivery_system=%d modulation=%d frequency=%d symbol_rate=%d inversion=%d\n",
 			c->delivery_system, c->modulation,
@@ -288,7 +289,7 @@ static int mn88472_init(struct dvb_frontend *fe)
 	u8 *fw_file = MN88472_FIRMWARE;
 	unsigned int csum;
 
-	dev_dbg(&client->dev, "\n");
+	dev_dbg(&client->dev, "%s:\n", __func__);
 
 	/* set cold state by default */
 	dev->warm = false;
@@ -371,7 +372,7 @@ static int mn88472_sleep(struct dvb_frontend *fe)
 	struct mn88472_dev *dev = i2c_get_clientdata(client);
 	int ret;
 
-	dev_dbg(&client->dev, "\n");
+	dev_dbg(&client->dev, "%s:\n", __func__);
 
 	/* power off */
 	ret = regmap_write(dev->regmap[2], 0x0b, 0x30);
-- 
1.9.1

Re: [PATCH 3/4] mn88472: elaborate debug printout

[Antti Palosaari]

That patch is simply wrong. Kernel logging system is able to add 
function name automatically - it should not be defined manually for 
debug logging.

See kernel documentation:
Documentation/dynamic-debug-howto.txt

around the line 213.

----------------------------------
The flags are:

   p    enables the pr_debug() callsite.
   f    Include the function name in the printed message
   l    Include line number in the printed message
   m    Include module name in the printed message
   t    Include thread ID in messages not generated from interrupt context
   _    No flags are set. (Or'd with others on input)
----------------------------------

Antti




On 12/13/2014 02:18 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/staging/media/mn88472/mn88472.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
> index 4d80046..746cc94 100644
> --- a/drivers/staging/media/mn88472/mn88472.c
> +++ b/drivers/staging/media/mn88472/mn88472.c
> @@ -33,6 +33,7 @@ static int mn88472_set_frontend(struct dvb_frontend *fe)
>   	u64 tmp;
>   	u8 delivery_system_val, if_val[3], bw_val[7], bw_val2;
>
> +	dev_dbg(&client->dev, "%s:\n", __func__);
>   	dev_dbg(&client->dev,
>   			"delivery_system=%d modulation=%d frequency=%d symbol_rate=%d inversion=%d\n",
>   			c->delivery_system, c->modulation,
> @@ -288,7 +289,7 @@ static int mn88472_init(struct dvb_frontend *fe)
>   	u8 *fw_file = MN88472_FIRMWARE;
>   	unsigned int csum;
>
> -	dev_dbg(&client->dev, "\n");
> +	dev_dbg(&client->dev, "%s:\n", __func__);
>
>   	/* set cold state by default */
>   	dev->warm = false;
> @@ -371,7 +372,7 @@ static int mn88472_sleep(struct dvb_frontend *fe)
>   	struct mn88472_dev *dev = i2c_get_clientdata(client);
>   	int ret;
>
> -	dev_dbg(&client->dev, "\n");
> +	dev_dbg(&client->dev, "%s:\n", __func__);
>
>   	/* power off */
>   	ret = regmap_write(dev->regmap[2], 0x0b, 0x30);
>

-- 
http://palosaari.fi/

[PATCH 4/4] mn88472: implemented ber reporting

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 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,
-- 
1.9.1

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

[Antti Palosaari]

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,
>

-- 
http://palosaari.fi/

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

[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 <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

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

[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 <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

-- 
http://palosaari.fi/

Re: [PATCH 1/4] mn88472: implement dvb-t signal lock

[Antti Palosaari]

That breaks DVB-C lock check as old "utmp = 0x08" was set according to 
DVB-C lock check, right?

Antti



On 12/13/2014 02:18 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/staging/media/mn88472/mn88472.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
> index 107552a..4d80046 100644
> --- a/drivers/staging/media/mn88472/mn88472.c
> +++ b/drivers/staging/media/mn88472/mn88472.c
> @@ -238,6 +238,7 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
>   	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>   	int ret;
>   	unsigned int utmp;
> +	int lock = 0;
>
>   	*status = 0;
>
> @@ -248,6 +249,12 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
>
>   	switch (c->delivery_system) {
>   	case SYS_DVBT:
> +		ret = regmap_read(dev->regmap[0], 0x7F, &utmp);
> +		if (ret)
> +			goto err;
> +		if ((utmp&0xF) > 8)
> +			lock = 1;
> +		break;
>   	case SYS_DVBT2:
>   		/* FIXME: implement me */
>   		utmp = 0x08; /* DVB-C lock value */
> @@ -262,7 +269,7 @@ static int mn88472_read_status(struct dvb_frontend *fe, fe_status_t *status)
>   		goto err;
>   	}
>
> -	if (utmp == 0x08)
> +	if (lock)
>   		*status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI |
>   				FE_HAS_SYNC | FE_HAS_LOCK;
>
>

-- 
http://palosaari.fi/

Re: [PATCH 1/4] mn88472: implement dvb-t signal lock

[Benjamin Larsson]

On 12/13/2014 04:52 AM, Antti Palosaari wrote:
> That breaks DVB-C lock check as old "utmp = 0x08" was set according to 
> DVB-C lock check, right?
>
> Antti

I have a dvb-c setup now. I will respin this patch with dvb-c support 
tested properly.

MvH
Benjamin Larsson