[BACKPORT] arch/arm/src/imxrt: Fix FlexCAN TX timeout aborting live mailboxes. - #395
Merged
dakejahl merged 3 commits intoAug 26, 2026
Merged
Conversation
…ailboxes. imxrt_txtimeout_work() had four defects that together let one expired frame take the interface down permanently. It aborted mailbox RXMBCOUNT + mbi while the deadline it consulted belongs to RXMBCOUNT + 1 + mbi, so every abort landed one mailbox low and mbi == 0 wrote CAN_TXMB_ABORT into the buffer reserved for the ERR005829 workaround, while the highest TX mailbox was never aborted at all. Its expiry test read `now.tv_sec > d.tv_sec || now.tv_usec > d.tv_usec`, which declares any deadline that crosses a second boundary expired: in that case the deadline's microsecond field is always the smaller of the two. The `now` it compared against was a struct timespec cast to a struct timeval, so writing tv_usec wrote over tv_nsec and tv_sec was whatever the cast happened to line up with. imxrt_txdone() cancelled the watchdog but left txmb[].deadline set, so a retired mailbox looked expired forever and the next watchdog expiry on any other mailbox aborted whatever frame had since been loaded there. The walk ran to TXMBCOUNT, which counts the reserved mailbox as well, so its last iteration addressed mailbox TOTALMBCOUNT - one past the ring, and mb_address[] one past its end. Only txmb[] never being written that far kept it in bounds. TXMBRINGSIZE now names the ring size that the rest of the driver already assumes. Aborting a frame that is already on the wire raises a bit error, so the transmit error counter climbs and the node goes error passive. Since imxrt_txmb_next() only hands out a mailbox above every pending one, a mailbox left in DATAORREMOTE also pins the allocator at TOTALMBCOUNT and transmit never recovers. Measured on an ARK FMU-v6XRT with a DroneCAN GNSS node on the bus, offering 736 frames/s (9% of a 1 Mbit/s bus) from the PX4 uavcan driver: before, the interface transmitted 0 frames/s with ECR[TXERRCNT] pinned at 128 and ESR1[FLTCONF] error passive, and stayed dead across a reboot. After, 734 frames/s, 0.1% loss, TXERRCNT 0, error active. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit babc85179b170c5a65b486c1fd075330c920ea3a)
…e the frame is sent. imxrt_transmit() wrote the caller's deadline into txmb[] before testing whether it had already passed, and the early return for an expired deadline then left that deadline behind on a mailbox holding no frame. The next watchdog expiry finds it, counts a transmit timeout that did not happen, and writes CAN_TXMB_ABORT into a mailbox the allocator may have handed to a live frame in the meantime. Compute the timeout first and store the deadline after the early return. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit 9f70b0de6911a3793698eea0d96e343a6e485f4f)
…g it in. MAXMB resets to 0x0f, so OR-ing the intended value into MCR can only ever raise it. Every configuration with fewer than 16 mailboxes therefore runs with MAXMB = 15 and FlexCAN arbitrates over mailboxes the driver never initialised: with the classic payload layout MB14 and MB15 hold power-on contents, and with a 64-byte CAN FD layout they are past the end of the mailbox RAM region entirely. s32k1xx_flexcan.c already does this; carry the same two lines over. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit 15d17b3f6aca8fe47079a2f61b1eb6a60e539be2)
dakejahl
force-pushed
the
dakejahl/imxrt-flexcan-tx-timeout
branch
from
August 26, 2026 01:40
0251ba9 to
2aa2056
Compare
dakejahl
changed the base branch from
px4_firmware_nuttx-10.3.0+
to
dakejahl/flexcan-cs-init
August 26, 2026 01:41
dakejahl
marked this pull request as ready for review
August 26, 2026 17:47
dakejahl
added a commit
that referenced
this pull request
Aug 26, 2026
…ailboxes. (#395) * [BACKPORT] arch/arm/src/imxrt: Fix FlexCAN TX timeout aborting live mailboxes. imxrt_txtimeout_work() had four defects that together let one expired frame take the interface down permanently. It aborted mailbox RXMBCOUNT + mbi while the deadline it consulted belongs to RXMBCOUNT + 1 + mbi, so every abort landed one mailbox low and mbi == 0 wrote CAN_TXMB_ABORT into the buffer reserved for the ERR005829 workaround, while the highest TX mailbox was never aborted at all. Its expiry test read `now.tv_sec > d.tv_sec || now.tv_usec > d.tv_usec`, which declares any deadline that crosses a second boundary expired: in that case the deadline's microsecond field is always the smaller of the two. The `now` it compared against was a struct timespec cast to a struct timeval, so writing tv_usec wrote over tv_nsec and tv_sec was whatever the cast happened to line up with. imxrt_txdone() cancelled the watchdog but left txmb[].deadline set, so a retired mailbox looked expired forever and the next watchdog expiry on any other mailbox aborted whatever frame had since been loaded there. The walk ran to TXMBCOUNT, which counts the reserved mailbox as well, so its last iteration addressed mailbox TOTALMBCOUNT - one past the ring, and mb_address[] one past its end. Only txmb[] never being written that far kept it in bounds. TXMBRINGSIZE now names the ring size that the rest of the driver already assumes. Aborting a frame that is already on the wire raises a bit error, so the transmit error counter climbs and the node goes error passive. Since imxrt_txmb_next() only hands out a mailbox above every pending one, a mailbox left in DATAORREMOTE also pins the allocator at TOTALMBCOUNT and transmit never recovers. Measured on an ARK FMU-v6XRT with a DroneCAN GNSS node on the bus, offering 736 frames/s (9% of a 1 Mbit/s bus) from the PX4 uavcan driver: before, the interface transmitted 0 frames/s with ECR[TXERRCNT] pinned at 128 and ESR1[FLTCONF] error passive, and stayed dead across a reboot. After, 734 frames/s, 0.1% loss, TXERRCNT 0, error active. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit babc85179b170c5a65b486c1fd075330c920ea3a) * [BACKPORT] arch/arm/src/imxrt: Store the FlexCAN TX deadline only once the frame is sent. imxrt_transmit() wrote the caller's deadline into txmb[] before testing whether it had already passed, and the early return for an expired deadline then left that deadline behind on a mailbox holding no frame. The next watchdog expiry finds it, counts a transmit timeout that did not happen, and writes CAN_TXMB_ABORT into a mailbox the allocator may have handed to a live frame in the meantime. Compute the timeout first and store the deadline after the early return. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit 9f70b0de6911a3793698eea0d96e343a6e485f4f) * [BACKPORT] arch/arm/src/imxrt: Assign FlexCAN MAXMB rather than OR-ing it in. MAXMB resets to 0x0f, so OR-ing the intended value into MCR can only ever raise it. Every configuration with fewer than 16 mailboxes therefore runs with MAXMB = 15 and FlexCAN arbitrates over mailboxes the driver never initialised: with the classic payload layout MB14 and MB15 hold power-on contents, and with a 64-byte CAN FD layout they are past the end of the mailbox RAM region entirely. s32k1xx_flexcan.c already does this; carry the same two lines over. Assisted-by: Claude:claude-opus-5 Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com> (cherry picked from commit 15d17b3f6aca8fe47079a2f61b1eb6a60e539be2) --------- Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased onto
px4_firmware_nuttx-10.3.0+now that #394 is in it as3d7f0016cd7, and grown from one fix to three — the extra two turned up while chasing an unrelated CAN FD framing bug in the same driver.Upstream at apache/nuttx#19970; draft until that merges. Stacked on #396 so both trees match apache/nuttx master; retarget to
px4_firmware_nuttx-10.3.0+once #396 lands.Summary
Three defects in the i.MX RT FlexCAN transmit path, all found while chasing an error-passive storm on an ARK FMU-v6XRT.
imxrt_txtimeout_work()aborts the wrong mailboxes. It aborts mailboxRXMBCOUNT + mbiwhile the deadline it consulted belongs toRXMBCOUNT + 1 + mbi, so every abort lands one mailbox low;mbi == 0writesCAN_TXMB_ABORTinto the buffer reserved for the ERR005829 workaround and the highest TX mailbox is never aborted at all. Its expiry test isnow.tv_sec > d.tv_sec || now.tv_usec > d.tv_usec, which declares any deadline crossing a second boundary expired, and thenowit compares against is astruct timespeccast to astruct timeval. The walk runs toTXMBCOUNT, which counts the reserved mailbox too, so the last iteration addresses one mailbox past the ring andmb_address[]one past its end — in bounds today only becausetxmb[]is never written that far. Andimxrt_txdone()cancels the watchdog but leavestxmb[].deadlineset, so a retired mailbox looks expired forever and the next expiry on any other mailbox aborts whatever frame has since been loaded there.Since
imxrt_txmb_next()only hands out a mailbox above every pending one, a mailbox left inDATAORREMOTEalso pins the allocator atTOTALMBCOUNTand transmit never recovers.imxrt_transmit()stores the deadline before it knows the frame is going out. The early return for an already-expired deadline then leaves that deadline on a mailbox holding no frame, and the next watchdog expiry counts a transmit timeout that did not happen and aborts a mailbox the allocator may since have handed to a live frame.MCR[MAXMB]is OR-ed in rather than assigned. MAXMB resets to 0x0f, so the OR can only raise it: every configuration with fewer than 16 mailboxes runs with MAXMB = 15 and FlexCAN arbitrates over mailboxes the driver never initialised — with the classic payload layout MB14 and MB15 hold power-on contents, and with a 64-byte CAN FD layout they are past the end of the mailbox RAM region.s32k1xx_flexcan.calready carries this fix; the same two lines go here.The same
timevalcast, the same OR-ed expiry test and the same unwritten deadline are inkinetis_flexcan.c,s32k1xx_flexcan.cands32k3xx_flexcan.c, and the MAXMB one in kinetis and s32k3xx. Those drivers indexpriv->tx[mbi]directly, so the mailbox off-by-one is imxrt's alone. Left untouched here because there is no hardware to test them on — happy to extend the PR if a maintainer prefers.Impact
Testing
ARK FMU-v6XRT (i.MX RT1176),
ark_fmu-v6xrt_default, arm-none-eabi-gcc 13.2.1. Two 1 Mbit/s DroneCAN buses with a GNSS node on each, driven by the PX4 uavcan driver. ECR, ESR1 and MCR read over SWD without halting the core.TX timeout fix, one bus at 736 offered frames/s (9% utilisation):
MAXMB fix,
MCRread back on both instances:No regression at 8-channel ESC RawCommand to both buses at 400 Hz plus RawIMU back — 705 offered frames/s per bus, 31% utilisation, 120 s: 0 error episodes, TXERRCNT 0, 0 transmit timeouts, 0.00% loss on both interfaces.
The deadline-store fix is structural; its window is too narrow to provoke deliberately, and it is covered by the same run showing no timeouts.