stm32-rs / stm32-eth

Embedded Ethernet driver in Rust
Apache License 2.0
147 stars 47 forks source link

Adding support for STM32F107 #23

Closed mitchmindtree closed 2 years ago

mitchmindtree commented 4 years ago

This PR is largely based on @torkeldanielsson's work here and the related upstream forks.

Currently, this still depends on @torkeldanielsson's stm32f1xx-hal patch which I've rebased and fixed a couple of compiler errors here. Specifically, the new ip-f107.rs example uses the rcc.cfgr.freeze_explicit method, though admittedly I'm not familiar enough with how stm32 clocks work to know why this is required. @torkeldanielsson maybe you could share your thoughts on this?

As mentioned, a new ip-f107.rs example has been added that is a copy of the existing ip.rs example. It is currently working nicely locally, my aliexpress dev board with a DP83848 connected via messy jumper wires responds to pings in ~0.5ms. Ideally, the two ip examples would be consolidated and each of the device-specific parts should be feature gated.

Also, note that the memory.x has been modified for the STM32f107 as the existing seemed to be written specifically for an STM32F4 device.

I've also altered the target in .cargo/config. Not sure what the best approach is for setting this. Perhaps in the readme we should just ensure that users explicilty specify their --target when building examples?

torkeldanielsson commented 4 years ago

I created the rcc.cfgr.freeze_explicit because I needed to set more clocks than the regular freeze did, and the calculations for doing it the way freeze does looked really complex. In the ST Cube IDE there is a configurator that optimizes to arrive at clocks, and for my use case I wanted to use both I2S and Ethernet, so it ran for a while. The way freeze works, it would require to run those same calculations and output the result. I think it seemed more sane to not have that be the only clock config option, so that's the reasoning for the freeze_explicit - just let me set all the clocks the way I know I want them set up...

thalesfragoso commented 4 years ago

Thanks for the PR.

I don't think we need rcc.cfgr.freeze_explicit, we pretty much only care for the hclk and the F1 HAL already has a method to choose it.

Also, w.r.t rprintln, I don't think we should use it, since it will force the users to use rtt-target and init rprint before using this implementation. However, we do need logging, but I would like to use the log crate with some macros to add/remove it with a feature at compile time (like smoltcp and rubble), but this should be work for another PR.

torkeldanielsson commented 4 years ago

@mitchmindtree does this work if you use the regular freeze function?

mitchmindtree commented 4 years ago

I don't think we need rcc.cfgr.freeze_explicit, we pretty much only care for the hclk and the F1 HAL already has a method to choose it.

I can't seem to select a value for the hclk that satisfies the freeze method. Each combination I've tried appears to hang indefinitely before freeze returns or hits one of the unreachable!() branches.

Here are the resulting clocks determined with @torkeldanielsson's freeze_explicit method that appears to work:

16:10:01.308     hclk: 50000000
16:10:01.308     pclk1: 25000000
16:10:01.308     pclk2: 50000000
16:10:01.308     pclk1_tim: 50000000
16:10:01.308     pclk2_tim: 50000000
16:10:01.308     sysclk: 50000000
16:10:01.308     adcclk: 12500000
16:10:01.308     usbclk_valid: true

When attempting to use freeze on stm32f1xx-hal 0.6.1 with just the hclk set to 50.mhz() like so:

    let clocks = rcc.cfgr.hclk(50.mhz()).freeze(&mut flash.acr);

I get a panic:

16:18:47.866 panicked at 'internal error: entered unreachable code', /home/mindtree/.cargo/registry/src/github.com-1ecc6299db9ec823/stm32f1xx-hal-0.6.1/src/rcc.rs:241:22

I.e. this line which appears to indicate that the sysclk is less than the hclk. This make sense as the default value appears to be the HSI clock value 8_000_000 defined here. If I also set the sysclk to 50 mhz like so:

    let clocks = rcc.cfgr.sysclk(50.mhz()).hclk(50.mhz()).freeze(&mut flash.acr);

I appear to hit the same panic branch. This seems to imply that the sysclk gets constrained to something much lower as a result of the PLL multiplier calculations above which are a little over my head at the moment - perhaps something is going awry here? Specifying the pclk clocks don't appear to help, but they don't appear to be checked until later on in the function anyway.

Fwiw, I also tried making the changes you suggested here (without the bit-banding) but this doesn't appear to have had any affect on this clock freezing process at least.

Also, w.r.t rprintln, I don't think we should use it

Ah of course, my mistake, I had added it for debugging (I'm yet to try setting up semihosting at all) and I totally forgot to remove it!

Just a heads up - I'm a little busy to dig deeper into this clock stuff myself at the moment, however if either of you have something you'd like me to try I'll do so and report back next time I get the chance!

thalesfragoso commented 4 years ago

When attempting to use freeze on stm32f1xx-hal 0.6.1 with just the hclk set to 50.mhz() like so:

You won't be able to use 50MHz only with HSI, for connectivity line devices you can only go up to 36MHz with HSI. It's also good to check if you can achieve such clocks with the divisors and multipliers you have on the cfg.

The connectivity line devices also have a PLL2 and PLL3 clock, but it doesn't seem to be supported by the HAL, but I don't think we need it for ethernet.

What external crystal do you have ? If it's 8MHz this should work:

let clocks = rcc.cfgr.use_hse(8.mhz()).sysclk(72.mhz()).hclk(72.mhz()).freeze(&mut flash.acr);
mitchmindtree commented 4 years ago

Thanks for the advice!

I'm using a little aliexpress dev board for the STM32F107, it appears to have an 8MHz crystal on board.

When running this line it gets a bit further but panics at a pclk1 assertion:

18:48:07.809 panicked at 'assertion failed: pclk1 <= 36_000_000', /home/mindtree/.cargo/registry/src/github.com-1ecc6299db9ec823/stm32f1xx-hal-0.6.1/src/rcc.rs:277:9

After setting pclk1 to 36.mhz(), we appear to get through the clock configuration successfully!

That said, once we enter the poll loop, the device does not appear to respond to pings as it does when using freeze_explicit. I can see that the ETH interrupt gets called once per second if I print inside the callback, however ping shows 100% packet loss and I can't see any activity from the device on wireshark. Any ideas on why this might be the case?

thalesfragoso commented 4 years ago

That said, once we enter the poll loop, the device does not appear to respond to pings as it does when using freeze_explicit. I can see that the ETH interrupt gets called once per second if I print inside the callback, however ping shows 100% packet loss and I can't see any activity from the device on wireshark. Any ideas on why this might be the case?

Not sure, actually, if you were using the same config as the one in the ip-f107 example it shouldn't even work because that config is for a 25MHz external crystal, with a 8MHz one you would end up with a 16MHz hclk, which isn't even enough for using ethernet.

I would also double check your pin configuration with the one given by the manual at page 969, it seems that some of them don't match. One more thing, are you still using the branch from the HAL for these new tests ?

mitchmindtree commented 4 years ago

it shouldn't even work because that config is for a 25MHz external crystal, with a 8MHz one you would end up with a 16MHz hclk, which isn't even enough for using ethernet.

Interesting. Just to clarify, not only does it respond to pings (with this copy-pasted freeze_explicit config), I've actually got the ethernet bootloader working with it and the round-trip through my dodgy jumper wires is pretty consistently around ~0.5ms.

Just to clarify w.r.t. the external crystal, I'm referring to the 8MHz external crystal I can see mounted on this dev board near the stm32f107RC. The DP83848 PHY that I'm using has its own 50MHz crystal. Does this perhaps provide any clarity?

I would also double check your pin configuration with the one given by the manual at page 969, it seems that some of them don't match

Thanks for pointing this out. I originally had them configured as they are in this table, though for a reason I cannot remember now decided I needed some of the floating inputs to be open-drain, and someone mentioned you can read from Output<OpenDrain> so I tried that and it appeared to work so I went with it. After switching them back to the pin configurations described in this table, it appears to work just as well with freeze_explicit, however still no luck having the device respond to pings when using freeze.

One more thing, are you still using the branch from the HAL for these new tests ?

I have been switching back to the latest crates.io version of stm32f1xx-hal each time I test the freeze method.

cavokz commented 3 years ago

@mitchmindtree , I've an STM32F107 connected to a LAN8710A PHY. Is there any update I can try?

mitchmindtree commented 3 years ago

Is there any update I can try?

I can't speak to the LAN8710A specifically, however we're currently using this for an artwork and all seems to be working fine, though we have another patch on top of this that adds support for multiple PHYs (see my "stm32f1-support-multi-phys" branch) as we're using an Ethernet switch IC rather than a single-port PHY chip.

FWIW we're also using a custom branch of stm32f1xx-hal (see my "morph-patches" branch), however I believe some of those patches have since been merged into master, and I don't remember off the top of my head if any of those were necessary to get our Ethernet working properly.

I'd be happy to see someone finish this PR by addressing thales' comments, otherwise I'm planning to re-visit all my PRs and branches and attempt to land them after we have finished installation mid-Jan.

cavokz commented 3 years ago

I'd be happy to see someone finish this PR by addressing thales' comments, otherwise I'm planning to re-visit all my PRs and branches and attempt to land them after we have finished installation mid-Jan.

I'm not in hurry but I'd like to have it before Jan. I'll give it a try. Thanks!

rfael commented 3 years ago

@mitchmindtree I created pull request into your fork which updates smoltcp crate version and ip-f107 example. Case in example shows situation where Ethernet Physical Layer Transceiver (DP8384I) is clocked from STM32F107 MCO pin. I used it in my project and everything is working well.