Merge branch 'bugfix/eth_lan8720_ci_v4.4' into 'release/v4.4'

esp_eth: start/stop and L2 test stability improvements (v4.4)

See merge request espressif/esp-idf!22322

components/esp_eth/src/esp_eth.c

@@ -43,7 +43,7 @@ typedef struct {
     uint32_t check_link_period_ms;
     eth_speed_t speed;
     eth_duplex_t duplex;
-    eth_link_t link;
+    _Atomic eth_link_t link;
     atomic_int ref_count;
     void *priv;
     _Atomic esp_eth_fsm_t fsm;
@@ -119,7 +119,7 @@ static esp_err_t eth_on_state_changed(esp_eth_mediator_t *eth, esp_eth_state_t s
     case ETH_STATE_LINK: {
         eth_link_t link = (eth_link_t)args;
         ESP_GOTO_ON_ERROR(mac->set_link(mac, link), err, TAG, "ethernet mac set link failed");
-        eth_driver->link = link;
+        atomic_store(&eth_driver->link, link);
         if (link == ETH_LINK_UP) {
             ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_CONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
                               TAG, "send ETHERNET_EVENT_CONNECTED event failed");
@@ -191,7 +191,7 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_
     atomic_init(&eth_driver->fsm, ESP_ETH_FSM_STOP);
     eth_driver->mac = mac;
     eth_driver->phy = phy;
-    eth_driver->link = ETH_LINK_DOWN;
+    atomic_init(&eth_driver->link, ETH_LINK_DOWN);
     eth_driver->duplex = ETH_DUPLEX_HALF;
     eth_driver->speed = ETH_SPEED_10M;
     eth_driver->stack_input = config->stack_input;
@@ -287,7 +287,15 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl)
     ESP_GOTO_ON_FALSE(atomic_compare_exchange_strong(&eth_driver->fsm, &expected_fsm, ESP_ETH_FSM_STOP),
                       ESP_ERR_INVALID_STATE, err, TAG, "driver not started yet");
     ESP_GOTO_ON_ERROR(esp_timer_stop(eth_driver->check_link_timer), err, TAG, "stop link timer failed");
-    ESP_GOTO_ON_ERROR(mac->stop(mac), err, TAG, "stop mac failed");
+
+    eth_link_t expected_link = ETH_LINK_UP;
+    if (atomic_compare_exchange_strong(&eth_driver->link, &expected_link, ETH_LINK_DOWN)){
+        // MAC is stopped by setting link down
+        ESP_GOTO_ON_ERROR(mac->set_link(mac, ETH_LINK_DOWN), err, TAG, "ethernet mac set link failed");
+        ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_DISCONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
+                            TAG, "send ETHERNET_EVENT_DISCONNECTED event failed");
+    }
+
     ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_STOP, &eth_driver, sizeof(esp_eth_driver_t *), 0),
                       err, TAG, "send ETHERNET_EVENT_STOP event failed");
 err:
@@ -313,9 +321,9 @@ esp_err_t esp_eth_transmit(esp_eth_handle_t hdl, void *buf, size_t length)
     esp_err_t ret = ESP_OK;
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
 
-    if (atomic_load(&eth_driver->fsm) != ESP_ETH_FSM_START) {
+    if (atomic_load(&eth_driver->link) != ETH_LINK_UP) {
         ret = ESP_ERR_INVALID_STATE;
-        ESP_LOGD(TAG, "Ethernet is not started");
+        ESP_LOGD(TAG, "Ethernet link is not up, can't transmit");
         goto err;
     }
 

components/esp_eth/src/esp_eth_mac_esp.c

@@ -33,7 +33,7 @@
 static const char *TAG = "esp.emac";
 
 #define PHY_OPERATION_TIMEOUT_US (1000)
-#define MAC_STOP_TIMEOUT_US (250)
+#define MAC_STOP_TIMEOUT_US (2500) // this is absolute maximum for 10Mbps, it is 10 times faster for 100Mbps
 #define FLOW_CONTROL_LOW_WATER_MARK (CONFIG_ETH_DMA_RX_BUFFER_NUM / 3)
 #define FLOW_CONTROL_HIGH_WATER_MARK (FLOW_CONTROL_LOW_WATER_MARK * 2)
 
@@ -240,7 +240,6 @@ static esp_err_t emac_esp32_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t *
     ESP_GOTO_ON_FALSE(buf && length, ESP_ERR_INVALID_ARG, err, TAG, "can't set buf and length to null");
     uint32_t receive_len = emac_hal_receive_frame(&emac->hal, buf, expected_len, &emac->frames_remain, &emac->free_rx_descriptor);
     /* we need to check the return value in case the buffer size is not enough */
-    ESP_LOGD(TAG, "receive len= %d", receive_len);
     ESP_GOTO_ON_FALSE(expected_len >= receive_len, ESP_ERR_INVALID_SIZE, err, TAG, "received buffer longer than expected");
     *length = receive_len;
     return ESP_OK;
@@ -349,8 +348,6 @@ static esp_err_t emac_esp32_init(esp_eth_mac_t *mac)
     ESP_GOTO_ON_FALSE(to < emac->sw_reset_timeout_ms / 10, ESP_ERR_TIMEOUT, err, TAG, "reset timeout");
     /* set smi clock */
     emac_hal_set_csr_clock_range(&emac->hal, esp_clk_apb_freq());
-    /* reset descriptor chain */
-    emac_hal_reset_desc_chain(&emac->hal);
     /* init mac registers by default */
     emac_hal_init_mac_default(&emac->hal);
     /* init dma registers by default */
@@ -384,6 +381,7 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac)
 static esp_err_t emac_esp32_start(esp_eth_mac_t *mac)
 {
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
+    /* reset descriptor chain */
     emac_hal_reset_desc_chain(&emac->hal);
     emac_hal_start(&emac->hal);
     return ESP_OK;

components/hal/emac_hal.c

@@ -12,6 +12,18 @@
 
 #define ETH_CRC_LENGTH (4)
 
+static esp_err_t emac_hal_flush_trans_fifo(emac_hal_context_t *hal)
+{
+    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
+    /* no other writes to the Operation Mode register until the flush tx fifo bit is cleared */
+    for (uint32_t i = 0; i < 1000; i++) {
+        if (emac_ll_get_flush_trans_fifo(hal->dma_regs) == 0) {
+            return ESP_OK;
+        }
+    }
+    return ESP_ERR_TIMEOUT;
+}
+
 void emac_hal_iomux_init_mii(void)
 {
     /* TX_CLK to GPIO0 */
@@ -277,7 +289,7 @@ void emac_hal_init_dma_default(emac_hal_context_t *hal)
     /* Disable Transmit Store Forward */
     emac_ll_trans_store_forward_enable(hal->dma_regs, false);
     /* Flush Transmit FIFO */
-    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
+    emac_hal_flush_trans_fifo(hal);
     /* Transmit Threshold Control */
     emac_ll_set_transmit_threshold(hal->dma_regs, EMAC_LL_TRANSMIT_THRESHOLD_CONTROL_64);
     /* Disable Forward Error Frame */
@@ -334,22 +346,21 @@ void emac_hal_start(emac_hal_context_t *hal)
 {
     /* Enable Ethernet MAC and DMA Interrupt */
     emac_ll_enable_corresponding_intr(hal->dma_regs, EMAC_LL_CONFIG_ENABLE_INTR_MASK);
-
+    /* Clear all pending interrupts */
-    /* Flush Transmit FIFO */
+    emac_ll_clear_all_pending_intr(hal->dma_regs);
-    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
-
-    /* Start DMA transmission */
-    emac_ll_start_stop_dma_transmit(hal->dma_regs, true);
-    /* Start DMA reception */
-    emac_ll_start_stop_dma_receive(hal->dma_regs, true);
 
     /* Enable transmit state machine of the MAC for transmission on the MII */
     emac_ll_transmit_enable(hal->mac_regs, true);
+    /* Start DMA transmission */
+    /* Note that the EMAC Databook states the DMA could be started prior enabling
+       the MAC transmitter. However, it turned out that such order may cause the MAC
+       transmitter hangs */
+    emac_ll_start_stop_dma_transmit(hal->dma_regs, true);
+
+    /* Start DMA reception */
+    emac_ll_start_stop_dma_receive(hal->dma_regs, true);
     /* Enable receive state machine of the MAC for reception from the MII */
     emac_ll_receive_enable(hal->mac_regs, true);
-
-    /* Clear all pending interrupts */
-    emac_ll_clear_all_pending_intr(hal->dma_regs);
 }
 
 esp_err_t emac_hal_stop(emac_hal_context_t *hal)
@@ -375,6 +386,9 @@ esp_err_t emac_hal_stop(emac_hal_context_t *hal)
     /* Stop DMA reception */
     emac_ll_start_stop_dma_receive(hal->dma_regs, false);
 
+    /* Flush Transmit FIFO */
+    emac_hal_flush_trans_fifo(hal);
+
     /* Disable Ethernet MAC and DMA Interrupt */
     emac_ll_disable_all_intr(hal->dma_regs);
 

components/hal/esp32/include/hal/emac_ll.h

@@ -417,6 +417,11 @@ static inline void emac_ll_flush_trans_fifo_enable(emac_dma_dev_t *dma_regs, boo
     dma_regs->dmaoperation_mode.flush_tx_fifo = enable;
 }
 
+static inline bool emac_ll_get_flush_trans_fifo(emac_dma_dev_t *dma_regs)
+{
+    return dma_regs->dmaoperation_mode.flush_tx_fifo;
+}
+
 static inline void emac_ll_set_transmit_threshold(emac_dma_dev_t *dma_regs, uint32_t threshold)
 {
     dma_regs->dmaoperation_mode.tx_thresh_ctrl = threshold;

examples/ethernet/enc28j60/components/eth_enc28j60/esp_eth_phy_enc28j60.c

@@ -213,6 +213,9 @@ esp_err_t enc28j60_set_phy_duplex(esp_eth_phy_t *phy, eth_duplex_t duplex)
     esp_eth_mediator_t *eth = enc28j60->eth;
     phcon1_reg_t phcon1;
 
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
+    enc28j60->link_status = ETH_LINK_DOWN;
+
     PHY_CHECK(eth->phy_reg_read(eth, enc28j60->addr, 0, &phcon1.val) == ESP_OK,
               "read PHCON1 failed", err);
     switch (duplex) {