Optimize STM32 Ethernet Drvier

1. Add phy_linkchange function to simplify eth_phy_isr and phy_monitor_thread_entry
2. phy_speed is uint8_t may case some problems, use enumeration instead of hardware-dependent values
3. Fix eth_phy_isr can not detect link up
4. Use macro to check phy status for flexibility
Signed-off-by: Nxiaofan <xfan1024@live.com>

bsp/stm32/libraries/HAL_Drivers/drv_eth.c

@@ -7,7 +7,8 @@
  * Date           Author       Notes
  * 2018-11-19     SummerGift   first version
  * 2018-12-25     zylx         fix some bugs
- * 2019-06-10     SummerGift   optimize PHY state detection process 
+ * 2019-06-10     SummerGift   optimize PHY state detection process
+ * 2019-09-03     xiaofan      optimize link change detection process
  */
 
 #include "board.h"
@@ -390,47 +391,94 @@ void HAL_ETH_ErrorCallback(ETH_HandleTypeDef *heth)
     LOG_E("eth err");
 }
 
-#ifdef PHY_USING_INTERRUPT_MODE
-static void eth_phy_isr(void *args)
+enum {
+    PHY_LINK        = (1 << 0),
+    PHY_100M        = (1 << 1),
+    PHY_FULL_DUPLEX = (1 << 2),
+};
+
+static void phy_linkchange()
 {
-    rt_uint32_t status = 0;
-    static rt_uint8_t link_status = 1;
+    static rt_uint8_t phy_speed = 0;
+    rt_uint8_t phy_speed_new = 0;
+    rt_uint32_t status;
 
-    HAL_ETH_ReadPHYRegister(&EthHandle, PHY_INTERRUPT_FLAG_REG, (uint32_t *)&status);
-    LOG_D("phy interrupt status reg is 0x%X", status);
     HAL_ETH_ReadPHYRegister(&EthHandle, PHY_BASIC_STATUS_REG, (uint32_t *)&status);
     LOG_D("phy basic status reg is 0x%X", status);
 
-    if (status & PHY_LINKED_STATUS_MASK)
+    if (status & (PHY_AUTONEGO_COMPLETE_MASK | PHY_LINKED_STATUS_MASK))
     {
-        if (link_status == 0)
+        rt_uint32_t SR;
+
+        phy_speed_new |= PHY_LINK;
+
+        SR = HAL_ETH_ReadPHYRegister(&EthHandle, PHY_Status_REG, (uint32_t *)&SR);
+        LOG_D("phy control status reg is 0x%X", SR);
+
+        if (PHY_Status_SPEED_100M(SR))
+        {
+            phy_speed_new |= PHY_100M;
+        }
+
+        if (PHY_Status_FULL_DUPLEX(SR))
+        {
+            phy_speed_new |= PHY_FULL_DUPLEX;
+        }
+    }
+
+    if (phy_speed != phy_speed_new) {
+        phy_speed = phy_speed_new;
+        if (phy_speed & PHY_LINK)
         {
-            link_status = 1;
             LOG_D("link up");
+            if (phy_speed & PHY_100M)
+            {
+                LOG_D("100Mbps");
+                stm32_eth_device.ETH_Speed = ETH_SPEED_100M;
+            }
+            else
+            {
+                stm32_eth_device.ETH_Speed = ETH_SPEED_10M;
+                LOG_D("10Mbps");
+            }
+
+            if (phy_speed & PHY_FULL_DUPLEX)
+            {
+                LOG_D("full-duplex");
+                stm32_eth_device.ETH_Mode = ETH_MODE_FULLDUPLEX;
+            }
+            else
+            {
+                LOG_D("half-duplex");
+                stm32_eth_device.ETH_Mode = ETH_MODE_HALFDUPLEX;
+            }
+
             /* send link up. */
             eth_device_linkchange(&stm32_eth_device.parent, RT_TRUE);
         }
-    }
-    else
-    {
-        if (link_status == 1)
+        else
         {
-            link_status = 0;
             LOG_I("link down");
-            /* send link down. */
             eth_device_linkchange(&stm32_eth_device.parent, RT_FALSE);
         }
     }
 }
+
+#ifdef PHY_USING_INTERRUPT_MODE
+static void eth_phy_isr(void *args)
+{
+    rt_uint32_t status = 0;
+
+    HAL_ETH_ReadPHYRegister(&EthHandle, PHY_INTERRUPT_FLAG_REG, (uint32_t *)&status);
+    LOG_D("phy interrupt status reg is 0x%X", status);
+
+    phy_linkchange();
+}
 #endif /* PHY_USING_INTERRUPT_MODE */
 
-static uint8_t phy_speed = 0;
-#define PHY_LINK_MASK       (1<<0)
 static void phy_monitor_thread_entry(void *parameter)
 {
     uint8_t phy_addr = 0xFF;
-    uint8_t phy_speed_new = 0;
-    rt_uint32_t status = 0;
     uint8_t detected_count = 0;
 
     while(phy_addr == 0xFF)
@@ -468,102 +516,29 @@ static void phy_monitor_thread_entry(void *parameter)
 
     while (1)
     {
-        phy_speed_new = 0;
+        phy_linkchange();
 
-        if(HAL_ETH_ReadPHYRegister(&EthHandle, PHY_BASIC_STATUS_REG, (uint32_t *)&status) == HAL_OK)
-        {
-            LOG_D("PHY BASIC STATUS REG:0x%04X", status);
-            
-            if (status & (PHY_AUTONEGO_COMPLETE_MASK | PHY_LINKED_STATUS_MASK))
-            {
-                rt_uint32_t SR;
-
-                phy_speed_new = PHY_LINK_MASK;
-
-                if(HAL_ETH_ReadPHYRegister(&EthHandle, PHY_Status_REG, (uint32_t *)&SR) == HAL_OK)
-                {
-                    LOG_D("PHY Control/Status REG:0x%04X ", SR); 
-
-                    if (SR & PHY_100M_MASK)
-                    {
-                        phy_speed_new |= PHY_100M_MASK;
-                    }
-                    else if (SR & PHY_10M_MASK)
-                    {
-                        phy_speed_new |= PHY_10M_MASK;
-                    }
-
-                    if (SR & PHY_FULL_DUPLEX_MASK)
-                    {
-                        phy_speed_new |= PHY_FULL_DUPLEX_MASK;
-                    }
-                }
-                else
-                {
-                    LOG_D("PHY PHY_Status_REG read error."); 
-                    rt_thread_mdelay(100);
-                    continue;
-                }
-            }
-        }
-        else
+        if (stm32_eth_device.parent.netif->flags & NETIF_FLAG_LINK_UP)
         {
-            LOG_D("PHY_BASIC_STATUS_REG read error."); 
-            rt_thread_mdelay(100);
-            continue;
-        }
-
-        /* linkchange */
-        if (phy_speed_new != phy_speed)
-        {
-            if (phy_speed_new & PHY_LINK_MASK)
-            {
-                LOG_D("link up ");
-
-                if (phy_speed_new & PHY_100M_MASK)
-                {
-                    LOG_D("100Mbps");
-                    stm32_eth_device.ETH_Speed = ETH_SPEED_100M;
-                }
-                else
-                {
-                    stm32_eth_device.ETH_Speed = ETH_SPEED_10M;
-                    LOG_D("10Mbps");
-                }
-
-                if (phy_speed_new & PHY_FULL_DUPLEX_MASK)
-                {
-                    LOG_D("full-duplex");
-                    stm32_eth_device.ETH_Mode = ETH_MODE_FULLDUPLEX;
-                }
-                else
-                {
-                    LOG_D("half-duplex");
-                    stm32_eth_device.ETH_Mode = ETH_MODE_HALFDUPLEX;
-                }
-
-                /* send link up. */
-                eth_device_linkchange(&stm32_eth_device.parent, RT_TRUE);
-
 #ifdef PHY_USING_INTERRUPT_MODE
-                /* configuration intterrupt pin */
-                rt_pin_mode(PHY_INT_PIN, PIN_MODE_INPUT_PULLUP);
-                rt_pin_attach_irq(PHY_INT_PIN, PIN_IRQ_MODE_FALLING, eth_phy_isr, (void *)"callbackargs");
-                rt_pin_irq_enable(PHY_INT_PIN, PIN_IRQ_ENABLE);
-
-                /* enable phy interrupt */
-                HAL_ETH_WritePHYRegister(&EthHandle, PHY_INTERRUPT_MASK_REG, PHY_INT_MASK);
-
-                break;
+            /* configuration intterrupt pin */
+            rt_pin_mode(PHY_INT_PIN, PIN_MODE_INPUT_PULLUP);
+            rt_pin_attach_irq(PHY_INT_PIN, PIN_IRQ_MODE_FALLING, eth_phy_isr, (void *)"callbackargs");
+            rt_pin_irq_enable(PHY_INT_PIN, PIN_IRQ_ENABLE);
+
+            /* enable phy interrupt */
+            HAL_ETH_WritePHYRegister(&EthHandle, PHY_INTERRUPT_MASK_REG, PHY_INT_MASK);
+#if defined(PHY_INTERRUPT_CTRL_REG)
+            HAL_ETH_WritePHYRegister(&EthHandle, PHY_INTERRUPT_CTRL_REG, PHY_INTERRUPT_EN);
 #endif
-            } /* link up. */
-            else
-            {
-                LOG_I("link down");
-                /* send link down. */
-                eth_device_linkchange(&stm32_eth_device.parent, RT_FALSE);
-            }
-            phy_speed = phy_speed_new;
+            break;
+#endif
+        } /* link up. */
+        else
+        {
+            LOG_I("link down");
+            /* send link down. */
+            eth_device_linkchange(&stm32_eth_device.parent, RT_FALSE);
         }
 
         rt_thread_delay(RT_TICK_PER_SECOND);

bsp/stm32/libraries/HAL_Drivers/drv_eth.h

@@ -48,6 +48,9 @@
 #define PHY_10M_MASK                (1<<2)
 #define PHY_100M_MASK               (1<<3)
 #define PHY_FULL_DUPLEX_MASK        (1<<4)
+#define PHY_Status_SPEED_10M(sr)    ((sr) & PHY_10M_MASK)
+#define PHY_Status_SPEED_100M(sr)   ((sr) & PHY_100M_MASK)
+#define PHY_Status_FULL_DUPLEX(sr)  ((sr) & PHY_FULL_DUPLEX_MASK)
 #endif /* PHY_USING_LAN8720A */
 
 #ifdef PHY_USING_DM9161CEP
@@ -55,6 +58,9 @@
 #define PHY_10M_MASK                ((1<<12) || (1<<13))
 #define PHY_100M_MASK               ((1<<14) || (1<<15))
 #define PHY_FULL_DUPLEX_MASK        ((1<<15) || (1<<13))
+#define PHY_Status_SPEED_10M(sr)    ((sr) & PHY_10M_MASK)
+#define PHY_Status_SPEED_100M(sr)   ((sr) & PHY_100M_MASK)
+#define PHY_Status_FULL_DUPLEX(sr)  ((sr) & PHY_FULL_DUPLEX_MASK)
 /*  The PHY interrupt source flag register. */
 #define PHY_INTERRUPT_FLAG_REG      0x15U
 /*  The PHY interrupt mask register. */