Re: [PATCH] mn88472: implement lock for all delivery systems

2014-12-16 Thread Benjamin Larsson

Hello.

On 12/16/2014 04:34 AM, Antti Palosaari wrote:

Moikka!

On 12/16/2014 01:40 AM, Benjamin Larsson wrote:

Signed-off-by: Benjamin Larsson benja...@southpole.se
---
  drivers/staging/media/mn88472/mn88472.c | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c 
b/drivers/staging/media/mn88472/mn88472.c

index 68f5036..426f0ed 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,21 +249,37 @@ 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 ((utmp0xF)  8)


You didn't read Kernel coding style doc?

around line 206 Documentation/CodingStyle
---
Use one space around (on each side of) most binary and ternary operators,
such as any of these:

=  +  -  *  /  %  |^  =  =  == !=  ?  :
---


Fixed.




+lock = 1;
+break;
  case SYS_DVBT2:
-/* FIXME: implement me */
-utmp = 0x08; /* DVB-C lock value */
+msleep(150);


This sleep does not look correct. Why it is here? In order to provide 
more time for lock waiting? In that case you must increase 
.get_tune_settings() timeout. On some other case you will need to add 
comment why such strange thing is needed.


Increased.




+ret = regmap_read(dev-regmap[2], 0x92, utmp);
+if (ret)
+goto err;
+if ((utmp0xF) = 0x07)
+*status |= FE_HAS_SIGNAL;
+if ((utmp0xF) = 0x0a)
+*status |= FE_HAS_CARRIER;
+if ((utmp0xF) = 0x0d)
+*status |= FE_HAS_VITERBI | FE_HAS_SYNC | FE_HAS_LOCK;
  break;
  case SYS_DVBC_ANNEX_A:
  ret = regmap_read(dev-regmap[1], 0x84, utmp);
  if (ret)
  goto err;
+if ((utmp0xF)  7)
+lock = 1;
  break;
  default:
  ret = -EINVAL;
  goto err;
  }

-if (utmp == 0x08)
+if (lock)
  *status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI |
  FE_HAS_SYNC | FE_HAS_LOCK;


Antti



Sent v2 patch.

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


[PATCH] mn88472: implement lock for all delivery systems

2014-12-15 Thread Benjamin Larsson
Signed-off-by: Benjamin Larsson benja...@southpole.se
---
 drivers/staging/media/mn88472/mn88472.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c 
b/drivers/staging/media/mn88472/mn88472.c
index 68f5036..426f0ed 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,21 +249,37 @@ 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 ((utmp0xF)  8)
+   lock = 1;
+   break;
case SYS_DVBT2:
-   /* FIXME: implement me */
-   utmp = 0x08; /* DVB-C lock value */
+   msleep(150);
+   ret = regmap_read(dev-regmap[2], 0x92, utmp);
+   if (ret)
+   goto err;
+   if ((utmp0xF) = 0x07)
+   *status |= FE_HAS_SIGNAL;
+   if ((utmp0xF) = 0x0a)
+   *status |= FE_HAS_CARRIER;
+   if ((utmp0xF) = 0x0d)
+   *status |= FE_HAS_VITERBI | FE_HAS_SYNC | FE_HAS_LOCK;
break;
case SYS_DVBC_ANNEX_A:
ret = regmap_read(dev-regmap[1], 0x84, utmp);
if (ret)
goto err;
+   if ((utmp0xF)  7)
+   lock = 1;
break;
default:
ret = -EINVAL;
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

--
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] mn88472: implement lock for all delivery systems

2014-12-15 Thread Antti Palosaari

Moikka!

On 12/16/2014 01:40 AM, Benjamin Larsson wrote:

Signed-off-by: Benjamin Larsson benja...@southpole.se
---
  drivers/staging/media/mn88472/mn88472.c | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c 
b/drivers/staging/media/mn88472/mn88472.c
index 68f5036..426f0ed 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,21 +249,37 @@ 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 ((utmp0xF)  8)


You didn't read Kernel coding style doc?

around line 206 Documentation/CodingStyle
---
Use one space around (on each side of) most binary and ternary operators,
such as any of these:

=  +  -  *  /  %  |^  =  =  ==  !=  ?  :
---


+   lock = 1;
+   break;
case SYS_DVBT2:
-   /* FIXME: implement me */
-   utmp = 0x08; /* DVB-C lock value */
+   msleep(150);


This sleep does not look correct. Why it is here? In order to provide 
more time for lock waiting? In that case you must increase 
.get_tune_settings() timeout. On some other case you will need to add 
comment why such strange thing is needed.



+   ret = regmap_read(dev-regmap[2], 0x92, utmp);
+   if (ret)
+   goto err;
+   if ((utmp0xF) = 0x07)
+   *status |= FE_HAS_SIGNAL;
+   if ((utmp0xF) = 0x0a)
+   *status |= FE_HAS_CARRIER;
+   if ((utmp0xF) = 0x0d)
+   *status |= FE_HAS_VITERBI | FE_HAS_SYNC | FE_HAS_LOCK;
break;
case SYS_DVBC_ANNEX_A:
ret = regmap_read(dev-regmap[1], 0x84, utmp);
if (ret)
goto err;
+   if ((utmp0xF)  7)
+   lock = 1;
break;
default:
ret = -EINVAL;
goto err;
}

-   if (utmp == 0x08)
+   if (lock)
*status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI |
FE_HAS_SYNC | FE_HAS_LOCK;


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