Skip to content

Commit 0133ecc

Browse files
Avoid race conditions in I2C(Wire) callbacks (#995)
Fixes #979 Make sure to read the last byte of I2C data in the case where the IRQ happens and the STOP signal is also asserted. Also ensure all branches of the IRQ handler look at the same point in time value for the IRQ status.
1 parent 913dfad commit 0133ecc

File tree

1 file changed

+24
-15
lines changed

1 file changed

+24
-15
lines changed

libraries/Wire/src/Wire.cpp

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,22 @@ void TwoWire::begin(uint8_t addr) {
141141
}
142142

143143
void TwoWire::onIRQ() {
144-
if (_i2c->hw->intr_stat & (1 << 12)) {
144+
// Make a local copy of the IRQ status up front. If it changes while we're
145+
// running the IRQ callback will fire again after returning. Avoids potential
146+
// race conditions
147+
volatile uint32_t irqstat = _i2c->hw->intr_stat;
148+
149+
// First, pull off any data available
150+
if (irqstat & (1 << 2)) {
151+
// RX_FULL
152+
if (_slaveStartDet && (_buffLen < (int)sizeof(_buff))) {
153+
_buff[_buffLen++] = _i2c->hw->data_cmd & 0xff;
154+
} else {
155+
_i2c->hw->data_cmd;
156+
}
157+
}
158+
// RESTART_DET
159+
if (irqstat & (1 << 12)) {
145160
if (_onReceiveCallback && _buffLen) {
146161
_onReceiveCallback(_buffLen);
147162
}
@@ -150,13 +165,15 @@ void TwoWire::onIRQ() {
150165
_slaveStartDet = false;
151166
_i2c->hw->clr_restart_det;
152167
}
153-
if (_i2c->hw->intr_stat & (1 << 10)) {
168+
// START_DET
169+
if (irqstat & (1 << 10)) {
154170
_buffLen = 0;
155171
_buffOff = 0;
156172
_slaveStartDet = true;
157173
_i2c->hw->clr_start_det;
158174
}
159-
if (_i2c->hw->intr_stat & (1 << 9)) {
175+
// STOP_DET
176+
if (irqstat & (1 << 9)) {
160177
if (_onReceiveCallback && _buffLen) {
161178
_onReceiveCallback(_buffLen);
162179
}
@@ -165,25 +182,17 @@ void TwoWire::onIRQ() {
165182
_slaveStartDet = false;
166183
_i2c->hw->clr_stop_det;
167184
}
168-
if (_i2c->hw->intr_stat & (1 << 6)) {
169-
// TX_ABRT
185+
// TX_ABRT
186+
if (irqstat & (1 << 6)) {
170187
_i2c->hw->clr_tx_abrt;
171188
}
172-
if (_i2c->hw->intr_stat & (1 << 5)) {
173-
// RD_REQ
189+
// RD_REQ
190+
if (irqstat & (1 << 5)) {
174191
if (_onRequestCallback) {
175192
_onRequestCallback();
176193
}
177194
_i2c->hw->clr_rd_req;
178195
}
179-
if (_i2c->hw->intr_stat & (1 << 2)) {
180-
// RX_FULL
181-
if (_slaveStartDet && (_buffLen < (int)sizeof(_buff))) {
182-
_buff[_buffLen++] = _i2c->hw->data_cmd & 0xff;
183-
} else {
184-
_i2c->hw->data_cmd;
185-
}
186-
}
187196
}
188197

189198
void TwoWire::end() {

0 commit comments

Comments
 (0)