Skip to content

Resolve a set of critical bugs in SERCOM and unify SPI/UART/I2C baudrate calculations #381

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

Merged
hathach merged 7 commits into from
Jan 26, 2026
Merged

Resolve a set of critical bugs in SERCOM and unify SPI/UART/I2C baudrate calculations #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e0a883c
Unify SERCOM clock frequency handling across UART/SPI/WIRE
crabel99 Jan 13, 2026
d2772f8
bug: resolve low speed I2C BAUD register overflow
crabel99 Jan 13, 2026
eebc1ea
avoid float division
hathach Jan 20, 2026
7941541
avoid float division 2
hathach Jan 23, 2026
445cc98
Add dynamic I2C speed mode with automatic HS address bit handling
crabel99 Jan 23, 2026
5e06eb6
Merge avoid float division changes
crabel99 Jan 23, 2026
262507f
minor clean up
hathach Jan 26, 2026
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
139 changes: 95 additions & 44 deletions cores/arduino/SERCOM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ SERCOM::SERCOM(Sercom* s)
{
sercom = s;

#if defined(__SAMD51__)
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
// A briefly-available but now deprecated feature had the SPI clock source
// set via a compile-time setting (MAX_SPI)...problem was this affected
// ALL SERCOMs, whereas some (anything read/write, e.g. SD cards) should
Expand All @@ -40,8 +40,8 @@ SERCOM::SERCOM(Sercom* s)
// per-peripheral basis. Nonetheless, we check SERCOM_SPI_FREQ_REF here
// (MAX_SPI * 2) to retain compatibility with any interim projects that
// might have relied on the compile-time setting. But please, don't.
#if SERCOM_SPI_FREQ_REF == F_CPU // F_CPU clock = GCLK0
clockSource = SERCOM_CLOCK_SOURCE_FCPU;
#if SERCOM_SPI_FREQ_REF == F_CPU // F_CPU clock = GCLK0
clockSource = SERCOM_CLOCK_SOURCE_100M;
#elif SERCOM_SPI_FREQ_REF == 48000000 // 48 MHz clock = GCLK1 (standard)
clockSource = SERCOM_CLOCK_SOURCE_48M;
#elif SERCOM_SPI_FREQ_REF == 100000000 // 100 MHz clock = GCLK2
Expand Down Expand Up @@ -80,11 +80,7 @@ void SERCOM::initUART(SercomUartMode mode, SercomUartSampleRate sampleRate, uint
// Asynchronous fractional mode (Table 24-2 in datasheet)
// BAUD = fref / (sampleRateValue * fbaud)
// (multiply by 8, to calculate fractional piece)
#if defined(__SAMD51__)
uint32_t baudTimes8 = (SERCOM_FREQ_REF * 8) / (sampleRateValue * baudrate);
#else
uint32_t baudTimes8 = (SystemCoreClock * 8) / (sampleRateValue * baudrate);
#endif
uint32_t baudTimes8 = (freqRef * 8) / (sampleRateValue * baudrate);

sercom->USART.BAUD.FRAC.FP = (baudTimes8 % 8);
sercom->USART.BAUD.FRAC.BAUD = (baudTimes8 / 8);
Expand Down Expand Up @@ -230,8 +226,8 @@ void SERCOM::initSPI(SercomSpiTXPad mosi, SercomRXPad miso, SercomSpiCharSize ch
resetSPI();
initClockNVIC();

#if defined(__SAMD51__)
sercom->SPI.CTRLA.reg = SERCOM_SPI_CTRLA_MODE(0x3) | // master mode
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
sercom->SPI.CTRLA.reg = SERCOM_SPI_CTRLA_MODE(0x3) | // master mode
SERCOM_SPI_CTRLA_DOPO(mosi) |
SERCOM_SPI_CTRLA_DIPO(miso) |
dataOrder << SERCOM_SPI_CTRLA_DORD_Pos;
Expand Down Expand Up @@ -322,13 +318,7 @@ SercomDataOrder SERCOM::getDataOrderSPI()
void SERCOM::setBaudrateSPI(uint8_t divider)
{
disableSPI(); // Register is enable-protected

#if defined(__SAMD51__)
sercom->SPI.BAUD.reg = calculateBaudrateSynchronous(freqRef / divider);
#else
sercom->SPI.BAUD.reg = calculateBaudrateSynchronous(SERCOM_SPI_FREQ_REF / divider);
#endif

enableSPI();
}

Expand Down Expand Up @@ -386,13 +376,11 @@ bool SERCOM::isDataRegisterEmptySPI()
// return sercom->SPI.INTFLAG.bit.RXC;
//}

uint8_t SERCOM::calculateBaudrateSynchronous(uint32_t baudrate) {
#if defined(__SAMD51__)
uint8_t SERCOM::calculateBaudrateSynchronous(uint32_t baudrate)
{
uint16_t b = freqRef / (2 * baudrate);
#else
uint16_t b = SERCOM_SPI_FREQ_REF / (2 * baudrate);
#endif
if(b > 0) b--; // Don't -1 on baud calc if already at 0
if (b > 0)
b--; // Don't -1 on baud calc if already at 0
return b;
}

Expand Down Expand Up @@ -488,14 +476,37 @@ void SERCOM::initMasterWIRE( uint32_t baudrate )


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

// Determine speed mode based on requested baudrate
const uint32_t topSpeeds[3] = {400000, 1000000, 3400000}; // {(sm/fm), (fm+), (hs)}
uint8_t speedBit;
uint8_t clockStretchMode; // See: 28.6.2.4.6 (SERCOM I2C Highspeed mode)

if (baudrate <= topSpeeds[0]) {
speedBit = 0; // Standard/Fast mode up to 400 khz
clockStretchMode = 0;
} else if (baudrate <= topSpeeds[1]) {
speedBit = 1; // Fast mode+ up to 1 Mhz
clockStretchMode = 0;
} else {
// High speed up to 3.4 Mhz
speedBit = 2;
clockStretchMode = 1;
}

// Synchronous arithmetic baudrate
#if defined(__SAMD51__)
sercom->I2CM.BAUD.bit.BAUD = SERCOM_FREQ_REF / ( 2 * baudrate) - 1 ;
#else
sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / ( 2 * baudrate) - 5 - (((SystemCoreClock / 1000000) * WIRE_RISE_TIME_NANOSECONDS) / (2 * 1000));
#endif
sercom->I2CM.CTRLA.bit.SPEED = speedBit;
sercom->I2CM.CTRLA.bit.SCLSM = clockStretchMode;

uint32_t minBaudrate = freqRef / 512; // BAUD = 255: SAMD51(@100MHz) ~195kHz, SAMD21 ~94kHz
uint32_t maxBaudrate = topSpeeds[speedBit];
baudrate = max(minBaudrate, min(baudrate, maxBaudrate));

if (speedBit == 0x2)
sercom->I2CM.BAUD.bit.HSBAUD = freqRef / (2 * baudrate) - 1;
else
sercom->I2CM.BAUD.bit.BAUD = freqRef / (2 * baudrate) - 5 -
(freqRef/1000000ul * WIRE_RISE_TIME_NANOSECONDS) / 2000;
}

void SERCOM::prepareNackBitWIRE( void )
Expand Down Expand Up @@ -551,7 +562,8 @@ bool SERCOM::startTransmissionWIRE(uint8_t address, SercomWireReadWriteFlag flag
}

// Send start and address
sercom->I2CM.ADDR.bit.ADDR = address;
sercom->I2CM.ADDR.reg = SERCOM_I2CM_ADDR_ADDR(address) |
((sercom->I2CM.CTRLA.bit.SPEED == 0x2) ? SERCOM_I2CM_ADDR_HS : 0);

// Address Transmitted
if ( flag == WIRE_WRITE_FLAG ) // Write mode
Expand Down Expand Up @@ -727,7 +739,7 @@ uint8_t SERCOM::readDataWIRE( void )
}
}

#if defined(__SAMD51__)
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)

static const struct {
Sercom *sercomPtr;
Expand Down Expand Up @@ -785,7 +797,47 @@ int8_t SERCOM::getSercomIndex(void) {
return -1;
}

#if defined(__SAMD51__)
uint32_t SERCOM::getSercomFreqRef(void)
{
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
int8_t idx = getSercomIndex();
uint8_t gen = 1; // default to GCLK1 (48 MHz) if we can't resolve

if (idx >= 0)
{
uint8_t pch = sercomData[idx].id_core;
gen = GCLK->PCHCTRL[pch].bit.GEN;
}

switch (gen)
{
case 0:
freqRef = 100000000UL;
break;
case 1:
freqRef = 48000000UL;
break;
case 2:
freqRef = 100000000UL;
break;
case 3:
freqRef = 32768UL;
break;
case 4:
freqRef = 12000000UL;
break;
default:
freqRef = 48000000UL;
break;
}
#else
freqRef = SystemCoreClock;
#endif

return freqRef;
}

#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
// This is currently for overriding an SPI SERCOM's clock source only --
// NOT for UART or WIRE SERCOMs, where it will have unintended consequences.
// It does not check.
Expand All @@ -804,16 +856,19 @@ void SERCOM::setClockSource(int8_t idx, SercomClockSource src, bool core) {
if(core) clockSource = src; // Save SercomClockSource value

// From cores/arduino/startup.c:
// GCLK0 = F_CPU
// GCLK0 = F_CPU (this is 120 MHz and exceeds SERCOM maximum)
// GCLK1 = 48 MHz
// GCLK2 = 100 MHz
// GCLK3 = XOSC32K
// GCLK4 = 12 MHz
if(src == SERCOM_CLOCK_SOURCE_FCPU) {
GCLK->PCHCTRL[clk_id].reg =
GCLK_PCHCTRL_GEN_GCLK0_Val | (1 << GCLK_PCHCTRL_CHEN_Pos);
if(core) freqRef = F_CPU; // Save clock frequency value
} else if(src == SERCOM_CLOCK_SOURCE_48M) {
GCLK_PCHCTRL_GEN_GCLK2_Val | (1 << GCLK_PCHCTRL_CHEN_Pos); // Guard Sercom from exceeding 100 MHz maximum
if (core)
freqRef = 100000000; // Save clock frequency value
}
else if (src == SERCOM_CLOCK_SOURCE_48M)
{
GCLK->PCHCTRL[clk_id].reg =
GCLK_PCHCTRL_GEN_GCLK1_Val | (1 << GCLK_PCHCTRL_CHEN_Pos);
if(core) freqRef = 48000000;
Expand All @@ -840,21 +895,15 @@ void SERCOM::initClockNVIC( void )
int8_t idx = getSercomIndex();
if(idx < 0) return; // We got a problem here

#if defined(__SAMD51__)
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)

for(uint8_t i=0; i<4; i++) {
NVIC_ClearPendingIRQ(sercomData[idx].irq[i]);
NVIC_SetPriority(sercomData[idx].irq[i], SERCOM_NVIC_PRIORITY);
NVIC_EnableIRQ(sercomData[idx].irq[i]);
}

// SPI DMA speed is dictated by the "slow clock" (I think...maybe) so
// BOTH are set to the same clock source (clk_slow isn't sourced from
// XOSC32K as in prior versions of SAMD core).
// This might have power implications for sleep code.

setClockSource(idx, clockSource, true); // true = core clock
setClockSource(idx, clockSource, false); // false = slow clock
setClockSource(idx, clockSource, true); // true = core clock

#else // end if SAMD51 (prob SAMD21)

Expand All @@ -875,4 +924,6 @@ void SERCOM::initClockNVIC( void )
while(GCLK->STATUS.reg & GCLK_STATUS_SYNCBUSY); // Wait for synchronization

#endif // end !SAMD51

getSercomFreqRef();
}
21 changes: 11 additions & 10 deletions cores/arduino/SERCOM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,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 @@ -233,11 +233,12 @@ class SERCOM
bool isRestartDetectedWIRE( void ) ;
bool isAddressMatch( void ) ;
bool isMasterReadOperationWIRE( void ) ;
bool isRXNackReceivedWIRE( void ) ;
bool isRXNackReceivedWIRE( void ) ;
int availableWIRE( void ) ;
uint8_t readDataWIRE( void ) ;
int8_t getSercomIndex(void);
#if defined(__SAMD51__)
uint32_t getSercomFreqRef(void);
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
// SERCOM clock source override is only available on
// SAMD51 (not 21) ... but these functions are declared
// regardless so user code doesn't need ifdefs or lengthy
Expand All @@ -253,11 +254,11 @@ class SERCOM
uint32_t getFreqRef(void) { return F_CPU; };
#endif

private:
Sercom* sercom;
#if defined(__SAMD51__)
private:
Sercom *sercom;
uint32_t freqRef = 48000000ul; // Frequency corresponding to clockSource
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
SercomClockSource clockSource;
uint32_t freqRef; // Frequency corresponding to clockSource
#endif
uint8_t calculateBaudrateSynchronous(uint32_t baudrate);
uint32_t division(uint32_t dividend, uint32_t divisor) ;
Expand Down