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

Revise some API docs due to aggressive combining #35

Merged
merged 5 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ Exclude the interrupt vector table when we're building for a target with an
operating system. This ensures you can build imxrt-ral in different contexts,
like build scripts.

Drop all register fields that are documented as "reserved" (first word of the
description, all lowercase). Dropping these fields changes the combiner's
approach for combining fieldsets, enabling correct documentation for non-
reserved fields.

Fix the documentation associated with IOMUXC field values.

## [0.5.0] 2022-12-27

Add support for NXP's i.MX RT 1176 dual-core MCUs. An `"imxrt1176_cm7"` feature
Expand Down
11 changes: 11 additions & 0 deletions devices/common_patches/ccm_cg_gpio5.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Clock gate 15 in CCM[CCGR1] is missing from 1050 and 1060 MCUs.
# It isn't reserved according to the reference manual. This
# clock gate is for GPIO5.
CCM:
CCGR1:
_modify:
CG15:
description: gpio5 clock (gpio5_clk_enable)
bitOffset: 30
bitWidth: 2
access: read-write
1 change: 1 addition & 0 deletions devices/imxrt1021.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1021.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
1 change: 1 addition & 0 deletions devices/imxrt1051.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1051.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb1.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
1 change: 1 addition & 0 deletions devices/imxrt1052.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1052.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb1.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
1 change: 1 addition & 0 deletions devices/imxrt1061.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1061.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb1.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
1 change: 1 addition & 0 deletions devices/imxrt1062.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1062.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb1.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
1 change: 1 addition & 0 deletions devices/imxrt1064.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _svd: "../svd/imxrt1064.svd"

_include:
- "common.yaml"
- "common_patches/ccm_cg_gpio5.yaml"
- "common_patches/pwm1/submodule_cluster.yaml"
- "common_patches/usb1.yaml"
- "common_patches/dma0/tcd_cluster.yaml"
Expand Down
12 changes: 12 additions & 0 deletions raltool-cfg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,15 @@ combines:
- iomuxc
- iomuxc_gpr
- iomuxc_snvs
- StrictEnumDescs:
- iomuxc
- iomuxc_gpr
- iomuxc_snvs
# IR paths that should never be combined.
#
# These regexes match IR paths. If there's a match, that element is *never* considered for
# combination by the combiner. When applied to blocks, this is a stronger "never combine"
# guarantee than StrictEnumNames. In particular, if an IOMUXC block were in this list, this
# would prevent the 1061 IOMUXC from being combined with the 1062 IOMUXC.
- NeverCombine:
- ccm::regs::Ccgr\d+
93 changes: 86 additions & 7 deletions raltool/src/combine.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Helper types to combine and consolidate IRs across devices.

use crate::ir;
use regex::Regex;
use serde::{Deserialize, Serialize};
use std::{
cmp::Ordering,
Expand Down Expand Up @@ -103,6 +104,30 @@
}
}

/// Equivalence adapter for checking paths against a set of regexes.
///
/// If there's a regex match, the element is deemed "never equivalent".
/// This check happens before calling the wrapped equivalence.
struct PathExcluded<'a, Equiv> {
exclusions: &'a [Regex],
equiv: Equiv,
}

impl<Equiv, E> Equivalence<E> for PathExcluded<'_, Equiv>
where
Equiv: Equivalence<E>,
{
fn compare(&self, left: CompareIr<E>, right: CompareIr<E>, path: IrPath) -> bool {
for exclusion in self.exclusions {
if exclusion.is_match(path) {
return false;
}
}

self.equiv.compare(left, right, path)
}
}

/// Ensure the items in two, possibly non-sorted contiguous
/// collections are equivalent.
fn equivalent_slices<E>(xs: &[E], ys: &[E], equiv: impl Fn(&E, &E) -> bool) -> bool {
Expand All @@ -124,7 +149,8 @@

#[derive(Clone, Copy)]
struct EquivalentEnums<'a> {
peripherals: &'a HashSet<String>,
names: &'a HashSet<String>,
descs: &'a HashSet<String>,
}

impl Equivalence<ir::Enum> for EquivalentEnums<'_> {
Expand All @@ -134,10 +160,13 @@
CompareIr { elem: b, .. }: CompareIr<ir::Enum>,
path: IrPath,
) -> bool {
let assert_name_equivalence = self.peripherals.contains(peripheral_part(path));
let assert_name_equivalence = self.names.contains(peripheral_part(path));
let assert_desc_equivalence = self.descs.contains(peripheral_part(path));
a.bit_size == b.bit_size
&& equivalent_slices(&a.variants, &b.variants, |q, r| {
q.value == r.value && (!assert_name_equivalence || q.name == r.name)
q.value == r.value
&& (!assert_name_equivalence || q.name == r.name)
&& (!assert_desc_equivalence || q.description == r.description)
})
}
}
Expand Down Expand Up @@ -317,15 +346,45 @@
impl<'ir> IrVersions<'ir> {
/// Define versions of IR elements from the collection of IRs.
pub fn from_irs(irs: &'ir [ir::IR], config: &Config) -> Self {
let exclusions: Vec<_> = config
.never_combine
.iter()
.map(|path| Regex::new(&path).unwrap())

Check warning on line 352 in raltool/src/combine.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> raltool/src/combine.rs:352:36 | 352 | .map(|path| Regex::new(&path).unwrap()) | ^^^^^ help: change this to: `path` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow = note: `#[warn(clippy::needless_borrow)]` on by default
.collect();
let exclusions = &exclusions;

let enums = EquivalentEnums {
peripherals: &config.strict_enum_names,
names: &config.strict_enum_names,
descs: &config.strict_enum_descs,
};
let fieldsets = EquivalentFieldSets { enums };
let blocks = EquivalentBlocks { fieldsets };

Self {
enums: VersionLookup::from_irs(enums, irs, |ir| &ir.enums),
fieldsets: VersionLookup::from_irs(fieldsets, irs, |ir| &ir.fieldsets),
blocks: VersionLookup::from_irs(blocks, irs, |ir| &ir.blocks),
enums: VersionLookup::from_irs(
PathExcluded {
exclusions,
equiv: enums,
},
irs,
|ir| &ir.enums,
),
fieldsets: VersionLookup::from_irs(
PathExcluded {
exclusions,
equiv: fieldsets,
},
irs,
|ir| &ir.fieldsets,
),
blocks: VersionLookup::from_irs(
PathExcluded {
exclusions,
equiv: blocks,
},
irs,
|ir| &ir.blocks,
),
}
}
/// Access an enum version that corresponds to this IR.
Expand Down Expand Up @@ -360,9 +419,9 @@
impl<T> std::cmp::Eq for RefHash<'_, T> {}

impl<T> Clone for RefHash<'_, T> {
fn clone(&self) -> Self {
Self(self.0)
}

Check warning on line 424 in raltool/src/combine.rs

View workflow job for this annotation

GitHub Actions / clippy

non-canonical implementation of `clone` on a `Copy` type

warning: non-canonical implementation of `clone` on a `Copy` type --> raltool/src/combine.rs:422:29 | 422 | fn clone(&self) -> Self { | _____________________________^ 423 | | Self(self.0) 424 | | } | |_____^ help: change this to: `{ *self }` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_canonical_clone_impl = note: `#[warn(clippy::non_canonical_clone_impl)]` on by default
}

impl<T> Copy for RefHash<'_, T> {}
Expand All @@ -372,6 +431,8 @@
#[derive(Default)]
pub struct Config {
strict_enum_names: HashSet<String>,
strict_enum_descs: HashSet<String>,
never_combine: HashSet<String>,
}

/// Combine all IRs into a single IR.
Expand Down Expand Up @@ -537,6 +598,18 @@
/// always safe to add to this list; however, it means there may be more
/// code generated.
StrictEnumNames(Vec<String>),
/// The list of peripheral names that require strict enum description
/// checks.
///
/// This is even stricter than [`StrictEnumNames`], since it asserts
/// equal descriptions (human-readable descriptions) for each enum variant.
StrictEnumDescs(Vec<String>),
/// The list of patterns (regex string) to never combine.
///
/// You should design patterns to the IR path names. Note that, unlike
/// transform regexes, these do not implicitly match only starting
/// characters.
NeverCombine(Vec<String>),
}

impl<I> From<I> for Config
Expand All @@ -551,6 +624,12 @@
Combine::StrictEnumNames(peripherals) => {
config.strict_enum_names.extend(peripherals);
}
Combine::StrictEnumDescs(peripherals) => {
config.strict_enum_descs.extend(peripherals);
}
Combine::NeverCombine(paths) => {
config.never_combine.extend(paths);
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions raltool/src/svd2ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@
warn!("unsupported derived_from in fieldset");
}

if let Some(desc) = &f.description {
if desc.to_lowercase().starts_with("reserved") {
continue;
}
}

let mut field = Field {
name: f.name.clone(),
description: f.description.clone(),
Expand Down Expand Up @@ -298,7 +304,7 @@
}
irqs.sort_by_key(|i| &i.name);

for (_n, &i) in irqs.iter().enumerate() {

Check warning on line 307 in raltool/src/svd2ir.rs

View workflow job for this annotation

GitHub Actions / clippy

you seem to use `.enumerate()` and immediately discard the index

warning: you seem to use `.enumerate()` and immediately discard the index --> raltool/src/svd2ir.rs:307:25 | 307 | for (_n, &i) in irqs.iter().enumerate() { | ^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_enumerate_index = note: `#[warn(clippy::unused_enumerate_index)]` on by default help: remove the `.enumerate()` call | 307 | for &i in irqs.iter() { | ~~ ~~~~~~~~~~~
let iname = i.name.to_ascii_uppercase();

if !device.interrupts.iter().any(|j| j.name == iname) {
Expand Down
Loading
Loading