Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable TWAI CLKOUT when initializing peripheral #1949

Closed
wants to merge 1 commit into from
Closed

Disable TWAI CLKOUT when initializing peripheral #1949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions esp-hal/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- SPI: disable and re-enable MISO and MOSI in `start_transfer_dma`, `start_read_bytes_dma` and `start_write_bytes_dma` accordingly (#1894)
- TWAI: GPIO pins are not configured as input and output (#1906)
- ESP32C6: Make ADC usable after TRNG deinicialization (#1945)
- TWAI: CLKOUT is not disabled on initialization (#1949)

### Removed

Expand Down
5 changes: 5 additions & 0 deletions esp-hal/src/twai/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,11 @@ where
.err_warning_limit()
.write(|w| unsafe { w.err_warning_limit().bits(96) });

// Disable CLKOUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain why you added this piece of code, not (or not only, I guess...) what it does. It's completely unobvious to me (as an uninformed reader) why this has any effect.

Also, do we want this done for all MCUs, when the issue only seems to affect C3? Slightly related, do we want to have a test case in our test suite for this? (I think yes!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently there is no support for using the CLKOUT functionality of the TWAI peripheral in esp-hal, so I do not think there is a reason to leave the clock divider active. This also matches the behaviour from the ESP-IDF, which in the default configuration also does not use the CLKOUT functionality.

In the default configuration clkout_divider is zero, and clkout_gpio is TWAI_IO_UNUSED. The clkout_divider configuration is passed from twai_driver_install_v2 to twai_hal_configure, which in turn calls twai_ll_set_clkout (https://github.com/espressif/esp-idf/blob/master/components/driver/twai/twai.c#L475 and https://github.com/espressif/esp-idf/blob/master/components/hal/twai_hal.c#L64).

The function twai_ll_set_clkout is chip specific, but for all chips with a TWAI peripheral does the exact same thing when passed zero. It sets the TWAI_CLOCK_OFF bit and sets TWAI_CD to zero. See esp32, esp32c3, esp32c6, esp32p4, esp32s2 and esp32s3. So I think it makes sense to match this configuration.

Why exactly this fixes the issue with receiving on GPIO1, I do not know. In theory leaving the clock divider active should not influence the receive operation, but I guess it does. If esp-hal is going to support the CLKOUT functionality, then we should probably add a note about receiving on GPIO1 when CLKOUT is enabled.

As for the test case, could you point me in the right direction where to add the test?

T::register_block()
.clock_divider()
.write(|w| unsafe { w.cd().bits(0).clock_off().set_bit() });

let mut cfg = TwaiConfiguration {
peripheral: PhantomData,
phantom: PhantomData,
Expand Down