Skip to content

Fix infinite recursion issue in SERCOM::startTransmissionWIRE #535

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Fix infinite recursion issue in SERCOM::startTransmissionWIRE #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6438545
Fix infinite recursion issue in SERCOM::startTransmissionWIRE
Jun 24, 2020
e576493
some small formatting changes and location of stop condition
Jul 7, 2020
a744583
fix Serial.flush() blocks forever #597
Apr 15, 2021
cbcb17a
fixed indent format in SERCOM.h, spaces to tabs
Apr 16, 2021
eca2581
bad assignment of setTimeout fixed (bool -> void)
Sep 30, 2021
0522249
change default wire timeout to 25ms (was 1000ms)
Oct 27, 2021
b30bf1c
take 2 on infinite recurssion issue in SERCOM::startTransmissionWIRE …
Nov 4, 2021
cc2883f
Update README.md
Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
This repository contains the source code and configuration files of the Arduino Core
for Atmel's SAMD21 processor (used on the Arduino/Genuino Zero, MKR1000 and MKRZero boards).

## My mods contained in 'wire_timeout' branch

This version of the core has the following changes:

1. A modified version of jrowberg's excellent i2c wire timeout implementation. See pull request [#439](https://github.com/arduino/ArduinoCore-samd/pull/439). By the way I've made the default timeout 25ms. An example modification of Adafruits SSD1306 library for a 128x64 OLED is [here](https://github.com/acicuc/Adafruit_SSD1306/tree/wire_timeout). Albeit old, it still demonstrates one use for the i2c timeout implementation.
2. A fix for the infamous infinite recursion SERCOM issue, see pull request [#535](https://github.com/arduino/ArduinoCore-samd/pull/535).
3. The Serial.flush() blocks forever fix, see issue [#597](https://github.com/arduino/ArduinoCore-samd/issues/597).

These are currently the main issues I've struck while working with the SAMD21 in the 'real' world. In particular 1&2 above have allowed for hot-swap and plug&play for devices on the i2c bus, and allows for complete device failure without locking up your entire system or blowing the stack! I'll add others as they raise their ugly heads within my projects.

## Installation on Arduino IDE

This core is available as a package in the Arduino IDE cores manager.
Expand Down
150 changes: 85 additions & 65 deletions cores/arduino/SERCOM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
SERCOM::SERCOM(Sercom* s)
{
sercom = s;
timeoutOccurred = false;
timeoutInterval = SERCOM_DEFAULT_I2C_OPERATION_TIMEOUT_MS;
}

/* =========================
Expand Down Expand Up @@ -111,12 +113,10 @@ void SERCOM::enableUART()

void SERCOM::flushUART()
{
// Skip checking transmission completion if data register is empty
if(isDataRegisterEmptyUART())
return;
// Wait for transmission to complete, if ok to do so.
while(!sercom->USART.INTFLAG.bit.TXC && onFlushWaitUartTXC);

// Wait for transmission to complete
while(!sercom->USART.INTFLAG.bit.TXC);
onFlushWaitUartTXC = false;
}

void SERCOM::clearStatusUART()
Expand Down Expand Up @@ -183,6 +183,10 @@ int SERCOM::writeDataUART(uint8_t data)

//Put data into DATA register
sercom->USART.DATA.reg = (uint16_t)data;

// indicate it's ok to wait for TXC flag when flushing
onFlushWaitUartTXC = true;

return 1;
}

Expand Down Expand Up @@ -393,11 +397,8 @@ void SERCOM::enableWIRE()

// Setting bus idle mode
sercom->I2CM.STATUS.bit.BUSSTATE = 1 ;
waitSyncWIRE();

while ( sercom->I2CM.SYNCBUSY.bit.SYSOP != 0 )
{
// Wait the SYSOP bit from SYNCBUSY coming back to 0
}
}

void SERCOM::disableWIRE()
Expand Down Expand Up @@ -433,10 +434,8 @@ void SERCOM::initSlaveWIRE( uint8_t ucAddress, bool enableGeneralCall )
SERCOM_I2CS_INTENSET_AMATCH | // Address Match
SERCOM_I2CS_INTENSET_DRDY ; // Data Ready

while ( sercom->I2CM.SYNCBUSY.bit.SYSOP != 0 )
{
// Wait the SYSOP bit from SYNCBUSY to come back to 0
}
waitSyncWIRE();

}

void SERCOM::initMasterWIRE( uint32_t baudrate )
Expand All @@ -453,12 +452,12 @@ void SERCOM::initMasterWIRE( uint32_t baudrate )
// Enable Smart mode and Quick Command
//sercom->I2CM.CTRLB.reg = SERCOM_I2CM_CTRLB_SMEN /*| SERCOM_I2CM_CTRLB_QCEN*/ ;


// Enable all interrupts
// sercom->I2CM.INTENSET.reg = SERCOM_I2CM_INTENSET_MB | SERCOM_I2CM_INTENSET_SB | SERCOM_I2CM_INTENSET_ERROR ;

// Synchronous arithmetic baudrate
sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / ( 2 * baudrate) - 5 - (((SystemCoreClock / 1000000) * WIRE_RISE_TIME_NANOSECONDS) / (2 * 1000));

}

void SERCOM::prepareNackBitWIRE( void )
Expand All @@ -485,71 +484,67 @@ void SERCOM::prepareCommandBitsWire(uint8_t cmd)
{
if(isMasterWIRE()) {
sercom->I2CM.CTRLB.bit.CMD = cmd;
waitSyncWIRE();
} else {
sercom->I2CS.CTRLB.bit.CMD = cmd;
}
}

void SERCOM::waitSyncWIRE() {
while(sercom->I2CM.SYNCBUSY.bit.SYSOP)
{
// Waiting for synchronization
}
} else {
sercom->I2CS.CTRLB.bit.CMD = cmd;
}
}

bool SERCOM::startTransmissionWIRE(uint8_t address, SercomWireReadWriteFlag flag)
{
bool restart = false; // transmission restart flag, is set to true on loss of arbitration

// 7-bits address + 1-bits R/W
address = (address << 0x1ul) | flag;

// If another master owns the bus or the last bus owner has not properly
// sent a stop, return failure early. This will prevent some misbehaved
// devices from deadlocking here at the cost of the caller being responsible
// for retrying the failed transmission. See SercomWireBusState for the
// possible bus states.
if(!isBusOwnerWIRE())
// mark timeout timer start
initTimeout();

do
{
if( isBusBusyWIRE() || (isArbLostWIRE() && !isBusIdleWIRE()) )
// Send start and address
sercom->I2CM.ADDR.bit.ADDR = address;
waitSyncWIRE();

// wait Address Transmitted
if ( flag == WIRE_WRITE_FLAG ) // Write mode
{
return false;
while (!sercom->I2CM.INTFLAG.bit.MB && !testTimeout())
{
// wait transmission complete
}
}
}

// Send start and address
sercom->I2CM.ADDR.bit.ADDR = address;

// Address Transmitted
if ( flag == WIRE_WRITE_FLAG ) // Write mode
{
while( !sercom->I2CM.INTFLAG.bit.MB )
else // Read mode
{
// Wait transmission complete
while (!sercom->I2CM.INTFLAG.bit.SB && !testTimeout())
{
// wait transmission complete
}
}

// Check for loss of arbitration (multiple masters starting communication at the same time)
if(!isBusOwnerWIRE())
if (!isBusOwnerWIRE() && !didTimeout())
{
// Restart communication
startTransmissionWIRE(address >> 1, flag);
}
}
else // Read mode
{
while( !sercom->I2CM.INTFLAG.bit.SB )
// small 1ms delay before restart
uint32_t start = micros();
while ((micros() - start) < 1000) yield();
restart = true;
} else
{
// If the slave NACKS the address, the MB bit will be set.
// In that case, send a stop condition and return false.
if (sercom->I2CM.INTFLAG.bit.MB) {
sercom->I2CM.CTRLB.bit.CMD = 3; // Stop condition
return false;
}
// Wait transmission complete
restart = false;
}

// Clean the 'Slave on Bus' flag, for further usage.
//sercom->I2CM.INTFLAG.bit.SB = 0x1ul;
}

} while (restart && !didTimeout());

//ACK received (0: ACK, 1: NACK)
if(sercom->I2CM.STATUS.bit.RXNACK)
//ACK received (0: ACK, 1: NACK) or timeout
if(sercom->I2CM.STATUS.bit.RXNACK || didTimeout())
{
return false;
}
Expand All @@ -563,17 +558,18 @@ bool SERCOM::sendDataMasterWIRE(uint8_t data)
{
//Send data
sercom->I2CM.DATA.bit.DATA = data;
waitSyncWIRE();

//Wait transmission successful
while(!sercom->I2CM.INTFLAG.bit.MB) {

// If a bus error occurs, the MB bit may never be set.
// Check the bus error bit and bail if it's set.
if (sercom->I2CM.STATUS.bit.BUSERR) {
return false;
}
initTimeout();
while (!sercom->I2CM.INTFLAG.bit.MB && !testTimeout())
{
// Wait transmission complete
}

// check for timeout condition
if ( didTimeout() ) return false;

//Problems on line? nack received?
if(sercom->I2CM.STATUS.bit.RXNACK)
return false;
Expand Down Expand Up @@ -665,9 +661,10 @@ uint8_t SERCOM::readDataWIRE( void )
{
if(isMasterWIRE())
{
while( sercom->I2CM.INTFLAG.bit.SB == 0 && sercom->I2CM.INTFLAG.bit.MB == 0 )
initTimeout();
while (!sercom->I2CM.INTFLAG.bit.SB && !testTimeout())
{
// Waiting complete receive
// wait receive
}

return sercom->I2CM.DATA.bit.DATA ;
Expand Down Expand Up @@ -739,3 +736,26 @@ void SERCOM::initClockNVIC( void )
/* Wait for synchronization */
}
}

void SERCOM::setTimeout( uint16_t ms )
{
timeoutInterval = ms;
}

bool SERCOM::didTimeout( void )
{
return timeoutOccurred;
}

void SERCOM::initTimeout( void )
{
timeoutOccurred = false;
timeoutRef = millis();
}

bool SERCOM::testTimeout( void )
{
if (!timeoutInterval) return false;
timeoutOccurred = (millis() - timeoutRef) > timeoutInterval;
return timeoutOccurred;
}
29 changes: 24 additions & 5 deletions cores/arduino/SERCOM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
#define SERCOM_FREQ_REF 48000000
#define SERCOM_NVIC_PRIORITY ((1<<__NVIC_PRIO_BITS) - 1)

// timeout detection default length (zero is disabled)
#define SERCOM_DEFAULT_I2C_OPERATION_TIMEOUT_MS 25

typedef enum
{
UART_EXT_CLOCK = 0,
Expand Down Expand Up @@ -191,10 +194,10 @@ class SERCOM

void resetWIRE( void ) ;
void enableWIRE( void ) ;
void disableWIRE( void );
void prepareNackBitWIRE( void ) ;
void prepareAckBitWIRE( void ) ;
void prepareCommandBitsWire(uint8_t cmd);
void disableWIRE( void );
void prepareNackBitWIRE( void ) ;
void prepareAckBitWIRE( void ) ;
void prepareCommandBitsWire(uint8_t cmd);
bool startTransmissionWIRE(uint8_t address, SercomWireReadWriteFlag flag) ;
bool sendDataMasterWIRE(uint8_t data) ;
bool sendDataSlaveWIRE(uint8_t data) ;
Expand All @@ -209,15 +212,31 @@ class SERCOM
bool isRestartDetectedWIRE( void ) ;
bool isAddressMatch( void ) ;
bool isMasterReadOperationWIRE( void ) ;
bool isRXNackReceivedWIRE( void ) ;
bool isRXNackReceivedWIRE( void ) ;
int availableWIRE( void ) ;
uint8_t readDataWIRE( void ) ;
void setTimeout( uint16_t ms );
bool didTimeout( void );

private:
Sercom* sercom;
uint8_t calculateBaudrateSynchronous(uint32_t baudrate) ;
uint32_t division(uint32_t dividend, uint32_t divisor) ;
void initClockNVIC( void ) ;

void waitSyncWIRE( void ) ;

// timeout detection and tx restart for I2C operations
void initTimeout( void );
bool testTimeout( void );
uint16_t timeoutInterval;
uint32_t timeoutRef;
bool timeoutOccurred;

// Flag set when data is loaded into sercom->USART.DATA.reg.
// Helps with preventing UART lockups when flushing on startup
// and the asyncronous nature of the DRE and TXC interrupt flags.
bool onFlushWaitUartTXC = false;
};

#endif
Loading