This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: Request for comments onCYG_I2C_BITBANG_SCL_LOW_SDA_INPUT patch
"Oyvind" == =?ISO-8859-1?Q?=D8yvind?= Harboe <ISO-8859-1> writes:
Oyvind> I've assembled a patch for a race condition where SDA can
Oyvind> be changed before SCL is stable low.
Oyvind> My I2C + hardware skills are fairly shaky, so any and all
Oyvind> comments are most welcome!
Looking at the patch to the bit-banging code:
Index: src/i2c.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/i2c/current/src/i2c.cxx,v
retrieving revision 1.3
diff -u -r1.3 i2c.cxx
--- src/i2c.cxx 15 Jul 2005 13:52:27 -0000 1.3
+++ src/i2c.cxx 6 Mar 2006 21:38:40 -0000
@@ -296,6 +296,10 @@
(*)> (*banger)(mash, (0 != mask) ? CYG_I2C_BITBANG_SCL_LOW :
CYG_I2C_BITBANG_SCL_LOW_SDA_INPUT);
HAL_DELAY_US(delay);
}
+ // SCL is stable low, we can now drop SDA
+ (*banger)(mash, CYG_I2C_BITBANG_SDA_HIGH);
+ HAL_DELAY_US(delay);
+
// The last bit has been sent, SCL is low, and SDA has been turned
// into an input. The device should be placing the acknowledge bit
// onto SDA. Reading the acknowledge bit still needs a clock cycle.
This makes no sense. Assuming typical hardware where a single GPIO pin
is used for SDA and another for SCL, at the end of the loop the SDA
pin will have been turned into an input. Setting it as an output-high
will only change a bit in some GPIO control register, it will have no
effect on what is happening on the wire.
There are different types of IO that are used for I2C, one correct, and
several bad ones. Like these:
1) GPIO (the correct way) where you just pull the SDA/SCL lines low by
permanently setting the output data to low, and use the direction of the I/O
for signaling (in=high/out=low). These signals need to be pulled logic high
by resistors.
2) GPIO where you for some reason need to keep SDA driving high for some
period, but you do have control of direcion (maybe for electronics where the
pullup is missing)
3) 2 I/O lines where one pin is permanently output (with serial resistor)
and another is permanently input wich is permanently connected together.
SCL_LOW_SDA_INPUT seems to be implemented for solution 2 and 3, cause
setting SCL low and SDA high simultaneously (or near simultaneously)
introduces 2 problems:
A) Race condition. According to the i2c specs, all devices must see a stable
clock low before SDA changes. Depending on the wiring distance, wiring load
and CPU speed, this may end up in a slave detecting start/stop conditions.
Replacing SCL_LOW_SDA_INPUT by CYG_I2C_BITBANG_SCL_LOW, introducing a delay
before releasing SDA will resolve this problem. The discussed changes will
do this, provided SCL_LOW_SDA_INPUT is implemented with fallback to SCL_LOW.
Ideally SCL_LOW_SDA_INPUT should be removed but this is maybe the best of
all evil solution for type 2&3 I/O.
B) for a short period of time, the master may drive SDA high, while a slave
is driving SDA low. Again, SCL_LOW_SDA_INPUT seems to be the best of all
evil solution to prevent this case when using type 2&3 I/O.
Conclusion:
On Oyvind's code changes, replacing (*) with
(*banger)(mash, CYG_I2C_BITBANG_SCL_LOW);
would be the best and only safe solution where you use proper type 1 I/O.
@@ -356,6 +360,9 @@
HAL_DELAY_US(delay);
(*)> (*banger)(mash, nak ? CYG_I2C_BITBANG_SCL_LOW :
CYG_I2C_BITBANG_SCL_LOW_SDA_INPUT);
HAL_DELAY_US(delay);
+ // SCL is stable low, we can now drop SDA
+ (*banger)(mash, CYG_I2C_BITBANG_SDA_HIGH);
+ HAL_DELAY_US(delay);
DEBUG("i2c bitbang read_byte, result %02x\n", result);
return result;
Unless the nak flag is set, the SDA GPIO pin will again be an input so
setting it output-high has no effect on the wires. If the nak flag is
set, i.e. if we are at the end of a receive, then we are equally
likely to want SDA high for a subsequent repeated start or low for a
stop. I see little point in forcing it high.
Same as above.. Changing SDA before SCL has stabelized to low, may cause
problems.
On the more general issue, if I understand correctly you are concerned
about what is happening on the SDA line during an SCL_LOW_SDA_INPUT
operation. If the SDA line was already high then it won't change, the
pull-up resistor will keep it high, so we are only worried about SDA
transitions from low to high.
True.
SCL dropping from high to low involves discharging the line's
capacitance through a low resistance. Further, the banger function
should perform this step first: the operation is SCL_LOW_SDA_INPUT,
not SDA_INPUT_SCL_LOW. SDA going low to high involves charging the
line's capacitance through a pull-up resistor. That should take rather
longer. On typical hardware I would expect SCL to have dropped to a
suitably low value well before any significant changes have happened
on the SDA line. There could be platform-specific problems where the
SCL line has a much higher capacitance than SDA, but those should be
addressed by some dummy cycles within the bit-bang function.
Yes, the order of SCL_LOW and SDA_INPUT is an important point. However I
would not trust your assumptions about "typical" hardware connected to the
bus. I2C may run over long cables (DDC) and have different complicated
loads. I would like to see the drivers stable on all solutions, where only
the bitrate should be adjusted to compensate for wiring complexity.
I assume these patches were the result of trying to get the
bit-banging code running on some real hardware. What is the hardware?
I work on the same project as Oyvind. The hardware is FPGA I/O's type 1
where there are several I2C bus'es, some onboardwith known loads, some
external where the load is unknown (both electrically, i2c implementation
type and wires), and where there is hot-plugging (DDC, external I/O
devices). The potential problem was actually discovered when implementing
SCL_LOW_SDA_INPUT in the wrong direction. I do not feel it's a safe solution
doing SCL_LOW then SDA_INPUT with an undefined delay in between.
Bart
--
Bart Veer eCos Configuration Architect
http://www.ecoscentric.com/ The eCos and RedBoot experts
Thanks for the attention,
Morten Leikvoll
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss