SDA set before SCL while preparing STOP in brzo_i2c_read()

[beefeater94]

In brzo_i2c.c lines 706-710, while preparing for a STOP, SDA is brought LOW a few instructions before SCL is brought LOW (i.e., an unintended START condition). Two of my three I2C slaves didn't mind, but the third one (BPS120 pressure sensor) did. Took me 4 days to find the problem :)

[pasko-zh]

Can you please attach your code? What's the size of the pullup resistor you used?

[beefeater94]

Here's the code and the scope trace. The pull-up was 10k in this particular test, but I also tried others and you can see on the scope that it's adequate for the very low speed used. The slave device is BPS120, which only supports a 2-byte read. The same hardware works fine with the Wire library (otherwise I'd probably have never found the problem).

When you switch from Wire to BRZO, the first BRZO read() goes through but messes up the STOP in the end. Then the next read() fails with NACK of slave address, and it again messes up the STOP, so the next read after that also fails, etc. If you then switch back to Wire, or do ACK polling in BRZO, the first operation might fail, but it will have a correct STOP at the end and the slave will recover.

What you see in the scope pic is the repeated fail with NACK of slave address. The black trace is SCL, the green is SDA, the upper graticule is zoom-in on the last SCL falling edge. You can see that SDA falls just ever so slightly ahead of SCL, which is a no-no. When I reordered the instructions in the assembly, it fixed the problem.

Bps120Test.zip

[pasko-zh]

When I designed the library, I've tested it mainly with 160 MHz CPU clock speed if I2C timings specifications are met, cf. pages 48—50 from "UM10204, I2C-bus specification from NXP".

Could you tell me what is the scale in the zoom in part of your scope trace, i.e. the dots? So that I can roughly check the time t_HD;STA? (And having a look at the other specs as well).

For a START condition t_HD;STA should be minimal 4.0, 0.6, 0.26 usec for Standard-, Fast- and Fast-mode Plus accordingly. Since you did your testing in Standard-mode, i.e. with 100 KHz, it should be greater than 4.0 usec to be harmful.

[beefeater94]

The scale in the upper graticule is 400ns/div (it's shown there near the bottom). So the time from SDA fall to SCL fall is ~50ns. If we really wanted a reliable START condition that all compliant slaves would recognize, it wouldn't be nearly enough. But UM10204 also says in section 3.1.3, "The data on the SDA line must be stable during the HIGH period of the clock". So my understanding is, a change in SDA while SCL is high, unless it is a properly timed START or STOP, is a violation of 3.1.3. Some slaves may be robust to this violation, but BPS120 apparently is not.

BTW my CPU runs at 80MHz. If you run at 160MHz, time from SDA fall to SCL fall may be less than 50ns. But it's still a violation of 3.1.3 to drop SDA before SCL.

[pasko-zh]

I could optimize it by setting SDA and SCL within one instruction, then it is the best :-)

[beefeater94]

I think all you need to do for this particular issue is swap lines 706 and 710, to set SCL LOW before updating SDA. That's what I did. It is legal to change SDA while SCL is LOW. Whereas if you update both with one instruction, there's still no guarantee which will happen first.

(I also considered adding an SDA hold time (i.e., a tiny delay after SCL LOW and before SDA update), just to make everything more robust. But other places in the code don't have an SDA hold, the standard doesn't seem to require it, and anyway there's already a 40ns hold just from using two instructions and a MEMW in between, so I didn't.)

[pasko-zh]

Or, combine the two bitmasks and do it one S16I... let's see...