Conversation
…SM for other purposes)
ianrrees
left a comment
There was a problem hiding this comment.
I feel like the value/cost this PR provides is not what the project needs at this time - the value seems to essentially be that it gives compile errors if a chip with HSM is targeted and the wrong SERCOM is used(?), but the cost is an increased support burden. If a person wants to make some firmware for these PIC32CS SG chips - can't they simply tell this crate to target the equivalent SAM chip, and avoid using the wrong SERCOM?
At a high level, our issue is that maintainers are overextended; we have foundational work (#892 in particular) that is bitrotting, and bugs that need fixing
|
|
||
| [dependencies] | ||
| aes = "0.8.4" | ||
| atsamd-hal-macros = {version = "0.3.0", path = "../atsamd-hal-macros"} |
There was a problem hiding this comment.
The formatting changes make this PR hard to review - if they're necessary then please open a separate PR for them
| # atsamd & atsame support for Rust | ||
|
|
||
| This repository holds various crates that support/enable working with Microchip (nee Atmel) `samd11`, `samd21`, `samd51` and `same5x` based devices using Rust. | ||
| This repository holds various crates that support/enable working with Microchip (nee Atmel) `samd11`, `samd21`, `samd51` and `same5x` based devices using Rust, and also the newer Microchip `PIC32CX SG` series |
There was a problem hiding this comment.
This language makes the PIC32CX SG feel really "bolted on", maybe something like "working with Microchip SAMD family of devices including SAMD11, SAMD21...PIC32CX SG."
There was a problem hiding this comment.
would
"This repository holds various crates that support/enable working with Atmel (Now Microchip) devices (samd11, samd21, samd51 and same5x), as well as Microchip's own PIC32CX SG series using Rust "
sound better then?
(We could change this into a list, to better future proof this if we eventually get around to 1 day adding the next generation PIC32C series of ICs)
| #[hal_cfg("sercom2")] | ||
| C: (Sercom2, Pad3, IoSet2), | ||
| #[hal_cfg("sercom4")] | ||
| #[hal_cfg(all("sercom4", not("hsm")))] |
There was a problem hiding this comment.
It's not immediately clear to me what this code actually means - if the HSM is present, does the chip have SERCOM4?
There was a problem hiding this comment.
Chips which contain the HSM module have these IOs hard wired to sercom 2 , thus cannot be used by any other peripheral (and the IOs are not even connected to the legs of the chip, so are not externally exposed)
|
Originally, this is what I wanted to do, just add a note to the readme, but then noticed this chip series has a new 80 pin variant that SAM5x doesn't have, and also slightly different peripheral counts (all chips have ethernet+2x can for example), so I then added the definitions to the devices yaml The stuff about HSM though I am on the fence about. It would make sense to these extra compile error checks to avoid someone doing something wrong, but equally, it may be easier to just add a note somewhere documenting it (as the chips that have HSM are about 4x as expensive, it may be fair to assume a user has one for the specific reason to use the HSM, so they know what they are doing) |
|
Probe-rs PR is merged, converting this from a draft |
Summary
As mentioned in #915, this specific PIC32C series is essentially a rebrand of SAME54, with the optional integration of a HSM Security module, and an extra register for NVM that allows for immutable boot.
I have used Microchips latest SVDs to generate the following new PACs, and applied a couple of XSL patches where necessary:
SG41 series - Immutable boot only
PIC32CXSG41064PIC32CXSG41080PIC32CXSG41100PIC32CXSG41128SG60 series - HSM Module only
PIC32CXSG60100PIC32CXSG60128SG61 series - Immutable boot and HSM
PIC32CXSG61100PIC32CXSG61128There is also now an additional family added to the devices.yaml file called
pic32cxsg- This is used mostly withhal_cfg(any(d5x,pic32cxsg))in the crate as most of the peripheral logic is the same between both, but there is the occasional difference, so this lets us better control logic where necessary (For example, on the ADC, TSEN temperature sensor is deprecated on the PIC32CXSG.Things to finalize