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 */
+ emac_ll_clear_all_pending_intr(hal->dma_regs);
 
- /* Flush Transmit FIFO */
- emac_ll_flush_trans_fifo_enable(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 transmit state machine of the MAC for transmission on the MII */
- emac_ll_transmit_enable(hal->mac_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) {