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

Cleanup IOMUXC feature #75

Merged
merged 13 commits into from
Jul 21, 2020
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
13 changes: 11 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,14 @@ jobs:
run: cd imxrt-hal && cargo fmt --all -- --check
- name: Run clippy (${{ matrix.feature }}) for HAL
run: cd imxrt-hal && cargo clippy --features ${{ matrix.feature }} -- -D warnings
- name: Run tests for iomuxc crates
run: cargo test --package=imxrt-iomuxc --package=imxrt-iomuxc-build --package=imxrt106x-iomuxc

build-iomuxc:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Check formatting
run: cd imxrt-iomuxc && cargo fmt -- --check
- name: Run tests
run: cd imxrt-iomuxc && cargo test --all-features
- name: Run clippy
run: cd imxrt-iomuxc && cargo clippy --all-features
13 changes: 11 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

Thanks for helping us build embedded Rust support for NXP's i.MX RT processors! Please open an issue if

- you find a bug in the RAL or HAL crates
- you find a bug in the HAL, RAL, or IOMUXC crates
- you have an idea for a feature
- something isn't clear in our documentation

## Development

The steps below are useful for developers who want to build and modify the RAL and the HAL. All steps assume that you've cloned the repository.
The steps below are useful for developers who want to build and modify this repository's crates. All steps assume that you've cloned the repository.

### Dependencies

Expand Down Expand Up @@ -43,6 +43,15 @@ Make sure you've generated the RAL (see above). When developing the HAL, specify
cargo check --features imxrt1062 --target thumbv7em-none-eabihf
```

### IOMUXC

The `imxrt-iomuxc` crate family does not require any feature flags, and it will build for your host. Consider using `--package` flags to build and test the crate family in one command:

```
cargo build --package=imxrt-iomuxc --package=imxrt106x-iomuxc --package=imxrt-iomuxc-build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right now, since you've removed the imxrt106x-iomuxc crate?

cargo test -p imxrt-iomuxc -p imxrt106x-iomuxc -p imxrt-iomuxc-build
```

### SVD Patches

To modify the RAL, you'll need to describe your change as an SVD patch. If you'd like to add patches to the i.MX RT SVD files, learn about [svdtools](https://github.com/stm32-rs/svdtools). Use some of the existing SVD patches as a guide.
Expand Down
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@ members = [

"imxrt-iomuxc",
"imxrt-iomuxc/imxrt-iomuxc-build",
"imxrt-iomuxc/imxrt106x-iomuxc",
]
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ The `imxrt-ral` supports all i.MX RT processors:

As with the HAL, the RAL also **requires** a feature flag to specify the processor variant. The RAL is [on crates.io](https://crates.io/crates/imxrt-ral). The RAL provides the `"rt"` feature flag, and the interrupt table definition, that's used by the HAL.

### IOMUXC

The `imxrt-iomuxc` crate family

- defines the i.MX RT pad configuration interface, and
- implements pad definitions for i.MX RT chip variants

The interface is re-exported in the HAL, but maintained in a separate collection of crates. Use the `imxrt-iomuxc` crate family if you'd like to build your own HAL, and you don't want to re-implement all of the code necessary to define and configure processor pads. Use the `imxrt-iomuxc` interfaces if you're creating a driver, and that driver treats processor pads as owned resources.

## Q/A

#### *Are there any board support packages (BSP) that use the `imxrt-hal` crate?*
Expand Down
103 changes: 103 additions & 0 deletions imxrt-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,112 @@
- `steal()` the top-level `Peripherals` object. The `steal()` method lets users use the `imxrt_hal`
as an RTIC device.
- DMA `Memcpy` may support interrupt handling
- A new interface for pad configuration, supported by the `imxrt-iomuxc` crate family. See the Changed
section for migration information.

### Changed

- **BREAKING** The pad configuration interface has simplified. Users will use the
interface exposed by the `imxrt-iomuxc` crate family. This section describes how you might
update your 0.3 (and earlier) code for the new interface.

- *Naming*: the `IOMUXC` instance exposes pads with a new naming convention. Previously, member
accessed resembled

```rust
let peripherals = imxrt_hal::Peripherals::take().unwrap();
peripherals.iomuxc.gpio_ad_b1_02;
```

Now, IOMUXC member access resembles

```rust
peripherals.iomuxc.ad_b1.p02
```

Generally, remove the "`gpio_`" prefix, and replace the second underscore with member access and
a "`p`" symbol.

- *Pad Types*: The interface simplifies the pad types, removing the alternate type state.
Usages resembling

```rust
use imxrt_hal as hal;

type MyPin = hal::iomuxc::gpio::GPIO_AD_B0_12<hal::iomuxc::Alt5>;
```

can be simply expressed as

```rust
type MyPin = hal::iomuxc::ad_b0::AD_B0_12;
```

Note the stuttering convention of `pad_group::pad_group_offset` to reference pad types.

- *No alternate transition*: there are no `altX()` methods. Usage resembling

```rust
let mut uart = uarts.uart2.init(
peripherals.iomuxc.gpio_ad_b1_02.alt2(),
peripherals.iomuxc.gpio_ad_b1_03.alt2(),
BAUD,
).unwrap()
```

should drop the `altX()` calls (after renaming the pads).

```rust
let mut uart = uarts.uart2.init(
peripherals.iomuxc.ad_b1.p02,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really nice API now, I really like the way this call looks with the changes made here

peripherals.iomuxc.ad_b1.p03,
BAUD,
).unwrap();
```

- *Type Tags*: all custom type-level constants, like `imxrt_hal::uart::module::_1` are now
`typenum` constants. There are no peripheral-specific constants. Usage resembling

```rust
use imxrt_hal::uart;

type MyTX = uart::Tx<uart::module::_3>;
```

should update to

```rust
use imxrt_hal::uart;
use imxrt_hal::iomuxc;

type MyTX = uart::Tx<iomuxc::consts::U3>;
```

- *GPIO*: the new IOMUXC driver results in a simpler GPIO interface. There is now a single GPIO
type that wraps an IOMUXC pad. Any GPIO type, like

```rust
use imxrt_hal as hal;

type HardwareFlag = hal::gpio::GPIO1IO26<hal::gpio::GPIO1, hal::gpio::Output>;
```

should change to

```rust
type HardwareFlag = hal::gpio::GPIO<hal::iomuxc::ad_b1::AD_B1_10, hal::gpio::Output>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And again here, the resulting type and API is much nicer now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what I was thinking when putting together the previous GPIO types 😛they were gnarly! There's still more gnarly HAL code I need to rethink...

```

The new GPIO types expose a no-return `toggle()` method, which shadows an embedded_hal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose there's a reason we don't need to return here, its because its effectively infallible correct? Very nice if so

trait method. If you notice compilation issues surrounding `toggle()`, try removing
`unwrap()` calls:

```rust
led.toggle().unwrap() // Old
led.toggle() // New
```
Or, qualify that the `ToggleableOutputPin` trait method should be called.

- **BREAKING** The HAL's `"rtfm"` feature is changed to `"rtic"`, reflecting the framework's
new name. Users who are relying on the `"rtfm"` feature should now use the `"rtic"` feature.
- **BREAKING** The `dma::{Config, ConfigBuilder}` types are gone. This affects the `dma::Peripheral`
Expand Down
10 changes: 2 additions & 8 deletions imxrt-hal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ log = "0.4.8"
version = "0.1.0"
path = "../imxrt-iomuxc"

# TODO this will be exposed by a 106x-specific HAL. We're
# exposing it here for backwards compatibility.
[dependencies.imxrt106x-iomuxc]
path = "../imxrt-iomuxc/imxrt106x-iomuxc"
optional = true

[lib]
bench = false

Expand All @@ -44,11 +38,11 @@ default = ["embedded-hal/unproven"] # Allows us to access the new digital pin tr
#imxrt1051 = ["imxrt-ral/imxrt1051"]
#imxrt1052 = ["imxrt-ral/imxrt1052"]
#imxrt1061 = ["imxrt-ral/imxrt1061"]
imxrt1062 = ["imxrt-ral/imxrt1062", "imxrt106x-iomuxc"]
imxrt1062 = ["imxrt-ral/imxrt1062", "imxrt-iomuxc/imxrt106x"]
#imxrt1064 = ["imxrt-ral/imxrt1064"]
rtic = ["imxrt-ral/rtic"]
rt = ["imxrt-ral/rt"]
nosync = ["imxrt-ral/nosync"]

[package.metadata.docs.rs]
features = ["imxrt1062"]
features = ["imxrt1062"]
13 changes: 2 additions & 11 deletions imxrt-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,7 @@
//! An [`embedded-hal`](https://crates.io/crates/embedded-hal) implementation
//! targeting processors in NXP's IMXRT106x family.
//!
//! The HAL is a WIP. More documentation will become available once more capabilities
//! are exposed.
//!
//! In some cases, the HAL simply re-exports peripherals from the peripheral access
//! crates (PAC). If they are not re-exported, all PAC components are available
//! in the `pac` module.
//!
//! To see examples of the HAL, check out the `teensy4-bsp` and the `teensy4-examples` crates.
//! We will skip documentation example and tests, since we cannot yet test them as part
//! of the `cargo test` workflow...
//! See the module-level documentation for more information and examples.

#![no_std]

Expand All @@ -26,7 +17,7 @@ pub mod iomuxc {
pub use imxrt_iomuxc::*;

#[cfg(feature = "imxrt1062")]
pub use imxrt106x_iomuxc::*;
pub use imxrt_iomuxc::imxrt106x::*;
}

pub mod ccm;
Expand Down
6 changes: 6 additions & 0 deletions imxrt-iomuxc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,9 @@ description = "Pad configuration interface for i.MX RT processors"

[dependencies]
typenum = "1.12.0"

[build-dependencies]
imxrt-iomuxc-build = { path = "imxrt-iomuxc-build" }

[features]
imxrt106x = []
112 changes: 97 additions & 15 deletions imxrt-iomuxc/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# imxrt-iomuxc

An experimental set of crates for i.MX RT pad configuration and resource management.
i.MX RT pad definitions and pin configuration.

## Goals

Expand All @@ -14,32 +14,114 @@ An experimental set of crates for i.MX RT pad configuration and resource managem

## Structure

- `imxrt-iomuxc`, the root crate, defines a set of traits that are implemented
- `imxrt-iomuxc` defines a set of traits that are implemented
on pads. The traits specify that a pad may be used for a certain function,
such as a UART transfer pin or an I2C clock pin. The root crate also provides
common functions to configure pads. `imxrt-iomuxc` is used in crates that need
to treat pads as resources.
- `imxrt106x-iomuxc` provides an implementation of the `imxrt-iomuxc` traits. It
identifies the 106x pads that may be used for UART, SPI, I2C, PWM, etc. It uses
`imxrt-iomuxc-build` to generate pads at compile time. When it comes time to
support other i.MX RT processor variants, we add a new crate, and implement the
pads there.
to treat pads as resources. Without any feature flags, the `imxrt-iomuxc` provides
the general pin configuration interface. There are no processor-specific APIs in
the default build.
- `imxrt-iomuxc` feature flags, like `imxrt106x`, enable processor-specific pad
definitions and pin implementations.
- `imxrt-iomuxc-build` provides **build-time** support for defining pads. It's
used by implementation crates to simply generate all of the pads. It implements
simple, common functionality across implementation crates family.
used to simply generate all of the pads. It also implements simple, common
functionality across pads, like GPIO pin traits.

## Users

i.MX RT HAL designers, or advanced driver designers, want to treat pads as resources.
They want to create strongly-typed, infallible interfaces that ensure a pad supports
a capability. These users depend on the traits defined by `imxrt-iomuxc` to create
their driver interfaces. These drivers will accept pads across all i.MX RT chip variants.
These designers *do not* enable any `imxrt-iomuxc` feature flags.

i.MX RT HAL implementers want an API to specify their pads' capabilities. These users
create a device-specific crate, like `imxrt106x-iomuxc`, to implement pads. To simplify
pad implementation, these users use the build-time support crate. As we build support
for more i.MX RT processors, we can know that these pads will plug-and-play with driver
interfaces that accept pads.
When users are ready to run their code on hardware, they enable a feature in the
`imxrt-iomuxc` crate that describes their i.MX RT variant. Users who want to generalize
their code for different i.MX RT variants enable more feature flags.

## Comparison to Variant-Specific IOMUXC Crates

A previous approach separated the `imxrt-iomuxc` interface and implementations
across crates. Rather than having an `imxrt106x` feature in a single `imxrt-iomuxc`
crate, we had separate crates, named like `imxrt106x-iomuxc`, that implemented the
`imxrt-iomuxc` interfaces. We decided to use feature flags after realizing it was
not only easier to maintain, but also equivalent to the multi-crate approach. This
section compares the maintenance and equivalence of the two approaches.

### Easier to maintain

A single crate with feature flags is easier to maintain than an interface crate with
separate implementation crates:

- The interface crate does not need to support "public but internal" APIs. We signaled
these APIs behind `#[doc(hidden)]` attributes on public types. When using features,
these APIs are truly private. The approach reduces the documentation burden, since
we do not need to identify which APIs are truly public, and which APIs are internal.
- It's easier to release and version a single crate than multiple crates. We don't need
to plan an approach for releasing separate interface and implementation crates. Users
do not need to be concerned with our versioning and release strategy.
- It's easier to document and study. We may generate documentation for the interface
and all implementations simply using `cargo doc --all-features`. It's easier to link
documentation across the implementations and the interface.

### Equivalence

A single crate with feature flags is equivalent to an interface crate and separate
implementation crates. Consider a user who wants to use the IOMUXC pin configuration
interfaces. That user would depend on `imxrt-iomuxc`, regardless of the approach.

Now, consider a user who wants to use their code on an i.MX RT 106x processor variant.
Under the old approach, that user would include the `imxrt106x-iomuxc` crate, which
includes the `imxrt-iomuxc` crate:

```toml
[dependencies]
imxrt106x-iomuxc = "0.1"
# imxrt-iomuxc = "0.1" - implicit dependency
```

When using feature flags, the user enables the `imxrt106x` feature:

```toml
[dependencies]
imxrt106x-iomuxc = { version = "0.1", features = ["imxrt106x"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be imxrt-iomuxc

```

Both approaches result in the same changes to the dependency graph: the graph now includes code
for i.MX RT 106x processor pads.

Since features are additive, users who want to support more processors enable more feature flags.
This would have translated to the user explicitly including more crates:

```toml
[dependencies]
imxrt106x-iomuxc = { version = "0.1", features = ["imxrt102x", "imxrt106x"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is now imxrt-ioxmuc correct?


# Equivalent:

[dependencies]
imxrt102x-iomuxc = "0.1"
imxrt106x-iomuxc = "0.1"
# imxrt-iomuxc = "0.1" - implicit dependency
```

Depending on the release strategy, the user would need to maintain the version for all
implementation crates. The feature-flag approach requires a single version, which may
make it easier for the user.

### Discussion

Since the approaches are equivalent, the change has no effect on a split i.MX RT HAL. An
`imxrt-hal[-common]` crate would depend on the `imxrt-iomuxc` crate without feature flags.
Then, a processor-specific HAL would depend on `imxrt-iomuxc` with the appropriate feature
flag. We achive the goals of a split HAL, as users are unconcerned with feature flags.

We realize that this approach perpetuates the need for feature flags. However,
the `imxrt-iomuxc` crate is a much lower-level interface than the HAL crates; it's equivalent
to a RAL crate or PAC. The `imxrt-iomuxc` crate is intended for HAL developers, not HAL users.
HAL developers cannot escape feature flags, since using RALs and PACs already necessitates feature
flags. By adopting a single `imxrt-iomuxc` crate with feature flags, HAL developers continue to use
feature flags to support the i.MX RT variants.

## License

Expand Down
Loading