diff --git a/arch/arm/src/common/stm32/Kconfig.eth b/arch/arm/src/common/stm32/Kconfig.eth index e90f665e08d1d..7a5359fa578e4 100644 --- a/arch/arm/src/common/stm32/Kconfig.eth +++ b/arch/arm/src/common/stm32/Kconfig.eth @@ -52,6 +52,27 @@ endif # !STM32_AUTONEG if STM32_AUTONEG +config STM32_AUTONEG_10FD_ONLY + bool "Advertise only 10BASE-T full duplex" + default n + ---help--- + Restrict the autonegotiation advertisement to 10BASE-T full + duplex, so that both ends negotiate that and nothing else. + + The reason to want this is electrical, not economic. 100BASE-TX + signalling sits at about 1 V with MLT-3 coding; 10BASE-T uses + 2.5 V Manchester at a tenth of the frequency, and shrugs off + interference that corrupts 100BASE-TX outright. On a board + where another subsystem couples noise into the PHY -- a parallel + RGB display sharing the board with the magnetics, say -- a + clean 10 Mbps link outperforms a 100 Mbps one that is losing + frames to bit errors. + + Restricting the advertisement is not the same as disabling + autonegotiation: a forced MCR leaves the partner to parallel + detection, which cannot detect duplex and will pick half -- + a genuine mismatch that stalls bulk traffic completely. + config STM32_PHYSR int "PHY Status Register Address (decimal)" ---help--- diff --git a/arch/arm/src/stm32h7/stm32_ethernet.c b/arch/arm/src/stm32h7/stm32_ethernet.c index 2ae195bf09bd5..291a87205f0d0 100644 --- a/arch/arm/src/stm32h7/stm32_ethernet.c +++ b/arch/arm/src/stm32h7/stm32_ethernet.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -1127,6 +1128,17 @@ static struct eth_desc_s *stm32_get_next_txdesc(struct stm32_ethmac_s *priv, * ****************************************************************************/ +static bool stm32_txringfull(FAR struct stm32_ethmac_s *priv) +{ + /* The ring is full when the head descriptor still belongs to the DMA. + * Transmitting into it anyway corrupts a frame in flight; with + * assertions built in it panics from the RX work queue instead. + */ + + return (priv->txhead->des3 & ETH_TDES3_RD_OWN) != 0 || + priv->txhead->des0 != 0; +} + static int stm32_transmit(struct stm32_ethmac_s *priv) { struct eth_desc_s *txdesc; @@ -1968,9 +1980,20 @@ static void stm32_receive(struct stm32_ethmac_s *priv) if (priv->dev.d_len > 0) { - /* And send the packet */ + /* Send the reply, unless the TX ring is full. The reply + * to received data is almost always an acknowledgement, and + * a peer that misses one retransmits; overwriting a frame + * the DMA still owns recovers from nothing. + */ - stm32_transmit(priv); + if (!stm32_txringfull(priv)) + { + stm32_transmit(priv); + } + else + { + priv->dev.d_len = 0; + } } } else @@ -1991,9 +2014,20 @@ static void stm32_receive(struct stm32_ethmac_s *priv) if (priv->dev.d_len > 0) { - /* And send the packet */ + /* Send the reply, unless the TX ring is full. The reply + * to received data is almost always an acknowledgement, and + * a peer that misses one retransmits; overwriting a frame + * the DMA still owns recovers from nothing. + */ - stm32_transmit(priv); + if (!stm32_txringfull(priv)) + { + stm32_transmit(priv); + } + else + { + priv->dev.d_len = 0; + } } } else @@ -2014,7 +2048,19 @@ static void stm32_receive(struct stm32_ethmac_s *priv) if (priv->dev.d_len > 0) { - stm32_transmit(priv); + /* Send the reply, unless the TX ring is full. A peer + * that misses an ARP reply asks again; overwriting a + * frame the DMA still owns recovers from nothing. + */ + + if (!stm32_txringfull(priv)) + { + stm32_transmit(priv); + } + else + { + priv->dev.d_len = 0; + } } } else @@ -3349,10 +3395,26 @@ static int stm32_phyinit(struct stm32_ethmac_s *priv) return -ETIMEDOUT; } - /* Enable auto-negotiation */ +#ifdef CONFIG_STM32_AUTONEG_10FD_ONLY + /* Advertise only 10BASE-T full duplex, so that negotiation lands + * there on both ends. See the help text of the option for why a + * board would want a slower link on purpose. + */ + + ret = mdio_write(priv->mdio, CONFIG_STM32_PHYADDR, MII_ADVERTISE, + MII_ADVERTISE_10BASETXFULL | MII_ADVERTISE_CSMA); + if (ret < 0) + { + nerr("ERROR: Failed to write the PHY ANAR: %d\n", ret); + return ret; + } +#endif + + /* Enable and restart auto-negotiation */ ret = mdio_write(priv->mdio, - CONFIG_STM32_PHYADDR, MII_MCR, MII_MCR_ANENABLE); + CONFIG_STM32_PHYADDR, MII_MCR, + MII_MCR_ANENABLE | MII_MCR_ANRESTART); if (ret < 0) { nerr("ERROR: Failed to enable auto-negotiation: %d\n", ret); @@ -3462,7 +3524,12 @@ static int stm32_phyinit(struct stm32_ethmac_s *priv) phyval |= MII_MCR_SPEED100; #endif - ret = stm32_phywrite(CONFIG_STM32_PHYADDR, MII_MCR, phyval, 0xffff); + /* mdio_write, not stm32_phywrite: this driver never had the latter. + * The call was carried over from the F7 driver and nothing had ever + * built this path. + */ + + ret = mdio_write(priv->mdio, CONFIG_STM32_PHYADDR, MII_MCR, phyval); if (ret < 0) { nerr("ERROR: Failed to write the PHY MCR: %d\n", ret); @@ -3481,9 +3548,14 @@ static int stm32_phyinit(struct stm32_ethmac_s *priv) #endif #endif - ninfo("Duplex: %s Speed: %d MBps\n", - priv->fduplex ? "FULL" : "HALF", - priv->mbps100 ? 100 : 10); + /* Diagnostic: say what was negotiated even without net debug. A + * duplex mismatch looks exactly like a bad cable and nothing else + * says which of the two it is. + */ + + syslog(LOG_INFO, "stm32_eth: link %s-duplex %d Mbps\n", + priv->fduplex ? "full" : "half", + priv->mbps100 ? 100 : 10); return OK; }