Skip to content

Commit 1539033

Browse files
pi-anlpi-anl
committed
stm32/eth: Optimize PHY lifecycle and status management.
This refactors PHY management for better efficiency and fixes isconnected() returning false with static IP configurations. Changes: - Move PHY init from eth_mac_init() to eth_start() - Add eth_phy_shutdown() called from eth_stop() - Optimize eth_link_status() to poll then use tracked state - Remove redundant interrupt-based PHY polling - Add 100ms PHY settling delay after initialization Benefits: - PHY only initialized when interface starts (not on every reset) - More efficient status checks (on-demand polling only) - Fixes isconnected() with static IP configurations - Cleaner PHY lifecycle management - Reduced interrupt overhead Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
1 parent cb3833e commit 1539033

File tree

3 files changed

+110
-115
lines changed

3 files changed

+110
-115
lines changed

‎ports/stm32/eth.c

Lines changed: 105 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ eth_t eth_instance;
129129

130130
static void eth_mac_deinit(eth_t *self);
131131
static void eth_process_frame(eth_t *self, size_t len, const uint8_t *buf);
132-
static void eth_netif_init_early(eth_t *self);
133-
static void eth_phy_link_status_poll(eth_t *self);
132+
static void eth_lwip_init(eth_t *self);
134133
static void eth_phy_configure_autoneg(eth_t *self);
134+
static int eth_phy_init(eth_t *self);
135135

136136
void eth_phy_write(uint32_t phy_addr, uint32_t reg, uint32_t val) {
137137
#if defined(STM32H5) || defined(STM32H7)
@@ -190,6 +190,10 @@ uint32_t eth_phy_read(uint32_t phy_addr, uint32_t reg) {
190190
}
191191

192192
int eth_init(eth_t *self, int mac_idx, uint32_t phy_addr, int phy_type) {
193+
if (self->netif.input != NULL) {
194+
// Already initialised.
195+
return 0;
196+
}
193197
mp_hal_get_mac(mac_idx, &self->netif.hwaddr[0]);
194198
self->netif.hwaddr_len = 6;
195199
self->phy_addr = phy_addr;
@@ -227,8 +231,8 @@ int eth_init(eth_t *self, int mac_idx, uint32_t phy_addr, int phy_type) {
227231
__HAL_RCC_ETH_CLK_ENABLE();
228232
#endif
229233

230-
// Initialize netif structure early so static IP can be configured before active(True)
231-
eth_netif_init_early(self);
234+
// Initialize netif and register with LWIP
235+
eth_lwip_init(self);
232236

233237
return 0;
234238
}
@@ -260,7 +264,6 @@ static int eth_mac_init(eth_t *self) {
260264
#if defined(STM32H5)
261265
__HAL_RCC_SBS_CLK_ENABLE();
262266
SBS->PMCR = (SBS->PMCR & ~SBS_PMCR_ETH_SEL_PHY_Msk) | SBS_PMCR_ETH_SEL_PHY_2;
263-
HAL_SBS_ETHInterfaceSelect(SBS_ETH_RMII);
264267
#elif defined(STM32H7)
265268
SYSCFG->PMCR = (SYSCFG->PMCR & ~SYSCFG_PMCR_EPIS_SEL_Msk) | SYSCFG_PMCR_EPIS_SEL_2;
266269
#else
@@ -349,33 +352,6 @@ static int eth_mac_init(eth_t *self) {
349352
ETH->DMACCR &= ~(ETH_DMACCR_DSL_Msk);
350353
#endif
351354

352-
// Reset the PHY and wait for reset to complete
353-
eth_phy_write(self->phy_addr, PHY_BCR, PHY_BCR_SOFT_RESET);
354-
mp_hal_delay_ms(50);
355-
356-
// Wait for PHY reset to complete (but don't wait for link)
357-
t0 = mp_hal_ticks_ms();
358-
while (eth_phy_read(self->phy_addr, PHY_BCR) & PHY_BCR_SOFT_RESET) {
359-
if (mp_hal_ticks_ms() - t0 > 1000) { // 1 second timeout for reset
360-
eth_mac_deinit(self);
361-
return -MP_ETIMEDOUT;
362-
}
363-
mp_hal_delay_ms(2);
364-
}
365-
366-
// Initialize link status tracking (current state, whatever it is)
367-
uint16_t bsr = eth_phy_read(self->phy_addr, PHY_BSR);
368-
self->last_link_status = (bsr & PHY_BSR_LINK_STATUS) != 0;
369-
370-
// Configure autonegotiation if link is already up, otherwise it will be
371-
// configured when link comes up via the polling function
372-
if (self->last_link_status) {
373-
eth_phy_configure_autoneg(self);
374-
}
375-
376-
// Enable PHY link change interrupts (for future use if PHY interrupt pin available)
377-
eth_phy_enable_link_interrupts(self->phy_addr);
378-
379355
// Burst mode configuration
380356
#if defined(STM32H5) || defined(STM32H7)
381357
ETH->DMASBMR = ETH->DMASBMR & ~ETH_DMASBMR_AAL & ~ETH_DMASBMR_FB;
@@ -689,14 +665,6 @@ void ETH_IRQHandler(void) {
689665
eth_dma_rx_free();
690666
}
691667
}
692-
693-
// Check for PHY link status changes (polled approach)
694-
// This runs on every RX interrupt, providing reasonable responsiveness
695-
static uint32_t link_check_counter = 0;
696-
if (++link_check_counter >= 100) { // Check every ~100 RX interrupts
697-
link_check_counter = 0;
698-
eth_phy_link_status_poll(&eth_instance);
699-
}
700668
}
701669

702670
/*******************************************************************************/
@@ -758,8 +726,9 @@ static err_t eth_netif_init(struct netif *netif) {
758726
return ERR_OK;
759727
}
760728

761-
static void eth_netif_init_early(eth_t *self) {
762-
// Initialize netif structure but don't add to network stack yet
729+
static void eth_lwip_init(eth_t *self) {
730+
MICROPY_PY_LWIP_ENTER
731+
// Initialize netif structure
763732
struct netif *n = &self->netif;
764733
n->name[0] = 'e';
765734
n->name[1] = '0';
@@ -772,48 +741,19 @@ static void eth_netif_init_early(eth_t *self) {
772741
IP_ADDR4(&ipconfig[2], 192, 168, 0, 1); // Gateway
773742
IP_ADDR4(&ipconfig[3], 8, 8, 8, 8); // DNS
774743

775-
netif_set_addr(n, ip_2_ip4(&ipconfig[0]), ip_2_ip4(&ipconfig[1]), ip_2_ip4(&ipconfig[2]));
744+
netif_add(n, ip_2_ip4(&ipconfig[0]), ip_2_ip4(&ipconfig[1]), ip_2_ip4(&ipconfig[2]), self, eth_netif_init, ethernet_input);
745+
netif_set_hostname(n, mod_network_hostname_data);
746+
747+
ip_addr_t dns_addr;
748+
IP_ADDR4(&dns_addr, 8, 8, 8, 8);
749+
dns_setserver(0, &dns_addr);
776750

777751
// Initialize DHCP structure
778752
dhcp_set_struct(n, &self->dhcp_struct);
779-
}
780-
781-
static void eth_lwip_init(eth_t *self) {
782-
MICROPY_PY_LWIP_ENTER
783-
784-
struct netif *n = &self->netif;
785-
786-
// Add netif to network stack (only if not already added)
787-
if (netif_find(n->name) == NULL) {
788-
ip_addr_t dns_addr;
789-
IP_ADDR4(&dns_addr, 8, 8, 8, 8);
790-
791-
netif_add(n, netif_ip4_addr(n), netif_ip4_netmask(n), netif_ip4_gw(n), self, eth_netif_init, ethernet_input);
792-
netif_set_hostname(n, mod_network_hostname_data);
793-
netif_set_default(n);
794-
netif_set_up(n);
795-
796-
dns_setserver(0, &dns_addr);
797-
798-
#if LWIP_IPV6
799-
netif_create_ip6_linklocal_address(n, 1);
800-
#endif
801-
}
802-
803-
MICROPY_PY_LWIP_EXIT
804-
}
805-
806-
static void eth_start_dhcp_if_needed(eth_t *self) {
807-
MICROPY_PY_LWIP_ENTER
808-
struct netif *netif = &self->netif;
809-
810-
// Check if a static IP address has been configured
811-
if (ip4_addr_isany_val(*netif_ip4_addr(netif))) {
812-
// No static IP configured, start DHCP
813-
dhcp_start(netif);
814-
}
815-
// If static IP is already configured, don't start DHCP
816753

754+
#if LWIP_IPV6
755+
netif_create_ip6_linklocal_address(n, 1);
756+
#endif
817757
MICROPY_PY_LWIP_EXIT
818758
}
819759

@@ -833,7 +773,11 @@ static void eth_phy_configure_autoneg(eth_t *self) {
833773
eth_phy_write(self->phy_addr, PHY_BCR, PHY_BCR_AUTONEG_EN);
834774
}
835775

836-
static void eth_phy_link_status_poll(eth_t *self) {
776+
void eth_phy_link_status_poll() {
777+
eth_t *self = &eth_instance;
778+
if (!self->enabled) {
779+
return;
780+
}
837781
// Poll PHY link status and handle state changes
838782
uint16_t bsr = eth_phy_read(self->phy_addr, PHY_BSR);
839783
bool current_link_status = (bsr & PHY_BSR_LINK_STATUS) != 0;
@@ -844,6 +788,7 @@ static void eth_phy_link_status_poll(eth_t *self) {
844788

845789
// Update LWIP netif link status to reflect physical cable connection
846790
struct netif *netif = &self->netif;
791+
MICROPY_PY_LWIP_ENTER
847792
if (current_link_status) {
848793
// Cable is physically connected
849794
netif_set_link_up(netif);
@@ -854,36 +799,24 @@ static void eth_phy_link_status_poll(eth_t *self) {
854799
eth_phy_configure_autoneg(self);
855800
}
856801

857-
// Restart DHCP if interface is up and DHCP is enabled
858-
if (netif_is_up(netif) && netif_dhcp_data(netif)) {
859-
dhcp_renew(netif);
802+
// Start or restart DHCP if interface is up
803+
if (netif_is_up(netif)) {
804+
if (netif_dhcp_data(netif)) {
805+
// DHCP already running, renew lease
806+
dhcp_renew(netif);
807+
} else if (ip4_addr_isany_val(*netif_ip4_addr(netif))) {
808+
// No static IP and no DHCP running, start DHCP
809+
dhcp_start(netif);
810+
}
860811
}
861812
} else {
862813
// Cable is physically disconnected
863814
netif_set_link_down(netif);
864815
}
816+
MICROPY_PY_LWIP_EXIT
865817
}
866818
}
867819

868-
static void eth_lwip_deinit(eth_t *self) {
869-
MICROPY_PY_LWIP_ENTER
870-
struct netif *netif = &self->netif;
871-
872-
// Stop DHCP if running
873-
if (netif_dhcp_data(netif)) {
874-
dhcp_stop(netif);
875-
}
876-
877-
// Remove from network stack but keep netif structure for reuse
878-
for (struct netif *n = netif_list; n != NULL; n = n->next) {
879-
if (n == netif) {
880-
netif_remove(n);
881-
break;
882-
}
883-
}
884-
MICROPY_PY_LWIP_EXIT
885-
}
886-
887820
static void eth_process_frame(eth_t *self, size_t len, const uint8_t *buf) {
888821
eth_trace(self, len, buf, NETUTILS_TRACE_NEWLINE);
889822

@@ -904,17 +837,21 @@ struct netif *eth_netif(eth_t *self) {
904837
}
905838

906839
int eth_link_status(eth_t *self) {
840+
// Do a quick poll to ensure link status is current
841+
eth_phy_link_status_poll();
842+
907843
struct netif *netif = &self->netif;
908844
if ((netif->flags & (NETIF_FLAG_UP | NETIF_FLAG_LINK_UP))
909845
== (NETIF_FLAG_UP | NETIF_FLAG_LINK_UP)) {
910-
if (netif->ip_addr.addr != 0) {
911-
return 3; // link up
846+
if (!ip4_addr_isany_val(*netif_ip4_addr(netif))) {
847+
return 3; // link up with IP
912848
} else {
913-
return 2; // link no-ip;
849+
return 2; // link up, no IP
914850
}
915851
} else {
916-
if (eth_phy_read(self->phy_addr, PHY_BSR) & PHY_BSR_LINK_STATUS) {
917-
return 1; // link up
852+
// Use tracked link status instead of direct PHY read
853+
if (self->last_link_status) {
854+
return 1; // physical link up but interface down
918855
} else {
919856
return 0; // link down
920857
}
@@ -934,21 +871,52 @@ int eth_start(eth_t *self) {
934871
return ret;
935872
}
936873

937-
// Initialize LWIP netif (only once, safe to call multiple times)
938-
eth_lwip_init(self);
874+
// Initialize PHY (reset and configure)
875+
ret = eth_phy_init(self);
876+
if (ret < 0) {
877+
eth_mac_deinit(self);
878+
return ret;
879+
}
880+
881+
MICROPY_PY_LWIP_ENTER
882+
struct netif *n = &self->netif;
939883

940-
// Start DHCP if no static IP has been configured
941-
eth_start_dhcp_if_needed(self);
884+
// Enable the interface in LWIP
885+
netif_set_default(n);
886+
netif_set_up(n);
942887

943-
// Do an initial link status poll
944-
eth_phy_link_status_poll(self);
888+
// Do an initial link status poll after PHY has had time to initialize
889+
eth_phy_link_status_poll();
890+
891+
// Start DHCP if no static IP has been configured and link is up
892+
if (ip4_addr_isany_val(*netif_ip4_addr(n))) {
893+
// No static IP configured, start DHCP if link is up
894+
if (n->flags & NETIF_FLAG_LINK_UP) {
895+
dhcp_start(n);
896+
}
897+
// Note: If link is down, DHCP will be started later when link comes up
898+
// via the dhcp_renew() call in eth_phy_link_status_poll()
899+
}
900+
901+
MICROPY_PY_LWIP_EXIT
945902

946903
self->enabled = true;
947904
return 0;
948905
}
949906

950907
int eth_stop(eth_t *self) {
951-
eth_lwip_deinit(self);
908+
// Stop DHCP if running
909+
if (netif_dhcp_data(&self->netif)) {
910+
dhcp_stop(&self->netif);
911+
}
912+
netif_set_down(&self->netif);
913+
914+
// Shutdown PHY
915+
eth_low_power_mode(self, true);
916+
917+
// Clear link status tracking
918+
self->last_link_status = false;
919+
952920
eth_mac_deinit(self);
953921
self->enabled = false;
954922
return 0;
@@ -979,4 +947,26 @@ void eth_low_power_mode(eth_t *self, bool enable) {
979947
eth_phy_write(self->phy_addr, PHY_BCR, bcr & (~PHY_BCR_POWER_DOWN));
980948
}
981949
}
950+
951+
static int eth_phy_init(eth_t *self) {
952+
// Reset the PHY and wait for reset to complete
953+
eth_phy_write(self->phy_addr, PHY_BCR, PHY_BCR_SOFT_RESET);
954+
mp_hal_delay_ms(10);
955+
956+
// Wait for PHY reset to complete (but don't wait for link)
957+
uint32_t t0 = mp_hal_ticks_ms();
958+
while (eth_phy_read(self->phy_addr, PHY_BCR) & PHY_BCR_SOFT_RESET) {
959+
if (mp_hal_ticks_ms() - t0 > 1000) { // 1 second timeout for reset
960+
return -MP_ETIMEDOUT;
961+
}
962+
mp_hal_delay_ms(2);
963+
}
964+
965+
// Initialize link status tracking (current state, whatever it is)
966+
self->last_link_status = false;
967+
eth_phy_link_status_poll();
968+
969+
return 0;
970+
}
971+
982972
#endif // defined(MICROPY_HW_ETH_MDC)

‎ports/stm32/eth.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,6 @@ bool eth_is_enabled(eth_t *self);
4444
int eth_start(eth_t *self);
4545
int eth_stop(eth_t *self);
4646
void eth_low_power_mode(eth_t *self, bool enable);
47+
void eth_phy_link_status_poll();
4748

4849
#endif // MICROPY_INCLUDED_STM32_ETH_H

‎ports/stm32/mpnetworkport.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ static void pyb_lwip_poll(void) {
7070
wiznet5k_poll();
7171
#endif
7272

73+
#if defined(MICROPY_HW_ETH_MDC)
74+
eth_phy_link_status_poll();
75+
#endif
76+
7377
// Run the lwIP internal updates
7478
sys_check_timeouts();
7579

0 commit comments

Comments
 (0)