arduino / ArduinoCore-samd

Arduino Core for SAMD21 CPU
GNU Lesser General Public License v2.1
472 stars 720 forks source link

I2C handling out-of-order on PA08/PA09/SERCOM_ALT. #398

Open alexwhittemore opened 5 years ago

alexwhittemore commented 5 years ago

I've got a custom board using a SAMD21E18. In variant.h:

/* External I2C */
#define PIN_WIRE1_SDA         (3u)
#define PIN_WIRE1_SCL         (4u)
#define PERIPH_WIRE1          sercom2
#define WIRE1_IT_HANDLER      SERCOM2_Handler

static const uint8_t SDA1 = PIN_WIRE1_SDA;
static const uint8_t SCL1 = PIN_WIRE1_SCL;

Pin 3 is PA08, pin r is PA09.

This i2c works in my sketch using Wire1. The issue I'm running into is using this in slave mode.

Wire1.begin(SLAVE_ADDR);
Wire1.onRequest(requestEvent);
Wire1.onReceive(receiveEvent);

The software is configured such that the slave behaves like a common register-access I2C device - the master writes an address (1 byte), then sets up a read to get the data from that address.

Whenever my master (raspberry pi, as it happens) sends the 1-byte address payload, then subsequently tries to read the data back, the read operation (requestEvent) gets handled BEFORE the write operation (receive event), so that the address byte isn't properly set first. I've verified this by setting up time-stamping in those two functions - the requestEvent stores micros() when it happened, as does the receiveEvent, and next loop through the sketch they're printed out for comparison. I've also set a breakpoint on Wire.cpp:214 to capture the first I2C event that ever happens, and it's the request that gets handled rather than the receive.

For what it's worth, the Raspberry Pi is running the I2C bus at 50kHz, which seems slow. Perhaps that's somehow relevant?

To provide further evidence I'm not insane, the workaround is relatively simple - in the master code, any time I want to do a "read," I issue it twice - once to set the address (discard the returned data, since we know it's from a bogus address), and again to get the actual data desired, after a 1ms delay just for safety's sake.

Is there any sane reason the handler callbacks would be happening out-of-order from what's happening on the wire?

chris-apricity commented 5 years ago

@alexwhittemore Hey alex it's Chris Woodall. I am running into this issue too... Did you make any progress?

alexwhittemore commented 5 years ago

Nope :(. Current code in production employs the "double tap" workaround. But actually, we're not totally certain that fixes the problem totally - we've encountered rare failures where incorrect bits get returned in the response that may or may not be indicative of the same failed read address setting we see here. That could well be an electrical issue, though.

In the mean time, our longer-term plan is to transition from our current I2C register-based architecture to a command-response protocol running over UART (which currently handles some of the communication in our system).

Bummer, because I'd SURE love to fix this one.

chris-apricity commented 5 years ago

@alexwhittemore that is really unfortunate... It is kind of sad. I think the hardware should be capable of it, so it seems like it must be a software issue of some sort. I have had success by performing a write and then an address-less read, so it seems like it is a race condition of some sort, or maybe a priority issue with the callbacks (That seems fairly likely)

alexwhittemore commented 5 years ago

Out of curiosity, are you using those same 2 i2c pins? I haven't tested i2c slave functionality on any other pins, since the board I'm using is hardwired, but PA08 is a non-masking interrupt pin. It shouldn't matter: I believe non-masking only applies to the GPIO peripheral, so with the pins configured for SERCOM operation, that NMI functionality should be irrelevant. But then, I don't know that for sure.

chris-apricity commented 5 years ago

I am using PA23 and PA22 which are the defaults for Feather M0 which we used as our base design/prototype board

https://learn.adafruit.com/adafruit-feather-m0-basic-proto/pinouts

alexwhittemore commented 5 years ago

Interesting - then it sure seems like NMI must have nothing to do with it.

chris-apricity commented 5 years ago

It is all dispatched from this function it looks like. Data is available when onRequest is used. The current work around is to call onReceive from within onRequest (for us). We still get an impotent onReceive() call afterwards...

chris-apricity commented 5 years ago

My latest theory is that the check for !sercom->isMasterReadOperationWIRE() fails when a read request is started too fast after a write has occured, which blocks onReceive() from getting called... see this line

I haven't had a chance to check this thoroughly at the moment.

Currently the non-library workaround I have is

void onReceive(int size) {
  if (Wire.available()) {
  }
}

void onRequest(void) {
  #if defined(ARDUINO_ARCH_SAMD)
  if (Wire.available() == 1) {
    onReceive(1);
  }
  #endif 

  // Handle normal request logic

Which has some failure cases possibly. It might be good to take a closer look with a debugger.