diff --git a/armsrc/cmd.c b/armsrc/cmd.c index 7e9edb1e3..a2d210a0d 100644 --- a/armsrc/cmd.c +++ b/armsrc/cmd.c @@ -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; } diff --git a/armsrc/pm5_cep.c b/armsrc/pm5_cep.c index d3a976f3f..ac9f425dc 100644 --- a/armsrc/pm5_cep.c +++ b/armsrc/pm5_cep.c @@ -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; } diff --git a/armsrc/pm5_cep.h b/armsrc/pm5_cep.h index fafbd33d1..d040bd1ef 100644 --- a/armsrc/pm5_cep.h +++ b/armsrc/pm5_cep.h @@ -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