mux: port embassy-stm32 AFIO/AF unification pattern to ch32-hal#177
Open
andelf wants to merge 9 commits into
Open
mux: port embassy-stm32 AFIO/AF unification pattern to ch32-hal#177andelf wants to merge 9 commits into
andelf wants to merge 9 commits into
Conversation
First step in porting embassy-stm32's mux abstraction. cfg(afio) fires for any chip whose peripheral metadata has peripheral.remap.is_some() set (V1/V2/V3/X0/L1 families). H4 chips have no peripheral.remap entries since the H4 AFIO is the STM32F4-style per-pin 4-bit AF mux — they get cfg(not(afio)) and will dispatch to the per-pin AF code path once that's wired in. No behaviour change on its own — the cfg name is unused by the rest of the HAL yet. Following commits will gate the new pin_trait/new_pin macros on it. V307 example still builds clean (the only changes are warnings emitted by upstream).
cfg(afio)-gated marker types used by the new pin-trait shape. Mirrors the AfioRemap family in embassy-stm32::gpio. The markers are nominal const-generic structs with no trait — rustc enforces tx/rx pins of a single peripheral instance to agree on a single Remap<V> at compile time, replacing the runtime-validated const REMAP: u8 pattern. Unused until the pin_trait! macros land — the V307 example still builds clean (only pre-existing unused-import warnings).
Replaces the `<const REMAP: u8>` const-generic style on USART / SPI / I2C
/ CAN / TIM drivers with the embassy-stm32-style `<#[cfg(afio)] A>`
marker generic. Pins now carry their AFIO group as a nominal type
(`gpio::Remap<V>` / `gpio::RemapBool<V>` / `gpio::RemapNotApplicable`),
so a mismatched remap group between TX and RX of a single peripheral
becomes a compile-time E0277 instead of a silent AFIO PCFR misconfigure
at runtime.
Build.rs splits pin emission three ways:
- peripheral has `p.remap.is_some()` -> `pin_trait_afio_impl!` with
register / setter / `Remap<V>` or `RemapBool<V>` picked via
`is_bool_field()` against the AFIO IR.
- peripheral kind in {USART, SPI, I2C, CAN, TIM} but the chip has no
central remap entry (e.g. V3 USART1's split-bit RM is not yet
expressible) -> `pin_trait_impl!(..., RemapNotApplicable)`, deduped
per (signal, peri, pin) to avoid E0119 when the same pin shows up
in multiple remap groups in metadata.
- otherwise -> plain `pin_trait_impl!(..., af_num)`.
Driver call sites drop `T::set_remap(REMAP)` and instead call
`pin.afio_remap()` on each pin under `#[cfg(afio)]`. The
`SealedRemapPeripheral` / `RemapPeripheral` traits, their build.rs
emission, and the `patches.rs` manual impls are all retired.
USB (usbd / usbhs) and SDIO drop their unused `<T, 0>` literal.
H4 path (cfg(not(afio))) is wired through but inert -- pin trait emits
`af_num()`, and adding AF-mux support is a follow-up that needs an
`AfType` enum and gpio_v2-style `set_as_af(af_num, af_type)`.
Verified against ch32v003/v006/v103/v203/v208/v305/v307/x035/l103/641/643
examples (cargo check, 0 errors).
PWM and SPI examples need explicit marker turbofish in the new mux scheme: `Remap<N>` for multi-bit AFIO fields, `RemapBool<B>` for 1-bit ones. The compiler tells you which when inference is ambiguous (e.g. ch32v307 PA15 is TIM2_CH1 in both remap groups 1 and 3). Affected: - ch32v307 blinky_pwm: Remap<1> - ch32v103 pwm: Remap<0> - ch32v208 pwm_led: Remap<1> - ch32v006 pwm: Remap<0> (afio_v00x TIM1_RM is 4-bit) - ch32v003 pwm: Remap<0> (afio_v003 TIM1_RM is 2-bit) - ch32v003 spi-lcd: RemapBool<false> (afio_v003 SPI1_RM is 1-bit) - ch641 pwm: RemapBool<false> (afio_ch641 TIM1_RM is 1-bit) - ch643 pwm: Remap<0> (TIM1_RM is 3-bit)
The standard `cargo:rustc-cfg=<r.kind>` loop a few lines above already emits `cfg(afio)` whenever the chip has an AFIO peripheral. The extra `any(|p| p.remap.is_some())` emission added in prep commit c34cced is both redundant (V3 chips have an AFIO peripheral anyway) and wrong- intentioned for the future H4 path — H4 also has an AFIO peripheral, so once H4 chips enter the matrix they'll need a different discriminator than "has any p.remap". For this PR, `cfg(afio)` keeps its current meaning ("chip has an AFIO peripheral") which is sufficient for every family we currently support (V1/V2/V3/X0/L1/V0/641/643). H4 wiring will introduce its own gate (likely `cfg(not(afio_h4))` or similar) in a follow-up PR. Also corrects the V3 USART1 split-RM comment: the value is split across PCFR1 + PCFR2, not two bits of PCFR1.
`Remap` alone is ambiguous in WCH context — both V3 (central PCFR) and H4 (per-pin AFR) name registers/fields after "remap", and WCH HAL already uses the word for several unrelated things. Mirror embassy-stm32's `AfioRemap*` prefix verbatim so the type identifies the source of truth (AFIO peripheral) and aligns 1:1 with embassy. Touches src/gpio.rs (the marker definitions), src/macros.rs (doc comments), build.rs (emission idents), and the 8 example files that turbofish the marker explicitly. Structural note for future readers: the cfg(afio) gate maps to a GPIO-architecture difference. STM32 F4+ puts AF selection in the GPIO peripheral (GPIOx.AFRL/AFRH), so cfg(not(afio)) drivers reach AF through `pin.set_as_af(af_num, ...)` on the Pin trait. WCH H4 keeps the AF selectors inside the AFIO peripheral (AFIO.GPIOx_AFLR/AFHR), so the future H4 path will need to drive those writes from AFIO-rooted code even though it's still the "per-pin AF" branch semantically.
The post-rebase build.rs accumulated a HashSet<(String, String, String)>
keyed by token-stream `.to_string()` plus three multi-line block
comments. Slim it down to embassy's shape:
- dedup key is `(p.name, regs.kind, pin.pin)` — `&'static str`
triples from metadata, no TokenStream stringification.
- merge `type_and_values` into a single TokenStream literal
(`quote!(AfioRemap, [#v])` / `quote!(AfioRemapBool, [#b])`),
same shape as embassy's `pin_trait_afio_impl!` arg.
- replace the if/else-if/else cascade with a `match (&p.remap,
in_afio_list)` — the three valid branches plus an explicit
`_ => None` for "dup in NA list".
- drop the 18 lines of "what this is and why" comments that
embassy-stm32's equivalent block doesn't carry.
No behaviour change. `cargo check` still passes on all 11 example
families.
Mirrors embassy-stm32 gpio_v1's `AfType` struct and the `set_as_af` method on `SealedPin`. Drivers can now express any pin direction (input-with-pull, output push-pull, output open-drain) as a single `AfType` value built via `AfType::input(pull)` or `AfType::output(output_type, speed)`, and feed it through one `set_as_af()` call instead of juggling `set_as_af_output(AFType, Speed)` + `set_as_input(Pull)` plus a separate `afio_remap()` call. `Speed::to_mode()` is added as a const helper so `AfType::output` can stay `const fn`. The pre-existing `From<Speed> for vals::Mode` now delegates to it. Drops the previous `From<AFType> for vals::Cnf` direction-overloaded constants in favor of the explicit `AfType::output` constructor. The old `set_as_af_output`/`set_as_input` methods remain for backwards compatibility but new code should reach for `set_as_af` via the `new_pin!` / `set_as_af!` macros added in the next commit.
Adds the `new_pin!` and `set_as_af!` macros to macros.rs (mirroring
embassy-stm32 verbatim) and rewrites every driver `new*()` call site
to use them. This collapses the previous pattern
tx.set_as_af_output(AFType::OutputPushPull, Speed::High);
#[cfg(afio)]
tx.afio_remap();
// ...later...
Some(tx.into())
into a single
new_pin!(tx, AfType::output(OutputType::PushPull, Speed::High))
The 96 scattered `#[cfg(afio)] pin.afio_remap();` calls go away
because the macros own the cfg dispatch. Drivers are direction-
agnostic — input pins go through `new_pin!(pin, AfType::input(pull))`,
output pins through `AfType::output(...)`.
This also fixes a correctness bug: the CAN driver's `new_inner`
configured pins via raw `pin.set_mode_cnf(...)` calls, which the
earlier driver migration regex didn't match, so the
`#[cfg(afio)] pin.afio_remap()` injection silently skipped CAN. On V3
chips a user typing `Remap<2>` for CAN1 pins would compile but the
PCFR1.CAN1_RM bits would never get written — runtime CAN1 stayed on
the default group. Routing CAN through the same `set_as_af!` macro
restores the typesafe contract.
Driver-by-driver:
- USART: 14 sites
- SPI: 8 sites
- I2C: 1 site (with cfg(gpio_x0) open-drain fallback)
- CAN: 2 sites in `new_inner` (was the bug)
- timer simple_pwm + complementary_pwm: 1 macro each
USART's `new_half_duplex` / `new_blocking_half_duplex` now select
push-pull vs open-drain via a `#[cfg(not(gpio_x0))] let af = ...`
pair before the `new_pin!` call, matching the family-conditional
behaviour the original code had.
All 11 example families (ch32v003 ch32v006 ch32v103 ch32v203
ch32v208 ch32v305 ch32v307 ch32l103 ch32x035 ch641 ch643) build
clean with `cargo check`.
There was a problem hiding this comment.
Pull request overview
This PR refactors the HAL’s pin-trait/remap plumbing toward an embassy-stm32-style split between AFIO/PCFR central remap and per-pin AF mux, by introducing cfg(afio)-gated marker generics on pin traits and switching multiple drivers from const REMAP: u8 to the new marker-based approach.
Changes:
- Reworks
pin_trait!/pin_trait_impl!and adds AFIO-specific impl emission (pin_trait_afio_impl!) plusif_afio!for signature shaping. - Migrates USART/SPI/I2C/TIM PWM/CAN (and some USB/SDIO pin traits) away from
const REMAP: u8toward#[cfg(afio)] Amarkers andpin.afio_remap()calls. - Removes the
RemapPeripheralabstraction and deletespatches.rs, updating exports/imports and examples accordingly.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| build.rs | Adds AFIO-aware pin-trait impl generation and is_bool_field, but currently misses enabling cfg(afio) and has a TokenStream extension bug. |
| src/macros.rs | Redefines pin_trait! to be dual-shape under cfg(afio); adds pin_trait_afio_impl! and if_afio!. |
| src/gpio.rs | Adds Remap<V>, RemapBool<V>, RemapNotApplicable marker structs under cfg(afio). |
| src/peripheral.rs | Removes SealedRemapPeripheral / RemapPeripheral. |
| src/lib.rs | Stops re-exporting RemapPeripheral and removes patches module. |
| src/patches.rs | Deleted (previously contained per-peripheral remap shims/fakes). |
| src/usart/mod.rs | Migrates constructors to marker generic A and calls afio_remap(); contains duplicate remap calls in half-duplex constructors. |
| src/spi.rs | Migrates SPI constructors to marker generic A and per-pin afio_remap() calls. |
| src/i2c.rs | Migrates I2C constructors to marker generic A and per-pin afio_remap() calls. |
| src/timer/mod.rs | Removes RemapPeripheral bounds and updates timer pin traits to carry @A. |
| src/timer/simple_pwm.rs | Migrates PWM pin constructors to marker generic A and uses pin.afio_remap(). |
| src/timer/complementary_pwm.rs | Same migration as simple PWM for complementary outputs. |
| src/can/can.rs | Migrates CAN constructors to marker generic A but currently doesn’t apply AFIO remap at runtime. |
| src/usbd.rs | Updates USB DP/DM pin trait usage to the new trait shape (drops the old <T, 0> const generic). |
| src/usbhs/mod.rs | Same DP/DM signature update as usbd.rs. |
| src/sdio.rs | Drops old <T, 0> const generic pin trait usage in constructors. |
| examples/ch643/src/bin/pwm.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch641/src/bin/pwm.rs | Updates PWM example to use RemapBool<...> marker. |
| examples/ch32v307/src/bin/blinky_pwm.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch32v208/src/bin/pwm_led.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch32v103/src/bin/pwm.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch32v006/src/bin/pwm.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch32v003/src/bin/pwm.rs | Updates PWM example to use Remap<...> marker. |
| examples/ch32v003/src/bin/spi-lcd-st7735-cube.rs | Updates SPI example to use RemapBool<...> marker. |
Comments suppressed due to low confidence (1)
src/usart/mod.rs:999
- Same as the async half-duplex constructor:
tx.afio_remap()is present twice under#[cfg(afio)], so it will execute twice on non-gpio_x0chips (and twice ongpio_x0as well). Collapse this to a single call to avoid redundant AFIO PCFR writes and keep the cfg structure clearer.
#[cfg(not(gpio_x0))]
tx.set_as_af_output(AFType::OutputOpenDrain, Speed::High);
#[cfg(afio)]
tx.afio_remap();
#[cfg(gpio_x0)]
tx.set_as_af_output(AFType::OutputPushPull, Speed::High);
#[cfg(afio)]
tx.afio_remap();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
82
to
88
| } | ||
|
|
||
| println!("cargo:rustc-check-cfg=cfg(afio)"); | ||
|
|
||
| let mut gpio_lines = 16; | ||
| match &*chip_family { | ||
| "ch32v0" | "ch32m0" => { |
| }); | ||
|
|
||
| // panic!("{} {}", peri, pin_name); | ||
| g.extend(pin_trait_impl); |
Comment on lines
276
to
277
| // //here should remap functionality be added | ||
| // T::remap(0b10); |
Comment on lines
+902
to
+909
| #[cfg(not(gpio_x0))] | ||
| tx.set_as_af_output(AFType::OutputOpenDrain, Speed::High); | ||
| #[cfg(afio)] | ||
| tx.afio_remap(); | ||
| #[cfg(gpio_x0)] | ||
| tx.set_as_af_output(AFType::OutputPushPull, Speed::High); | ||
| T::set_remap(REMAP); | ||
| #[cfg(afio)] | ||
| tx.afio_remap(); |
Comment on lines
+231
to
+241
| impl<'d, T: Instance> UartTx<'d, T, Async> { | ||
| /// Useful if you only want Uart Tx. It saves 1 pin and consumes a little less power. | ||
| pub fn new<const REMAP: u8>( | ||
| pub fn new<#[cfg(afio)] A>( | ||
| peri: Peri<'d, T>, | ||
| tx: Peri<'d, impl TxPin<T, REMAP>>, | ||
| tx: Peri<'d, if_afio!(impl TxPin<T, A>)>, | ||
| tx_dma: Peri<'d, impl TxDma<T>>, | ||
| config: Config, | ||
| ) -> Result<Self, ConfigError> { | ||
| tx.set_as_af_output(AFType::OutputPushPull, Speed::High); | ||
| T::set_remap(REMAP); | ||
| #[cfg(afio)] | ||
| tx.afio_remap(); |
andelf
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Lay groundwork to unify two pin-mux mechanisms behind one HAL API, mirroring embassy-stm32's
cfg(afio)split:Today
Uart::new::<REMAP: u8>(...)works for the first via a manually-asserted const generic. H4 will arrive with no PCFR register and per-pin AF codes — the trait shape needs to differ. Following embassy-stm32 we'll cfg-gate theAmarker generic oncfg(afio), with build.rs picking whichpin_trait_impl!flavour to emit per chip.What this PR adds (preparation only, no behaviour change)
build.rsemitscargo:rustc-cfg=afiowheneverMETADATA.peripherals.iter().any(|p| p.remap.is_some())— i.e. the chip has at least one PCFR-style remappable peripheral. Also registers it withrustc-check-cfg. H4 chips will have allperipheral.remap = Nonesocfg(afio)stays off there.gpio.rsadds three marker structs (Remap<V: u8>,RemapBool<V: bool>,RemapNotApplicable) gated oncfg(afio). They'll be the type identities the pin trait carries to make rustc enforce tx/rx remap-group matching at compile time. Mirrors embassy-stm32'sAfioRemap/AfioRemapBool/AfioRemapNotApplicable.Neither change is referenced anywhere else yet. V307 / V103 / V003 examples still build clean against this branch.
What's next (out of scope for this PR)
The bulk of the migration in subsequent PR(s):
macros.rs: portpin_trait!to optionally accept@Remap, addif_afio!,new_pin!,pin_trait_afio_impl!(one impl per pin × remap value writing AFIO PCFR), makepin_trait_impl!cfg-gate overafio.build.rs: replace thepin_trait_impl!(.., remap_value)emission with the embassy-stm32-style branch —pin_trait_afio_impl!ifperipheral.remap.is_some()elsepin_trait_impl!(.., af_or_zero, RemapNotApplicable).usart/mod.rs+spi/mod.rs+i2c/mod.rs+can.rsdrivers: eachnew*function changes signature from<const REMAP: u8>to<#[cfg(afio)] A>, pin args wrapped inif_afio!(impl TxPin<T, A>), body replacesT::set_remap(REMAP)with the pin'safio_remap()(cfg-gated). User call sites stay unchanged:Uart::new(p.USART1, p.PA9, p.PA10, ...)works both before and after — the const generic disappears, the remap-group is inferred from pin types instead.peripheral.rs: retireRemapPeripheral/SealedRemapPeripheraltraits once nothing referencesT::set_remap().Reference for the port: embassy-stm32/src/{macros.rs,gpio.rs,build.rs} on commit https://github.com/embassy-rs/embassy/tree/main/embassy-stm32 — the structure transplants almost verbatim.
Why split
The macro / driver migration is invasive enough that landing it in one PR with the trivial cfg-emission and marker types makes review painful. This PR is the no-op base everything else rebases onto.
Test plan
cargo checkon existing V307 examples builds clean (only pre-existing unused-import warnings)cfg(not(afio))path