* [PATCH 0/4] dvb_frontend: few DTV validation routines
@ 2012-08-17 2:03 Antti Palosaari
2012-08-17 2:03 ` [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation Antti Palosaari
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Antti Palosaari @ 2012-08-17 2:03 UTC (permalink / raw)
To: linux-media; +Cc: Hin-Tak Leung, Antti Palosaari
It could be nice to validate transmission parameters, coming from
the userspace, against standards before those are passed to the
individual chipset driver. As a starting point towards that
I implemented checks for few common standards. Those checks could
be better as I added almost none checks for comparing allowed
parameter combinations. I found it quite time consuming to search
all allowed parameters and combinations...
Those checks are now exported from the dvb-frontend, making for
example demodulator driver possible to call. Maybe someday those
could be used by frontend itself to validate data before pass call
for the driver.
I also noticed our documentation lacks quite totally possible values,
only possible parameters were listed. How do we expect application
makers could know those?
Antti Palosaari (4):
dvb_frontend: add routine for DVB-T parameter validation
dvb_frontend: add routine for DVB-T2 parameter validation
dvb_frontend: add routine for DVB-C annex A parameter validation
dvb_frontend: add routine for DTMB parameter validation
drivers/media/dvb-core/dvb_frontend.c | 405 ++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_frontend.h | 5 +
2 files changed, 410 insertions(+)
--
1.7.11.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation
2012-08-17 2:03 [PATCH 0/4] dvb_frontend: few DTV validation routines Antti Palosaari
@ 2012-08-17 2:03 ` Antti Palosaari
2012-09-11 19:23 ` Mauro Carvalho Chehab
2012-08-17 2:03 ` [PATCH 2/4] dvb_frontend: add routine for DVB-T2 " Antti Palosaari
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Antti Palosaari @ 2012-08-17 2:03 UTC (permalink / raw)
To: linux-media; +Cc: Hin-Tak Leung, Antti Palosaari
Common routine for use of dvb-core, demodulator and tuner for check
given DVB-T parameters correctness.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb-core/dvb_frontend.c | 136 ++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_frontend.h | 2 +
2 files changed, 138 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index d29d41a..4abb648 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2505,6 +2505,142 @@ int dvb_frontend_resume(struct dvb_frontend *fe)
}
EXPORT_SYMBOL(dvb_frontend_resume);
+int dvb_validate_params_dvbt(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+
+ dev_dbg(fe->dvb->device, "%s:\n", __func__);
+
+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
+ c->delivery_system);
+ return -EINVAL;
+ }
+
+ if (c->frequency >= 174000000 && c->frequency <= 230000000) {
+ ;
+ } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
+ c->frequency);
+ return -EINVAL;
+ }
+
+ switch (c->bandwidth_hz) {
+ case 6000000:
+ case 7000000:
+ case 8000000:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
+ c->bandwidth_hz);
+ return -EINVAL;
+ }
+
+ switch (c->transmission_mode) {
+ case TRANSMISSION_MODE_AUTO:
+ case TRANSMISSION_MODE_2K:
+ case TRANSMISSION_MODE_8K:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
+ c->transmission_mode);
+ return -EINVAL;
+ }
+
+ switch (c->modulation) {
+ case QAM_AUTO:
+ case QPSK:
+ case QAM_16:
+ case QAM_64:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
+ c->modulation);
+ return -EINVAL;
+ }
+
+ switch (c->guard_interval) {
+ case GUARD_INTERVAL_AUTO:
+ case GUARD_INTERVAL_1_32:
+ case GUARD_INTERVAL_1_16:
+ case GUARD_INTERVAL_1_8:
+ case GUARD_INTERVAL_1_4:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
+ c->guard_interval);
+ return -EINVAL;
+ }
+
+ switch (c->hierarchy) {
+ case HIERARCHY_NONE:
+ case HIERARCHY_AUTO:
+ case HIERARCHY_1:
+ case HIERARCHY_2:
+ case HIERARCHY_4:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: hierarchy=%d\n", __func__,
+ c->hierarchy);
+ return -EINVAL;
+ }
+
+ switch (c->code_rate_HP) {
+ case FEC_AUTO:
+ case FEC_1_2:
+ case FEC_2_3:
+ case FEC_3_4:
+ case FEC_5_6:
+ case FEC_7_8:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: code_rate_HP=%d\n", __func__,
+ c->code_rate_HP);
+ return -EINVAL;
+ }
+
+ /*
+ * code_rate_LP is used only with hierarchical coding
+ */
+ if (c->hierarchy == HIERARCHY_NONE) {
+ switch (c->code_rate_LP) {
+ case FEC_NONE:
+ /*
+ * TODO: FEC_AUTO here is wrong, but for some reason
+ * dtv_set_frontend() forces it.
+ */
+ case FEC_AUTO:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
+ __func__, c->code_rate_LP);
+ return -EINVAL;
+ }
+ } else {
+ switch (c->code_rate_LP) {
+ case FEC_AUTO:
+ case FEC_1_2:
+ case FEC_2_3:
+ case FEC_3_4:
+ case FEC_5_6:
+ case FEC_7_8:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
+ __func__, c->code_rate_LP);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(dvb_validate_params_dvbt);
+
int dvb_register_frontend(struct dvb_adapter* dvb,
struct dvb_frontend* fe)
{
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index 74ba563..6df0c44 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -425,4 +425,6 @@ extern int dvb_frontend_resume(struct dvb_frontend *fe);
extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
+extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
+
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] dvb_frontend: add routine for DVB-T2 parameter validation
2012-08-17 2:03 [PATCH 0/4] dvb_frontend: few DTV validation routines Antti Palosaari
2012-08-17 2:03 ` [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation Antti Palosaari
@ 2012-08-17 2:03 ` Antti Palosaari
2012-09-11 19:33 ` Mauro Carvalho Chehab
2012-08-17 2:03 ` [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A " Antti Palosaari
2012-08-17 2:03 ` [PATCH 4/4] dvb_frontend: add routine for DTMB " Antti Palosaari
3 siblings, 1 reply; 13+ messages in thread
From: Antti Palosaari @ 2012-08-17 2:03 UTC (permalink / raw)
To: linux-media; +Cc: Hin-Tak Leung, Antti Palosaari
Common routine for use of dvb-core, demodulator and tuner for check
given DVB-T2 parameters correctness.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb-core/dvb_frontend.c | 118 ++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_frontend.h | 1 +
2 files changed, 119 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 4abb648..6413c74 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2641,6 +2641,124 @@ int dvb_validate_params_dvbt(struct dvb_frontend *fe)
}
EXPORT_SYMBOL(dvb_validate_params_dvbt);
+int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+
+ dev_dbg(fe->dvb->device, "%s:\n", __func__);
+
+ switch (c->delivery_system) {
+ case SYS_DVBT2:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
+ c->delivery_system);
+ return -EINVAL;
+ }
+
+ /*
+ * DVB-T2 specification as such does not specify any frequency bands.
+ * Define real life limits still. L-Band 1452 - 1492 MHz may exits in
+ * future too.
+ */
+ if (c->frequency >= 174000000 && c->frequency <= 230000000) {
+ ;
+ } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
+ c->frequency);
+ return -EINVAL;
+ }
+
+ switch (c->bandwidth_hz) {
+ case 6000000:
+ case 7000000:
+ case 8000000:
+ case 1700000:
+ case 5000000:
+ case 10000000:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
+ c->bandwidth_hz);
+ return -EINVAL;
+ }
+
+ /*
+ * Valid Physical Layer Pipe (PLP) values are 0 - 255
+ */
+ if (c->dvbt2_plp_id <= 255) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: dvbt2_plp_id=%d\n", __func__,
+ c->dvbt2_plp_id);
+ return -EINVAL;
+ }
+
+ switch (c->transmission_mode) {
+ case TRANSMISSION_MODE_AUTO:
+ case TRANSMISSION_MODE_2K:
+ case TRANSMISSION_MODE_8K:
+ case TRANSMISSION_MODE_1K:
+ case TRANSMISSION_MODE_4K:
+ case TRANSMISSION_MODE_16K:
+ case TRANSMISSION_MODE_32K:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
+ c->transmission_mode);
+ return -EINVAL;
+ }
+
+ switch (c->modulation) {
+ case QAM_AUTO:
+ case QPSK:
+ case QAM_16:
+ case QAM_64:
+ case QAM_256:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
+ c->modulation);
+ return -EINVAL;
+ }
+
+ switch (c->guard_interval) {
+ case GUARD_INTERVAL_AUTO:
+ case GUARD_INTERVAL_1_32:
+ case GUARD_INTERVAL_1_16:
+ case GUARD_INTERVAL_1_8:
+ case GUARD_INTERVAL_1_4:
+ case GUARD_INTERVAL_1_128:
+ case GUARD_INTERVAL_19_128:
+ case GUARD_INTERVAL_19_256:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
+ c->guard_interval);
+ return -EINVAL;
+ }
+
+ switch (c->fec_inner) {
+ case FEC_AUTO:
+ case FEC_1_2:
+ case FEC_3_5:
+ case FEC_2_3:
+ case FEC_3_4:
+ case FEC_4_5:
+ case FEC_5_6:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
+ c->fec_inner);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(dvb_validate_params_dvbt2);
+
int dvb_register_frontend(struct dvb_adapter* dvb,
struct dvb_frontend* fe)
{
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index 6df0c44..bcd572d 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -426,5 +426,6 @@ extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
+extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A parameter validation
2012-08-17 2:03 [PATCH 0/4] dvb_frontend: few DTV validation routines Antti Palosaari
2012-08-17 2:03 ` [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation Antti Palosaari
2012-08-17 2:03 ` [PATCH 2/4] dvb_frontend: add routine for DVB-T2 " Antti Palosaari
@ 2012-08-17 2:03 ` Antti Palosaari
2012-09-11 19:40 ` Mauro Carvalho Chehab
2012-08-17 2:03 ` [PATCH 4/4] dvb_frontend: add routine for DTMB " Antti Palosaari
3 siblings, 1 reply; 13+ messages in thread
From: Antti Palosaari @ 2012-08-17 2:03 UTC (permalink / raw)
To: linux-media; +Cc: Hin-Tak Leung, Antti Palosaari
Common routine for use of dvb-core, demodulator and tuner for check
given DVB-C annex A parameters correctness.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb-core/dvb_frontend.c | 54 +++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_frontend.h | 1 +
2 files changed, 55 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 6413c74..6a19c87 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2759,6 +2759,60 @@ int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
}
EXPORT_SYMBOL(dvb_validate_params_dvbt2);
+int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+
+ dev_dbg(fe->dvb->device, "%s:\n", __func__);
+
+ switch (c->delivery_system) {
+ case SYS_DVBC_ANNEX_A:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
+ c->delivery_system);
+ return -EINVAL;
+ }
+
+ /*
+ * TODO: NorDig Unified 2.2 specifies input frequency range
+ * 110 - 862 MHz. Do not limit it now as w_scan seems to start from
+ * 73 MHz until reason is clear.
+ */
+ if (c->frequency >= 0 && c->frequency <= 862000000) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
+ c->frequency);
+ return -EINVAL;
+ }
+
+ if (c->symbol_rate >= 1000000 && c->symbol_rate <= 7000000) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: symbol_rate=%d\n", __func__,
+ c->symbol_rate);
+ return -EINVAL;
+ }
+
+ switch (c->modulation) {
+ case QAM_AUTO:
+ case QAM_16:
+ case QAM_32:
+ case QAM_64:
+ case QAM_128:
+ case QAM_256:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
+ c->modulation);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
+
int dvb_register_frontend(struct dvb_adapter* dvb,
struct dvb_frontend* fe)
{
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index bcd572d..e6e6fe1 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -427,5 +427,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
+extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] dvb_frontend: add routine for DTMB parameter validation
2012-08-17 2:03 [PATCH 0/4] dvb_frontend: few DTV validation routines Antti Palosaari
` (2 preceding siblings ...)
2012-08-17 2:03 ` [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A " Antti Palosaari
@ 2012-08-17 2:03 ` Antti Palosaari
2012-09-11 19:43 ` Mauro Carvalho Chehab
3 siblings, 1 reply; 13+ messages in thread
From: Antti Palosaari @ 2012-08-17 2:03 UTC (permalink / raw)
To: linux-media; +Cc: Hin-Tak Leung, Antti Palosaari
Common routine for use of dvb-core, demodulator and tuner for check
given DTMB parameters correctness.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb-core/dvb_frontend.c | 97 +++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_frontend.h | 1 +
2 files changed, 98 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 6a19c87..7c3ba26 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2813,6 +2813,103 @@ int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
}
EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
+int dvb_validate_params_dtmb(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+
+ dev_dbg(fe->dvb->device, "%s:\n", __func__);
+
+ switch (c->delivery_system) {
+ case SYS_DTMB:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
+ c->delivery_system);
+ return -EINVAL;
+ }
+
+ if (c->frequency >= 470000000 && c->frequency <= 862000000) {
+ ;
+ } else {
+ dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
+ c->frequency);
+ return -EINVAL;
+ }
+
+ switch (c->bandwidth_hz) {
+ case 8000000:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
+ c->bandwidth_hz);
+ return -EINVAL;
+ }
+
+ switch (c->modulation) {
+ case QAM_AUTO:
+ case QPSK: /* QAM4 */
+ case QAM_16:
+ case QAM_32:
+ case QAM_64:
+ case QAM_4_NR:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
+ c->modulation);
+ return -EINVAL;
+ }
+
+ switch (c->transmission_mode) {
+ case TRANSMISSION_MODE_AUTO:
+ case TRANSMISSION_MODE_C1:
+ case TRANSMISSION_MODE_C3780:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
+ c->transmission_mode);
+ return -EINVAL;
+ }
+
+ switch (c->guard_interval) {
+ case GUARD_INTERVAL_AUTO:
+ case GUARD_INTERVAL_PN420:
+ case GUARD_INTERVAL_PN595:
+ case GUARD_INTERVAL_PN945:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
+ c->guard_interval);
+ return -EINVAL;
+ }
+
+ /* inner coding LDPC */
+ switch (c->fec_inner) {
+ case FEC_AUTO:
+ case FEC_2_5: /* 0.4 */
+ case FEC_3_5: /* 0.6 */
+ case FEC_4_5: /* 0.8 */
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
+ c->fec_inner);
+ return -EINVAL;
+ }
+
+ switch (c->interleaving) {
+ case INTERLEAVING_AUTO:
+ case INTERLEAVING_240:
+ case INTERLEAVING_720:
+ break;
+ default:
+ dev_dbg(fe->dvb->device, "%s: interleaving=%d\n", __func__,
+ c->interleaving);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(dvb_validate_params_dtmb);
+
int dvb_register_frontend(struct dvb_adapter* dvb,
struct dvb_frontend* fe)
{
diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
index e6e6fe1..9499039 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -428,5 +428,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
+extern int dvb_validate_params_dtmb(struct dvb_frontend *fe);
#endif
--
1.7.11.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation
2012-08-17 2:03 ` [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation Antti Palosaari
@ 2012-09-11 19:23 ` Mauro Carvalho Chehab
2012-09-15 23:42 ` Antti Palosaari
0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2012-09-11 19:23 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media, Hin-Tak Leung
Em 16-08-2012 23:03, Antti Palosaari escreveu:
> Common routine for use of dvb-core, demodulator and tuner for check
> given DVB-T parameters correctness.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/dvb-core/dvb_frontend.c | 136 ++++++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvb_frontend.h | 2 +
> 2 files changed, 138 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index d29d41a..4abb648 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2505,6 +2505,142 @@ int dvb_frontend_resume(struct dvb_frontend *fe)
> }
> EXPORT_SYMBOL(dvb_frontend_resume);
>
> +int dvb_validate_params_dvbt(struct dvb_frontend *fe)
> +{
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +
> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
> +
> + switch (c->delivery_system) {
> + case SYS_DVBT:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
> + c->delivery_system);
> + return -EINVAL;
> + }
Why do you need to check if the system is DVB-T here? The routine name
is only for DVB-T! It should either be a generic routine for all standards,
or not having such checks, otherwise, it will end by checking if the
delivery system is DVB-T on multiple places.
IMO, a dvb_validate_params() call that checks for all types is better.
Not sure if you have seen, but there is already something like that at
the dvb_frontend.c: dvb_frontend_check_parameters(). So, please let's not
reinvent the wheel.
> +
> + if (c->frequency >= 174000000 && c->frequency <= 230000000) {
> + ;
> + } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
> + c->frequency);
> + return -EINVAL;
> + }
Hmm... dvb_frontend_check_parameters() already checks for the min and max
frequencies, based on tuner/demod capabilities.
Also, I don't see any reason why the range between 230 MHz and 470 MHz should
explicitly excluded here. Are you sure that there aren't any Country somewhere
that might be using part of this range for TV? AFAIKT, most tuners will support
this range anyway, so enforcing drivers to not use them without any really good
reason doesn't make much sense.
> +
> + switch (c->bandwidth_hz) {
> + case 6000000:
> + case 7000000:
> + case 8000000:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
> + c->bandwidth_hz);
> + return -EINVAL;
> + }
Hmm... 0 is a valid value (it means AUTO, as documented at the DVB API spec).
Also, 5 MHz is also a valid value for DVB-T (see Annex G of EN 300.744 v 1.6.1).
I don't doubt you'll find some places with 5MHz on DVB-T outside Europe.
> +
> + switch (c->transmission_mode) {
> + case TRANSMISSION_MODE_AUTO:
> + case TRANSMISSION_MODE_2K:
> + case TRANSMISSION_MODE_8K:
> + break;
DVB-T specs also allow 4K for 5 MHz bandwidth.
> + default:
> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
> + c->transmission_mode);
> + return -EINVAL;
> + }
> +
> + switch (c->modulation) {
> + case QAM_AUTO:
> + case QPSK:
> + case QAM_16:
> + case QAM_64:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
> + c->modulation);
> + return -EINVAL;
> + }
> +
> + switch (c->guard_interval) {
> + case GUARD_INTERVAL_AUTO:
> + case GUARD_INTERVAL_1_32:
> + case GUARD_INTERVAL_1_16:
> + case GUARD_INTERVAL_1_8:
> + case GUARD_INTERVAL_1_4:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
> + c->guard_interval);
> + return -EINVAL;
> + }
> +
> + switch (c->hierarchy) {
> + case HIERARCHY_NONE:
> + case HIERARCHY_AUTO:
> + case HIERARCHY_1:
> + case HIERARCHY_2:
> + case HIERARCHY_4:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: hierarchy=%d\n", __func__,
> + c->hierarchy);
> + return -EINVAL;
> + }
> +
> + switch (c->code_rate_HP) {
> + case FEC_AUTO:
> + case FEC_1_2:
> + case FEC_2_3:
> + case FEC_3_4:
> + case FEC_5_6:
> + case FEC_7_8:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: code_rate_HP=%d\n", __func__,
> + c->code_rate_HP);
> + return -EINVAL;
> + }
> +
> + /*
> + * code_rate_LP is used only with hierarchical coding
> + */
> + if (c->hierarchy == HIERARCHY_NONE) {
> + switch (c->code_rate_LP) {
> + case FEC_NONE:
> + /*
> + * TODO: FEC_AUTO here is wrong, but for some reason
> + * dtv_set_frontend() forces it.
> + */
> + case FEC_AUTO:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
> + __func__, c->code_rate_LP);
> + return -EINVAL;
> + }
> + } else {
> + switch (c->code_rate_LP) {
> + case FEC_AUTO:
> + case FEC_1_2:
> + case FEC_2_3:
> + case FEC_3_4:
> + case FEC_5_6:
> + case FEC_7_8:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
> + __func__, c->code_rate_LP);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dvb_validate_params_dvbt);
Please use EXPORT_SYMBOL_GPL().
> +
> int dvb_register_frontend(struct dvb_adapter* dvb,
> struct dvb_frontend* fe)
> {
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index 74ba563..6df0c44 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -425,4 +425,6 @@ extern int dvb_frontend_resume(struct dvb_frontend *fe);
> extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
> extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>
> +extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
> +
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] dvb_frontend: add routine for DVB-T2 parameter validation
2012-08-17 2:03 ` [PATCH 2/4] dvb_frontend: add routine for DVB-T2 " Antti Palosaari
@ 2012-09-11 19:33 ` Mauro Carvalho Chehab
2012-09-16 0:05 ` Antti Palosaari
0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2012-09-11 19:33 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media, Hin-Tak Leung
Em 16-08-2012 23:03, Antti Palosaari escreveu:
> Common routine for use of dvb-core, demodulator and tuner for check
> given DVB-T2 parameters correctness.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/dvb-core/dvb_frontend.c | 118 ++++++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvb_frontend.h | 1 +
> 2 files changed, 119 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 4abb648..6413c74 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2641,6 +2641,124 @@ int dvb_validate_params_dvbt(struct dvb_frontend *fe)
> }
> EXPORT_SYMBOL(dvb_validate_params_dvbt);
>
> +int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
> +{
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +
> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
> +
> + switch (c->delivery_system) {
> + case SYS_DVBT2:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
> + c->delivery_system);
> + return -EINVAL;
> + }
Same comments made on patch 1/4 apply here.
> +
> + /*
> + * DVB-T2 specification as such does not specify any frequency bands.
> + * Define real life limits still. L-Band 1452 - 1492 MHz may exits in
> + * future too.
> + */
> + if (c->frequency >= 174000000 && c->frequency <= 230000000) {
> + ;
> + } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
> + c->frequency);
> + return -EINVAL;
> + }
Same comments made on patch 1/4 apply here.
> +
> + switch (c->bandwidth_hz) {
> + case 6000000:
> + case 7000000:
> + case 8000000:
> + case 1700000:
> + case 5000000:
> + case 10000000:
> + break;
0 is also valid. Also, better to sort the entries here.
> + default:
> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
> + c->bandwidth_hz);
> + return -EINVAL;
> + }
> +
> + /*
> + * Valid Physical Layer Pipe (PLP) values are 0 - 255
> + */
> + if (c->dvbt2_plp_id <= 255) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: dvbt2_plp_id=%d\n", __func__,
> + c->dvbt2_plp_id);
> + return -EINVAL;
> + }
Is it possible to disable it for DVB-T2? If so, a new value
is needed here (~0), according with our discussions related to
multistream patches.
> +
> + switch (c->transmission_mode) {
> + case TRANSMISSION_MODE_AUTO:
> + case TRANSMISSION_MODE_2K:
> + case TRANSMISSION_MODE_8K:
> + case TRANSMISSION_MODE_1K:
> + case TRANSMISSION_MODE_4K:
> + case TRANSMISSION_MODE_16K:
> + case TRANSMISSION_MODE_32K:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
> + c->transmission_mode);
> + return -EINVAL;
> + }
> +
> + switch (c->modulation) {
> + case QAM_AUTO:
> + case QPSK:
> + case QAM_16:
> + case QAM_64:
> + case QAM_256:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
> + c->modulation);
> + return -EINVAL;
> + }
> +
> + switch (c->guard_interval) {
> + case GUARD_INTERVAL_AUTO:
> + case GUARD_INTERVAL_1_32:
> + case GUARD_INTERVAL_1_16:
> + case GUARD_INTERVAL_1_8:
> + case GUARD_INTERVAL_1_4:
> + case GUARD_INTERVAL_1_128:
> + case GUARD_INTERVAL_19_128:
> + case GUARD_INTERVAL_19_256:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
> + c->guard_interval);
> + return -EINVAL;
> + }
> +
> + switch (c->fec_inner) {
> + case FEC_AUTO:
> + case FEC_1_2:
> + case FEC_3_5:
> + case FEC_2_3:
> + case FEC_3_4:
> + case FEC_4_5:
> + case FEC_5_6:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
> + c->fec_inner);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dvb_validate_params_dvbt2);
Same comments made on patch 1/4 apply here.
> +
> int dvb_register_frontend(struct dvb_adapter* dvb,
> struct dvb_frontend* fe)
> {
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index 6df0c44..bcd572d 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -426,5 +426,6 @@ extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
> extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>
> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
> +extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
>
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A parameter validation
2012-08-17 2:03 ` [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A " Antti Palosaari
@ 2012-09-11 19:40 ` Mauro Carvalho Chehab
2012-09-16 0:14 ` Antti Palosaari
0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2012-09-11 19:40 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media, Hin-Tak Leung
Em 16-08-2012 23:03, Antti Palosaari escreveu:
> Common routine for use of dvb-core, demodulator and tuner for check
> given DVB-C annex A parameters correctness.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
I won't repeat myself on the stuff I commented on patch 1/4.
> ---
> drivers/media/dvb-core/dvb_frontend.c | 54 +++++++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvb_frontend.h | 1 +
> 2 files changed, 55 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 6413c74..6a19c87 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2759,6 +2759,60 @@ int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
> }
> EXPORT_SYMBOL(dvb_validate_params_dvbt2);
>
> +int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
> +{
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +
> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
> +
> + switch (c->delivery_system) {
> + case SYS_DVBC_ANNEX_A:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
> + c->delivery_system);
> + return -EINVAL;
> + }
> +
> + /*
> + * TODO: NorDig Unified 2.2 specifies input frequency range
> + * 110 - 862 MHz. Do not limit it now as w_scan seems to start from
> + * 73 MHz until reason is clear.
> + */
> + if (c->frequency >= 0 && c->frequency <= 862000000) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
> + c->frequency);
> + return -EINVAL;
> + }
> +
> + if (c->symbol_rate >= 1000000 && c->symbol_rate <= 7000000) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: symbol_rate=%d\n", __func__,
> + c->symbol_rate);
> + return -EINVAL;
> + }
Hmm... Not sure if it is a good idea to limit the symbol rate for DVB-C
(especially the upper range), as some cable operators could be doing weird
things. This should be hardware-dependent, instead. Btw, the DVB core already
check those limits:
case SYS_DVBC_ANNEX_A:
case SYS_DVBC_ANNEX_C:
if ((fe->ops.info.symbol_rate_min &&
c->symbol_rate < fe->ops.info.symbol_rate_min) ||
(fe->ops.info.symbol_rate_max &&
c->symbol_rate > fe->ops.info.symbol_rate_max)) {
dev_warn(fe->dvb->device, "DVB: adapter %i frontend %i symbol rate %u out of range (%u..%u)\n",
fe->dvb->num, fe->id, c->symbol_rate,
fe->ops.info.symbol_rate_min,
fe->ops.info.symbol_rate_max);
return -EINVAL;
}
Btw, DVB-C annex A and C are very similar. From hardware PoV, the only difference
I'm aware of is at the saw filter. So, the same checks can be applied for
both types. Well, it can be a little more rigid for modulation, on Annex C,
but we need some support for someone at Japan, in order to be sure that
they're using just one type of QAM there.
> +
> + switch (c->modulation) {
> + case QAM_AUTO:
> + case QAM_16:
> + case QAM_32:
> + case QAM_64:
> + case QAM_128:
> + case QAM_256:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
> + c->modulation);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
> +
> int dvb_register_frontend(struct dvb_adapter* dvb,
> struct dvb_frontend* fe)
> {
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index bcd572d..e6e6fe1 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -427,5 +427,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>
> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
> extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
> +extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
>
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] dvb_frontend: add routine for DTMB parameter validation
2012-08-17 2:03 ` [PATCH 4/4] dvb_frontend: add routine for DTMB " Antti Palosaari
@ 2012-09-11 19:43 ` Mauro Carvalho Chehab
2012-09-16 0:27 ` Antti Palosaari
0 siblings, 1 reply; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2012-09-11 19:43 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media, Hin-Tak Leung
Em 16-08-2012 23:03, Antti Palosaari escreveu:
> Common routine for use of dvb-core, demodulator and tuner for check
> given DTMB parameters correctness.
I won't repeat myself on the stuff I commented on patch 1/4.
I dunno much about this standard, nor I have the specs, so, I'm
assuming that you did the right checks here.
In any case, as there's just one driver for this standard that doesn't work
on "AUTO" mode, the only driver that could break here is your driver.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/dvb-core/dvb_frontend.c | 97 +++++++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvb_frontend.h | 1 +
> 2 files changed, 98 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index 6a19c87..7c3ba26 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2813,6 +2813,103 @@ int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
> }
> EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
>
> +int dvb_validate_params_dtmb(struct dvb_frontend *fe)
> +{
> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +
> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
> +
> + switch (c->delivery_system) {
> + case SYS_DTMB:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
> + c->delivery_system);
> + return -EINVAL;
> + }
> +
> + if (c->frequency >= 470000000 && c->frequency <= 862000000) {
> + ;
> + } else {
> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
> + c->frequency);
> + return -EINVAL;
> + }
> +
> + switch (c->bandwidth_hz) {
> + case 8000000:
> + break;
Again, 0 should be accepted, as it means AUTO.
> + default:
> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
> + c->bandwidth_hz);
> + return -EINVAL;
> + }
> +
> + switch (c->modulation) {
> + case QAM_AUTO:
> + case QPSK: /* QAM4 */
> + case QAM_16:
> + case QAM_32:
> + case QAM_64:
> + case QAM_4_NR:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
> + c->modulation);
> + return -EINVAL;
> + }
> +
> + switch (c->transmission_mode) {
> + case TRANSMISSION_MODE_AUTO:
> + case TRANSMISSION_MODE_C1:
> + case TRANSMISSION_MODE_C3780:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
> + c->transmission_mode);
> + return -EINVAL;
> + }
> +
> + switch (c->guard_interval) {
> + case GUARD_INTERVAL_AUTO:
> + case GUARD_INTERVAL_PN420:
> + case GUARD_INTERVAL_PN595:
> + case GUARD_INTERVAL_PN945:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
> + c->guard_interval);
> + return -EINVAL;
> + }
> +
> + /* inner coding LDPC */
> + switch (c->fec_inner) {
> + case FEC_AUTO:
> + case FEC_2_5: /* 0.4 */
> + case FEC_3_5: /* 0.6 */
> + case FEC_4_5: /* 0.8 */
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
> + c->fec_inner);
> + return -EINVAL;
> + }
> +
> + switch (c->interleaving) {
> + case INTERLEAVING_AUTO:
> + case INTERLEAVING_240:
> + case INTERLEAVING_720:
> + break;
> + default:
> + dev_dbg(fe->dvb->device, "%s: interleaving=%d\n", __func__,
> + c->interleaving);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dvb_validate_params_dtmb);
> +
> int dvb_register_frontend(struct dvb_adapter* dvb,
> struct dvb_frontend* fe)
> {
> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
> index e6e6fe1..9499039 100644
> --- a/drivers/media/dvb-core/dvb_frontend.h
> +++ b/drivers/media/dvb-core/dvb_frontend.h
> @@ -428,5 +428,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
> extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
> extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
> +extern int dvb_validate_params_dtmb(struct dvb_frontend *fe);
>
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation
2012-09-11 19:23 ` Mauro Carvalho Chehab
@ 2012-09-15 23:42 ` Antti Palosaari
0 siblings, 0 replies; 13+ messages in thread
From: Antti Palosaari @ 2012-09-15 23:42 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Hin-Tak Leung
On 09/11/2012 10:23 PM, Mauro Carvalho Chehab wrote:
> Em 16-08-2012 23:03, Antti Palosaari escreveu:
>> Common routine for use of dvb-core, demodulator and tuner for check
>> given DVB-T parameters correctness.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> drivers/media/dvb-core/dvb_frontend.c | 136 ++++++++++++++++++++++++++++++++++
>> drivers/media/dvb-core/dvb_frontend.h | 2 +
>> 2 files changed, 138 insertions(+)
>>
>> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
>> index d29d41a..4abb648 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb-core/dvb_frontend.c
>> @@ -2505,6 +2505,142 @@ int dvb_frontend_resume(struct dvb_frontend *fe)
>> }
>> EXPORT_SYMBOL(dvb_frontend_resume);
>>
>> +int dvb_validate_params_dvbt(struct dvb_frontend *fe)
>> +{
>> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +
>> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
>> +
>> + switch (c->delivery_system) {
>> + case SYS_DVBT:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
>> + c->delivery_system);
>> + return -EINVAL;
>> + }
>
> Why do you need to check if the system is DVB-T here? The routine name
> is only for DVB-T! It should either be a generic routine for all standards,
> or not having such checks, otherwise, it will end by checking if the
> delivery system is DVB-T on multiple places.
It is currently checked somewhere already. Idea was to provide single
function which does all the checks as well as it could do. If frontend
is defined to support SYS_DVBT you still must check somewhere call from
the userspace is really for delivery_system == SYS_DVBT. So, you look
first from frontend ops supported delivery system, then call that
delivery system to ensure one got from userspace request is really that.
if (frontend.ops.delsys == SYS_DVBT)
dvb_validate_params_dvbt()
if (frontend.ops.delsys == SYS_DVBC)
dvb_validate_params_dvbc()
> IMO, a dvb_validate_params() call that checks for all types is better.
Surely it is and it was the plan too. But could you guess how much work
it is to make good checks for all supported delivery systems used. Even
finding out all possible values for each parameter is quite a lot of
reading standards not to mention checks for invalid parameter combinations.
> Not sure if you have seen, but there is already something like that at
> the dvb_frontend.c: dvb_frontend_check_parameters(). So, please let's not
> reinvent the wheel.
Those check are quite broken currently by design as new chips supports
multiple standards. This changed after MFE support. One simple example,
frontend supports both DVB-T and DVB-C. How could you checks frequency
limit using current approach? Not possible.
Due to that I see new way based of television standards are needed. When
all these checks are done by frontend in some day in future it is
possible to remove those old and broken values from tuner/demod ops.
>> +
>> + if (c->frequency >= 174000000 && c->frequency <= 230000000) {
>> + ;
>> + } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
>> + c->frequency);
>> + return -EINVAL;
>> + }
>
> Hmm... dvb_frontend_check_parameters() already checks for the min and max
> frequencies, based on tuner/demod capabilities.
Answered that just earlier.
> Also, I don't see any reason why the range between 230 MHz and 470 MHz should
> explicitly excluded here. Are you sure that there aren't any Country somewhere
> that might be using part of this range for TV? AFAIKT, most tuners will support
> this range anyway, so enforcing drivers to not use them without any really good
> reason doesn't make much sense.
There is no other reason than it is not used. Demodulator operates
baseband signal and thus no any other limit for RF than those coming
from baseband signal characteristics. Hardware limits are coming from
the RF tuner.
There is mainly two possibilities.
1) make general checks based of used standards
2) leave checking for each driver (allowing all the combinations
hardware could do regardless if it is required by specification or not)
>> + switch (c->bandwidth_hz) {
>> + case 6000000:
>> + case 7000000:
>> + case 8000000:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
>> + c->bandwidth_hz);
>> + return -EINVAL;
>> + }
>
> Hmm... 0 is a valid value (it means AUTO, as documented at the DVB API spec).
ok, sounds crazy but there could be still hardware that detects it
automatically for OFDM modulated signal. I wonder how tuner filters
(RF/IF) are configured in that case...
> Also, 5 MHz is also a valid value for DVB-T (see Annex G of EN 300.744 v 1.6.1).
> I don't doubt you'll find some places with 5MHz on DVB-T outside Europe.
Yes, it is latest annex (DVB-T Annex G). Also 4k FFT mode from DVB-H is
derived.
>> +
>> + switch (c->transmission_mode) {
>> + case TRANSMISSION_MODE_AUTO:
>> + case TRANSMISSION_MODE_2K:
>> + case TRANSMISSION_MODE_8K:
>> + break;
>
> DVB-T specs also allow 4K for 5 MHz bandwidth.
Seems to allow nowadays.
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
>> + c->transmission_mode);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->modulation) {
>> + case QAM_AUTO:
>> + case QPSK:
>> + case QAM_16:
>> + case QAM_64:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
>> + c->modulation);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->guard_interval) {
>> + case GUARD_INTERVAL_AUTO:
>> + case GUARD_INTERVAL_1_32:
>> + case GUARD_INTERVAL_1_16:
>> + case GUARD_INTERVAL_1_8:
>> + case GUARD_INTERVAL_1_4:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
>> + c->guard_interval);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->hierarchy) {
>> + case HIERARCHY_NONE:
>> + case HIERARCHY_AUTO:
>> + case HIERARCHY_1:
>> + case HIERARCHY_2:
>> + case HIERARCHY_4:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: hierarchy=%d\n", __func__,
>> + c->hierarchy);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->code_rate_HP) {
>> + case FEC_AUTO:
>> + case FEC_1_2:
>> + case FEC_2_3:
>> + case FEC_3_4:
>> + case FEC_5_6:
>> + case FEC_7_8:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: code_rate_HP=%d\n", __func__,
>> + c->code_rate_HP);
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * code_rate_LP is used only with hierarchical coding
>> + */
>> + if (c->hierarchy == HIERARCHY_NONE) {
>> + switch (c->code_rate_LP) {
>> + case FEC_NONE:
>> + /*
>> + * TODO: FEC_AUTO here is wrong, but for some reason
>> + * dtv_set_frontend() forces it.
>> + */
>> + case FEC_AUTO:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
>> + __func__, c->code_rate_LP);
>> + return -EINVAL;
>> + }
>> + } else {
>> + switch (c->code_rate_LP) {
>> + case FEC_AUTO:
>> + case FEC_1_2:
>> + case FEC_2_3:
>> + case FEC_3_4:
>> + case FEC_5_6:
>> + case FEC_7_8:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: code_rate_LP=%d\n",
>> + __func__, c->code_rate_LP);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(dvb_validate_params_dvbt);
>
> Please use EXPORT_SYMBOL_GPL().
>
>> +
>> int dvb_register_frontend(struct dvb_adapter* dvb,
>> struct dvb_frontend* fe)
>> {
>> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
>> index 74ba563..6df0c44 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.h
>> +++ b/drivers/media/dvb-core/dvb_frontend.h
>> @@ -425,4 +425,6 @@ extern int dvb_frontend_resume(struct dvb_frontend *fe);
>> extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
>> extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>>
>> +extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
>> +
>> #endif
>>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] dvb_frontend: add routine for DVB-T2 parameter validation
2012-09-11 19:33 ` Mauro Carvalho Chehab
@ 2012-09-16 0:05 ` Antti Palosaari
0 siblings, 0 replies; 13+ messages in thread
From: Antti Palosaari @ 2012-09-16 0:05 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Hin-Tak Leung
On 09/11/2012 10:33 PM, Mauro Carvalho Chehab wrote:
> Em 16-08-2012 23:03, Antti Palosaari escreveu:
>> Common routine for use of dvb-core, demodulator and tuner for check
>> given DVB-T2 parameters correctness.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> drivers/media/dvb-core/dvb_frontend.c | 118 ++++++++++++++++++++++++++++++++++
>> drivers/media/dvb-core/dvb_frontend.h | 1 +
>> 2 files changed, 119 insertions(+)
>>
>> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
>> index 4abb648..6413c74 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb-core/dvb_frontend.c
>> @@ -2641,6 +2641,124 @@ int dvb_validate_params_dvbt(struct dvb_frontend *fe)
>> }
>> EXPORT_SYMBOL(dvb_validate_params_dvbt);
>>
>> +int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
>> +{
>> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +
>> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
>> +
>> + switch (c->delivery_system) {
>> + case SYS_DVBT2:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
>> + c->delivery_system);
>> + return -EINVAL;
>> + }
>
> Same comments made on patch 1/4 apply here.
>
>> +
>> + /*
>> + * DVB-T2 specification as such does not specify any frequency bands.
>> + * Define real life limits still. L-Band 1452 - 1492 MHz may exits in
>> + * future too.
>> + */
>> + if (c->frequency >= 174000000 && c->frequency <= 230000000) {
>> + ;
>> + } else if (c->frequency >= 470000000 && c->frequency <= 862000000) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
>> + c->frequency);
>> + return -EINVAL;
>> + }
>
> Same comments made on patch 1/4 apply here.
Maybe it is then better to move these limits totally for the
responsibility of RF-tuner driver and use limits tuner really can do.
Personally I still like more to see real limits... But those limits are
still changing over the time when ITU radio conference makes new
allocations.
For example here in Finland upper channels from UHF DVB-T band are
already taken for LTE, one year ago or so. IIRC it is not officially yet
allocated for LTE as thereis transition period ongoing worldwide. But we
need so badly fast wireless internet connections for rural areas that it
was taken in use early, and even not used near Russian border as they
are still using those channels for TV.
>> + switch (c->bandwidth_hz) {
>> + case 6000000:
>> + case 7000000:
>> + case 8000000:
>> + case 1700000:
>> + case 5000000:
>> + case 10000000:
>> + break;
>
> 0 is also valid. Also, better to sort the entries here.
>
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
>> + c->bandwidth_hz);
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Valid Physical Layer Pipe (PLP) values are 0 - 255
>> + */
>> + if (c->dvbt2_plp_id <= 255) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: dvbt2_plp_id=%d\n", __func__,
>> + c->dvbt2_plp_id);
>> + return -EINVAL;
>> + }
>
> Is it possible to disable it for DVB-T2? If so, a new value
> is needed here (~0), according with our discussions related to
> multistream patches.
Have to check, but I think so.
IIRC there was also some other errors in current DVB-T2 API. Smallest
nominal bw was defined wrong and also hierarchy. Hierarchy is used for
DVB-T, for DVB-T2 it is replaced with PLP.
>> + switch (c->transmission_mode) {
>> + case TRANSMISSION_MODE_AUTO:
>> + case TRANSMISSION_MODE_2K:
>> + case TRANSMISSION_MODE_8K:
>> + case TRANSMISSION_MODE_1K:
>> + case TRANSMISSION_MODE_4K:
>> + case TRANSMISSION_MODE_16K:
>> + case TRANSMISSION_MODE_32K:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
>> + c->transmission_mode);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->modulation) {
>> + case QAM_AUTO:
>> + case QPSK:
>> + case QAM_16:
>> + case QAM_64:
>> + case QAM_256:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
>> + c->modulation);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->guard_interval) {
>> + case GUARD_INTERVAL_AUTO:
>> + case GUARD_INTERVAL_1_32:
>> + case GUARD_INTERVAL_1_16:
>> + case GUARD_INTERVAL_1_8:
>> + case GUARD_INTERVAL_1_4:
>> + case GUARD_INTERVAL_1_128:
>> + case GUARD_INTERVAL_19_128:
>> + case GUARD_INTERVAL_19_256:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
>> + c->guard_interval);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->fec_inner) {
>> + case FEC_AUTO:
>> + case FEC_1_2:
>> + case FEC_3_5:
>> + case FEC_2_3:
>> + case FEC_3_4:
>> + case FEC_4_5:
>> + case FEC_5_6:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
>> + c->fec_inner);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(dvb_validate_params_dvbt2);
>
> Same comments made on patch 1/4 apply here.
>
>> +
>> int dvb_register_frontend(struct dvb_adapter* dvb,
>> struct dvb_frontend* fe)
>> {
>> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
>> index 6df0c44..bcd572d 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.h
>> +++ b/drivers/media/dvb-core/dvb_frontend.h
>> @@ -426,5 +426,6 @@ extern void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec);
>> extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>>
>> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
>> +extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
>>
>> #endif
>>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A parameter validation
2012-09-11 19:40 ` Mauro Carvalho Chehab
@ 2012-09-16 0:14 ` Antti Palosaari
0 siblings, 0 replies; 13+ messages in thread
From: Antti Palosaari @ 2012-09-16 0:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Hin-Tak Leung
On 09/11/2012 10:40 PM, Mauro Carvalho Chehab wrote:
> Em 16-08-2012 23:03, Antti Palosaari escreveu:
>> Common routine for use of dvb-core, demodulator and tuner for check
>> given DVB-C annex A parameters correctness.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>
> I won't repeat myself on the stuff I commented on patch 1/4.
>
>> ---
>> drivers/media/dvb-core/dvb_frontend.c | 54 +++++++++++++++++++++++++++++++++++
>> drivers/media/dvb-core/dvb_frontend.h | 1 +
>> 2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
>> index 6413c74..6a19c87 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb-core/dvb_frontend.c
>> @@ -2759,6 +2759,60 @@ int dvb_validate_params_dvbt2(struct dvb_frontend *fe)
>> }
>> EXPORT_SYMBOL(dvb_validate_params_dvbt2);
>>
>> +int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
>> +{
>> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +
>> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
>> +
>> + switch (c->delivery_system) {
>> + case SYS_DVBC_ANNEX_A:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
>> + c->delivery_system);
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * TODO: NorDig Unified 2.2 specifies input frequency range
>> + * 110 - 862 MHz. Do not limit it now as w_scan seems to start from
>> + * 73 MHz until reason is clear.
>> + */
>> + if (c->frequency >= 0 && c->frequency <= 862000000) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
>> + c->frequency);
>> + return -EINVAL;
>> + }
>> +
>> + if (c->symbol_rate >= 1000000 && c->symbol_rate <= 7000000) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: symbol_rate=%d\n", __func__,
>> + c->symbol_rate);
>> + return -EINVAL;
>> + }
>
> Hmm... Not sure if it is a good idea to limit the symbol rate for DVB-C
> (especially the upper range), as some cable operators could be doing weird
> things. This should be hardware-dependent, instead. Btw, the DVB core already
> check those limits:
>
> case SYS_DVBC_ANNEX_A:
> case SYS_DVBC_ANNEX_C:
> if ((fe->ops.info.symbol_rate_min &&
> c->symbol_rate < fe->ops.info.symbol_rate_min) ||
> (fe->ops.info.symbol_rate_max &&
> c->symbol_rate > fe->ops.info.symbol_rate_max)) {
> dev_warn(fe->dvb->device, "DVB: adapter %i frontend %i symbol rate %u out of range (%u..%u)\n",
> fe->dvb->num, fe->id, c->symbol_rate,
> fe->ops.info.symbol_rate_min,
> fe->ops.info.symbol_rate_max);
> return -EINVAL;
> }
>
The aim for these checks were to check correctness very general level. I
found symbol rate to be very challenging thus so relaxed checks. Nothing
prevents driver to do better checks. Maximum symbol rate used here is
more than theoretical maximum for nominal 8MHz radio channel used. But
again, like for DVB-T corner cases, someone could use wider radio
channel in lab :) In real life I quite sure 8 MHz is maximum used.
In real life there is still quite only few possible values used.
> Btw, DVB-C annex A and C are very similar. From hardware PoV, the only difference
> I'm aware of is at the saw filter. So, the same checks can be applied for
> both types. Well, it can be a little more rigid for modulation, on Annex C,
> but we need some support for someone at Japan, in order to be sure that
> they're using just one type of QAM there.
>
>> +
>> + switch (c->modulation) {
>> + case QAM_AUTO:
>> + case QAM_16:
>> + case QAM_32:
>> + case QAM_64:
>> + case QAM_128:
>> + case QAM_256:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
>> + c->modulation);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
>> +
>> int dvb_register_frontend(struct dvb_adapter* dvb,
>> struct dvb_frontend* fe)
>> {
>> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
>> index bcd572d..e6e6fe1 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.h
>> +++ b/drivers/media/dvb-core/dvb_frontend.h
>> @@ -427,5 +427,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>>
>> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
>> extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
>> +extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
>>
>> #endif
>>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] dvb_frontend: add routine for DTMB parameter validation
2012-09-11 19:43 ` Mauro Carvalho Chehab
@ 2012-09-16 0:27 ` Antti Palosaari
0 siblings, 0 replies; 13+ messages in thread
From: Antti Palosaari @ 2012-09-16 0:27 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, Hin-Tak Leung
On 09/11/2012 10:43 PM, Mauro Carvalho Chehab wrote:
> Em 16-08-2012 23:03, Antti Palosaari escreveu:
>> Common routine for use of dvb-core, demodulator and tuner for check
>> given DTMB parameters correctness.
>
> I won't repeat myself on the stuff I commented on patch 1/4.
>
> I dunno much about this standard, nor I have the specs, so, I'm
> assuming that you did the right checks here.
I am not 100% sure for bandwidth. When I did that driver I never got it
working other than 8 MHz (used modulator allowed to set 5, 6, 7, 8).
There is mentioned also 6 and 7 MHz in many places over the Net. 8 MHz
is still surely the only real one and also I suspect it is the only one
specified too. Like any other terrestrial modulation, hardware still
could support more freely selectable configuration.
> In any case, as there's just one driver for this standard that doesn't work
> on "AUTO" mode, the only driver that could break here is your driver.
hd29l2 driver you mean is currently forced to AUTO mode and is abusing
API DVB-T delivery system.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> drivers/media/dvb-core/dvb_frontend.c | 97 +++++++++++++++++++++++++++++++++++
>> drivers/media/dvb-core/dvb_frontend.h | 1 +
>> 2 files changed, 98 insertions(+)
>>
>> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
>> index 6a19c87..7c3ba26 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb-core/dvb_frontend.c
>> @@ -2813,6 +2813,103 @@ int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe)
>> }
>> EXPORT_SYMBOL(dvb_validate_params_dvbc_annex_a);
>>
>> +int dvb_validate_params_dtmb(struct dvb_frontend *fe)
>> +{
>> + struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +
>> + dev_dbg(fe->dvb->device, "%s:\n", __func__);
>> +
>> + switch (c->delivery_system) {
>> + case SYS_DTMB:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: delivery_system=%d\n", __func__,
>> + c->delivery_system);
>> + return -EINVAL;
>> + }
>> +
>> + if (c->frequency >= 470000000 && c->frequency <= 862000000) {
>> + ;
>> + } else {
>> + dev_dbg(fe->dvb->device, "%s: frequency=%d\n", __func__,
>> + c->frequency);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->bandwidth_hz) {
>> + case 8000000:
>> + break;
>
> Again, 0 should be accepted, as it means AUTO.
ok
>
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: bandwidth_hz=%d\n", __func__,
>> + c->bandwidth_hz);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->modulation) {
>> + case QAM_AUTO:
>> + case QPSK: /* QAM4 */
>> + case QAM_16:
>> + case QAM_32:
>> + case QAM_64:
>> + case QAM_4_NR:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: modulation=%d\n", __func__,
>> + c->modulation);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->transmission_mode) {
>> + case TRANSMISSION_MODE_AUTO:
>> + case TRANSMISSION_MODE_C1:
>> + case TRANSMISSION_MODE_C3780:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: transmission_mode=%d\n", __func__,
>> + c->transmission_mode);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->guard_interval) {
>> + case GUARD_INTERVAL_AUTO:
>> + case GUARD_INTERVAL_PN420:
>> + case GUARD_INTERVAL_PN595:
>> + case GUARD_INTERVAL_PN945:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: guard_interval=%d\n", __func__,
>> + c->guard_interval);
>> + return -EINVAL;
>> + }
>> +
>> + /* inner coding LDPC */
>> + switch (c->fec_inner) {
>> + case FEC_AUTO:
>> + case FEC_2_5: /* 0.4 */
>> + case FEC_3_5: /* 0.6 */
>> + case FEC_4_5: /* 0.8 */
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: fec_inner=%d\n", __func__,
>> + c->fec_inner);
>> + return -EINVAL;
>> + }
>> +
>> + switch (c->interleaving) {
>> + case INTERLEAVING_AUTO:
>> + case INTERLEAVING_240:
>> + case INTERLEAVING_720:
>> + break;
>> + default:
>> + dev_dbg(fe->dvb->device, "%s: interleaving=%d\n", __func__,
>> + c->interleaving);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(dvb_validate_params_dtmb);
>> +
>> int dvb_register_frontend(struct dvb_adapter* dvb,
>> struct dvb_frontend* fe)
>> {
>> diff --git a/drivers/media/dvb-core/dvb_frontend.h b/drivers/media/dvb-core/dvb_frontend.h
>> index e6e6fe1..9499039 100644
>> --- a/drivers/media/dvb-core/dvb_frontend.h
>> +++ b/drivers/media/dvb-core/dvb_frontend.h
>> @@ -428,5 +428,6 @@ extern s32 timeval_usec_diff(struct timeval lasttime, struct timeval curtime);
>> extern int dvb_validate_params_dvbt(struct dvb_frontend *fe);
>> extern int dvb_validate_params_dvbt2(struct dvb_frontend *fe);
>> extern int dvb_validate_params_dvbc_annex_a(struct dvb_frontend *fe);
>> +extern int dvb_validate_params_dtmb(struct dvb_frontend *fe);
>>
>> #endif
>>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-09-16 0:27 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-17 2:03 [PATCH 0/4] dvb_frontend: few DTV validation routines Antti Palosaari
2012-08-17 2:03 ` [PATCH 1/4] dvb_frontend: add routine for DVB-T parameter validation Antti Palosaari
2012-09-11 19:23 ` Mauro Carvalho Chehab
2012-09-15 23:42 ` Antti Palosaari
2012-08-17 2:03 ` [PATCH 2/4] dvb_frontend: add routine for DVB-T2 " Antti Palosaari
2012-09-11 19:33 ` Mauro Carvalho Chehab
2012-09-16 0:05 ` Antti Palosaari
2012-08-17 2:03 ` [PATCH 3/4] dvb_frontend: add routine for DVB-C annex A " Antti Palosaari
2012-09-11 19:40 ` Mauro Carvalho Chehab
2012-09-16 0:14 ` Antti Palosaari
2012-08-17 2:03 ` [PATCH 4/4] dvb_frontend: add routine for DTMB " Antti Palosaari
2012-09-11 19:43 ` Mauro Carvalho Chehab
2012-09-16 0:27 ` Antti Palosaari
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).