Implement MII interface with feature flag

[sgasse]

The eth/v2 module of embassy-stm32 currently only supports RMII (reduced media independent interface) interfaces with 9 pins. However some chips only/better support a MII interface with 14 pins. With this PR, I want to propose adding support for MII interfaces to embassy.

Credits to figuring this out go to my colleague @KloolK, who sat down last year and implemented the necessary changes in a commit here for an older version of embassy.

This PR is meant as discussion basis. Introducing MII as a feature was the least noisy to show the necessary changes. However in the current setup, the features are not additive. Having separate structs for MII and RMII may be a better solution.

[Dirbaio]

very nice!

instead of using Cargo features, could you make it into separate new_mii, new_rmii constructors? This makes it consistent with how other drivers work, e.g. uart has new, new_with_rtscts etc, FMC has separate constructors for the amount of bits/banks.

[sgasse]

Thank you for the quick feedback @Dirbaio 🙂 I updated the patch according to your suggestions. I also tested that it still works with my STM32H745XI discovery board.

Assumptions that I made:

• Turning the individual pins into mutable references with let mut pin = pin.info_ref() behind the into_ref! macro has no side effects and can be moved after the block setting up the clocks.
• If we want to have the same driver struct in the code for MII and RMII without compile features, we need a const generic parameter because the two variants differ in the length of the pin array (and there is no heap to allocate that to...).
• If we want to have functions returning a struct with a generic parameter set to a specific value (but different between functions), they cannot be methods of the generic struct itself. Hence I moved new_mii and new_rmii out of impl Ethernet.

Questions where I would like to have your input:

• What is the rcc_h5 config about? I cannot find it in the Cargo.toml of embassy-stm32. There is one part of the code guarded by #[cfg(rcc_h5)], but for my board, it takes the `#[cfg(not(rcc_h5))] path so I cannot even test if the RMII setting is fine there.
• Does the eth/v1 module need adjustments if I add const NUM_PINS: usize to eth/v2/Ethernet? Can I just add the parameter even if unused? (the failing CI jobs look like it)
• In general, am I missing a common no_std way of initializing a struct with two lengths of an array member so that no generic parameter is needed?

[Dirbaio]

code looks good, can you fix CI?

What is the rcc_h5 config about? I cannot find it in the Cargo.toml of embassy-stm32. There is one part of the code guarded by #[cfg(rcc_h5)], but for my board, it takes the `#[cfg(not(rcc_h5))] path so I cannot even test if the RMII setting is fine there.

they come from here https://github.com/embassy-rs/embassy/blob/main/embassy-stm32/build.rs#L54

in particular rcc_h5 is active on stm32h5 chips.

[sgasse]

Thanks for the info @Dirbaio ! I'd like to fix the CI, but I have a hard time reproducing the issue locally. Which steps are necessary to run ci.sh locally? I could not find cargo batch with either cargo install cargo-batch, cargo search cargo-batch or on Google. Running the commands in isolation asks for a nightly compiler due to the out-dir parameter, so feel like there is too much difference between what CI does and how I attempt to reproduce it.

As far as I can tell, the issue in CI stems from the fact that I changed the public API of Ethernet::new only in v2 to the two functions Ethernet::new_rmii and Ehternet::new_mii. Unfortunately, I could not find a combination of cfg directives to make CI happy and without being able to reproduce it locally, I do not want to waste CI minutes by pushing again and again.

Here in the ethernet module, we conditionally include either the v1 or v2 version of the ethernet module implementation. In the test, I tried a few combinations and ultimately settled on this:

#[cfg(not(eth_v2))]
let device = Ethernet::new(
    ...
);

#[cfg(eth_v2)]
let device = Ethernet::new_rmii(
    ...
);

From the error in CI, it looks like the test code tries to use the v1 function Ethernet::new where the module only provides the v2 function Ethernet::new_rmii.:

error[E0599]: no function or associated item named `new` found for struct `embassy_stm32::eth::Ethernet` in the current scope
   --> /ci/code/tests/stm32/src/bin/eth.rs:75:28
    |
75  |     let device = Ethernet::new(
    |                            ^^^ function or associated item not found in `Ethernet<'_, _, _>`

...

error: could not compile `embassy-stm32-tests` (bin "eth") due to previous error
     target: Target(CompileTarget { name: "thumbv7em-none-eabi" })
     features: ["chrono", "eth", "rng", "stm32h563zi"]

I imagine the problem is that the flags passed to build the test binary are different from the flags passed to build the embassy-stm32 library. However I did not find any mention of eth_v2 in the build.rs, Cargo.toml or anywhere outside of the one usage in embassy-stm32/src/eth/mod.rs, thus I am not sure for which test case I should add them (if that is at all the right approach). Do you have a hint?

[Dirbaio]

Which steps are necessary to run ci.sh locally?

• cargo install --git https://github.com/embassy-rs/cargo-batch
• ./ci.sh

Unfortunately, I could not find a combination of cfg directives to make CI happy and without being able to reproduce it locally, I do not want to waste CI minutes by pushing again and again.

CI tells you at the end

error: could not compile `embassy-stm32-tests` (bin "eth") due to previous error
     target: Target(CompileTarget { name: "thumbv7em-none-eabi" })
     features: ["chrono", "eth", "rng", "stm32h563zi"]

so to reproduce

cd tests/stm32
cargo build --release --target thumbv7em-none-eabi --features stm32h563zi

[Dirbaio]

ah yes the issue is the eth_v2 cfgs are only available within the embassy-stm32 crate. so it'll always pick the not(eth_v2) one.

This reveals an inconsistency between v1 and v2 though! and consistency between versions matters. Maybe we should name v1 new_rmii too? or perhaps name the functions in v2 new and new_mii. since RMII seems way more widely used.

[Dirbaio]

or perhaps name the functions in v2 new and new_mii

ended doing this. Also fixed some fun issue with PC2_C pins in stm32-data...

thanks for the PR!