* [PATCH 1/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices
@ 2012-03-14 14:27 Antti Palosaari
2012-03-14 14:27 ` [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2 Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2012-03-14 14:27 UTC (permalink / raw)
To: linux-media; +Cc: Gordon Hecker, Antti Palosaari
From: Gordon Hecker <ghecker@gmx.de>
The i2c failures were caused by enabling both i2c gates
at the same time while putting the tuners asleep.
This patch removes the init() and sleep() callbacks from the tuner,
to prevent frontend.c from calling
i2c_gate_ctrl
tuner init / sleep
i2c_gate_ctrl
without holding the lock.
tuner init() and sleep() are instead called in frontend init() and
sleep().
Signed-off-by: Gordon Hecker <ghecker@gmx.de>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb/dvb-usb/af9015.c | 31 +++++++++++++++++++++++++++++++
drivers/media/dvb/dvb-usb/af9015.h | 2 ++
2 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 282a43d..9307b4ca 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -1141,7 +1141,18 @@ static int af9015_af9013_init(struct dvb_frontend *fe)
return -EAGAIN;
ret = priv->init[adap->id](fe);
+ if (ret)
+ goto err_unlock;
+
+ if (priv->tuner_ops_init[adap->id]) {
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 1);
+ ret = priv->tuner_ops_init[adap->id](fe);
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 0);
+ }
+err_unlock:
mutex_unlock(&adap->dev->usb_mutex);
return ret;
@@ -1157,8 +1168,19 @@ static int af9015_af9013_sleep(struct dvb_frontend *fe)
if (mutex_lock_interruptible(&adap->dev->usb_mutex))
return -EAGAIN;
+ if (priv->tuner_ops_sleep[adap->id]) {
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 1);
+ ret = priv->tuner_ops_sleep[adap->id](fe);
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 0);
+ if (ret)
+ goto err_unlock;
+ }
+
ret = priv->sleep[adap->id](fe);
+err_unlock:
mutex_unlock(&adap->dev->usb_mutex);
return ret;
@@ -1283,6 +1305,7 @@ static struct mxl5007t_config af9015_mxl5007t_config = {
static int af9015_tuner_attach(struct dvb_usb_adapter *adap)
{
int ret;
+ struct af9015_state *state = adap->dev->priv;
deb_info("%s:\n", __func__);
switch (af9015_af9013_config[adap->id].tuner) {
@@ -1340,6 +1363,14 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap)
err("Unknown tuner id:%d",
af9015_af9013_config[adap->id].tuner);
}
+
+ state->tuner_ops_sleep[adap->id] =
+ adap->fe_adap[0].fe->ops.tuner_ops.sleep;
+ adap->fe_adap[0].fe->ops.tuner_ops.sleep = 0;
+
+ state->tuner_ops_init[adap->id] =
+ adap->fe_adap[0].fe->ops.tuner_ops.init;
+ adap->fe_adap[0].fe->ops.tuner_ops.init = 0;
return ret;
}
diff --git a/drivers/media/dvb/dvb-usb/af9015.h b/drivers/media/dvb/dvb-usb/af9015.h
index f619063..ee2ec5b 100644
--- a/drivers/media/dvb/dvb-usb/af9015.h
+++ b/drivers/media/dvb/dvb-usb/af9015.h
@@ -108,6 +108,8 @@ struct af9015_state {
int (*read_status[2]) (struct dvb_frontend *fe, fe_status_t *status);
int (*init[2]) (struct dvb_frontend *fe);
int (*sleep[2]) (struct dvb_frontend *fe);
+ int (*tuner_ops_init[2]) (struct dvb_frontend *fe);
+ int (*tuner_ops_sleep[2]) (struct dvb_frontend *fe);
};
struct af9015_config {
--
1.7.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2
2012-03-14 14:27 [PATCH 1/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices Antti Palosaari
@ 2012-03-14 14:27 ` Antti Palosaari
2012-03-14 21:14 ` Malcolm Priestley
0 siblings, 1 reply; 4+ messages in thread
From: Antti Palosaari @ 2012-03-14 14:27 UTC (permalink / raw)
To: linux-media; +Cc: Antti Palosaari
Some changes for previous patch I liked to do.
Just move tuner init and sleep to own functions from the demod
init and sleep functions. Functionality remains still almost the same.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/dvb/dvb-usb/af9015.c | 74 ++++++++++++++++++++++-------------
drivers/media/dvb/dvb-usb/af9015.h | 4 +-
2 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 9307b4ca..7e70ea5 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -1141,18 +1141,7 @@ static int af9015_af9013_init(struct dvb_frontend *fe)
return -EAGAIN;
ret = priv->init[adap->id](fe);
- if (ret)
- goto err_unlock;
-
- if (priv->tuner_ops_init[adap->id]) {
- if (fe->ops.i2c_gate_ctrl)
- fe->ops.i2c_gate_ctrl(fe, 1);
- ret = priv->tuner_ops_init[adap->id](fe);
- if (fe->ops.i2c_gate_ctrl)
- fe->ops.i2c_gate_ctrl(fe, 0);
- }
-err_unlock:
mutex_unlock(&adap->dev->usb_mutex);
return ret;
@@ -1168,24 +1157,48 @@ static int af9015_af9013_sleep(struct dvb_frontend *fe)
if (mutex_lock_interruptible(&adap->dev->usb_mutex))
return -EAGAIN;
- if (priv->tuner_ops_sleep[adap->id]) {
- if (fe->ops.i2c_gate_ctrl)
- fe->ops.i2c_gate_ctrl(fe, 1);
- ret = priv->tuner_ops_sleep[adap->id](fe);
- if (fe->ops.i2c_gate_ctrl)
- fe->ops.i2c_gate_ctrl(fe, 0);
- if (ret)
- goto err_unlock;
- }
-
ret = priv->sleep[adap->id](fe);
-err_unlock:
mutex_unlock(&adap->dev->usb_mutex);
return ret;
}
+/* override tuner callbacks for resource locking */
+static int af9015_tuner_init(struct dvb_frontend *fe)
+{
+ int ret;
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+ struct af9015_state *priv = adap->dev->priv;
+
+ if (mutex_lock_interruptible(&adap->dev->usb_mutex))
+ return -EAGAIN;
+
+ ret = priv->tuner_init[adap->id](fe);
+
+ mutex_unlock(&adap->dev->usb_mutex);
+
+ return ret;
+}
+
+/* override tuner callbacks for resource locking */
+static int af9015_tuner_sleep(struct dvb_frontend *fe)
+{
+ int ret;
+ struct dvb_usb_adapter *adap = fe->dvb->priv;
+ struct af9015_state *priv = adap->dev->priv;
+
+ if (mutex_lock_interruptible(&adap->dev->usb_mutex))
+ return -EAGAIN;
+
+ ret = priv->tuner_sleep[adap->id](fe);
+
+ mutex_unlock(&adap->dev->usb_mutex);
+
+ return ret;
+}
+
+
static int af9015_af9013_frontend_attach(struct dvb_usb_adapter *adap)
{
int ret;
@@ -1364,13 +1377,18 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap)
af9015_af9013_config[adap->id].tuner);
}
- state->tuner_ops_sleep[adap->id] =
- adap->fe_adap[0].fe->ops.tuner_ops.sleep;
- adap->fe_adap[0].fe->ops.tuner_ops.sleep = 0;
+ if (adap->fe_adap[0].fe->ops.tuner_ops.init) {
+ state->tuner_init[adap->id] =
+ adap->fe_adap[0].fe->ops.tuner_ops.init;
+ adap->fe_adap[0].fe->ops.tuner_ops.init = af9015_tuner_init;
+ }
+
+ if (adap->fe_adap[0].fe->ops.tuner_ops.sleep) {
+ state->tuner_sleep[adap->id] =
+ adap->fe_adap[0].fe->ops.tuner_ops.sleep;
+ adap->fe_adap[0].fe->ops.tuner_ops.sleep = af9015_tuner_sleep;
+ }
- state->tuner_ops_init[adap->id] =
- adap->fe_adap[0].fe->ops.tuner_ops.init;
- adap->fe_adap[0].fe->ops.tuner_ops.init = 0;
return ret;
}
diff --git a/drivers/media/dvb/dvb-usb/af9015.h b/drivers/media/dvb/dvb-usb/af9015.h
index ee2ec5b..2f68419 100644
--- a/drivers/media/dvb/dvb-usb/af9015.h
+++ b/drivers/media/dvb/dvb-usb/af9015.h
@@ -108,8 +108,8 @@ struct af9015_state {
int (*read_status[2]) (struct dvb_frontend *fe, fe_status_t *status);
int (*init[2]) (struct dvb_frontend *fe);
int (*sleep[2]) (struct dvb_frontend *fe);
- int (*tuner_ops_init[2]) (struct dvb_frontend *fe);
- int (*tuner_ops_sleep[2]) (struct dvb_frontend *fe);
+ int (*tuner_init[2]) (struct dvb_frontend *fe);
+ int (*tuner_sleep[2]) (struct dvb_frontend *fe);
};
struct af9015_config {
--
1.7.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2
2012-03-14 14:27 ` [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2 Antti Palosaari
@ 2012-03-14 21:14 ` Malcolm Priestley
2012-03-14 21:36 ` Antti Palosaari
0 siblings, 1 reply; 4+ messages in thread
From: Malcolm Priestley @ 2012-03-14 21:14 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On Wed, 2012-03-14 at 16:27 +0200, Antti Palosaari wrote:
> Some changes for previous patch I liked to do.
> Just move tuner init and sleep to own functions from the demod
> init and sleep functions. Functionality remains still almost the same.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/dvb/dvb-usb/af9015.c | 74 ++++++++++++++++++++++-------------
> drivers/media/dvb/dvb-usb/af9015.h | 4 +-
> 2 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
> index 9307b4ca..7e70ea5 100644
> --- a/drivers/media/dvb/dvb-usb/af9015.c
> +++ b/drivers/media/dvb/dvb-usb/af9015.c
> @@ -1141,18 +1141,7 @@ static int af9015_af9013_init(struct dvb_frontend *fe)
> return -EAGAIN;
>
> ret = priv->init[adap->id](fe);
> - if (ret)
> - goto err_unlock;
> -
> - if (priv->tuner_ops_init[adap->id]) {
> - if (fe->ops.i2c_gate_ctrl)
> - fe->ops.i2c_gate_ctrl(fe, 1);
> - ret = priv->tuner_ops_init[adap->id](fe);
> - if (fe->ops.i2c_gate_ctrl)
> - fe->ops.i2c_gate_ctrl(fe, 0);
> - }
>
> -err_unlock:
> mutex_unlock(&adap->dev->usb_mutex);
>
> return ret;
> @@ -1168,24 +1157,48 @@ static int af9015_af9013_sleep(struct dvb_frontend *fe)
> if (mutex_lock_interruptible(&adap->dev->usb_mutex))
> return -EAGAIN;
>
> - if (priv->tuner_ops_sleep[adap->id]) {
> - if (fe->ops.i2c_gate_ctrl)
> - fe->ops.i2c_gate_ctrl(fe, 1);
> - ret = priv->tuner_ops_sleep[adap->id](fe);
> - if (fe->ops.i2c_gate_ctrl)
> - fe->ops.i2c_gate_ctrl(fe, 0);
> - if (ret)
> - goto err_unlock;
> - }
> -
> ret = priv->sleep[adap->id](fe);
>
> -err_unlock:
> mutex_unlock(&adap->dev->usb_mutex);
>
> return ret;
> }
>
> +/* override tuner callbacks for resource locking */
> +static int af9015_tuner_init(struct dvb_frontend *fe)
> +{
> + int ret;
> + struct dvb_usb_adapter *adap = fe->dvb->priv;
> + struct af9015_state *priv = adap->dev->priv;
> +
> + if (mutex_lock_interruptible(&adap->dev->usb_mutex))
> + return -EAGAIN;
Hi Antti
I think using mutex_lock_interruptible errors into dvb-usb causes false
errors and errors caused by missed registers.
I prefer to use mutex_lock only return genuine device errors.
Regards
Malcolm
> +
> + ret = priv->tuner_init[adap->id](fe);
> +
> + mutex_unlock(&adap->dev->usb_mutex);
> +
> + return ret;
> +}
> +
> +/* override tuner callbacks for resource locking */
> +static int af9015_tuner_sleep(struct dvb_frontend *fe)
> +{
> + int ret;
> + struct dvb_usb_adapter *adap = fe->dvb->priv;
> + struct af9015_state *priv = adap->dev->priv;
> +
> + if (mutex_lock_interruptible(&adap->dev->usb_mutex))
> + return -EAGAIN;
> +
> + ret = priv->tuner_sleep[adap->id](fe);
> +
> + mutex_unlock(&adap->dev->usb_mutex);
> +
> + return ret;
> +}
> +
> +
> static int af9015_af9013_frontend_attach(struct dvb_usb_adapter *adap)
> {
> int ret;
> @@ -1364,13 +1377,18 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap)
> af9015_af9013_config[adap->id].tuner);
> }
>
> - state->tuner_ops_sleep[adap->id] =
> - adap->fe_adap[0].fe->ops.tuner_ops.sleep;
> - adap->fe_adap[0].fe->ops.tuner_ops.sleep = 0;
> + if (adap->fe_adap[0].fe->ops.tuner_ops.init) {
> + state->tuner_init[adap->id] =
> + adap->fe_adap[0].fe->ops.tuner_ops.init;
> + adap->fe_adap[0].fe->ops.tuner_ops.init = af9015_tuner_init;
> + }
> +
> + if (adap->fe_adap[0].fe->ops.tuner_ops.sleep) {
> + state->tuner_sleep[adap->id] =
> + adap->fe_adap[0].fe->ops.tuner_ops.sleep;
> + adap->fe_adap[0].fe->ops.tuner_ops.sleep = af9015_tuner_sleep;
> + }
>
> - state->tuner_ops_init[adap->id] =
> - adap->fe_adap[0].fe->ops.tuner_ops.init;
> - adap->fe_adap[0].fe->ops.tuner_ops.init = 0;
> return ret;
> }
>
> diff --git a/drivers/media/dvb/dvb-usb/af9015.h b/drivers/media/dvb/dvb-usb/af9015.h
> index ee2ec5b..2f68419 100644
> --- a/drivers/media/dvb/dvb-usb/af9015.h
> +++ b/drivers/media/dvb/dvb-usb/af9015.h
> @@ -108,8 +108,8 @@ struct af9015_state {
> int (*read_status[2]) (struct dvb_frontend *fe, fe_status_t *status);
> int (*init[2]) (struct dvb_frontend *fe);
> int (*sleep[2]) (struct dvb_frontend *fe);
> - int (*tuner_ops_init[2]) (struct dvb_frontend *fe);
> - int (*tuner_ops_sleep[2]) (struct dvb_frontend *fe);
> + int (*tuner_init[2]) (struct dvb_frontend *fe);
> + int (*tuner_sleep[2]) (struct dvb_frontend *fe);
> };
>
> struct af9015_config {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2
2012-03-14 21:14 ` Malcolm Priestley
@ 2012-03-14 21:36 ` Antti Palosaari
0 siblings, 0 replies; 4+ messages in thread
From: Antti Palosaari @ 2012-03-14 21:36 UTC (permalink / raw)
To: Malcolm Priestley; +Cc: linux-media
On 14.03.2012 23:14, Malcolm Priestley wrote:
> On Wed, 2012-03-14 at 16:27 +0200, Antti Palosaari wrote:
>> Some changes for previous patch I liked to do.
>> Just move tuner init and sleep to own functions from the demod
>> init and sleep functions. Functionality remains still almost the same.
>>
>> Signed-off-by: Antti Palosaari<crope@iki.fi>
>> ---
>> drivers/media/dvb/dvb-usb/af9015.c | 74 ++++++++++++++++++++++-------------
>> drivers/media/dvb/dvb-usb/af9015.h | 4 +-
>> 2 files changed, 48 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
>> index 9307b4ca..7e70ea5 100644
>> --- a/drivers/media/dvb/dvb-usb/af9015.c
>> +++ b/drivers/media/dvb/dvb-usb/af9015.c
>> @@ -1141,18 +1141,7 @@ static int af9015_af9013_init(struct dvb_frontend *fe)
>> return -EAGAIN;
>>
>> ret = priv->init[adap->id](fe);
>> - if (ret)
>> - goto err_unlock;
>> -
>> - if (priv->tuner_ops_init[adap->id]) {
>> - if (fe->ops.i2c_gate_ctrl)
>> - fe->ops.i2c_gate_ctrl(fe, 1);
>> - ret = priv->tuner_ops_init[adap->id](fe);
>> - if (fe->ops.i2c_gate_ctrl)
>> - fe->ops.i2c_gate_ctrl(fe, 0);
>> - }
>>
>> -err_unlock:
>> mutex_unlock(&adap->dev->usb_mutex);
>>
>> return ret;
>> @@ -1168,24 +1157,48 @@ static int af9015_af9013_sleep(struct dvb_frontend *fe)
>> if (mutex_lock_interruptible(&adap->dev->usb_mutex))
>> return -EAGAIN;
>>
>> - if (priv->tuner_ops_sleep[adap->id]) {
>> - if (fe->ops.i2c_gate_ctrl)
>> - fe->ops.i2c_gate_ctrl(fe, 1);
>> - ret = priv->tuner_ops_sleep[adap->id](fe);
>> - if (fe->ops.i2c_gate_ctrl)
>> - fe->ops.i2c_gate_ctrl(fe, 0);
>> - if (ret)
>> - goto err_unlock;
>> - }
>> -
>> ret = priv->sleep[adap->id](fe);
>>
>> -err_unlock:
>> mutex_unlock(&adap->dev->usb_mutex);
>>
>> return ret;
>> }
>>
>> +/* override tuner callbacks for resource locking */
>> +static int af9015_tuner_init(struct dvb_frontend *fe)
>> +{
>> + int ret;
>> + struct dvb_usb_adapter *adap = fe->dvb->priv;
>> + struct af9015_state *priv = adap->dev->priv;
>> +
>> + if (mutex_lock_interruptible(&adap->dev->usb_mutex))
>> + return -EAGAIN;
> Hi Antti
>
> I think using mutex_lock_interruptible errors into dvb-usb causes false
> errors and errors caused by missed registers.
>
> I prefer to use mutex_lock only return genuine device errors.
IIRC documentation says mutex_lock_interruptible() must be used instead
of mutex_lock() when possible. I don't see any reason why I it should be
changed.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-03-14 21:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-14 14:27 [PATCH 1/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices Antti Palosaari
2012-03-14 14:27 ` [PATCH 2/2 FOR 3.4] af9015: fix i2c failures for dual-tuner devices - part 2 Antti Palosaari
2012-03-14 21:14 ` Malcolm Priestley
2012-03-14 21:36 ` Antti Palosaari
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).