mn88472: implement lock for all delivery systems
Commit Message
Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
drivers/staging/media/mn88472/mn88472.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
Comments
Moikka!
On 12/16/2014 01:40 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@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 ((utmp&0xF) > 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 ((utmp&0xF) >= 0x07)
> + *status |= FE_HAS_SIGNAL;
> + if ((utmp&0xF) >= 0x0a)
> + *status |= FE_HAS_CARRIER;
> + if ((utmp&0xF) >= 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 ((utmp&0xF) > 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
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 <benjamin@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 ((utmp&0xF) > 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 ((utmp&0xF) >= 0x07)
>> + *status |= FE_HAS_SIGNAL;
>> + if ((utmp&0xF) >= 0x0a)
>> + *status |= FE_HAS_CARRIER;
>> + if ((utmp&0xF) >= 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 ((utmp&0xF) > 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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -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 ((utmp&0xF) > 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 ((utmp&0xF) >= 0x07)
+ *status |= FE_HAS_SIGNAL;
+ if ((utmp&0xF) >= 0x0a)
+ *status |= FE_HAS_CARRIER;
+ if ((utmp&0xF) >= 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 ((utmp&0xF) > 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;