Re: [ECOS] Request for comments on CYG_I2C_BITBANG_SCL_LOW_SDA_INPUT patch

[Bart Veer <bartv at ecoscentric dot com>]

>>>>> "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.

@@ -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.

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.

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.

I assume these patches were the result of trying to get the
bit-banging code running on some real hardware. What is the hardware?

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss