ACK polling

[valkuc]

Hello,
I need to read and write to AT24C32 i2c eeprom. To ensure data was written by chip I need to use "acknowledge polling".
From datasheet:

ACKNOWLEDGE POLLING: Once the internally-timed write cycle has started and the
EEPROM inputs are disabled, acknowledge polling can be initiated. This involves sending
a start condition followed by the device address word. The read/write bit is
representative of the operation desired. Only if the internal write cycle has completed
will the EEPROM respond with a zero, allowing the read or write sequence to continue.

How can I do ACK polling with you library?

[valkuc]

Using "classic" i2c_master lib this implemented next way:

do {
    i2c_master_start();
    i2c_master_writeByte((uint8)((AT24C_ADDR << 1) | 1));
} while (!i2c_master_checkAck());
i2c_master_stop();

[pasko-zh]

In i2c there is a mechanism called clock stretching, which is used by slaves to signal to the master to wait, i.e. they are pulling SCL low. This is fully supported with brzo i2c.

Now, it seems that this device is implementing this somehow with SDA. From the description in the datasheet I understood ACK polling this way:

After the 8th SCL cycle has finished, the 9th cycle begins, and on the raising edge of SCL the master samples SDA. If SDA is high then it means NACK by the slave. This is the standard i2c operation. Now, this guy seems to keep SDA high for twr msecs (i.e. it is not yet ACKnowledging), before it will pull SDA low, to signal an ACK. i.e., it "stretches" SDA. The datasheet doesn't say what the slave is doing during twr msec with SCL. I guess, it won't pull it down (i.e. not stretching it). If the device is not pulling SCL low, then brzo i2c will read SDA being high and interprete it as NACK.

So, it looks to me that this is very very special way of "clock" (i.e. SDA) stretching.... and I even would go that far to say that it is not really according to the i2c standard. What you would need to do is to wait twr msec after writes with a delay(twr), where twr is 10 msec according to the datasheet if vcc > 2.5V.

I also had a look in the microchip 24AA256/24LC256/24FC256 datasheet under section 7.0, there is a diagram explaining it better (for my understanding at least ;-) If the device could work with a STOP sequence after an NACK was returned, then you can do it with brzo i2c, otherwise not.

[valkuc]

Looks like Microchip EEPROM have same logic for "ACK polling" as Atmel. So in your opinion I just need to set clock stretching to 10000 before issuing write command to EEPROM?

UPD: It's possible to implement this "ACK polling" in assembly in your library?

[pasko-zh]

1. I have some doubts that during ACK polling, SCL is pulled low. It is not clear, at least to me, from the datasheets. But since they did not mention anything on SCL stretching, I think it won't work with clock stretching. Since I don't have the EEPROM I cannot try myself.
   Could you give this a try, please?
2. If I understood the datasheet correctly, then this ACK polling is not mandatory: "Once the Stop condition for a Write command has been issued from the master, the device initiates the internally timed write cycle. ACK polling can be initiated immediately." Therefore you could maybe do something like: Write one or more byte in one brzo transaction, after that use delay(10) to allow the internal write to happen and then go on with other brzo transactions.
   Could you give this a try, too, please?
3. I wrote the library to be as close to the i2c specs as possible. Therefore I would have to implement an additional function, something like brzo_i2c_ACK_polling(max_timeout).

[valkuc]

1. Sure I will try it today evening (after my main work)
2. Yes, it's actually not mandatory. It was specially introduced to minimize delays between consequent eeprom write operations. So actually I can just use delay(10ms), but if there is a possibility to use approach specially designed for this purpose - why not use it.
3. Well, it would be nice to have such functionality if clock stretching will not work. I2C EEPROMs sometimes can be quite useful compared to SPI flash and it would be nice if you library can work with them.

P.S.
In my project there is DS3231 module with AT24C32 EEPROM onboard. Currently I use "i2c_master" library to interface both DS3231 and EEPROM chips, but sometimes read/write operations get failed for unknown for me reason (probably timing issues and occured interrupts). So I decided to rework my code using your I2C driver.

[valkuc]

Here is my investigation.
I was able to do some emulation of ASK polling by doing next:

uint8 data[1];
do
{
    brzo_i2c_start_transaction(AT24C_ADDR, 100);
    brzo_i2c_read(data, 1, false);
}
while(brzo_i2c_end_transaction() != 0);

but this more looks like workaround than correct approach. Here is decoded signal from Logic Analizer, writing at address 0 array of 10 bytes all value 3:

Time [s], Analyzer Name, Decoded Protocol Result
0.000005000000000,I2C,Setup Write to ['160'] + ACK
0.000097500000000,I2C,'0' + ACK
0.000189500000000,I2C,'0' + ACK
0.000385500000000,I2C,'3' + ACK
0.000477500000000,I2C,'3' + ACK
0.000569500000000,I2C,'3' + ACK
0.000661500000000,I2C,'3' + ACK
0.000754000000000,I2C,'3' + ACK
0.000846000000000,I2C,'3' + ACK
0.000938000000000,I2C,'3' + ACK
0.001030000000000,I2C,'3' + ACK
0.001122000000000,I2C,'3' + ACK
0.001214000000000,I2C,'3' + ACK
0.001330500000000,I2C,Setup Read to ['161'] + NAK
0.001446500000000,I2C,Setup Read to ['161'] + NAK
0.001562500000000,I2C,Setup Read to ['161'] + NAK
0.001678500000000,I2C,Setup Read to ['161'] + Missing ACK/NAK
0.001794500000000,I2C,Setup Read to ['161'] + Missing ACK/NAK
0.001910500000000,I2C,Setup Read to ['161'] + NAK
0.002026500000000,I2C,Setup Read to ['161'] + NAK
0.002142000000000,I2C,Setup Read to ['161'] + NAK
0.002258000000000,I2C,Setup Read to ['161'] + NAK
0.002374000000000,I2C,Setup Read to ['161'] + NAK
0.002490000000000,I2C,Setup Read to ['161'] + ACK
0.002582000000000,I2C,'255' + NAK

And here is what actually should be:

Time [s], Analyzer Name, Decoded Protocol Result
0.000016000000000,I2C,Setup Write to ['160'] + ACK
0.000242000000000,I2C,'0' + ACK
0.000451500000000,I2C,'0' + ACK
0.000661500000000,I2C,'1' + ACK
0.000870000000000,I2C,'1' + ACK
0.001078000000000,I2C,'1' + ACK
0.001286000000000,I2C,'1' + ACK
0.001494500000000,I2C,'1' + ACK
0.001702500000000,I2C,'1' + ACK
0.001911000000000,I2C,'1' + ACK
0.002119000000000,I2C,'1' + ACK
0.002327500000000,I2C,'1' + ACK
0.002535500000000,I2C,'1' + ACK
0.002796000000000,I2C,Setup Read to ['161'] + NAK
0.003025000000000,I2C,Setup Read to ['161'] + NAK
0.003254000000000,I2C,Setup Read to ['161'] + NAK
0.003483000000000,I2C,Setup Read to ['161'] + NAK
0.003712000000000,I2C,Setup Read to ['161'] + NAK
0.003941000000000,I2C,Setup Read to ['161'] + ACK

While investigated that, I found one thing that makes your library hard to use for EEPROM write operations. Pay attention on my Logic Analizer output, notice two "0" after "Setup write" and then ten "1". Zeroes is the target EEPROM address, ten "1" - is data. The tricky part here is that if I will use two separate calls to brzo_i2c_write I will have additional "Setup write" before address and data - and this is error. Address and Data must be sent one after another, so imagine that I have next function prototype to write data to EEPROM:
bool at24c_write(uint16_t addr, uint8_t* data, uint8_t len)
I can't write body like that because in this case I will have extra "Setup write" between address and data transmission:

uint8_t data_addr[2];
data_addr[0] = (uint8_t)(((unsigned)addr) >> 8);
data_addr[1] = (uint8_t)(((unsigned)addr) & 0xff);
brzo_i2c_write(data_addr, 2, true);
brzo_i2c_write(data, len, false);

To overrun this I need to malloc temporary array with size of data + 2, copy first two bytes of address into it, then copy data and after that send it in one brzo_i2c_write call. I guess this looks like redundant memory allocation:

uint8 *d = os_malloc(len + 2);
d[0] = (uint8)(((unsigned)addr) >> 8);
d[1] = (uint8)(((unsigned)addr) & 0xff);
os_memcpy(d+2, data, len);
brzo_i2c_write(d, len+2, false);
os_free(d);

So actually it would be very nice to have ability to control I2C at a bit lower level than you currently provide. Compared to "i2c_master", your library provide ideal SCL clock and timings and with you lib I get rid of my problem that I mentioned in previous post. But in current implementation it's not very comfortable to use it. By the way, there is an issue #1 - using Arduino "Wire" interface EEPROM write operation can be done like this:

Wire.beginTransmission(m_deviceid);
Wire.write((int)(add >> 8));   // left-part of pointer address
Wire.write((int)(add & 0xFF)); // and the right
Wire.write(data);
Wire.endTransmission();

[valkuc]

Here is some ideas:

1. Move device address transmission to separate function (maybe in start_transaction?) or introduce bool argument to write function to control send or not to send device address.
2. Allow read function to be called without arguments to just transmit device address and direction.

[pasko-zh]

A big thank you for your investigations 👏

Since my "other work" keeps me rather busy these days, my answer is a bit short:

• I thought of a kind of ACK emulation as well, but with some loops of brzo_i2c_write(..., ..., true) for the ACK polling instead, i.e. omitting the STOP while polling (cf. figure 7-1 in the microchip datasheet), but... I didn't suggest it, because I felt that's an unnecessary overhead/complication and doesn't really (properly) solve the ACK polling...
• I have to find some time for 1) and 2) and the other part in your answer.

btw: Since you mentioned "i2c_master", are you using the native SDK only or are you on the arduino toolchain?

EDIT: Will take a look at ACK Polling this weekend.

[valkuc]

Yes, I'm working with native non-os SDK (https://github.com/CHERTS/esp8266-devkit). Your library does not work out-of-box with it, but with minor modifications (like changing boolean to bool and replacing pin related arguments and code from brzo_i2c_setup to native GPIO manipulations with PIN_FUNC_SELECT and GPIO_REG_WRITE) - all ok.

About ACK polling, it looks like really good thing because in my examples using ACK polling I have achieved only 1.4ms EEPROM post-write delay instead of hard-coded 10ms. So it was required to wait less more than in 5 times.

[mroavi]

I'm also trying to get the brzo_i2c library to work with a native non-os SDK (https://github.com/pfalcon/esp-open-sdk) . The compiler is giving me an error that I don't know how to fix:

modules/brzo_i2c.c:67:2: error: can't find a register in class 'RL_REGS' while reloading 'asm' asm volatile ( ^ modules/brzo_i2c.c:67:2: error: 'asm' operand has impossible constraints make: *** [build/modules/brzo_i2c.o] Error 1

The line it points to is the asm volatile ( inside the brzo_i2c_write function.

valkuc: Did you also run into this problem? Any idea of how to fix it?

[valkuc]

No, I did not have this issue. Probably you miss necessary includes.

[mroavi]

Perhaps it could also be related to the fact that I had to remove the ICACHE_RAM_ATTR specifiers since the compiler was complaining as well. This is what it outputs:

modules/brzo_i2c.c:49:22: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'brzo_i2c_write'
 void ICACHE_RAM_ATTR brzo_i2c_write(uint8_t *data, uint8_t no_of_bytes, bool repeated_start) {
                      ^
modules/brzo_i2c.c:350:22: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'brzo_i2c_read'

Did you have issues with these specifiers as well?

[valkuc]

Switch to gnu99 standard to solve issue with "asm volatile" (i.e. -std=gnu99 in CFLAGS). Or gnu89 if you currently on c89.
UPD: nevermind, yes there is no ICACHE_RAM_ATTR definition in native SDK.

[mroavi]

Didn't help. I get the same error using either -std=gnu99 or -std=gnu89 (with -std=gnu99 I get a bunch of new errors). By the way, I'm using the newest SDK from espressif: ESP8266_NONOS_SDK_V2.0.0_16_07_19.

The CFLAG variable in my Makefile looks like this:

# compiler flags using during compilation of source files
CFLAGS      =   -g          \
                -Wpointer-arith     \
                -Wundef         \
                -Wl,-EL         \
                -fno-inline-functions   \
                -nostdlib       \
                -mlongcalls \
                -mtext-section-literals \
                -ffunction-sections \
                -fdata-sections \
                -fno-builtin-printf\
                -DICACHE_FLASH \
                -DBUID_TIME=\"$(DATETIME)\" \
                -std=gnu89

[pasko-zh]

@valkuc : Great that you managed to compile it with the native SDK! I had a longer discussion with Pete Scargill about compiling my lib with the native SDK. As it seems, he was not successful.
Would you mind making your bzro-i2c version for the native SDK available? Or even make pull request to this repo, something like /nativeSDK for the sources?