[PATCH] af9013: fix i2c failures for dual-tuner devices

2012-01-20 Thread Gordon Hecker
This patch fixes the following error messages with a
Terratec Cinergy T Stick Dual RC DVB-T device.

af9013: i2c wr failed=-1 reg=d607 len=1
af9015: command failed:2
af9013: i2c rd failed=-1 reg=d607 len=1
af9015: command failed:1

It implements exclusive access to i2c for only one frontend at a time
through a use-counter that is increased for each af9013_i2c_gate_ctrl-enable
or i2c-read/write and decreased accordingly. The use-counter remains
incremented after af9013_i2c_gate_ctrl-enable until the corresponding
disable.

Debug output was added.

ToDo:
 * Replace frontend by adapter (the dual-tuner devices do actually
   provide two adapters with one frontend each)
 * move af9013_i2c_gate_mutex, locked_fe, af9013_i2c_gate_ctrl_usecnt
   to the usb device
---
 drivers/media/dvb/frontends/af9013.c |   93 +-
 1 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/drivers/media/dvb/frontends/af9013.c 
b/drivers/media/dvb/frontends/af9013.c
index 6bcbcf5..ab69585 100644
--- a/drivers/media/dvb/frontends/af9013.c
+++ b/drivers/media/dvb/frontends/af9013.c
@@ -28,6 +28,54 @@ int af9013_debug;
 module_param_named(debug, af9013_debug, int, 0644);
 MODULE_PARM_DESC(debug, Turn on/off frontend debugging (default:off).);
 
+static DEFINE_MUTEX(af9013_i2c_gate_mutex);
+static struct dvb_frontend *locked_fe = 0;
+static int af9013_i2c_gate_ctrl_usecnt = 0;
+
+#define GATE_NOOP0
+#define GATE_ENABLE  1
+#define GATE_DISABLE 2
+
+static int af9013_i2c_gate_inc_use_count(struct dvb_frontend *fe, int gate_op)
+{
+   int success = 0;
+   while (1) {
+   if (mutex_lock_interruptible(af9013_i2c_gate_mutex) != 0) {
+   return -EAGAIN;
+   }
+   if (af9013_i2c_gate_ctrl_usecnt == 0 || locked_fe == fe) {
+   success = 1;
+   locked_fe = fe;
+   if (gate_op != GATE_DISABLE) {
+   af9013_i2c_gate_ctrl_usecnt++;
+   }
+   }
+   mutex_unlock(af9013_i2c_gate_mutex);
+   if (success) {
+   break;
+   }
+   schedule();
+   }
+   dbg(%s: %d (%d), __func__, af9013_i2c_gate_ctrl_usecnt, fe-dvb-num);
+   return 0;
+}
+
+static int af9013_i2c_gate_dec_use_count(struct dvb_frontend *fe, int gate_op)
+{
+   if (mutex_lock_interruptible(af9013_i2c_gate_mutex) != 0) {
+   return -EAGAIN;
+   }
+   if (gate_op != GATE_ENABLE) {
+   af9013_i2c_gate_ctrl_usecnt--;
+   }
+   if (af9013_i2c_gate_ctrl_usecnt == 0) {
+   locked_fe = 0;
+   }
+   mutex_unlock(af9013_i2c_gate_mutex);
+   dbg(%s: %d (%d), __func__, af9013_i2c_gate_ctrl_usecnt, fe-dvb-num);
+   return 0;
+}
+
 struct af9013_state {
struct i2c_adapter *i2c;
struct dvb_frontend fe;
@@ -44,7 +92,6 @@ struct af9013_state {
unsigned long set_frontend_jiffies;
unsigned long read_status_jiffies;
bool first_tune;
-   bool i2c_gate_state;
unsigned int statistics_step:3;
struct delayed_work statistics_work;
 };
@@ -54,6 +101,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 
mbox, u16 reg,
const u8 *val, int len)
 {
int ret;
+   struct dvb_frontend *fe = (priv-fe);
u8 buf[3+len];
struct i2c_msg msg[1] = {
{
@@ -69,6 +117,10 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 
mbox, u16 reg,
buf[2] = mbox;
memcpy(buf[3], val, len);
 
+   if (af9013_i2c_gate_inc_use_count(fe, GATE_NOOP) != 0) {
+   return -EAGAIN;
+   }
+   dbg(%s: I2C write reg:%04x (%d), __func__, reg, fe-dvb-num);
ret = i2c_transfer(priv-i2c, msg, 1);
if (ret == 1) {
ret = 0;
@@ -76,6 +128,10 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 
mbox, u16 reg,
warn(i2c wr failed=%d reg=%04x len=%d, ret, reg, len);
ret = -EREMOTEIO;
}
+
+   if (af9013_i2c_gate_dec_use_count(fe, GATE_NOOP) != 0) {
+   return -EAGAIN;
+   }
return ret;
 }
 
@@ -84,6 +140,7 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 
mbox, u16 reg,
u8 *val, int len)
 {
int ret;
+   struct dvb_frontend *fe = (priv-fe);
u8 buf[3];
struct i2c_msg msg[2] = {
{
@@ -103,6 +160,10 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, 
u8 mbox, u16 reg,
buf[1] = (reg  0)  0xff;
buf[2] = mbox;
 
+   if (af9013_i2c_gate_inc_use_count(fe, GATE_NOOP) != 0) {
+   return -EAGAIN;
+   }
+   dbg(%s: I2C read reg:%04x (%d), __func__, reg, priv-fe.dvb-num);
ret = i2c_transfer(priv-i2c, msg, 2);
if (ret == 2) {
ret = 0;
@@ -110,6 +171,9 @@ static int af9013_rd_regs_i2c(struct 

Re: [PATCH] af9013: fix i2c failures for dual-tuner devices

2012-01-20 Thread Antti Palosaari

On 01/20/2012 11:18 PM, Gordon Hecker wrote:

This patch fixes the following error messages with a
Terratec Cinergy T Stick Dual RC DVB-T device.

af9013: i2c wr failed=-1 reg=d607 len=1
af9015: command failed:2
af9013: i2c rd failed=-1 reg=d607 len=1
af9015: command failed:1

It implements exclusive access to i2c for only one frontend at a time
through a use-counter that is increased for each af9013_i2c_gate_ctrl-enable
or i2c-read/write and decreased accordingly. The use-counter remains
incremented after af9013_i2c_gate_ctrl-enable until the corresponding
disable.

Debug output was added.

ToDo:
  * Replace frontend by adapter (the dual-tuner devices do actually
provide two adapters with one frontend each)
  * move af9013_i2c_gate_mutex, locked_fe, af9013_i2c_gate_ctrl_usecnt
to the usb device


Again that same issue. But after af9013 rewrote those should not be very 
significant problem... If you looked, as I can guess, AF9015 code you 
can see callbacks lock that are for resolving same issue. Almost all 
callbacks are locked, but I left few rarely called callbacks without 
lock due to performance point of view. You can guess it causes always 
latency to wait other thread finish callback...


I think you may resolve that just adding one or two callback lock more, 
likely for tuner init() and sleep().


I don't like that you are adding such code for the demod driver - 
problem is USB-bridge, its I2C adapter and firmware. Due to that such 
code should be in AF9015 driver. There has been originally two I2C 
adapters in af9015 but I removed those. And there is actually only one 
I2C adapter in AF9015.

See these for more info about I2C bus connections:
http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt
http://palosaari.fi/linux/v4l-dvb/controlling_tuner.txt

I think that piece of code is not very important and I dont like to see 
it in current af9013 driver. Do needed changes for af9015 instead.


regards
Antti

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html