From: Antti Palosaari <crope@iki.fi>
To: Luis Alves <ljalvs@gmail.com>, mkrufky@linuxtv.org
Cc: linux-media@vger.kernel.org, mchehab@infradead.org
Subject: Re: [PATCH 1/2] cx24117: Changed the way common data struct was being passed to the demod.
Date: Thu, 03 Oct 2013 01:37:55 +0300 [thread overview]
Message-ID: <524CA043.8080205@iki.fi> (raw)
In-Reply-To: <1380751751-4842-1-git-send-email-ljalvs@gmail.com>
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/
next prev parent reply other threads:[~2013-10-02 22:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2013-10-02 22:54 ` Luis Alves
2013-10-02 23:16 ` Antti Palosaari
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=524CA043.8080205@iki.fi \
--to=crope@iki.fi \
--cc=linux-media@vger.kernel.org \
--cc=ljalvs@gmail.com \
--cc=mchehab@infradead.org \
--cc=mkrufky@linuxtv.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.