PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Manu Abraham]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities.patch --]
[-- Type: text/x-patch, Size: 24561 bytes --]

From 096bca663d076255852b88ce9a87c0f07fa17dec Mon Sep 17 00:00:00 2001
From: Manu Abraham <abraham.manu@gmail.com>
Date: Mon, 21 Nov 2011 19:58:03 +0530
Subject: [PATCH 12/13] CXD2820r: Query DVB frontend delivery capabilities

Override default delivery system information provided by FE_GET_INFO, so
that applications can enumerate delivery systems provided by the frontend.

Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
---
 drivers/media/dvb/frontends/cxd2820r_c.c    |   17 +-
 drivers/media/dvb/frontends/cxd2820r_core.c |  651 +++++++++------------------
 drivers/media/dvb/frontends/cxd2820r_priv.h |    5 +-
 drivers/media/dvb/frontends/cxd2820r_t.c    |   19 +-
 drivers/media/dvb/frontends/cxd2820r_t2.c   |   20 +-
 5 files changed, 265 insertions(+), 447 deletions(-)

diff --git a/drivers/media/dvb/frontends/cxd2820r_c.c b/drivers/media/dvb/frontends/cxd2820r_c.c
index b85f501..f6d0ff7 100644
--- a/drivers/media/dvb/frontends/cxd2820r_c.c
+++ b/drivers/media/dvb/frontends/cxd2820r_c.c
@@ -22,10 +22,11 @@
 #include "cxd2820r_priv.h"
 
 int cxd2820r_set_frontend_c(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *params)
+			    struct dvb_frontend_parameters *params)
 {
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct tuner_state tstate;
 	int ret, i;
 	u8 buf[2];
 	u16 if_ctl;
@@ -55,8 +56,18 @@ int cxd2820r_set_frontend_c(struct dvb_frontend *fe,
 		goto error;
 
 	/* program tuner */
-	if (fe->ops.tuner_ops.set_params)
-		fe->ops.tuner_ops.set_params(fe, params);
+	tstate.delsys = SYS_DVBC_ANNEX_AC;
+	tstate.frequency = c->frequency;
+
+	if (fe->ops.tuner_ops.set_state) {
+		fe->ops.tuner_ops.set_state(fe,
+					    DVBFE_TUNER_DELSYS    |
+					    DVBFE_TUNER_FREQUENCY,
+					    &tstate);
+	} else {
+		if (fe->ops.tuner_ops.set_params)
+			fe->ops.tuner_ops.set_params(fe, params);
+	}
 
 	if (priv->delivery_system !=  SYS_DVBC_ANNEX_AC) {
 		for (i = 0; i < ARRAY_SIZE(tab); i++) {
diff --git a/drivers/media/dvb/frontends/cxd2820r_core.c b/drivers/media/dvb/frontends/cxd2820r_core.c
index 036480f..5b0120a 100644
--- a/drivers/media/dvb/frontends/cxd2820r_core.c
+++ b/drivers/media/dvb/frontends/cxd2820r_core.c
@@ -240,43 +240,6 @@ error:
 	return ret;
 }
 
-/* lock FE */
-static int cxd2820r_lock(struct cxd2820r_priv *priv, int active_fe)
-{
-	int ret = 0;
-	dbg("%s: active_fe=%d", __func__, active_fe);
-
-	mutex_lock(&priv->fe_lock);
-
-	/* -1=NONE, 0=DVB-T/T2, 1=DVB-C */
-	if (priv->active_fe == active_fe)
-		;
-	else if (priv->active_fe == -1)
-		priv->active_fe = active_fe;
-	else
-		ret = -EBUSY;
-
-	mutex_unlock(&priv->fe_lock);
-
-	return ret;
-}
-
-/* unlock FE */
-static void cxd2820r_unlock(struct cxd2820r_priv *priv, int active_fe)
-{
-	dbg("%s: active_fe=%d", __func__, active_fe);
-
-	mutex_lock(&priv->fe_lock);
-
-	/* -1=NONE, 0=DVB-T/T2, 1=DVB-C */
-	if (priv->active_fe == active_fe)
-		priv->active_fe = -1;
-
-	mutex_unlock(&priv->fe_lock);
-
-	return;
-}
-
 /* 64 bit div with round closest, like DIV_ROUND_CLOSEST but 64 bit */
 u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor)
 {
@@ -284,378 +247,230 @@ u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor)
 }
 
 static int cxd2820r_set_frontend(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *p)
+				 struct dvb_frontend_parameters *p)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (priv->delivery_system) {
-		case SYS_UNDEFINED:
-			if (c->delivery_system == SYS_DVBT) {
-				/* SLEEP => DVB-T */
-				ret = cxd2820r_set_frontend_t(fe, p);
-			} else {
-				/* SLEEP => DVB-T2 */
-				ret = cxd2820r_set_frontend_t2(fe, p);
-			}
-			break;
-		case SYS_DVBT:
-			if (c->delivery_system == SYS_DVBT) {
-				/* DVB-T => DVB-T */
-				ret = cxd2820r_set_frontend_t(fe, p);
-			} else if (c->delivery_system == SYS_DVBT2) {
-				/* DVB-T => DVB-T2 */
-				ret = cxd2820r_sleep_t(fe);
-				if (ret)
-					break;
-				ret = cxd2820r_set_frontend_t2(fe, p);
-			}
-			break;
-		case SYS_DVBT2:
-			if (c->delivery_system == SYS_DVBT2) {
-				/* DVB-T2 => DVB-T2 */
-				ret = cxd2820r_set_frontend_t2(fe, p);
-			} else if (c->delivery_system == SYS_DVBT) {
-				/* DVB-T2 => DVB-T */
-				ret = cxd2820r_sleep_t2(fe);
-				if (ret)
-					break;
-				ret = cxd2820r_set_frontend_t(fe, p);
-			}
-			break;
-		default:
-			dbg("%s: error state=%d", __func__,
-				priv->delivery_system);
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (c->delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_init_t(fe);
+		if (ret < 0)
+			goto err;
+		ret = cxd2820r_set_frontend_t(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_init_t(fe);
+		if (ret < 0)
+			goto err;
+		ret = cxd2820r_set_frontend_t2(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	case SYS_DVBC_ANNEX_AC:
+		ret = cxd2820r_init_c(fe);
+		if (ret < 0)
+			goto err;
 		ret = cxd2820r_set_frontend_c(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	default:
+		dbg("%s: error state=%d", __func__, fe->dtv_property_cache.delivery_system);
+		ret = -EINVAL;
+		break;
 	}
-
+err:
 	return ret;
 }
-
 static int cxd2820r_read_status(struct dvb_frontend *fe, fe_status_t *status)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_read_status_t(fe, status);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_read_status_t2(fe, status);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_read_status_t(fe, status);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_read_status_t2(fe, status);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_read_status_c(fe, status);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_get_frontend(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *p)
+				 struct dvb_frontend_parameters *p)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_get_frontend_t(fe, p);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_get_frontend_t2(fe, p);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_get_frontend_t(fe, p);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_get_frontend_t2(fe, p);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_get_frontend_c(fe, p);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_read_ber(struct dvb_frontend *fe, u32 *ber)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_read_ber_t(fe, ber);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_read_ber_t2(fe, ber);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_read_ber_t(fe, ber);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_read_ber_t2(fe, ber);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_read_ber_c(fe, ber);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_read_signal_strength(struct dvb_frontend *fe, u16 *strength)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_read_signal_strength_t(fe, strength);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_read_signal_strength_t2(fe, strength);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_read_signal_strength_t(fe, strength);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_read_signal_strength_t2(fe, strength);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_read_signal_strength_c(fe, strength);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_read_snr(struct dvb_frontend *fe, u16 *snr)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_read_snr_t(fe, snr);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_read_snr_t2(fe, snr);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_read_snr_t(fe, snr);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_read_snr_t2(fe, snr);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_read_snr_c(fe, snr);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_read_ucblocks_t(fe, ucblocks);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_read_ucblocks_t2(fe, ucblocks);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_read_ucblocks_t(fe, ucblocks);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_read_ucblocks_t2(fe, ucblocks);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_read_ucblocks_c(fe, ucblocks);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_init(struct dvb_frontend *fe)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
-	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	priv->delivery_system = SYS_UNDEFINED;
-	/* delivery system is unknown at that (init) phase */
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		ret = cxd2820r_init_t(fe);
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
-
-		ret = cxd2820r_init_c(fe);
-	}
-
-	return ret;
+	return 0;
 }
 
 static int cxd2820r_sleep(struct dvb_frontend *fe)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_sleep_t(fe);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_sleep_t2(fe);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-
-		cxd2820r_unlock(priv, 0);
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_sleep_t(fe);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_sleep_t2(fe);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_sleep_c(fe);
-
-		cxd2820r_unlock(priv, 1);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static int cxd2820r_get_tune_settings(struct dvb_frontend *fe,
-	struct dvb_frontend_tune_settings *s)
+				      struct dvb_frontend_tune_settings *s)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (fe->dtv_property_cache.delivery_system) {
-		case SYS_DVBT:
-			ret = cxd2820r_get_tune_settings_t(fe, s);
-			break;
-		case SYS_DVBT2:
-			ret = cxd2820r_get_tune_settings_t2(fe, s);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (fe->dtv_property_cache.delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_get_tune_settings_t(fe, s);
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_get_tune_settings_t2(fe, s);
+		break;
+	case SYS_DVBC_ANNEX_AC:
 		ret = cxd2820r_get_tune_settings_c(fe, s);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
 	}
-
 	return ret;
 }
 
 static enum dvbfe_search cxd2820r_search(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *p)
+					 struct dvb_frontend_parameters *p)
 {
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
@@ -664,7 +479,7 @@ static enum dvbfe_search cxd2820r_search(struct dvb_frontend *fe,
 	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
 
 	/* switch between DVB-T and DVB-T2 when tune fails */
-	if (priv->last_tune_failed) {
+	if (priv->last_tune_failed && (priv->delivery_system != SYS_DVBC_ANNEX_AC)) {
 		if (priv->delivery_system == SYS_DVBT)
 			c->delivery_system = SYS_DVBT2;
 		else
@@ -727,9 +542,7 @@ static void cxd2820r_release(struct dvb_frontend *fe)
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	dbg("%s", __func__);
 
-	if (fe->ops.info.type == FE_OFDM)
-		kfree(priv);
-
+	kfree(priv);
 	return;
 }
 
@@ -742,128 +555,98 @@ static int cxd2820r_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
 	return cxd2820r_wr_reg_mask(priv, 0xdb, enable ? 1 : 0, 0x1);
 }
 
-static const struct dvb_frontend_ops cxd2820r_ops[2];
-
-struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
-	struct i2c_adapter *i2c, struct dvb_frontend *fe)
+static int cxd2820r_get_property(struct dvb_frontend *fe, struct dtv_property *p)
 {
-	int ret;
-	struct cxd2820r_priv *priv = NULL;
-	u8 tmp;
+	dbg("%s()\n", __func__);
+
+	switch (p->cmd) {
+	case DTV_ENUM_DELSYS:
+		p->u.buffer.data[0] = SYS_DVBT;
+		p->u.buffer.data[1] = SYS_DVBT2;
+		p->u.buffer.data[2] = SYS_DVBC_ANNEX_AC;
+		p->u.buffer.len = 3;
+		break;
+	default:
+		break;
+	}
+	return 0;
+}
 
-	if (fe == NULL) {
-		/* FE0 */
-		/* allocate memory for the internal priv */
-		priv = kzalloc(sizeof(struct cxd2820r_priv), GFP_KERNEL);
-		if (priv == NULL)
-			goto error;
+static const struct dvb_frontend_ops cxd2820r_ops = {
+	/* default: DVB-T/T2 */
+	.info = {
+		.name = "Sony CXD2820R (DVB-T/T2)",
+		.type = FE_OFDM,
+
+		.caps =	FE_CAN_FEC_1_2			|
+			FE_CAN_FEC_2_3			|
+			FE_CAN_FEC_3_4			|
+			FE_CAN_FEC_5_6			|
+			FE_CAN_FEC_7_8			|
+			FE_CAN_FEC_AUTO			|
+			FE_CAN_QPSK			|
+			FE_CAN_QAM_16			|
+			FE_CAN_QAM_64			|
+			FE_CAN_QAM_256			|
+			FE_CAN_QAM_AUTO			|
+			FE_CAN_TRANSMISSION_MODE_AUTO	|
+			FE_CAN_GUARD_INTERVAL_AUTO	|
+			FE_CAN_HIERARCHY_AUTO		|
+			FE_CAN_MUTE_TS			|
+			FE_CAN_2G_MODULATION
+		},
 
-		/* setup the priv */
-		priv->i2c = i2c;
-		memcpy(&priv->cfg, cfg, sizeof(struct cxd2820r_config));
-		mutex_init(&priv->fe_lock);
+	.release		= cxd2820r_release,
+	.init			= cxd2820r_init,
+	.sleep			= cxd2820r_sleep,
 
-		priv->active_fe = -1; /* NONE */
+	.get_tune_settings	= cxd2820r_get_tune_settings,
+	.i2c_gate_ctrl		= cxd2820r_i2c_gate_ctrl,
 
-		/* check if the demod is there */
-		priv->bank[0] = priv->bank[1] = 0xff;
-		ret = cxd2820r_rd_reg(priv, 0x000fd, &tmp);
-		dbg("%s: chip id=%02x", __func__, tmp);
-		if (ret || tmp != 0xe1)
-			goto error;
+	.get_frontend		= cxd2820r_get_frontend,
 
-		/* create frontends */
-		memcpy(&priv->fe[0].ops, &cxd2820r_ops[0],
-			sizeof(struct dvb_frontend_ops));
-		memcpy(&priv->fe[1].ops, &cxd2820r_ops[1],
-			sizeof(struct dvb_frontend_ops));
+	.get_frontend_algo	= cxd2820r_get_frontend_algo,
+	.search			= cxd2820r_search,
 
-		priv->fe[0].demodulator_priv = priv;
-		priv->fe[1].demodulator_priv = priv;
+	.read_status		= cxd2820r_read_status,
+	.read_snr		= cxd2820r_read_snr,
+	.read_ber		= cxd2820r_read_ber,
+	.read_ucblocks		= cxd2820r_read_ucblocks,
+	.read_signal_strength	= cxd2820r_read_signal_strength,
 
-		return &priv->fe[0];
+	.get_property		= cxd2820r_get_property,
+};
 
-	} else {
-		/* FE1: FE0 given as pointer, just return FE1 we have
-		 * already created */
-		priv = fe->demodulator_priv;
-		return &priv->fe[1];
-	}
+struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
+				     struct i2c_adapter *i2c,
+				     struct dvb_frontend *fe)
+{
+	struct cxd2820r_priv *priv = NULL;
+	int ret;
+	u8 tmp;
+
+	priv = kzalloc(sizeof (struct cxd2820r_priv), GFP_KERNEL);
+	if (!priv)
+		goto error;
 
+	priv->i2c = i2c;
+	memcpy(&priv->cfg, cfg, sizeof (struct cxd2820r_config));
+
+	priv->bank[0] = priv->bank[1] = 0xff;
+	ret = cxd2820r_rd_reg(priv, 0x000fd, &tmp);
+	dbg("%s: chip id=%02x", __func__, tmp);
+	if (ret || tmp != 0xe1)
+		goto error;
+
+	memcpy(&priv->fe.ops, &cxd2820r_ops, sizeof (struct dvb_frontend_ops));
+	priv->fe.demodulator_priv = priv;
+	return &priv->fe;
 error:
 	kfree(priv);
 	return NULL;
 }
 EXPORT_SYMBOL(cxd2820r_attach);
 
-static const struct dvb_frontend_ops cxd2820r_ops[2] = {
-	{
-		/* DVB-T/T2 */
-		.info = {
-			.name = "Sony CXD2820R (DVB-T/T2)",
-			.type = FE_OFDM,
-			.caps =
-				FE_CAN_FEC_1_2 | FE_CAN_FEC_2_3 |
-				FE_CAN_FEC_3_4 | FE_CAN_FEC_5_6 |
-				FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
-				FE_CAN_QPSK | FE_CAN_QAM_16 |
-				FE_CAN_QAM_64 | FE_CAN_QAM_256 |
-				FE_CAN_QAM_AUTO |
-				FE_CAN_TRANSMISSION_MODE_AUTO |
-				FE_CAN_GUARD_INTERVAL_AUTO |
-				FE_CAN_HIERARCHY_AUTO |
-				FE_CAN_MUTE_TS |
-				FE_CAN_2G_MODULATION
-		},
-
-		.release = cxd2820r_release,
-		.init = cxd2820r_init,
-		.sleep = cxd2820r_sleep,
-
-		.get_tune_settings = cxd2820r_get_tune_settings,
-		.i2c_gate_ctrl = cxd2820r_i2c_gate_ctrl,
-
-		.get_frontend = cxd2820r_get_frontend,
-
-		.get_frontend_algo = cxd2820r_get_frontend_algo,
-		.search = cxd2820r_search,
-
-		.read_status = cxd2820r_read_status,
-		.read_snr = cxd2820r_read_snr,
-		.read_ber = cxd2820r_read_ber,
-		.read_ucblocks = cxd2820r_read_ucblocks,
-		.read_signal_strength = cxd2820r_read_signal_strength,
-	},
-	{
-		/* DVB-C */
-		.info = {
-			.name = "Sony CXD2820R (DVB-C)",
-			.type = FE_QAM,
-			.caps =
-				FE_CAN_QAM_16 | FE_CAN_QAM_32 | FE_CAN_QAM_64 |
-				FE_CAN_QAM_128 | FE_CAN_QAM_256 |
-				FE_CAN_FEC_AUTO
-		},
-
-		.release = cxd2820r_release,
-		.init = cxd2820r_init,
-		.sleep = cxd2820r_sleep,
-
-		.get_tune_settings = cxd2820r_get_tune_settings,
-		.i2c_gate_ctrl = cxd2820r_i2c_gate_ctrl,
-
-		.set_frontend = cxd2820r_set_frontend,
-		.get_frontend = cxd2820r_get_frontend,
-
-		.read_status = cxd2820r_read_status,
-		.read_snr = cxd2820r_read_snr,
-		.read_ber = cxd2820r_read_ber,
-		.read_ucblocks = cxd2820r_read_ucblocks,
-		.read_signal_strength = cxd2820r_read_signal_strength,
-	},
-};
-
-
 MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
 MODULE_DESCRIPTION("Sony CXD2820R demodulator driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb/frontends/cxd2820r_priv.h b/drivers/media/dvb/frontends/cxd2820r_priv.h
index 9553913..94dcf7f 100644
--- a/drivers/media/dvb/frontends/cxd2820r_priv.h
+++ b/drivers/media/dvb/frontends/cxd2820r_priv.h
@@ -48,12 +48,9 @@ struct reg_val_mask {
 
 struct cxd2820r_priv {
 	struct i2c_adapter *i2c;
-	struct dvb_frontend fe[2];
+	struct dvb_frontend fe;
 	struct cxd2820r_config cfg;
 
-	struct mutex fe_lock; /* FE lock */
-	int active_fe:2; /* FE lock, -1=NONE, 0=DVB-T/T2, 1=DVB-C */
-
 	bool ber_running;
 
 	u8 bank[2];
diff --git a/drivers/media/dvb/frontends/cxd2820r_t.c b/drivers/media/dvb/frontends/cxd2820r_t.c
index a04f9c8..d5ffc15 100644
--- a/drivers/media/dvb/frontends/cxd2820r_t.c
+++ b/drivers/media/dvb/frontends/cxd2820r_t.c
@@ -22,10 +22,11 @@
 #include "cxd2820r_priv.h"
 
 int cxd2820r_set_frontend_t(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *p)
+			    struct dvb_frontend_parameters *p)
 {
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct tuner_state tstate;
 	int ret, i;
 	u32 if_khz, if_ctl;
 	u64 num;
@@ -63,8 +64,20 @@ int cxd2820r_set_frontend_t(struct dvb_frontend *fe,
 		goto error;
 
 	/* program tuner */
-	if (fe->ops.tuner_ops.set_params)
-		fe->ops.tuner_ops.set_params(fe, p);
+	tstate.delsys = SYS_DVBT;
+	tstate.frequency = c->frequency;
+	tstate.bandwidth = c->bandwidth_hz;
+
+	if (fe->ops.tuner_ops.set_state) {
+		fe->ops.tuner_ops.set_state(fe,
+					    DVBFE_TUNER_DELSYS    |
+					    DVBFE_TUNER_FREQUENCY |
+					    DVBFE_TUNER_BANDWIDTH,
+					    &tstate);
+	} else {
+		if (fe->ops.tuner_ops.set_params)
+			fe->ops.tuner_ops.set_params(fe, p);
+	}
 
 	if (priv->delivery_system != SYS_DVBT) {
 		for (i = 0; i < ARRAY_SIZE(tab); i++) {
diff --git a/drivers/media/dvb/frontends/cxd2820r_t2.c b/drivers/media/dvb/frontends/cxd2820r_t2.c
index 6548588..a6c9903 100644
--- a/drivers/media/dvb/frontends/cxd2820r_t2.c
+++ b/drivers/media/dvb/frontends/cxd2820r_t2.c
@@ -22,10 +22,11 @@
 #include "cxd2820r_priv.h"
 
 int cxd2820r_set_frontend_t2(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *params)
+			     struct dvb_frontend_parameters *params)
 {
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	struct tuner_state tstate;
 	int ret, i;
 	u32 if_khz, if_ctl;
 	u64 num;
@@ -77,8 +78,21 @@ int cxd2820r_set_frontend_t2(struct dvb_frontend *fe,
 		goto error;
 
 	/* program tuner */
-	if (fe->ops.tuner_ops.set_params)
-		fe->ops.tuner_ops.set_params(fe, params);
+
+	tstate.delsys = SYS_DVBT2;
+	tstate.frequency = c->frequency;
+	tstate.bandwidth = c->bandwidth_hz;
+
+	if (fe->ops.tuner_ops.set_state) {
+		fe->ops.tuner_ops.set_state(fe,
+					    DVBFE_TUNER_DELSYS    |
+					    DVBFE_TUNER_FREQUENCY |
+					    DVBFE_TUNER_BANDWIDTH,
+					    &tstate);
+	} else {
+		if (fe->ops.tuner_ops.set_params)
+			fe->ops.tuner_ops.set_params(fe, params);
+	}
 
 	if (priv->delivery_system != SYS_DVBT2) {
 		for (i = 0; i < ARRAY_SIZE(tab); i++) {
-- 
1.7.1

Re: PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Antti Palosaari]

Hello

On 11/21/2011 11:09 PM, Manu Abraham wrote:
>   	/* program tuner */
> -	if (fe->ops.tuner_ops.set_params)
> -		fe->ops.tuner_ops.set_params(fe, params);
> +	tstate.delsys = SYS_DVBC_ANNEX_AC;
> +	tstate.frequency = c->frequency;
> +
> +	if (fe->ops.tuner_ops.set_state) {
> +		fe->ops.tuner_ops.set_state(fe,
> +					    DVBFE_TUNER_DELSYS    |
> +					    DVBFE_TUNER_FREQUENCY,
> +					&tstate);

I want to raise that point, switch from .set_params() to .set_state() 
when programming tuner. I don't see it reasonable to introduce (yes, it 
have existed ages but not used) new way to program tuner.

Both demod and tuner got same params;
.set_frontend(struct dvb_frontend *, struct dvb_frontend_parameters *)
.set_params(struct dvb_frontend *, struct dvb_frontend_parameters *)

and can get access to APIv5 property_cache similarly. Both, demod and 
tuner, can read all those params that are now passed using .set_state()

There is some new tuner drivers which are already using APIv5.


regards
Antti

-- 
http://palosaari.fi/

Re: PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Manu Abraham]

Hi,

On Tue, Nov 22, 2011 at 3:58 AM, Antti Palosaari <crope@iki.fi> wrote:
> Hello
>
> On 11/21/2011 11:09 PM, Manu Abraham wrote:
>>
>>        /* program tuner */
>> -       if (fe->ops.tuner_ops.set_params)
>> -               fe->ops.tuner_ops.set_params(fe, params);
>> +       tstate.delsys = SYS_DVBC_ANNEX_AC;
>> +       tstate.frequency = c->frequency;
>> +
>> +       if (fe->ops.tuner_ops.set_state) {
>> +               fe->ops.tuner_ops.set_state(fe,
>> +                                           DVBFE_TUNER_DELSYS    |
>> +                                           DVBFE_TUNER_FREQUENCY,
>> +                                       &tstate);
>
> I want to raise that point, switch from .set_params() to .set_state() when
> programming tuner. I don't see it reasonable to introduce (yes, it have
> existed ages but not used) new way to program tuner.


I didn't introduce set_state() now. It was there for quite a long
while, as old v5API itself, IIRC.


>
> Both demod and tuner got same params;
> .set_frontend(struct dvb_frontend *, struct dvb_frontend_parameters *)
> .set_params(struct dvb_frontend *, struct dvb_frontend_parameters *)


The argument passed to set_params: struct dvb_frontend_parameters is
useless for any device that's been around recently. Although one can
get the parameters from the property_cache.

Using set_state(), makes it independant and less kludgy, simplifying
things. on the other hand it may be used with analog as well, llly to
Michael Krufky said.

Eventually, it just provides the tuner an independence from struct
dvb_frontend_parameters (which is rigid) and the frontend cache.

That said, a few tuners already uses the mentioned callback, stb6100,
tda8261, tda665x,

If you imply that you feel overly depressed by the use of the
set_state in the cxd2820r module ;-), then as a workaround, the
parameters required for operation can be retrieved from the property
cache, but then if tuner drivers are cleaned up by someone to remove
obsolete ? set_params, then you wouldn't have any other option, but to
later on fall back to set_state.

I am fine with either way, but for the tuners themselves, set_state
behaves a bit more better as it provides independence from the legacy
dvb_frontend_properties. It takes a bit of time for someone new to
understand that (he cannot use dvb_frontend_properties anymore)


>
> and can get access to APIv5 property_cache similarly. Both, demod and tuner,
> can read all those params that are now passed using .set_state()


If you want to pass other parameters, as what exists already in
tuner_state, that is not possible with set_params. If you can't have
the required parameters through a parameters which is passed, but then
why would you want to have such a parameter itself passed in the first
case ?

>
> There is some new tuner drivers which are already using APIv5.
>
>
> regards
> Antti


Eventually it is all a matter of taste. I am fine with either. ;-)

Regards,
Manu

Re: PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Michael Krufky]

On Mon, Nov 21, 2011 at 6:01 PM, Manu Abraham <abraham.manu@gmail.com> wrote:
> Hi,
>
> On Tue, Nov 22, 2011 at 3:58 AM, Antti Palosaari <crope@iki.fi> wrote:
>> Hello
>>
>> On 11/21/2011 11:09 PM, Manu Abraham wrote:
>>>
>>>        /* program tuner */
>>> -       if (fe->ops.tuner_ops.set_params)
>>> -               fe->ops.tuner_ops.set_params(fe, params);
>>> +       tstate.delsys = SYS_DVBC_ANNEX_AC;
>>> +       tstate.frequency = c->frequency;
>>> +
>>> +       if (fe->ops.tuner_ops.set_state) {
>>> +               fe->ops.tuner_ops.set_state(fe,
>>> +                                           DVBFE_TUNER_DELSYS    |
>>> +                                           DVBFE_TUNER_FREQUENCY,
>>> +                                       &tstate);
>>
>> I want to raise that point, switch from .set_params() to .set_state() when
>> programming tuner. I don't see it reasonable to introduce (yes, it have
>> existed ages but not used) new way to program tuner.
>
>
> I didn't introduce set_state() now. It was there for quite a long
> while, as old v5API itself, IIRC.
>
>
>>
>> Both demod and tuner got same params;
>> .set_frontend(struct dvb_frontend *, struct dvb_frontend_parameters *)
>> .set_params(struct dvb_frontend *, struct dvb_frontend_parameters *)
>
>
> The argument passed to set_params: struct dvb_frontend_parameters is
> useless for any device that's been around recently. Although one can
> get the parameters from the property_cache.
>
> Using set_state(), makes it independant and less kludgy, simplifying
> things. on the other hand it may be used with analog as well, llly to
> Michael Krufky said.
>
> Eventually, it just provides the tuner an independence from struct
> dvb_frontend_parameters (which is rigid) and the frontend cache.
>
> That said, a few tuners already uses the mentioned callback, stb6100,
> tda8261, tda665x,
>
> If you imply that you feel overly depressed by the use of the
> set_state in the cxd2820r module ;-), then as a workaround, the
> parameters required for operation can be retrieved from the property
> cache, but then if tuner drivers are cleaned up by someone to remove
> obsolete ? set_params, then you wouldn't have any other option, but to
> later on fall back to set_state.
>
> I am fine with either way, but for the tuners themselves, set_state
> behaves a bit more better as it provides independence from the legacy
> dvb_frontend_properties. It takes a bit of time for someone new to
> understand that (he cannot use dvb_frontend_properties anymore)
>
>
>>
>> and can get access to APIv5 property_cache similarly. Both, demod and tuner,
>> can read all those params that are now passed using .set_state()
>
>
> If you want to pass other parameters, as what exists already in
> tuner_state, that is not possible with set_params. If you can't have
> the required parameters through a parameters which is passed, but then
> why would you want to have such a parameter itself passed in the first
> case ?
>
>>
>> There is some new tuner drivers which are already using APIv5.
>>
>>
>> regards
>> Antti
>
>
> Eventually it is all a matter of taste. I am fine with either. ;-)
>
> Regards,
> Manu


Antti is correct, this *can* be done by accessing the property cache,
and I would naturally agree with him that we should not add yet a 3rd
entry point for tuning.

However, this set_state is v4l/dvb agnostic.  if we go with this
set_state callback, we can in fact eliminate both set_params *and*
set_analog_params callbacks, finally having a single entry point for
setting the tuner.

If the community would prefer to use set_params, I am fine with
that... but I also like the idea of unifying the set_params and
set_analog_params into a single call.  If the community wants to see
*that*, then lets go ahead with set_state.  I think this is a good
step into that direction.

Agreeing with Manu, it is indeed a matter of taste and I am fine with
either way.  If we choose the set_state way, then future steps can
unify the calls into a single entry point -- that would be the best,
ultimately, in my opinion.

Regards,
Mike Krufky

Re: PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Antti Palosaari]

On 11/22/2011 01:13 AM, Michael Krufky wrote:
> On Mon, Nov 21, 2011 at 6:01 PM, Manu Abraham<abraham.manu@gmail.com>  wrote:
>> Hi,
>>
>> On Tue, Nov 22, 2011 at 3:58 AM, Antti Palosaari<crope@iki.fi>  wrote:
>>> Hello
>>>
>>> On 11/21/2011 11:09 PM, Manu Abraham wrote:
>>>>
>>>>         /* program tuner */
>>>> -       if (fe->ops.tuner_ops.set_params)
>>>> -               fe->ops.tuner_ops.set_params(fe, params);
>>>> +       tstate.delsys = SYS_DVBC_ANNEX_AC;
>>>> +       tstate.frequency = c->frequency;
>>>> +
>>>> +       if (fe->ops.tuner_ops.set_state) {
>>>> +               fe->ops.tuner_ops.set_state(fe,
>>>> +                                           DVBFE_TUNER_DELSYS    |
>>>> +                                           DVBFE_TUNER_FREQUENCY,
>>>> +&tstate);

That is useless for the CXD2820R demodulator driver adding only some 
more code.

>>> I want to raise that point, switch from .set_params() to .set_state() when
>>> programming tuner. I don't see it reasonable to introduce (yes, it have
>>> existed ages but not used) new way to program tuner.
>>
>>
>> I didn't introduce set_state() now. It was there for quite a long
>> while, as old v5API itself, IIRC.

I looked now through all driver as for Kernel 3.2. There was three tuner 
drivers implementing .set_state().

Driver name and after the driver name is field it was used.

stb6100:
frequency
bandwidth

tda665x:
frequency

tda8261:
frequency

>>> Both demod and tuner got same params;
>>> .set_frontend(struct dvb_frontend *, struct dvb_frontend_parameters *)
>>> .set_params(struct dvb_frontend *, struct dvb_frontend_parameters *)
>>
>>
>> The argument passed to set_params: struct dvb_frontend_parameters is
>> useless for any device that's been around recently. Although one can
>> get the parameters from the property_cache.
>>
>> Using set_state(), makes it independant and less kludgy, simplifying
>> things. on the other hand it may be used with analog as well, llly to
>> Michael Krufky said.
>>
>> Eventually, it just provides the tuner an independence from struct
>> dvb_frontend_parameters (which is rigid) and the frontend cache.

Making tuners independent from  DVB FE could be good reason. But if we 
end-up doing that I think it must be done as different issue. Design and 
when there is consensus about implementation then switch to that.

>> That said, a few tuners already uses the mentioned callback, stb6100,
>> tda8261, tda665x,

Those were the only ones I found when grepping. Could you say where 
those callbacks are called? For some reason I didn't find any.

> Antti is correct, this *can* be done by accessing the property cache,
> and I would naturally agree with him that we should not add yet a 3rd
> entry point for tuning.
>
> However, this set_state is v4l/dvb agnostic.  if we go with this
> set_state callback, we can in fact eliminate both set_params *and*
> set_analog_params callbacks, finally having a single entry point for
> setting the tuner.
>
> If the community would prefer to use set_params, I am fine with
> that... but I also like the idea of unifying the set_params and
> set_analog_params into a single call.  If the community wants to see
> *that*, then lets go ahead with set_state.  I think this is a good
> step into that direction.
>
> Agreeing with Manu, it is indeed a matter of taste and I am fine with
> either way.  If we choose the set_state way, then future steps can
> unify the calls into a single entry point -- that would be the best,
> ultimately, in my opinion.

I see it is better to use DVB APIv5 as for now until there is better 
choice. Not making more unneeded deviation between tuner and demod drivers.


regards
Antti

-- 
http://palosaari.fi/

Re: PATCH 12/13: 0012-CXD2820r-Query-DVB-frontend-delivery-capabilities

[Mauro Carvalho Chehab]

Em 21-11-2011 21:01, Manu Abraham escreveu:
> Hi,
> 
> On Tue, Nov 22, 2011 at 3:58 AM, Antti Palosaari <crope@iki.fi> wrote:
>> Hello
>>
>> On 11/21/2011 11:09 PM, Manu Abraham wrote:
>>>
>>>        /* program tuner */
>>> -       if (fe->ops.tuner_ops.set_params)
>>> -               fe->ops.tuner_ops.set_params(fe, params);
>>> +       tstate.delsys = SYS_DVBC_ANNEX_AC;
>>> +       tstate.frequency = c->frequency;
>>> +
>>> +       if (fe->ops.tuner_ops.set_state) {
>>> +               fe->ops.tuner_ops.set_state(fe,
>>> +                                           DVBFE_TUNER_DELSYS    |
>>> +                                           DVBFE_TUNER_FREQUENCY,
>>> +                                       &tstate);
>>
>> I want to raise that point, switch from .set_params() to .set_state() when
>> programming tuner. I don't see it reasonable to introduce (yes, it have
>> existed ages but not used) new way to program tuner.
> 
> 
> I didn't introduce set_state() now. It was there for quite a long
> while, as old v5API itself, IIRC.
> 
> 
>>
>> Both demod and tuner got same params;
>> .set_frontend(struct dvb_frontend *, struct dvb_frontend_parameters *)
>> .set_params(struct dvb_frontend *, struct dvb_frontend_parameters *)
> 
> 
> The argument passed to set_params: struct dvb_frontend_parameters is
> useless for any device that's been around recently. Although one can
> get the parameters from the property_cache.

I like the idea of getting rid of struct dvb_frontend_parameters.
> 
> Using set_state(), makes it independant and less kludgy, simplifying
> things. on the other hand it may be used with analog as well, llly to
> Michael Krufky said.
> 
> Eventually, it just provides the tuner an independence from struct
> dvb_frontend_parameters (which is rigid) and the frontend cache.
> 
> That said, a few tuners already uses the mentioned callback, stb6100,
> tda8261, tda665x,

Hmm... while enum tuner_param is defined as a bitmask, stb6100 implements
it as:

static int stb6100_set_state(struct dvb_frontend *fe,
			     enum tuner_param param,
			     struct tuner_state *state)
{
	struct stb6100_state *tstate = fe->tuner_priv;

	switch (param) {
	case DVBFE_TUNER_FREQUENCY:
		stb6100_set_frequency(fe, state->frequency);
		tstate->frequency = state->frequency;
		break;
	case DVBFE_TUNER_TUNERSTEP:

And get_state current implementations return only one value each time.

I agree with Antti: the way it is designed doesn't seem to help much,
for a few reasons:

1) at get, one call is needed for each parameter, on the current drivers.
   Not a big issue, as patches fixing it may be added anytime.
2) The enum tuner_param field is limited to 32 bits. Currently, DTV_MAX_COMMAND
   is equal to 44. Ok, a tuner in general only need a small subset of the
   parameters, but 32 bits may starve too fast, depending on how complex
   will be the tuner staging for upcoming standards.
3) making tuner DVB or V4L agnostic would mean that the data would need
   to be copied from/to the tuner_state struct. The current struct has 6
   parameters. Patch 3/13 added 2 more parameters. If we pass the bandwidth
   calculus for DVB-S/S2/C/C2 to the frontend, rollback and symbol_rate would
   also needed to be there. Other data that is inside dvb_frontend would also
   require to be copied. For Analog standards, the analog parameters would
   also need to be there (like, for example, the analog standard).

I'm in favor of not trying to merge analog and dvb parameter setting.

I can see two approaches:

1) a simpler interface, like:

static int set_foo(struct dvb_frontend *fe);
	and
static int get_foo(struct dvb_frontend *fe);

2) a real cache-based struct:

Add some bitmap fields at struct dtv_frontend_properties in order to
indicate what parameters are new, on a set operations, and what
parameters are dirty (e. g. were modified by the tuner) on return.


This way, a call like:

err = tuner_ops->set_state(fe, DVBFE_TUNER_FREQUENCY, &state);

could still be done using the new interface, allowing drivers to just
set the frequency and preserving the rest.

This would also reduce the amount of time needed to flush data from/to
the cache inside the core (if needed). 

I suspect, however, that the code inside the core will reduce a lot if we 
get rid of struct dvb_frontend_parameters inside the demods/tuners.
So, IMHO, approach (1) is better.

> If you imply that you feel overly depressed by the use of the
> set_state in the cxd2820r module ;-), then as a workaround, the
> parameters required for operation can be retrieved from the property
> cache, but then if tuner drivers are cleaned up by someone to remove
> obsolete ? set_params, then you wouldn't have any other option, but to
> later on fall back to set_state.
> 
> I am fine with either way, but for the tuners themselves, set_state
> behaves a bit more better as it provides independence from the legacy
> dvb_frontend_properties. It takes a bit of time for someone new to
> understand that (he cannot use dvb_frontend_properties anymore)
> 
> 
>>
>> and can get access to APIv5 property_cache similarly. Both, demod and tuner,
>> can read all those params that are now passed using .set_state()
> 
> 
> If you want to pass other parameters, as what exists already in
> tuner_state, that is not possible with set_params. If you can't have
> the required parameters through a parameters which is passed, but then
> why would you want to have such a parameter itself passed in the first
> case ?
> 
>>
>> There is some new tuner drivers which are already using APIv5.
>>
>>
>> regards
>> Antti
> 
> 
> Eventually it is all a matter of taste. I am fine with either. ;-)
> 
> Regards,
> Manu
> --
> 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