* [PATCH RFC] dvb: LNA implementation changes
@ 2012-10-01 0:35 Antti Palosaari
2012-10-01 0:45 ` Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2012-10-01 0:35 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab, Antti Palosaari
* use dvb property cache
* implement get
* LNA_AUTO value changed
Hans and Mauro proposed use of cache implementation of get as they
were planning to extend LNA usage for analog side too.
LNA_AUTO value was changed from (~0U) to INT_MIN as (~0U) resulted
only -1 which is waste of numeric range if need to extend that in
the future.
Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
Reported-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb-core/dvb_frontend.c | 18 ++++++++++++++----
drivers/media/dvb-core/dvb_frontend.h | 4 +++-
drivers/media/usb/em28xx/em28xx-dvb.c | 9 +++++----
include/linux/dvb/frontend.h | 2 +-
4 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 8f58f24..246a3c5 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct dvb_frontend *fe)
break;
}
+ c->lna = LNA_AUTO;
+
return 0;
}
@@ -1054,6 +1056,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+
+ _DTV_CMD(DTV_LNA, 0, 0),
};
static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property *tvp)
@@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_d;
break;
+ case DTV_LNA:
+ tvp->u.data = c->lna;
+ break;
+
default:
return -EINVAL;
}
@@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
case DTV_INTERLEAVING:
c->interleaving = tvp->u.data;
break;
- case DTV_LNA:
- if (fe->ops.set_lna)
- r = fe->ops.set_lna(fe, tvp->u.data);
- break;
/* ISDB-T Support here */
case DTV_ISDBT_PARTIAL_RECEPTION:
@@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
break;
+ case DTV_LNA:
+ c->lna = tvp->u.data;
+ if (fe->ops.set_lna)
+ r = fe->ops.set_lna(fe);
+ break;
+
default:
return -EINVAL;
}
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index 44a445c..5d25953 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -303,7 +303,7 @@ struct dvb_frontend_ops {
int (*dishnetwork_send_legacy_command)(struct dvb_frontend* fe, unsigned long cmd);
int (*i2c_gate_ctrl)(struct dvb_frontend* fe, int enable);
int (*ts_bus_ctrl)(struct dvb_frontend* fe, int acquire);
- int (*set_lna)(struct dvb_frontend *, int);
+ int (*set_lna)(struct dvb_frontend *);
/* These callbacks are for devices that implement their own
* tuning algorithms, rather than a simple swzigzag
@@ -391,6 +391,8 @@ struct dtv_frontend_properties {
u8 atscmh_sccc_code_mode_b;
u8 atscmh_sccc_code_mode_c;
u8 atscmh_sccc_code_mode_d;
+
+ int lna;
};
struct dvb_frontend {
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 109474b..1166e8b 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -569,15 +569,16 @@ static void pctv_520e_init(struct em28xx *dev)
i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
};
-static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
+static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
struct em28xx *dev = fe->dvb->priv;
#ifdef CONFIG_GPIOLIB
struct em28xx_dvb *dvb = dev->dvb;
int ret;
unsigned long flags;
- if (val)
+ if (c->lna)
flags = GPIOF_OUT_INIT_LOW;
else
flags = GPIOF_OUT_INIT_HIGH;
@@ -590,8 +591,8 @@ static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
return ret;
#else
- dev_warn(&dev->udev->dev, "%s: LNA control is disabled\n",
- KBUILD_MODNAME);
+ dev_warn(&dev->udev->dev, "%s: LNA control is disabled (lna=%d)\n",
+ KBUILD_MODNAME, c->lna);
return 0;
#endif
}
diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
index c12d452..6c97457 100644
--- a/include/linux/dvb/frontend.h
+++ b/include/linux/dvb/frontend.h
@@ -439,7 +439,7 @@ enum atscmh_rs_code_mode {
};
#define NO_STREAM_ID_FILTER (~0U)
-#define LNA_AUTO (~0U)
+#define LNA_AUTO INT_MIN
struct dtv_cmds_h {
char *name; /* A display name for debugging purposes */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] dvb: LNA implementation changes
2012-10-01 0:35 [PATCH RFC] dvb: LNA implementation changes Antti Palosaari
@ 2012-10-01 0:45 ` Antti Palosaari
2012-10-02 0:22 ` Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2012-10-01 0:45 UTC (permalink / raw)
Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
I added few comments for things what I was a little but unsure. Please
comment.
On 10/01/2012 03:35 AM, Antti Palosaari wrote:
> * use dvb property cache
> * implement get
> * LNA_AUTO value changed
>
> Hans and Mauro proposed use of cache implementation of get as they
> were planning to extend LNA usage for analog side too.
>
> LNA_AUTO value was changed from (~0U) to INT_MIN as (~0U) resulted
> only -1 which is waste of numeric range if need to extend that in
> the future.
>
> Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
> Reported-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/dvb-core/dvb_frontend.c | 18 ++++++++++++++----
> drivers/media/dvb-core/dvb_frontend.h | 4 +++-
> drivers/media/usb/em28xx/em28xx-dvb.c | 9 +++++----
> include/linux/dvb/frontend.h | 2 +-
> 4 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 8f58f24..246a3c5 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct dvb_frontend *fe)
> break;
> }
>
> + c->lna = LNA_AUTO;
> +
> return 0;
> }
>
> @@ -1054,6 +1056,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
> +
> + _DTV_CMD(DTV_LNA, 0, 0),
> };
>
> static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property *tvp)
> @@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
> tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_d;
> break;
>
> + case DTV_LNA:
> + tvp->u.data = c->lna;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
> case DTV_INTERLEAVING:
> c->interleaving = tvp->u.data;
> break;
> - case DTV_LNA:
> - if (fe->ops.set_lna)
> - r = fe->ops.set_lna(fe, tvp->u.data);
> - break;
>
> /* ISDB-T Support here */
> case DTV_ISDBT_PARTIAL_RECEPTION:
> @@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
> fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
> break;
>
> + case DTV_LNA:
> + c->lna = tvp->u.data;
> + if (fe->ops.set_lna)
> + r = fe->ops.set_lna(fe);
> + break;
> +
> default:
> return -EINVAL;
> }
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index 44a445c..5d25953 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -303,7 +303,7 @@ struct dvb_frontend_ops {
> int (*dishnetwork_send_legacy_command)(struct dvb_frontend* fe, unsigned long cmd);
> int (*i2c_gate_ctrl)(struct dvb_frontend* fe, int enable);
> int (*ts_bus_ctrl)(struct dvb_frontend* fe, int acquire);
> - int (*set_lna)(struct dvb_frontend *, int);
> + int (*set_lna)(struct dvb_frontend *);
>
> /* These callbacks are for devices that implement their own
> * tuning algorithms, rather than a simple swzigzag
> @@ -391,6 +391,8 @@ struct dtv_frontend_properties {
> u8 atscmh_sccc_code_mode_b;
> u8 atscmh_sccc_code_mode_c;
> u8 atscmh_sccc_code_mode_d;
> +
> + int lna;
Is it reason or coincidence that all the other variables are unsigned here?
> };
>
> struct dvb_frontend {
> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
> index 109474b..1166e8b 100644
> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -569,15 +569,16 @@ static void pctv_520e_init(struct em28xx *dev)
> i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
> };
>
> -static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
> +static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
> {
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> struct em28xx *dev = fe->dvb->priv;
> #ifdef CONFIG_GPIOLIB
> struct em28xx_dvb *dvb = dev->dvb;
> int ret;
> unsigned long flags;
>
> - if (val)
> + if (c->lna)
> flags = GPIOF_OUT_INIT_LOW;
> else
> flags = GPIOF_OUT_INIT_HIGH;
> @@ -590,8 +591,8 @@ static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
>
> return ret;
> #else
> - dev_warn(&dev->udev->dev, "%s: LNA control is disabled\n",
> - KBUILD_MODNAME);
> + dev_warn(&dev->udev->dev, "%s: LNA control is disabled (lna=%d)\n",
> + KBUILD_MODNAME, c->lna);
> return 0;
> #endif
> }
> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
> index c12d452..6c97457 100644
> --- a/include/linux/dvb/frontend.h
> +++ b/include/linux/dvb/frontend.h
> @@ -439,7 +439,7 @@ enum atscmh_rs_code_mode {
> };
>
> #define NO_STREAM_ID_FILTER (~0U)
> -#define LNA_AUTO (~0U)
> +#define LNA_AUTO INT_MIN
That's (INT_MIN) again here because I used int in struct
dtv_frontend_properties.
I would like to use signed value that this could be extended later like
use of attenuation.
>
> struct dtv_cmds_h {
> char *name; /* A display name for debugging purposes */
>
And one question still. If we use signed value for LNA then value 0
could be used as a LNA_AUTO. Like:
-1 = LNA disabled
0 = LNA AUTO
1 = LNA enabled
It is easy to change it now as it is new feature for 3.7. After that
everything goes harder, so I really would like to get comments before
-rc1 :)
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] dvb: LNA implementation changes
2012-10-01 0:45 ` Antti Palosaari
@ 2012-10-02 0:22 ` Antti Palosaari
2012-10-02 7:33 ` Hans Verkuil
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2012-10-02 0:22 UTC (permalink / raw)
To: linux-media, Hans Verkuil, Mauro Carvalho Chehab
Ping!
u.data is defined __u32. Does it mean we could only use unsigned values
when DVB API v5 ?
If yes, I will change LNA according to that and use 32bit maximum as
LNA_AUTO.
struct dtv_property {
__u32 cmd;
__u32 reserved[3];
union {
__u32 data;
struct {
__u8 data[32];
__u32 len;
__u32 reserved1[3];
void *reserved2;
} buffer;
} u;
int result;
} __attribute__ ((packed));
On 10/01/2012 03:45 AM, Antti Palosaari wrote:
> I added few comments for things what I was a little but unsure. Please
> comment.
>
> On 10/01/2012 03:35 AM, Antti Palosaari wrote:
>> * use dvb property cache
>> * implement get
>> * LNA_AUTO value changed
>>
>> Hans and Mauro proposed use of cache implementation of get as they
>> were planning to extend LNA usage for analog side too.
>>
>> LNA_AUTO value was changed from (~0U) to INT_MIN as (~0U) resulted
>> only -1 which is waste of numeric range if need to extend that in
>> the future.
>>
>> Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
>> Reported-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> drivers/media/dvb-core/dvb_frontend.c | 18 ++++++++++++++----
>> drivers/media/dvb-core/dvb_frontend.h | 4 +++-
>> drivers/media/usb/em28xx/em28xx-dvb.c | 9 +++++----
>> include/linux/dvb/frontend.h | 2 +-
>> 4 files changed, 23 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/dvb-core/dvb_frontend.c
>> b/drivers/media/dvb-core/dvb_frontend.c
>> index 8f58f24..246a3c5 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb-core/dvb_frontend.c
>> @@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct
>> dvb_frontend *fe)
>> break;
>> }
>>
>> + c->lna = LNA_AUTO;
>> +
>> return 0;
>> }
>>
>> @@ -1054,6 +1056,8 @@ static struct dtv_cmds_h
>> dtv_cmds[DTV_MAX_COMMAND + 1] = {
>> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
>> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
>> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
>> +
>> + _DTV_CMD(DTV_LNA, 0, 0),
>> };
>>
>> static void dtv_property_dump(struct dvb_frontend *fe, struct
>> dtv_property *tvp)
>> @@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct
>> dvb_frontend *fe,
>> tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_d;
>> break;
>>
>> + case DTV_LNA:
>> + tvp->u.data = c->lna;
>> + break;
>> +
>> default:
>> return -EINVAL;
>> }
>> @@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct
>> dvb_frontend *fe,
>> case DTV_INTERLEAVING:
>> c->interleaving = tvp->u.data;
>> break;
>> - case DTV_LNA:
>> - if (fe->ops.set_lna)
>> - r = fe->ops.set_lna(fe, tvp->u.data);
>> - break;
>>
>> /* ISDB-T Support here */
>> case DTV_ISDBT_PARTIAL_RECEPTION:
>> @@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct
>> dvb_frontend *fe,
>> fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
>> break;
>>
>> + case DTV_LNA:
>> + c->lna = tvp->u.data;
>> + if (fe->ops.set_lna)
>> + r = fe->ops.set_lna(fe);
>> + break;
>> +
>> default:
>> return -EINVAL;
>> }
>> diff --git a/drivers/media/dvb-core/dvb_frontend.h
>> b/drivers/media/dvb-core/dvb_frontend.h
>> index 44a445c..5d25953 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.h
>> +++ b/drivers/media/dvb-core/dvb_frontend.h
>> @@ -303,7 +303,7 @@ struct dvb_frontend_ops {
>> int (*dishnetwork_send_legacy_command)(struct dvb_frontend* fe,
>> unsigned long cmd);
>> int (*i2c_gate_ctrl)(struct dvb_frontend* fe, int enable);
>> int (*ts_bus_ctrl)(struct dvb_frontend* fe, int acquire);
>> - int (*set_lna)(struct dvb_frontend *, int);
>> + int (*set_lna)(struct dvb_frontend *);
>>
>> /* These callbacks are for devices that implement their own
>> * tuning algorithms, rather than a simple swzigzag
>> @@ -391,6 +391,8 @@ struct dtv_frontend_properties {
>> u8 atscmh_sccc_code_mode_b;
>> u8 atscmh_sccc_code_mode_c;
>> u8 atscmh_sccc_code_mode_d;
>> +
>> + int lna;
>
> Is it reason or coincidence that all the other variables are unsigned here?
>
>> };
>>
>> struct dvb_frontend {
>> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
>> b/drivers/media/usb/em28xx/em28xx-dvb.c
>> index 109474b..1166e8b 100644
>> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
>> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
>> @@ -569,15 +569,16 @@ static void pctv_520e_init(struct em28xx *dev)
>> i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
>> };
>>
>> -static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
>> +static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
>> {
>> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> struct em28xx *dev = fe->dvb->priv;
>> #ifdef CONFIG_GPIOLIB
>> struct em28xx_dvb *dvb = dev->dvb;
>> int ret;
>> unsigned long flags;
>>
>> - if (val)
>> + if (c->lna)
>> flags = GPIOF_OUT_INIT_LOW;
>> else
>> flags = GPIOF_OUT_INIT_HIGH;
>> @@ -590,8 +591,8 @@ static int em28xx_pctv_290e_set_lna(struct
>> dvb_frontend *fe, int val)
>>
>> return ret;
>> #else
>> - dev_warn(&dev->udev->dev, "%s: LNA control is disabled\n",
>> - KBUILD_MODNAME);
>> + dev_warn(&dev->udev->dev, "%s: LNA control is disabled (lna=%d)\n",
>> + KBUILD_MODNAME, c->lna);
>> return 0;
>> #endif
>> }
>> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
>> index c12d452..6c97457 100644
>> --- a/include/linux/dvb/frontend.h
>> +++ b/include/linux/dvb/frontend.h
>> @@ -439,7 +439,7 @@ enum atscmh_rs_code_mode {
>> };
>>
>> #define NO_STREAM_ID_FILTER (~0U)
>> -#define LNA_AUTO (~0U)
>> +#define LNA_AUTO INT_MIN
>
> That's (INT_MIN) again here because I used int in struct
> dtv_frontend_properties.
>
> I would like to use signed value that this could be extended later like
> use of attenuation.
>
>
>>
>> struct dtv_cmds_h {
>> char *name; /* A display name for debugging purposes */
>>
>
> And one question still. If we use signed value for LNA then value 0
> could be used as a LNA_AUTO. Like:
> -1 = LNA disabled
> 0 = LNA AUTO
> 1 = LNA enabled
>
>
> It is easy to change it now as it is new feature for 3.7. After that
> everything goes harder, so I really would like to get comments before
> -rc1 :)
>
> regards
> Antti
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] dvb: LNA implementation changes
2012-10-02 0:22 ` Antti Palosaari
@ 2012-10-02 7:33 ` Hans Verkuil
0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2012-10-02 7:33 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media, Mauro Carvalho Chehab
On Tue 2 October 2012 02:22:55 Antti Palosaari wrote:
> Ping!
>
> u.data is defined __u32. Does it mean we could only use unsigned values
> when DVB API v5 ?
Looking at the dtv_property I'd say it only supports unsigned.
So you need an enum like this:
enum fe_lna {
LNA_AUTO,
LNA_OFF,
LNA_ON,
};
But Mauro knows that better than I do.
Regards,
Hans
> If yes, I will change LNA according to that and use 32bit maximum as
> LNA_AUTO.
>
> struct dtv_property {
> __u32 cmd;
> __u32 reserved[3];
> union {
> __u32 data;
> struct {
> __u8 data[32];
> __u32 len;
> __u32 reserved1[3];
> void *reserved2;
> } buffer;
> } u;
> int result;
> } __attribute__ ((packed));
>
>
> On 10/01/2012 03:45 AM, Antti Palosaari wrote:
> > I added few comments for things what I was a little but unsure. Please
> > comment.
> >
> > On 10/01/2012 03:35 AM, Antti Palosaari wrote:
> >> * use dvb property cache
> >> * implement get
> >> * LNA_AUTO value changed
> >>
> >> Hans and Mauro proposed use of cache implementation of get as they
> >> were planning to extend LNA usage for analog side too.
> >>
> >> LNA_AUTO value was changed from (~0U) to INT_MIN as (~0U) resulted
> >> only -1 which is waste of numeric range if need to extend that in
> >> the future.
> >>
> >> Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
> >> Reported-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> >> Signed-off-by: Antti Palosaari <crope@iki.fi>
> >> ---
> >> drivers/media/dvb-core/dvb_frontend.c | 18 ++++++++++++++----
> >> drivers/media/dvb-core/dvb_frontend.h | 4 +++-
> >> drivers/media/usb/em28xx/em28xx-dvb.c | 9 +++++----
> >> include/linux/dvb/frontend.h | 2 +-
> >> 4 files changed, 23 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/media/dvb-core/dvb_frontend.c
> >> b/drivers/media/dvb-core/dvb_frontend.c
> >> index 8f58f24..246a3c5 100644
> >> --- a/drivers/media/dvb-core/dvb_frontend.c
> >> +++ b/drivers/media/dvb-core/dvb_frontend.c
> >> @@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct
> >> dvb_frontend *fe)
> >> break;
> >> }
> >>
> >> + c->lna = LNA_AUTO;
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -1054,6 +1056,8 @@ static struct dtv_cmds_h
> >> dtv_cmds[DTV_MAX_COMMAND + 1] = {
> >> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
> >> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
> >> _DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
> >> +
> >> + _DTV_CMD(DTV_LNA, 0, 0),
> >> };
> >>
> >> static void dtv_property_dump(struct dvb_frontend *fe, struct
> >> dtv_property *tvp)
> >> @@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct
> >> dvb_frontend *fe,
> >> tvp->u.data = fe->dtv_property_cache.atscmh_sccc_code_mode_d;
> >> break;
> >>
> >> + case DTV_LNA:
> >> + tvp->u.data = c->lna;
> >> + break;
> >> +
> >> default:
> >> return -EINVAL;
> >> }
> >> @@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct
> >> dvb_frontend *fe,
> >> case DTV_INTERLEAVING:
> >> c->interleaving = tvp->u.data;
> >> break;
> >> - case DTV_LNA:
> >> - if (fe->ops.set_lna)
> >> - r = fe->ops.set_lna(fe, tvp->u.data);
> >> - break;
> >>
> >> /* ISDB-T Support here */
> >> case DTV_ISDBT_PARTIAL_RECEPTION:
> >> @@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct
> >> dvb_frontend *fe,
> >> fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
> >> break;
> >>
> >> + case DTV_LNA:
> >> + c->lna = tvp->u.data;
> >> + if (fe->ops.set_lna)
> >> + r = fe->ops.set_lna(fe);
> >> + break;
> >> +
> >> default:
> >> return -EINVAL;
> >> }
> >> diff --git a/drivers/media/dvb-core/dvb_frontend.h
> >> b/drivers/media/dvb-core/dvb_frontend.h
> >> index 44a445c..5d25953 100644
> >> --- a/drivers/media/dvb-core/dvb_frontend.h
> >> +++ b/drivers/media/dvb-core/dvb_frontend.h
> >> @@ -303,7 +303,7 @@ struct dvb_frontend_ops {
> >> int (*dishnetwork_send_legacy_command)(struct dvb_frontend* fe,
> >> unsigned long cmd);
> >> int (*i2c_gate_ctrl)(struct dvb_frontend* fe, int enable);
> >> int (*ts_bus_ctrl)(struct dvb_frontend* fe, int acquire);
> >> - int (*set_lna)(struct dvb_frontend *, int);
> >> + int (*set_lna)(struct dvb_frontend *);
> >>
> >> /* These callbacks are for devices that implement their own
> >> * tuning algorithms, rather than a simple swzigzag
> >> @@ -391,6 +391,8 @@ struct dtv_frontend_properties {
> >> u8 atscmh_sccc_code_mode_b;
> >> u8 atscmh_sccc_code_mode_c;
> >> u8 atscmh_sccc_code_mode_d;
> >> +
> >> + int lna;
> >
> > Is it reason or coincidence that all the other variables are unsigned here?
> >
> >> };
> >>
> >> struct dvb_frontend {
> >> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
> >> b/drivers/media/usb/em28xx/em28xx-dvb.c
> >> index 109474b..1166e8b 100644
> >> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> >> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> >> @@ -569,15 +569,16 @@ static void pctv_520e_init(struct em28xx *dev)
> >> i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
> >> };
> >>
> >> -static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
> >> +static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
> >> {
> >> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> >> struct em28xx *dev = fe->dvb->priv;
> >> #ifdef CONFIG_GPIOLIB
> >> struct em28xx_dvb *dvb = dev->dvb;
> >> int ret;
> >> unsigned long flags;
> >>
> >> - if (val)
> >> + if (c->lna)
> >> flags = GPIOF_OUT_INIT_LOW;
> >> else
> >> flags = GPIOF_OUT_INIT_HIGH;
> >> @@ -590,8 +591,8 @@ static int em28xx_pctv_290e_set_lna(struct
> >> dvb_frontend *fe, int val)
> >>
> >> return ret;
> >> #else
> >> - dev_warn(&dev->udev->dev, "%s: LNA control is disabled\n",
> >> - KBUILD_MODNAME);
> >> + dev_warn(&dev->udev->dev, "%s: LNA control is disabled (lna=%d)\n",
> >> + KBUILD_MODNAME, c->lna);
> >> return 0;
> >> #endif
> >> }
> >> diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
> >> index c12d452..6c97457 100644
> >> --- a/include/linux/dvb/frontend.h
> >> +++ b/include/linux/dvb/frontend.h
> >> @@ -439,7 +439,7 @@ enum atscmh_rs_code_mode {
> >> };
> >>
> >> #define NO_STREAM_ID_FILTER (~0U)
> >> -#define LNA_AUTO (~0U)
> >> +#define LNA_AUTO INT_MIN
> >
> > That's (INT_MIN) again here because I used int in struct
> > dtv_frontend_properties.
> >
> > I would like to use signed value that this could be extended later like
> > use of attenuation.
> >
> >
> >>
> >> struct dtv_cmds_h {
> >> char *name; /* A display name for debugging purposes */
> >>
> >
> > And one question still. If we use signed value for LNA then value 0
> > could be used as a LNA_AUTO. Like:
> > -1 = LNA disabled
> > 0 = LNA AUTO
> > 1 = LNA enabled
> >
> >
> > It is easy to change it now as it is new feature for 3.7. After that
> > everything goes harder, so I really would like to get comments before
> > -rc1 :)
> >
> > regards
> > Antti
> >
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-02 7:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-01 0:35 [PATCH RFC] dvb: LNA implementation changes Antti Palosaari
2012-10-01 0:45 ` Antti Palosaari
2012-10-02 0:22 ` Antti Palosaari
2012-10-02 7:33 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).