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

diff --git a/components/ethernet/emac_common.h b/components/ethernet/emac_common.h
index c8627738345291b85f86bb1b3f2caff016469414..5c8437611c1daaca0def956170a989f37e62fdb5 100644 (file)
--- a/components/ethernet/emac_common.h
+++ b/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 {

diff --git a/components/ethernet/emac_dev.c b/components/ethernet/emac_dev.c
index c439f8a085be3ff5db53fc50121712c2f6b7053f..d07386b2bc5eacb20f6e7c66d37af42c6d86df7d 100644 (file)
--- a/components/ethernet/emac_dev.c
+++ b/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) {

diff --git a/components/ethernet/emac_dev.h b/components/ethernet/emac_dev.h
index 73bb627d0c027283324fa19b621ca480b9b7d83c..9d4cf63c9ba8a89efa44d0a2604f47928ce57b7d 100644 (file)
--- a/components/ethernet/emac_dev.h
+++ b/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);

diff --git a/components/ethernet/emac_main.c b/components/ethernet/emac_main.c
index 5f91f6eba6f4123206840e5610e89acf7fce54a1..5a00a1fd9cfb99c10d71430a4bf043abdcbec363 100644 (file)
--- a/components/ethernet/emac_main.c
+++ b/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;

diff --git a/components/ethernet/eth_phy/phy_lan8720.c b/components/ethernet/eth_phy/phy_lan8720.c
index 0914ce1dc7c77e754bea41a3c652fe7a1d0c77c4..7b9958e6596910e105b82dca378c574722008bc2 100644 (file)
--- a/components/ethernet/eth_phy/phy_lan8720.c
+++ b/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()

diff --git a/components/ethernet/eth_phy/phy_tlk110.c b/components/ethernet/eth_phy/phy_tlk110.c
index 2f9cf50b6d7bdd6064f0cfc096c18020834918e6..ffd556df150e65f618111a0ea015968a559e7a27 100644 (file)
--- a/components/ethernet/eth_phy/phy_tlk110.c
+++ b/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()

diff --git a/components/ethernet/include/esp_eth.h b/components/ethernet/include/esp_eth.h
index 0d39b8efaa7bcf6f1fc6a0a4a3c9fd50257a7ba1..f35bbf4dfcc05a46a8de174ee91c7b7dfc3077ec 100644 (file)
--- a/components/ethernet/include/esp_eth.h
+++ b/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

diff --git a/components/ethernet/include/eth_phy/phy_lan8720.h b/components/ethernet/include/eth_phy/phy_lan8720.h
index 8c579ef5e400037c9353c8edc08adce9193187df..325820bd86685baaa02d08d4089a0d0466d52030 100644 (file)
--- a/components/ethernet/include/eth_phy/phy_lan8720.h
+++ b/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
  *

diff --git a/components/ethernet/include/eth_phy/phy_tlk110.h b/components/ethernet/include/eth_phy/phy_tlk110.h
index ff61c322c7a5b9bb844d3340e83aabe49e5251b8..e6b2d9749976d09172c9e527280549ea67d04e10 100644 (file)
--- a/components/ethernet/include/eth_phy/phy_tlk110.h
+++ b/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
  *

diff --git a/components/lwip/port/esp32/netif/ethernetif.c b/components/lwip/port/esp32/netif/ethernetif.c
index 4cea6a38ff58b9b7dcd2009a1aad5922e1c31bd3..e17dd1bc456bbdcbcb36c3ee23ff749b0cdf2a45 100644 (file)
--- a/components/lwip/port/esp32/netif/ethernetif.c
+++ b/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;