i2c.SLOW results in 35Kbit/s

[RadianM]

The actual i2c.SLOW speed doesn't appear to be documented but empirically appears to be only around 35Kbits/s rather than the 100Kbits/s I was expecting for "slow". looking at the SCL line:

A little digging finds i2c_master.c making frequent calls to i2c_master_wait() typically for 5us (presumably intended to acheive 100Kbits/s bit-banging). i2c_master_wait() is defined in i2c_master.h as os_delay_us() yet clearly the resulting delay is considerably greater than promised. The overheads of branching and GPIO bit manipulation account for (wasted) delays that could be subtracted from the timing values.

This code doesn't seem to have had much love since 2014 but strikes me as being worthy of improvement. Just messing around with a clone of the dev repository I was able to get ~100Kbits/s by tweaking the delays ('5us' -> '2us') and ~200Kbits/s by removing the delay calls altogether. I'm not sure if this is entirely bullet-proof but I have three different I2C peripherals all running happily with six times the throughput now.

Do we know if there is a good reason not to tweak things in this area?

[pjsg]

What happens to the timings when you run with a 160MHz clock rather than the standard 80MHz.

node.setcpufreq(node.CPU160MHZ)

I'm all in favour of improving the speed provided that the resulting waveforms are good for both CPU speeds.

[TerryE]

I looked at this a couple of years back. The relative delays are a flaw because they don't include the processing drlays in fetching and executing code during bit banging. Tweaking the delays is not the way to go, but maybe using CPU clocks and the CPU tick register would enable us to get closer to 100Kbit/s.

In the end, I didn't bother because the I²C driver is based on the Expressive supplied driver with a few tweaks, and it works fine for all of the I²C devices that I use.

But if you want to develop a robust alterative that works at 100Kbit/s then test it out and submit the PR.

[devsaurus]

But if you want to develop a robust alternative

Maybe there's even a blueprint for such an alternative. I'd suggest to investigate the i2c driver mentioned by @elektro255 in #2184 (comment).

[TerryE]

Armin, I don't disagree, but we need a contributor to do the leg work. Volunteers please take one step forward, 😄

[RadianM]

@pjsg The stock firmware shows a small increase from 34.7K to 38.5K at 160MHz. My 'very hacked' version (simply removed all the delay calls) doubles neatly to 400K and continues to work without a hitch. While this probably isn't fit for general consumption it certainly gets me out of a hole. It also confirms the potential for much faster i2c - as witnessed in #2184

I'm currently in a desparate race to engineer a solution (using a TCA9554A port expander, ADC128D818 8-channel A/D, and QT1070 touch sensor) so I haven't got the luxury of time for producing a sanitised (PR-able) version - I also have an awful mental block for all things GIT related.

The structure of the nodemcu firmware isn't very clear in my mind either - would it be as simple as piecing together the brzo_i2c code into a revised i2c_master.c?

[TerryE]

The I2C bus architecture is designed to allow the master to set the pace. The slave tolerances on timing are usually quite sloppy, and the nominal clock rates really relate to the edge transition times that the slave must be able to latch. However this isn't the case with all I2C devices, so this is very much a case of it depends on your devices.

I wouldn't recommend going the @valkuc route as this level of Xtensa assembly is very high maintenance. Of the developers on this project, I think that only @jmattsson and myself have any familiarity with the Xtensa instruction set (and my problem is that after the first dozen or so assembly instruction sets that you learn, they all become a bit of a blur!!). The ICACHE_IRAM is also at a premium, so this could also cause some resourcing problems.

I was looking at a simpler approach which was that i2c_master_setDC(), and the corresponding get, are pretty much always followed by an os_delay_us() of 5 uSec. whereas the whole sequence should be on a 5 uSec strobe, so why not just add a poll of the CPU clock register inline using an rsr.ccount instruction at the end start of the setDC code and enter the main body after an interval of 5 uSec from he last clock edge (possibly reduced by the call overhead the routine). You could then dump nearly all waits and this would actually both simplify and speed up the code. Note that this counter loop would needs to be CPU frequency aware, but the clocks per 5 uSec won't change during the scope of a single I2C byte operation.

The other thing that you need to be aware of is that you can't mask interrupts during the entire bit-bang sequence as this breaks the no masked interrupts for >50 uSec rule. However this isn't normally an issue as most I2C slaves will tolerate the odd HW interrupt firing within the sequence.

But I've got Lua 5.3 to do first as this will deliver more benefit to more a wider percentage of users. 😢

PS. On reflection the throttle would be better placed on the entry to the setDC code.