So is it possible to only modify the udelay used in the kernel i2c
code to polling and make no other change? If that works, then it
might be something to discuss with the i2c maintainer to see if we
can add a UDELAY_POLLING flag or a new function pointer to
implement this in the kernel itself.
I had a look, at it seems that there are numerous differences in
method. To make sure that the algorithm was logically the same would
require quite a lot to change in i2c-algo-bit, and I'm not sure that
the work is really necessary, as it doesn't seem to be a problem for
most hardware. The differences are in things like the polling of the
SDA line; i2c-algo-bit will poll SCL but not SDA; retries; i2c bus
resetting if required in i2c_stop; numerous small delays of 5 SCL
reads (tied to the PCI clock I guess).
How much of that is actually required to make the Zilog chip happy,
or prevent the issue of i2c not working at all with multiple cards,
I've no idea. The real problem is that I don't know what causes the
issue in the first place -- I just reverse engineered the hauppauge
implementation. Without having a specific goal other than making the
implementation the same as what they do, it's going to be a fair bit
of effort. I guess I'd just leave it and chalk it up as an
engineering solution (not all that pretty, but it works).
Let me know what you think,
OK, that's no problem. But in that case I'd like to have some
documentation on this in the form of comments in the source and/or as a
document in ivtv/doc. If you comment the source, then please use the
source in svn trunk, that's the most important one as that will end up
in the kernel.
I've attached some comments against the trunk. Also applies to 0.4 with
a minor reject in the last hunk. Let me know if they are satisfactory.
Great job, by the way! Thanks for putting in the effort of making this
work!
Appreciated, but I think you are the one who really deserves the thanks
for all of your work on the driver!
Thanks,
Mark
Index: ivtv-i2c.c
===================================================================
--- ivtv-i2c.c (revision 3132)
+++ ivtv-i2c.c (working copy)
@@ -17,6 +17,47 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+/*
+ This file includes an i2c implementation that was reverse engineered
+ from the Hauppauge windows driver. Older ivtv versions used i2c-algo-bit,
+ which whilst fine under most circumstances, had trouble with the Zilog
+ CPU on the PVR-150 which handles IR functions (occasional inability to
+ communicate with the chip until it was reset) and also with the i2c
+ bus being completely unreachable when multiple PVR cards were present.
+
+ The implementation is very similar to i2c-algo-bit, but there are enough
+ subtle differences that the two are hard to merge. The general strategy
+ employed by i2c-algo-bit is to use udelay() to implement the timing
+ when putting out bits on the scl/sda lines. The general strategy taken
+ here is to poll the lines for state changes (see ivtv_waitscl and
+ ivtv_waitsda). In addition there are small delays at various locations
+ which poll the SCL line 5 times (ivtv_scldelay). I would guess that
+ since this is memory mapped I/O that the length of those delays is tied
+ to the PCI bus clock. There is some extra code to do with recovery
+ and retries. Since it is not known what causes the actual i2c problems
+ in the first place, the only goal if one was to attempt to use
+ i2c-algo-bit would be to try to make it follow the same code path.
+ This would be a lot of work, and I'm also not convinced that it would
+ provide a generic benefit to i2c-algo-bit. Therefore consider this
+ an engineering solution -- not pretty, but it works.
+
+ Some more general comments about what we are doing:
+
+ The i2c bus is a 2 wire serial bus, with clock (SCL) and data (SDA)
+ lines. To communicate on the bus (as a master, we don't act as a slave),
+ we first initiate a start condition (ivtv_start). We then write the
+ address of the device that we want to communicate with, along with a flag
+ that indicates whether this is a read or a write. The slave then issues
+ an ACK signal (ivtv_ack), which tells us that it is ready for reading /
+ writing. We then proceed with reading or writing (ivtv_read/ivtv_write),
+ and finally issue a stop condition (ivtv_stop) to make the bus available
+ to other masters.
+
+ There is an additional form of transaction where a write may be
+ immediately followed by a read. In this case, there is no intervening
+ stop condition. (Only the msp3400 chip uses this method of data transfer).
+ */
+
#include "ivtv-driver.h"
#include "ivtv-cards.h"
#include "ivtv-gpio.h"
@@ -114,6 +155,7 @@
return 0;
}
+/* Set the serial clock line to the desired state */
static void ivtv_setscl(struct ivtv *itv, int state)
{
/* write them out */
@@ -121,6 +163,7 @@
writel(~state, itv->reg_mem + IVTV_REG_I2C_SETSCL_OFFSET);
}
+/* Set the serial data line to the desired state */
static void ivtv_setsda(struct ivtv *itv, int state)
{
/* write them out */
@@ -128,16 +171,19 @@
writel(~state & 1, itv->reg_mem + IVTV_REG_I2C_SETSDA_OFFSET);
}
+/* Read the serial clock line */
static int ivtv_getscl(struct ivtv *itv)
{
return readl(itv->reg_mem + IVTV_REG_I2C_GETSCL_OFFSET) & 1;
}
+/* Read the serial data line */
static int ivtv_getsda(struct ivtv *itv)
{
return readl(itv->reg_mem + IVTV_REG_I2C_GETSDA_OFFSET) & 1;
}
+/* Implement a short delay by polling the serial clock line */
static void ivtv_scldelay(struct ivtv *itv)
{
int i;
@@ -146,6 +192,7 @@
ivtv_getscl(itv);
}
+/* Wait for the serial clock line to become set to a specific value */
static int ivtv_waitscl(struct ivtv *itv, int val)
{
int i;
@@ -158,6 +205,7 @@
return 0;
}
+/* Wait for the serial data line to become set to a specific value */
static int ivtv_waitsda(struct ivtv *itv, int val)
{
int i;
@@ -170,6 +218,7 @@
return 0;
}
+/* Wait for the slave to issue an ACK */
static int ivtv_ack(struct ivtv *itv)
{
int ret = 0;
@@ -197,6 +246,7 @@
return ret;
}
+/* Write a single byte to the i2c bus and wait for the slave to ACK */
static int ivtv_sendbyte(struct ivtv *itv, unsigned char byte)
{
int i, bit;
@@ -228,6 +278,8 @@
return ivtv_ack(itv);
}
+/* Read a byte from the i2c bus and send a NACK if applicable (i.e. for the
+ final byte) */
static int ivtv_readbyte(struct ivtv *itv, unsigned char *byte, int nack)
{
int i;
@@ -258,6 +310,8 @@
return 0;
}
+/* Issue a start condition on the i2c bus to alert slaves to prepare for
+ an address write */
static int ivtv_start(struct ivtv *itv)
{
int sda;
@@ -283,7 +337,7 @@
return 0;
}
-
+/* Issue a stop condition on the i2c bus to release it */
static int ivtv_stop(struct ivtv *itv)
{
int i;
@@ -319,6 +373,8 @@
return 0;
}
+/* Write a message to the given i2c slave. do_stop may be 0 to prevent
+ issuing the i2c stop condition (when following with a read) */
static int ivtv_write(struct ivtv *itv, unsigned char addr, unsigned char
*data, u32 len, int do_stop)
{
int retry, ret = -EREMOTEIO;
@@ -341,6 +397,7 @@
return ret;
}
+/* Read data from the given i2c slave. A stop condition is always issued. */
static int ivtv_read(struct ivtv *itv, unsigned char addr, unsigned char
*data, u32 len)
{
int retry, ret = -EREMOTEIO;
@@ -360,6 +417,9 @@
return ret;
}
+/* Kernel i2c transfer implementation. Takes a number of messages to be read
+ or written. If a read follows a write, this will occur without an
+ intervening stop condition */
static int ivtv_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg *msgs, int
num)
{
struct ivtv *itv = i2c_get_adapdata(i2c_adap);
@@ -381,6 +441,7 @@
return retval ? retval : num;
}
+/* Kernel i2c capabilities */
static u32 ivtv_functionality(struct i2c_adapter *adap)
{
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel
