From f4bb5e3546d194c65ab6b7b5f3bf5553e54f6d98 Mon Sep 17 00:00:00 2001 From: Willem Melching Date: Wed, 21 Oct 2026 11:04:49 +0200 Subject: [PATCH] Preserve frame ordering on TX --- README.md | 20 +++++++++++--- app/CMakeLists.txt | 5 ++++ app/main.c | 39 +++++++++++++++++++++++++++ cmake/mcan_tx_fifo.cmake | 31 ++++++++++++++++++++++ scripts/mcan_tx_fifo.py | 57 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 cmake/mcan_tx_fifo.cmake create mode 100644 scripts/mcan_tx_fifo.py diff --git a/README.md b/README.md index 56234ba..4041051 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,11 @@ the three logical USB CAN channels. `app/` supplies a small entry point and black/red harness support while reusing upstream CANnectivity's USB protocol, USB descriptors, LEDs and timestamps. -The dependency checkouts remain unmodified. GitHub Actions builds all three variants. +The dependency checkouts remain unmodified. Red builds generate a local copy of +Zephyr's M_CAN driver with FIFO transmission and hardware put-index allocation; +the build stops if the expected upstream code changes. The local override lives +in `cmake/mcan_tx_fifo.cmake` and `scripts/mcan_tx_fifo.py`. +GitHub Actions builds all three variants. Pushing a Git tag also publishes a GitHub Release with all three binaries after the build succeeds. @@ -164,6 +168,12 @@ The ESP is held off, GMLAN stays asleep, and unused K/L-line TX pins stay at UAR idle. K-line, L-line, GMLAN, Wi-Fi and openpilot's safety/forwarding features are outside this adapter's scope. CAN termination must be provided on the bench bus. +White and black configure bxCAN transmit mailboxes in request order (`TXFP`). +This preserves frame order under load, including repeated frames with the same +CAN ID, as required by protocols such as ISO-TP. The upstream Zephyr default +prioritizes identifiers and then mailbox numbers, which can reorder frames with +the same ID when a lower-numbered mailbox is reused. + PA13 and PA14 serve board controls, as in the factory firmware, and overlap the SWD pins. Hardware ROM DFU is the intended flashing/recovery route. @@ -188,8 +198,12 @@ Black shares white's 16 MHz crystal, 96 MHz CPU clock and 1 MHz TIM2 timestamps. Its GPS is held off and the USB load switch is enabled. Red uses a 25 MHz crystal, 240 MHz CPU, 80 MHz FDCAN kernel clock, and HSI48 for full-speed USB. TIM2 also runs at 1 MHz. Each FDCAN controller gets a separate region of the shared 10 KiB -message RAM, with 64-byte payloads and data bitrates up to 5 Mbit/s. Red targets -the STM32H725 with direct SMPS supply. +message RAM, with 64-byte payloads and data bitrates up to 5 Mbit/s. Red uses +transmit FIFO mode (`TXBC.TFQM = 0`) and allocates slots through the hardware +FIFO put index (`TXFQS.TFQPI`). This preserves frame order under load, including +repeated CAN IDs, while keeping multiple transmissions queued. Like white and +black's `TXFP` setting, this selects submission order across different IDs too. +Red targets the STM32H725 with direct SMPS supply. ### Harness connection and orientation diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 53e9e0b..157a28e 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -18,3 +18,8 @@ target_sources_ifdef(CONFIG_CANNECTIVITY_TIMESTAMP_COUNTER app PRIVATE if(CONFIG_BOARD_PANDA_BLACK OR CONFIG_BOARD_PANDA_RED) target_sources(app PRIVATE harness.c) endif() + +# Red panda uses an M_CAN FIFO so equal-ID frames keep their submission order. +if(CONFIG_BOARD_PANDA_RED) + include(${CMAKE_CURRENT_SOURCE_DIR}/../cmake/mcan_tx_fifo.cmake) +endif() diff --git a/app/main.c b/app/main.c index d1b767b..af787e0 100644 --- a/app/main.c +++ b/app/main.c @@ -18,6 +18,38 @@ #define CHANNEL_CONTROLLER(node) DEVICE_DT_GET(DT_PHANDLE(node, can_controller)) +#ifdef CONFIG_CAN_STM32_BXCAN +#include + +#define CHANNEL_BXCAN(node) ((CAN_TypeDef *)DT_REG_ADDR(DT_PHANDLE(node, can_controller))) + +static int panda_can_tx_order_init(void) +{ + const struct device *const devices[] = { + CANNECTIVITY_DT_FOREACH_CHANNEL_SEP(CHANNEL_CONTROLLER, (,)) + }; + CAN_TypeDef *const controllers[] = { + CANNECTIVITY_DT_FOREACH_CHANNEL_SEP(CHANNEL_BXCAN, (,)) + }; + + /* Zephyr leaves stopped bxCAN controllers in initialization mode. Its + * driver disables TXFP and reuses the lowest free mailbox, allowing newer + * frames with the same CAN ID to overtake older ones under load. Select + * request-order priority before gs_usb can start any of the channels. + */ + for (size_t i = 0; i < ARRAY_SIZE(controllers); i++) { + if (!device_is_ready(devices[i])) { + return -ENODEV; + } + if ((controllers[i]->MSR & CAN_MSR_INAK) == 0U) { + return -EBUSY; + } + controllers[i]->MCR |= CAN_MCR_TXFP; + } + return 0; +} +#endif + static const struct gs_usb_ops usb_ops = { .timestamp = cannectivity_timestamp_get, .event = cannectivity_led_event, @@ -56,6 +88,13 @@ int main(void) if (err != 0) { return err; } +#ifdef CONFIG_CAN_STM32_BXCAN + err = panda_can_tx_order_init(); + if (err != 0) { + return err; + } +#endif + err = gs_usb_register(usb, channels, ARRAY_SIZE(channels), &usb_ops, NULL); if (err != 0) { return err; diff --git a/cmake/mcan_tx_fifo.cmake b/cmake/mcan_tx_fifo.cmake new file mode 100644 index 0000000..402123f --- /dev/null +++ b/cmake/mcan_tx_fifo.cmake @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: Apache-2.0 + +# Build a locally patched copy; west-managed dependencies stay untouched. +set(mcan_source "${ZEPHYR_BASE}/drivers/can/can_mcan.c") +set(mcan_generator "${CMAKE_CURRENT_LIST_DIR}/../scripts/mcan_tx_fifo.py") +set(mcan_fifo_source "${CMAKE_CURRENT_BINARY_DIR}/can_mcan_tx_fifo.c") +set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS + "${mcan_source}" "${mcan_generator}") +execute_process( + COMMAND "${PYTHON_EXECUTABLE}" "${mcan_generator}" + "${mcan_source}" "${mcan_fifo_source}" + RESULT_VARIABLE mcan_result + ERROR_VARIABLE mcan_error) +if(NOT mcan_result EQUAL 0) + message(FATAL_ERROR "Could not prepare local M_CAN FIFO driver: ${mcan_error}") +endif() + +get_target_property(can_sources drivers__can SOURCES) +set(mcan_replaced FALSE) +foreach(source IN LISTS can_sources) + get_filename_component(source_name "${source}" NAME) + if(source_name STREQUAL "can_mcan.c") + list(REMOVE_ITEM can_sources "${source}") + set(mcan_replaced TRUE) + endif() +endforeach() +if(NOT mcan_replaced) + message(FATAL_ERROR "Could not find Zephyr M_CAN driver to replace") +endif() +set_property(TARGET drivers__can PROPERTY SOURCES "${can_sources}") +target_sources(drivers__can PRIVATE "${mcan_fifo_source}") diff --git a/scripts/mcan_tx_fifo.py b/scripts/mcan_tx_fifo.py new file mode 100644 index 0000000..3b15e21 --- /dev/null +++ b/scripts/mcan_tx_fifo.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +"""Generate the red panda M_CAN FIFO driver from the pinned Zephyr source.""" + +from pathlib import Path +import sys + + +def fifo_source(source: str) -> str: + replacements = [ + ( + "config->mram_elements[CAN_MCAN_MRAM_CFG_TX_BUFFER]) | CAN_MCAN_TXBC_TFQM;", + "config->mram_elements[CAN_MCAN_MRAM_CFG_TX_BUFFER]);", + ), + ( + """ /* Acquire a free TX buffer */ + for (int i = 0; i < cbs->num_tx; i++) { + if (cbs->tx[i].function == NULL) { + put_idx = i; + break; + } + } + + /* A free TX buffer should always be available since the data->tx_sem was acquired */ + __ASSERT_NO_MSG(put_idx < cbs->num_tx);""", + """ /* FIFO mode requires the hardware put index, including after wraparound. + * A lower-numbered free buffer may belong to a different FIFO position. + */ + err = can_mcan_read_reg(dev, CAN_MCAN_TXFQS, ®); + if (err != 0) { + goto err_unlock; + } + + put_idx = FIELD_GET(CAN_MCAN_TXFQS_TFQPI, reg); + if ((reg & CAN_MCAN_TXFQS_TFQF) != 0U || put_idx >= cbs->num_tx || + cbs->tx[put_idx].function != NULL) { + err = -EAGAIN; + goto err_unlock; + }""", + ), + ] + for old, new in replacements: + if source.count(old) != 1: + raise ValueError("Zephyr M_CAN source changed; review the local FIFO fix") + source = source.replace(old, new, 1) + return source + + +def main() -> None: + source, destination = map(Path, sys.argv[1:]) + result = fifo_source(source.read_text()) + if not destination.exists() or destination.read_text() != result: + destination.write_text(result) + + +if __name__ == "__main__": + main()