Add STM32H7 Ethernet support with lwIP - #1373
Conversation
|
Note: the loopback HW tests are intermittently failing with a dribble error. It looks similar to the errata 2.26.20 but the CRC failure bit isn't set. Ignoring the error still gives the correct full packet sequence without any drops so I don't know what is going on. Need to investigate more. |
There was a problem hiding this comment.
I'm very impressed with the structure and cleanliness of this code. Everything is very well committed and easy to understand except for the STM32H7 driver itself. I tried to follow along with the RM, but got bored in the middle. So I will just try and reproduce this on my boards. I also have a custom STM32H5 board from work with the LAN8742A and I think the peripheral is the exact same.
Excellent work!
| modm_assert(result, "eth.tx.commit", "Acquired Ethernet transmit commit failed", | ||
| static_cast<uintptr_t>(result.error)); | ||
| MODM_LOG_INFO << "TX acquired sequence=" << sequence << modm::endl; | ||
| } |
There was a problem hiding this comment.
isn't modm_assert a bit heavy here? How severe are these failures? Does it warrant abandoning executing entirely?
There was a problem hiding this comment.
In a real app I would recommend handling these more gracefully. These calls would fail if the link went down, for example, which is probably not a good reason to crash. I figured it was easier to understand the logic flow if the example used asserts. Is the convention for examples that it should implement recommended error handling patterns instead?
|
Thank you! Is there any high level feedback or guidance you can give on the structure of the H7 driver I might be able to iterate on? I feel the same that it's very dense and the sequences are difficult to follow. There's a mix of different levels of detail (mixing high level re-usable sequences with bit twiddling regs) which I spent a while trying to rearrange. I'm thinking maybe splitting out the DMA descriptor management from MAC state/configuration. |
|
I think it's just fine, my criticism was more of the general complexity of this peripheral and the futility of trying to review it expertly, but that's to be expected. Tbh I would wait a little for refactorings until we can add/verify more families. This peripheral is Synopsis IP, so it'll likely show up in several STM32 families, either identically (H5 seems to have no changes at a glance) or with minor changes. Then we can still figure out what the commonalities are. |
|
Was able to reproduce nucleo_h753zi/ethernet with a (old) NUCLEO-H743ZI board. I did notice that just leaving the board running without Ethernet plugged in periodically abandons. Is this a bug? |
|
Both examples work on NUCLEO-H743ZI and NUCLEO-H723ZG. |
|
With small fixes the code also works as is on the NUCLEO-H563ZI ! |
|
I pushed some small fixups to enable Ethernet on NUCLEO-H563ZI, NUCLEO-H743ZI, NUCLEO-H723ZG. I would move the examples to the generic folder and add these boards to the xml like in the usb example. Then you can also put the Python code in the same folder. I didn't do this to not make your refactorings more annoying. |
|
Thank you for testing!
To make sure I understand -- at the start of that trace, Ethernet was connected, and then you disconnected it and left it disconnected? Which silicon revision is the chip? I am not seeing this on my Nucleo-H753. I left it for 10mins unplugged and tried plugging/unplugging and it seemed to handle the link state transitions OK. "Assertion 'eth.link' @ 0x00000002 (2) failed!" is a MacStopTimeout error (this is my own error type, not an ST error) and it lumps together a bunch of different possible failing checks. It means it detected the link state change from the PHY, began teardown of the MAC DMA, and got stuck somewhere. The Ethernet MAC is buggy as hell, but it's especially bad for revisions Y/W. Circled below are the relevant errata that can cause a hang when you do this procedure. In principle, all the recommended workarounds are implemented in this PR. There's a fair amount of complexity in the driver implementation (including 9 of 10 possible MacStopTimeout origins) attempting to work around these deadlock conditions during the stop sequence. 
My H753 is revision V, so doesn't have this errata. I have validated the sequence doesn't cause misbehavior in the "happy path" but have no way to validate that the errata workaround actually does what it's designed to do on the affected revisions. Your finding might be telling us that I have a bug in the errata workaround. There are two efficiency features I have in the driver which expose us to those errata. As a test we can try disabling them and see if the issue goes away.
I pushed some commits to test here: https://github.com/WasabiFan/modm/commits/h7-ethernet-timeout-debug/ . Try just fff3135 first and see what the logs say, then try adding the following ones. Tangentially, I am inclined to remove the flush from the stop sequence for real as well. The RM says it gives some perf improvement on stop and the ST implementation does do the flush but it seems like unnecessary complexity, I think I didn't get the perf benefit because I wait for MAC idle anyway, and stop perf is pretty meaningless. The "Assertion 'eth.phy.link' @ 0x00000002 (2) failed!" is definitely a bug somewhere. That's an MDIO timeout error from Phy::readLinkStatus. I am under the impression that the PHY should always be accessible via MDIO. Printing "link down" means it was able to read from the PHY at least once, but then it failed the next time. I don't see any relevant errata about this. I'd wonder whether the Ethernet clock was miscomputed on this board and is inadvertently glitching. I don't see anything about a minimum time between transactions that we could be violating. Evidently the PHY or the MAC's MDIO interface is in a bad state but I don't have a good guess about why. Can you check to confirm what frequency the Ethernet clocks are configured for on your board and what the CPU core clock is? Is it in VOS0 or VOS1 scaling mode? I see an errata mentioning VOS0 is broken on H743 revision Y and the max frequency is 400MHz as a result. I wouldn't expect that to exclusively affect Ethernet though. |
|
Currently the new driver is in Apparently the H5 series has some chips using the 4.20 revision (same as H7) and some chips using a 5.40 revision. At a glance I don't see any disruptive differences between those. It seems like it just adds optional features. |
|
Commits added up until now are fixups covering all the feedback except relocating the examples and lwip API to make it easier to review the incremental changes. (edited to add) Note that I also added some lwip hosted tests to CI and made a change to the assert module to try to fix a macOS build failure on an earlier push. |
Renamed to allow for modm::ethernet namespace to be used by interface types.
Microchip LAN87xx documentation requires: "Soft Reset: 1 = software reset. Bit is self-clearing. When setting this bit do not set other bits in this register."
The assertion struct fields were already aligned but the struct itself was given alignment=1. macOS linker errored because unaligned pointers might break relocation:
ld: warning: alignment (1) of atom 'lC12' (/Users/runner/work/modm/modm/build/generated-unittest/hosted/scons-release/modm/libmodm.a[39](single_thread_guard.o)) is too small and may result in unaligned pointers
ld: warning: alignment (1) of atom 'lC13' (/Users/runner/work/modm/modm/build/generated-unittest/hosted/scons-release/modm/libmodm.a[39](single_thread_guard.o)) is too small and may result in unaligned pointers
ld: pointer not aligned in 'lC13'+0x8 (/Users/runner/work/modm/modm/build/generated-unittest/hosted/scons-release/modm/libmodm.a[39](single_thread_guard.o))
collect2: error: ld returned 1 exit status
5afaa9b to
b739338
Compare
|
All feedback should be addressed. I rebased on latest develop but didn't autosquash so the fixups are all still there and can be reviewed incrementally. The code re-orgs are in new fixups next to the old ones. Open items:
|
|
Yes, correct I have rev Y of the STM32H743. It happens sporadically even after a reboot without connecting the cable. But connecting and disconnecting it definitely provokes it faster. Doesn't happen with the other two (H723ZG is rev Z, H563 is rev X). I switched to VOS1, since the BSP used VOS0, just in case. On fff3135: Disabling OSP didn't help, on b4b651e: Disabling stop flush also didn't help. On 28af3dc:
Yes, I think that's better! |
|
Haha ok, it looks like we've probably root-caused one bug and discovered another. I realize that the bare Ethernet example never handles received frames. It just transmits. The DMADSR value shown says the RX is stalled waiting for an available RX descriptor to save a received frame. Since the example never pops received packets, it can never get un-stuck if all the descriptors are full. I'll look into the stop sequence to figure out how to un-stick the peripheral and can add some RX prints in the example just to make it more complete. That doesn't explain why we received packets at all if the link was never connected. Additionally, the logs you show have repeated link up/down/up/down. Assuming that isn't you physically disconnecting and reconnecting the cable, it means we are somehow misinterpreting the PHY state, or the PHY is messed up. It's conceivable that the PHY being messed up is generating phantom packets which then also trigger the RX stop hang. I'll have to think about what the best debug step is. I could pass you some prints for all the PHY register reads at least. I'll also improve the example to print the MAC error counters and see if anything there gives us a clue. Edited to add: I re-tested on my H753 with Ethernet disconnected and didn't see the link state toggling. I'll see if I can provoke it into the same stalled RX state and perhaps that somehow is causing the PHY issue. |
|
On these newer runs did you eventually see the "eth.phy.link" abandon as well or only the "eth.link" abandon? |
|
Sorry I was using my Internet connection for this test and plugged and unplugged the cable repeatedly. |
|
Ahh I see, ok, then maybe the symptom we see there is purely the example not handling RX and your network has some traffic arriving at the board which mine doesn't. Doesn't explain the earlier PHY MDIO timeout though. I have local changes for the GMAC4 rename, fixing the stall if RX packets are pending, and some example/test improvements. Haven't gotten it fully cleaned up yet but should have some time tomorrow and will push the updates. |
|
Yes, good point, I didn't see the difference between At both b4b651e and 28af3dc (using VOS1), it still fails after a while. I added a number in the upper nibble of the |
2 participants
This PR adds STM32H7 Ethernet support. The approach taken here is:
LwipEthernetclass which glues lwIP to the MAC driver and PHY driver. The caller still directly invokes lwIP for sending/receiving data but this handles the initialization and periodic servicing. The servicing poll routines are intended to be called from a fiber.For the moment, I left the F4/F7 driver intact aside from some renames and a bugfix per reading the PHY datasheet. It's not tested/validated and no new functionality was added. I just got an F7 Nucleo so if I have some time I might implement the MAC driver layer required to use lwIP with the older STM32s. I also didn't implement support for FreeRTOS to use the H7 Ethernet MAC. So FreeRTOS only supports F4/F7 and lwIP only supports H7.
The PR is intended to be reviewed in individual commits and merged without squashing.
Notes:
All verification was done on an H753 Nucleo. I validated ~94Mbps TCP throughput bidirectional which I think is the right ceiling with framing and such, no packet drops or retransmissions over an extended run. The Ethernet fibers collectively used ~25% of the theoretical CPU time when doing a full bandwidth TCP echo (or 40% if using Newlib Nano, hence the new lbuild option). The error conditions were lightly tested to the extent convenient. Error conditions which are straightforward to emulate in unit/loopback tests do so, but some aren't validated. I haven't physically tested plugging into a link partner that is limited to 10Mbit so that mode is just covered by loopback tests.