Add a CAN FD driver for the ESP32-C5 by okhsunrog · Pull Request #6280 · esp-rs/esp-hal

okhsunrog · 2026-09-07T23:36:32Z

Submission Checklist 📝

I have updated existing examples or added new ones (if applicable).
I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
I have added changelog entries and/or migration guide notes in the sections below, or I will ask a maintainer to add the skip-changelog or manual-changelog label as appropriate.
My changes are in accordance to the esp-rs developer guidelines

Extra:

I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

Closes #5163.

This adds esp_hal::canfd, a driver for the CTU CAN FD core in the ESP32-C5. It is a separate peripheral from the classic TWAI controller, with a different register map and a different frame model, so it is a new driver rather than an extension of esp_hal::twai. The API follows the shape of the other drivers: a Config with the builder-lite pattern, Blocking and Async modes with into_async, with_rx and with_tx, and controller instances described in metadata.

Classic CAN 2.0 and CAN FD frames with up to 64 bytes of payload, and bit rate switching for the data phase.
Nominal and data phase bit timing, the secondary sample point, the retransmission limit, and the Normal, ListenOnly, SelfTest and LoopbackSelfTest modes.
Three mask filters and one range filter, each with its own set of accepted frame kinds.
Timestamps on received frames from the controller's own timer.
Async receive and transmit, and a borrow-based split() into an RX and a TX half.
Both controllers, TWAI0 and TWAI1, with their function clock in the clock tree.
embedded_can::Frame for classic frames through ClassicFrame, and embedded_can::Error for the error capture.
A wake lock held while the controller is on the bus, so automatic light sleep cannot gate the clock in the middle of a frame.
An open-drain mode for wiring two nodes together without transceivers, which the HIL tests and the example use.

The TX buffer RAM was missing from the SVD; esp-rs/esp-pacs#506 added it, so this PR bumps the esp-pacs revision to 06599476 in every crate that pins it and writes the TX buffers through the generated accessors. The bump also brings in the other esp-pacs changes since 5cd68d2: the MCPWM register rework (#415) and the CLINT blocks for ESP32-P4/H4/S31 (#505).

One thing left for a follow-up: the controller has four TX buffers and arbitrates between them by priority, and the blocking transmit() already exposes that by returning the buffer index. transmit_async() borrows the TX half for the whole transmission, so through the async API only one frame is in flight at a time; queueing more is possible with the blocking transmit() from the same object. A proper async queue would return a per-buffer transfer handle from a queue() method, so up to four futures can be awaited at once, with an explicit choice of cancellation semantics (abort on drop, or detach). I would rather agree on that shape here than add a second form of transmit_async in this PR.

Related, and documented on transmit(): the hardware arbitrates between armed buffers by priority and, for equal priorities, by buffer index, not by the order in which frames were queued. So with several frames in flight and equal priorities, a frame queued into a buffer that just became free goes out before frames still waiting in higher-numbered buffers. ESP-IDF sidesteps this by keeping frames in a software queue and filling the hardware slots in index order only when the hardware is idle. Keeping strict FIFO order by default, by having the driver renumber the three-bit buffer priorities by age on every transmit(), is about forty lines; it would need a way to coexist with set_tx_priority(), for example a Config option choosing between FIFO and priority ordering. I would like to hear which shape you prefer before adding it.

Testing

Tested on a Waveshare ESP32-C5-Zero with both controllers wired to each other through the two common test pins, GPIO9 and GPIO10, open-drain. The HIL suite in hil-test/src/bin/canfd.rs (40 tests) covers loopback, two-node exchange with acknowledgement and arbitration, async wakeups and cancellation, RX overrun, leaving and joining the bus in the middle of a frame, bus-off and recovery through injected bit errors, timestamps, filters, and the light-sleep wake lock.

On the HIL runner, with the jumper and no external pull-up, the two-node tests pass with the data phase at the default 2 Mbit/s: the two wired pins pull the line recessive together. The single-node loopback tests have one pull-up and failed at 2 Mbit/s there, so they run their data phase at 1 Mbit/s.

Changelog

esp-hal

Added: canfd driver for the CAN FD controller of the ESP32-C5.

esp-metadata-generated

No changelog necessary.

esp-radio

No changelog necessary.

esp-rom-sys

No changelog necessary.

esp-storage

No changelog necessary.

esp-phy

No changelog necessary.

The TWAI function clock joins the ESP32-C5 clock tree as a mux between XTAL_CLK and PLL_F80M, with TWAI0 and TWAI1 as peripheral clocks, and the driver table lists both controllers with their system peripheral, signals, and support status. The instances reach the driver through the generated for_each_canfd! macro.

The CAN FD controller is a CTU CAN FD core with its own register map and frame model, so this is a new driver next to the classic TWAI one. It follows the shape of the other drivers: a Config with the builder-lite pattern, Blocking and Async modes, with_rx and with_tx, and instances described in metadata. Classic CAN 2.0 and CAN FD frames with up to 64 bytes of payload and bit rate switching; nominal and data phase timing, the secondary sample point and the retransmission limit; Normal, ListenOnly, SelfTest and LoopbackSelfTest modes; three mask filters and one range filter, each with its own accepted frame kinds; timestamps from the controller's timer; async receive and transmit, and a borrow-based split into RX and TX halves; embedded_can::Frame for classic frames through ClassicFrame; a wake lock held while the controller is on the bus. Every wait is bounded in bit times derived from the configured bit rate, so the slowest legal timing still joins and leaves the bus cleanly. Where the ESP32-C5 TRM v1.1 and the hardware disagree, the code follows the hardware and says so at the site. The TX buffer RAM is addressed by offset: it is missing from the SVD and hence from the PAC.

The tests run on the two controllers of one chip, wired through the common test pins as an open-drain bus: loopback, two-node exchange with acknowledgement and arbitration, async wakeups and cancellation, RX overrun, leaving and joining the bus in the middle of a frame, bus-off and recovery through injected bit errors, timestamps, filters and the light-sleep wake lock. The example mirrors the classic TWAI one.

The hardware arbitrates between armed buffers by priority and then by buffer index, not by the order in which frames were queued. With equal priorities a frame queued into a buffer that just became free goes out before frames still waiting in higher-numbered buffers. Say so on transmit() and on set_tx_priority(), with the two ways to keep strict order.

github-actions · 2026-09-08T06:03:48Z

[HIL trust list]

Trusted users for this PR (click to expand)

@okhsunrog

github-actions · 2026-09-08T06:03:49Z

Author @okhsunrog was trusted for this PR via the trusted-author label.
They can now use /hil quick, /hil full and /hil <CHIPS> and their features.

playfulFence

Привет! Спасибо за PR 😄

A couple of nitpicks here and there

playfulFence · 2026-09-08T12:01:46Z

+
+/// Converts a payload length in bytes to the smallest CAN FD data length code
+/// that can carry it.
+pub const fn len_to_dlc(len: u8) -> u8 {


it's pub and exported as pub, so maybe we want an assert for max len here?
same for dlc_to_len

Done the way you suggested: debug_assert! on the input, and both helpers saturate in release (len > 64 maps to code 15, dlc > 15 to 64 bytes), with that spelled out in the docs. len_to_dlc(65) used to produce a code that does not exist. The alternative was returning Option, but that changes two public const fn signatures for input the frame constructors already reject; happy to switch if you prefer that. One detail: the asserts are core::debug_assert!, because the crate's own macro becomes defmt::debug_assert! under the defmt feature and that cannot run inside a const fn.

Does this need to be public, though? If we make the users call the conversion, they can make a mistake. Can we hide this behind the frame API, by taking a byte length, and adding getters for both DLC and data length?

I'll take a look how to make this nicer

Brings in the TWAI FD TX buffer memory for the ESP32-C5 (esp-pacs#506), the MCPWM register rework (esp-pacs#415) and the CLINT blocks for the ESP32-P4, H4 and S31 (esp-pacs#505). Every crate that pins esp-pacs moves together; esp-lp-hal pins the LP PACs at their own revision and stays put.

The TX buffer RAM is in the PAC now, as four arrays of 20 words at the offsets the driver used to compute by hand, so the address arithmetic and the note about the SVD gap go away.

okhsunrog · 2026-09-08T17:48:26Z

/hil esp32c5 --test canfd

github-actions · 2026-09-08T17:48:51Z

Triggered HIL run for #6280 (chips: esp32c5).

Run: https://github.com/esp-rs/esp-hal/actions/runs/34259323939

Status update: ❌ HIL (per-chip) run failed (conclusion: failure).

okhsunrog · 2026-09-08T18:12:04Z

/hil esp32c5 --test canfd

github-actions · 2026-09-08T18:12:31Z

Triggered HIL run for #6280 (chips: esp32c5).

Run: https://github.com/esp-rs/esp-hal/actions/runs/34261649384

Status update: ❌ HIL (per-chip) run failed (conclusion: failure).

On the runner the two-node tests pass at the default 2 Mbit/s: the two wired pins pull the line recessive with both internal pull-ups. The loopback tests use one pin, and its single pull-up is too slow for that rate, so they failed there. Their data phase now runs at 1 Mbit/s; the secondary sample point test keeps the default timing, because its numbers are about that timing and it transmits nothing.

okhsunrog · 2026-09-08T18:34:25Z

/hil esp32c5 --test canfd

github-actions · 2026-09-08T18:34:52Z

Triggered HIL run for #6280 (chips: esp32c5).

Run: https://github.com/esp-rs/esp-hal/actions/runs/34263866039

Status update: ✅ HIL (per-chip) run succeeded.

- The function clock mux is switched after the controller is disabled, not between quiescing and disabling it. - MAX_RETRANSMIT_LIMIT is exported, since UnsupportedRetransmitLimit names it. - FrameError::InvalidDataLengthCode had no producer and is gone: the constructors take a payload length and derive the code. - Identity::is_ctu_can_fd() replaces the public device ID constant that only existed to be compared by hand. - set_tx_priority is on the TX half, and rx_overrun, clear_rx_overrun and rx_buffer_words on the RX half, so they stay reachable after split(); the driver methods delegate. - len_to_dlc and dlc_to_len assert their input in debug builds and saturate otherwise. The asserts use core::debug_assert!, because the crate's own macro is the defmt one under that feature and cannot run in a const fn.

bugadani · 2026-09-09T07:43:44Z

+//! a queue overflows: once the hardware RX buffer is full, no software buffer
+//! could have helped anyway.
+
+use embassy_sync::waitqueue::AtomicWaker;


Suggested change

use embassy_sync::waitqueue::AtomicWaker;

use crate::asynch::AtomicWaker;

At least for now this will be the better fit for S31

bugadani · 2026-09-09T07:48:50Z

+
+/// Largest retransmission limit the four-bit `RTRTH` field can hold
+/// (TRM 38.3.8.4).
+pub const MAX_RETRANSMIT_LIMIT: u8 = 15;


I really dislike encoding hardware-specific constants, we should be able to derive this from the PAC

bugadani · 2026-09-09T07:52:23Z

+    /// `dlc` must already be encoded; use [`len_to_dlc`] to derive it from a
+    /// payload length.
+    #[allow(clippy::too_many_arguments)]
+    pub fn build(


This will need a nicer constructor. We can either pass a struct or an enum where the bools are named, or build an actual builder, but 4 bools back to back in what looks like the primary way to construct a frame is not great.

bugadani · 2026-09-09T07:53:35Z

+    }
+
+    /// Copies the payload into `buf` and returns its length in bytes.
+    pub fn data(&self, buf: &mut [u8; MAX_DATA_LEN]) -> usize {


Couldn't we just return the data as a &[u8]?

bugadani · 2026-09-09T07:58:25Z

+/// Low-level accessor for one CAN FD controller.
+pub(super) struct Ll {
+    regs: *const RegisterBlock,
+}
+
+// SAFETY: the pointer addresses a memory-mapped register block that exists for
+// the whole life of the program, so it can be used from any context. Without
+// these the drivers holding an `Ll` could not be moved into a task or shared
+// with an interrupt handler through a `static`. Whether concurrent access is
+// sound is decided one level up: the drivers only read through `&self`, and
+// everything that writes takes `&mut self`.
+unsafe impl Send for Ll {}
+unsafe impl Sync for Ll {}


We tend to call these Driver

bugadani · 2026-09-09T08:00:30Z

I got bored halfway through, typing review comments on a PR this size is impossibly slow. But there is quite a bit of unnecessary Claudish in comments, if you would run this through a prompt that cuts most of that fluff out ("Cut back comments to the minimum essential information" works for me) that can get rid of redundant things like explaining why Send/Sync is necessary on a struct that wraps a pointer.

bugadani · 2026-09-09T08:04:02Z

+
+use hil_test as _;
+
+mod canfd {


Let's unwrap this a level, this module just makes the code shift rightward for absolutely no reason

bugadani · 2026-09-09T08:07:17Z

+            let (loopback_pin, _) = hil_test::common_test_pins!(peripherals);
+            let (rx, tx) = unsafe { loopback_pin.split() };


Why are you complicating this? The two pins are connected physically

This is copy paste from early attempts when I tested without connected pins, will fix

bugadani · 2026-09-09T08:17:18Z

+    fn config() -> Config {
+        Config::default()
+            .with_mode(Mode::LoopbackSelfTest)
+            .with_no_transceiver(true)


Why are we forcing opendrain pins? Most of the tests work fine without this, the ones that don't do legitimately need open-drain to not destroy hardware, but that can be localised to those tests. Without this config I think you could test the higher baud rates.

github-actions · 2026-09-10T16:29:33Z

New commits in main have made this PR unmergeable. Please resolve the conflicts.

okhsunrog added 3 commits September 8, 2026 02:30

okhsunrog requested review from MabezDev, bjoernQ, bugadani and playfulFence as code owners September 7, 2026 23:36

bugadani added the trusted-author Allow the author of this Pull Request to run HIL tests and the `binary-size` test. label Sep 8, 2026

playfulFence reviewed Sep 8, 2026

View reviewed changes

okhsunrog added 2 commits September 8, 2026 20:33

esp-hal: write CAN FD TX buffers through the PAC

fa1fa5a

The TX buffer RAM is in the PAC now, as four arrays of 20 words at the offsets the driver used to compute by hand, so the address arithmetic and the note about the SVD gap go away.

okhsunrog requested review from JurajSadel and SergioGasquez as code owners September 8, 2026 17:33

okhsunrog force-pushed the canfd-esp32c5 branch from c4669a3 to 0d1ef5c Compare September 8, 2026 18:32

okhsunrog force-pushed the canfd-esp32c5 branch from 4e1abe8 to 4032a05 Compare September 8, 2026 19:07

okhsunrog requested a review from playfulFence September 8, 2026 20:57

bugadani reviewed Sep 9, 2026

View reviewed changes

Comment thread esp-metadata/devices/esp32c5/clocks.toml

bugadani reviewed Sep 9, 2026

View reviewed changes

github-actions Bot added the merge-conflict Merge conflict detected. Automatically added/removed by CI. label Sep 10, 2026

	use embassy_sync::waitqueue::AtomicWaker;
	use crate::asynch::AtomicWaker;

		let (loopback_pin, _) = hil_test::common_test_pins!(peripherals);
		let (rx, tx) = unsafe { loopback_pin.split() };

Conversation

okhsunrog commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Submission Checklist 📝

Extra:

Pull Request Details 📖

Description

Testing

Changelog

esp-hal

esp-metadata-generated

esp-radio

esp-rom-sys

esp-storage

esp-phy

Uh oh!

github-actions Bot commented Sep 8, 2026

[HIL trust list]

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

playfulFence left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okhsunrog commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

okhsunrog commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

okhsunrog commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bugadani commented Sep 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

okhsunrog commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading