public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
@ 2013-10-02 22:09 Luis Alves
  2013-10-02 22:16 ` Mauro Carvalho Chehab
  2013-10-02 22:37 ` Antti Palosaari
  0 siblings, 2 replies; 5+ messages in thread
From: Luis Alves @ 2013-10-02 22:09 UTC (permalink / raw)
  To: mkrufky; +Cc: crope, linux-media, mchehab, Luis Alves

Hi Mike,

It's done (also tested and apparently working good)!

I didn't know if two separated patches were needed (one for the cx24117 and the other for the cx23885) but I've splited it.
As you pointed out, this series of patches are to be used against your cx24117 branch.

Regards,
Luis

Signed-off-by: Luis Alves <ljalvs@gmail.com>
---
 drivers/media/dvb-frontends/cx24117.c |   72 +++++++++++++++++++++++----------
 drivers/media/dvb-frontends/cx24117.h |    4 +-
 2 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/drivers/media/dvb-frontends/cx24117.c b/drivers/media/dvb-frontends/cx24117.c
index 3b63913..9087309 100644
--- a/drivers/media/dvb-frontends/cx24117.c
+++ b/drivers/media/dvb-frontends/cx24117.c
@@ -31,6 +31,7 @@
 #include <linux/init.h>
 #include <linux/firmware.h>
 
+#include "tuner-i2c.h"
 #include "dvb_frontend.h"
 #include "cx24117.h"
 
@@ -145,6 +146,9 @@ enum cmds {
 	CMD_TUNERSLEEP  = 0x36,
 };
 
+static LIST_HEAD(hybrid_tuner_instance_list);
+static DEFINE_MUTEX(cx24117_list_mutex);
+
 /* The Demod/Tuner can't easily provide these, we cache them */
 struct cx24117_tuning {
 	u32 frequency;
@@ -176,9 +180,11 @@ struct cx24117_priv {
 	u8 demod_address;
 	struct i2c_adapter *i2c;
 	u8 skip_fw_load;
-
 	struct mutex fe_lock;
-	atomic_t fe_nr;
+
+	/* Used for sharing this struct between demods */
+	struct tuner_i2c_props i2c_props;
+	struct list_head hybrid_tuner_instance_list;
 };
 
 /* one per each fe */
@@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
 	dev_dbg(&state->priv->i2c->dev,
 		"%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
 		__func__, state->demod, fw->size, fw->data[0], fw->data[1],
-		fw->data[fw->size-2], fw->data[fw->size-1]);
+		fw->data[fw->size - 2], fw->data[fw->size - 1]);
 
 	cx24117_writereg(state, 0xea, 0x00);
 	cx24117_writereg(state, 0xea, 0x01);
@@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct dvb_frontend *fe,
 	return 0;
 }
 
+static int cx24117_get_priv(struct cx24117_priv **priv,
+	struct i2c_adapter *i2c, u8 client_address)
+{
+	int ret;
+
+	mutex_lock(&cx24117_list_mutex);
+	ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
+		hybrid_tuner_instance_list, i2c, client_address, "cx24117");
+	mutex_unlock(&cx24117_list_mutex);
+
+	return ret;
+}
+
+static void cx24117_release_priv(struct cx24117_priv *priv)
+{
+	mutex_lock(&cx24117_list_mutex);
+	if (priv != NULL)
+		hybrid_tuner_release_state(priv);
+	mutex_unlock(&cx24117_list_mutex);
+}
+
 static void cx24117_release(struct dvb_frontend *fe)
 {
 	struct cx24117_state *state = fe->demodulator_priv;
 	dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
 		__func__, state->demod);
-	if (!atomic_dec_and_test(&state->priv->fe_nr))
-		kfree(state->priv);
+	cx24117_release_priv(state->priv);
 	kfree(state);
 }
 
 static struct dvb_frontend_ops cx24117_ops;
 
 struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
-	struct i2c_adapter *i2c, struct dvb_frontend *fe)
+	struct i2c_adapter *i2c)
 {
 	struct cx24117_state *state = NULL;
 	struct cx24117_priv *priv = NULL;
 	int demod = 0;
 
-	/* first frontend attaching */
-	/* allocate shared priv struct */
-	if (fe == NULL) {
-		priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
-		if (priv == NULL)
-			goto error1;
+	/* get the common data struct for both demods */
+	demod = cx24117_get_priv(&priv, i2c, config->demod_address);
+
+	switch (demod) {
+	case 0:
+		dev_err(&state->priv->i2c->dev,
+			"%s: Error attaching frontend %d\n",
+			KBUILD_MODNAME, demod);
+		goto error1;
+		break;
+	case 1:
+		/* new priv instance */
 		priv->i2c = i2c;
 		priv->demod_address = config->demod_address;
 		mutex_init(&priv->fe_lock);
-	} else {
-		demod = 1;
-		priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
+		break;
+	default:
+		/* existing priv instance */
+		break;
 	}
 
 	/* allocate memory for the internal state */
@@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
 	if (state == NULL)
 		goto error2;
 
-	state->demod = demod;
+	state->demod = demod - 1;
 	state->priv = priv;
 
 	/* test i2c bus for ack */
@@ -1163,12 +1196,9 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
 			goto error3;
 	}
 
-	/* nr of frontends using the module */
-	atomic_inc(&priv->fe_nr);
-
 	dev_info(&state->priv->i2c->dev,
 		"%s: Attaching frontend %d\n",
-		KBUILD_MODNAME, demod);
+		KBUILD_MODNAME, state->demod);
 
 	/* create dvb_frontend */
 	memcpy(&state->frontend.ops, &cx24117_ops,
@@ -1179,7 +1209,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
 error3:
 	kfree(state);
 error2:
-	kfree(priv);
+	cx24117_release_priv(priv);
 error1:
 	return NULL;
 }
diff --git a/drivers/media/dvb-frontends/cx24117.h b/drivers/media/dvb-frontends/cx24117.h
index 5bc8f11..4e59e95 100644
--- a/drivers/media/dvb-frontends/cx24117.h
+++ b/drivers/media/dvb-frontends/cx24117.h
@@ -33,11 +33,11 @@ struct cx24117_config {
 #if IS_ENABLED(CONFIG_DVB_CX24117)
 extern struct dvb_frontend *cx24117_attach(
 	const struct cx24117_config *config,
-	struct i2c_adapter *i2c, struct dvb_frontend *fe);
+	struct i2c_adapter *i2c);
 #else
 static inline struct dvb_frontend *cx24117_attach(
 	const struct cx24117_config *config,
-	struct i2c_adapter *i2c, struct dvb_frontend *fe)
+	struct i2c_adapter *i2c)
 {
 	dev_warn(&i2c->dev, "%s: driver disabled by Kconfig\n", __func__);
 	return NULL;
-- 
1.7.9.5


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

* Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
  2013-10-02 22:09 [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod Luis Alves
@ 2013-10-02 22:16 ` Mauro Carvalho Chehab
  2013-10-02 22:37 ` Antti Palosaari
  1 sibling, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2013-10-02 22:16 UTC (permalink / raw)
  To: Luis Alves; +Cc: mkrufky, crope, linux-media

Em Wed,  2 Oct 2013 23:09:11 +0100
Luis Alves <ljalvs@gmail.com> escreveu:

> Hi Mike,
> 
> It's done (also tested and apparently working good)!
> 
> I didn't know if two separated patches were needed (one for the cx24117 and the other for the cx23885) but I've splited it.

If the patch causes regression or breaks compilation, it should be folded.

I suspect that this is the case, as you're changing the interface here.

> As you pointed out, this series of patches are to be used against your cx24117 branch.
> 
> Regards,
> Luis
> 
> Signed-off-by: Luis Alves <ljalvs@gmail.com>
> ---
>  drivers/media/dvb-frontends/cx24117.c |   72 +++++++++++++++++++++++----------
>  drivers/media/dvb-frontends/cx24117.h |    4 +-
>  2 files changed, 53 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/cx24117.c b/drivers/media/dvb-frontends/cx24117.c
> index 3b63913..9087309 100644
> --- a/drivers/media/dvb-frontends/cx24117.c
> +++ b/drivers/media/dvb-frontends/cx24117.c
> @@ -31,6 +31,7 @@
>  #include <linux/init.h>
>  #include <linux/firmware.h>
>  
> +#include "tuner-i2c.h"
>  #include "dvb_frontend.h"
>  #include "cx24117.h"
>  
> @@ -145,6 +146,9 @@ enum cmds {
>  	CMD_TUNERSLEEP  = 0x36,
>  };
>  
> +static LIST_HEAD(hybrid_tuner_instance_list);
> +static DEFINE_MUTEX(cx24117_list_mutex);
> +
>  /* The Demod/Tuner can't easily provide these, we cache them */
>  struct cx24117_tuning {
>  	u32 frequency;
> @@ -176,9 +180,11 @@ struct cx24117_priv {
>  	u8 demod_address;
>  	struct i2c_adapter *i2c;
>  	u8 skip_fw_load;
> -
>  	struct mutex fe_lock;
> -	atomic_t fe_nr;
> +
> +	/* Used for sharing this struct between demods */
> +	struct tuner_i2c_props i2c_props;
> +	struct list_head hybrid_tuner_instance_list;
>  };
>  
>  /* one per each fe */
> @@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
>  	dev_dbg(&state->priv->i2c->dev,
>  		"%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
>  		__func__, state->demod, fw->size, fw->data[0], fw->data[1],
> -		fw->data[fw->size-2], fw->data[fw->size-1]);
> +		fw->data[fw->size - 2], fw->data[fw->size - 1]);
>  
>  	cx24117_writereg(state, 0xea, 0x00);
>  	cx24117_writereg(state, 0xea, 0x01);
> @@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct dvb_frontend *fe,
>  	return 0;
>  }
>  
> +static int cx24117_get_priv(struct cx24117_priv **priv,
> +	struct i2c_adapter *i2c, u8 client_address)
> +{
> +	int ret;
> +
> +	mutex_lock(&cx24117_list_mutex);
> +	ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
> +		hybrid_tuner_instance_list, i2c, client_address, "cx24117");
> +	mutex_unlock(&cx24117_list_mutex);
> +
> +	return ret;
> +}
> +
> +static void cx24117_release_priv(struct cx24117_priv *priv)
> +{
> +	mutex_lock(&cx24117_list_mutex);
> +	if (priv != NULL)
> +		hybrid_tuner_release_state(priv);
> +	mutex_unlock(&cx24117_list_mutex);
> +}
> +
>  static void cx24117_release(struct dvb_frontend *fe)
>  {
>  	struct cx24117_state *state = fe->demodulator_priv;
>  	dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
>  		__func__, state->demod);
> -	if (!atomic_dec_and_test(&state->priv->fe_nr))
> -		kfree(state->priv);
> +	cx24117_release_priv(state->priv);
>  	kfree(state);
>  }
>  
>  static struct dvb_frontend_ops cx24117_ops;
>  
>  struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe)
> +	struct i2c_adapter *i2c)
>  {
>  	struct cx24117_state *state = NULL;
>  	struct cx24117_priv *priv = NULL;
>  	int demod = 0;
>  
> -	/* first frontend attaching */
> -	/* allocate shared priv struct */
> -	if (fe == NULL) {
> -		priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
> -		if (priv == NULL)
> -			goto error1;
> +	/* get the common data struct for both demods */
> +	demod = cx24117_get_priv(&priv, i2c, config->demod_address);
> +
> +	switch (demod) {
> +	case 0:
> +		dev_err(&state->priv->i2c->dev,
> +			"%s: Error attaching frontend %d\n",
> +			KBUILD_MODNAME, demod);
> +		goto error1;
> +		break;
> +	case 1:
> +		/* new priv instance */
>  		priv->i2c = i2c;
>  		priv->demod_address = config->demod_address;
>  		mutex_init(&priv->fe_lock);
> -	} else {
> -		demod = 1;
> -		priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
> +		break;
> +	default:
> +		/* existing priv instance */
> +		break;
>  	}
>  
>  	/* allocate memory for the internal state */
> @@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>  	if (state == NULL)
>  		goto error2;
>  
> -	state->demod = demod;
> +	state->demod = demod - 1;
>  	state->priv = priv;
>  
>  	/* test i2c bus for ack */
> @@ -1163,12 +1196,9 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>  			goto error3;
>  	}
>  
> -	/* nr of frontends using the module */
> -	atomic_inc(&priv->fe_nr);
> -
>  	dev_info(&state->priv->i2c->dev,
>  		"%s: Attaching frontend %d\n",
> -		KBUILD_MODNAME, demod);
> +		KBUILD_MODNAME, state->demod);
>  
>  	/* create dvb_frontend */
>  	memcpy(&state->frontend.ops, &cx24117_ops,
> @@ -1179,7 +1209,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>  error3:
>  	kfree(state);
>  error2:
> -	kfree(priv);
> +	cx24117_release_priv(priv);
>  error1:
>  	return NULL;
>  }
> diff --git a/drivers/media/dvb-frontends/cx24117.h b/drivers/media/dvb-frontends/cx24117.h
> index 5bc8f11..4e59e95 100644
> --- a/drivers/media/dvb-frontends/cx24117.h
> +++ b/drivers/media/dvb-frontends/cx24117.h
> @@ -33,11 +33,11 @@ struct cx24117_config {
>  #if IS_ENABLED(CONFIG_DVB_CX24117)
>  extern struct dvb_frontend *cx24117_attach(
>  	const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe);
> +	struct i2c_adapter *i2c);
>  #else
>  static inline struct dvb_frontend *cx24117_attach(
>  	const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe)
> +	struct i2c_adapter *i2c)
>  {
>  	dev_warn(&i2c->dev, "%s: driver disabled by Kconfig\n", __func__);
>  	return NULL;




Cheers,
Mauro

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

* Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
  2013-10-02 22:09 [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod Luis Alves
  2013-10-02 22:16 ` Mauro Carvalho Chehab
@ 2013-10-02 22:37 ` Antti Palosaari
  2013-10-02 22:54   ` Luis Alves
  1 sibling, 1 reply; 5+ messages in thread
From: Antti Palosaari @ 2013-10-02 22:37 UTC (permalink / raw)
  To: Luis Alves, mkrufky; +Cc: linux-media, mchehab

On 03.10.2013 01:09, Luis Alves wrote:
> Hi Mike,
>
> It's done (also tested and apparently working good)!
>
> I didn't know if two separated patches were needed (one for the cx24117 and the other for the cx23885) but I've splited it.
> As you pointed out, this series of patches are to be used against your cx24117 branch.
>
> Regards,
> Luis
>
> Signed-off-by: Luis Alves <ljalvs@gmail.com>

I am not very familiar how that hybrid tuner request works, but it seems 
to be based of driver global list.

I wonder if that works as it should. What happens when you have two 
cx24117 demods equipped, having total of 4 frontends? Does it block 
access only for one demod at the time (as it should block only one per 
chip)?

regards
Antti


> ---
>   drivers/media/dvb-frontends/cx24117.c |   72 +++++++++++++++++++++++----------
>   drivers/media/dvb-frontends/cx24117.h |    4 +-
>   2 files changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/cx24117.c b/drivers/media/dvb-frontends/cx24117.c
> index 3b63913..9087309 100644
> --- a/drivers/media/dvb-frontends/cx24117.c
> +++ b/drivers/media/dvb-frontends/cx24117.c
> @@ -31,6 +31,7 @@
>   #include <linux/init.h>
>   #include <linux/firmware.h>
>
> +#include "tuner-i2c.h"
>   #include "dvb_frontend.h"
>   #include "cx24117.h"
>
> @@ -145,6 +146,9 @@ enum cmds {
>   	CMD_TUNERSLEEP  = 0x36,
>   };
>
> +static LIST_HEAD(hybrid_tuner_instance_list);
> +static DEFINE_MUTEX(cx24117_list_mutex);
> +
>   /* The Demod/Tuner can't easily provide these, we cache them */
>   struct cx24117_tuning {
>   	u32 frequency;
> @@ -176,9 +180,11 @@ struct cx24117_priv {
>   	u8 demod_address;
>   	struct i2c_adapter *i2c;
>   	u8 skip_fw_load;
> -
>   	struct mutex fe_lock;
> -	atomic_t fe_nr;
> +
> +	/* Used for sharing this struct between demods */
> +	struct tuner_i2c_props i2c_props;
> +	struct list_head hybrid_tuner_instance_list;
>   };
>
>   /* one per each fe */
> @@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend *fe,
>   	dev_dbg(&state->priv->i2c->dev,
>   		"%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
>   		__func__, state->demod, fw->size, fw->data[0], fw->data[1],
> -		fw->data[fw->size-2], fw->data[fw->size-1]);
> +		fw->data[fw->size - 2], fw->data[fw->size - 1]);
>
>   	cx24117_writereg(state, 0xea, 0x00);
>   	cx24117_writereg(state, 0xea, 0x01);
> @@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct dvb_frontend *fe,
>   	return 0;
>   }
>
> +static int cx24117_get_priv(struct cx24117_priv **priv,
> +	struct i2c_adapter *i2c, u8 client_address)
> +{
> +	int ret;
> +
> +	mutex_lock(&cx24117_list_mutex);
> +	ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
> +		hybrid_tuner_instance_list, i2c, client_address, "cx24117");
> +	mutex_unlock(&cx24117_list_mutex);
> +
> +	return ret;
> +}
> +
> +static void cx24117_release_priv(struct cx24117_priv *priv)
> +{
> +	mutex_lock(&cx24117_list_mutex);
> +	if (priv != NULL)
> +		hybrid_tuner_release_state(priv);
> +	mutex_unlock(&cx24117_list_mutex);
> +}
> +
>   static void cx24117_release(struct dvb_frontend *fe)
>   {
>   	struct cx24117_state *state = fe->demodulator_priv;
>   	dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
>   		__func__, state->demod);
> -	if (!atomic_dec_and_test(&state->priv->fe_nr))
> -		kfree(state->priv);
> +	cx24117_release_priv(state->priv);
>   	kfree(state);
>   }
>
>   static struct dvb_frontend_ops cx24117_ops;
>
>   struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe)
> +	struct i2c_adapter *i2c)
>   {
>   	struct cx24117_state *state = NULL;
>   	struct cx24117_priv *priv = NULL;
>   	int demod = 0;
>
> -	/* first frontend attaching */
> -	/* allocate shared priv struct */
> -	if (fe == NULL) {
> -		priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
> -		if (priv == NULL)
> -			goto error1;
> +	/* get the common data struct for both demods */
> +	demod = cx24117_get_priv(&priv, i2c, config->demod_address);
> +
> +	switch (demod) {
> +	case 0:
> +		dev_err(&state->priv->i2c->dev,
> +			"%s: Error attaching frontend %d\n",
> +			KBUILD_MODNAME, demod);
> +		goto error1;
> +		break;
> +	case 1:
> +		/* new priv instance */
>   		priv->i2c = i2c;
>   		priv->demod_address = config->demod_address;
>   		mutex_init(&priv->fe_lock);
> -	} else {
> -		demod = 1;
> -		priv = ((struct cx24117_state *) fe->demodulator_priv)->priv;
> +		break;
> +	default:
> +		/* existing priv instance */
> +		break;
>   	}
>
>   	/* allocate memory for the internal state */
> @@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>   	if (state == NULL)
>   		goto error2;
>
> -	state->demod = demod;
> +	state->demod = demod - 1;
>   	state->priv = priv;
>
>   	/* test i2c bus for ack */
> @@ -1163,12 +1196,9 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>   			goto error3;
>   	}
>
> -	/* nr of frontends using the module */
> -	atomic_inc(&priv->fe_nr);
> -
>   	dev_info(&state->priv->i2c->dev,
>   		"%s: Attaching frontend %d\n",
> -		KBUILD_MODNAME, demod);
> +		KBUILD_MODNAME, state->demod);
>
>   	/* create dvb_frontend */
>   	memcpy(&state->frontend.ops, &cx24117_ops,
> @@ -1179,7 +1209,7 @@ struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>   error3:
>   	kfree(state);
>   error2:
> -	kfree(priv);
> +	cx24117_release_priv(priv);
>   error1:
>   	return NULL;
>   }
> diff --git a/drivers/media/dvb-frontends/cx24117.h b/drivers/media/dvb-frontends/cx24117.h
> index 5bc8f11..4e59e95 100644
> --- a/drivers/media/dvb-frontends/cx24117.h
> +++ b/drivers/media/dvb-frontends/cx24117.h
> @@ -33,11 +33,11 @@ struct cx24117_config {
>   #if IS_ENABLED(CONFIG_DVB_CX24117)
>   extern struct dvb_frontend *cx24117_attach(
>   	const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe);
> +	struct i2c_adapter *i2c);
>   #else
>   static inline struct dvb_frontend *cx24117_attach(
>   	const struct cx24117_config *config,
> -	struct i2c_adapter *i2c, struct dvb_frontend *fe)
> +	struct i2c_adapter *i2c)
>   {
>   	dev_warn(&i2c->dev, "%s: driver disabled by Kconfig\n", __func__);
>   	return NULL;
>


-- 
http://palosaari.fi/

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

* Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
  2013-10-02 22:37 ` Antti Palosaari
@ 2013-10-02 22:54   ` Luis Alves
  2013-10-02 23:16     ` Antti Palosaari
  0 siblings, 1 reply; 5+ messages in thread
From: Luis Alves @ 2013-10-02 22:54 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: Michael Krufky, linux-media, mchehab

I Antti,

I think it's safe to use because the hybrid_tuner_request_state will
make sure that the i2c_adapter_id is the same for both demods.

On the other hand, I think I need to re-send this changes as one single file.

Regards,
Luis


2013/10/2 Antti Palosaari <crope@iki.fi>:
> On 03.10.2013 01:09, Luis Alves wrote:
>>
>> Hi Mike,
>>
>> It's done (also tested and apparently working good)!
>>
>> I didn't know if two separated patches were needed (one for the cx24117
>> and the other for the cx23885) but I've splited it.
>> As you pointed out, this series of patches are to be used against your
>> cx24117 branch.
>>
>> Regards,
>> Luis
>>
>> Signed-off-by: Luis Alves <ljalvs@gmail.com>
>
>
> I am not very familiar how that hybrid tuner request works, but it seems to
> be based of driver global list.
>
> I wonder if that works as it should. What happens when you have two cx24117
> demods equipped, having total of 4 frontends? Does it block access only for
> one demod at the time (as it should block only one per chip)?
>
> regards
> Antti
>
>
>
>> ---
>>   drivers/media/dvb-frontends/cx24117.c |   72
>> +++++++++++++++++++++++----------
>>   drivers/media/dvb-frontends/cx24117.h |    4 +-
>>   2 files changed, 53 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/media/dvb-frontends/cx24117.c
>> b/drivers/media/dvb-frontends/cx24117.c
>> index 3b63913..9087309 100644
>> --- a/drivers/media/dvb-frontends/cx24117.c
>> +++ b/drivers/media/dvb-frontends/cx24117.c
>> @@ -31,6 +31,7 @@
>>   #include <linux/init.h>
>>   #include <linux/firmware.h>
>>
>> +#include "tuner-i2c.h"
>>   #include "dvb_frontend.h"
>>   #include "cx24117.h"
>>
>> @@ -145,6 +146,9 @@ enum cmds {
>>         CMD_TUNERSLEEP  = 0x36,
>>   };
>>
>> +static LIST_HEAD(hybrid_tuner_instance_list);
>> +static DEFINE_MUTEX(cx24117_list_mutex);
>> +
>>   /* The Demod/Tuner can't easily provide these, we cache them */
>>   struct cx24117_tuning {
>>         u32 frequency;
>> @@ -176,9 +180,11 @@ struct cx24117_priv {
>>         u8 demod_address;
>>         struct i2c_adapter *i2c;
>>         u8 skip_fw_load;
>> -
>>         struct mutex fe_lock;
>> -       atomic_t fe_nr;
>> +
>> +       /* Used for sharing this struct between demods */
>> +       struct tuner_i2c_props i2c_props;
>> +       struct list_head hybrid_tuner_instance_list;
>>   };
>>
>>   /* one per each fe */
>> @@ -536,7 +542,7 @@ static int cx24117_load_firmware(struct dvb_frontend
>> *fe,
>>         dev_dbg(&state->priv->i2c->dev,
>>                 "%s() demod%d FW is %zu bytes (%02x %02x .. %02x %02x)\n",
>>                 __func__, state->demod, fw->size, fw->data[0],
>> fw->data[1],
>> -               fw->data[fw->size-2], fw->data[fw->size-1]);
>> +               fw->data[fw->size - 2], fw->data[fw->size - 1]);
>>
>>         cx24117_writereg(state, 0xea, 0x00);
>>         cx24117_writereg(state, 0xea, 0x01);
>> @@ -1116,37 +1122,64 @@ static int cx24117_diseqc_send_burst(struct
>> dvb_frontend *fe,
>>         return 0;
>>   }
>>
>> +static int cx24117_get_priv(struct cx24117_priv **priv,
>> +       struct i2c_adapter *i2c, u8 client_address)
>> +{
>> +       int ret;
>> +
>> +       mutex_lock(&cx24117_list_mutex);
>> +       ret = hybrid_tuner_request_state(struct cx24117_priv, (*priv),
>> +               hybrid_tuner_instance_list, i2c, client_address,
>> "cx24117");
>> +       mutex_unlock(&cx24117_list_mutex);
>> +
>> +       return ret;
>> +}
>> +
>> +static void cx24117_release_priv(struct cx24117_priv *priv)
>> +{
>> +       mutex_lock(&cx24117_list_mutex);
>> +       if (priv != NULL)
>> +               hybrid_tuner_release_state(priv);
>> +       mutex_unlock(&cx24117_list_mutex);
>> +}
>> +
>>   static void cx24117_release(struct dvb_frontend *fe)
>>   {
>>         struct cx24117_state *state = fe->demodulator_priv;
>>         dev_dbg(&state->priv->i2c->dev, "%s demod%d\n",
>>                 __func__, state->demod);
>> -       if (!atomic_dec_and_test(&state->priv->fe_nr))
>> -               kfree(state->priv);
>> +       cx24117_release_priv(state->priv);
>>         kfree(state);
>>   }
>>
>>   static struct dvb_frontend_ops cx24117_ops;
>>
>>   struct dvb_frontend *cx24117_attach(const struct cx24117_config *config,
>> -       struct i2c_adapter *i2c, struct dvb_frontend *fe)
>> +       struct i2c_adapter *i2c)
>>   {
>>         struct cx24117_state *state = NULL;
>>         struct cx24117_priv *priv = NULL;
>>         int demod = 0;
>>
>> -       /* first frontend attaching */
>> -       /* allocate shared priv struct */
>> -       if (fe == NULL) {
>> -               priv = kzalloc(sizeof(struct cx24117_priv), GFP_KERNEL);
>> -               if (priv == NULL)
>> -                       goto error1;
>> +       /* get the common data struct for both demods */
>> +       demod = cx24117_get_priv(&priv, i2c, config->demod_address);
>> +
>> +       switch (demod) {
>> +       case 0:
>> +               dev_err(&state->priv->i2c->dev,
>> +                       "%s: Error attaching frontend %d\n",
>> +                       KBUILD_MODNAME, demod);
>> +               goto error1;
>> +               break;
>> +       case 1:
>> +               /* new priv instance */
>>                 priv->i2c = i2c;
>>                 priv->demod_address = config->demod_address;
>>                 mutex_init(&priv->fe_lock);
>> -       } else {
>> -               demod = 1;
>> -               priv = ((struct cx24117_state *)
>> fe->demodulator_priv)->priv;
>> +               break;
>> +       default:
>> +               /* existing priv instance */
>> +               break;
>>         }
>>
>>         /* allocate memory for the internal state */
>> @@ -1154,7 +1187,7 @@ struct dvb_frontend *cx24117_attach(const struct
>> cx24117_config *config,
>>         if (state == NULL)
>>                 goto error2;
>>
>> -       state->demod = demod;
>> +       state->demod = demod - 1;
>>         state->priv = priv;
>>
>>         /* test i2c bus for ack */
>> @@ -1163,12 +1196,9 @@ struct dvb_frontend *cx24117_attach(const struct
>> cx24117_config *config,
>>                         goto error3;
>>         }
>>
>> -       /* nr of frontends using the module */
>> -       atomic_inc(&priv->fe_nr);
>> -
>>         dev_info(&state->priv->i2c->dev,
>>                 "%s: Attaching frontend %d\n",
>> -               KBUILD_MODNAME, demod);
>> +               KBUILD_MODNAME, state->demod);
>>
>>         /* create dvb_frontend */
>>         memcpy(&state->frontend.ops, &cx24117_ops,
>> @@ -1179,7 +1209,7 @@ struct dvb_frontend *cx24117_attach(const struct
>> cx24117_config *config,
>>   error3:
>>         kfree(state);
>>   error2:
>> -       kfree(priv);
>> +       cx24117_release_priv(priv);
>>   error1:
>>         return NULL;
>>   }
>> diff --git a/drivers/media/dvb-frontends/cx24117.h
>> b/drivers/media/dvb-frontends/cx24117.h
>> index 5bc8f11..4e59e95 100644
>> --- a/drivers/media/dvb-frontends/cx24117.h
>> +++ b/drivers/media/dvb-frontends/cx24117.h
>> @@ -33,11 +33,11 @@ struct cx24117_config {
>>   #if IS_ENABLED(CONFIG_DVB_CX24117)
>>   extern struct dvb_frontend *cx24117_attach(
>>         const struct cx24117_config *config,
>> -       struct i2c_adapter *i2c, struct dvb_frontend *fe);
>> +       struct i2c_adapter *i2c);
>>   #else
>>   static inline struct dvb_frontend *cx24117_attach(
>>         const struct cx24117_config *config,
>> -       struct i2c_adapter *i2c, struct dvb_frontend *fe)
>> +       struct i2c_adapter *i2c)
>>   {
>>         dev_warn(&i2c->dev, "%s: driver disabled by Kconfig\n", __func__);
>>         return NULL;
>>
>
>
> --
> http://palosaari.fi/

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

* Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
  2013-10-02 22:54   ` Luis Alves
@ 2013-10-02 23:16     ` Antti Palosaari
  0 siblings, 0 replies; 5+ messages in thread
From: Antti Palosaari @ 2013-10-02 23:16 UTC (permalink / raw)
  To: Luis Alves; +Cc: Michael Krufky, linux-media, mchehab

On 03.10.2013 01:54, Luis Alves wrote:
> I Antti,
>
> I think it's safe to use because the hybrid_tuner_request_state will
> make sure that the i2c_adapter_id is the same for both demods.

I also looked that tuner-i2c.h hybrid_tuner_request_state() and if I am 
not misunderstanding the criteria to return state is i2c adapter id and 
i2c slave address. Yea, it seems to be correct even there is quad card 
used as slave address must be different in that case!

regards
Antti

-- 
http://palosaari.fi/

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

end of thread, other threads:[~2013-10-02 23:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-02 22:09 [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod Luis Alves
2013-10-02 22:16 ` Mauro Carvalho Chehab
2013-10-02 22:37 ` Antti Palosaari
2013-10-02 22:54   ` Luis Alves
2013-10-02 23:16     ` Antti Palosaari

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