All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.