ethernet: fix infinite loop when init phy or reset mac

1. fix infinite loop problem when init phy device
2. fix infinite loop problem when reset mac
3. fix little bugs in ethernetif_init
4. fix incompatible return value between lwip and esp-idf

Closes https://github.com/espressif/esp-idf/issues/2331
Closes https://github.com/espressif/esp-idf/issues/2141

components/ethernet/emac_common.h

@@ -77,6 +77,7 @@ struct emac_config_data {
     bool emac_flow_ctrl_partner_support;
     eth_phy_get_partner_pause_enable_func  emac_phy_get_partner_pause_enable;
     eth_phy_power_enable_func  emac_phy_power_enable;
+    uint32_t reset_timeout_ms;
 };
 
 enum emac_post_type {

components/ethernet/emac_dev.c

@@ -14,7 +14,8 @@
 
 #include <stdio.h>
 #include <string.h>
-
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
 #include "rom/ets_sys.h"
 #include "rom/gpio.h"
 
@@ -32,8 +33,6 @@
 
 #include "emac_common.h"
 
-static const char *TAG = "emac";
-
 void emac_enable_flowctrl(void)
 {
     REG_SET_BIT(EMAC_GMACFC_REG, EMAC_TFCE);
@@ -72,18 +71,6 @@ void emac_disable_dma_rx(void)
     REG_CLR_BIT(EMAC_DMAOPERATION_MODE_REG, EMAC_START_STOP_RX);
 }
 
-void emac_reset(void)
-{
-    REG_SET_BIT(EMAC_DMABUSMODE_REG, EMAC_SW_RST);
-
-    while (REG_GET_BIT(EMAC_DMABUSMODE_REG, EMAC_SW_RST) == 1) {
-        //nothing to do ,if stop here,maybe emac have not clk input.
-        ESP_LOGI(TAG, "emac resetting ....");
-    }
-
-    ESP_LOGI(TAG, "emac reset done");
-}
-
 void emac_enable_clk(bool enable)
 {
     if (enable == true) {

components/ethernet/emac_dev.h

@@ -40,7 +40,7 @@ typedef struct dma_extended_desc {
 }dma_extended_desc_t;
 
 void emac_enable_clk(bool enable);
-void emac_reset(void);
+esp_err_t emac_reset(void);
 void emac_set_gpio_pin_rmii(void);
 void emac_set_gpio_pin_mii(void);
 uint32_t emac_read_mac_version(void);

components/ethernet/emac_main.c

@@ -101,6 +101,11 @@ esp_err_t esp_eth_set_mac(const uint8_t mac[6])
     }
 }
 
+eth_speed_mode_t esp_eth_get_speed(void)
+{
+    return emac_config.emac_phy_get_speed_mode();
+}
+
 static void emac_setup_tx_desc(struct dma_extended_desc *tx_desc, uint32_t size)
 {
     tx_desc->basic.desc1 = size & 0xfff;
@@ -274,6 +279,33 @@ esp_err_t esp_eth_smi_wait_value(uint32_t reg_num, uint16_t value,
     return ESP_ERR_TIMEOUT;
 }
 
+esp_err_t emac_reset(void)
+{
+    REG_SET_BIT(EMAC_DMABUSMODE_REG, EMAC_SW_RST);
+    if (emac_config.reset_timeout_ms) {
+        int start = xTaskGetTickCount();
+        uint32_t timeout_ticks = (emac_config.reset_timeout_ms + portTICK_PERIOD_MS - 1) / portTICK_PERIOD_MS;
+        while (timeout_ticks == 0 || (xTaskGetTickCount() - start < timeout_ticks)) {
+            if (REG_GET_BIT(EMAC_DMABUSMODE_REG, EMAC_SW_RST) != EMAC_SW_RST) {
+                goto reset_ok;
+            }
+            vTaskDelay(1);
+        }
+        ESP_LOGE(TAG, "Reset EMAC Timeout");
+        return ESP_ERR_TIMEOUT;
+    }
+    /* infinite wait loop */
+    else {
+        while (REG_GET_BIT(EMAC_DMABUSMODE_REG, EMAC_SW_RST) == EMAC_SW_RST) {
+            //nothing to do ,if stop here,maybe emac have not clk input.
+            ESP_LOGI(TAG, "emac resetting ....");
+        }
+    }
+reset_ok:
+    ESP_LOGI(TAG, "emac reset done");
+    return ESP_OK;
+}
+
 static void emac_set_user_config_data(eth_config_t *config)
 {
     emac_config.phy_addr = config->phy_addr;
@@ -286,6 +318,7 @@ static void emac_set_user_config_data(eth_config_t *config)
     emac_config.emac_phy_check_init = config->phy_check_init;
     emac_config.emac_phy_get_speed_mode = config->phy_get_speed_mode;
     emac_config.emac_phy_get_duplex_mode = config->phy_get_duplex_mode;
+    emac_config.reset_timeout_ms = config->reset_timeout_ms;
 #if DMA_RX_BUF_NUM > 9
     emac_config.emac_flow_ctrl_enable = config->flow_ctrl_enable;
 #else
@@ -710,14 +743,14 @@ esp_err_t esp_eth_tx(uint8_t *buf, uint16_t size)
     if (emac_config.emac_status != EMAC_RUNTIME_START) {
         ESP_LOGE(TAG, "tx netif is not ready, emac_status=%d",
                  emac_config.emac_status);
-        ret = ERR_IF;
+        ret = ESP_ERR_INVALID_STATE;
         return ret;
     }
 
     xSemaphoreTakeRecursive(emac_tx_xMutex, portMAX_DELAY);
     if (emac_config.cnt_tx == DMA_TX_BUF_NUM - 1) {
         ESP_LOGD(TAG, "tx buf full");
-        ret = ERR_MEM;
+        ret = ESP_ERR_NO_MEM;
         goto _exit;
     }
 
@@ -812,7 +845,10 @@ static void emac_start(void *param)
     cmd->err = EMAC_CMD_OK;
     emac_enable_clk(true);
 
-    emac_reset();
+    if (emac_reset() != ESP_OK) {
+        return;
+    }
+    emac_reset_dma_chain();
     emac_dma_init();
 
     emac_set_macaddr_reg();
@@ -822,8 +858,6 @@ static void emac_start(void *param)
 
     emac_mac_init();
 
-    emac_config.phy_init();
-
     //ptp TODO
 
     emac_enable_intr();
@@ -872,6 +906,12 @@ esp_err_t esp_eth_enable(void)
     esp_pm_lock_acquire(s_pm_lock);
 #endif //CONFIG_PM_ENABLE
 
+    /* init phy device */
+    if (emac_config.phy_init() != ESP_OK) {
+        ESP_LOGE(TAG, "Initialise PHY device Timeout");
+        return ESP_FAIL;
+    }
+
     if (emac_config.emac_status != EMAC_RUNTIME_NOT_INIT) {
         if (emac_ioctl(SIG_EMAC_START, (emac_par_t)(&post_cmd)) != 0) {
             open_cmd.err = EMAC_CMD_FAIL;
@@ -903,8 +943,6 @@ static void emac_stop(void *param)
     emac_process_link_updown(false);
 
     emac_disable_intr();
-    emac_reset_dma_chain();
-    emac_reset();
     emac_enable_clk(false);
 
     emac_config.emac_status = EMAC_RUNTIME_STOP;
@@ -1156,7 +1194,6 @@ esp_err_t esp_eth_init_internal(eth_config_t *config)
     xTaskCreate(emac_task, "emacT", EMAC_TASK_STACK_SIZE, NULL,
                 EMAC_TASK_PRIORITY, &emac_task_hdl);
 
-    emac_enable_clk(false);
     esp_intr_alloc(ETS_ETH_MAC_INTR_SOURCE, 0, emac_process_intr, NULL, NULL);
 
     emac_config.emac_status = EMAC_RUNTIME_INIT;

components/ethernet/eth_phy/phy_lan8720.c

@@ -59,7 +59,7 @@ void phy_lan8720_check_phy_init(void)
 
 eth_speed_mode_t phy_lan8720_get_speed_mode(void)
 {
-    if(esp_eth_smi_read(PHY_SPECIAL_CONTROL_STATUS_REG) & SPEED_INDICATION_100T) {
+    if (esp_eth_smi_read(PHY_SPECIAL_CONTROL_STATUS_REG) & SPEED_INDICATION_100T) {
         ESP_LOGD(TAG, "phy_lan8720_get_speed_mode(100)");
         return ETH_SPEED_MODE_100M;
     } else {
@@ -70,7 +70,7 @@ eth_speed_mode_t phy_lan8720_get_speed_mode(void)
 
 eth_duplex_mode_t phy_lan8720_get_duplex_mode(void)
 {
-    if(esp_eth_smi_read(PHY_SPECIAL_CONTROL_STATUS_REG) & DUPLEX_INDICATION_FULL) {
+    if (esp_eth_smi_read(PHY_SPECIAL_CONTROL_STATUS_REG) & DUPLEX_INDICATION_FULL) {
         ESP_LOGD(TAG, "phy_lan8720_get_duplex_mode(FULL)");
         return ETH_MODE_FULLDUPLEX;
     } else {
@@ -88,7 +88,7 @@ void phy_lan8720_power_enable(bool enable)
     }
 }
 
-void phy_lan8720_init(void)
+esp_err_t phy_lan8720_init(void)
 {
     ESP_LOGD(TAG, "phy_lan8720_init()");
     phy_lan8720_dump_registers();
@@ -96,20 +96,23 @@ void phy_lan8720_init(void)
     esp_eth_smi_write(MII_BASIC_MODE_CONTROL_REG, MII_SOFTWARE_RESET);
 
     esp_err_t res1, res2;
-    do {
-        // Call esp_eth_smi_wait_value() with a timeout so it prints an error periodically
-        res1 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_1_REG, LAN8720_PHY_ID1, UINT16_MAX, 1000);
-        res2 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_2_REG, LAN8720_PHY_ID2, LAN8720_PHY_ID2_MASK, 1000);
-    } while(res1 != ESP_OK || res2 != ESP_OK);
+    // Call esp_eth_smi_wait_value() with a timeout so it prints an error periodically
+    res1 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_1_REG, LAN8720_PHY_ID1, UINT16_MAX, 1000);
+    res2 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_2_REG, LAN8720_PHY_ID2, LAN8720_PHY_ID2_MASK, 1000);
 
     esp_eth_smi_write(SW_STRAP_CONTROL_REG,
                       DEFAULT_STRAP_CONFIG | SW_STRAP_CONFIG_DONE);
 
-
     ets_delay_us(300);
 
     // TODO: only enable if config.flow_ctrl_enable == true
     phy_mii_enable_flow_ctrl();
+
+    if (res1 == ESP_OK && res2 == ESP_OK) {
+        return ESP_OK;
+    } else {
+        return ESP_ERR_TIMEOUT;
+    }
 }
 
 const eth_config_t phy_lan8720_default_ethernet_config = {
@@ -128,6 +131,7 @@ const eth_config_t phy_lan8720_default_ethernet_config = {
     .phy_get_speed_mode = phy_lan8720_get_speed_mode,
     .phy_get_duplex_mode = phy_lan8720_get_duplex_mode,
     .phy_get_partner_pause_enable = phy_mii_get_partner_pause_enable,
+    .reset_timeout_ms = 1000
 };
 
 void phy_lan8720_dump_registers()

components/ethernet/eth_phy/phy_tlk110.c

@@ -64,7 +64,7 @@ void phy_tlk110_check_phy_init(void)
 
 eth_speed_mode_t phy_tlk110_get_speed_mode(void)
 {
-    if((esp_eth_smi_read(PHY_STATUS_REG) & SPEED_STATUS ) != SPEED_STATUS) {
+    if ((esp_eth_smi_read(PHY_STATUS_REG) & SPEED_STATUS ) != SPEED_STATUS) {
         ESP_LOGD(TAG, "phy_tlk110_get_speed_mode(100)");
         return ETH_SPEED_MODE_100M;
     } else {
@@ -75,7 +75,7 @@ eth_speed_mode_t phy_tlk110_get_speed_mode(void)
 
 eth_duplex_mode_t phy_tlk110_get_duplex_mode(void)
 {
-    if((esp_eth_smi_read(PHY_STATUS_REG) & DUPLEX_STATUS ) == DUPLEX_STATUS) {
+    if ((esp_eth_smi_read(PHY_STATUS_REG) & DUPLEX_STATUS ) == DUPLEX_STATUS) {
         ESP_LOGD(TAG, "phy_tlk110_get_duplex_mode(FULL)");
         return ETH_MODE_FULLDUPLEX;
     } else {
@@ -94,7 +94,7 @@ void phy_tlk110_power_enable(bool enable)
     }
 }
 
-void phy_tlk110_init(void)
+esp_err_t phy_tlk110_init(void)
 {
     ESP_LOGD(TAG, "phy_tlk110_init()");
     phy_tlk110_dump_registers();
@@ -102,11 +102,9 @@ void phy_tlk110_init(void)
     esp_eth_smi_write(PHY_RESET_CONTROL_REG, SOFTWARE_RESET);
 
     esp_err_t res1, res2;
-    do {
-        // Call esp_eth_smi_wait_value() with a timeout so it prints an error periodically
-        res1 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_1_REG, TLK110_PHY_ID1, UINT16_MAX, 1000);
-        res2 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_2_REG, TLK110_PHY_ID2, TLK110_PHY_ID2_MASK, 1000);
-    } while(res1 != ESP_OK || res2 != ESP_OK);
+    // Call esp_eth_smi_wait_value() with a timeout so it prints an error periodically
+    res1 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_1_REG, TLK110_PHY_ID1, UINT16_MAX, 1000);
+    res2 = esp_eth_smi_wait_value(MII_PHY_IDENTIFIER_2_REG, TLK110_PHY_ID2, TLK110_PHY_ID2_MASK, 1000);
 
     esp_eth_smi_write(SW_STRAP_CONTROL_REG,
                       DEFAULT_STRAP_CONFIG | SW_STRAP_CONFIG_DONE);
@@ -115,6 +113,12 @@ void phy_tlk110_init(void)
 
     // TODO: only do this if config.flow_ctrl_enable == true
     phy_mii_enable_flow_ctrl();
+
+    if (res1 == ESP_OK && res2 == ESP_OK) {
+        return ESP_OK;
+    } else {
+        return ESP_ERR_TIMEOUT;
+    }
 }
 
 const eth_config_t phy_tlk110_default_ethernet_config = {
@@ -132,6 +136,7 @@ const eth_config_t phy_tlk110_default_ethernet_config = {
     .phy_get_duplex_mode = phy_tlk110_get_duplex_mode,
     .phy_get_partner_pause_enable = phy_mii_get_partner_pause_enable,
     .phy_power_enable = phy_tlk110_power_enable,
+    .reset_timeout_ms = 1000
 };
 
 void phy_tlk110_dump_registers()

components/ethernet/include/esp_eth.h

@@ -83,7 +83,7 @@ typedef bool (*eth_phy_check_link_func)(void);
 typedef void (*eth_phy_check_init_func)(void);
 typedef eth_speed_mode_t (*eth_phy_get_speed_mode_func)(void);
 typedef eth_duplex_mode_t (*eth_phy_get_duplex_mode_func)(void);
-typedef void (*eth_phy_func)(void);
+typedef esp_err_t (*eth_phy_func)(void);
 typedef esp_err_t (*eth_tcpip_input_func)(void *buffer, uint16_t len, void *eb);
 typedef void (*eth_gpio_config_func)(void);
 typedef bool (*eth_phy_get_partner_pause_enable_func)(void);
@@ -107,6 +107,7 @@ typedef struct {
     bool flow_ctrl_enable;                      /*!< flag of flow ctrl enable */
     eth_phy_get_partner_pause_enable_func  phy_get_partner_pause_enable; /*!< get partner pause enable */
     eth_phy_power_enable_func  phy_power_enable;  /*!< enable or disable phy power */
+    uint32_t reset_timeout_ms;                  /*!< timeout value for reset emac */
 
 } eth_config_t;
 
@@ -273,6 +274,14 @@ void esp_eth_free_rx_buf(void *buf);
  */
 esp_err_t esp_eth_set_mac(const uint8_t mac[6]);
 
+/**
+ * @brief Get Ethernet link speed
+ *
+ * @return eth_speed_mode_t ETH_SPEED_MODE_10M when link speed is 10Mbps
+ *                          ETH_SPEED_MODE_100M when link speed is 100Mbps
+ */
+eth_speed_mode_t esp_eth_get_speed(void);
+
 #ifdef __cplusplus
 }
 #endif

components/ethernet/include/eth_phy/phy_lan8720.h

@@ -51,7 +51,7 @@ void phy_lan8720_power_enable(bool);
 
 /** @brief Default LAN8720 phy_init function.
  */
-void phy_lan8720_init(void);
+esp_err_t phy_lan8720_init(void);
 
 /** @brief Default LAN8720 PHY configuration
  *

components/ethernet/include/eth_phy/phy_tlk110.h

@@ -50,7 +50,7 @@ void phy_tlk110_power_enable(bool);
 
 /** @brief Default TLK110 phy_init function.
  */
-void phy_tlk110_init(void);
+esp_err_t phy_tlk110_init(void);
 
 /** @brief Default TLK110 PHY configuration
  *

components/lwip/port/esp32/netif/ethernetif.c

@@ -108,7 +108,7 @@ ethernet_low_level_output(struct netif *netif, struct pbuf *p)
 {
   struct pbuf *q = p;
   esp_interface_t eth_if = tcpip_adapter_get_esp_if(netif);
-  err_t ret;
+  esp_err_t ret;
 
   if (eth_if != ESP_IF_ETH) {
     LWIP_DEBUGF(NETIF_DEBUG, ("eth_if=%d netif=%p pbuf=%p len=%d\n", eth_if, netif, p, p->len));
@@ -130,8 +130,13 @@ ethernet_low_level_output(struct netif *netif, struct pbuf *p)
     ret = esp_eth_tx(q->payload, q->len);
     pbuf_free(q);
   }
-
-  return ret;
+  /* error occured when no memory or peripheral wrong state */
+  if (ret != ESP_OK)
+  {
+    return ERR_ABRT;
+  } else {
+    return ERR_OK;
+  }
 }
 
 /**
@@ -230,7 +235,11 @@ ethernetif_init(struct netif *netif)
    * The last argument should be replaced with your link speed, in units
    * of bits per second.
    */
-  NETIF_INIT_SNMP(netif, snmp_ifType_ethernet_csmacd, LINK_SPEED_OF_YOUR_NETIF_IN_BPS);
+  if(esp_eth_get_speed() == ETH_SPEED_MODE_100M){
+    NETIF_INIT_SNMP(netif, snmp_ifType_ethernet_csmacd, 100000000);
+  } else {
+    NETIF_INIT_SNMP(netif, snmp_ifType_ethernet_csmacd, 10000000);
+  }
 
   netif->name[0] = IFNAME0;
   netif->name[1] = IFNAME1;