Merge branch 'bugfix/stop_mac_can_be_controlled_by_user_v4.0' into 'release/v4.0'

Bugfix/stop mac can be controlled by user v4.0

See merge request espressif/esp-idf!8762

components/esp_eth/include/esp_eth_mac.h

@@ -73,6 +73,30 @@ struct esp_eth_mac_s {
     */
     esp_err_t (*deinit)(esp_eth_mac_t *mac);
 
+    /**
+    * @brief Start Ethernet MAC
+    *
+    * @param[in] mac: Ethernet MAC instance
+    *
+    * @return
+    *      - ESP_OK: start Ethernet MAC successfully
+    *      - ESP_FAIL: start Ethernet MAC failed because some other error occurred
+    *
+    */
+    esp_err_t (*start)(esp_eth_mac_t *mac);
+
+    /**
+    * @brief Stop Ethernet MAC
+    *
+    * @param[in] mac: Ethernet MAC instance
+    *
+    * @return
+    *      - ESP_OK: stop Ethernet MAC successfully
+    *      - ESP_FAIL: stop Ethernet MAC failed because some error occurred
+    *
+    */
+    esp_err_t (*stop)(esp_eth_mac_t *mac);
+
     /**
     * @brief Transmit packet from Ethernet MAC
     *

components/esp_eth/src/esp_eth.c

@@ -34,7 +34,10 @@ static const char *TAG = "esp_eth";
 
 ESP_EVENT_DEFINE_BASE(ETH_EVENT);
 
-#define ESP_ETH_FLAGS_STARTED (1<<0)
+typedef enum {
+    ESP_ETH_FSM_STOP,
+    ESP_ETH_FSM_START
+} esp_eth_fsm_t;
 
 /**
  * @brief The Ethernet driver mainly consists of PHY, MAC and
@@ -55,7 +58,7 @@ typedef struct {
     eth_duplex_t duplex;
     eth_link_t link;
     atomic_int ref_count;
-    uint32_t flags;
+    _Atomic esp_eth_fsm_t fsm;
     esp_err_t (*stack_input)(esp_eth_handle_t eth_handle, uint8_t *buffer, uint32_t length);
     esp_err_t (*on_lowlevel_init_done)(esp_eth_handle_t eth_handle);
     esp_err_t (*on_lowlevel_deinit_done)(esp_eth_handle_t eth_handle);
@@ -173,6 +176,7 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_
     esp_eth_driver_t *eth_driver = heap_caps_calloc(1, sizeof(esp_eth_driver_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
     ETH_CHECK(eth_driver, "request memory for eth_driver failed", err, ESP_ERR_NO_MEM);
     atomic_init(&eth_driver->ref_count, 1);
+    atomic_init(&eth_driver->fsm, ESP_ETH_FSM_STOP);
     eth_driver->mac = mac;
     eth_driver->phy = phy;
     eth_driver->link = ETH_LINK_DOWN;
@@ -212,9 +216,20 @@ esp_err_t esp_eth_driver_uninstall(esp_eth_handle_t hdl)
     esp_err_t ret = ESP_OK;
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
     ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG);
+    // check if driver has started
+    esp_eth_fsm_t expected_fsm = ESP_ETH_FSM_STOP;
+    if (!atomic_compare_exchange_strong(&eth_driver->fsm, &expected_fsm, ESP_ETH_FSM_STOP)) {
+        ESP_LOGW(TAG, "driver not stopped yet");
+        ret = ESP_ERR_INVALID_STATE;
+        goto err;
+    }
     // don't uninstall driver unless there's only one reference
-    ETH_CHECK(atomic_load(&eth_driver->ref_count) == 1,
-              "more than one reference to ethernet driver", err, ESP_ERR_INVALID_STATE);
+    int expected_ref_count = 1;
+    if (!atomic_compare_exchange_strong(&eth_driver->ref_count, &expected_ref_count, 0)) {
+        ESP_LOGE(TAG, "%d ethernet reference in use", expected_ref_count);
+        ret = ESP_ERR_INVALID_STATE;
+        goto err;
+    }
     esp_eth_mac_t *mac = eth_driver->mac;
     esp_eth_phy_t *phy = eth_driver->phy;
     ETH_CHECK(xTimerDelete(eth_driver->check_link_timer, 0) == pdPASS, "delete eth_link_timer failed", err, ESP_FAIL);
@@ -232,12 +247,12 @@ esp_err_t esp_eth_start(esp_eth_handle_t hdl)
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
     ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG);
     // check if driver has started
-    if (eth_driver->flags & ESP_ETH_FLAGS_STARTED) {
+    esp_eth_fsm_t expected_fsm = ESP_ETH_FSM_STOP;
+    if (!atomic_compare_exchange_strong(&eth_driver->fsm, &expected_fsm, ESP_ETH_FSM_START)) {
         ESP_LOGW(TAG, "driver started already");
         ret = ESP_ERR_INVALID_STATE;
         goto err;
     }
-    eth_driver->flags |= ESP_ETH_FLAGS_STARTED;
     ETH_CHECK(eth_driver->phy->reset(eth_driver->phy) == ESP_OK, "reset phy failed", err, ESP_FAIL);
     ETH_CHECK(xTimerStart(eth_driver->check_link_timer, 0) == pdPASS,
               "start eth_link_timer failed", err, ESP_FAIL);
@@ -256,17 +271,18 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl)
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
     ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG);
     // check if driver has started
-    if (eth_driver->flags & ESP_ETH_FLAGS_STARTED) {
-        eth_driver->flags &= ~ESP_ETH_FLAGS_STARTED;
-        ETH_CHECK(xTimerStop(eth_driver->check_link_timer, 0) == pdPASS,
-                  "stop eth_link_timer failed", err, ESP_FAIL);
-        ETH_CHECK(esp_event_post(ETH_EVENT, ETHERNET_EVENT_STOP, &eth_driver, sizeof(eth_driver), 0) == ESP_OK,
-                  "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL);
-    } else {
+    esp_eth_fsm_t expected_fsm = ESP_ETH_FSM_START;
+    if (!atomic_compare_exchange_strong(&eth_driver->fsm, &expected_fsm, ESP_ETH_FSM_STOP)) {
         ESP_LOGW(TAG, "driver not started yet");
         ret = ESP_ERR_INVALID_STATE;
         goto err;
     }
+    esp_eth_mac_t *mac = eth_driver->mac;
+    ETH_CHECK(mac->stop(mac) == ESP_OK, "stop mac failed", err, ESP_FAIL);
+    ETH_CHECK(xTimerStop(eth_driver->check_link_timer, 0) == pdPASS,
+              "stop eth_link_timer failed", err, ESP_FAIL);
+    ETH_CHECK(esp_event_post(ETH_EVENT, ETHERNET_EVENT_STOP, &eth_driver, sizeof(eth_driver), 0) == ESP_OK,
+              "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL);
     return ESP_OK;
 err:
     return ret;

components/esp_eth/src/esp_eth_mac_dm9051.c

@@ -337,9 +337,10 @@ err:
 /**
  * @brief start dm9051: enable interrupt and start receive
  */
-static esp_err_t dm9051_start(emac_dm9051_t *emac)
+static esp_err_t emac_dm9051_start(esp_eth_mac_t *mac)
 {
     esp_err_t ret = ESP_OK;
+    emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent);
     /* enable interrupt */
     MAC_CHECK(dm9051_register_write(emac, DM9051_IMR, IMR_ALL) == ESP_OK, "write IMR failed", err, ESP_FAIL);
     /* enable rx */
@@ -355,9 +356,10 @@ err:
 /**
  * @brief stop dm9051: disable interrupt and stop receive
  */
-static esp_err_t dm9051_stop(emac_dm9051_t *emac)
+static esp_err_t emac_dm9051_stop(esp_eth_mac_t *mac)
 {
     esp_err_t ret = ESP_OK;
+    emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent);
     /* disable interrupt */
     MAC_CHECK(dm9051_register_write(emac, DM9051_IMR, 0x00) == ESP_OK, "write IMR failed", err, ESP_FAIL);
     /* disable rx */
@@ -519,11 +521,11 @@ static esp_err_t emac_dm9051_set_link(esp_eth_mac_t *mac, eth_link_t link)
     switch (link) {
     case ETH_LINK_UP:
         MAC_CHECK(nsr & NSR_LINKST, "phy is not link up", err, ESP_ERR_INVALID_STATE);
-        MAC_CHECK(dm9051_start(emac) == ESP_OK, "dm9051 start failed", err, ESP_FAIL);
+        MAC_CHECK(mac->start(mac) == ESP_OK, "dm9051 start failed", err, ESP_FAIL);
         break;
     case ETH_LINK_DOWN:
         MAC_CHECK(!(nsr & NSR_LINKST), "phy is not link down", err, ESP_ERR_INVALID_STATE);
-        MAC_CHECK(dm9051_stop(emac) == ESP_OK, "dm9051 stop failed", err, ESP_FAIL);
+        MAC_CHECK(mac->stop(mac) == ESP_OK, "dm9051 stop failed", err, ESP_FAIL);
         break;
     default:
         MAC_CHECK(false, "unknown link status", err, ESP_ERR_INVALID_ARG);
@@ -629,12 +631,12 @@ static esp_err_t emac_dm9051_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t
     MAC_CHECK(dm9051_register_read(emac, DM9051_MRCMDX, &rxbyte) == ESP_OK, "read MRCMDX failed", err, ESP_FAIL);
     /* rxbyte must be 0xFF, 0 or 1 */
     if (rxbyte > 1) {
-        MAC_CHECK(dm9051_stop(emac) == ESP_OK, "stop dm9051 failed", err, ESP_FAIL);
+        MAC_CHECK(mac->stop(mac) == 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);
         ets_delay_us(10);
-        MAC_CHECK(dm9051_start(emac) == ESP_OK, "start dm9051 failed", err, ESP_FAIL);
+        MAC_CHECK(mac->start(mac) == 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_peek(emac, (uint8_t *)&header, sizeof(header)) == ESP_OK,
@@ -698,7 +700,7 @@ static esp_err_t emac_dm9051_deinit(esp_eth_mac_t *mac)
 {
     emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent);
     esp_eth_mediator_t *eth = emac->eth;
-    dm9051_stop(emac);
+    mac->stop(mac);
     gpio_isr_handler_remove(emac->int_gpio_num);
     gpio_reset_pin(emac->int_gpio_num);
     eth->on_state_changed(eth, ETH_STATE_DEINIT, NULL);
@@ -731,6 +733,8 @@ esp_eth_mac_t *esp_eth_mac_new_dm9051(const eth_dm9051_config_t *dm9051_config,
     emac->parent.set_mediator = emac_dm9051_set_mediator;
     emac->parent.init = emac_dm9051_init;
     emac->parent.deinit = emac_dm9051_deinit;
+    emac->parent.start = emac_dm9051_start;
+    emac->parent.stop = emac_dm9051_stop;
     emac->parent.del = emac_dm9051_del;
     emac->parent.write_phy_reg = emac_dm9051_write_phy_reg;
     emac->parent.read_phy_reg = emac_dm9051_read_phy_reg;

components/esp_eth/src/esp_eth_mac_esp32.c

@@ -336,6 +336,20 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac)
     return ESP_OK;
 }
 
+static esp_err_t emac_esp32_start(esp_eth_mac_t *mac)
+{
+    emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
+    emac_hal_start(&emac->hal);
+    return ESP_OK;
+}
+
+static esp_err_t emac_esp32_stop(esp_eth_mac_t *mac)
+{
+    emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
+    emac_hal_stop(&emac->hal);
+    return ESP_OK;
+}
+
 static esp_err_t emac_esp32_del(esp_eth_mac_t *mac)
 {
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
@@ -410,6 +424,8 @@ esp_eth_mac_t *esp_eth_mac_new_esp32(const eth_mac_config_t *config)
     emac->parent.set_mediator = emac_esp32_set_mediator;
     emac->parent.init = emac_esp32_init;
     emac->parent.deinit = emac_esp32_deinit;
+    emac->parent.start = emac_esp32_start;
+    emac->parent.stop = emac_esp32_stop;
     emac->parent.del = emac_esp32_del;
     emac->parent.write_phy_reg = emac_esp32_write_phy_reg;
     emac->parent.read_phy_reg = emac_esp32_read_phy_reg;

components/esp_eth/src/esp_eth_phy_dm9051.c

@@ -94,6 +94,11 @@ static esp_err_t dm9051_update_link_duplex_speed(phy_dm9051_t *dm9051)
     eth_duplex_t duplex = ETH_DUPLEX_HALF;
     bmsr_reg_t bmsr;
     dscsr_reg_t dscsr;
+    // BMSR is a latch low register
+    // after power up, the first latched value must be 0, which means down
+    // to speed up power up link speed, double read this register as a workaround
+    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_BMSR_REG_ADDR, &(bmsr.val)) == ESP_OK,
               "read BMSR failed", err);
     eth_link_t link = bmsr.link_status ? ETH_LINK_UP : ETH_LINK_DOWN;