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

Jump to bottom

STM32G4 FDCAN driver #607

Merged
merged 8 commits into from
Apr 13, 2021
Merged

STM32G4 FDCAN driver #607

merged 8 commits into from
Apr 13, 2021

Conversation

chris-durand
Copy link
Member

@chris-durand chris-durand commented Apr 4, 2021 •
edited
Loading

This is a follow-up to #325 and adds an FDCAN driver working for standard CAN with an interface similar to the bxCAN driver. Do not consider this driver done, support for FDCAN features is totally missing. The intention was to provide a working implementation of standard CAN first before we start experimenting with redesigning APIs for FDCAN support. So it still has the integrated buffers like the bxCAN driver, not the concept of #294 to implement buffering outside of the driver.

This is tested in hardware on a custom board with a G473RB. The Nucleo example compiles but is untested. An on-target unit test for a Nucleo board using the FDCAN internal loopback mode will follow.

One thing that is still not solved is proper handling of the global clock prescaler which affects all instances. Configuring it is really ugly because the FDCAN1 instance needs to be put into initialization mode to reconfigure it. Right now it simply does that on initialize. Solved by moving common clock settings to Rcc.

A simple but suboptimal solution could be letting the user configure it inside SystemClock and the FDCAN driver does not need to touch it at all. If your data rates are not less than 50 kbit/s standard rate (and 300 kbit/s fast data bitrate for canfd with bit rate switching) a prescaler of 1 will do anyway, even with a 170MHz peripheral clock. An option is yet missing to bypass the fast bitrate assert if CANFD is not used at all. Implemented now.

Another aspect to consider is trying to create a common interface for CAN filters for different drivers. This is not straightforward since the semantics and feature sets differ between drivers. FDCAN has 8 filters that apply to extended id messages and 28 for standard 2.0A messages. bxCAN does not have this strict distinction and treats the frame format as a bit of the id and mask. This is not supported for FDCAN. Furthermore, FDCAN filters do not consider the RTR flag. Because I didn't find an obvious solution for an easy to use API, I skipped that effort for now, ignored @rleh's modm::can::Filter type and went for a less ambitious hardware specific API. Refactoring of CAN filters is out of scope for this PR.

If you look at the SAM E5x manual, you will find that the message RAM data structures are identical to the STM32 FDCAN implementation. I guess both vendors did some copy-paste from an automotive standard. Does anyone know more details?

  • Working standard CAN
  • CAN filter support
  • Hardware unit test in loopback mode
  • Test hardware unit test on Nucleo board (Thanks for testing, @rleh)
  • Test example with Nucleo and CAN transceiver

cc @rleh @salkinium

@chris-durand chris-durand marked this pull request as draft April 4, 2021 19:57
@rleh
Copy link
Member

rleh commented Apr 4, 2021

This is tested in hardware on a custom board with a G473RB.

I can test the example on the Nucleo-G474RE board.

@chris-durand chris-durand added the ci:hal Triggers the exhaustive HAL compile CI jobs label Apr 5, 2021
@rleh
Copy link
Member

rleh commented Apr 6, 2021

This is tested in hardware on a custom board with a G473RB.

I can test the example on the Nucleo-G474RE board.

Done, works 👍🏽

CAN Test Program
Initializing Fdcan1...
Setting up Filter for Fdcan1...
Initializing Fdcan2...
Setting up Filter for Fdcan2...
Sending message on Fdcan1...
Sending message on Fdcan2...
Fdcan2: Message is available...
id = 0001, len = 1, flags = rE, data = 11 
Fdcan1: Message is available...
id = 0001, len = 1, flags = rE, data = 22

grafik

rleh
rleh requested changes Apr 6, 2021
View reviewed changes
Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Nice work, thank you a lot! 👍🏽

I really like the MessageRam abstraction!

src/modm/architecture/interface/can_filter.hpp Outdated Show resolved Hide resolved
src/modm/architecture/interface/can_filter.cpp Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/can.cpp.in Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/can.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/can.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/can.hpp.in Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/message_ram.hpp Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/message_ram.hpp Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/message_ram.hpp Outdated Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/message_ram.hpp Show resolved Hide resolved
@rleh
Copy link
Member

rleh commented Apr 6, 2021

Aside from the minor suggested changes, I would like to merge this PR soon and add the other features in future PRs, or is there something against that?
(I want to prevent this PR from becoming stale (again)).

@rleh
Copy link
Member

rleh commented Apr 6, 2021

If you look at the SAM E5x manual, you will find that the message RAM data structures are identical to the STM32 FDCAN implementation. I guess both vendors did some copy-paste from an automotive standard. Does anyone know more details?

I guess ST and Atmel/Microchip bought the Fdcan IP from the same IP-vendor?

@chris-durand
Copy link
Member Author

I will squash the commits after review.

@chris-durand chris-durand changed the title WIP: STM32G4 FDCAN driver STM32G4 FDCAN driver Apr 12, 2021
@chris-durand chris-durand marked this pull request as ready for review April 12, 2021 21:14
@chris-durand chris-durand requested review from rleh and salkinium April 12, 2021 21:15
@chris-durand chris-durand added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Apr 12, 2021
salkinium
salkinium reviewed Apr 13, 2021
View reviewed changes
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

A very thorough driver!

src/modm/architecture/interface/can_message.hpp Show resolved Hide resolved
src/modm/platform/can/stm32-fdcan/can.cpp.in Outdated Show resolved Hide resolved
src/modm/platform/clock/stm32/rcc.cpp.in Outdated Show resolved Hide resolved
src/modm/board/nucleo_g474re/board.hpp Outdated Show resolved Hide resolved
src/modm/platform/clock/stm32/rcc.hpp.in Show resolved Hide resolved
@chris-durand chris-durand added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Apr 13, 2021
@salkinium
Copy link
Member

Rebase?

rleh and others added 5 commits April 13, 2021 16:01
@rleh @chris-durand
[stm32] Add fdcan driver
5f0d61b
@chris-durand
[architecture] Add type-safe wrappers for CAN identifiers
f7726f5
@chris-durand
[stm32] Finish FDCAN driver to support standard CAN + refactoring
f3c6be9 
Finish FDCAN driver to at least work with standard CAN. Message RAM
handling is factored out to a separate source file. FDCAN features and a
new concept for buffering can be implemented later.
@rleh @chris-durand
[examples] Add Nucleo-G474RE CAN example
d8977ad
@chris-durand
[test] Add FDCAN hardware unit test for Nucleo G474RE
2d2199b
@chris-durand
Copy link
Member Author

done

@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Apr 13, 2021
@salkinium salkinium merged commit 2d2199b into modm-io:develop Apr 13, 2021
@chris-durand chris-durand deleted the feature/fdcan branch April 13, 2021 15:27
@salkinium salkinium added this to the 2021q2 milestone Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rleh rleh rleh approved these changes

@salkinium salkinium salkinium approved these changes

Assignees
No one assigned
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs example 🔑 feature 🚧
Milestone
2021q2
Development

Successfully merging this pull request may close these issues.
3 participants
@chris-durand @rleh @salkinium