On Tue, 2008-04-15 at 18:22 -0400, Andy Walls wrote:
> Matthew Eaton wrote:
> > On Mon, Apr 14, 2008 at 7:13 PM, Andy Walls <cwalls at radix.net> wrote:
> > > 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.
> >
> > Still getting the (ret=-121) errors. I can compile i2c_algo_bit with
> > debug support and send a log if you think it would be helpful.
>
> Thanks for taking the time to test and provide feedback.
>
> Yes debug output from i2c_algo_bit that would be helpful.
>
> Could you also provide the output of:
> $ su - root
> # lspci -tv
> # lspci -vvvnnxxx
> # lsmod
>
>
> I'll post a revised version of the patch later tonight that does things
> differently and provides some debug messages as well.
The new patch is attached. Please back out the previous patch and give
this one a try. It tries to verify an I2C line has actually been set
before returning to i2c_algo_bit. (A further improvement could be to
retry the write, if all the readbacks fail).
Using 'modprobe cx18 debug=320' on my machine I notice that
1. The SCL line always moves to the commanded state immediately.
2. The SDA line *almost* always moves to the commanded state, and if it
does so, it does it immediately.
3. If the SDA line does not move to the commanded state, it is always
the case that it didn't go 1/high when expected .
Given the above
1. I don't appear to have a PCI bus problem
2. A slave on both I2C buses of my HVR-1600 occasionally gets stuck
pulling the bus SDA line low (which is electrically OK on an open
collector bus, but logically not what it's expected to do, if
i2c_algo_bit does its job properly).
Here's a snip of the dmesg output:
[...]
cx180 i2c: setsda1 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsda1 SDA confirmed in correct state after readback #0
cx180 i2c: setsc11 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsc11 SCL confirmed in correct state after readback #0
cx180 i2c: setsc11 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsc11 SCL confirmed in correct state after readback #0
cx180 i2c: setsda1 CX18_REG_I2C_1_WR confirmed written after readback #0
cx18-0 i2c: setsda1 SDA not in expected state (1) after 1500 readbacks
cx180 i2c: setsc11 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsc11 SCL confirmed in correct state after readback #0
cx180 i2c: setsc11 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsc11 SCL confirmed in correct state after readback #0
cx180 i2c: setsda1 CX18_REG_I2C_1_WR confirmed written after readback #0
cx180 i2c: setsda1 SDA confirmed in correct state after readback #0
[...]
cx180 i2c: setsda2 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsda2 SDA confirmed in correct state after readback #0
cx180 i2c: setsc12 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsc12 SCL confirmed in correct state after readback #0
cx180 i2c: setsc12 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsc12 SCL confirmed in correct state after readback #0
cx180 i2c: setsda2 CX18_REG_I2C_2_WR confirmed written after readback #0
cx18-0 i2c: setsda2 SDA not in expected state (1) after 1500 readbacks
cx180 i2c: setsc12 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsc12 SCL confirmed in correct state after readback #0
cx180 i2c: setsc12 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsc12 SCL confirmed in correct state after readback #0
cx180 i2c: setsda2 CX18_REG_I2C_2_WR confirmed written after readback #0
cx180 i2c: setsda2 SDA confirmed in correct state after readback #0
[...]
Regards,
Andy
> Regards,
> Andy
>
> > Regards,
> > Matt
--- 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-15 19:37:09.000000000 -0400
@@ -172,26 +172,88 @@ static int detach_inform(struct i2c_clie
return 0;
}
+#define MAX_READBACKS (1500)
+
static void cx18_setscl1(void *data, int state)
{
struct cx18 *cx = (struct cx18 *)data;
u32 r = read_reg(CX18_REG_I2C_1_WR);
+ int i;
if (state)
- write_reg(r | SETSCL_BIT, CX18_REG_I2C_1_WR);
+ r |= SETSCL_BIT;
+ else
+ r &= ~SETSCL_BIT;
+
+ write_reg(r, CX18_REG_I2C_1_WR);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_1_WR) & SETSCL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsc11 CX18_REG_I2C_1_WR confirmed written "
+ "after readback #%d\n", i);
+ else
+ CX18_DEBUG_I2C("setsc11 failed to confirm CX18_REG_I2C_1_WR "
+ " written after %d readbacks\n", MAX_READBACKS);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_1_RD) & GETSCL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsc11 SCL confirmed in correct state "
+ "after readback #%d\n", i);
else
- write_reg(r & ~SETSCL_BIT, CX18_REG_I2C_1_WR);
+ CX18_DEBUG_I2C("setsc11 SCL not in expected state (%d) "
+ "after %d readbacks (clock stretching slave?)\n",
+ state ? 1 : 0, MAX_READBACKS);
}
static void cx18_setsda1(void *data, int state)
{
struct cx18 *cx = (struct cx18 *)data;
u32 r = read_reg(CX18_REG_I2C_1_WR);
+ int i;
if (state)
- write_reg(r | SETSDL_BIT, CX18_REG_I2C_1_WR);
+ r |= SETSDL_BIT;
+ else
+ r &= ~SETSDL_BIT;
+
+ write_reg(r, CX18_REG_I2C_1_WR);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_1_WR) & SETSDL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsda1 CX18_REG_I2C_1_WR confirmed written "
+ "after readback #%d\n", i);
+ else
+ CX18_DEBUG_I2C("setsda1 failed to confirm CX18_REG_I2C_1_WR "
+ " written after %d readbacks\n", MAX_READBACKS);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_1_RD) & GETSDL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsda1 SDA confirmed in correct state "
+ "after readback #%d\n", i);
else
- write_reg(r & ~SETSDL_BIT, CX18_REG_I2C_1_WR);
+ CX18_DEBUG_I2C("setsda1 SDA not in expected state (%d) "
+ "after %d readbacks\n",
+ state ? 1 : 0, MAX_READBACKS);
}
static int cx18_getscl1(void *data)
@@ -212,22 +274,82 @@ static void cx18_setscl2(void *data, int
{
struct cx18 *cx = (struct cx18 *)data;
u32 r = read_reg(CX18_REG_I2C_2_WR);
+ int i;
if (state)
- write_reg(r | SETSCL_BIT, CX18_REG_I2C_2_WR);
+ r |= SETSCL_BIT;
+ else
+ r &= ~SETSCL_BIT;
+
+ write_reg(r, CX18_REG_I2C_2_WR);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_2_WR) & SETSCL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsc12 CX18_REG_I2C_2_WR confirmed written "
+ "after readback #%d\n", i);
else
- write_reg(r & ~SETSCL_BIT, CX18_REG_I2C_2_WR);
+ CX18_DEBUG_I2C("setsc12 failed to confirm CX18_REG_I2C_2_WR "
+ " written after %d readbacks\n", MAX_READBACKS);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_2_RD) & GETSCL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsc12 SCL confirmed in correct state "
+ "after readback #%d\n", i);
+ else
+ CX18_DEBUG_I2C("setsc12 SCL not in expected state (%d) "
+ "after %d readbacks (clock stretching slave?)\n",
+ state ? 1: 0, MAX_READBACKS);
}
static void cx18_setsda2(void *data, int state)
{
struct cx18 *cx = (struct cx18 *)data;
u32 r = read_reg(CX18_REG_I2C_2_WR);
+ int i;
if (state)
- write_reg(r | SETSDL_BIT, CX18_REG_I2C_2_WR);
+ r |= SETSDL_BIT;
else
- write_reg(r & ~SETSDL_BIT, CX18_REG_I2C_2_WR);
+ r &= ~SETSDL_BIT;
+
+ write_reg(r, CX18_REG_I2C_2_WR);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_2_WR) & SETSDL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsda2 CX18_REG_I2C_2_WR confirmed written "
+ "after readback #%d\n", i);
+ else
+ CX18_DEBUG_I2C("setsda2 failed to confirm CX18_REG_I2C_2_WR "
+ " written after %d readbacks\n", MAX_READBACKS);
+
+ for (i = 0; i < MAX_READBACKS; i++)
+ {
+ r = read_reg(CX18_REG_I2C_2_RD) & GETSDL_BIT;
+ if ((state && r) || (!state && !r))
+ break;
+ }
+ if (i < MAX_READBACKS)
+ CX18_DEBUG_HI_I2C("setsda2 SDA confirmed in correct state "
+ "after readback #%d\n", i);
+ else
+ CX18_DEBUG_I2C("setsda2 SDA not in expected state (%d) "
+ "after %d readbacks\n",
+ state ? 1 : 0, MAX_READBACKS);
}
static int cx18_getscl2(void *data)
@@ -438,9 +560,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);
@@ -464,7 +586,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
