Skip to content

Commit

Permalink
Added fixes for priority escalation
Browse files Browse the repository at this point in the history
  • Loading branch information
nox771 committed Jan 3, 2018
1 parent 91d511e commit 0a1b061
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 61 deletions.
120 changes: 72 additions & 48 deletions i2c_t3.cpp
Expand Up @@ -54,6 +54,8 @@ struct i2cStruct i2c_t3::i2cData[] =
#endif
};

volatile uint8_t i2c_t3::isrActive = 0;


// ------------------------------------------------------------------------------------------------------
// Constructor/Destructor
Expand Down Expand Up @@ -386,44 +388,42 @@ void i2c_t3::setRate_(struct i2cStruct* i2c, uint32_t busFreq, uint32_t i2cFreq)

uint8_t i2c_t3::pinConfigure_(struct i2cStruct* i2c, uint8_t bus, uint8_t pinSCL, uint8_t pinSDA, i2c_pullup pullup, uint8_t reconfig)
{
uint8_t validAlt, retval=0;
uint8_t validAltSCL, validAltSDA;
volatile uint32_t* pcr;

if(reconfig && (*(i2c->S) & I2C_S_BUSY)) return 0; // if reconfig return immediately if bus busy (reconfig=0 for init)

// Verify new SCL pin is different and valid, or reconfig=0 (re-init)
//
validAlt = validPin_(bus, pinSCL, 1);
if((pinSCL != i2c->currentSCL && validAlt) || !reconfig)
validAltSCL = validPin_(bus, pinSCL, 1);
if((pinSCL != i2c->currentSCL && validAltSCL) || !reconfig)
{
// If reconfig set, switch previous pin to non-I2C input
if(reconfig) pinMode(i2c->currentSCL, (i2c->currentPullup == I2C_PULLUP_EXT) ? INPUT : INPUT_PULLUP);
// Config new pin
PIN_CONFIG_ALT(configSCL, validAlt);
PIN_CONFIG_ALT(configSCL, validAltSCL);
pcr = portConfigRegister(pinSCL);
*pcr = configSCL;
i2c->currentSCL = pinSCL;
i2c->currentPullup = pullup;
retval = 1;
}

// Verify new SDA pin is different and valid (not necessarily same Alt as SCL), or reconfig=0 (re-init)
//
validAlt = validPin_(bus, pinSDA, 2);
if((pinSDA != i2c->currentSDA && validAlt) || !reconfig)
validAltSDA = validPin_(bus, pinSDA, 2);
if((pinSDA != i2c->currentSDA && validAltSDA) || !reconfig)
{
// If reconfig set, switch previous pin to non-I2C input
if(reconfig) pinMode(i2c->currentSDA, (i2c->currentPullup == I2C_PULLUP_EXT) ? INPUT : INPUT_PULLUP);
// Config new pin
PIN_CONFIG_ALT(configSDA, validAlt);
PIN_CONFIG_ALT(configSDA, validAltSDA);
pcr = portConfigRegister(pinSDA);
*pcr = configSDA;
i2c->currentSDA = pinSDA;
i2c->currentPullup = pullup;
retval = 1;
}

return retval;
return (validAltSCL && validAltSDA);
}


Expand All @@ -438,7 +438,6 @@ uint8_t i2c_t3::pinConfigure_(struct i2cStruct* i2c, uint8_t bus, uint8_t pinSCL
uint8_t i2c_t3::acquireBus_(struct i2cStruct* i2c, uint8_t bus, uint32_t timeout, uint8_t& forceImm)
{
elapsedMicros deltaT;
int irqPriority, currPriority;

// update timeout
timeout = (timeout == 0) ? i2c->defTimeout : timeout;
Expand Down Expand Up @@ -489,48 +488,56 @@ uint8_t i2c_t3::acquireBus_(struct i2cStruct* i2c, uint8_t bus, uint32_t timeout
}
}

// For ISR operation, check if current routine has higher priority than I2C IRQ, and if so
// either escalate priority of I2C IRQ or send I2C using immediate mode
if(i2c->opMode == I2C_OP_MODE_ISR || i2c->opMode == I2C_OP_MODE_DMA)
{
currPriority = nvic_execution_priority();
switch(bus)
{
case 0: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C0); break;
#if defined(__MKL26Z64__) || defined(__MK20DX256__) || defined(__MK64FX512__) || defined(__MK66FX1M0__) // LC/3.1/3.2/3.5/3.6
case 1: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C1); break;
#endif
#if defined(__MK64FX512__) || defined(__MK66FX1M0__) // 3.5/3.6
case 2: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C2); break;
#endif
#if defined(__MK66FX1M0__) // 3.6
case 3: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C3); break;
#endif
default: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C0); break;
}
if(currPriority <= irqPriority)
#ifndef I2C_DISABLE_PRIORITY_CHECK
// For ISR operation, check if current routine has higher priority than I2C IRQ, and if so
// either escalate priority of I2C IRQ or send I2C using immediate mode.
//
// This check is disabled if the routine is called during an active I2C ISR (assumes it is
// called from ISR callback). This is to prevent runaway escalation with nested Wire calls.
//
int irqPriority, currPriority;
if(!i2c_t3::isrActive && (i2c->opMode == I2C_OP_MODE_ISR || i2c->opMode == I2C_OP_MODE_DMA))
{
if(currPriority < 16)
forceImm = 1; // current priority cannot be surpassed, force Immediate mode
else
currPriority = nvic_execution_priority();
switch(bus)
{
case 0: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C0); break;
#if defined(__MKL26Z64__) || defined(__MK20DX256__) || defined(__MK64FX512__) || defined(__MK66FX1M0__) // LC/3.1/3.2/3.5/3.6
case 1: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C1); break;
#endif
#if defined(__MK64FX512__) || defined(__MK66FX1M0__) // 3.5/3.6
case 2: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C2); break;
#endif
#if defined(__MK66FX1M0__) // 3.6
case 3: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C3); break;
#endif
default: irqPriority = NVIC_GET_PRIORITY(IRQ_I2C0); break;
}
if(currPriority <= irqPriority)
{
switch(bus)
if(currPriority < 16)
forceImm = 1; // current priority cannot be surpassed, force Immediate mode
else
{
case 0: NVIC_SET_PRIORITY(IRQ_I2C0, currPriority-16); break;
#if defined(__MKL26Z64__) || defined(__MK20DX256__) || defined(__MK64FX512__) || defined(__MK66FX1M0__) // LC/3.1/3.2/3.5/3.6
case 1: NVIC_SET_PRIORITY(IRQ_I2C1, currPriority-16); break;
#endif
#if defined(__MK64FX512__) || defined(__MK66FX1M0__) // 3.5/3.6
case 2: NVIC_SET_PRIORITY(IRQ_I2C2, currPriority-16); break;
#endif
#if defined(__MK66FX1M0__) // 3.6
case 3: NVIC_SET_PRIORITY(IRQ_I2C3, currPriority-16); break;
#endif
default: NVIC_SET_PRIORITY(IRQ_I2C0, currPriority-16); break;
switch(bus)
{
case 0: NVIC_SET_PRIORITY(IRQ_I2C0, currPriority-16); break;
#if defined(__MKL26Z64__) || defined(__MK20DX256__) || defined(__MK64FX512__) || defined(__MK66FX1M0__) // LC/3.1/3.2/3.5/3.6
case 1: NVIC_SET_PRIORITY(IRQ_I2C1, currPriority-16); break;
#endif
#if defined(__MK64FX512__) || defined(__MK66FX1M0__) // 3.5/3.6
case 2: NVIC_SET_PRIORITY(IRQ_I2C2, currPriority-16); break;
#endif
#if defined(__MK66FX1M0__) // 3.6
case 3: NVIC_SET_PRIORITY(IRQ_I2C3, currPriority-16); break;
#endif
default: NVIC_SET_PRIORITY(IRQ_I2C0, currPriority-16); break;
}
}
}
}
}
#endif

return 1;
}

Expand Down Expand Up @@ -1152,6 +1159,7 @@ void i2c0_isr(void) // I2C0 ISR
void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
{
uint8_t status, c1, data;
i2c_t3::isrActive++;

status = *(i2c->S);
c1 = *(i2c->C1);
Expand Down Expand Up @@ -1211,6 +1219,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->S) = I2C_S_IICIF; // clear intr
if(i2c->user_onError != nullptr) i2c->user_onError(); // run Error callback if DMA error or ARBL
}
i2c_t3::isrActive--;
return;
} // end DMA Tx
else
Expand Down Expand Up @@ -1285,6 +1294,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->S) = I2C_S_IICIF; // clear intr
}
}
i2c_t3::isrActive--;
return;
}
else if(i2c->currentStatus == I2C_SEND_ADDR)
Expand Down Expand Up @@ -1332,6 +1342,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
data = *(i2c->D); // dummy read
*(i2c->S) = I2C_S_IICIF; // clear intr
}
i2c_t3::isrActive--;
return;
}
else if(i2c->currentStatus == I2C_TIMEOUT)
Expand All @@ -1344,6 +1355,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->S) = I2C_S_IICIF; // clear intr
I2C_ERR_INC(I2C_ERRCNT_TIMEOUT);
if(i2c->user_onError != nullptr) i2c->user_onError(); // run Error callback if timeout
i2c_t3::isrActive--;
return;
}
else
Expand All @@ -1352,6 +1364,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
// send STOP, change to Rx mode, intr disabled
*(i2c->C1) = I2C_C1_IICEN;
*(i2c->S) = I2C_S_IICIF; // clear intr
i2c_t3::isrActive--;
return;
}
} // end ISR Tx
Expand Down Expand Up @@ -1401,6 +1414,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
I2C_ERR_INC(I2C_ERRCNT_DMA_ERR);
if(i2c->user_onError != nullptr) i2c->user_onError(); // run Error callback if DMA error
}
i2c_t3::isrActive--;
return;
}
else
Expand Down Expand Up @@ -1445,6 +1459,8 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
}
if(i2c->currentStatus == I2C_TIMEOUT && !i2c->timeoutRxNAK)
i2c->timeoutRxNAK = 1; // set flag to indicate NAK sent

i2c_t3::isrActive--;
return;
}
}
Expand All @@ -1467,6 +1483,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->FLT) |= I2C_FLT_STOPF | I2C_FLT_STARTF; // clear STOP/START intr
#endif
*(i2c->S) = I2C_S_IICIF; // clear intr
i2c_t3::isrActive--;
return;
}
}
Expand Down Expand Up @@ -1516,6 +1533,7 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->FLT) |= I2C_FLT_STOPF | I2C_FLT_STARTF; // clear STOP/START intr
#endif
*(i2c->S) = I2C_S_IICIF; // clear intr
i2c_t3::isrActive--;
return;
}
if(c1 & I2C_C1_TX)
Expand Down Expand Up @@ -1552,11 +1570,12 @@ void i2c_isr_handler(struct i2cStruct* i2c, uint8_t bus)
*(i2c->S) = I2C_S_IICIF; // clear intr
i2c->currentStatus = I2C_WAITING;
// Slave Rx complete, run callback
if(i2c->user_onReceive != nullptr)
if(i2c->user_onReceive != nullptr)
{
i2c->rxBufferIndex = 0;
i2c->user_onReceive(i2c->rxBufferLength);
}
i2c_t3::isrActive--;
return;
}
#endif
Expand Down Expand Up @@ -1647,6 +1666,11 @@ i2c_t3 Wire = i2c_t3(0); // I2C0
Changelog
------------------------------------------------------------------------------------------------------
- (v10.1) Modified 02Jan18 by Brian (nox771 at gmail.com)
- Added User #define to disable priority checks entirely
- Added i2c_t3::isrActive flag to dynamically disable priority checks during ISR & callbacks.
This is to prevent runaway priority escalation in cases of callbacks with nested Wire calls.
- (v10.0) Modified 21Oct17 by Brian (nox771 at gmail.com)
- Default assignments have been added to many functions for pins/pullup/rate/op_mode, so
all those parameters are now optional in many function calls (marked ^ below)
Expand Down
48 changes: 35 additions & 13 deletions i2c_t3.h
Expand Up @@ -102,14 +102,25 @@
//#define I2C_AUTO_RETRY

// ------------------------------------------------------------------------------------------------------
// Error counters - uncomment to make the library track error counts. Error counts can be retrieved or
// zeroed using the getErrorCount() and zeroErrorCount() functions respectively.
// When included, errors will be tracked on the following (Master-mode only):
// Reset Bus (auto-retry only), Timeout, Addr NAK, Data NAK, Arb Lost, Bus Not Acquired,
// Error counters - uncomment to make the library track error counts. Error counts can be retrieved or
// zeroed using the getErrorCount() and zeroErrorCount() functions respectively.
// When included, errors will be tracked on the following (Master-mode only):
// Reset Bus (auto-retry only), Timeout, Addr NAK, Data NAK, Arb Lost, Bus Not Acquired,
// DMA Errors
//
#define I2C_ERROR_COUNTERS

// ------------------------------------------------------------------------------------------------------
// Disable priority check - uncomment this to entirely disable auto priority escalation. Normally
// priority escalation occurs to insure I2C ISR operates at a higher priority
// than the calling function (to prevent ISR stall if the calling function
// blocks). Uncommenting this will disable the check and cause I2C ISR to
// remain at default priority. It is recommended to disable this check and
// manually set ISR priority levels when using complex configurations.
//
//#define I2C_DISABLE_PRIORITY_CHECK


// ======================================================================================================
// == End User Define Section ===========================================================================
// ======================================================================================================
Expand Down Expand Up @@ -450,6 +461,12 @@ class i2c_t3 : public Stream
// I2C structure pointer - this is a local, passed as an argument to base functions
// since static functions cannot see it.
struct i2cStruct* i2c;
//
// I2C ISR Active flag - this is used to disable priority escalation. Increment to 1 to disable priority
// check. This is a global flag to prevent complex cross-bus issues. It is only
// incremented/decremented, not set.
//
static volatile uint8_t isrActive;

// ------------------------------------------------------------------------------------------------------
// Constructor
Expand Down Expand Up @@ -482,7 +499,7 @@ class i2c_t3 : public Stream
// Initialize I2C (Master) - initializes I2C as Master mode, external pullups, 100kHz rate,
// and default pin setting
// default pin setting SCL/SDA:
// Wire: 19/18
// Wire: 19/18
// Wire1: 29/30 (3.1/3.2), 22/23 (LC), 37/38 (3.5/3.6)
// Wire2: 3/4 (3.5/3.6)
// Wire3: 57/56 (3.6)
Expand All @@ -494,7 +511,7 @@ class i2c_t3 : public Stream
// Initialize I2C (Slave) - initializes I2C as Slave mode using address, external pullups, 100kHz rate,
// and default pin setting
// default pin setting SCL/SDA:
// Wire: 19/18
// Wire: 19/18
// Wire1: 29/30 (3.1/3.2), 22/23 (LC), 37/38 (3.5/3.6)
// Wire2: 3/4 (3.5/3.6)
// Wire3: 57/56 (3.6)
Expand All @@ -516,8 +533,8 @@ class i2c_t3 : public Stream
// ^pins = pins to use, can be specified as 'i2c_pins' enum,
// or as 'SCL,SDA' pair (using any valid SCL or SDA), options are:
// Interface Devices Pin Name SCL SDA Default
// --------- ------- -------------- ----- ----- -------
// Wire All I2C_PINS_16_17 16 17
// --------- ------- -------------- ----- ----- -------
// Wire All I2C_PINS_16_17 16 17
// Wire All I2C_PINS_18_19 19* 18 +
// Wire 3.5/3.6 I2C_PINS_7_8 7 8
// Wire 3.5/3.6 I2C_PINS_33_34 33 34
Expand Down Expand Up @@ -646,8 +663,8 @@ class i2c_t3 : public Stream
// pins = pins to use, can be specified as 'i2c_pins' enum,
// or as 'SCL,SDA' pair (using any valid SCL or SDA), options are:
// Interface Devices Pin Name SCL SDA Default
// --------- ------- -------------- ----- ----- -------
// Wire All I2C_PINS_16_17 16 17
// --------- ------- -------------- ----- ----- -------
// Wire All I2C_PINS_16_17 16 17
// Wire All I2C_PINS_18_19 19* 18 +
// Wire 3.5/3.6 I2C_PINS_7_8 7 8
// Wire 3.5/3.6 I2C_PINS_33_34 33 34
Expand Down Expand Up @@ -677,7 +694,7 @@ class i2c_t3 : public Stream
// ------------------------------------------------------------------------------------------------------
// Get SCL/SDA - get the current SCL or SDA pin
// return: pin used
//
//
inline uint8_t getSCL(void) { return i2c->currentSCL; }
inline uint8_t getSDA(void) { return i2c->currentSDA; }

Expand Down Expand Up @@ -743,7 +760,7 @@ class i2c_t3 : public Stream
// used to indicate if command should end with a STOP (I2C_STOP) or not (I2C_NOSTOP). Use
// done(), finish(), or onTransmitDone() callback to determine completion and
// status() to determine success/fail. Note that sendTransmission() does not currently
// support timeouts (aside from initial bus acquisition which does support it).
// support timeouts (aside from initial bus acquisition which does support it).
// return: none
// parameters:
// ^i2c_stop = I2C_NOSTOP, I2C_STOP (default STOP)
Expand Down Expand Up @@ -780,7 +797,7 @@ class i2c_t3 : public Stream
// Start Master Receive - non-blocking routine, starts request for length bytes from slave at address. Receive
// data will be placed in the Rx buffer. i2c_stop parameter can be used to indicate if
// command should end with a STOP (I2C_STOP) or not (I2C_NOSTOP). Use done(), finish()
// or onReqFromDone() callback to determine completion and status() to determine
// or onReqFromDone() callback to determine completion and status() to determine
// success/fail.
// return: none
// parameters:
Expand Down Expand Up @@ -987,6 +1004,11 @@ extern i2c_t3 Wire;
Changelog
------------------------------------------------------------------------------------------------------
- (v10.1) Modified 02Jan18 by Brian (nox771 at gmail.com)
- Added User #define to disable priority checks entirely
- Added i2c_t3::isrActive flag to dynamically disable priority checks during ISR & callbacks.
This is to prevent runaway priority escalation in cases of callbacks with nested Wire calls.
- (v10.0) Modified 21Oct17 by Brian (nox771 at gmail.com)
- Default assignments have been added to many functions for pins/pullup/rate/op_mode, so
all those parameters are now optional in many function calls (marked ^ below)
Expand Down

0 comments on commit 0a1b061

Please sign in to comment.