PM5 CEP: recover from AT32 SPI1 errata, bound all SPI waits

Problem

With PR #3672/#3674 landed, CEP handshakes reliably but the SPI transport
hangs PM5 hard (needs a physical reset, no watchdog on this platform)
after some number of attach/reconnect cycles - sometimes the first
cycle, sometimes dozens of pings in. Logic-analyzer capture of a hang in
progress showed a ~1.2s burst of rapid CS-gated retries, and in one
capture, three periodic transactions after that burst got progressively
slower (2.1ms -> 3.9ms -> 3.2ms -> 5.2ms) instead of recovering.

Root causes (two, compounding)

1. cep_spi_write_sync() had seven bare `while (flag == RESET);` spins
   waiting on SPI1's TDBE/BF flags, with no timeout. PM5 is the SPI
   slave - TDBE only clears when the Flipper is actively clocking, on
   its own ~20-120ms schedule, completely async to when PM5's reply is
   ready. Queue a reply between two master polls and the wait never
   returns - full main-loop death.
2. Artery's own errata sheet for the AT32F435/437 (ES0003) documents
   that in SPI slave hardware-CS mode, the internal clock sync is not
   guaranteed to reset on every CS falling edge - a bad sync can persist
   across transactions with no self-correction. This is PM5's exact CEP
   configuration (SPI_CS_HARDWARE_MODE, slave mode).

Change

armsrc/pm5_cep.c:
- cep_spi_write_sync() rewritten around a bounded cep_spi_wait_tdbe()
  helper (same spin-iteration-count pattern already used for reads) -
  returns PM3_EIO on timeout instead of hanging.
- New cep_spi_resync(): hardware-resets SPI1 via crm_periph_reset()
  (a register re-init alone doesn't clear the errata's bad internal
  state) and brings it back up. This is Artery's own documented recovery
  for ES0003 ("reset SPI and restart communication"), triggered by our
  existing NG-frame CRC/length checks instead of the SPI peripheral's
  hardware CRC feature.
- cep_do_handshake()'s SPI-stall recovery path was itself unbounded
  (retried forever as long as the CC controller reported "still
  attached") and missing the StartTicks()/I2C_init(true) guard every
  other I2C call site in this file has (same bug class as #3672) - now
  bounded (CEP_HANDSHAKE_REPLY_MAX_RETRIES) with the guard added.
- cep_spi_data_available() now discards the Flipper's 0x55 idle-poll
  filler bytes before reading the real length header, instead of ever
  interpreting one as (part of) a frame length.

armsrc/cmd.c: wire cep_spi_resync() into every CEP-reachable error exit
in receive_ng_internal() (short/overflow/CRC-mismatch reads) and both
reply_old()/reply_ng_internal()'s write-failure paths, so any detected
corruption resets SPI1 before the next transaction rather than letting a
bad sync persist. Add a WITH_CEP-gated inline stub for cep_spi_resync()
so receive_ng_internal()'s call sites don't need per-site #ifdef guards
on non-CEP platforms.

Behaviour change

None for USB/FPC/BWM. CEP: a corrupted or stalled SPI1 transaction now
self-heals (one dropped reply, then normal operation) instead of
wedging the device.

Testing

- make clean && make PLATFORM=PM5 PLATFORM_EXTRAS=BWM and
  make clean && make PLATFORM=PM3RDV4 - both zero warnings.
- Hardware: flashed to a real PM5 paired with a real Flipper Zero FAP.
  - 60s continuous-connection capture with zero USB activity (isolates
    CEP from any client-side interaction): clean, no anomalies.
  - 110s capture with 18 real `hw status` calls interspersed: all 18
    succeeded, CEP stayed attached throughout.
  - 5 full attach/detach cycles with `hf search` against a real NTAG213
    card before/during/after each attach (240s, logic-analyzer capture
    throughout): all 15 reads returned the identical UID regardless of
    CEP state, zero hangs, final liveness check succeeded with no reset
    needed.
- Not tested: clang toolchain (firmware-only diff).

Checklist

- [x] Proper style of own code, no unrelated reformatting included
- [x] Builds warning-free with gcc (PM5 and RDV4)
- [ ] clang not tried (firmware-only diff)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Harkness
2026-10-04 14:25:00 +02:00
committed by Philippe Teuwen
co-authored by Claude Sonnet 5
parent 2e2720a1da
commit 8faafcebb0
3 changed files with 123 additions and 18 deletions
+20 -1
View File
@@ -22,6 +22,11 @@
#endif
#ifdef WITH_CEP
#include "pm5_cep.h"
#else
// receive_ng_internal()'s `cep` parameter is always false without WITH_CEP,
// so this is never actually called - just keeps those call sites free of
// per-site #if defined(WITH_CEP) guards.
static inline void cep_spi_resync(void) {}
#endif
#include "crc16.h"
#include "string.h"
@@ -91,6 +96,9 @@ int reply_old(uint64_t cmd, uint64_t arg0, uint64_t arg1, uint64_t arg2, const v
if (g_reply_via_cep) {
#if defined(WITH_CEP)
resultcep = cep_spi_write_sync((uint8_t *)&txcmd, sizeof(PacketResponseOLD));
if (resultcep != PM3_SUCCESS) {
cep_spi_resync();
}
#else
return PM3_EDEVNOTSUPP;
#endif
@@ -173,6 +181,9 @@ static int reply_ng_internal(uint16_t cmd, int8_t status, uint8_t reason, const
if (g_reply_via_cep) {
#if defined(WITH_CEP)
resultcep = cep_spi_write_sync((uint8_t *)tx, txBufferNGLen);
if (resultcep != PM3_SUCCESS) {
cep_spi_resync();
}
#else
return PM3_EDEVNOTSUPP;
#endif
@@ -214,6 +225,7 @@ static int receive_ng_internal(PacketCommandNG *rx, uint32_t read_ng(uint8_t *da
}
if (bytes != sizeof(PacketCommandNGPreamble)) {
if (cep) cep_spi_resync();
return PM3_EIO;
}
@@ -225,27 +237,33 @@ static int receive_ng_internal(PacketCommandNG *rx, uint32_t read_ng(uint8_t *da
if (rx->magic == COMMANDNG_PREAMBLE_MAGIC) { // New style NG command
if (length > PM3_CMD_DATA_SIZE) {
if (cep) cep_spi_resync();
return PM3_EOVFLOW;
}
// Get the core and variable length payload
bytes = read_ng((uint8_t *)&rx_raw.data, length);
if (bytes != length) {
if (cep) cep_spi_resync();
return PM3_EIO;
}
// Get the postamble
bytes = read_ng((uint8_t *)&rx_raw.foopost, sizeof(PacketCommandNGPostamble));
if (bytes != sizeof(PacketCommandNGPostamble)) {
if (cep) cep_spi_resync();
return PM3_EIO;
}
// Check CRC, accept MAGIC as placeholder
// Check CRC, accept MAGIC as placeholder - a mismatch here is the
// AT32 errata's own recommended trigger to reset SPI (see
// cep_spi_resync()'s comment).
rx->crc = rx_raw.foopost.crc;
if (rx->crc != COMMANDNG_POSTAMBLE_MAGIC) {
uint8_t first, second;
compute_crc(CRC_14443_A, (uint8_t *)&rx_raw, sizeof(PacketCommandNGPreamble) + length, &first, &second);
if ((first << 8) + second != rx->crc) {
if (cep) cep_spi_resync();
return PM3_EIO;
}
}
@@ -264,6 +282,7 @@ static int receive_ng_internal(PacketCommandNG *rx, uint32_t read_ng(uint8_t *da
} else {
uint64_t arg[3] = {0};
if (length < sizeof(arg)) {
if (cep) cep_spi_resync();
return PM3_EIO;
}
+94 -15
View File
@@ -47,6 +47,13 @@
#define CEP_SPI_BYTE_TIMEOUT 100000 // spin-wait iterations per byte (matches source)
#define CEP_SPI_RESPONSE_TIMEOUT_US (1000 * 1000) // 1s, matches source
#define CEP_HANDSHAKE_RX_TIMEOUT_MS 2000 // bail out of the receive loop rather than ever hang the main loop
#define CEP_HANDSHAKE_REPLY_MAX_RETRIES 3 // bound the stall-recovery loop below - still-attached isn't still-working forever
// 0x55 filler the Flipper sends while its own SPI poll finds nothing new -
// discard it so the real length header is recognized as soon as it starts,
// instead of ever reading a 0x55 byte as (part of) a frame length.
#define CEP_SYNC_BYTE 0x55
#define CEP_SYNC_MAX_DISCARD 600
static bool s_cep_active = false; // handshake completed, SPI transport live
@@ -155,6 +162,20 @@ static void cep_spi_init(void) {
spi_enable(SPI1, TRUE);
}
// AT32F435/437 errata ES0003, "CS falling edge not synchronized in SPI
// slave hardware CS mode": the peripheral's internal clock sync isn't
// guaranteed to reset on every CS edge, so a bad sync can persist across
// transactions. Artery's documented recovery is to detect the corruption
// (we already do, via the NG frame's CRC) and reset SPI before retrying -
// a register re-init alone isn't enough, the peripheral needs resetting
// through the clock/reset controller to actually clear that state.
void cep_spi_resync(void) {
spi_enable(SPI1, FALSE);
crm_periph_reset(CRM_SPI1_PERIPH_RESET, TRUE);
crm_periph_reset(CRM_SPI1_PERIPH_RESET, FALSE);
cep_spi_init();
}
void cep_init(void) {
cep_usart_init();
cep_spi_init();
@@ -208,13 +229,23 @@ static bool cep_do_handshake(void) {
uint8_t response[] = {0x04, 0x00, 'y', 'e', 's', 0x00};
for (size_t i = 0; i < sizeof(response); i++) {
uint32_t wait_start = GetTicks();
int recovery_attempts = 0;
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET) {
if (GetTicks() - wait_start > CEP_SPI_RESPONSE_TIMEOUT_US) {
// Bound this: "still attached" isn't "still working" forever,
// and I2C_BufferReadRaw() needs the ticks timer running like
// every other call site (see cep_attach_poll()'s comment).
if (++recovery_attempts > CEP_HANDSHAKE_REPLY_MAX_RETRIES) {
return false;
}
StartTicks();
I2C_init(true);
uint8_t cc_ctrl_data;
bool ret = I2C_BufferReadRaw(&cc_ctrl_data, 1, CEP_CC_STATUS_REG, CEP_CC_CONTROLLER_ADDR << 1);
if (ret && ((cc_ctrl_data >> 6 & 0x03) == 0)) {
return false; // disconnected mid-reply
if (!ret || ((cc_ctrl_data >> 6 & 0x03) == 0)) {
return false; // disconnected mid-reply, or can't tell - bail either way
}
wait_start = GetTicks();
}
}
spi_i2s_data_transmit(SPI1, response[i]);
@@ -277,16 +308,33 @@ void cep_attach_poll(void) {
// SPI NG-frame transport (verbatim from the unit test - already correct)
// ---------------------------------------------------------------------------
static bool cep_spi_read_byte(uint8_t *out) {
uint64_t timeout = 0;
while (spi_i2s_flag_get(SPI1, SPI_I2S_RDBF_FLAG) == RESET) {
if (timeout++ > CEP_SPI_BYTE_TIMEOUT) {
return false;
}
}
*out = spi_i2s_data_receive(SPI1);
return true;
}
bool cep_spi_data_available(void) {
uint8_t len_header[2] = {0x00};
for (size_t i = 0; i < sizeof(len_header); ++i) {
uint64_t timeout = 0;
while (spi_i2s_flag_get(SPI1, SPI_I2S_RDBF_FLAG) == RESET) {
if (timeout++ > CEP_SPI_BYTE_TIMEOUT) {
return false;
}
size_t discarded = 0;
while (1) {
if (!cep_spi_read_byte(&len_header[0])) {
return false;
}
len_header[i] = spi_i2s_data_receive(SPI1);
if (len_header[0] != CEP_SYNC_BYTE) {
break;
}
if (++discarded > CEP_SYNC_MAX_DISCARD) {
return false;
}
}
if (!cep_spi_read_byte(&len_header[1])) {
return false;
}
uint16_t data_len = (len_header[1] << 8) | len_header[0];
if (data_len > PM3_CMD_DATA_SIZE * 2) {
@@ -308,25 +356,56 @@ uint32_t cep_spi_read_ng(uint8_t *data, size_t len) {
return len;
}
// PM5 is the SPI slave: TDBE only clears when the master is actively
// clocking, which happens on the Flipper's own ~20-120ms poll schedule,
// completely async to when PM5 has a reply ready. An unbounded wait here
// hangs the main loop forever (no recovery, not even a watchdog reset) the
// moment a reply is queued between two master polls - bound every wait so
// a slow/absent master degrades to a dropped reply, not a dead device.
static bool cep_spi_wait_tdbe(void) {
uint64_t timeout = 0;
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET) {
if (timeout++ > CEP_SPI_BYTE_TIMEOUT) {
return false;
}
}
return true;
}
int cep_spi_write_sync(uint8_t *data, size_t len) {
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET);
if (!cep_spi_wait_tdbe()) {
return PM3_EIO;
}
spi_i2s_data_transmit(SPI1, len & 0xFF);
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET);
if (!cep_spi_wait_tdbe()) {
return PM3_EIO;
}
spi_i2s_data_transmit(SPI1, (len >> 8) & 0xFF);
for (size_t i = 0; i < len; ++i) {
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET);
if (!cep_spi_wait_tdbe()) {
return PM3_EIO;
}
spi_i2s_data_transmit(SPI1, data[i]);
}
// Two trailing zero bytes: the agreed "no more data" idle marker.
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET);
if (!cep_spi_wait_tdbe()) {
return PM3_EIO;
}
spi_i2s_data_transmit(SPI1, 0x00);
while (spi_i2s_flag_get(SPI1, SPI_I2S_TDBE_FLAG) == RESET);
if (!cep_spi_wait_tdbe()) {
return PM3_EIO;
}
spi_i2s_data_transmit(SPI1, 0x00);
while (spi_i2s_flag_get(SPI1, SPI_I2S_BF_FLAG) == SET);
uint64_t timeout = 0;
while (spi_i2s_flag_get(SPI1, SPI_I2S_BF_FLAG) == SET) {
if (timeout++ > CEP_SPI_BYTE_TIMEOUT) {
return PM3_EIO;
}
}
return PM3_SUCCESS;
}
+9 -2
View File
@@ -56,8 +56,15 @@ bool cep_spi_data_available(void);
uint32_t cep_spi_read_ng(uint8_t *data, size_t len);
// Write `len` raw NG bytes (a whole PacketResponseNG/OLD frame) out over
// SPI1 as one length-prefixed packet. Always returns PM3_SUCCESS today (the
// underlying spi_i2s_data_transmit() calls are blocking-until-ready).
// SPI1 as one length-prefixed packet. Returns PM3_EIO if the master isn't
// clocking within CEP_SPI_BYTE_TIMEOUT at any point - this can legitimately
// happen (PM5's reply is ready on its own schedule, not the master's), the
// caller just loses this one reply rather than hanging forever.
int cep_spi_write_sync(uint8_t *data, size_t len);
// Hardware-reset SPI1 and bring it back up (see AT32F435/437 errata ES0003
// in pm5_cep.c). Call on any CEP frame error so a corrupted CS-edge sync
// can't persist into the next transaction.
void cep_spi_resync(void);
#endif // __PM5_CEP_H