Incorrect SAM D5x/E5x and D21/DA1 SERCOM Register Configurations

### Operating System

MacOS

### Arduino IDE version

PlatformIO Core, version 6.1.18

### Board

All M4 Variants

### ArduinoCore version

1.7.16

### Sketch as ATTACHED TXT

# Bug Report: SERCOM Clock Configuration Errors on SAMD51/SAME5x

## Summary

The Adafruit Arduino SAMD core has four critical bugs in SERCOM clock configuration affecting SAMD51/SAME5x chips:

1. **SLOW Clock Specification Violation**: `initClockNVIC()` routes 100 MHz to the shared `GCLK_SERCOMx_SLOW` peripheral (`PCHCTRL[3]`), which has a **12 MHz maximum** specification—an 8.3× overdrive that violates datasheet Table 54-6.
2. **Runtime Clock Source Not Supported**: UART and I2C libraries use a hardcoded `SERCOM_FREQ_REF` constant (48 MHz) for baud calculations instead of querying the actual hardware register (`GCLK->PCHCTRL[id_core].bit.GEN`), preventing runtime clock source changes and causing silent baud rate errors when `setClockSource()` is called.
3. **I2C `BAUD` Register Overflow**: Baud rate calculations lack bounds checking for the 8-bit `BAUD` register (max value 255), causing silent truncation at low baud rates—requesting 100 Hz I2C on a 100 MHz clock produces ~223 kHz actual speed (2,232× faster) due to register overflow.
4. **Wrong Register and Formula for High-Speed I2C**: The code branches on chip type (`#if defined(__SAMD51__)`) instead of I2C mode (`CTRLA.bit.SPEED`), causing it to write to BAUD instead of `HSBAUD` in high-speed mode and use the wrong formula on SAM D5x/E5x for Fm (400 kHz) and Fm+ (1 MHz) I2C.

These bugs cause specification violations, prevent dynamic power management, and introduce silent failures difficult to diagnose in production.

---

## Bug #1: SLOW Clock Exceeds Maximum Specification

### Location

`cores/arduino/SERCOM.cpp`, line ~843 in `initClockNVIC()`

### Issue

The code sets `GCLK_SERCOMx_SLOW` to the same clock source as `GCLK_SERCOMx_CORE`, potentially routing 100 MHz to a peripheral rated at a maximum of 12 MHz.

### Current Code

```cpp
void SERCOM::initClockNVIC( void )
{
    ...
    setClockSource(idx, clockSource, true);  // true  = core clock
    setClockSource(idx, clockSource, false); // false = slow clock ← BUG
    ...
}
```

### Datasheet Violation

**Table 54-6: Maximum Peripheral Clock Frequencies** specifies:

- `fGCLK_SERCOMx_SLOW`: **12 MHz maximum** ("Common SERCOMx slow input clock frequency")
- `fGCLK_SERCOMx_CORE`: **100 MHz maximum** ("SERCOMx input clock frequency")

When `clockSource = SERCOM_CLOCK_SOURCE_100M`, the SLOW clock is driven at 100 MHz, **8.3× over specification**.

### Root Cause

From **Table 14-9: PCHCTRLm Mapping**:

- `SERCOMx_CORE` clocks use **individual** `PCHCTRL` indices: m = {7, 8, 23, 24, 34, 35, 36, 37} (one per SERCOM0-7)
- `SERCOMx_SLOW` clock uses **shared** `PCHCTRL` index: m = 3 (common to all SERCOMs and a host of other peripherals)

The SLOW clock is shared across all SERCOMs SLOW, SD/MMC Host Controller (SDHC) SLOW,  and the Fractional Digital Phase Lock Loop (FDPLL) 32kHz input. Setting the shared register to 100 MHz:

1. Violates the 12 MHz maximum specification
2. May cause interference between SERCOMs
3. Has unknown power/stability implications associated with the FDPLL

### Fix

Delete the SLOW clock assignment entirely. According to the datasheet section 33.5.3, only the CORE clock "is required to clock the SERCOM while working as a host."

```cpp
setClockSource(idx, clockSource, true);  // true  = core clock
// Remove: setClockSource(idx, clockSource, false);
```

---

## Bug #2: Hardcoded `SERCOM_FREQ_REF` Ignores Actual Clock Source

### Location

`cores/arduino/SERCOM.h`, line ~38

### Issue

`SERCOM_FREQ_REF` is hardcoded to 48 MHz across all SAMD variants:

```cpp
#define SERCOM_FREQ_REF 48000000ul  // ← WRONG for SAMD51 when using 100 MHz clock
```

This breaks UART/I2C baud rates whenever the SERCOM CORE clock is set to a different frequency via `setClockSource()`.

### Affected Functions

1. **`initMasterWIRE()`** (line ~551):

   ```cpp
   sercom->I2CM.BAUD.bit.BAUD = SERCOM_FREQ_REF / (2 * baudrate) - 1;
   ```

2. **`initUART()`** (line ~91):

   ```cpp
   uint32_t baudTimes8 = (SERCOM_FREQ_REF * 8) / (sampleRateValue * baudrate);
   ```

### Architecture Details

**Table 14-7: Generator Selection** (implied from `cores/arduino/startup.c`):

| GCLK Generator | Frequency | SercomClockSource Enum |
| -------------- | --------- | ---------------------- |
| GCLK0 | `F_CPU` | `SERCOM_CLOCK_SOURCE_FCPU` |
| GCLK1 | 48 MHz | `SERCOM_CLOCK_SOURCE_48M` |
| GCLK2 | 100 MHz | `SERCOM_CLOCK_SOURCE_100M` |
| GCLK3 | XOSC32K | `SERCOM_CLOCK_SOURCE_32K` |
| GCLK4 | 12 MHz | `SERCOM_CLOCK_SOURCE_12M` |

**Table 14-9: PCHCTRLm Mapping**:

| PCHCTRL Index (m) | Name | Description |
| ----------------- | ---- | ----------- |
| 3 | `GCLK_SERCOM[0..7]_SLOW` | SERCOM[0..7] Slow (shared) |
| 7 | `GCLK_SERCOM0_CORE` | SERCOM0 input clock frequency |
| 8 | `GCLK_SERCOM1_CORE` | SERCOM1 input clock frequency |
| 23 | `GCLK_SERCOM2_CORE` | SERCOM2 input clock frequency |
| 24 | `GCLK_SERCOM3_CORE` | SERCOM3 input clock frequency |
| 34 | `GCLK_SERCOM4_CORE` | SERCOM4 input clock frequency |
| 35 | `GCLK_SERCOM5_CORE` | SERCOM5 input clock frequency |
| 36 | `GCLK_SERCOM6_CORE` | SERCOM6 input clock frequency |
| 37 | `GCLK_SERCOM7_CORE` | SERCOM7 input clock frequency |

### Current Behavior

- `setClockSource()` correctly updates `freqRef` instance variable and writes to `GCLK->PCHCTRL[id_core].bit.GEN`
- SPI's `calculateBaudrateSynchronous()` correctly uses `freqRef` on SAM D51
- **BUT** UART and I2C ignore `freqRef` and use hardcoded `SERCOM_FREQ_REF = 48 MHz`

### Impact

When MAX_SPI sets `SERCOM_CLOCK_SOURCE_100M` or the user manually sets a 100 MHz clock:

- **Requested**: 400 kHz I2C
- **Actual**: ~200 kHz I2C (2× error)
- **Cause**: Baud calculation uses a 48 MHz constant, but the hardware runs at 100 MHz

Formula error:

```text
BAUD_register = 48MHz / (2 × 400kHz) - 1 = 59
Actual_rate = 100MHz / (2 × (59+1)) = 833 kHz  ← Wrong!
```

### Correct Fix Option 1: Query Hardware Register

Read the actual clock frequency from the `PCHCTRL` register:

```cpp
// In initMasterWIRE() and initUART()
uint8_t gen = GCLK->PCHCTRL[sercomData[idx].id_core].bit.GEN;

// Map generator to frequency (per Table 14-7)
uint32_t fREF;
switch(gen) {
  case 0: fREF = F_CPU; break;            // GCLK0
  case 1: fREF = 48000000; break;         // GCLK1
  case 2: fREF = 100000000; break;        // GCLK2
  case 3: fREF = 32768; break;            // GCLK3 (XOSC32K)
  case 4: fREF = 12000000; break;         // GCLK4
  default: fREF = SERCOM_FREQ_REF; break; // Fallback should work with SAM D21/DA1
}

// I2C
sercom->I2CM.BAUD.bit.BAUD = fREF / (2 * baudrate) - 1;

// UART
uint32_t baudTimes8 = (fREF * 8) / (sampleRateValue * baudrate);
```

### Correct Fix Option 2: Use Existing freqRef Instance Variable

The `freqRef` member variable is already correctly maintained by `setClockSource()`. Extend its usage to UART/I2C:

```cpp
// Requires making freqRef accessible to init functions or refactoring
sercom->I2CM.BAUD.bit.BAUD = freqRef / (2 * baudrate) - 1;
```

However, this requires architectural changes because `freqRef` is a SERCOM class member, whereas `initMasterWIRE()` is called before `setClockSource()` in some cases.

**Recommendation**: Use Option 1 (querying the hardware register) for the most robust solution.

**Additional Benefits of Hardware Register Query**:

1. Enables runtime clock source changes without library modification
2. Eliminates compile-time constant dependency
3. Single source of truth: hardware register
4. Allows dynamic power management by switching clock sources at runtime

---

## Bug #3: I2C BAUD Register Overflow on Low Baud Rates

### Location

`libraries/Wire/Wire.cpp`, implementation in SERCOM core

### Issue

The BAUD register calculation can overflow for certain baudrate/clock combinations, particularly at very low baud rates:

```cpp
#if defined(__SAMD51__)
  sercom->I2CM.BAUD.bit.BAUD = SERCOM_FREQ_REF / (2 * baudrate) - 1;
#else
  sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / (2 * baudrate) - 5 - ...;
#endif
```

The I2C BAUD register is **8 bits** (value range 0-255), but the formula can produce values that exceed this range.

### Example Overflow Scenarios

**Case 1: 100 Hz baud on 100 MHz clock**

```text
BAUD = 100,000,000 / (2 × 100) - 1
     = 500,000 - 1
     = 499,999  ← EXCEEDS 8-bit max (255)!
```

**Case 2: 1 kHz baud on 100 MHz clock**

```text
BAUD = 100,000,000 / (2 × 1,000) - 1
     = 50,000 - 1
     = 49,999  ← Still exceeds 8-bit max
```

### Datasheet Limits

The I2C `BAUD` register field (from SAM D5x datasheet, Section 33.6.2.3) is 8 bits:

```text
For synchronous operation, the `BAUD` register value is 8 bits (0 to 255).
```

**Maximum valid `BAUD` value**: 255

**This limits minimum I2C speed to**:

- On 48 MHz: `fREF / (2 × (BAUD_max + 1)) = 48M / (2 × 256) ≈ 93.75 kHz`
- On 100 MHz: `fREF / (2 × (BAUD_max + 1)) = 100M / (2 × 256) ≈ 195.3 kHz`

### Current Framework Behavior (Bug)

The overflow silently truncates the value due to the 8-bit register size:

```cpp
sercom->I2CM.BAUD.bit.BAUD = 499999;  // Assigned
// Register only uses lower 8 bits: 499999 & 0xFF = 223
// Actual I2C speed becomes: 100MHz / (2 × (223+1)) ≈ 223.2 kHz
```

**Result**: Requested 100 Hz, actual ~223 kHz (2,232× faster!)

### Comparison: WireDMA Library Fix

The WireDMA library correctly guards against this overflow:

```cpp
// From user's WireDMA implementation
#if defined(__SAMD51__) || defined(__SAME51__) || defined(__SAME53__) || defined(__SAME54__)
    const uint32_t fREF = 100000000UL;
#else
    const uint32_t fREF = 48000000UL;
#endif

// Minimum baud rate check
if (_baudrate < (fREF / (2 * 256))) {
    _baudrate = fREF / (2 * 256);  // Clamp to minimum
    _PL(F("WARNING: Baudrate too low, clamped to "), _baudrate);
}

uint16_t baud = fREF / (2 * _baudrate) - 1;

// Ensure value fits in 8-bit register
if (baud > 255) {
    baud = 255;  // Saturate to maximum
    _PL(F("WARNING: Baudrate register overflow, saturated to 255"));
}

_sercomHw->I2CM.BAUD.bit.BAUD = baud;
```

### Fix

Add bounds checking to prevent silent truncation:

```cpp
uint32_t fREF;
#if defined(__SAMD51__)
int8_t idx = getSercomIndex();
uint8_t gen = GCLK->PCHCTRL[sercomData[idx].id_core].bit.GEN;
switch(gen) {
    case 0: fREF = F_CPU; break;
    case 1: fREF = 48000000; break;
    case 2: fREF = 100000000; break;
    case 3: fREF = 32768; break;
    case 4: fREF = 12000000; break;
    default: fREF = 48000000; break;
}
#else
fREF = SERCOM_FREQ_REF;
#endif

const uint32_t minBaudrate = fREF / 512;  // BAUD = 255: SAMD51 ~195kHz, SAMD21 ~94kHz
// Max depends on CTRLA.bit.SPEED: 0x0=400kHz (Sm/Fm), 0x1=1MHz (Fm+), 0x2=3.4MHz (Hs-mode)
const uint32_t maxBaudrate = fREF / 14;   // Example guard for BAUD = 6 with fREF of 48 MHz and SPEED of 0x3
baudrate = max(minBaudrate, min(baudrate, maxBaudrate));
sercom->I2CM.BAUD.bit.BAUD = (fREF / (2 * baudrate)) - 1;
```

### Affected Configurations

| Clock Source | Frequency | Min I2C Baud Rate |
| ------------ | --------- | ----------------- |
| GCLK1 (48 MHz) | 48 MHz | ~93.8 kHz |
| GCLK2 (100 MHz) | 100 MHz | ~195 kHz |

---

## Bug #4: Wrong Register and Formula for Low-Speed I2C Mode

### Location

`cores/arduino/SERCOM.cpp`, line ~551 in `initMasterWIRE()`

### Issue

The code uses a chip-type conditional (`#if defined(__SAMD51__)`) to apply different baud formulas; the datasheet for both the SAM D21 and SAM D51 chip families specifies the same low-speed `BAUD` calculation. The formula in the chip conditional is for `CTRLA.bit.SPEED === 0x2` but **Adafruit's library doesn't even support high-speed I2C mode** (`SPEED=0x2`). The real bug is that for **low-speed I2C modes (SPEED=0x0/0x1), SAMD51 and SAMD21 should use identical settings**, but SAMD51 incorrectly uses the 3.4 MHz high-speed clock. There is an additional error: the `BAUD.bit.BUAD` register is the wrong one for `SPEED=0x2`; it should be `BAUD.bit.HSBAUD`.

Current buggy code:

```cpp
#if defined(__SAMD51__)
  sercom->I2CM.BAUD.bit.BAUD = SERCOM_FREQ_REF / ( 2 * baudrate) - 1 ;
#else
  sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / ( 2 * baudrate) - 5 - ...;
#endif
```

### Problems

1. **Wrong Formula on SAMD51**: When in low-speed mode (SPEED=0x0/0x1), SAMD51 should use the **same rise-time formula as SAMD21**, not the simplified formula.
2. **No High-Speed I2C Support**: Adafruit's library never implements high-speed mode (SPEED=0x2), so HSBAUD is never written to, which is ok.

### Root Cause

The distinction is **not** between chip types but between **I2C `SPEED` modes**:

- **`CTRLA.bit.SPEED = 0x0/0x1`** (Sm/Fm/Fm+, up to 1 MHz): Uses 8-bit `BAUD` register with rise-time compensation
  - **Both SAM D21 and SAM D51 should use the same clock source for these modes**
- **`CTRLA.bit.SPEED = 0x2`** (Hs-mode, 3.4 MHz): Uses 8-bit `HSBAUD` register (not supported by Adafruit)

### Impact

- **Formula mismatch**: SAMD51 omits rise-time compensation that SAMD21 includes, diverging the baud calculations
- **No path to high-speed I2C**: Adafruit's architecture can never support high-speed I2C without a complete rewrite to handle `HSBAUD`. It is safe to delete that branch, or it can be left as is until Hs-mode support is added to the library.

### Fix

Replace chip-type conditional with I2C `SPEED` mode conditional. For low-speed modes, use the same formula for both chips (with rise-time compensation). Optionally add high-speed I2C support:

```cpp
uint8_t speed = sercom->I2CM.CTRLA.bit.SPEED;

if (speed == 0x2) {
    // High-speed mode (Hs-mode, >1 MHz) uses HSBAUD register
    // Note: Adafruit's library doesn't support this mode, but proper support would use:
    sercom->I2CM.BAUD.bit.HSBAUD = fREF / (2 * baudrate) - 1;
} else {
    // Standard/Fast/Fast-mode Plus (Sm/Fm/Fm+, ≤1 MHz) use BAUD register with rise time
    //formula applies identically to both SAMD21 and SAMD51
    sercom->I2CM.BAUD.bit.BAUD = fREF / (2 * baudrate) - 5 - fREF * WIRE_RISE_TIME_NANOSECONDS / (2e9f);
}
```

This correctly:

1. Uses the same formula for low-speed modes on both chip types (with rise-time compensation)
2. Provides a path for high-speed I2C support in the future
3. Bases the decision on hardware configuration (`SPEED` register), not compile-time chip type

---

## Reproduction Steps

### Bug #1 (SLOW Clock Overspeed)

1. Add diagnostic code to read `GCLK->PCHCTRL[3].bit.GEN` after SERCOM initialization
2. Initialize any SERCOM with `MAX_SPI=100000000` or `SERCOM_CLOCK_SOURCE_100M`
3. **Observed**: SLOW clock (`PCHCTRL[3]`) routed to `GCLK2` (100 MHz)
4. **Expected**: SLOW clock should remain ≤ 12 MHz per Table 54-6

### Bug #2 (Runtime Clock Source Changes)

1. Initialize I2C at 400 kHz on default 48 MHz clock: `Wire.begin(); Wire.setClock(400000);`
2. Manually call `setClockSource()` to switch SERCOM to 100 MHz at runtime
3. Re-initialize: `Wire.setClock(400000);`
4. Measure actual SCL frequency with oscilloscope or logic analyzer
5. **Observed**: ~200 kHz (2× slower than requested due to stale BAUD calculation)
6. **Expected**: 400 kHz

### Bug #3 (BAUD Register Overflow)

1. Initialize I2C at very low baud rate on 100 MHz clock: `Wire.setClock(100);`
2. Check actual `BAUD` register value (should be 499,999 but truncates)
3. Measure actual SCL frequency
4. **Observed**: ~223 kHz (due to 8-bit truncation: 499,999 & 0xFF = 223)
5. **Expected**: Error/warning or clamped to minimum ~195 kHz

### Bug #4 (incorrect `BAUD` calculation for SAM D51)
1. Initialize I2C in master mode.
2. Check the actual `BAUD` register value against the correct formula.
3.  Measure SCL frequency
4. **Observed** clock frequency will be slightly higher than what is expected

---

## Datasheet References

**[SAM D5x/E5x Family Data Sheet (DS60001507)](https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/DataSheets/SAM-D5x-E5x-Family-Data-Sheet-DS60001507.pdf)**:

1. **Section 33.5.3: SERCOM Clocks**
   > "The SERCOM uses two generic clocks: GCLK_SERCOMx_CORE and GCLK_SERCOMx_SLOW. The core clock (GCLK_SERCOMx_CORE) is required to clock the SERCOM while working as a host. The slow clock (GCLK_SERCOMx_SLOW) is only required for certain functions."

2. **Table 14-9: PCHCTRLm Mapping**
   - Shows SERCOM CORE clocks at PCHCTRL indices {7, 8, 23, 24, 34, 35, 36, 37}
   - Shows SERCOM SLOW clock at PCHCTRL index 3 (shared)

3. **Table 54-6: Maximum Peripheral Clock Frequencies**

   | Symbol | Description | Max. | Units |
   | ------ | ----------- | ---- | ----- |
   | `fGCLK_SERCOMx_SLOW` | Common SERCOMx slow input clock frequency | 12 | MHz |
   | `fGCLK_SERCOMx_CORE` | SERCOMx input clock frequency | 100 | MHz |

**Framework Reference**:

- `cores/arduino/startup.c`: Defines GCLK0-4 generator assignments

---

## Affected Versions

- **Adafruit Arduino SAMD core**: v1.7.16 (likely all SAMD51 versions)
- **Affected Chips**: SAMD51, SAME51, SAME53, SAME54

---

## Severity

| Bug | Severity | Impact |
| --- | -------- | ------ |
| #1 (SLOW Overspeed) | **High** | Violates hardware specification (8.3× overdrive), potential instability/damage |
| #2 (Runtime Clock Changes) | **Medium** | Prevents dynamic power management, silent baud errors on clock switching |
| #3 (BAUD Overflow) | **Medium** | Silent truncation for very low baud rates (<94 kHz on 48 MHz), hidden bugs |
| #4 (Wrong Register/Formula for Hs-mode) | **Low** | Silently breaks high-speed I2C (SPEED=0x2) and Fm+ mode (SPEED=0x1) |

**Combined Critical** - All four bugs together cause silent failures that are difficult to diagnose in production.

---

## Proposed Pull Request Changes

### Change 1: Remove SLOW clock assignment

**File**: `cores/arduino/SERCOM.cpp`
**Function**: `initClockNVIC()`

```cpp
// Before:
setClockSource(idx, clockSource, true);  // true  = core clock
setClockSource(idx, clockSource, false); // false = slow clock

// After:
setClockSource(idx, clockSource, true);  // true  = core clock
```

### Change 2: Fix I2C baud rate calculation with overflow protection

**File**: `cores/arduino/SERCOM.cpp` or `libraries/Wire/Wire.cpp`
**Function**: `initMasterWIRE()`

```cpp
// Before:
#if defined(__SAMD51__)
  sercom->I2CM.BAUD.bit.BAUD = SERCOM_FREQ_REF / (2 * baudrate) - 1;
#else
  sercom->I2CM.BAUD.bit.BAUD = SystemCoreClock / (2 * baudrate) - 5 - ...;
#endif

// After:
  uint32_t fREF;
#if defined(__SAMD51__)
  // Query actual SERCOM CORE clock frequency
  int8_t idx = getSercomIndex();
  uint8_t gen = GCLK->PCHCTRL[sercomData[idx].id_core].bit.GEN;
  switch(gen) {
    case 0: fREF = F_CPU; break;
    case 1: fREF = 48000000; break;
    case 2: fREF = 100000000; break;
    case 3: fREF = 32768; break;
    case 4: fREF = 12000000; break;
    default: fREF = 48000000; break;
  }
#else
  fREF = SystemCoreClock;
#endif

// Guard rails: BAUD register is 8-bit (0-255)
// Min: BAUD=255 -> SAMD51 ~195kHz, SAMD21 ~94kHz
const uint32_t minBaudrate = fREF / 512;  // BAUD = 255

// Max baudrate depends on CTRLA.bit.SPEED setting for I2C mode
uint32_t maxBaudrate;
uint8_t speed = sercom->I2CM.CTRLA.bit.SPEED;
switch(speed) {
case 0x0:  // Standard-mode (Sm) and Fast-mode (Fm)
    maxBaudrate = 400000;
    break;
case 0x1:  // Fast-mode Plus (Fm+)
    maxBaudrate = 1000000;
    break;
case 0x2:  // High-speed mode (Hs-mode)
    maxBaudrate = 3400000;
    break;
default:   // Conservative fallback
    maxBaudrate = TWI_CLOCK;
    break;
}
baudrate = max(minBaudrate, min(baudrate, maxBaudrate));

// Both SAMD21 and SAMD51 use identical formula for low-speed I2C modes (SPEED=0x0/0x1):
// fSCL = fGCLK / (10 + 2×BAUD + fGCLK×TRISE)  [datasheet sections 28.6.2.4.1 and 36.6.2.4.1]
// Simplified to: BAUD = fGCLK/(2×fSCL) - 5 - (fGCLK×TRISE_ns/2000)
// High-speed mode (SPEED=0x2) uses the HSBAUD register with a different formula.
if (speed==0x2)
sercom->I2CM.BAUD.bit.HSBAUD = fREF / (2 * baudrate) - 1;
else
sercom->I2CM.BAUD.bit.BAUD = fREF / (2 * baudrate) - 5 - fREF * WIRE_RISE_TIME_NANOSECONDS / (2e9f);
```

### Change 3: Fix UART baud rate calculation

**File**: `cores/arduino/SERCOM.cpp`
**Function**: `initUART()`

```cpp
// Before:
#if defined(__SAMD51__)
    uint32_t baudTimes8 = (SERCOM_FREQ_REF * 8) / (sampleRateValue * baudrate);
#else
    uint32_t baudTimes8 = (SystemCoreClock * 8) / (sampleRateValue * baudrate);
#endif

// After:
    uint32_t fREF;
#if defined(__SAMD51__)
    // Query actual SERCOM CORE clock frequency
    int8_t idx = getSercomIndex();
    uint8_t gen = GCLK->PCHCTRL[sercomData[idx].id_core].bit.GEN;
    switch(gen) {
      case 0: fREF = F_CPU; break;
      case 1: fREF = 48000000; break;
      case 2: fREF = 100000000; break;
      case 3: fREF = 32768; break;
      case 4: fREF = 12000000; break;
      default: fREF = 48000000; break;
    }
#else
    fREF = SystemCoreClock;
#endif
    
    uint32_t baudTimes8 = (fREF * 8) / (sampleRateValue * baudrate);
```

---

## Testing Recommendations

1. **Clock Frequency Verification**:
   - Verify SLOW clock (PCHCTRL[3]) remains ≤ 12 MHz in all configurations
   - Verify CORE clocks match requested frequencies
2. **Runtime Clock Switching**:
   - Test I2C switching between 48 MHz ↔ 100 MHz at runtime
   - Verify baud rates recalculate correctly after `setClockSource()` calls
3. **Low Baud Rate Edge Cases**:
   - Test I2C at 10 kHz, 1 kHz, 100 Hz with 100 MHz clock
   - Verify overflow protection prevents silent truncation
   - Verify minimum baud rate clamping works correctly
4. **Multi-SERCOM Scenarios**:
   - Test SPI + I2C + UART simultaneously with different clock sources per SERCOM
   - Verify no interference between SERCOMs
5. **Register Value Inspection**:
   - Read back BAUD register values at various baudrates
   - Ensure values are within 8-bit range (0-255)
6. **Hardware Validation**:
   - Use a logic analyzer to verify that the actual SCL/SCLK frequencies match the requested values
   - Test at standard rates: I2C (100k, 400k, 1M), UART (9600, 115200), SPI (1M, 12M, 24M)
7. **Extreme Configurations**:
   - Test all five GCLK generator sources (0-4) as SERCOM clock sources
   - Verify 32 kHz XOSC32K operation for ultra-low-power scenarios

---

## Additional Notes

The comment in `initClockNVIC()` stating "SPI DMA speed is dictated by the 'slow clock' (I think...maybe)" is speculative and appears incorrect based on datasheet analysis. The SLOW clock's actual purpose spans several peripherals beyond the SERCOMs, but it is definitively **not** used for baud rate generation—that is the CORE clock's function and is set per SERCOM per section 33.5.3.


###
Edited for ordering, and clarity, 12 Jan 26.

### Screenshots

<img width="804" height="512" alt="Image" src="https://github.com/user-attachments/assets/a98fc9fc-2c07-406f-9943-36d648fdc6df" />
<img width="766" height="222" alt="Image" src="https://github.com/user-attachments/assets/0e7e9907-1313-4d2f-b235-28845acf1f48" />

<img width="696" height="905" alt="Image" src="https://github.com/user-attachments/assets/212ae726-c898-4681-ba6b-515b0131fea0" />

GCLK Generator	Frequency	SercomClockSource Enum
GCLK0	`F_CPU`	`SERCOM_CLOCK_SOURCE_FCPU`
GCLK1	48 MHz	`SERCOM_CLOCK_SOURCE_48M`
GCLK2	100 MHz	`SERCOM_CLOCK_SOURCE_100M`
GCLK3	XOSC32K	`SERCOM_CLOCK_SOURCE_32K`
GCLK4	12 MHz	`SERCOM_CLOCK_SOURCE_12M`

PCHCTRL Index (m)	Name	Description
3	`GCLK_SERCOM[0..7]_SLOW`	SERCOM[0..7] Slow (shared)
7	`GCLK_SERCOM0_CORE`	SERCOM0 input clock frequency
8	`GCLK_SERCOM1_CORE`	SERCOM1 input clock frequency
23	`GCLK_SERCOM2_CORE`	SERCOM2 input clock frequency
24	`GCLK_SERCOM3_CORE`	SERCOM3 input clock frequency
34	`GCLK_SERCOM4_CORE`	SERCOM4 input clock frequency
35	`GCLK_SERCOM5_CORE`	SERCOM5 input clock frequency
36	`GCLK_SERCOM6_CORE`	SERCOM6 input clock frequency
37	`GCLK_SERCOM7_CORE`	SERCOM7 input clock frequency

Symbol	Description	Max.	Units
`fGCLK_SERCOMx_SLOW`	Common SERCOMx slow input clock frequency	12	MHz
`fGCLK_SERCOMx_CORE`	SERCOMx input clock frequency	100	MHz

Bug	Severity	Impact
#1 (SLOW Overspeed)	High	Violates hardware specification (8.3× overdrive), potential instability/damage
#2 (Runtime Clock Changes)	Medium	Prevents dynamic power management, silent baud errors on clock switching
#3 (BAUD Overflow)	Medium	Silent truncation for very low baud rates (<94 kHz on 48 MHz), hidden bugs
#4 (Wrong Register/Formula for Hs-mode)	Low	Silently breaks high-speed I2C (SPEED=0x2) and Fm+ mode (SPEED=0x1)

Incorrect SAM D5x/E5x and D21/DA1 SERCOM Register Configurations #380

Description

Operating System

Arduino IDE version

Board

ArduinoCore version

Sketch as ATTACHED TXT

Bug Report: SERCOM Clock Configuration Errors on SAMD51/SAME5x

Summary

Bug #1: SLOW Clock Exceeds Maximum Specification

Location

Issue

Current Code

Datasheet Violation

Root Cause

Fix

Bug #2: Hardcoded SERCOM_FREQ_REF Ignores Actual Clock Source

Location

Issue

Affected Functions

Architecture Details

Current Behavior

Impact

Correct Fix Option 1: Query Hardware Register

Correct Fix Option 2: Use Existing freqRef Instance Variable

Bug #3: I2C BAUD Register Overflow on Low Baud Rates

Location

Issue

Example Overflow Scenarios

Datasheet Limits

Current Framework Behavior (Bug)

Comparison: WireDMA Library Fix

Fix

Affected Configurations

Bug #4: Wrong Register and Formula for Low-Speed I2C Mode

Location

Issue

Problems

Root Cause

Impact

Fix

Reproduction Steps

Bug #1 (SLOW Clock Overspeed)

Bug #2 (Runtime Clock Source Changes)

Bug #3 (BAUD Register Overflow)

Bug #4 (incorrect BAUD calculation for SAM D51)

Datasheet References

Affected Versions

Severity

Proposed Pull Request Changes

Change 1: Remove SLOW clock assignment

Change 2: Fix I2C baud rate calculation with overflow protection

Change 3: Fix UART baud rate calculation

Testing Recommendations

Additional Notes

Screenshots

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Bug #2: Hardcoded `SERCOM_FREQ_REF` Ignores Actual Clock Source

Bug #4 (incorrect `BAUD` calculation for SAM D51)