Merge branch 'refactor/ethernet_remove_blocking_in_timer_callback' into 'master'

Ethernet: optimize and bugfix

Closes IDF-1051

See merge request espressif/esp-idf!6213

components/esp_eth/include/esp_eth.h

@@ -148,9 +148,14 @@ esp_err_t esp_eth_transmit(esp_eth_handle_t hdl, uint8_t *buf, uint32_t length);
 * @param[out] buf: buffer to preserve the received packet
 * @param[out] length: length of the received packet
 *
+* @note Before this function got invoked, the value of "length" should set by user, equals the size of buffer.
+*       After the function returned, the value of "length" means the real length of received data.
+*
 * @return
 *       - ESP_OK: receive frame buffer successfully
 *       - ESP_ERR_INVALID_ARG: receive frame buffer failed because of some invalid argument
+*       - ESP_ERR_INVALID_SIZE: input buffer size is not enough to hold the incoming data.
+*                               in this case, value of returned "length" indicates the real size of incoming data.
 *       - ESP_FAIL: receive frame buffer failed because some other error occurred
 */
 esp_err_t esp_eth_receive(esp_eth_handle_t hdl, uint8_t *buf, uint32_t *length);

components/esp_eth/include/esp_eth_mac.h

@@ -99,10 +99,14 @@ struct esp_eth_mac_s {
     * @param[out] length: length of the received packet
     *
     * @note Memory of buf is allocated in the Layer2, make sure it get free after process.
+    * @note Before this function got invoked, the value of "length" should set by user, equals the size of buffer.
+    *       After the function returned, the value of "length" means the real length of received data.
     *
     * @return
     *      - ESP_OK: receive packet successfully
     *      - ESP_ERR_INVALID_ARG: receive packet failed because of invalid argument
+    *      - ESP_ERR_INVALID_SIZE: input buffer size is not enough to hold the incoming data.
+    *                              in this case, value of returned "length" indicates the real size of incoming data.
     *      - ESP_FAIL: receive packet failed because some other error occurred
     *
     */

components/esp_eth/src/esp_eth_mac_dm9051.c

@@ -38,7 +38,6 @@ static const char *TAG = "emac_dm9051";
         }                                                                         \
     } while (0)
 
-#define RX_QUEUE_WAIT_MS (100)
 #define DM9051_SPI_LOCK_TIMEOUT_MS (50)
 #define DM9051_PHY_OPERATION_TIMEOUT_US (1000)
 
@@ -308,6 +307,30 @@ static esp_err_t dm9051_memory_read(emac_dm9051_t *emac, uint8_t *buffer, uint32
     return ret;
 }
 
+/**
+ * @brief peek buffer from dm9051 internal memory (without internal cursor moved)
+ */
+static esp_err_t dm9051_memory_peek(emac_dm9051_t *emac, uint8_t *buffer, uint32_t len)
+{
+    esp_err_t ret = ESP_OK;
+    spi_transaction_t trans = {
+        .cmd = DM9051_SPI_RD,
+        .addr = DM9051_MRCMDX1,
+        .length = len * 8,
+        .rx_buffer = buffer
+    };
+    if (dm9051_lock(emac)) {
+        if (spi_device_polling_transmit(emac->spi_hdl, &trans) != ESP_OK) {
+            ESP_LOGE(TAG, "%s(%d): spi transmit failed", __FUNCTION__, __LINE__);
+            ret = ESP_FAIL;
+        }
+        dm9051_unlock(emac);
+    } else {
+        ret = ESP_ERR_TIMEOUT;
+    }
+    return ret;
+}
+
 /**
  * @brief read mac address from internal registers
  */
@@ -501,26 +524,27 @@ static void emac_dm9051_task(void *arg)
     uint8_t *buffer = NULL;
     uint32_t length = 0;
     while (1) {
-        if (ulTaskNotifyTake(pdFALSE, pdMS_TO_TICKS(RX_QUEUE_WAIT_MS))) {
-            /* clear interrupt status */
-            dm9051_register_read(emac, DM9051_ISR, &status);
-            dm9051_register_write(emac, DM9051_ISR, status);
-            /* packet received */
-            if (status & ISR_PR) {
-                do {
-                    buffer = (uint8_t *)heap_caps_malloc(ETH_MAX_PACKET_SIZE, MALLOC_CAP_DMA);
-                    if (emac->parent.receive(&emac->parent, buffer, &length) == ESP_OK) {
-                        /* pass the buffer to stack (e.g. TCP/IP layer) */
-                        if (length) {
-                            emac->eth->stack_input(emac->eth, buffer, length);
-                        } else {
-                            free(buffer);
-                        }
+        // block indefinitely until some task notifies me
+        ulTaskNotifyTake(pdFALSE, portMAX_DELAY);
+        /* clear interrupt status */
+        dm9051_register_read(emac, DM9051_ISR, &status);
+        dm9051_register_write(emac, DM9051_ISR, status);
+        /* packet received */
+        if (status & ISR_PR) {
+            do {
+                length = ETH_MAX_PACKET_SIZE;
+                buffer = (uint8_t *)heap_caps_malloc(length, MALLOC_CAP_DMA);
+                if (emac->parent.receive(&emac->parent, buffer, &length) == ESP_OK) {
+                    /* pass the buffer to stack (e.g. TCP/IP layer) */
+                    if (length) {
+                        emac->eth->stack_input(emac->eth, buffer, length);
                     } else {
                         free(buffer);
                     }
-                } while (emac->packets_remain);
-            }
+                } else {
+                    free(buffer);
+                }
+            } while (emac->packets_remain);
         }
     }
     vTaskDelete(NULL);
@@ -744,13 +768,26 @@ static esp_err_t emac_dm9051_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t
     if (rxbyte > 1) {
         MAC_CHECK(dm9051_stop(emac) == ESP_OK, "stop dm9051 failed", err, ESP_FAIL);
         /* reset rx fifo pointer */
-        MAC_CHECK(dm9051_register_write(emac, DM9051_MPTRCR, MPTRCR_RST_RX) == ESP_OK, "write MPTRCR failed", err, ESP_FAIL);
+        MAC_CHECK(dm9051_register_write(emac, DM9051_MPTRCR, MPTRCR_RST_RX) == ESP_OK,
+                  "write MPTRCR failed", err, ESP_FAIL);
         ets_delay_us(10);
         MAC_CHECK(dm9051_start(emac) == ESP_OK, "start dm9051 failed", err, ESP_FAIL);
         MAC_CHECK(false, "reset rx fifo pointer", err, ESP_FAIL);
     } else if (rxbyte) {
-        MAC_CHECK(dm9051_memory_read(emac, (uint8_t *)&header, sizeof(header)) == ESP_OK, "read rx header failed", err, ESP_FAIL);
+        MAC_CHECK(dm9051_memory_peek(emac, (uint8_t *)&header, sizeof(header)) == ESP_OK,
+                  "peek rx header failed", err, ESP_FAIL);
         rx_len = header.length_low + (header.length_high << 8);
+        /* check if the buffer can hold all the incoming data */
+        if (*length < rx_len - 4) {
+            ESP_LOGE(TAG, "buffer size too small");
+            /* tell upper layer the size we need */
+            *length = rx_len - 4;
+            ret = ESP_ERR_INVALID_SIZE;
+            goto err;
+        }
+        MAC_CHECK(*length >= rx_len - 4, "buffer size too small", err, ESP_ERR_INVALID_SIZE);
+        MAC_CHECK(dm9051_memory_read(emac, (uint8_t *)&header, sizeof(header)) == ESP_OK,
+                  "read rx header failed", err, ESP_FAIL);
         MAC_CHECK(dm9051_memory_read(emac, buf, rx_len) == ESP_OK, "read rx data failed", err, ESP_FAIL);
         MAC_CHECK(!(header.status & 0xBF), "receive status error: %xH", err, ESP_FAIL, header.status);
         *length = rx_len - 4; // substract the CRC length (4Bytes)
@@ -818,9 +855,10 @@ static esp_err_t emac_dm9051_del(esp_eth_mac_t *mac)
 esp_eth_mac_t *esp_eth_mac_new_dm9051(const eth_dm9051_config_t *dm9051_config, const eth_mac_config_t *mac_config)
 {
     esp_eth_mac_t *ret = NULL;
+    emac_dm9051_t *emac = NULL;
     MAC_CHECK(dm9051_config, "can't set dm9051 specific config to null", err, NULL);
     MAC_CHECK(mac_config, "can't set mac config to null", err, NULL);
-    emac_dm9051_t *emac = calloc(1, sizeof(emac_dm9051_t));
+    emac = calloc(1, sizeof(emac_dm9051_t));
     MAC_CHECK(emac, "calloc emac failed", err, NULL);
     /* bind methods and attributes */
     emac->sw_reset_timeout_ms = mac_config->sw_reset_timeout_ms;
@@ -841,16 +879,22 @@ esp_eth_mac_t *esp_eth_mac_new_dm9051(const eth_dm9051_config_t *dm9051_config,
     emac->parent.receive = emac_dm9051_receive;
     /* create mutex */
     emac->spi_lock = xSemaphoreCreateMutex();
-    MAC_CHECK(emac->spi_lock, "create lock failed", err_lock, NULL);
+    MAC_CHECK(emac->spi_lock, "create lock failed", err, NULL);
     /* create dm9051 task */
     BaseType_t xReturned = xTaskCreate(emac_dm9051_task, "dm9051_tsk", mac_config->rx_task_stack_size, emac,
                                        mac_config->rx_task_prio, &emac->rx_task_hdl);
-    MAC_CHECK(xReturned == pdPASS, "create dm9051 task failed", err_tsk, NULL);
+    MAC_CHECK(xReturned == pdPASS, "create dm9051 task failed", err, NULL);
     return &(emac->parent);
-err_tsk:
-    vSemaphoreDelete(emac->spi_lock);
-err_lock:
-    free(emac);
+
 err:
+    if (emac) {
+        if (emac->rx_task_hdl) {
+            vTaskDelete(emac->rx_task_hdl);
+        }
+        if (emac->spi_lock) {
+            vSemaphoreDelete(emac->spi_lock);
+        }
+        free(emac);
+    }
     return ret;
 }

components/esp_eth/src/esp_eth_mac_esp32.c

@@ -40,7 +40,6 @@ static const char *TAG = "emac_esp32";
         }                                                                         \
     } while (0)
 
-#define RX_QUEUE_WAIT_MS (20)
 #define PHY_OPERATION_TIMEOUT_US (1000)
 
 typedef struct {
@@ -224,7 +223,16 @@ static esp_err_t emac_esp32_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t *
     esp_err_t ret = ESP_OK;
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
     MAC_CHECK(buf && length, "can't set buf and length to null", err, ESP_ERR_INVALID_ARG);
-    *length = emac_hal_receive_frame(&emac->hal, buf, &emac->frames_remain);
+    uint32_t receive_len = emac_hal_receive_frame(&emac->hal, buf, *length, &emac->frames_remain);
+    /* we need to check the return value in case the buffer size is not enough */
+    if (*length < receive_len) {
+        ESP_LOGE(TAG, "buffer size too small");
+        /* tell upper layer the size we need */
+        *length = receive_len;
+        ret = ESP_ERR_INVALID_SIZE;
+        goto err;
+    }
+    *length = receive_len;
     return ESP_OK;
 err:
     return ret;
@@ -236,21 +244,22 @@ static void emac_esp32_rx_task(void *arg)
     uint8_t *buffer = NULL;
     uint32_t length = 0;
     while (1) {
-        if (ulTaskNotifyTake(pdFALSE, pdMS_TO_TICKS(RX_QUEUE_WAIT_MS))) {
-            do {
-                buffer = (uint8_t *)malloc(ETH_MAX_PACKET_SIZE);
-                if (emac_esp32_receive(&emac->parent, buffer, &length) == ESP_OK) {
-                    /* pass the buffer to stack (e.g. TCP/IP layer) */
-                    if (length) {
-                        emac->eth->stack_input(emac->eth, buffer, length);
-                    } else {
-                        free(buffer);
-                    }
+        // block indefinitely until some task notifies me
+        ulTaskNotifyTake(pdFALSE, portMAX_DELAY);
+        do {
+            length = ETH_MAX_PACKET_SIZE;
+            buffer = (uint8_t *)malloc(length);
+            if (emac_esp32_receive(&emac->parent, buffer, &length) == ESP_OK) {
+                /* pass the buffer to stack (e.g. TCP/IP layer) */
+                if (length) {
+                    emac->eth->stack_input(emac->eth, buffer, length);
                 } else {
                     free(buffer);
                 }
-            } while (emac->frames_remain);
-        }
+            } else {
+                free(buffer);
+            }
+        } while (emac->frames_remain);
     }
     vTaskDelete(NULL);
 }
@@ -358,42 +367,29 @@ void emac_esp32_isr_handler(void *args)
 esp_eth_mac_t *esp_eth_mac_new_esp32(const eth_mac_config_t *config)
 {
     esp_eth_mac_t *ret = NULL;
+    void *descriptors = NULL;
+    emac_esp32_t *emac = NULL;
     MAC_CHECK(config, "can't set mac config to null", err, NULL);
-    emac_esp32_t *emac = calloc(1, sizeof(emac_esp32_t));
+    emac = calloc(1, sizeof(emac_esp32_t));
     MAC_CHECK(emac, "calloc emac failed", err, NULL);
     /* alloc memory for ethernet dma descriptor */
     uint32_t desc_size = CONFIG_ETH_DMA_RX_BUFFER_NUM * sizeof(eth_dma_rx_descriptor_t) +
                          CONFIG_ETH_DMA_TX_BUFFER_NUM * sizeof(eth_dma_tx_descriptor_t);
-    void *descriptors = heap_caps_calloc(1, desc_size, MALLOC_CAP_DMA);
-    MAC_CHECK(descriptors, "calloc descriptors failed", err_desc, NULL);
+    descriptors = heap_caps_calloc(1, desc_size, MALLOC_CAP_DMA);
+    MAC_CHECK(descriptors, "calloc descriptors failed", err, NULL);
     int i = 0;
     /* alloc memory for ethernet dma buffer */
     for (i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) {
         emac->rx_buf[i] = heap_caps_calloc(1, CONFIG_ETH_DMA_BUFFER_SIZE, MALLOC_CAP_DMA);
         if (!(emac->rx_buf[i])) {
-            break;
-        }
-    }
-    if (i != CONFIG_ETH_DMA_RX_BUFFER_NUM) {
-        for (--i; i >= 0; i--) {
-            free(emac->rx_buf[i]);
+            goto err;
         }
-        goto err_buffer;
     }
     for (i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) {
         emac->tx_buf[i] = heap_caps_calloc(1, CONFIG_ETH_DMA_BUFFER_SIZE, MALLOC_CAP_DMA);
         if (!(emac->tx_buf[i])) {
-            break;
-        }
-    }
-    if (i != CONFIG_ETH_DMA_TX_BUFFER_NUM) {
-        for (--i; i >= 0; i--) {
-            free(emac->tx_buf[i]);
-        }
-        for (i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) {
-            free(emac->rx_buf[i]);
+            goto err;
         }
-        goto err_buffer;
     }
     /* initialize hal layer driver */
     emac_hal_init(&emac->hal, descriptors, emac->rx_buf, emac->tx_buf);
@@ -415,26 +411,32 @@ esp_eth_mac_t *esp_eth_mac_new_esp32(const eth_mac_config_t *config)
     /* Interrupt configuration */
     MAC_CHECK(esp_intr_alloc(ETS_ETH_MAC_INTR_SOURCE, ESP_INTR_FLAG_IRAM, emac_esp32_isr_handler,
                              &emac->hal, &(emac->intr_hdl)) == ESP_OK,
-              "alloc emac interrupt failed", err_intr, NULL);
+              "alloc emac interrupt failed", err, NULL);
     /* create rx task */
     BaseType_t xReturned = xTaskCreate(emac_esp32_rx_task, "emac_rx", config->rx_task_stack_size, emac,
                                        config->rx_task_prio, &emac->rx_task_hdl);
-    MAC_CHECK(xReturned == pdPASS, "create emac_rx task failed", err_task, NULL);
+    MAC_CHECK(xReturned == pdPASS, "create emac_rx task failed", err, NULL);
     return &(emac->parent);
-err_task:
-    esp_intr_free(emac->intr_hdl);
-err_intr:
-    for (int i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) {
-        free(emac->tx_buf[i]);
+
+err:
+    if (emac) {
+        if (emac->rx_task_hdl) {
+            vTaskDelete(emac->rx_task_hdl);
+        }
+        if (emac->intr_hdl) {
+            esp_intr_free(emac->intr_hdl);
+        }
+        for (int i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) {
+            free(emac->tx_buf[i]);
+        }
+        for (int i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) {
+            free(emac->rx_buf[i]);
+        }
+        free(emac);
     }
-    for (int i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) {
-        free(emac->rx_buf[i]);
+    if (descriptors) {
+        free(descriptors);
     }
-err_buffer:
-    free(descriptors);
-err_desc:
-    free(emac);
-err:
     return ret;
 }
 

components/esp_eth/src/esp_eth_phy_dm9051.c

@@ -79,16 +79,55 @@ typedef union {
 typedef struct {
     esp_eth_phy_t parent;
     esp_eth_mediator_t *eth;
-    const char *name;
     uint32_t addr;
     uint32_t reset_timeout_ms;
     uint32_t autonego_timeout_ms;
     eth_link_t link_status;
 } phy_dm9051_t;
 
+static esp_err_t dm9051_update_link_duplex_speed(phy_dm9051_t *dm9051)
+{
+    esp_eth_mediator_t *eth = dm9051->eth;
+    eth_speed_t speed = ETH_SPEED_10M;
+    eth_duplex_t duplex = ETH_DUPLEX_HALF;
+    bmsr_reg_t bmsr;
+    dscsr_reg_t dscsr;
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+              "read BMSR failed", err);
+    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
+    /* check if link status changed */
+    if (dm9051->link_status != link) {
+        /* when link up, read negotiation result */
+        if (link == ETH_LINK_UP) {
+            PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCSR_REG_ADDR, &(dscsr.val)) == ESP_OK,
+                      "read DSCSR failed", err);
+            if (dscsr.fdx100 || dscsr.hdx100) {
+                speed = ETH_SPEED_100M;
+            } else {
+                speed = ETH_SPEED_10M;
+            }
+            if (dscsr.fdx100 || dscsr.fdx10) {
+                duplex = ETH_DUPLEX_FULL;
+            } else {
+                duplex = ETH_DUPLEX_HALF;
+            }
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK,
+                      "change speed failed", err);
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK,
+                      "change duplex failed", err);
+        }
+        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK,
+                  "change link failed", err);
+        dm9051->link_status = link;
+    }
+    return ESP_OK;
+err:
+    return ESP_FAIL;
+}
+
 static esp_err_t dm9051_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth)
 {
-    PHY_CHECK(eth, "can't set mediator for dm9051 to null", err);
+    PHY_CHECK(eth, "can't set mediator to null", err);
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
     dm9051->eth = eth;
     return ESP_OK;
@@ -99,19 +138,8 @@ err:
 static esp_err_t dm9051_get_link(esp_eth_phy_t *phy)
 {
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
-    esp_eth_mediator_t *eth = dm9051->eth;
-    bmsr_reg_t bmsr;
-
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    if (dm9051->link_status != link) {
-        if (link == ETH_LINK_UP) {
-            phy->negotiate(phy);
-        } else {
-            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-            dm9051->link_status = link;
-        }
-    }
+    /* Updata information about link, speed, duplex */
+    PHY_CHECK(dm9051_update_link_duplex_speed(dm9051) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -122,17 +150,22 @@ static esp_err_t dm9051_reset(esp_eth_phy_t *phy)
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
     esp_eth_mediator_t *eth = dm9051->eth;
     dscr_reg_t dscr;
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, &(dscr.val)) == ESP_OK, "read DSCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, &(dscr.val)) == ESP_OK,
+              "read DSCR failed", err);
     dscr.smrst = 1;
-    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, dscr.val) == ESP_OK, "write DSCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, dscr.val) == ESP_OK,
+              "write DSCR failed", err);
     bmcr_reg_t bmcr = {.reset = 1};
-    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for reset complete */
     uint32_t to = 0;
     for (to = 0; to < dm9051->reset_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
-        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, &(dscr.val)) == ESP_OK, "read DSCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                  "read BMCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCR_REG_ADDR, &(dscr.val)) == ESP_OK,
+                  "read DSCR failed", err);
         if (!bmcr.reset && !dscr.smrst) {
             break;
         }
@@ -154,15 +187,18 @@ static esp_err_t dm9051_negotiate(esp_eth_phy_t *phy)
         .en_auto_nego = 1,     /* Auto Negotiation */
         .restart_auto_nego = 1 /* Restart Auto Negotiation */
     };
-    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for auto negotiation complete */
     bmsr_reg_t bmsr;
     dscsr_reg_t dscsr;
     uint32_t to = 0;
     for (to = 0; to < dm9051->autonego_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCSR_REG_ADDR, &(dscsr.val)) == ESP_OK, "read DSCSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+                  "read BMSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCSR_REG_ADDR, &(dscsr.val)) == ESP_OK,
+                  "read DSCSR failed", err);
         if (bmsr.auto_nego_complete && dscsr.anmb & 0x08) {
             break;
         }
@@ -171,27 +207,7 @@ static esp_err_t dm9051_negotiate(esp_eth_phy_t *phy)
         ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout");
     }
     /* Updata information about link, speed, duplex */
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_DSCSR_REG_ADDR, &(dscsr.val)) == ESP_OK, "read DSCSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    eth_speed_t speed = ETH_SPEED_10M;
-    eth_duplex_t duplex = ETH_DUPLEX_HALF;
-    if (dscsr.fdx100 || dscsr.hdx100) {
-        speed = ETH_SPEED_100M;
-    } else {
-        speed = ETH_SPEED_10M;
-    }
-    if (dscsr.fdx100 || dscsr.fdx10) {
-        duplex = ETH_DUPLEX_FULL;
-    } else {
-        duplex = ETH_DUPLEX_HALF;
-    }
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "send speed event failed", err);
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "send duplex event failed", err);
-    if (dm9051->link_status != link) {
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-        dm9051->link_status = link;
-    }
+    PHY_CHECK(dm9051_update_link_duplex_speed(dm9051) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -202,7 +218,8 @@ static esp_err_t dm9051_pwrctl(esp_eth_phy_t *phy, bool enable)
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
     esp_eth_mediator_t *eth = dm9051->eth;
     bmcr_reg_t bmcr;
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         /* Enable IEEE Power Down Mode */
         bmcr.power_down = 1;
@@ -210,8 +227,10 @@ static esp_err_t dm9051_pwrctl(esp_eth_phy_t *phy, bool enable)
         /* Disable IEEE Power Down Mode */
         bmcr.power_down = 0;
     }
-    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         PHY_CHECK(bmcr.power_down == 1, "power down failed", err);
     } else {
@@ -231,7 +250,7 @@ static esp_err_t dm9051_set_addr(esp_eth_phy_t *phy, uint32_t addr)
 
 static esp_err_t dm9051_get_addr(esp_eth_phy_t *phy, uint32_t *addr)
 {
-    PHY_CHECK(addr, "get phy address failed", err);
+    PHY_CHECK(addr, "addr can't be null", err);
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
     *addr = dm9051->addr;
     return ESP_OK;
@@ -251,15 +270,18 @@ static esp_err_t dm9051_init(esp_eth_phy_t *phy)
     phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent);
     esp_eth_mediator_t *eth = dm9051->eth;
     /* Power on Ethernet PHY */
-    PHY_CHECK(dm9051_pwrctl(phy, true) == ESP_OK, "power on Ethernet PHY failed", err);
+    PHY_CHECK(dm9051_pwrctl(phy, true) == ESP_OK, "power control failed", err);
     /* Reset Ethernet PHY */
-    PHY_CHECK(dm9051_reset(phy) == ESP_OK, "reset Ethernet PHY failed", err);
+    PHY_CHECK(dm9051_reset(phy) == ESP_OK, "reset failed", err);
     /* Check PHY ID */
     phyidr1_reg_t id1;
     phyidr2_reg_t id2;
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK, "read ID1 failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK, "read ID2 failed", err);
-    PHY_CHECK(id1.oui_msb == 0x0181 && id2.oui_lsb == 0x2E && id2.vendor_model == 0x0A, "wrong PHY chip ID", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK,
+              "read ID1 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dm9051->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK,
+              "read ID2 failed", err);
+    PHY_CHECK(id1.oui_msb == 0x0181 && id2.oui_lsb == 0x2E && id2.vendor_model == 0x0A,
+              "wrong chip ID", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -268,7 +290,7 @@ err:
 static esp_err_t dm9051_deinit(esp_eth_phy_t *phy)
 {
     /* Power off Ethernet PHY */
-    PHY_CHECK(dm9051_pwrctl(phy, false) == ESP_OK, "power off Ethernet PHY failed", err);
+    PHY_CHECK(dm9051_pwrctl(phy, false) == ESP_OK, "power control failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -279,8 +301,7 @@ esp_eth_phy_t *esp_eth_phy_new_dm9051(const eth_phy_config_t *config)
     PHY_CHECK(config, "can't set phy config to null", err);
     PHY_CHECK(config->phy_addr == 1, "dm9051's phy address can only set to 1", err);
     phy_dm9051_t *dm9051 = calloc(1, sizeof(phy_dm9051_t));
-    PHY_CHECK(dm9051, "calloc dm9051 object failed", err);
-    dm9051->name = "dm9051";
+    PHY_CHECK(dm9051, "calloc dm9051 failed", err);
     dm9051->addr = config->phy_addr;
     dm9051->reset_timeout_ms = config->reset_timeout_ms;
     dm9051->link_status = ETH_LINK_DOWN;
@@ -295,7 +316,6 @@ esp_eth_phy_t *esp_eth_phy_new_dm9051(const eth_phy_config_t *config)
     dm9051->parent.get_addr = dm9051_get_addr;
     dm9051->parent.set_addr = dm9051_set_addr;
     dm9051->parent.del = dm9051_del;
-
     return &(dm9051->parent);
 err:
     return NULL;

components/esp_eth/src/esp_eth_phy_dp83848.c

@@ -85,16 +85,55 @@ typedef union {
 typedef struct {
     esp_eth_phy_t parent;
     esp_eth_mediator_t *eth;
-    const char *name;
     uint32_t addr;
     uint32_t reset_timeout_ms;
     uint32_t autonego_timeout_ms;
     eth_link_t link_status;
 } phy_dp83848_t;
 
+static esp_err_t dp83848_update_link_duplex_speed(phy_dp83848_t *dp83848)
+{
+    esp_eth_mediator_t *eth = dp83848->eth;
+    eth_speed_t speed = ETH_SPEED_10M;
+    eth_duplex_t duplex = ETH_DUPLEX_HALF;
+    bmsr_reg_t bmsr;
+    physts_reg_t physts;
+    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+              "read BMSR failed", err);
+    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
+    /* check if link status changed */
+    if (dp83848->link_status != link) {
+        /* when link up, read negotiation result */
+        if (link == ETH_LINK_UP) {
+            PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_STS_REG_ADDR, &(physts.val)) == ESP_OK,
+                      "read PHYSTS failed", err);
+            if (physts.speed_status) {
+                speed = ETH_SPEED_10M;
+            } else {
+                speed = ETH_SPEED_100M;
+            }
+            if (physts.duplex_status) {
+                duplex = ETH_DUPLEX_FULL;
+            } else {
+                duplex = ETH_DUPLEX_HALF;
+            }
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK,
+                      "change speed failed", err);
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK,
+                      "change duplex failed", err);
+        }
+        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK,
+                  "change link failed", err);
+        dp83848->link_status = link;
+    }
+    return ESP_OK;
+err:
+    return ESP_FAIL;
+}
+
 static esp_err_t dp83848_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth)
 {
-    PHY_CHECK(eth, "can't set mediator for dp83848 to null", err);
+    PHY_CHECK(eth, "can't set mediator to null", err);
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
     dp83848->eth = eth;
     return ESP_OK;
@@ -105,19 +144,8 @@ err:
 static esp_err_t dp83848_get_link(esp_eth_phy_t *phy)
 {
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
-    esp_eth_mediator_t *eth = dp83848->eth;
-    bmsr_reg_t bmsr;
-
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    if (dp83848->link_status != link) {
-        if (link == ETH_LINK_UP) {
-            phy->negotiate(phy);
-        } else {
-            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-            dp83848->link_status = link;
-        }
-    }
+    /* Updata information about link, speed, duplex */
+    PHY_CHECK(dp83848_update_link_duplex_speed(dp83848) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -128,17 +156,19 @@ static esp_err_t dp83848_reset(esp_eth_phy_t *phy)
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
     esp_eth_mediator_t *eth = dp83848->eth;
     bmcr_reg_t bmcr = {.reset = 1};
-    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for reset complete */
     uint32_t to = 0;
     for (to = 0; to < dp83848->reset_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                  "read BMCR failed", err);
         if (!bmcr.reset) {
             break;
         }
     }
-    PHY_CHECK(to < dp83848->reset_timeout_ms / 10, "PHY reset timeout", err);
+    PHY_CHECK(to < dp83848->reset_timeout_ms / 10, "reset timeout", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -155,45 +185,28 @@ static esp_err_t dp83848_negotiate(esp_eth_phy_t *phy)
         .en_auto_nego = 1,     /* Auto Negotiation */
         .restart_auto_nego = 1 /* Restart Auto Negotiation */
     };
-    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for auto negotiation complete */
     bmsr_reg_t bmsr;
     physts_reg_t physts;
     uint32_t to = 0;
     for (to = 0; to < dp83848->autonego_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_STS_REG_ADDR, &(physts.val)) == ESP_OK, "read PHYSTS failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+                  "read BMSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_STS_REG_ADDR, &(physts.val)) == ESP_OK,
+                  "read PHYSTS failed", err);
         if (bmsr.auto_nego_complete && physts.auto_nego_complete) {
             break;
         }
     }
     /* Auto negotiation failed, maybe no network cable plugged in, so output a warning */
     if (to >= dp83848->autonego_timeout_ms / 10) {
-        ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout");
+        ESP_LOGW(TAG, "auto negotiation timeout");
     }
     /* Updata information about link, speed, duplex */
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_STS_REG_ADDR, &(physts.val)) == ESP_OK, "read PHYSTS failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    eth_speed_t speed = ETH_SPEED_10M;
-    eth_duplex_t duplex = ETH_DUPLEX_HALF;
-    if (physts.speed_status) {
-        speed = ETH_SPEED_10M;
-    } else {
-        speed = ETH_SPEED_100M;
-    }
-    if (physts.duplex_status) {
-        duplex = ETH_DUPLEX_FULL;
-    } else {
-        duplex = ETH_DUPLEX_HALF;
-    }
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "send speed event failed", err);
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "send duplex event failed", err);
-    if (dp83848->link_status != link) {
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-        dp83848->link_status = link;
-    }
+    PHY_CHECK(dp83848_update_link_duplex_speed(dp83848) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -204,7 +217,8 @@ static esp_err_t dp83848_pwrctl(esp_eth_phy_t *phy, bool enable)
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
     esp_eth_mediator_t *eth = dp83848->eth;
     bmcr_reg_t bmcr;
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         /* Enable IEEE Power Down Mode */
         bmcr.power_down = 1;
@@ -212,8 +226,10 @@ static esp_err_t dp83848_pwrctl(esp_eth_phy_t *phy, bool enable)
         /* Disable IEEE Power Down Mode */
         bmcr.power_down = 0;
     }
-    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         PHY_CHECK(bmcr.power_down == 1, "power down failed", err);
     } else {
@@ -233,7 +249,7 @@ static esp_err_t dp83848_set_addr(esp_eth_phy_t *phy, uint32_t addr)
 
 static esp_err_t dp83848_get_addr(esp_eth_phy_t *phy, uint32_t *addr)
 {
-    PHY_CHECK(addr, "get phy address failed", err);
+    PHY_CHECK(addr, "addr can't be null", err);
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
     *addr = dp83848->addr;
     return ESP_OK;
@@ -253,15 +269,18 @@ static esp_err_t dp83848_init(esp_eth_phy_t *phy)
     phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent);
     esp_eth_mediator_t *eth = dp83848->eth;
     /* Power on Ethernet PHY */
-    PHY_CHECK(dp83848_pwrctl(phy, true) == ESP_OK, "power on Ethernet PHY failed", err);
+    PHY_CHECK(dp83848_pwrctl(phy, true) == ESP_OK, "power control failed", err);
     /* Reset Ethernet PHY */
-    PHY_CHECK(dp83848_reset(phy) == ESP_OK, "reset Ethernet PHY failed", err);
+    PHY_CHECK(dp83848_reset(phy) == ESP_OK, "reset failed", err);
     /* Check PHY ID */
     phyidr1_reg_t id1;
     phyidr2_reg_t id2;
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK, "read ID1 failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK, "read ID2 failed", err);
-    PHY_CHECK(id1.oui_msb == 0x2000 && id2.oui_lsb == 0x17 && id2.vendor_model == 0x09, "wrong PHY chip ID", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK,
+              "read ID1 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, dp83848->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK,
+              "read ID2 failed", err);
+    PHY_CHECK(id1.oui_msb == 0x2000 && id2.oui_lsb == 0x17 && id2.vendor_model == 0x09,
+              "wrong chip ID", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -270,7 +289,7 @@ err:
 static esp_err_t dp83848_deinit(esp_eth_phy_t *phy)
 {
     /* Power off Ethernet PHY */
-    PHY_CHECK(dp83848_pwrctl(phy, false) == ESP_OK, "power off Ethernet PHY failed", err);
+    PHY_CHECK(dp83848_pwrctl(phy, false) == ESP_OK, "power control failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -280,8 +299,7 @@ esp_eth_phy_t *esp_eth_phy_new_dp83848(const eth_phy_config_t *config)
 {
     PHY_CHECK(config, "can't set phy config to null", err);
     phy_dp83848_t *dp83848 = calloc(1, sizeof(phy_dp83848_t));
-    PHY_CHECK(dp83848, "calloc dp83848 object failed", err);
-    dp83848->name = "dp83848";
+    PHY_CHECK(dp83848, "calloc dp83848 failed", err);
     dp83848->addr = config->phy_addr;
     dp83848->reset_timeout_ms = config->reset_timeout_ms;
     dp83848->link_status = ETH_LINK_DOWN;

components/esp_eth/src/esp_eth_phy_ip101.c

@@ -102,7 +102,6 @@ typedef union {
 typedef struct {
     esp_eth_phy_t parent;
     esp_eth_mediator_t *eth;
-    const char *name;
     uint32_t addr;
     uint32_t reset_timeout_ms;
     uint32_t autonego_timeout_ms;
@@ -121,9 +120,60 @@ err:
     return ESP_FAIL;
 }
 
+static esp_err_t ip101_update_link_duplex_speed(phy_ip101_t *ip101)
+{
+    esp_eth_mediator_t *eth = ip101->eth;
+    eth_speed_t speed = ETH_SPEED_10M;
+    eth_duplex_t duplex = ETH_DUPLEX_HALF;
+    cssr_reg_t cssr;
+    bmsr_reg_t bmsr;
+    PHY_CHECK(ip101_page_select(ip101, 16) == ESP_OK, "select page 16 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+              "read BMSR failed", err);
+    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
+    /* check if link status changed */
+    if (ip101->link_status != link) {
+        /* when link up, read negotiation result */
+        if (link == ETH_LINK_UP) {
+            PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_CSSR_REG_ADDR, &(cssr.val)) == ESP_OK,
+                      "read CSSR failed", err);
+            switch (cssr.op_mode) {
+            case 1: //10M Half
+                speed = ETH_SPEED_10M;
+                duplex = ETH_DUPLEX_HALF;
+                break;
+            case 2: //100M Half
+                speed = ETH_SPEED_100M;
+                duplex = ETH_DUPLEX_HALF;
+                break;
+            case 5: //10M Full
+                speed = ETH_SPEED_10M;
+                duplex = ETH_DUPLEX_FULL;
+                break;
+            case 6: //100M Full
+                speed = ETH_SPEED_100M;
+                duplex = ETH_DUPLEX_FULL;
+                break;
+            default:
+                break;
+            }
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK,
+                      "change speed failed", err);
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK,
+                      "change duplex failed", err);
+        }
+        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK,
+                  "chagne link failed", err);
+        ip101->link_status = link;
+    }
+    return ESP_OK;
+err:
+    return ESP_FAIL;
+}
+
 static esp_err_t ip101_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth)
 {
-    PHY_CHECK(eth, "can't set mediator for ip101 to null", err);
+    PHY_CHECK(eth, "can't set mediator to null", err);
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     ip101->eth = eth;
     return ESP_OK;
@@ -134,19 +184,8 @@ err:
 static esp_err_t ip101_get_link(esp_eth_phy_t *phy)
 {
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
-    esp_eth_mediator_t *eth = ip101->eth;
-    bmsr_reg_t bmsr;
-
-    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    if (ip101->link_status != link) {
-        if (link == ETH_LINK_UP) {
-            phy->negotiate(phy);
-        } else {
-            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-            ip101->link_status = link;
-        }
-    }
+    /* Updata information about link, speed, duplex */
+    PHY_CHECK(ip101_update_link_duplex_speed(ip101) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -157,17 +196,19 @@ static esp_err_t ip101_reset(esp_eth_phy_t *phy)
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     esp_eth_mediator_t *eth = ip101->eth;
     bmcr_reg_t bmcr = {.reset = 1};
-    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* wait for reset complete */
-    uint32_t to = 0; 
+    uint32_t to = 0;
     for (to = 0; to < ip101->reset_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                  "read BMCR failed", err);
         if (!bmcr.reset) {
             break;
         }
     }
-    PHY_CHECK(to < ip101->reset_timeout_ms / 10, "PHY reset timeout", err);
+    PHY_CHECK(to < ip101->reset_timeout_ms / 10, "reset timeout", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -177,65 +218,32 @@ static esp_err_t ip101_negotiate(esp_eth_phy_t *phy)
 {
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     esp_eth_mediator_t *eth = ip101->eth;
-    /* Start auto negotiation */
+    /* Restart auto negotiation */
     bmcr_reg_t bmcr = {
         .speed_select = 1,     /* 100Mbps */
         .duplex_mode = 1,      /* Full Duplex */
         .en_auto_nego = 1,     /* Auto Negotiation */
         .restart_auto_nego = 1 /* Restart Auto Negotiation */
     };
-    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for auto negotiation complete */
     bmsr_reg_t bmsr;
     uint32_t to = 0;
     for (to = 0; to < ip101->autonego_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+                  "read BMSR failed", err);
         if (bmsr.auto_nego_complete) {
             break;
         }
     }
     /* Auto negotiation failed, maybe no network cable plugged in, so output a warning */
     if (to >= ip101->autonego_timeout_ms / 10) {
-        ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout");
+        ESP_LOGW(TAG, "auto negotiation timeout");
     }
-    PHY_CHECK(ip101_page_select(ip101, 16) == ESP_OK, "select page failed", err);
     /* Updata information about link, speed, duplex */
-    cssr_reg_t cssr;
-    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_CSSR_REG_ADDR, &(cssr.val)) == ESP_OK, "read CSSR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    eth_speed_t speed = ETH_SPEED_10M;
-    eth_duplex_t duplex = ETH_DUPLEX_HALF;
-    switch (cssr.op_mode) {
-    case 0: //Link off
-        link = ETH_LINK_DOWN;
-        break;
-    case 1: //10M Half
-        speed = ETH_SPEED_10M;
-        duplex = ETH_DUPLEX_HALF;
-        break;
-    case 2: //100M Half
-        speed = ETH_SPEED_100M;
-        duplex = ETH_DUPLEX_HALF;
-        break;
-    case 5: //10M Full
-        speed = ETH_SPEED_10M;
-        duplex = ETH_DUPLEX_FULL;
-        break;
-    case 6: //100M Full
-        speed = ETH_SPEED_100M;
-        duplex = ETH_DUPLEX_FULL;
-        break;
-    default:
-        break;
-    }
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "send speed event failed", err);
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "send duplex event failed", err);
-    if (ip101->link_status != link) {
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-        ip101->link_status = link;
-    }
+    PHY_CHECK(ip101_update_link_duplex_speed(ip101) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -246,7 +254,8 @@ static esp_err_t ip101_pwrctl(esp_eth_phy_t *phy, bool enable)
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     esp_eth_mediator_t *eth = ip101->eth;
     bmcr_reg_t bmcr;
-    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         /* Enable IEEE Power Down Mode */
         bmcr.power_down = 1;
@@ -254,8 +263,10 @@ static esp_err_t ip101_pwrctl(esp_eth_phy_t *phy, bool enable)
         /* Disable IEEE Power Down Mode */
         bmcr.power_down = 0;
     }
-    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         PHY_CHECK(bmcr.power_down == 1, "power down failed", err);
     } else {
@@ -275,7 +286,7 @@ static esp_err_t ip101_set_addr(esp_eth_phy_t *phy, uint32_t addr)
 
 static esp_err_t ip101_get_addr(esp_eth_phy_t *phy, uint32_t *addr)
 {
-    PHY_CHECK(addr, "get phy address failed", err);
+    PHY_CHECK(addr, "addr can't be null", err);
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     *addr = ip101->addr;
     return ESP_OK;
@@ -295,15 +306,15 @@ static esp_err_t ip101_init(esp_eth_phy_t *phy)
     phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent);
     esp_eth_mediator_t *eth = ip101->eth;
     /* Power on Ethernet PHY */
-    PHY_CHECK(ip101_pwrctl(phy, true) == ESP_OK, "power on Ethernet PHY failed", err);
+    PHY_CHECK(ip101_pwrctl(phy, true) == ESP_OK, "power control failed", err);
     /* Reset Ethernet PHY */
-    PHY_CHECK(ip101_reset(phy) == ESP_OK, "reset Ethernet PHY failed", err);
+    PHY_CHECK(ip101_reset(phy) == ESP_OK, "reset failed", err);
     /* Check PHY ID */
     phyidr1_reg_t id1;
     phyidr2_reg_t id2;
     PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK, "read ID1 failed", err);
     PHY_CHECK(eth->phy_reg_read(eth, ip101->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK, "read ID2 failed", err);
-    PHY_CHECK(id1.oui_msb == 0x243 && id2.oui_lsb == 0x3 && id2.vendor_model == 0x5, "wrong PHY chip ID", err);
+    PHY_CHECK(id1.oui_msb == 0x243 && id2.oui_lsb == 0x3 && id2.vendor_model == 0x5, "wrong chip ID", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -312,7 +323,7 @@ err:
 static esp_err_t ip101_deinit(esp_eth_phy_t *phy)
 {
     /* Power off Ethernet PHY */
-    PHY_CHECK(ip101_pwrctl(phy, false) == ESP_OK, "power off Ethernet PHY failed", err);
+    PHY_CHECK(ip101_pwrctl(phy, false) == ESP_OK, "power control failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -322,8 +333,7 @@ esp_eth_phy_t *esp_eth_phy_new_ip101(const eth_phy_config_t *config)
 {
     PHY_CHECK(config, "can't set phy config to null", err);
     phy_ip101_t *ip101 = calloc(1, sizeof(phy_ip101_t));
-    PHY_CHECK(ip101, "calloc ip101 object failed", err);
-    ip101->name = "ip101";
+    PHY_CHECK(ip101, "calloc ip101 failed", err);
     ip101->addr = config->phy_addr;
     ip101->reset_timeout_ms = config->reset_timeout_ms;
     ip101->link_status = ETH_LINK_DOWN;

components/esp_eth/src/esp_eth_phy_lan8720.c

@@ -157,16 +157,65 @@ typedef union {
 typedef struct {
     esp_eth_phy_t parent;
     esp_eth_mediator_t *eth;
-    const char *name;
     uint32_t addr;
     uint32_t reset_timeout_ms;
     uint32_t autonego_timeout_ms;
     eth_link_t link_status;
 } phy_lan8720_t;
 
+static esp_err_t lan8720_update_link_duplex_speed(phy_lan8720_t *lan8720)
+{
+    esp_eth_mediator_t *eth = lan8720->eth;
+    eth_speed_t speed = ETH_SPEED_10M;
+    eth_duplex_t duplex = ETH_DUPLEX_HALF;
+    bmsr_reg_t bmsr;
+    pscsr_reg_t pscsr;
+    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+              "read BMSR failed", err);
+    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
+    /* check if link status changed */
+    if (lan8720->link_status != link) {
+        /* when link up, read negotiation result */
+        if (link == ETH_LINK_UP) {
+            PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_PSCSR_REG_ADDR, &(pscsr.val)) == ESP_OK,
+                      "read PSCSR failed", err);
+            switch (pscsr.speed_indication) {
+            case 1: //10Base-T half-duplex
+                speed = ETH_SPEED_10M;
+                duplex = ETH_DUPLEX_HALF;
+                break;
+            case 2: //100Base-TX half-duplex
+                speed = ETH_SPEED_100M;
+                duplex = ETH_DUPLEX_HALF;
+                break;
+            case 5: //10Base-T full-duplex
+                speed = ETH_SPEED_10M;
+                duplex = ETH_DUPLEX_FULL;
+                break;
+            case 6: //100Base-TX full-duplex
+                speed = ETH_SPEED_100M;
+                duplex = ETH_DUPLEX_FULL;
+                break;
+            default:
+                break;
+            }
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK,
+                      "change speed failed", err);
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK,
+                      "change duplex failed", err);
+        }
+        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK,
+                  "change link failed", err);
+        lan8720->link_status = link;
+    }
+    return ESP_OK;
+err:
+    return ESP_FAIL;
+}
+
 static esp_err_t lan8720_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth)
 {
-    PHY_CHECK(eth, "can't set mediator for lan8720 to null", err);
+    PHY_CHECK(eth, "can't set mediator to null", err);
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     lan8720->eth = eth;
     return ESP_OK;
@@ -177,19 +226,8 @@ err:
 static esp_err_t lan8720_get_link(esp_eth_phy_t *phy)
 {
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
-    esp_eth_mediator_t *eth = lan8720->eth;
-    bmsr_reg_t bmsr;
-
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    if (lan8720->link_status != link) {
-        if (link == ETH_LINK_UP) {
-            phy->negotiate(phy);
-        } else {
-            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-            lan8720->link_status = link;
-        }
-    }
+    /* Updata information about link, speed, duplex */
+    PHY_CHECK(lan8720_update_link_duplex_speed(lan8720) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -200,17 +238,19 @@ static esp_err_t lan8720_reset(esp_eth_phy_t *phy)
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     esp_eth_mediator_t *eth = lan8720->eth;
     bmcr_reg_t bmcr = {.reset = 1};
-    PHY_CHECK(eth->phy_reg_write(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* wait for reset complete */
     uint32_t to = 0;
     for (to = 0; to < lan8720->reset_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                  "read BMCR failed", err);
         if (!bmcr.reset) {
             break;
         }
     }
-    PHY_CHECK(to < lan8720->reset_timeout_ms / 10, "PHY reset timeout", err);
+    PHY_CHECK(to < lan8720->reset_timeout_ms / 10, "reset timeout", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -220,7 +260,7 @@ static esp_err_t lan8720_negotiate(esp_eth_phy_t *phy)
 {
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     esp_eth_mediator_t *eth = lan8720->eth;
-    /* Start auto negotiation */
+    /* Restart auto negotiation */
     bmcr_reg_t bmcr = {
         .speed_select = 1,     /* 100Mbps */
         .duplex_mode = 1,      /* Full Duplex */
@@ -234,48 +274,20 @@ static esp_err_t lan8720_negotiate(esp_eth_phy_t *phy)
     int32_t to = 0;
     for (to = 0; to < lan8720->autonego_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_PSCSR_REG_ADDR, &(pscsr.val)) == ESP_OK, "read PSCSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+                  "read BMSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_PSCSR_REG_ADDR, &(pscsr.val)) == ESP_OK,
+                  "read PSCSR failed", err);
         if (bmsr.auto_nego_complete && pscsr.auto_nego_done) {
             break;
         }
     }
     /* Auto negotiation failed, maybe no network cable plugged in, so output a warning */
     if (to >= lan8720->autonego_timeout_ms / 10) {
-        ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout");
+        ESP_LOGW(TAG, "auto negotiation timeout");
     }
     /* Updata information about link, speed, duplex */
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_PSCSR_REG_ADDR, &(pscsr.val)) == ESP_OK, "read PSCSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    eth_speed_t speed = ETH_SPEED_10M;
-    eth_duplex_t duplex = ETH_DUPLEX_HALF;
-    switch (pscsr.speed_indication) {
-    case 1: //10Base-T half-duplex
-        speed = ETH_SPEED_10M;
-        duplex = ETH_DUPLEX_HALF;
-        break;
-    case 2: //100Base-TX half-duplex
-        speed = ETH_SPEED_100M;
-        duplex = ETH_DUPLEX_HALF;
-        break;
-    case 5: //10Base-T full-duplex
-        speed = ETH_SPEED_10M;
-        duplex = ETH_DUPLEX_FULL;
-        break;
-    case 6: //100Base-TX full-duplex
-        speed = ETH_SPEED_100M;
-        duplex = ETH_DUPLEX_FULL;
-        break;
-    default:
-        break;
-    }
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "send speed event failed", err);
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "send duplex event failed", err);
-    if (lan8720->link_status != link) {
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-        lan8720->link_status = link;
-    }
+    PHY_CHECK(lan8720_update_link_duplex_speed(lan8720) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -286,7 +298,8 @@ static esp_err_t lan8720_pwrctl(esp_eth_phy_t *phy, bool enable)
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     esp_eth_mediator_t *eth = lan8720->eth;
     bmcr_reg_t bmcr;
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         /* General Power Down Mode */
         bmcr.power_down = 1;
@@ -294,8 +307,10 @@ static esp_err_t lan8720_pwrctl(esp_eth_phy_t *phy, bool enable)
         /* Normal operation Mode */
         bmcr.power_down = 0;
     }
-    PHY_CHECK(eth->phy_reg_write(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         PHY_CHECK(bmcr.power_down == 1, "power down failed", err);
     } else {
@@ -315,7 +330,7 @@ static esp_err_t lan8720_set_addr(esp_eth_phy_t *phy, uint32_t addr)
 
 static esp_err_t lan8720_get_addr(esp_eth_phy_t *phy, uint32_t *addr)
 {
-    PHY_CHECK(addr, "get phy address failed", err);
+    PHY_CHECK(addr, "addr can't be null", err);
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     *addr = lan8720->addr;
     return ESP_OK;
@@ -335,15 +350,17 @@ static esp_err_t lan8720_init(esp_eth_phy_t *phy)
     phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent);
     esp_eth_mediator_t *eth = lan8720->eth;
     /* Power on Ethernet PHY */
-    PHY_CHECK(lan8720_pwrctl(phy, true) == ESP_OK, "power on Ethernet PHY failed", err);
+    PHY_CHECK(lan8720_pwrctl(phy, true) == ESP_OK, "power control failed", err);
     /* Reset Ethernet PHY */
-    PHY_CHECK(lan8720_reset(phy) == ESP_OK, "reset Ethernet PHY failed", err);
+    PHY_CHECK(lan8720_reset(phy) == ESP_OK, "reset failed", err);
     /* Check PHY ID */
     phyidr1_reg_t id1;
     phyidr2_reg_t id2;
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK, "read ID1 failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK, "read ID2 failed", err);
-    PHY_CHECK(id1.oui_msb == 0x7 && id2.oui_lsb == 0x30 && id2.vendor_model == 0xF, "wrong PHY chip ID", err);
+    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK,
+              "read ID1 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, lan8720->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK,
+              "read ID2 failed", err);
+    PHY_CHECK(id1.oui_msb == 0x7 && id2.oui_lsb == 0x30 && id2.vendor_model == 0xF, "wrong chip ID", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -352,7 +369,7 @@ err:
 static esp_err_t lan8720_deinit(esp_eth_phy_t *phy)
 {
     /* Power off Ethernet PHY */
-    PHY_CHECK(lan8720_pwrctl(phy, false) == ESP_OK, "power off Ethernet PHY failed", err);
+    PHY_CHECK(lan8720_pwrctl(phy, false) == ESP_OK, "power control failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -362,8 +379,7 @@ esp_eth_phy_t *esp_eth_phy_new_lan8720(const eth_phy_config_t *config)
 {
     PHY_CHECK(config, "can't set phy config to null", err);
     phy_lan8720_t *lan8720 = calloc(1, sizeof(phy_lan8720_t));
-    PHY_CHECK(lan8720, "calloc lan8720 object failed", err);
-    lan8720->name = "lan8720";
+    PHY_CHECK(lan8720, "calloc lan8720 failed", err);
     lan8720->addr = config->phy_addr;
     lan8720->reset_timeout_ms = config->reset_timeout_ms;
     lan8720->link_status = ETH_LINK_DOWN;

components/esp_eth/src/esp_eth_phy_rtl8201.c

@@ -63,7 +63,6 @@ typedef union {
 typedef struct {
     esp_eth_phy_t parent;
     esp_eth_mediator_t *eth;
-    const char *name;
     uint32_t addr;
     uint32_t reset_timeout_ms;
     uint32_t autonego_timeout_ms;
@@ -82,9 +81,50 @@ err:
     return ESP_FAIL;
 }
 
+static esp_err_t rtl8201_update_link_duplex_speed(phy_rtl8201_t *rtl8201)
+{
+    esp_eth_mediator_t *eth = rtl8201->eth;
+    eth_speed_t speed = ETH_SPEED_10M;
+    eth_duplex_t duplex = ETH_DUPLEX_HALF;
+    bmcr_reg_t bmcr;
+    bmsr_reg_t bmsr;
+    PHY_CHECK(rtl8201_page_select(rtl8201, 0) == ESP_OK, "select page 0 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+              "read BMSR failed", err);
+    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
+    /* check if link status changed */
+    if (rtl8201->link_status != link) {
+        /* when link up, read negotiation result */
+        if (link == ETH_LINK_UP) {
+            PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                      "read BMCR failed", err);
+            if (bmcr.speed_select) {
+                speed = ETH_SPEED_100M;
+            } else {
+                speed = ETH_SPEED_10M;
+            }
+            if (bmcr.duplex_mode) {
+                duplex = ETH_DUPLEX_FULL;
+            } else {
+                duplex = ETH_DUPLEX_HALF;
+            }
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK,
+                      "change speed failed", err);
+            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK,
+                      "change duplex failed", err);
+        }
+        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK,
+                  "change link failed", err);
+        rtl8201->link_status = link;
+    }
+    return ESP_OK;
+err:
+    return ESP_FAIL;
+}
+
 static esp_err_t rtl8201_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth)
 {
-    PHY_CHECK(eth, "can't set mediator for rtl8201 to null", err);
+    PHY_CHECK(eth, "can't set mediator to null", err);
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     rtl8201->eth = eth;
     return ESP_OK;
@@ -95,19 +135,8 @@ err:
 static esp_err_t rtl8201_get_link(esp_eth_phy_t *phy)
 {
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
-    esp_eth_mediator_t *eth = rtl8201->eth;
-    bmsr_reg_t bmsr;
-
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    if (rtl8201->link_status != link) {
-        if (link == ETH_LINK_UP) {
-            phy->negotiate(phy);
-        } else {
-            PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-            rtl8201->link_status = link;
-        }
-    }
+    /* Updata information about link, speed, duplex */
+    PHY_CHECK(rtl8201_update_link_duplex_speed(rtl8201) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -118,17 +147,19 @@ static esp_err_t rtl8201_reset(esp_eth_phy_t *phy)
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     esp_eth_mediator_t *eth = rtl8201->eth;
     bmcr_reg_t bmcr = {.reset = 1};
-    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for reset complete */
     uint32_t to = 0;
     for (to = 0; to < rtl8201->reset_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+                  "read BMCR failed", err);
         if (!bmcr.reset) {
             break;
         }
     }
-    PHY_CHECK(to < rtl8201->reset_timeout_ms / 10, "PHY reset timeout", err);
+    PHY_CHECK(to < rtl8201->reset_timeout_ms / 10, "reset timeout", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -138,51 +169,32 @@ static esp_err_t rtl8201_negotiate(esp_eth_phy_t *phy)
 {
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     esp_eth_mediator_t *eth = rtl8201->eth;
-    /* Start auto negotiation */
+    /* Restart auto negotiation */
     bmcr_reg_t bmcr = {
         .speed_select = 1,     /* 100Mbps */
         .duplex_mode = 1,      /* Full Duplex */
         .en_auto_nego = 1,     /* Auto Negotiation */
         .restart_auto_nego = 1 /* Restart Auto Negotiation */
     };
-    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
     /* Wait for auto negotiation complete */
     bmsr_reg_t bmsr;
     uint32_t to = 0;
     for (to = 0; to < rtl8201->autonego_timeout_ms / 10; to++) {
         vTaskDelay(pdMS_TO_TICKS(10));
-        PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
+        PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
+                  "read BMSR failed", err);
         if (bmsr.auto_nego_complete) {
             break;
         }
     }
     /* Auto negotiation failed, maybe no network cable plugged in, so output a warning */
     if (to >= rtl8201->autonego_timeout_ms / 10) {
-        ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout");
+        ESP_LOGW(TAG, "auto negotiation timeout");
     }
-    PHY_CHECK(rtl8201_page_select(rtl8201, 0) == ESP_OK, "select page failed", err);
     /* Updata information about link, speed, duplex */
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK, "read BMSR failed", err);
-    eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;
-    eth_speed_t speed = ETH_SPEED_10M;
-    eth_duplex_t duplex = ETH_DUPLEX_HALF;
-    if (bmcr.speed_select) {
-        speed = ETH_SPEED_100M;
-    } else {
-        speed = ETH_SPEED_10M;
-    }
-    if (bmcr.duplex_mode) {
-        duplex = ETH_DUPLEX_FULL;
-    } else {
-        duplex = ETH_DUPLEX_HALF;
-    }
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "send speed event failed", err);
-    PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "send duplex event failed", err);
-    if (rtl8201->link_status != link) {
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "send link event failed", err);
-        rtl8201->link_status = link;
-    }
+    PHY_CHECK(rtl8201_update_link_duplex_speed(rtl8201) == ESP_OK, "update link duplex speed failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -193,7 +205,8 @@ static esp_err_t rtl8201_pwrctl(esp_eth_phy_t *phy, bool enable)
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     esp_eth_mediator_t *eth = rtl8201->eth;
     bmcr_reg_t bmcr;
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         /* Enable IEEE Power Down Mode */
         bmcr.power_down = 1;
@@ -201,8 +214,10 @@ static esp_err_t rtl8201_pwrctl(esp_eth_phy_t *phy, bool enable)
         /* Disable IEEE Power Down Mode */
         bmcr.power_down = 0;
     }
-    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK, "write BMCR failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK, "read BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_write(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, bmcr.val) == ESP_OK,
+              "write BMCR failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)) == ESP_OK,
+              "read BMCR failed", err);
     if (!enable) {
         PHY_CHECK(bmcr.power_down == 1, "power down failed", err);
     } else {
@@ -222,7 +237,7 @@ static esp_err_t rtl8201_set_addr(esp_eth_phy_t *phy, uint32_t addr)
 
 static esp_err_t rtl8201_get_addr(esp_eth_phy_t *phy, uint32_t *addr)
 {
-    PHY_CHECK(addr, "get phy address failed", err);
+    PHY_CHECK(addr, "addr can't be null", err);
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     *addr = rtl8201->addr;
     return ESP_OK;
@@ -242,15 +257,18 @@ static esp_err_t rtl8201_init(esp_eth_phy_t *phy)
     phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent);
     esp_eth_mediator_t *eth = rtl8201->eth;
     /* Power on Ethernet PHY */
-    PHY_CHECK(rtl8201_pwrctl(phy, true) == ESP_OK, "power on Ethernet PHY failed", err);
+    PHY_CHECK(rtl8201_pwrctl(phy, true) == ESP_OK, "power control failed", err);
     /* Reset Ethernet PHY */
-    PHY_CHECK(rtl8201_reset(phy) == ESP_OK, "reset Ethernet PHY failed", err);
+    PHY_CHECK(rtl8201_reset(phy) == ESP_OK, "reset failed", err);
     /* Check PHY ID */
     phyidr1_reg_t id1;
     phyidr2_reg_t id2;
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK, "read ID1 failed", err);
-    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK, "read ID2 failed", err);
-    PHY_CHECK(id1.oui_msb == 0x1C && id2.oui_lsb == 0x32 && id2.vendor_model == 0x1, "wrong PHY chip ID", err);
+    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_IDR1_REG_ADDR, &(id1.val)) == ESP_OK,
+              "read ID1 failed", err);
+    PHY_CHECK(eth->phy_reg_read(eth, rtl8201->addr, ETH_PHY_IDR2_REG_ADDR, &(id2.val)) == ESP_OK,
+              "read ID2 failed", err);
+    PHY_CHECK(id1.oui_msb == 0x1C && id2.oui_lsb == 0x32 && id2.vendor_model == 0x1,
+              "wrong chip ID", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -259,7 +277,7 @@ err:
 static esp_err_t rtl8201_deinit(esp_eth_phy_t *phy)
 {
     /* Power off Ethernet PHY */
-    PHY_CHECK(rtl8201_pwrctl(phy, false) == ESP_OK, "power off Ethernet PHY failed", err);
+    PHY_CHECK(rtl8201_pwrctl(phy, false) == ESP_OK, "power control failed", err);
     return ESP_OK;
 err:
     return ESP_FAIL;
@@ -269,8 +287,7 @@ esp_eth_phy_t *esp_eth_phy_new_rtl8201(const eth_phy_config_t *config)
 {
     PHY_CHECK(config, "can't set phy config to null", err);
     phy_rtl8201_t *rtl8201 = calloc(1, sizeof(phy_rtl8201_t));
-    PHY_CHECK(rtl8201, "calloc rtl8201 object failed", err);
-    rtl8201->name = "rtl8201";
+    PHY_CHECK(rtl8201, "calloc rtl8201 failed", err);
     rtl8201->addr = config->phy_addr;
     rtl8201->reset_timeout_ms = config->reset_timeout_ms;
     rtl8201->link_status = ETH_LINK_DOWN;

components/esp_eth/test/test_emac.c

@@ -81,7 +81,7 @@ TEST_CASE("esp32 ethernet io test", "[ethernet][test_env=UT_T2_Ethernet]")
     uint8_t mac_addr[6];
     memset(mac_addr, 0, sizeof(mac_addr));
     TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_G_MAC_ADDR, mac_addr));
-    ESP_LOGI(TAG, "Ethernet MAC Address: %d:%d:%d:%d:%d:%d",
+    ESP_LOGI(TAG, "Ethernet MAC Address: %02x:%02x:%02x:%02x:%02x:%02x",
              mac_addr[0], mac_addr[1], mac_addr[2], mac_addr[3], mac_addr[4], mac_addr[5]);
     TEST_ASSERT(mac_addr[0] != 0);
     /* get PHY address */

components/soc/esp32/emac_hal.c

@@ -482,7 +482,7 @@ void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t len
     hal->dma_regs->dmatxpolldemand = 0;
 }
 
-uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t *frames_remain)
+uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t size, uint32_t *frames_remain)
 {
     eth_dma_rx_descriptor_t *desc_iter = NULL;
     eth_dma_rx_descriptor_t *first_desc = NULL;
@@ -501,6 +501,12 @@ uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t
         if (desc_iter->RDES0.LastDescriptor) {
             /* Get the Frame Length of the received packet: substruct 4 bytes of the CRC */
             len = desc_iter->RDES0.FrameLength - ETH_CRC_LENGTH;
+            /* check if the buffer can store the whole frame */
+            if (len > size) {
+                /* return the real size that we want */
+                /* user need to compare the return value to the size they prepared when this function returned */
+                return len;
+            }
             /* update unhandled frame count */
             frame_count++;
         }

components/soc/esp32/include/hal/emac.h

@@ -380,7 +380,7 @@ uint32_t emac_hal_get_tx_desc_owner(emac_hal_context_t *hal);
 
 void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t length);
 
-uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t *frames_remain);
+uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t size, uint32_t *frames_remain);
 
 void emac_hal_isr(void *arg);