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.

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

FlexIO module naming consistency for 1011, 1021 #37

Closed
mciantyre opened this issue Jun 15, 2023 · 6 comments · Fixed by #38
Closed

FlexIO module naming consistency for 1011, 1021 #37

mciantyre opened this issue Jun 15, 2023 · 6 comments · Fixed by #38
Milestone

Comments

@mciantyre
Copy link
Member

Since the 1011 and 1021 MCUs only have one FlexIO peripheral instance, the RAL generation pipeline names the module as flexio1.

pub mod flexio1 {

This prevents us from using a consistent module path in Rust code. See the (current) build status of imxrt-rs/imxrt-hal#139 for an example of an issue that manifests when names aren't consistent.

Use renaming techniques (SVD patches, raltool transforms) to rename flexio1 to flexio in 1011 and 1021 MCUs.

@mciantyre
Copy link
Member Author

Here's how this was solved for ADC and PWM peripherals on the 1011. Something similar should still work for FlexIO.

# Rename ADC1 => ADC to keep module names consistent
# across chips:
#
# // Before
# use imxrt_ral::adc1;
# // After
# use imxrt_ral::adc;
#
# Same for PWM.
_modify:
ADC1:
name: ADC
PWM1:
name: PWM

@Finomnis
Copy link
Contributor

This is a breaking change, though, so it will require a major version bump. Although you are probably aware.

@mciantyre
Copy link
Member Author

mciantyre commented Jun 15, 2023

I haven't tried anything yet, but I think there's a backwards-compatible approach. After correcting the module name, I'll create a 0.5 release branch. Then, I'll manually add modules for the old name, something like

#[deprecated(since = "0.5.1", note = "Use the 'flexio' module instead of this module")]
pub mod flexio1 {
  pub use super::flexio::*;
}

If that all works, we could release that in 0.5.1 for immediate use in the HAL. I assume the maintainer burden to keep the patches for the rest of the 0.5 release. A commit with some smoke tests could help us make sure this all works, and help me with any longer-term maintenance.


The path imxrt_ral::flexio::FLEXIO, without the '1' on FLEXIO, is consistent with the way the ADC instance types and pointer items are generated after the 1011 module rename. The ADC instance types / pointer item is just ADC, not ADC1. (Good eye; this would play into the aliasing idea.)

Specific to the clock gate routines, the HAL handles this with a branch that checks for the "sole instance" const generic. Here's the ADC clock gate implementation.

@Finomnis
Copy link
Contributor

Finomnis commented Jun 15, 2023

@mciantyre I added the first part of your suggestion. If you volunteer to provide the 0.5.1, then I feel like this is all set :)

@Finomnis
Copy link
Contributor

Finomnis commented Jun 15, 2023

I also added a namespace existence smoke test in that PR.

@Finomnis
Copy link
Contributor

@mciantyre Any feedback on this? What's missing?

@mciantyre mciantyre added this to the Release 0.5.1 milestone Jun 27, 2023
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 a pull request may close this issue.

2 participants