Re: [PATCH] [media] dib0090: remove manual configuration system
Hi Mauro, On Thu, 2014-09-04 at 12:36 -0300, Mauro Carvalho Chehab wrote: > Em Thu, 22 May 2014 14:48:07 +0200 > Paul Bolle escreveu: > > > dib0900.c has always shipped with its own, manual, configuration > > system. There a three problems with it. > > > > 1) macros that are defined, but not used: > > CONFIG_SYS_DVBT > > CONFIG_DIB0090_USE_PWM_AGC > > > > 2) checks for macros that are always true: > > CONFIG_SYS_ISDBT > > CONFIG_BAND_CBAND > > CONFIG_BAND_VHF > > CONFIG_BAND_UHF > > > > 3) checks for macros that are never defined and are always false: > > CONFIG_BAND_SBAND > > CONFIG_STANDARD_DAB > > CONFIG_STANDARD_DVBT > > CONFIG_TUNER_DIB0090_P1B_SUPPORT > > CONFIG_BAND_LBAND > > > > Remove all references to these macros, and, of course, remove the code > > hidden behind the macros that are never defined too. > > IMHO, it is OK to remove the macros that are always true and > the ones that aren't used. I see. I hope to send a v2 that does that one of these days. > However, I don't like the idea of > removing the other macros. This is a tuner driver that can be used > on other bands, and some day we might end implementing analog support > for the Dibcom driver or to add something that will require the code > there. So, IMHO, better to keep the code there. But would you consider a patch that at least moves those macros out of the CONFIG_* namespace (ie, a patch that prefixes those macros with, say, DIB0090_)? Paul Bolle -- 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] [media] dib0090: remove manual configuration system
Hi Paul,
Em Thu, 22 May 2014 14:48:07 +0200
Paul Bolle escreveu:
> dib0900.c has always shipped with its own, manual, configuration
> system. There a three problems with it.
>
> 1) macros that are defined, but not used:
> CONFIG_SYS_DVBT
> CONFIG_DIB0090_USE_PWM_AGC
>
> 2) checks for macros that are always true:
> CONFIG_SYS_ISDBT
> CONFIG_BAND_CBAND
> CONFIG_BAND_VHF
> CONFIG_BAND_UHF
>
> 3) checks for macros that are never defined and are always false:
> CONFIG_BAND_SBAND
> CONFIG_STANDARD_DAB
> CONFIG_STANDARD_DVBT
> CONFIG_TUNER_DIB0090_P1B_SUPPORT
> CONFIG_BAND_LBAND
>
> Remove all references to these macros, and, of course, remove the code
> hidden behind the macros that are never defined too.
IMHO, it is OK to remove the macros that are always true and
the ones that aren't used. However, I don't like the idea of
removing the other macros. This is a tuner driver that can be used
on other bands, and some day we might end implementing analog support
for the Dibcom driver or to add something that will require the code
there. So, IMHO, better to keep the code there.
Regards,
Mauro
>
> Signed-off-by: Paul Bolle
> ---
> 0) Compile tested. I don't have the hardware.
>
> 1) This might be a bit hard to review. Should I split it in two or three
> patches?
>
> 2) dib0070.c has a reference to CONFIG_SYS_ISDBT. I'll remove it in a
> future patch.
>
> 3) If this gets accepted I might be inclined to clean up the coding
> style in a future patch. It needs cleaning up quite a bit.
>
> drivers/media/dvb-frontends/dib0090.c | 130
> --
> 1 file changed, 130 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/dib0090.c
> b/drivers/media/dvb-frontends/dib0090.c
> index 3ee22ff76315..bb50fec4f475 100644
> --- a/drivers/media/dvb-frontends/dib0090.c
> +++ b/drivers/media/dvb-frontends/dib0090.c
> @@ -46,13 +46,6 @@ MODULE_PARM_DESC(debug, "turn on debugging (default: 0)");
> } \
> } while (0)
>
> -#define CONFIG_SYS_DVBT
> -#define CONFIG_SYS_ISDBT
> -#define CONFIG_BAND_CBAND
> -#define CONFIG_BAND_VHF
> -#define CONFIG_BAND_UHF
> -#define CONFIG_DIB0090_USE_PWM_AGC
> -
> #define EN_LNA0 0x8000
> #define EN_LNA1 0x4000
> #define EN_LNA2 0x2000
> @@ -1165,24 +1158,14 @@ int dib0090_gain_control(struct dvb_frontend *fe)
> state->agc_freeze = 0;
> dib0090_write_reg(state, 0x04, 0x0);
>
> -#ifdef CONFIG_BAND_SBAND
> - if (state->current_band == BAND_SBAND) {
> - dib0090_set_rframp(state, rf_ramp_sband);
> - dib0090_set_bbramp(state, bb_ramp_boost);
> - } else
> -#endif
> -#ifdef CONFIG_BAND_VHF
> if (state->current_band == BAND_VHF && !state->identity.p1g) {
> dib0090_set_rframp(state, rf_ramp_pwm_vhf);
> dib0090_set_bbramp(state, bb_ramp_pwm_normal);
> } else
> -#endif
> -#ifdef CONFIG_BAND_CBAND
> if (state->current_band == BAND_CBAND && !state->identity.p1g) {
> dib0090_set_rframp(state, rf_ramp_pwm_cband);
> dib0090_set_bbramp(state, bb_ramp_pwm_normal);
> } else
> -#endif
> if ((state->current_band == BAND_CBAND || state->current_band
> == BAND_VHF) && state->identity.p1g) {
> dib0090_set_rframp(state, rf_ramp_pwm_cband_7090p);
> dib0090_set_bbramp(state, bb_ramp_pwm_normal_socs);
> @@ -1220,14 +1203,12 @@ int dib0090_gain_control(struct dvb_frontend *fe)
>
> if (*tune_state == CT_AGC_STEP_0) {
> if (wbd_error < 0 && state->rf_gain_limit > 0 &&
> !state->identity.p1g) {
> -#ifdef CONFIG_BAND_CBAND
> /* in case of CBAND tune reduce first the
> lt_gain2 before adjusting the RF gain */
> u8 ltg2 = (state->rf_lt_def >> 10) & 0x7;
> if (state->current_band == BAND_CBAND && ltg2) {
> ltg2 >>= 1;
> state->rf_lt_def &= ltg2 << 10; /*
> reduce in 3 steps from 7 to 0 */
> }
> -#endif
> } else {
> state->agc_step = 0;
> *tune_state = CT_AGC_STEP_1;
> @@ -1238,16 +1219,6 @@ int dib0090_gain_control(struct dvb_frontend *fe)
> adc = (adc * ((s32) 355774) + (((s32) 1) << 20)) >> 21;
> /* included in [0:-700] */
>
> adc_error = (s16) (((s32) ADC_TARGET) - adc);
> -#ifdef CONFIG_STANDARD_DAB
> - if (state->fe->dtv_property_cache.delivery_system ==
> STANDARD_DAB)
> - adc_error -= 10;
> -#endif
> -#ifdef CONFIG_STANDARD_DVBT
> - if (state->fe->dtv_property_cache.delivery_system ==
> STANDARD_D
[PATCH] [media] dib0090: remove manual configuration system
dib0900.c has always shipped with its own, manual, configuration
system. There a three problems with it.
1) macros that are defined, but not used:
CONFIG_SYS_DVBT
CONFIG_DIB0090_USE_PWM_AGC
2) checks for macros that are always true:
CONFIG_SYS_ISDBT
CONFIG_BAND_CBAND
CONFIG_BAND_VHF
CONFIG_BAND_UHF
3) checks for macros that are never defined and are always false:
CONFIG_BAND_SBAND
CONFIG_STANDARD_DAB
CONFIG_STANDARD_DVBT
CONFIG_TUNER_DIB0090_P1B_SUPPORT
CONFIG_BAND_LBAND
Remove all references to these macros, and, of course, remove the code
hidden behind the macros that are never defined too.
Signed-off-by: Paul Bolle
---
0) Compile tested. I don't have the hardware.
1) This might be a bit hard to review. Should I split it in two or three
patches?
2) dib0070.c has a reference to CONFIG_SYS_ISDBT. I'll remove it in a
future patch.
3) If this gets accepted I might be inclined to clean up the coding
style in a future patch. It needs cleaning up quite a bit.
drivers/media/dvb-frontends/dib0090.c | 130 --
1 file changed, 130 deletions(-)
diff --git a/drivers/media/dvb-frontends/dib0090.c
b/drivers/media/dvb-frontends/dib0090.c
index 3ee22ff76315..bb50fec4f475 100644
--- a/drivers/media/dvb-frontends/dib0090.c
+++ b/drivers/media/dvb-frontends/dib0090.c
@@ -46,13 +46,6 @@ MODULE_PARM_DESC(debug, "turn on debugging (default: 0)");
} \
} while (0)
-#define CONFIG_SYS_DVBT
-#define CONFIG_SYS_ISDBT
-#define CONFIG_BAND_CBAND
-#define CONFIG_BAND_VHF
-#define CONFIG_BAND_UHF
-#define CONFIG_DIB0090_USE_PWM_AGC
-
#define EN_LNA0 0x8000
#define EN_LNA1 0x4000
#define EN_LNA2 0x2000
@@ -1165,24 +1158,14 @@ int dib0090_gain_control(struct dvb_frontend *fe)
state->agc_freeze = 0;
dib0090_write_reg(state, 0x04, 0x0);
-#ifdef CONFIG_BAND_SBAND
- if (state->current_band == BAND_SBAND) {
- dib0090_set_rframp(state, rf_ramp_sband);
- dib0090_set_bbramp(state, bb_ramp_boost);
- } else
-#endif
-#ifdef CONFIG_BAND_VHF
if (state->current_band == BAND_VHF && !state->identity.p1g) {
dib0090_set_rframp(state, rf_ramp_pwm_vhf);
dib0090_set_bbramp(state, bb_ramp_pwm_normal);
} else
-#endif
-#ifdef CONFIG_BAND_CBAND
if (state->current_band == BAND_CBAND && !state->identity.p1g) {
dib0090_set_rframp(state, rf_ramp_pwm_cband);
dib0090_set_bbramp(state, bb_ramp_pwm_normal);
} else
-#endif
if ((state->current_band == BAND_CBAND || state->current_band
== BAND_VHF) && state->identity.p1g) {
dib0090_set_rframp(state, rf_ramp_pwm_cband_7090p);
dib0090_set_bbramp(state, bb_ramp_pwm_normal_socs);
@@ -1220,14 +1203,12 @@ int dib0090_gain_control(struct dvb_frontend *fe)
if (*tune_state == CT_AGC_STEP_0) {
if (wbd_error < 0 && state->rf_gain_limit > 0 &&
!state->identity.p1g) {
-#ifdef CONFIG_BAND_CBAND
/* in case of CBAND tune reduce first the
lt_gain2 before adjusting the RF gain */
u8 ltg2 = (state->rf_lt_def >> 10) & 0x7;
if (state->current_band == BAND_CBAND && ltg2) {
ltg2 >>= 1;
state->rf_lt_def &= ltg2 << 10; /*
reduce in 3 steps from 7 to 0 */
}
-#endif
} else {
state->agc_step = 0;
*tune_state = CT_AGC_STEP_1;
@@ -1238,16 +1219,6 @@ int dib0090_gain_control(struct dvb_frontend *fe)
adc = (adc * ((s32) 355774) + (((s32) 1) << 20)) >> 21;
/* included in [0:-700] */
adc_error = (s16) (((s32) ADC_TARGET) - adc);
-#ifdef CONFIG_STANDARD_DAB
- if (state->fe->dtv_property_cache.delivery_system ==
STANDARD_DAB)
- adc_error -= 10;
-#endif
-#ifdef CONFIG_STANDARD_DVBT
- if (state->fe->dtv_property_cache.delivery_system ==
STANDARD_DVBT &&
-
(state->fe->dtv_property_cache.modulation == QAM_64 ||
state->fe->dtv_property_cache.modulation == QAM_16))
- adc_error += 60;
-#endif
-#ifdef CONFIG_SYS_ISDBT
if ((state->fe->dtv_property_cache.delivery_system ==
SYS_ISDBT) && (((state->fe->dtv_property_cache.layer[0].segment_count >
0)
&&
@@ -1274,17 +1245,9 @@ int dib0090_gain_control(struct dvb_frontend *fe)
