-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
stm32/eth: Improve Ethernet driver with link detection and static IP support. #17613
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
base: master
Are you sure you want to change the base?
Conversation
|
Code size report: |
|
Thanks @andrewleech for this PR. I've done some test based on my use case around the use of This is the test result from the micropython version (branch master commit 35f15cf) that I am using: This is the test result from your PR:
|
|
Should this still be in draft? It's looking pretty complete from our testing! |
ports/stm32/eth_phy.h
Outdated
| #define PHY_SPEED_100FULL (6) | ||
| #define PHY_DUPLEX (4) | ||
|
|
||
| // PHY interrupt registers (common for LAN87xx and DP838xx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These constants aren't used anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes I'd initially thought I'd be able to use phy interrupts to trigger the link up/down events, however in many/most cases (including the H5 nucleo) the phy used shares a pin for both ref clock out and interrupt out aka if you're using the reference clock (like we are) you can't get interrupts from the phy.
ports/stm32/eth.c
Outdated
| __HAL_RCC_ETHRX_CLK_SLEEP_ENABLE(); | ||
| __HAL_RCC_ETH_CLK_SLEEP_DISABLE(); | ||
| __HAL_RCC_ETHTX_CLK_SLEEP_DISABLE(); | ||
| __HAL_RCC_ETHRX_CLK_SLEEP_DISABLE(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change really necessary? I would think that it's OK to just enable the clock sleep feature at the start, and not need to disable/enable it during reset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the ENABLE lines the way he was previously is actually what I found prevented .active(True) from working when the ethernet cable was connected (on the H5 at least). The reset just after this would timeout unless the cable was connected.
I'm not sure if explicitly disabling it here is necessary, though I thought it might be a good idea because I don't really know what state the system is in at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I see. Maybe it has to do with the WFI hiding in the mp_hal_delay_ms(2) (because the loop below that line is a busy loop and does not do WFI).
Well, probably it's OK to just disable clock-sleep altogether, because we do want ETH to run "in the background" when the CPU does WFI. All other peripherals do that (disable clock-sleep), eg USB.
| #if defined(STM32H5) | ||
| __HAL_RCC_ETH_RELEASE_RESET(); | ||
|
|
||
| __HAL_RCC_ETH_CLK_SLEEP_ENABLE(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll test again with
__HAL_RCC_ETH_CLK_SLEEP_DISABLE(); for these to be explicit, with comment why.
dd0aa4c to
8e1dbeb
Compare
CLK_SLEEP Configuration Investigation and FixDuring code review, we discovered that the restructure commit (90297bc) inadvertently removed all Investigation: CLK_SLEEP SemanticsThe macro naming is counter-intuitive:
From STM32H5 HAL documentation:
Why clocks must stay enabled: The ETH peripheral needs clocks during CPU sleep to receive packets and generate interrupts, which is necessary for DHCP and other network traffic. Fix AppliedRestored explicit // Release ETH peripheral from reset and enable clocks during CPU sleep.
// Note: CLK_SLEEP_ENABLE means clocks stay ON during sleep (not OFF).
// Clocks must continue during sleep to allow the ETH peripheral to receive
// packets and generate interrupts when the CPU enters sleep mode (WFI),
// which is necessary for DHCP and other network traffic.Testing on NUCLEO_H563ZITest 1: Interface activation without cable
Test 2: Hot-plug cable detection (2-hour monitoring)
Confirmed: ETH peripheral receiving packets and processing DHCP during CPU sleep as expected. No regressions from explicit CLK_SLEEP configuration. Commit: 095f4db |
Move mod_network_init() to run before boot.py execution, allowing network interfaces like network.LAN() to be instantiated in boot.py. The previous order caused failures when users tried to create network interfaces in boot.py because the network subsystem wasn't initialized until after boot.py completed. This change is safe because LWIP is already initialized earlier in main() and mod_network_init() only initializes the NIC list. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
907d820 to
ae87211
Compare
This restructures LWIP initialization and adds background PHY link status polling to support static IP configuration before active(True), eliminate timeouts without cable, and enable hot-plug detection. Background polling rationale: STM32 Nucleo boards do not wire PHY interrupt lines to the MCU, making polling the only method to detect cable unplug/replug events during runtime. Changes: - Split netif init into early and late phases for IP config before active() - Remove blocking PHY autonegotiation loops from initialization - Add background link status polling (needed due to lack of PHY interrupt) - Move PHY init to eth_start() for cleaner lifecycle - Optimize eth_link_status() with on-demand polling - Fix active() to return interface state vs link state Benefits: - Static IP can be configured before active(True) - active(True) succeeds immediately without cable (~28ms vs 10s timeout) - Hot-plug detection working (unplug/replug cable) - 56x faster interface activation (1586ms → 28ms) Tested on NUCLEO_H563ZI and NUCLEO_F429ZI. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Add dynamic MAC speed/duplex reconfiguration via autonegotiation polling and fix DHCP restart after cable hot-plug. MAC Speed/Duplex: - Enable non-blocking PHY autonegotiation at initialization - Poll for autoneg completion in background - Update MAC configuration to match negotiated PHY speed/duplex - Add 5-second autoneg timeout with fallback to 10Mbps Half Duplex - Protect MAC reconfiguration with IRQ disable/enable DHCP Hot-Plug: - Fix DHCP not restarting after cable replug (was stuck at 0.0.0.0) - Root cause: Called dhcp_renew() on stopped client (ineffective) - Solution: Use dhcp_stop() + dhcp_start() to restart DHCP - Preserve static IP during hot-plug (check dhcp->state != DHCP_STATE_OFF) Tested on NUCLEO_H563ZI and NUCLEO_F429ZI with cable hot-plug scenarios. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Replace HAL_NVIC_DisableIRQ with software flag to prevent 30ms IRQ blackout during MAC speed/duplex reconfiguration. Issue: During MAC reconfiguration, code disabled all ETH interrupts at NVIC level for 30ms (3x 10ms delays), blocking systick, USB, and other peripheral interrupts. Solution: - Add mac_reconfig_in_progress flag to eth_t struct - IRQ handler checks flag and returns early if set - Replace blind 10ms delays with DMA descriptor polling (max 100ms timeout) - IRQ handler can run but exits immediately via flag check Impact: IRQ blackout reduced from 30ms to 0ms (handler responsive). Tested on NUCLEO_H563ZI. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Per IEEE 802.3 spec, PHY_BSR link status bit latches low on link loss and requires two consecutive reads to get current state. Issue: After cable unplug→replug, first PHY_BSR read returns 0 (latched) even though physical link is restored, causing missed link-up events and unreliable hot-plug detection. Solution: Double-read PHY_BSR in all link status checks: - First read: Clears the latched-low state - Second read: Returns true current link status Locations: - eth_phy_link_status_poll(): Background polling (line 779) - eth_link_status(): Direct PHY read fallback (line 945) Impact: Reliable hot-plug detection after cable unplug/replug cycles. Tested on NUCLEO_H563ZI and NUCLEO_F429ZI. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Add inline comment clarifying that active() returns interface enabled state, not link/cable connection status. Context: Commit 90297bc changed active() semantics to support static IP configuration before active(True) and eliminate cable-wait timeouts. Previous behavior: - active() returned True only if cable physically connected Current behavior: - active() returns True if interface enabled (regardless of cable) - status() should be used to check physical link/cable state This is a breaking API change for code that relied on active() to check cable connection. Users should migrate to status() for link detection. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Add link status verification before MAC reconfiguration to prevent reconfiguring on dead link if cable unplugged between checks. Issue: eth_phy_link_status_poll() reads PHY_BSR at start, then enters MAC reconfiguration block. If cable unplugged between these two points, code attempts MAC speed/duplex reconfiguration on dead link. Scenario: 1. Line 779: PHY_BSR read shows link up 2. Lines 784-818: Link state change handling 3. Cable unplugged here 4. Line 820: MAC reconfiguration begins (link now down) Solution: Re-check last_link_status before MAC reconfiguration. Abort if link went down since initial check. Impact: Prevents wasted CPU cycles and transient errors from configuring MAC on non-existent link. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Replace dhcp_renew() with dhcp_stop() + dhcp_start() after MAC speed/ duplex reconfiguration to handle all DHCP states reliably. Issue: After MAC reconfiguration, code called dhcp_renew() to restart DHCP. However, dhcp_renew() only works when DHCP is in BOUND state. If MAC reconfiguration happens during DHCP negotiation (SELECTING, REQUESTING, etc.), dhcp_renew() fails silently. Solution: Use dhcp_stop() + dhcp_start() pattern (same as cable replug handling in lines 792-795). This works correctly regardless of current DHCP state. Impact: DHCP reliably restarts after MAC reconfiguration in all states, not just BOUND. Matches existing hot-plug behavior for consistency. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Print warning message when autonegotiation times out and code falls back to 10Mbps Half-Duplex mode. Issue: After 5-second autonegotiation timeout (PHY_AUTONEG_TIMEOUT_MS), code silently falls back to 10Mbps Half-Duplex - 10x slower than typical 100Mbps Full-Duplex. Users experience slow network with no indication of root cause. Solution: Print warning message to console when fallback occurs: "ETH: Autonegotiation timeout, using 10Mbps Half-Duplex" Impact: Users can diagnose slow network performance and investigate autonegotiation failures (bad cable, switch issues, etc.). Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Static IP configured before active(True) was being overwritten when MAC speed/duplex reconfiguration occurred (~2s after link up). The MAC reconfiguration code restarted DHCP whenever a DHCP struct existed, without checking if a static IP was configured. Created eth_dhcp_restart_if_needed() helper function that checks if IP is 0.0.0.0 before restarting DHCP. Consolidated 3 locations with duplicated DHCP restart logic to use this helper, reducing code size by 30 lines while fixing the bug. This ensures static IP is preserved through MAC reconfiguration, cable unplug/replug, and interface activation. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
…leep. The CLK_SLEEP_ENABLE calls were removed during the restructure (commit 90297bc), leaving the driver dependent on the HAL default state. While the default enables clocks during sleep, explicit configuration is more robust and documents the requirement. ETH clocks must remain enabled during CPU sleep (WFI) to allow the peripheral to receive packets and generate interrupts, which is necessary for DHCP and other network traffic. Tested on NUCLEO_F429ZI: DHCP acquisition successful. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
ae87211 to
5c2c306
Compare
Summary
This PR implements background link detection, hot-plug support, and multipleminor bug fixes for the STM32 Ethernet driver. The changes eliminate blocking timeouts, enable static IP configuration workflows, and fix reliability issues discovered during testing and code review.
Key Features:
LAN.active(True)with proper preservationCritical Fixes:
Breaking Change:
LAN.active()now returns interface enabled state instead of link status. UseLAN.status()orLAN.isconnected()to check cable connection.Testing
Hardware Tested:
Test Coverage - 11 Scenarios:
Basic Functionality:
active(True)without cable → Returns in 28ms, no timeoutactive()→ Returns current state via direct PHY readactive(True)succeeds immediatelyStatic IP Configuration:
5. Static IP before
active()→ IP preserved, DHCP not started6. Static IP after DHCP → Static IP replaces DHCP
7. Static IP through toggle → IP survives
active(False)/active(True)8. Static IP during hot-plug → IP preserved during cable unplug/replug
Hot-Plug Detection:
9. Boot without cable + plug-in → Link detected, DHCP starts immediately
10. Hot-unplug detection → Status 3→0 within 1 second
11. Hot-plug DHCP recovery → Status 0→3, DHCP completes in 2-4 seconds
Performance Improvements:
active(True)with cableactive(True)without cableBoard-Specific Issues Found & Fixed:
Detailed Changes
1. Network Initialization Order (1307179)
Problem:
network.LAN()couldn't be instantiated in boot.py due to initialization order.Solution: Moved
mod_network_init()before boot.py execution. Safe because LWIP initialization already occurs earlier; mod_network_init only sets up the NIC list.Impact: Enables network configuration in boot.py.
Files:
stm32/main.c,stm32/mpnetworkport.c2. Background Link Detection (90297bc)
Problem: No mechanism to detect cable unplug/replug events. Blocking PHY initialization caused multi-second delays.
Architectural Changes:
eth_phy_link_status_poll()called every ~250ms from background timernetif_set_link_up/down()Rationale for Polling: STM32 Nucleo boards don't wire PHY interrupt lines to MCU. Polling is the only method to detect cable state changes without hardware modifications.
Functional Changes:
active(True)returns immediately (~28ms) regardless of cable stateactive(True)active()now returns interface enabled state (not link status) - BREAKING CHANGEPerformance: 56× faster activation, 2× faster DHCP
Files:
stm32/eth.c(+262 lines),stm32/eth.h,stm32/network_lan.c,stm32/mpnetworkport.c3. MAC Speed/Duplex and DHCP Hot-Plug (e75650e)
Problem: MAC speed/duplex hardcoded at initialization. DHCP didn't restart after cable replug.
MAC Speed/Duplex:
DHCP Hot-Plug:
dhcp_stop()+dhcp_start()patternIssue Fixed: F429ZI regression where MAC/PHY speed mismatch prevented DHCP.
Files:
stm32/eth.c(+100 lines),stm32/eth_phy.hMigration Guide
Breaking Change:
active()MethodOld Behavior (v1.26.1 and earlier):
New Behavior (this PR):
Rationale: Separates interface administrative state from physical link state, consistent with other network stacks and allows
active(True)to succeed without blocking on cable presence.Example Usage
Static IP Before Activation
Boot.py Network Configuration
Hot-Plug Detection