atsamd
atsamd copied to clipboard
Add BSP for Adafruit Feather M4 CAN Express
Summary
Adds a BSP for the Adafruit Feather M4 CAN Express board.
Blinky, Neopixel and USB examples have been tested on actual hardware.
There was some discussion in #622 about using features for CAN support rather than adding a new BSP. However, as this board uses a SAME51 part (rather than the SAMD on the M4 Express) I thought it made sense to keep it separate as it would have dependencies on a different PAC.
The CAN module is not yet exposed in the BSP. My intention would be to support the embedded_can HAL using the mcan crate (cf. #654).
Checklist
- [X]
CHANGELOG.mdfor the BSP or HAL updated - [X] All new or modified code is well documented, especially public items
- [X] No new warnings or clippy suggestions have been introduced (see CI or check locally)
If Adding a new Board
- [X] Board CI added to
crates.json - [X] Board is properly following "Tier 2" conventions, unless otherwise decided to be "Tier 1"
@jboynes, using a different chip/BSP is definitely a stronger reason to have a different BSP. But it's also not a total dealbreaker to reuse the existing BSP. Other than CAN, are there any other major differences between the E51J and D51J chips? And what about the boards themselves? Are the pinouts and peripherals essentially identical between the two boards? Or is there variation there as well?
We might be able to get away with some hacking in Cargo.toml, which I think would benefit everyone. You would get free Tier 1 support and there would be less code duplication.
To my knowledge, the only difference between the D51J and E51J is the presence of the CAN module.
The chip identifies differently which could affect debugging (e.g. the "chip" value in [metadata]). By default though, flashing is done using hf2 which does not rely this. The board does have pads to attach a SWD probe though.
Differences on the board compared to the M4 are:
batterypinV_DIVchanged fromPB01toPB00neopixeldata pinD8_NEOPIXchanged fromPB03toPB02PB03is now used as power for the neopixelNEO_PWR
The on-board CAN transceiver uses the following pins which are unused on the basic M4:
PB13is used to enable the +5V supply for the CAN transceiverBOOST_ENPB12is used to put the CAN transceiver in standby modeCAN1_SPB14andPB15are used for CAN dataCAN1_TXandCAN1_RX
Another advantage of having this as a separate BSP crate is that it can version separately from the feather_m4 one, especially as the embedded_can HAL is still relatively new so there are likely to be CAN-specific changes that won't affect most users. It would also allow the base BSP to be updated with needing to test on CAN hardware.
I'm quite curious how this BSP will develop. My understanding is that you want to implement embedded-can on mcan in some opinionated way that will kind of work for toy projects. As far as my knowledge goes embedded-can is quite limited and makes a lot of assumptions (like that TX and RX is supposed to use the same frame type which in MCAN is not that straightforward). This is why we only use primitives from embedded-can, like Id enums and such.
I'm quite curious how this BSP will develop. My understanding is that you want to implement
embedded-canonmcanin some opinionated way that will kind of work for toy projects. As far as my knowledge goesembedded-canis quite limited and makes a lot of assumptions (like that TX and RX is supposed to use the same frame type which in MCAN is not that straightforward). This is why we only use primitives fromembedded-can, like Id enums and such.
I don't have many opinions here :shrug:. My thought was that embedded_hal was using embedded_can so that would be a natural way to go. I agree the API looks fairly basic at the moment (e.g. I see no support for CAN-FD, filters, priorities, etc.) which just means there's work to do. That's mainly why I thought that any BSP for a board with CAN would see more churn than one without. It's also a case for not making this a Tier 1 BSP, at least for now.
Having more boards supported would allow more testing of the HAL and its abstractions. This PR adds a SAM E51-based board, and I've proposed adding a SAM C21 (Cortex-M0) board as well for some diversity in Microchip/Atmel devices. And of course, there are other devices using Bosch's M_CAN IP as well (e.g. the STM32G4 (cf. fdcan) and LPC5516 MCUs, or the TCAN4550 external controller).
@jboynes, I think this case is right on the deciding line. The small board differences could be handled with #[cfg] attributes in the bsp_pins! macro, and selecting the correct chip variant could be done with a Cargo feature. But I don't see any way to change the package metadata. That's probably the biggest driver to using a separate BSP. However, by going the separate route, you give up Tier 1 support and increase code duplication. We could eventually consider upgrading the board to Tier 1, but it probably wouldn't happen immediately.
I think I'll leave it up to you (and possibly anyone else who wants to weigh in). Would you prefer this as a separate BSP? If so, we can merge it.
Would you prefer this as a separate BSP? If so, we can merge it.
I think it would be preferable for now, because:
- It can be Tier 2
- Being Tier 2, it can iterate more quickly in response to changes in the CAN HAL
- It can still be upgraded to Tier 1 when things stabilize
There's not a lot of production code to maintain in the BSP itself. I think most of the maintenance work would be in tracking dependencies. Ideally, the BSP would only define devices on the board and the pin mappings that connect to them; things that are board specific, and only things that are board specific.
I will explore if there's a way to refactor this to reduce the duplication. I'll take a look at unifying with the atsme54_xpro as that's also Tier 2 and would also be affected by the CAN support.