public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] dvb_frontend cleanup, S2API regression fix
@ 2011-05-08 23:03 Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend Andreas Oberritter
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

Here are some patches to clean up some dvb_frontend code
and to allow reading back detected tuning parameters
through S2API.

The patches have only been compile-tested for now. Feedback
would be appreciated, to minimize the risk of new regressions.

They apply on top of two of the DVB-T2 patches previously
submitted:

https://patchwork.kernel.org/patch/766442/
https://patchwork.kernel.org/patch/766822/

Andreas Oberritter (8):
  DVB: return meaningful error codes in dvb_frontend
  DVB: dtv_property_cache_submit shouldn't modifiy the cache
  DVB: call get_property at the end of dtv_property_process_get
  DVB: dvb_frontend: rename parameters to parameters_in
  DVB: dvb_frontend: remove unused assignments
  DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache
  DVB: dvb_frontend: add parameters_out
  DVB: allow to read back of detected parameters through S2API

 drivers/media/dvb/dvb-core/dvb_frontend.c |  356 +++++++++++++++--------------
 1 files changed, 186 insertions(+), 170 deletions(-)

-- 
1.7.2.5


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache Andreas Oberritter
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

- Return values should not be ORed. Abort early instead.
- Return -EINVAL instead of -1.

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index d04ef09..be0f631 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -1327,12 +1327,12 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 		tvp->u.data = fe->dtv_property_cache.dvbt2_plp_id;
 		break;
 	default:
-		r = -1;
+		return -EINVAL;
 	}
 
 	dtv_property_dump(tvp);
 
-	return r;
+	return 0;
 }
 
 static int dtv_property_process_set(struct dvb_frontend *fe,
@@ -1344,11 +1344,11 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	dtv_property_dump(tvp);
 
 	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.set_property)
+	if (fe->ops.set_property) {
 		r = fe->ops.set_property(fe, tvp);
-
-	if (r < 0)
-		return r;
+		if (r < 0)
+			return r;
+	}
 
 	switch(tvp->cmd) {
 	case DTV_CLEAR:
@@ -1367,7 +1367,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		dprintk("%s() Finalised property cache\n", __func__);
 		dtv_property_cache_submit(fe);
 
-		r |= dvb_frontend_ioctl_legacy(file, FE_SET_FRONTEND,
+		r = dvb_frontend_ioctl_legacy(file, FE_SET_FRONTEND,
 			&fepriv->parameters);
 		break;
 	case DTV_FREQUENCY:
@@ -1485,7 +1485,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		fe->dtv_property_cache.dvbt2_plp_id = tvp->u.data;
 		break;
 	default:
-		r = -1;
+		return -EINVAL;
 	}
 
 	return r;
@@ -1559,8 +1559,10 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 		}
 
 		for (i = 0; i < tvps->num; i++) {
-			(tvp + i)->result = dtv_property_process_set(fe, tvp + i, file);
-			err |= (tvp + i)->result;
+			err = dtv_property_process_set(fe, tvp + i, file);
+			if (err < 0)
+				goto out;
+			(tvp + i)->result = err;
 		}
 
 		if(fe->dtv_property_cache.state == DTV_TUNE)
@@ -1591,8 +1593,10 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 		}
 
 		for (i = 0; i < tvps->num; i++) {
-			(tvp + i)->result = dtv_property_process_get(fe, tvp + i, file);
-			err |= (tvp + i)->result;
+			err = dtv_property_process_get(fe, tvp + i, file);
+			if (err < 0)
+				goto out;
+			(tvp + i)->result = err;
 		}
 
 		if (copy_to_user(tvps->props, tvp, tvps->num * sizeof(struct dtv_property))) {
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-09  3:58   ` Mauro Carvalho Chehab
  2011-05-08 23:03 ` [PATCH 3/8] DVB: call get_property at the end of dtv_property_process_get Andreas Oberritter
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

- Use const pointers and remove assignments.
- delivery_system already gets assigned by DTV_DELIVERY_SYSTEM
  and dtv_property_cache_sync.

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index be0f631..1ac7633 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -1074,7 +1074,7 @@ static void dtv_property_cache_sync(struct dvb_frontend *fe,
  */
 static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
 {
-	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	struct dvb_frontend_parameters *p = &fepriv->parameters;
 
@@ -1086,14 +1086,12 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
 		dprintk("%s() Preparing QPSK req\n", __func__);
 		p->u.qpsk.symbol_rate = c->symbol_rate;
 		p->u.qpsk.fec_inner = c->fec_inner;
-		c->delivery_system = SYS_DVBS;
 		break;
 	case FE_QAM:
 		dprintk("%s() Preparing QAM req\n", __func__);
 		p->u.qam.symbol_rate = c->symbol_rate;
 		p->u.qam.fec_inner = c->fec_inner;
 		p->u.qam.modulation = c->modulation;
-		c->delivery_system = SYS_DVBC_ANNEX_AC;
 		break;
 	case FE_OFDM:
 		dprintk("%s() Preparing OFDM req\n", __func__);
@@ -1111,15 +1109,10 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
 		p->u.ofdm.transmission_mode = c->transmission_mode;
 		p->u.ofdm.guard_interval = c->guard_interval;
 		p->u.ofdm.hierarchy_information = c->hierarchy;
-		c->delivery_system = SYS_DVBT;
 		break;
 	case FE_ATSC:
 		dprintk("%s() Preparing VSB req\n", __func__);
 		p->u.vsb.modulation = c->modulation;
-		if ((c->modulation == VSB_8) || (c->modulation == VSB_16))
-			c->delivery_system = SYS_ATSC;
-		else
-			c->delivery_system = SYS_DVBC_ANNEX_B;
 		break;
 	}
 }
@@ -1129,7 +1122,7 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
  */
 static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
 {
-	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	struct dvb_frontend_parameters *p = &fepriv->parameters;
 
@@ -1170,7 +1163,7 @@ static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
 
 static void dtv_property_cache_submit(struct dvb_frontend *fe)
 {
-	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 
 	/* For legacy delivery systems we don't need the delivery_system to
 	 * be specified, but we populate the older structures from the cache
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/8] DVB: call get_property at the end of dtv_property_process_get
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 4/8] DVB: dvb_frontend: rename parameters to parameters_in Andreas Oberritter
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

- Drivers should be able to override properties returned to the user.
- The default values get prefilled from the cache.

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index 1ac7633..bcb4186 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -1196,14 +1196,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct dtv_property *tvp,
 				    struct file *file)
 {
-	int r = 0;
-
-	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.get_property)
-		r = fe->ops.get_property(fe, tvp);
-
-	if (r < 0)
-		return r;
+	int r;
 
 	switch(tvp->cmd) {
 	case DTV_FREQUENCY:
@@ -1323,6 +1316,13 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 		return -EINVAL;
 	}
 
+	/* Allow the frontend to override outgoing properties */
+	if (fe->ops.get_property) {
+		r = fe->ops.get_property(fe, tvp);
+		if (r < 0)
+			return r;
+	}
+
 	dtv_property_dump(tvp);
 
 	return 0;
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/8] DVB: dvb_frontend: rename parameters to parameters_in
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
                   ` (2 preceding siblings ...)
  2011-05-08 23:03 ` [PATCH 3/8] DVB: call get_property at the end of dtv_property_process_get Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 5/8] DVB: dvb_frontend: remove unused assignments Andreas Oberritter
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

- Preparation to distinguish input parameters from output parameters.

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   60 ++++++++++++++--------------
 1 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index bcb4186..aca8457 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -105,7 +105,7 @@ struct dvb_frontend_private {
 
 	/* thread/frontend values */
 	struct dvb_device *dvbdev;
-	struct dvb_frontend_parameters parameters;
+	struct dvb_frontend_parameters parameters_in;
 	struct dvb_fe_events events;
 	struct semaphore sem;
 	struct list_head list_head;
@@ -160,7 +160,7 @@ static void dvb_frontend_add_event(struct dvb_frontend *fe, fe_status_t status)
 
 	e = &events->events[events->eventw];
 
-	memcpy (&e->parameters, &fepriv->parameters,
+	memcpy (&e->parameters, &fepriv->parameters_in,
 		sizeof (struct dvb_frontend_parameters));
 
 	if (status & FE_HAS_LOCK)
@@ -277,12 +277,12 @@ static int dvb_frontend_swzigzag_autotune(struct dvb_frontend *fe, int check_wra
 	int ready = 0;
 	int fe_set_err = 0;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
-	int original_inversion = fepriv->parameters.inversion;
-	u32 original_frequency = fepriv->parameters.frequency;
+	int original_inversion = fepriv->parameters_in.inversion;
+	u32 original_frequency = fepriv->parameters_in.frequency;
 
 	/* are we using autoinversion? */
 	autoinversion = ((!(fe->ops.info.caps & FE_CAN_INVERSION_AUTO)) &&
-			 (fepriv->parameters.inversion == INVERSION_AUTO));
+			 (fepriv->parameters_in.inversion == INVERSION_AUTO));
 
 	/* setup parameters correctly */
 	while(!ready) {
@@ -348,18 +348,18 @@ static int dvb_frontend_swzigzag_autotune(struct dvb_frontend *fe, int check_wra
 		fepriv->auto_step, fepriv->auto_sub_step, fepriv->started_auto_step);
 
 	/* set the frontend itself */
-	fepriv->parameters.frequency += fepriv->lnb_drift;
+	fepriv->parameters_in.frequency += fepriv->lnb_drift;
 	if (autoinversion)
-		fepriv->parameters.inversion = fepriv->inversion;
+		fepriv->parameters_in.inversion = fepriv->inversion;
 	if (fe->ops.set_frontend)
-		fe_set_err = fe->ops.set_frontend(fe, &fepriv->parameters);
+		fe_set_err = fe->ops.set_frontend(fe, &fepriv->parameters_in);
 	if (fe_set_err < 0) {
 		fepriv->state = FESTATE_ERROR;
 		return fe_set_err;
 	}
 
-	fepriv->parameters.frequency = original_frequency;
-	fepriv->parameters.inversion = original_inversion;
+	fepriv->parameters_in.frequency = original_frequency;
+	fepriv->parameters_in.inversion = original_inversion;
 
 	fepriv->auto_sub_step++;
 	return 0;
@@ -383,7 +383,7 @@ static void dvb_frontend_swzigzag(struct dvb_frontend *fe)
 		if (fepriv->state & FESTATE_RETUNE) {
 			if (fe->ops.set_frontend)
 				retval = fe->ops.set_frontend(fe,
-							&fepriv->parameters);
+							&fepriv->parameters_in);
 			if (retval < 0)
 				fepriv->state = FESTATE_ERROR;
 			else
@@ -413,8 +413,8 @@ static void dvb_frontend_swzigzag(struct dvb_frontend *fe)
 
 		/* if we're tuned, then we have determined the correct inversion */
 		if ((!(fe->ops.info.caps & FE_CAN_INVERSION_AUTO)) &&
-		    (fepriv->parameters.inversion == INVERSION_AUTO)) {
-			fepriv->parameters.inversion = fepriv->inversion;
+		    (fepriv->parameters_in.inversion == INVERSION_AUTO)) {
+			fepriv->parameters_in.inversion = fepriv->inversion;
 		}
 		return;
 	}
@@ -594,7 +594,7 @@ restart:
 
 				if (fepriv->state & FESTATE_RETUNE) {
 					dprintk("%s: Retune requested, FESTATE_RETUNE\n", __func__);
-					params = &fepriv->parameters;
+					params = &fepriv->parameters_in;
 					fepriv->state = FESTATE_TUNED;
 				}
 
@@ -616,7 +616,7 @@ restart:
 				dprintk("%s: Frontend ALGO = DVBFE_ALGO_CUSTOM, state=%d\n", __func__, fepriv->state);
 				if (fepriv->state & FESTATE_RETUNE) {
 					dprintk("%s: Retune requested, FESTAT_RETUNE\n", __func__);
-					params = &fepriv->parameters;
+					params = &fepriv->parameters_in;
 					fepriv->state = FESTATE_TUNED;
 				}
 				/* Case where we are going to search for a carrier
@@ -625,7 +625,7 @@ restart:
 				 */
 				if (fepriv->algo_status & DVBFE_ALGO_SEARCH_AGAIN) {
 					if (fe->ops.search) {
-						fepriv->algo_status = fe->ops.search(fe, &fepriv->parameters);
+						fepriv->algo_status = fe->ops.search(fe, &fepriv->parameters_in);
 						/* We did do a search as was requested, the flags are
 						 * now unset as well and has the flags wrt to search.
 						 */
@@ -636,7 +636,7 @@ restart:
 				/* Track the carrier if the search was successful */
 				if (fepriv->algo_status == DVBFE_ALGO_SEARCH_SUCCESS) {
 					if (fe->ops.track)
-						fe->ops.track(fe, &fepriv->parameters);
+						fe->ops.track(fe, &fepriv->parameters_in);
 				} else {
 					fepriv->algo_status |= DVBFE_ALGO_SEARCH_AGAIN;
 					fepriv->delay = HZ / 2;
@@ -1076,7 +1076,7 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
 {
 	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
-	struct dvb_frontend_parameters *p = &fepriv->parameters;
+	struct dvb_frontend_parameters *p = &fepriv->parameters_in;
 
 	p->frequency = c->frequency;
 	p->inversion = c->inversion;
@@ -1124,7 +1124,7 @@ static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
 {
 	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
-	struct dvb_frontend_parameters *p = &fepriv->parameters;
+	struct dvb_frontend_parameters *p = &fepriv->parameters_in;
 
 	p->frequency = c->frequency;
 	p->inversion = c->inversion;
@@ -1361,7 +1361,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		dtv_property_cache_submit(fe);
 
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_FRONTEND,
-			&fepriv->parameters);
+			&fepriv->parameters_in);
 		break;
 	case DTV_FREQUENCY:
 		fe->dtv_property_cache.frequency = tvp->u.data;
@@ -1792,7 +1792,7 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 		struct dvb_frontend_tune_settings fetunesettings;
 
 		if(fe->dtv_property_cache.state == DTV_TUNE) {
-			if (dvb_frontend_check_parameters(fe, &fepriv->parameters) < 0) {
+			if (dvb_frontend_check_parameters(fe, &fepriv->parameters_in) < 0) {
 				err = -EINVAL;
 				break;
 			}
@@ -1802,9 +1802,9 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 				break;
 			}
 
-			memcpy (&fepriv->parameters, parg,
+			memcpy (&fepriv->parameters_in, parg,
 				sizeof (struct dvb_frontend_parameters));
-			dtv_property_cache_sync(fe, &fepriv->parameters);
+			dtv_property_cache_sync(fe, &fepriv->parameters_in);
 		}
 
 		memset(&fetunesettings, 0, sizeof(struct dvb_frontend_tune_settings));
@@ -1813,15 +1813,15 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 
 		/* force auto frequency inversion if requested */
 		if (dvb_force_auto_inversion) {
-			fepriv->parameters.inversion = INVERSION_AUTO;
+			fepriv->parameters_in.inversion = INVERSION_AUTO;
 			fetunesettings.parameters.inversion = INVERSION_AUTO;
 		}
 		if (fe->ops.info.type == FE_OFDM) {
 			/* without hierarchical coding code_rate_LP is irrelevant,
 			 * so we tolerate the otherwise invalid FEC_NONE setting */
-			if (fepriv->parameters.u.ofdm.hierarchy_information == HIERARCHY_NONE &&
-			    fepriv->parameters.u.ofdm.code_rate_LP == FEC_NONE)
-				fepriv->parameters.u.ofdm.code_rate_LP = FEC_AUTO;
+			if (fepriv->parameters_in.u.ofdm.hierarchy_information == HIERARCHY_NONE &&
+			    fepriv->parameters_in.u.ofdm.code_rate_LP == FEC_NONE)
+				fepriv->parameters_in.u.ofdm.code_rate_LP = FEC_AUTO;
 		}
 
 		/* get frontend-specific tuning settings */
@@ -1834,8 +1834,8 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 			switch(fe->ops.info.type) {
 			case FE_QPSK:
 				fepriv->min_delay = HZ/20;
-				fepriv->step_size = fepriv->parameters.u.qpsk.symbol_rate / 16000;
-				fepriv->max_drift = fepriv->parameters.u.qpsk.symbol_rate / 2000;
+				fepriv->step_size = fepriv->parameters_in.u.qpsk.symbol_rate / 16000;
+				fepriv->max_drift = fepriv->parameters_in.u.qpsk.symbol_rate / 2000;
 				break;
 
 			case FE_QAM:
@@ -1877,7 +1877,7 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 
 	case FE_GET_FRONTEND:
 		if (fe->ops.get_frontend) {
-			memcpy (parg, &fepriv->parameters, sizeof (struct dvb_frontend_parameters));
+			memcpy (parg, &fepriv->parameters_in, sizeof (struct dvb_frontend_parameters));
 			err = fe->ops.get_frontend(fe, (struct dvb_frontend_parameters*) parg);
 		}
 		break;
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/8] DVB: dvb_frontend: remove unused assignments
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
                   ` (3 preceding siblings ...)
  2011-05-08 23:03 ` [PATCH 4/8] DVB: dvb_frontend: rename parameters to parameters_in Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 6/8] DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache Andreas Oberritter
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index aca8457..9775cdc 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -612,11 +612,9 @@ restart:
 				dvb_frontend_swzigzag(fe);
 				break;
 			case DVBFE_ALGO_CUSTOM:
-				params = NULL; /* have we been asked to RETUNE ?	*/
 				dprintk("%s: Frontend ALGO = DVBFE_ALGO_CUSTOM, state=%d\n", __func__, fepriv->state);
 				if (fepriv->state & FESTATE_RETUNE) {
 					dprintk("%s: Retune requested, FESTAT_RETUNE\n", __func__);
-					params = &fepriv->parameters_in;
 					fepriv->state = FESTATE_TUNED;
 				}
 				/* Case where we are going to search for a carrier
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/8] DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
                   ` (4 preceding siblings ...)
  2011-05-08 23:03 ` [PATCH 5/8] DVB: dvb_frontend: remove unused assignments Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 7/8] DVB: dvb_frontend: add parameters_out Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 8/8] DVB: allow to read back of detected parameters through S2API Andreas Oberritter
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |  211 +++++++++++++++--------------
 1 files changed, 108 insertions(+), 103 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index 9775cdc..d4485c8 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -858,34 +858,34 @@ static int dvb_frontend_check_parameters(struct dvb_frontend *fe,
 
 static int dvb_frontend_clear_cache(struct dvb_frontend *fe)
 {
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int i;
 
-	memset(&(fe->dtv_property_cache), 0,
-			sizeof(struct dtv_frontend_properties));
-
-	fe->dtv_property_cache.state = DTV_CLEAR;
-	fe->dtv_property_cache.delivery_system = SYS_UNDEFINED;
-	fe->dtv_property_cache.inversion = INVERSION_AUTO;
-	fe->dtv_property_cache.fec_inner = FEC_AUTO;
-	fe->dtv_property_cache.transmission_mode = TRANSMISSION_MODE_AUTO;
-	fe->dtv_property_cache.bandwidth_hz = BANDWIDTH_AUTO;
-	fe->dtv_property_cache.guard_interval = GUARD_INTERVAL_AUTO;
-	fe->dtv_property_cache.hierarchy = HIERARCHY_AUTO;
-	fe->dtv_property_cache.symbol_rate = QAM_AUTO;
-	fe->dtv_property_cache.code_rate_HP = FEC_AUTO;
-	fe->dtv_property_cache.code_rate_LP = FEC_AUTO;
-
-	fe->dtv_property_cache.isdbt_partial_reception = -1;
-	fe->dtv_property_cache.isdbt_sb_mode = -1;
-	fe->dtv_property_cache.isdbt_sb_subchannel = -1;
-	fe->dtv_property_cache.isdbt_sb_segment_idx = -1;
-	fe->dtv_property_cache.isdbt_sb_segment_count = -1;
-	fe->dtv_property_cache.isdbt_layer_enabled = 0x7;
+	memset(c, 0, sizeof(struct dtv_frontend_properties));
+
+	c->state = DTV_CLEAR;
+	c->delivery_system = SYS_UNDEFINED;
+	c->inversion = INVERSION_AUTO;
+	c->fec_inner = FEC_AUTO;
+	c->transmission_mode = TRANSMISSION_MODE_AUTO;
+	c->bandwidth_hz = BANDWIDTH_AUTO;
+	c->guard_interval = GUARD_INTERVAL_AUTO;
+	c->hierarchy = HIERARCHY_AUTO;
+	c->symbol_rate = QAM_AUTO;
+	c->code_rate_HP = FEC_AUTO;
+	c->code_rate_LP = FEC_AUTO;
+
+	c->isdbt_partial_reception = -1;
+	c->isdbt_sb_mode = -1;
+	c->isdbt_sb_subchannel = -1;
+	c->isdbt_sb_segment_idx = -1;
+	c->isdbt_sb_segment_count = -1;
+	c->isdbt_layer_enabled = 0x7;
 	for (i = 0; i < 3; i++) {
-		fe->dtv_property_cache.layer[i].fec = FEC_AUTO;
-		fe->dtv_property_cache.layer[i].modulation = QAM_AUTO;
-		fe->dtv_property_cache.layer[i].interleaving = -1;
-		fe->dtv_property_cache.layer[i].segment_count = -1;
+		c->layer[i].fec = FEC_AUTO;
+		c->layer[i].modulation = QAM_AUTO;
+		c->layer[i].interleaving = -1;
+		c->layer[i].segment_count = -1;
 	}
 
 	return 0;
@@ -1194,121 +1194,122 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct dtv_property *tvp,
 				    struct file *file)
 {
+	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int r;
 
 	switch(tvp->cmd) {
 	case DTV_FREQUENCY:
-		tvp->u.data = fe->dtv_property_cache.frequency;
+		tvp->u.data = c->frequency;
 		break;
 	case DTV_MODULATION:
-		tvp->u.data = fe->dtv_property_cache.modulation;
+		tvp->u.data = c->modulation;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		tvp->u.data = fe->dtv_property_cache.bandwidth_hz;
+		tvp->u.data = c->bandwidth_hz;
 		break;
 	case DTV_INVERSION:
-		tvp->u.data = fe->dtv_property_cache.inversion;
+		tvp->u.data = c->inversion;
 		break;
 	case DTV_SYMBOL_RATE:
-		tvp->u.data = fe->dtv_property_cache.symbol_rate;
+		tvp->u.data = c->symbol_rate;
 		break;
 	case DTV_INNER_FEC:
-		tvp->u.data = fe->dtv_property_cache.fec_inner;
+		tvp->u.data = c->fec_inner;
 		break;
 	case DTV_PILOT:
-		tvp->u.data = fe->dtv_property_cache.pilot;
+		tvp->u.data = c->pilot;
 		break;
 	case DTV_ROLLOFF:
-		tvp->u.data = fe->dtv_property_cache.rolloff;
+		tvp->u.data = c->rolloff;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		tvp->u.data = fe->dtv_property_cache.delivery_system;
+		tvp->u.data = c->delivery_system;
 		break;
 	case DTV_VOLTAGE:
-		tvp->u.data = fe->dtv_property_cache.voltage;
+		tvp->u.data = c->voltage;
 		break;
 	case DTV_TONE:
-		tvp->u.data = fe->dtv_property_cache.sectone;
+		tvp->u.data = c->sectone;
 		break;
 	case DTV_API_VERSION:
 		tvp->u.data = (DVB_API_VERSION << 8) | DVB_API_VERSION_MINOR;
 		break;
 	case DTV_CODE_RATE_HP:
-		tvp->u.data = fe->dtv_property_cache.code_rate_HP;
+		tvp->u.data = c->code_rate_HP;
 		break;
 	case DTV_CODE_RATE_LP:
-		tvp->u.data = fe->dtv_property_cache.code_rate_LP;
+		tvp->u.data = c->code_rate_LP;
 		break;
 	case DTV_GUARD_INTERVAL:
-		tvp->u.data = fe->dtv_property_cache.guard_interval;
+		tvp->u.data = c->guard_interval;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		tvp->u.data = fe->dtv_property_cache.transmission_mode;
+		tvp->u.data = c->transmission_mode;
 		break;
 	case DTV_HIERARCHY:
-		tvp->u.data = fe->dtv_property_cache.hierarchy;
+		tvp->u.data = c->hierarchy;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		tvp->u.data = fe->dtv_property_cache.isdbt_partial_reception;
+		tvp->u.data = c->isdbt_partial_reception;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		tvp->u.data = fe->dtv_property_cache.isdbt_sb_mode;
+		tvp->u.data = c->isdbt_sb_mode;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		tvp->u.data = fe->dtv_property_cache.isdbt_sb_subchannel;
+		tvp->u.data = c->isdbt_sb_subchannel;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		tvp->u.data = fe->dtv_property_cache.isdbt_sb_segment_idx;
+		tvp->u.data = c->isdbt_sb_segment_idx;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		tvp->u.data = fe->dtv_property_cache.isdbt_sb_segment_count;
+		tvp->u.data = c->isdbt_sb_segment_count;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		tvp->u.data = fe->dtv_property_cache.isdbt_layer_enabled;
+		tvp->u.data = c->isdbt_layer_enabled;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		tvp->u.data = fe->dtv_property_cache.layer[0].fec;
+		tvp->u.data = c->layer[0].fec;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		tvp->u.data = fe->dtv_property_cache.layer[0].modulation;
+		tvp->u.data = c->layer[0].modulation;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		tvp->u.data = fe->dtv_property_cache.layer[0].segment_count;
+		tvp->u.data = c->layer[0].segment_count;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		tvp->u.data = fe->dtv_property_cache.layer[0].interleaving;
+		tvp->u.data = c->layer[0].interleaving;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		tvp->u.data = fe->dtv_property_cache.layer[1].fec;
+		tvp->u.data = c->layer[1].fec;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		tvp->u.data = fe->dtv_property_cache.layer[1].modulation;
+		tvp->u.data = c->layer[1].modulation;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		tvp->u.data = fe->dtv_property_cache.layer[1].segment_count;
+		tvp->u.data = c->layer[1].segment_count;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		tvp->u.data = fe->dtv_property_cache.layer[1].interleaving;
+		tvp->u.data = c->layer[1].interleaving;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		tvp->u.data = fe->dtv_property_cache.layer[2].fec;
+		tvp->u.data = c->layer[2].fec;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		tvp->u.data = fe->dtv_property_cache.layer[2].modulation;
+		tvp->u.data = c->layer[2].modulation;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		tvp->u.data = fe->dtv_property_cache.layer[2].segment_count;
+		tvp->u.data = c->layer[2].segment_count;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		tvp->u.data = fe->dtv_property_cache.layer[2].interleaving;
+		tvp->u.data = c->layer[2].interleaving;
 		break;
 	case DTV_ISDBS_TS_ID:
-		tvp->u.data = fe->dtv_property_cache.isdbs_ts_id;
+		tvp->u.data = c->isdbs_ts_id;
 		break;
 	case DTV_DVBT2_PLP_ID:
-		tvp->u.data = fe->dtv_property_cache.dvbt2_plp_id;
+		tvp->u.data = c->dvbt2_plp_id;
 		break;
 	default:
 		return -EINVAL;
@@ -1331,6 +1332,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 				    struct file *file)
 {
 	int r = 0;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	dtv_property_dump(tvp);
 
@@ -1354,7 +1356,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
 		 * ioctl.
 		 */
-		fe->dtv_property_cache.state = tvp->cmd;
+		c->state = tvp->cmd;
 		dprintk("%s() Finalised property cache\n", __func__);
 		dtv_property_cache_submit(fe);
 
@@ -1362,118 +1364,118 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 			&fepriv->parameters_in);
 		break;
 	case DTV_FREQUENCY:
-		fe->dtv_property_cache.frequency = tvp->u.data;
+		c->frequency = tvp->u.data;
 		break;
 	case DTV_MODULATION:
-		fe->dtv_property_cache.modulation = tvp->u.data;
+		c->modulation = tvp->u.data;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		fe->dtv_property_cache.bandwidth_hz = tvp->u.data;
+		c->bandwidth_hz = tvp->u.data;
 		break;
 	case DTV_INVERSION:
-		fe->dtv_property_cache.inversion = tvp->u.data;
+		c->inversion = tvp->u.data;
 		break;
 	case DTV_SYMBOL_RATE:
-		fe->dtv_property_cache.symbol_rate = tvp->u.data;
+		c->symbol_rate = tvp->u.data;
 		break;
 	case DTV_INNER_FEC:
-		fe->dtv_property_cache.fec_inner = tvp->u.data;
+		c->fec_inner = tvp->u.data;
 		break;
 	case DTV_PILOT:
-		fe->dtv_property_cache.pilot = tvp->u.data;
+		c->pilot = tvp->u.data;
 		break;
 	case DTV_ROLLOFF:
-		fe->dtv_property_cache.rolloff = tvp->u.data;
+		c->rolloff = tvp->u.data;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		fe->dtv_property_cache.delivery_system = tvp->u.data;
+		c->delivery_system = tvp->u.data;
 		break;
 	case DTV_VOLTAGE:
-		fe->dtv_property_cache.voltage = tvp->u.data;
+		c->voltage = tvp->u.data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
-			(void *)fe->dtv_property_cache.voltage);
+			(void *)c->voltage);
 		break;
 	case DTV_TONE:
-		fe->dtv_property_cache.sectone = tvp->u.data;
+		c->sectone = tvp->u.data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
-			(void *)fe->dtv_property_cache.sectone);
+			(void *)c->sectone);
 		break;
 	case DTV_CODE_RATE_HP:
-		fe->dtv_property_cache.code_rate_HP = tvp->u.data;
+		c->code_rate_HP = tvp->u.data;
 		break;
 	case DTV_CODE_RATE_LP:
-		fe->dtv_property_cache.code_rate_LP = tvp->u.data;
+		c->code_rate_LP = tvp->u.data;
 		break;
 	case DTV_GUARD_INTERVAL:
-		fe->dtv_property_cache.guard_interval = tvp->u.data;
+		c->guard_interval = tvp->u.data;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		fe->dtv_property_cache.transmission_mode = tvp->u.data;
+		c->transmission_mode = tvp->u.data;
 		break;
 	case DTV_HIERARCHY:
-		fe->dtv_property_cache.hierarchy = tvp->u.data;
+		c->hierarchy = tvp->u.data;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		fe->dtv_property_cache.isdbt_partial_reception = tvp->u.data;
+		c->isdbt_partial_reception = tvp->u.data;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		fe->dtv_property_cache.isdbt_sb_mode = tvp->u.data;
+		c->isdbt_sb_mode = tvp->u.data;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		fe->dtv_property_cache.isdbt_sb_subchannel = tvp->u.data;
+		c->isdbt_sb_subchannel = tvp->u.data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		fe->dtv_property_cache.isdbt_sb_segment_idx = tvp->u.data;
+		c->isdbt_sb_segment_idx = tvp->u.data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		fe->dtv_property_cache.isdbt_sb_segment_count = tvp->u.data;
+		c->isdbt_sb_segment_count = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		fe->dtv_property_cache.isdbt_layer_enabled = tvp->u.data;
+		c->isdbt_layer_enabled = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		fe->dtv_property_cache.layer[0].fec = tvp->u.data;
+		c->layer[0].fec = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		fe->dtv_property_cache.layer[0].modulation = tvp->u.data;
+		c->layer[0].modulation = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		fe->dtv_property_cache.layer[0].segment_count = tvp->u.data;
+		c->layer[0].segment_count = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		fe->dtv_property_cache.layer[0].interleaving = tvp->u.data;
+		c->layer[0].interleaving = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		fe->dtv_property_cache.layer[1].fec = tvp->u.data;
+		c->layer[1].fec = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		fe->dtv_property_cache.layer[1].modulation = tvp->u.data;
+		c->layer[1].modulation = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		fe->dtv_property_cache.layer[1].segment_count = tvp->u.data;
+		c->layer[1].segment_count = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		fe->dtv_property_cache.layer[1].interleaving = tvp->u.data;
+		c->layer[1].interleaving = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		fe->dtv_property_cache.layer[2].fec = tvp->u.data;
+		c->layer[2].fec = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		fe->dtv_property_cache.layer[2].modulation = tvp->u.data;
+		c->layer[2].modulation = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		fe->dtv_property_cache.layer[2].segment_count = tvp->u.data;
+		c->layer[2].segment_count = tvp->u.data;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		fe->dtv_property_cache.layer[2].interleaving = tvp->u.data;
+		c->layer[2].interleaving = tvp->u.data;
 		break;
 	case DTV_ISDBS_TS_ID:
-		fe->dtv_property_cache.isdbs_ts_id = tvp->u.data;
+		c->isdbs_ts_id = tvp->u.data;
 		break;
 	case DTV_DVBT2_PLP_ID:
-		fe->dtv_property_cache.dvbt2_plp_id = tvp->u.data;
+		c->dvbt2_plp_id = tvp->u.data;
 		break;
 	default:
 		return -EINVAL;
@@ -1487,6 +1489,7 @@ static int dvb_frontend_ioctl(struct file *file,
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_frontend *fe = dvbdev->priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	int err = -EOPNOTSUPP;
 
@@ -1506,7 +1509,7 @@ static int dvb_frontend_ioctl(struct file *file,
 	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
-		fe->dtv_property_cache.state = DTV_UNDEFINED;
+		c->state = DTV_UNDEFINED;
 		err = dvb_frontend_ioctl_legacy(file, cmd, parg);
 	}
 
@@ -1519,6 +1522,7 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_frontend *fe = dvbdev->priv;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int err = 0;
 
 	struct dtv_properties *tvps = NULL;
@@ -1556,7 +1560,7 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			(tvp + i)->result = err;
 		}
 
-		if(fe->dtv_property_cache.state == DTV_TUNE)
+		if (c->state == DTV_TUNE)
 			dprintk("%s() Property cache is full, tuning\n", __func__);
 
 	} else
@@ -1787,9 +1791,10 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 		break;
 
 	case FE_SET_FRONTEND: {
+		struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 		struct dvb_frontend_tune_settings fetunesettings;
 
-		if(fe->dtv_property_cache.state == DTV_TUNE) {
+		if (c->state == DTV_TUNE) {
 			if (dvb_frontend_check_parameters(fe, &fepriv->parameters_in) < 0) {
 				err = -EINVAL;
 				break;
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/8] DVB: dvb_frontend: add parameters_out
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
                   ` (5 preceding siblings ...)
  2011-05-08 23:03 ` [PATCH 6/8] DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  2011-05-08 23:03 ` [PATCH 8/8] DVB: allow to read back of detected parameters through S2API Andreas Oberritter
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

- Holds the parameters detected by the demod.
- Updated on every call to get_frontend, either through ioctl or when
  a frontend event occurs.
- Reset to input parameters after every call to set_frontend, tune or
  search/track.

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index d4485c8..b41e5dc 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -106,6 +106,7 @@ struct dvb_frontend_private {
 	/* thread/frontend values */
 	struct dvb_device *dvbdev;
 	struct dvb_frontend_parameters parameters_in;
+	struct dvb_frontend_parameters parameters_out;
 	struct dvb_fe_events events;
 	struct semaphore sem;
 	struct list_head list_head;
@@ -160,12 +161,11 @@ static void dvb_frontend_add_event(struct dvb_frontend *fe, fe_status_t status)
 
 	e = &events->events[events->eventw];
 
-	memcpy (&e->parameters, &fepriv->parameters_in,
-		sizeof (struct dvb_frontend_parameters));
-
 	if (status & FE_HAS_LOCK)
 		if (fe->ops.get_frontend)
-			fe->ops.get_frontend(fe, &e->parameters);
+			fe->ops.get_frontend(fe, &fepriv->parameters_out);
+
+	e->parameters = fepriv->parameters_out;
 
 	events->eventw = wp;
 
@@ -353,6 +353,7 @@ static int dvb_frontend_swzigzag_autotune(struct dvb_frontend *fe, int check_wra
 		fepriv->parameters_in.inversion = fepriv->inversion;
 	if (fe->ops.set_frontend)
 		fe_set_err = fe->ops.set_frontend(fe, &fepriv->parameters_in);
+	fepriv->parameters_out = fepriv->parameters_in;
 	if (fe_set_err < 0) {
 		fepriv->state = FESTATE_ERROR;
 		return fe_set_err;
@@ -384,6 +385,7 @@ static void dvb_frontend_swzigzag(struct dvb_frontend *fe)
 			if (fe->ops.set_frontend)
 				retval = fe->ops.set_frontend(fe,
 							&fepriv->parameters_in);
+			fepriv->parameters_out = fepriv->parameters_in;
 			if (retval < 0)
 				fepriv->state = FESTATE_ERROR;
 			else
@@ -600,6 +602,8 @@ restart:
 
 				if (fe->ops.tune)
 					fe->ops.tune(fe, params, fepriv->tune_mode_flags, &fepriv->delay, &s);
+				if (params)
+					fepriv->parameters_out = *params;
 
 				if (s != fepriv->status && !(fepriv->tune_mode_flags & FE_TUNE_MODE_ONESHOT)) {
 					dprintk("%s: state changed, adding current state\n", __func__);
@@ -639,6 +643,7 @@ restart:
 					fepriv->algo_status |= DVBFE_ALGO_SEARCH_AGAIN;
 					fepriv->delay = HZ / 2;
 				}
+				fepriv->parameters_out = fepriv->parameters_in;
 				fe->ops.read_status(fe, &s);
 				if (s != fepriv->status) {
 					dvb_frontend_add_event(fe, s); /* update event list */
@@ -1880,8 +1885,8 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 
 	case FE_GET_FRONTEND:
 		if (fe->ops.get_frontend) {
-			memcpy (parg, &fepriv->parameters_in, sizeof (struct dvb_frontend_parameters));
-			err = fe->ops.get_frontend(fe, (struct dvb_frontend_parameters*) parg);
+			err = fe->ops.get_frontend(fe, &fepriv->parameters_out);
+			memcpy(parg, &fepriv->parameters_out, sizeof(struct dvb_frontend_parameters));
 		}
 		break;
 
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 8/8] DVB: allow to read back of detected parameters through S2API
  2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
                   ` (6 preceding siblings ...)
  2011-05-08 23:03 ` [PATCH 7/8] DVB: dvb_frontend: add parameters_out Andreas Oberritter
@ 2011-05-08 23:03 ` Andreas Oberritter
  7 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-08 23:03 UTC (permalink / raw)
  To: linux-media; +Cc: Thierry LELEGARD

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
---
 drivers/media/dvb/dvb-core/dvb_frontend.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index b41e5dc..a0f0458 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -1023,10 +1023,9 @@ static int is_legacy_delivery_system(fe_delivery_system_t s)
  * it's being used for the legacy or new API, reducing code and complexity.
  */
 static void dtv_property_cache_sync(struct dvb_frontend *fe,
-				    struct dvb_frontend_parameters *p)
+				    struct dtv_frontend_properties *c,
+				    const struct dvb_frontend_parameters *p)
 {
-	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-
 	c->frequency = p->frequency;
 	c->inversion = p->inversion;
 
@@ -1200,8 +1199,20 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 				    struct file *file)
 {
 	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct dvb_frontend_private *fepriv = fe->frontend_priv;
+	struct dtv_frontend_properties cdetected;
 	int r;
 
+	/*
+	 * If the driver implements a get_frontend function, then convert
+	 * detected parameters to S2API properties.
+	 */
+	if (fe->ops.get_frontend) {
+		cdetected = *c;
+		dtv_property_cache_sync(fe, &cdetected, &fepriv->parameters_out);
+		c = &cdetected;
+	}
+
 	switch(tvp->cmd) {
 	case DTV_FREQUENCY:
 		tvp->u.data = c->frequency;
@@ -1812,7 +1823,7 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 
 			memcpy (&fepriv->parameters_in, parg,
 				sizeof (struct dvb_frontend_parameters));
-			dtv_property_cache_sync(fe, &fepriv->parameters_in);
+			dtv_property_cache_sync(fe, c, &fepriv->parameters_in);
 		}
 
 		memset(&fetunesettings, 0, sizeof(struct dvb_frontend_tune_settings));
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache
  2011-05-08 23:03 ` [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache Andreas Oberritter
@ 2011-05-09  3:58   ` Mauro Carvalho Chehab
  2011-05-09  4:12     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2011-05-09  3:58 UTC (permalink / raw)
  To: Andreas Oberritter; +Cc: linux-media, Thierry LELEGARD

Em 09-05-2011 01:03, Andreas Oberritter escreveu:
> - Use const pointers and remove assignments.

That's OK.

> - delivery_system already gets assigned by DTV_DELIVERY_SYSTEM
>   and dtv_property_cache_sync.

The logic for those legacy params is too complex. It is easy that
a latter patch to break the implicit set via dtv_property_cache_sync().

Do you actually see a bug caused by the extra set for the delivery system?
If not, I prefer to keep this extra re-assignment.
> 
> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
> ---
>  drivers/media/dvb/dvb-core/dvb_frontend.c |   13 +++----------
>  1 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
> index be0f631..1ac7633 100644
> --- a/drivers/media/dvb/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
> @@ -1074,7 +1074,7 @@ static void dtv_property_cache_sync(struct dvb_frontend *fe,
>   */
>  static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>  {
> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>  	struct dvb_frontend_private *fepriv = fe->frontend_priv;
>  	struct dvb_frontend_parameters *p = &fepriv->parameters;
>  
> @@ -1086,14 +1086,12 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>  		dprintk("%s() Preparing QPSK req\n", __func__);
>  		p->u.qpsk.symbol_rate = c->symbol_rate;
>  		p->u.qpsk.fec_inner = c->fec_inner;
> -		c->delivery_system = SYS_DVBS;
>  		break;
>  	case FE_QAM:
>  		dprintk("%s() Preparing QAM req\n", __func__);
>  		p->u.qam.symbol_rate = c->symbol_rate;
>  		p->u.qam.fec_inner = c->fec_inner;
>  		p->u.qam.modulation = c->modulation;
> -		c->delivery_system = SYS_DVBC_ANNEX_AC;
>  		break;
>  	case FE_OFDM:
>  		dprintk("%s() Preparing OFDM req\n", __func__);
> @@ -1111,15 +1109,10 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>  		p->u.ofdm.transmission_mode = c->transmission_mode;
>  		p->u.ofdm.guard_interval = c->guard_interval;
>  		p->u.ofdm.hierarchy_information = c->hierarchy;
> -		c->delivery_system = SYS_DVBT;
>  		break;
>  	case FE_ATSC:
>  		dprintk("%s() Preparing VSB req\n", __func__);
>  		p->u.vsb.modulation = c->modulation;
> -		if ((c->modulation == VSB_8) || (c->modulation == VSB_16))
> -			c->delivery_system = SYS_ATSC;
> -		else
> -			c->delivery_system = SYS_DVBC_ANNEX_B;
>  		break;
>  	}
>  }
> @@ -1129,7 +1122,7 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>   */
>  static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
>  {
> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>  	struct dvb_frontend_private *fepriv = fe->frontend_priv;
>  	struct dvb_frontend_parameters *p = &fepriv->parameters;
>  
> @@ -1170,7 +1163,7 @@ static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
>  
>  static void dtv_property_cache_submit(struct dvb_frontend *fe)
>  {
> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>  
>  	/* For legacy delivery systems we don't need the delivery_system to
>  	 * be specified, but we populate the older structures from the cache


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache
  2011-05-09  3:58   ` Mauro Carvalho Chehab
@ 2011-05-09  4:12     ` Mauro Carvalho Chehab
  2011-05-09 10:12       ` Andreas Oberritter
  0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2011-05-09  4:12 UTC (permalink / raw)
  To: Andreas Oberritter; +Cc: linux-media, Thierry LELEGARD

Em 09-05-2011 05:58, Mauro Carvalho Chehab escreveu:
> Em 09-05-2011 01:03, Andreas Oberritter escreveu:
>> - Use const pointers and remove assignments.
> 
> That's OK.
> 
>> - delivery_system already gets assigned by DTV_DELIVERY_SYSTEM
>>   and dtv_property_cache_sync.
> 
> The logic for those legacy params is too complex. It is easy that
> a latter patch to break the implicit set via dtv_property_cache_sync().
> 
> Do you actually see a bug caused by the extra set for the delivery system?
> If not, I prefer to keep this extra re-assignment.

Hmm... after applying all patches the logic change, and patch 2 may actually
make sense. I'll need to re-examine the patch series. 

On a quick look, if applied as-is, I suspect that git bisect
will break dvb in the middle of the patch series.

Anyway, patch 1/8 is OK. For now, I'll apply only this patch. I'll delay the
others until I have more time. I'm currently traveling abroad, due to Linaro
Development Summit, so, I don't have much time for review (and I'm also suffering
for a 5 hours jet-leg).

>>
>> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
>> ---
>>  drivers/media/dvb/dvb-core/dvb_frontend.c |   13 +++----------
>>  1 files changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
>> index be0f631..1ac7633 100644
>> --- a/drivers/media/dvb/dvb-core/dvb_frontend.c
>> +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
>> @@ -1074,7 +1074,7 @@ static void dtv_property_cache_sync(struct dvb_frontend *fe,
>>   */
>>  static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>>  {
>> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>  	struct dvb_frontend_private *fepriv = fe->frontend_priv;
>>  	struct dvb_frontend_parameters *p = &fepriv->parameters;
>>  
>> @@ -1086,14 +1086,12 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>>  		dprintk("%s() Preparing QPSK req\n", __func__);
>>  		p->u.qpsk.symbol_rate = c->symbol_rate;
>>  		p->u.qpsk.fec_inner = c->fec_inner;
>> -		c->delivery_system = SYS_DVBS;
>>  		break;
>>  	case FE_QAM:
>>  		dprintk("%s() Preparing QAM req\n", __func__);
>>  		p->u.qam.symbol_rate = c->symbol_rate;
>>  		p->u.qam.fec_inner = c->fec_inner;
>>  		p->u.qam.modulation = c->modulation;
>> -		c->delivery_system = SYS_DVBC_ANNEX_AC;
>>  		break;
>>  	case FE_OFDM:
>>  		dprintk("%s() Preparing OFDM req\n", __func__);
>> @@ -1111,15 +1109,10 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>>  		p->u.ofdm.transmission_mode = c->transmission_mode;
>>  		p->u.ofdm.guard_interval = c->guard_interval;
>>  		p->u.ofdm.hierarchy_information = c->hierarchy;
>> -		c->delivery_system = SYS_DVBT;
>>  		break;
>>  	case FE_ATSC:
>>  		dprintk("%s() Preparing VSB req\n", __func__);
>>  		p->u.vsb.modulation = c->modulation;
>> -		if ((c->modulation == VSB_8) || (c->modulation == VSB_16))
>> -			c->delivery_system = SYS_ATSC;
>> -		else
>> -			c->delivery_system = SYS_DVBC_ANNEX_B;
>>  		break;
>>  	}
>>  }
>> @@ -1129,7 +1122,7 @@ static void dtv_property_legacy_params_sync(struct dvb_frontend *fe)
>>   */
>>  static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
>>  {
>> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>  	struct dvb_frontend_private *fepriv = fe->frontend_priv;
>>  	struct dvb_frontend_parameters *p = &fepriv->parameters;
>>  
>> @@ -1170,7 +1163,7 @@ static void dtv_property_adv_params_sync(struct dvb_frontend *fe)
>>  
>>  static void dtv_property_cache_submit(struct dvb_frontend *fe)
>>  {
>> -	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>> +	const struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>>  
>>  	/* For legacy delivery systems we don't need the delivery_system to
>>  	 * be specified, but we populate the older structures from the cache
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache
  2011-05-09  4:12     ` Mauro Carvalho Chehab
@ 2011-05-09 10:12       ` Andreas Oberritter
  0 siblings, 0 replies; 12+ messages in thread
From: Andreas Oberritter @ 2011-05-09 10:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Thierry LELEGARD

On 05/09/2011 06:12 AM, Mauro Carvalho Chehab wrote:
> Em 09-05-2011 05:58, Mauro Carvalho Chehab escreveu:
>> Em 09-05-2011 01:03, Andreas Oberritter escreveu:
>>> - Use const pointers and remove assignments.
>>
>> That's OK.
>>
>>> - delivery_system already gets assigned by DTV_DELIVERY_SYSTEM
>>>   and dtv_property_cache_sync.
>>
>> The logic for those legacy params is too complex. It is easy that
>> a latter patch to break the implicit set via dtv_property_cache_sync().
>>
>> Do you actually see a bug caused by the extra set for the delivery system?
>> If not, I prefer to keep this extra re-assignment.

No, but I was suprised to see the dtv_frontend_properties getting
modified in this function. There are three functions converting between
old and new structures:

dtv_property_cache_sync:
  converts from dvb_frontend_parameters to dtv_frontend_properties

dtv_property_legacy_params_sync and dtv_property_adv_params_sync:
  convert from dtv_frontend_properties to dvb_frontend_parameters

Assigning to fields of a source structure indicates a logical error. I
haven't found any comment on why this should be needed in the Git
history or in the mailing list archives.

Btw., I think that two functions should be enough. Any reason for not
merging dtv_property_adv_params_sync into
dtv_property_legacy_params_sync and calling the latter unconditionally?

> Hmm... after applying all patches the logic change, and patch 2 may actually
> make sense. I'll need to re-examine the patch series. 
> 
> On a quick look, if applied as-is, I suspect that git bisect
> will break dvb in the middle of the patch series.

Patches 6 and 8 depend on the patches mentioned in the cover letter. All
other patches should apply and compile cleanly. Which patch do you
suspect to break bisectability?

> Anyway, patch 1/8 is OK. For now, I'll apply only this patch. I'll delay the
> others until I have more time. I'm currently traveling abroad, due to Linaro
> Development Summit, so, I don't have much time for review (and I'm also suffering
> for a 5 hours jet-leg).

OK, there's no hurry.

Regards,
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-05-09 10:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-08 23:03 [PATCH 0/8] dvb_frontend cleanup, S2API regression fix Andreas Oberritter
2011-05-08 23:03 ` [PATCH 1/8] DVB: return meaningful error codes in dvb_frontend Andreas Oberritter
2011-05-08 23:03 ` [PATCH 2/8] DVB: dtv_property_cache_submit shouldn't modifiy the cache Andreas Oberritter
2011-05-09  3:58   ` Mauro Carvalho Chehab
2011-05-09  4:12     ` Mauro Carvalho Chehab
2011-05-09 10:12       ` Andreas Oberritter
2011-05-08 23:03 ` [PATCH 3/8] DVB: call get_property at the end of dtv_property_process_get Andreas Oberritter
2011-05-08 23:03 ` [PATCH 4/8] DVB: dvb_frontend: rename parameters to parameters_in Andreas Oberritter
2011-05-08 23:03 ` [PATCH 5/8] DVB: dvb_frontend: remove unused assignments Andreas Oberritter
2011-05-08 23:03 ` [PATCH 6/8] DVB: dvb_frontend: use shortcut to access fe->dtv_property_cache Andreas Oberritter
2011-05-08 23:03 ` [PATCH 7/8] DVB: dvb_frontend: add parameters_out Andreas Oberritter
2011-05-08 23:03 ` [PATCH 8/8] DVB: allow to read back of detected parameters through S2API Andreas Oberritter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox