Attached is a patch to fix the i2c problems many HVR-1600 users have
been having, such as an unreadable EEPROM.
I cannot test the patch myself (aside from verifying that it does no
harm), because my system doesn't exhibit cx18 driver i2c bus errors.
If you have "(ret=-121)" messages in dmesg related to reading the EEPROM
or setting up the digital tuner, please test this patch and provide
feedback: positive or negative.
This patch is incompatible with the I2C patch I sent to the list in a
prior e-mail; do not apply that previous patch with this patch.
Details of my hypothesis about the problem and the solution:
After reading the I2C specification, PCI Local Bus Specification v3.0,
and Sebastian Schonberg's interesting 2002 PhD dissertation on "Using
PCI Bus Systems in Real-Time Environments", I think I know what the
problem is:
a. The time to complete a PCI memory transfer to any particular device
on the PCI is very dependent on the number of PCI devices in the
systems, the behavior of the devices on the PCI bus, the PCI bus
arbiter's arbitration scheme, and the software and events that stimulate
PCI bus transfers. The time to complete any one transfer can be upper
bounded on a per systems basis, but generally not controlled well on a
system with only a round-robin arbiter.
b. i2c_algo_bit makes the (reasonable) assumption that when a callback
function that commands to change the state of the I2C master's SCL or
SDA line drivers returns, the commanded state change has occurred. (What
other assumption could it make, right?)
c. the cx18 driver did nothing to ensure that commands to change the
state of the I2C master's SDA and SCL line drivers had actually made it
to the I2C master (the cx23418), before returning. Ooops.
d. I2C transfers for the HVR-1600 in and of themselves are not real time
critical transfers. However, the time delays between SCA and SCL
transitions are often real time critical - they can't happen too close
together. One must ensure a state change command has been actually
transmitted to the cx23418 before starting any time delay loop or timer.
Ooops.
The attached patch forces certain I2C related cx23418 PCI memory
register writes complete before returning control and allowing time
delay loops to start. It does this by performing a readback of the
address that was just written. The earliest the readback can start is
not until after the initial write has completed. On an idle PCI bus at
33 MHz, this readback adds a minimum of 120 ns (4*30ns PCI bus cycles)
to the time to set the SDA or SCL line. (An I2C bus running at 100 kHz
has a 10 usec clock period.)
On a heavily loaded PCI bus, the delay added by the readback could be
much longer than 120 ns, but so could the delay for the initial write,
and that was the problem in the first place. Since the I2C bus can run
as slow as we need it too, these longer delays aren't a problem, as long
as we maintain the minimum delays between SDA and SCL transitions.
Regards,
Andy
--- cx18-f763c5864f9a/linux/drivers/media/video/cx18/cx18-i2c.c.orig 2008-04-12 07:59:27.000000000 -0400
+++ cx18-f763c5864f9a/linux/drivers/media/video/cx18/cx18-i2c.c 2008-04-14 17:46:49.000000000 -0400
@@ -178,9 +178,9 @@ static void cx18_setscl1(void *data, int
u32 r = read_reg(CX18_REG_I2C_1_WR);
if (state)
- write_reg(r | SETSCL_BIT, CX18_REG_I2C_1_WR);
+ write_reg_sync(r | SETSCL_BIT, CX18_REG_I2C_1_WR);
else
- write_reg(r & ~SETSCL_BIT, CX18_REG_I2C_1_WR);
+ write_reg_sync(r & ~SETSCL_BIT, CX18_REG_I2C_1_WR);
}
static void cx18_setsda1(void *data, int state)
@@ -189,9 +189,9 @@ static void cx18_setsda1(void *data, int
u32 r = read_reg(CX18_REG_I2C_1_WR);
if (state)
- write_reg(r | SETSDL_BIT, CX18_REG_I2C_1_WR);
+ write_reg_sync(r | SETSDL_BIT, CX18_REG_I2C_1_WR);
else
- write_reg(r & ~SETSDL_BIT, CX18_REG_I2C_1_WR);
+ write_reg_sync(r & ~SETSDL_BIT, CX18_REG_I2C_1_WR);
}
static int cx18_getscl1(void *data)
@@ -214,9 +214,9 @@ static void cx18_setscl2(void *data, int
u32 r = read_reg(CX18_REG_I2C_2_WR);
if (state)
- write_reg(r | SETSCL_BIT, CX18_REG_I2C_2_WR);
+ write_reg_sync(r | SETSCL_BIT, CX18_REG_I2C_2_WR);
else
- write_reg(r & ~SETSCL_BIT, CX18_REG_I2C_2_WR);
+ write_reg_sync(r & ~SETSCL_BIT, CX18_REG_I2C_2_WR);
}
static void cx18_setsda2(void *data, int state)
@@ -225,9 +225,9 @@ static void cx18_setsda2(void *data, int
u32 r = read_reg(CX18_REG_I2C_2_WR);
if (state)
- write_reg(r | SETSDL_BIT, CX18_REG_I2C_2_WR);
+ write_reg_sync(r | SETSDL_BIT, CX18_REG_I2C_2_WR);
else
- write_reg(r & ~SETSDL_BIT, CX18_REG_I2C_2_WR);
+ write_reg_sync(r & ~SETSDL_BIT, CX18_REG_I2C_2_WR);
}
static int cx18_getscl2(void *data)
@@ -438,9 +438,9 @@ int init_cx18_i2c(struct cx18 *cx)
write_reg(0x10001000, 0xc71024); /* Clock Enable */
}
/* courtesy of Steven Toth <[EMAIL PROTECTED]> */
- write_reg(0x00C00000, 0xc7001C);
+ write_reg_sync(0x00C00000, 0xc7001C);
mdelay(10);
- write_reg(0x00C000C0, 0xc7001C);
+ write_reg_sync(0x00C000C0, 0xc7001C);
mdelay(10);
write_reg(0x00C00000, 0xc7001C);
@@ -449,12 +449,12 @@ int init_cx18_i2c(struct cx18 *cx)
write_reg(0x00c00000, 0xc730c4); /* Clear any stale intrs */
write_reg(0x00021c0f & ~4, CX18_REG_I2C_1_WR); /* Hw I2C1 Clock Freq ~100kHz */
- cx18_setscl1(cx, 1);
cx18_setsda1(cx, 1);
+ cx18_setscl1(cx, 1);
write_reg(0x00021c0f & ~4, CX18_REG_I2C_2_WR); /* Hw I2C2 Clock Freq ~100kHz */
- cx18_setscl2(cx, 1);
cx18_setsda2(cx, 1);
+ cx18_setscl2(cx, 1);
return i2c_bit_add_bus(&cx->i2c_adap[0]) || i2c_bit_add_bus(&cx->i2c_adap[1]);
}
@@ -464,7 +464,7 @@ void exit_cx18_i2c(struct cx18 *cx)
int i;
CX18_DEBUG_I2C("i2c exit\n");
write_reg(read_reg(CX18_REG_I2C_1_WR) | 4, CX18_REG_I2C_1_WR);
- write_reg(read_reg(CX18_REG_I2C_2_WR) | 4, CX18_REG_I2C_2_WR);
+ write_reg_sync(read_reg(CX18_REG_I2C_2_WR) | 4, CX18_REG_I2C_2_WR);
for (i = 0; i < 2; i++) {
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20)
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel
