Skip to content

Stm32l412/Stm32l422 PAC specialisation #264

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.

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

Merged
merged 13 commits into from
Dec 10, 2021
Merged

Conversation

Crzyrndm
Copy link
Contributor

@Crzyrndm Crzyrndm commented Oct 24, 2021

#29
based on #261 for device feature gating. Almost all of the changes for this PR are in rtc.rs
Modifications derived from RM0394 Rev4 section 34.x and stm32l4xx_hal_driver v1.13.0
Feedback welcome on the approach taken here

STM32L4 PAC provides a specialised L412 register map which failed to compile with the current code due to differences in the RTC interrupt status handling. This modifies the RTC functions to conditionally modify the RTC register interactions when the L412 or L422 features are set

Questions:

  • The feature gating can get quite messy in come places. RTC::check_interrupt is probably the worst example here with only the EXTI interactions being shared, but even in the basic cases (i.e. single line differences), things could get much messier if more than two options were involved (no #elif/else really doesn't help clarity here). I dont see many previous examples of conditional code (and nothing complex), so I was wondering if there was a standard style that is recommended for this sort of thing
  • I've left the L4P/L4Q feature commented next to many/all of the changes since the C SDK is quite clear that the RTC registers are quite similar to the L41/L42. They are commented because without a PAC to match, the crate would fail to compile with that feature enabled

Note: I do not have an L412/L422 which I can use to verify these changes. The RTC wakeup timer and alarms are the most affected by this so this example would be a decent start at verification
Note2: this is not a complete addition of L41/L42 features, just those that are required for currently implemented functionality to compile with the updated PAC. This does nothing about the peripheral gating as mentioned here

@Crzyrndm Crzyrndm mentioned this pull request Oct 24, 2021
@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Oct 24, 2021

As a side note: The RTC differences are laid out in AN4759 (L41/2 and L4P/Q products are based on the RTC3 model, otherwise RTC2 model). There's more than a few series specific bits laid out in the feature tables, but there is potential for a single RTC interface crate for all STM32 HAL crates to be extracted here

@Crzyrndm
Copy link
Contributor Author

Moved all of the conditionals behind a trait with a conditional impl. The trait isn't exactly neccesary here (could just be impl Rtc or a private module that is conditional) but it does give the best error messages when a function isn't implemented in both sides (indicates an impl issue rather than a caller issue)

Easy enough to change if required, opinions on this structure definitely wanted

@korken89
Copy link
Collaborator

korken89 commented Dec 8, 2021

I think this can be closed now?

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Dec 8, 2021

Pretty sure the RTC changes here weren't in the merged PR (L41/2 and L4P/Q have a different RTC peripheral, reference table 3 of AN4759). Will clean this up for the weekend

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Dec 9, 2021

Rebased. Dropped the traits and just have rtc2/rtc3 modules

RTC3 backup registers are a bit of an issue. I don't see them in the PAC (reference manual has the under a TAMP peripheral). The related functions are no-ops in the rtc3 module for now

@korken89 korken89 merged commit 0e66113 into stm32-rs:master Dec 10, 2021
@Crzyrndm Crzyrndm deleted the stm32l41x branch December 10, 2021 18:43
@Crzyrndm Crzyrndm mentioned this pull request Dec 11, 2021
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants