Skip to content

Commit 6438545

Browse files
author
Andrew Cunningham
committed
Fix infinite recursion issue in SERCOM::startTransmissionWIRE
* If a significant hardware issue is struck startTransmissionWIRE calls itself to restart sending the address+r/w byte. The stack can be blown very quickly in the event of a prolonged hardware issue that warrants transmission restarts. A restart limit has been added (currently set at 5) to fix this issue. * This fixes issue #476 and it builds on work by @jrowberg in which a timeout was added addressing issue #439. * Some code simplification and tidying has followed from #439 and #476 being resolved.
1 parent 166478f commit 6438545

File tree

4 files changed

+164
-89
lines changed

4 files changed

+164
-89
lines changed

‎cores/arduino/SERCOM.cpp

Lines changed: 73 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
SERCOM::SERCOM(Sercom* s)
3030
{
3131
sercom = s;
32+
timeoutOccurred = false;
33+
timeoutInterval = SERCOM_DEFAULT_I2C_OPERATION_TIMEOUT_MS;
3234
}
3335

3436
/* =========================
@@ -393,11 +395,8 @@ void SERCOM::enableWIRE()
393395

394396
// Setting bus idle mode
395397
sercom->I2CM.STATUS.bit.BUSSTATE = 1 ;
398+
waitSyncWIRE();
396399

397-
while ( sercom->I2CM.SYNCBUSY.bit.SYSOP != 0 )
398-
{
399-
// Wait the SYSOP bit from SYNCBUSY coming back to 0
400-
}
401400
}
402401

403402
void SERCOM::disableWIRE()
@@ -433,10 +432,8 @@ void SERCOM::initSlaveWIRE( uint8_t ucAddress, bool enableGeneralCall )
433432
SERCOM_I2CS_INTENSET_AMATCH | // Address Match
434433
SERCOM_I2CS_INTENSET_DRDY ; // Data Ready
435434

436-
while ( sercom->I2CM.SYNCBUSY.bit.SYSOP != 0 )
437-
{
438-
// Wait the SYSOP bit from SYNCBUSY to come back to 0
439-
}
435+
waitSyncWIRE();
436+
440437
}
441438

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

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

460456
// Synchronous arithmetic baudrate
461457
sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / ( 2 * baudrate) - 5 - (((SystemCoreClock / 1000000) * WIRE_RISE_TIME_NANOSECONDS) / (2 * 1000));
458+
462459
}
463460

464461
void SERCOM::prepareNackBitWIRE( void )
@@ -485,68 +482,65 @@ void SERCOM::prepareCommandBitsWire(uint8_t cmd)
485482
{
486483
if(isMasterWIRE()) {
487484
sercom->I2CM.CTRLB.bit.CMD = cmd;
485+
waitSyncWIRE();
486+
} else {
487+
sercom->I2CS.CTRLB.bit.CMD = cmd;
488+
}
489+
}
488490

491+
void SERCOM::waitSyncWIRE() {
489492
while(sercom->I2CM.SYNCBUSY.bit.SYSOP)
490493
{
491494
// Waiting for synchronization
492495
}
493-
} else {
494-
sercom->I2CS.CTRLB.bit.CMD = cmd;
495-
}
496496
}
497497

498498
bool SERCOM::startTransmissionWIRE(uint8_t address, SercomWireReadWriteFlag flag)
499499
{
500500
// 7-bits address + 1-bits R/W
501501
address = (address << 0x1ul) | flag;
502502

503-
// If another master owns the bus or the last bus owner has not properly
504-
// sent a stop, return failure early. This will prevent some misbehaved
505-
// devices from deadlocking here at the cost of the caller being responsible
506-
// for retrying the failed transmission. See SercomWireBusState for the
507-
// possible bus states.
508-
if(!isBusOwnerWIRE())
509-
{
510-
if( isBusBusyWIRE() || (isArbLostWIRE() && !isBusIdleWIRE()) )
511-
{
512-
return false;
513-
}
514-
}
515-
516503
// Send start and address
517504
sercom->I2CM.ADDR.bit.ADDR = address;
505+
waitSyncWIRE();
506+
507+
// wait Address Transmitted
508+
initTimeout();
518509

519-
// Address Transmitted
520510
if ( flag == WIRE_WRITE_FLAG ) // Write mode
521511
{
522-
while( !sercom->I2CM.INTFLAG.bit.MB )
523-
{
524-
// Wait transmission complete
525-
}
526-
// Check for loss of arbitration (multiple masters starting communication at the same time)
527-
if(!isBusOwnerWIRE())
512+
while (!sercom->I2CM.INTFLAG.bit.MB && !testTimeout())
528513
{
529-
// Restart communication
530-
startTransmissionWIRE(address >> 1, flag);
514+
// wait transmission complete
531515
}
532516
}
533517
else // Read mode
534518
{
535-
while( !sercom->I2CM.INTFLAG.bit.SB )
519+
while (!sercom->I2CM.INTFLAG.bit.SB && !testTimeout())
536520
{
537-
// If the slave NACKS the address, the MB bit will be set.
538-
// In that case, send a stop condition and return false.
539-
if (sercom->I2CM.INTFLAG.bit.MB) {
540-
sercom->I2CM.CTRLB.bit.CMD = 3; // Stop condition
541-
return false;
542-
}
543-
// Wait transmission complete
521+
// wait transmission complete
544522
}
545-
546-
// Clean the 'Slave on Bus' flag, for further usage.
547-
//sercom->I2CM.INTFLAG.bit.SB = 0x1ul;
548523
}
549524

525+
// Check for loss of arbitration (multiple masters starting communication at the same time)
526+
// or timeout
527+
if(!isBusOwnerWIRE() || didTimeout()) {
528+
529+
restartTX_cnt++; // increment restart counter
530+
531+
if (restartTX_cnt >= restartTX_limit) {
532+
// only allow limited restarts
533+
restartTX_cnt = 0;
534+
return false;
535+
} else {
536+
// Restart communication after small delay
537+
if (!didTimeout()) delayMicroseconds(1000); // delay if not already timed out
538+
539+
// Restart
540+
startTransmissionWIRE(address >> 1, flag);
541+
}
542+
543+
} else restartTX_cnt = 0;
550544

551545
//ACK received (0: ACK, 1: NACK)
552546
if(sercom->I2CM.STATUS.bit.RXNACK)
@@ -563,17 +557,18 @@ bool SERCOM::sendDataMasterWIRE(uint8_t data)
563557
{
564558
//Send data
565559
sercom->I2CM.DATA.bit.DATA = data;
560+
waitSyncWIRE();
566561

567562
//Wait transmission successful
568-
while(!sercom->I2CM.INTFLAG.bit.MB) {
569-
570-
// If a bus error occurs, the MB bit may never be set.
571-
// Check the bus error bit and bail if it's set.
572-
if (sercom->I2CM.STATUS.bit.BUSERR) {
573-
return false;
574-
}
563+
initTimeout();
564+
while (!sercom->I2CM.INTFLAG.bit.MB && !testTimeout())
565+
{
566+
// Wait transmission complete
575567
}
576568

569+
// check for timeout condition
570+
if ( didTimeout() ) return false;
571+
577572
//Problems on line? nack received?
578573
if(sercom->I2CM.STATUS.bit.RXNACK)
579574
return false;
@@ -665,9 +660,10 @@ uint8_t SERCOM::readDataWIRE( void )
665660
{
666661
if(isMasterWIRE())
667662
{
668-
while( sercom->I2CM.INTFLAG.bit.SB == 0 && sercom->I2CM.INTFLAG.bit.MB == 0 )
663+
initTimeout();
664+
while (!sercom->I2CM.INTFLAG.bit.SB && !testTimeout())
669665
{
670-
// Waiting complete receive
666+
// wait receive
671667
}
672668

673669
return sercom->I2CM.DATA.bit.DATA ;
@@ -739,3 +735,26 @@ void SERCOM::initClockNVIC( void )
739735
/* Wait for synchronization */
740736
}
741737
}
738+
739+
void SERCOM::setTimeout( uint16_t ms )
740+
{
741+
timeoutInterval = ms;
742+
}
743+
744+
bool SERCOM::didTimeout( void )
745+
{
746+
return timeoutOccurred;
747+
}
748+
749+
void SERCOM::initTimeout( void )
750+
{
751+
timeoutOccurred = false;
752+
timeoutRef = millis();
753+
}
754+
755+
bool SERCOM::testTimeout( void )
756+
{
757+
if (!timeoutInterval) return false;
758+
timeoutOccurred = (millis() - timeoutRef) > timeoutInterval;
759+
return timeoutOccurred;
760+
}

‎cores/arduino/SERCOM.h

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
#define SERCOM_FREQ_REF 48000000
2525
#define SERCOM_NVIC_PRIORITY ((1<<__NVIC_PRIO_BITS) - 1)
2626

27+
// timeout detection default length (zero is disabled)
28+
#define SERCOM_DEFAULT_I2C_OPERATION_TIMEOUT_MS 1000
29+
2730
typedef enum
2831
{
2932
UART_EXT_CLOCK = 0,
@@ -191,10 +194,10 @@ class SERCOM
191194

192195
void resetWIRE( void ) ;
193196
void enableWIRE( void ) ;
194-
void disableWIRE( void );
195-
void prepareNackBitWIRE( void ) ;
196-
void prepareAckBitWIRE( void ) ;
197-
void prepareCommandBitsWire(uint8_t cmd);
197+
void disableWIRE( void );
198+
void prepareNackBitWIRE( void ) ;
199+
void prepareAckBitWIRE( void ) ;
200+
void prepareCommandBitsWire(uint8_t cmd);
198201
bool startTransmissionWIRE(uint8_t address, SercomWireReadWriteFlag flag) ;
199202
bool sendDataMasterWIRE(uint8_t data) ;
200203
bool sendDataSlaveWIRE(uint8_t data) ;
@@ -209,15 +212,26 @@ class SERCOM
209212
bool isRestartDetectedWIRE( void ) ;
210213
bool isAddressMatch( void ) ;
211214
bool isMasterReadOperationWIRE( void ) ;
212-
bool isRXNackReceivedWIRE( void ) ;
215+
bool isRXNackReceivedWIRE( void ) ;
213216
int availableWIRE( void ) ;
214217
uint8_t readDataWIRE( void ) ;
218+
void setTimeout( uint16_t ms );
219+
bool didTimeout( void );
215220

216221
private:
217222
Sercom* sercom;
218223
uint8_t calculateBaudrateSynchronous(uint32_t baudrate) ;
219224
uint32_t division(uint32_t dividend, uint32_t divisor) ;
220225
void initClockNVIC( void ) ;
226+
227+
void waitSyncWIRE( void ) ;
228+
229+
// timeout detection and tx restart for I2C operations
230+
void initTimeout( void );
231+
bool testTimeout( void );
232+
uint16_t timeoutInterval;
233+
uint32_t timeoutRef, restartTX_cnt, restartTX_limit = 5;
234+
bool timeoutOccurred;
221235
};
222236

223237
#endif

0 commit comments

Comments
 (0)