Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C stability hypothesis #745

Closed
stickbreaker opened this issue Oct 19, 2017 · 8 comments
Closed

I2C stability hypothesis #745

stickbreaker opened this issue Oct 19, 2017 · 8 comments

Comments

@stickbreaker
Copy link
Contributor

Hardware:

Board: WeMos Bluetooth& Battery, (Using ESP32 Dev Module in Arduino)
Core Installation/update date: 05/OCT/2017?
IDE name: Arduino IDE 1.8.5
Flash Frequency: 80Mhz
Upload Speed: 921600

Description:

I am getting different results from the I2C subsystem depending on how often(fast) calls are made.

I am accessing 24LCxx EEPROMS, after a data write sequence, the chip can take up to 5ms to preform the write. While this internal cycle is completing the EEPROM will not ACK its address. So after a write command, I can either delay for 5ms (worst case) or poll the EEPROM until it ACK's.

first Sketch

This sketch is producing unexpected I2C error codes. (5) I2C_ERROR_BUSY

uint32_t notReady=0;

bool i2cReady(uint8_t adr){
uint32_t timeout=millis();
bool ready=false;
while((millis()-timeout<100)&&(!ready)){
	Wire.beginTransmission(adr);
	int err=Wire.endTransmission();
	ready=(err==0);
	if(!ready){
		notReady++; // howmany times this loop executed
		if(err!=2) Serial.printf("{%d}",err); // 2 is NAK, I am only expecting 0:ok or 2:NAK
		}
	}
return ready;
}

Debug output

I have ERROR level debugging enabled.

[E][esp32-hal-i2c.c:160] i2cWrite(): Busy Timeout! Addr: 50
{5}[E][esp32-hal-i2c.c:160] i2cWrite(): Busy Timeout! Addr: 50
{5}0x50:  0  0  2  0 4k Bytes 2 

each of the {5} are unexpected errors. The [esp32-hal-i2c.c:160] should not have occurred.

Second Sketch that functions as expected:

uint32_t notReady=0;

bool i2cReady(uint8_t adr){
uint32_t timeout=millis();
bool ready=false;
while((millis()-timeout<100)&&(!ready)){
	Wire.beginTransmission(adr);
	int err=Wire.endTransmission();
	ready=(err==0);
	if(!ready){
		notReady++;
//		if(err!=2)
			Serial.printf("{%d}",err);
		}
	}
return ready;
}

Second Debug Output

ERROR level debugging is still enabled

{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}{2}0x50:  0  0  16  0 4k Bytes 14 

The only difference between these two examples is that the second one sends a three character Serial output each time Wire.endTransmission() returns an error(which slows down the rate of I2C calls). The first one only sends these three characters if (err!=2) ( much faster I2C calls).

What I think is happening is that the I2C bus is still returning to IDLE from the STOP bit. The subsystem is correctly reporting the 'bus' is busy because the pullup on the SDA line only supplies a finite current.

I will see if I can create a test circuit and use my scope to capture events. I'll modify i2cWrite() to generate a trigger pulse and compare it to SDA and SCL.

Chuck.

@me-no-dev
Copy link
Member

what does the documentation for the eeprom say about how long is the wait time?

@stickbreaker
Copy link
Contributor Author

@me-no-dev

Microchip Technology Inc.

Data Sheet DS21754M (c) 2010 24LC512

pg.4  AC Characteristics Param. No. 17 Twc Write Cycle Time (byte or page) Max is 5ms.

pg.12 ACKNOWLEDGE POLLING:  
Since the device will not acknowledge during a write cycle, this can be used to determine
when the cycle is complete (this feature can be used to maximize bus throughput).

As you can see ACK polling is a standard method used to determine when the internal write cycle has completed. The current HAL-I2C is not correctly implementing the I2C subsystem.

I think the esp32-hal-i2c.c:237-239 is the genesis of this problem. i2c->dev->command[2].done is set to true as soon as the command is parsed. The STOP bit has not cycled through the I2C State Machine to the bus. I think I2C_TRANS_COMPLETE_INT must be used to detect when the 'Master' has finished the STOP command.

Is there a reason why HAL-I2C does not utilize the I2C controller interrupts?

Would is be possible/acceptable if I wrote a HAL-I2C that was interrupt driven? I have a few ideas about the structure and flow. I would like a I2C implementation that is not restricted to the 32 byte Arduino maximum transmission length. Reading through the ESP32 Technical Reference Manual V2.4 , I think I understand the theory, I would like to attempt a solution.
I need a 'best practice' ISR example, with valid syntax (IRAM declarations, WatchDog timeout rules, semaphore names, app callback rules, message dispatch ... )

Chuck.

@lonerzzz
Copy link
Contributor

@stickbreaker Good analysis. I think you are on to something.

I am just finishing a version of the HAL-I2C library to handle longer than 32 byte messages because the END command mechanism is broken on the current chip so I am using an alternative approach that avoids its usage. My library works for longer messages but I was still having the occasional problem of instability and had not yet started to play with the interrupts.

Part of the problem is that the interrupts may also have issues, they do for the END command. I will play with that tomorrow in the context of my library version. I will let you know what I find. If you have other observations. Please pass them along.

@stickbreaker
Copy link
Contributor Author

@lonerzzz Where are the errata for this chip? I am interested in using the ESP32, but, alas, I heavily use I2C.

Is your work on github? I would be interested in your solution. I was thinking about how the END command could be use for bigger blocks.

I spent the morning reading through the ESP-IDF about interrupts and external sources. Am I correct that, a single interrupt can be assigned to multiple sources. What I mean, would it make sense if I used

esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusreg, uint32_t intrstatusmask, intr_handler_t handler, void *arg, intr_handle_t *ret_handle)

with intrstatusreg and intrstatusmask for each of the possible I2C interrupts, or allocate one interrupt with a mask covering all of the individual interrupts?

I can see were the *arg value could be used for a global control block pointer, or a event control block containing an interrupt identifier, and the global I2C control block pointer.

I don't know enough to decide which method would be better, adding multiple ISR's, one for each event or one ISR that discovers which events are pending. Also, the Documentation is pretty light on interactions and timing considerations.

I would be interested in looking over you shoulder, maybe 'interfering'. 😏

Chuck.

@lonerzzz
Copy link
Contributor

@stickbreaker Errata would be very handy, good documentation would be nice. Unfortunately, it is sparse. I too am heavily dependent on I2C so have had to tackle the issue myself. The support folks on the IDF forums haven't even formally acknowledged the I2C problems as far as I have seen despite different observations from different users. I will put the code up later today on a branch. It handles the long messages but suffers from the same intermittent issues that I had not yet had a chance to dive deeper into.

As I have not dived too deep into the interrupts so far, I am not the best person to answer. I am about where you are in looking at the reference manual for this. That said, you can look at the status registers to see which interrupts are pending without writing the actual handlers. That was how others and myself discovered the shortcomings of the hardware so far discovered.

@stickbreaker
Copy link
Contributor Author

stickbreaker commented Oct 20, 2017

@lonerzzz I just posted an idea about END operation in #682, maybe it is actually functioning as designed.

Chuck.

@stickbreaker
Copy link
Contributor Author

stickbreaker commented Oct 20, 2017

Verified NAK causes early return from I2C Subsystem BEFORE Stop has completed!

Here is cobbled together NAK screen capture, in the upper right corner by the Red & Black arrow are two vertical blue ticks, they mark the NAK exit from the Endless while(1) in i2cWrite().
NAK_Exploded.jpg

I modified the NAK exit to generate timing marks. As you can see the Second blip is just prior to the return. If another I2C transaction is started quickly, the STOP might still be in progress.

if(i2c->dev->int_raw.ack_err) {

//Transmission did not finish and ACK_ERR is set
    if(i2c->dev->int_raw.ack_err) {
  digitalWrite(4,HIGH); // first tick
  digitalWrite(4,LOW);
       log_w("Ack Error! Addr: %x", address >> 1);
       while((i2c->dev->status_reg.bus_busy) && ((millis() - startAt)>50));
       I2C_MUTEX_UNLOCK();
  digitalWrite(4,HIGH);  // Second Tick
  digitalWrite(4,LOW);
  return I2C_ERROR_ACK;
}

I'll work (done #750) on a temp fix while @lonerzzz produces his fork.

Chuck.

p.s. also, that status_reg.bus_busy&&((millis() - startAt)>50) seams bogus; As I read it, it says: " if the bus is busy and this transaction has taken over 50 milliseconds, wait until the bus is no longer busy." a second corollary would be: "If this is a short transaction don't wait for the bus to clear!"

Should it say: "If the bus is busy and it has been LESS than 50ms wait up to 50ms or until the bus is clear"

@stickbreaker
Copy link
Contributor Author

Found coding error in esp-hal-i2c.c.

Submitted pull #750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants