diff --git a/components/esp_eth/include/esp_eth_mac.h b/components/esp_eth/include/esp_eth_mac.h index 8ef030d8a9..70b475b0a9 100644 --- a/components/esp_eth/include/esp_eth_mac.h +++ b/components/esp_eth/include/esp_eth_mac.h @@ -18,8 +18,9 @@ extern "C" { #endif #include -#include "esp_eth_com.h" +#include #include "sdkconfig.h" +#include "esp_eth_com.h" #if CONFIG_ETH_USE_SPI_ETHERNET #include "driver/gpio.h" #include "driver/spi_master.h" @@ -37,6 +38,12 @@ typedef struct esp_eth_mac_s esp_eth_mac_t; * */ struct esp_eth_mac_s { + /** + * @brief Reference count of MAC instance + * + */ + atomic_int ref_count; + /** * @brief Set mediator for Ethernet MAC * @@ -310,6 +317,17 @@ esp_eth_mac_t *esp_eth_mac_new_dm9051(const eth_dm9051_config_t *dm9051_config, #if CONFIG_ETH_USE_OPENETH +/** +* @brief Create OpenETH MAC instance +* +* @note This API is only used for qemu simulation +* +* @param config: Ethernet MAC configuration +* +* @return +* - instance: create MAC instance successfully +* - NULL: create MAC instance failed because some error occurred +*/ esp_eth_mac_t *esp_eth_mac_new_openeth(const eth_mac_config_t *config); #endif // CONFIG_ETH_USE_OPENETH diff --git a/components/esp_eth/include/esp_eth_phy.h b/components/esp_eth/include/esp_eth_phy.h index b30fdad5dd..5a0080d5dc 100644 --- a/components/esp_eth/include/esp_eth_phy.h +++ b/components/esp_eth/include/esp_eth_phy.h @@ -18,8 +18,9 @@ extern "C" { #endif #include -#include "esp_eth_com.h" +#include #include "sdkconfig.h" +#include "esp_eth_com.h" /** * @brief Ethernet PHY @@ -32,6 +33,12 @@ typedef struct esp_eth_phy_s esp_eth_phy_t; * */ struct esp_eth_phy_s { + /** + * @brief Reference count of PHY instance + * + */ + atomic_int ref_count; + /** * @brief Set mediator for PHY * diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 643a303c03..c583d7a85b 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -148,7 +148,15 @@ static void eth_check_link_timer_cb(TimerHandle_t xTimer) { esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)pvTimerGetTimerID(xTimer); esp_eth_phy_t *phy = eth_driver->phy; + /* add reference to prevent other thread deleting the phy instance */ + atomic_fetch_add(&phy->ref_count, 1); phy->get_link(phy); + /* only the timer obtains the instance, i.e. all other references to this phy has been released */ + if (atomic_load(&phy->ref_count) == 1) { + phy->del(phy); // since this phy doesn't have other owners, delete it + } else { + atomic_fetch_sub(&phy->ref_count, 1); + } } static esp_err_t esp_eth_post_attach_driver_start(esp_netif_t * esp_netif, void * args) @@ -206,6 +214,8 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_ ETH_CHECK(eth_driver, "request memory for eth_driver failed", err, ESP_ERR_NO_MEM); eth_driver->mac = mac; eth_driver->phy = phy; + atomic_fetch_add(&phy->ref_count, 1); + atomic_fetch_add(&mac->ref_count, 1); eth_driver->link = ETH_LINK_DOWN; eth_driver->duplex = ETH_DUPLEX_HALF; eth_driver->speed = ETH_SPEED_10M; @@ -233,6 +243,8 @@ err_init_phy: mac->deinit(mac); err_init_mac: err_mediator: + atomic_fetch_sub(&phy->ref_count, 1); + atomic_fetch_sub(&mac->ref_count, 1); free(eth_driver); err: return ret; @@ -250,6 +262,8 @@ esp_err_t esp_eth_driver_uninstall(esp_eth_handle_t hdl) "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL); ETH_CHECK(phy->deinit(phy) == ESP_OK, "deinit phy failed", err, ESP_FAIL); ETH_CHECK(mac->deinit(mac) == ESP_OK, "deinit mac failed", err, ESP_FAIL); + atomic_fetch_sub(&phy->ref_count, 1); + atomic_fetch_sub(&mac->ref_count, 1); free(eth_driver); return ESP_OK; err: diff --git a/components/esp_eth/src/esp_eth_mac_dm9051.c b/components/esp_eth/src/esp_eth_mac_dm9051.c index 17bad75e54..b1dfadd30c 100644 --- a/components/esp_eth/src/esp_eth_mac_dm9051.c +++ b/components/esp_eth/src/esp_eth_mac_dm9051.c @@ -846,9 +846,11 @@ static esp_err_t emac_dm9051_deinit(esp_eth_mac_t *mac) static esp_err_t emac_dm9051_del(esp_eth_mac_t *mac) { emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent); - vTaskDelete(emac->rx_task_hdl); - vSemaphoreDelete(emac->spi_lock); - free(emac); + if (atomic_fetch_sub(&mac->ref_count, 1) == 1) { + vTaskDelete(emac->rx_task_hdl); + vSemaphoreDelete(emac->spi_lock); + free(emac); + } return ESP_OK; } @@ -877,6 +879,7 @@ esp_eth_mac_t *esp_eth_mac_new_dm9051(const eth_dm9051_config_t *dm9051_config, emac->parent.set_promiscuous = emac_dm9051_set_promiscuous; emac->parent.transmit = emac_dm9051_transmit; emac->parent.receive = emac_dm9051_receive; + atomic_init(&emac->parent.ref_count, 1); /* create mutex */ emac->spi_lock = xSemaphoreCreateMutex(); MAC_CHECK(emac->spi_lock, "create lock failed", err, NULL); diff --git a/components/esp_eth/src/esp_eth_mac_esp32.c b/components/esp_eth/src/esp_eth_mac_esp32.c index dc68238d42..d8cbc15004 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/components/esp_eth/src/esp_eth_mac_esp32.c @@ -339,17 +339,19 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac) static esp_err_t emac_esp32_del(esp_eth_mac_t *mac) { emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent); - esp_intr_free(emac->intr_hdl); - vTaskDelete(emac->rx_task_hdl); - int i = 0; - for (i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) { - free(emac->hal.rx_buf[i]); + if (atomic_fetch_sub(&mac->ref_count, 1) == 1) { + esp_intr_free(emac->intr_hdl); + vTaskDelete(emac->rx_task_hdl); + int i = 0; + for (i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) { + free(emac->hal.rx_buf[i]); + } + for (i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) { + free(emac->hal.tx_buf[i]); + } + free(emac->hal.descriptors); + free(emac); } - for (i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) { - free(emac->hal.tx_buf[i]); - } - free(emac->hal.descriptors); - free(emac); return ESP_OK; } @@ -408,6 +410,7 @@ esp_eth_mac_t *esp_eth_mac_new_esp32(const eth_mac_config_t *config) emac->parent.set_promiscuous = emac_esp32_set_promiscuous; emac->parent.transmit = emac_esp32_transmit; emac->parent.receive = emac_esp32_receive; + atomic_init(&emac->parent.ref_count, 1); /* 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, diff --git a/components/esp_eth/src/esp_eth_mac_openeth.c b/components/esp_eth/src/esp_eth_mac_openeth.c index e110e405bf..1c3afca349 100644 --- a/components/esp_eth/src/esp_eth_mac_openeth.c +++ b/components/esp_eth/src/esp_eth_mac_openeth.c @@ -66,7 +66,7 @@ static esp_err_t emac_opencores_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32 static IRAM_ATTR void emac_opencores_isr_handler(void *args) { - emac_opencores_t *emac = (emac_opencores_t*) args; + emac_opencores_t *emac = (emac_opencores_t *) args; BaseType_t high_task_wakeup; uint32_t status = REG_READ(OPENETH_INT_SOURCE_REG); @@ -94,7 +94,7 @@ static void emac_opencores_rx_task(void *arg) uint32_t length = 0; while (1) { if (ulTaskNotifyTake(pdFALSE, portMAX_DELAY)) { - while(true) { + while (true) { buffer = (uint8_t *)malloc(ETH_MAX_PACKET_SIZE); length = ETH_MAX_PACKET_SIZE; if (emac_opencores_receive(&emac->parent, buffer, &length) == ESP_OK) { @@ -243,7 +243,7 @@ static esp_err_t emac_opencores_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint3 while (bytes_remaining > 0) { uint32_t will_write = MIN(bytes_remaining, DMA_BUF_SIZE); memcpy(emac->tx_buf[emac->cur_tx_desc], buf, will_write); - openeth_tx_desc_t* desc_ptr = openeth_tx_desc(emac->cur_tx_desc); + openeth_tx_desc_t *desc_ptr = openeth_tx_desc(emac->cur_tx_desc); openeth_tx_desc_t desc_val = *desc_ptr; desc_val.wr = (emac->cur_tx_desc == TX_BUF_COUNT - 1); desc_val.len = will_write; @@ -294,7 +294,7 @@ static esp_err_t emac_opencores_init(esp_eth_mac_t *mac) esp_eth_mediator_t *eth = emac->eth; MAC_CHECK(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL) == ESP_OK, "lowlevel init failed", err, ESP_FAIL); MAC_CHECK(esp_read_mac(emac->addr, ESP_MAC_ETH) == ESP_OK, "fetch ethernet mac address failed", err, ESP_FAIL); - + // Sanity check if (REG_READ(OPENETH_MODER_REG) != OPENETH_MODER_DEFAULT) { ESP_LOGE(TAG, "CONFIG_ETH_USE_OPENETH should only be used when running in QEMU."); @@ -323,15 +323,17 @@ static esp_err_t emac_opencores_deinit(esp_eth_mac_t *mac) static esp_err_t emac_opencores_del(esp_eth_mac_t *mac) { emac_opencores_t *emac = __containerof(mac, emac_opencores_t, parent); - esp_intr_free(emac->intr_hdl); - vTaskDelete(emac->rx_task_hdl); - for (int i = 0; i < RX_BUF_COUNT; i++) { - free(emac->rx_buf[i]); + if (atomic_fetch_sub(&mac->ref_count, 1) == 1) { + esp_intr_free(emac->intr_hdl); + vTaskDelete(emac->rx_task_hdl); + for (int i = 0; i < RX_BUF_COUNT; i++) { + free(emac->rx_buf[i]); + } + for (int i = 0; i < TX_BUF_COUNT; i++) { + free(emac->tx_buf[i]); + } + free(emac); } - for (int i = 0; i < TX_BUF_COUNT; i++) { - free(emac->tx_buf[i]); - } - free(emac); return ESP_OK; } @@ -378,7 +380,8 @@ esp_eth_mac_t *esp_eth_mac_new_openeth(const eth_mac_config_t *config) emac->parent.set_promiscuous = emac_opencores_set_promiscuous; emac->parent.transmit = emac_opencores_transmit; emac->parent.receive = emac_opencores_receive; - + atomic_init(&emac->parent.ref_count, 1); + // Initialize the interrupt MAC_CHECK(esp_intr_alloc(OPENETH_INTR_SOURCE, ESP_INTR_FLAG_IRAM, emac_opencores_isr_handler, emac, &(emac->intr_hdl)) == ESP_OK, diff --git a/components/esp_eth/src/esp_eth_phy_dm9051.c b/components/esp_eth/src/esp_eth_phy_dm9051.c index 070a800500..a04a5568bd 100644 --- a/components/esp_eth/src/esp_eth_phy_dm9051.c +++ b/components/esp_eth/src/esp_eth_phy_dm9051.c @@ -261,7 +261,9 @@ err: static esp_err_t dm9051_del(esp_eth_phy_t *phy) { phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent); - free(dm9051); + if (atomic_fetch_sub(&phy->ref_count, 1) == 1) { + free(dm9051); + } return ESP_OK; } @@ -316,6 +318,8 @@ 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; + atomic_init(&dm9051->parent.ref_count, 1); + return &(dm9051->parent); err: return NULL; diff --git a/components/esp_eth/src/esp_eth_phy_dp83848.c b/components/esp_eth/src/esp_eth_phy_dp83848.c index f889a47ddf..5f98fc0442 100644 --- a/components/esp_eth/src/esp_eth_phy_dp83848.c +++ b/components/esp_eth/src/esp_eth_phy_dp83848.c @@ -260,7 +260,9 @@ err: static esp_err_t dp83848_del(esp_eth_phy_t *phy) { phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent); - free(dp83848); + if (atomic_fetch_sub(&phy->ref_count, 1) == 1) { + free(dp83848); + } return ESP_OK; } @@ -314,6 +316,7 @@ esp_eth_phy_t *esp_eth_phy_new_dp83848(const eth_phy_config_t *config) dp83848->parent.get_addr = dp83848_get_addr; dp83848->parent.set_addr = dp83848_set_addr; dp83848->parent.del = dp83848_del; + atomic_init(&dp83848->parent.ref_count, 1); return &(dp83848->parent); err: diff --git a/components/esp_eth/src/esp_eth_phy_ip101.c b/components/esp_eth/src/esp_eth_phy_ip101.c index 93312f2ae9..9c6dc5d06b 100644 --- a/components/esp_eth/src/esp_eth_phy_ip101.c +++ b/components/esp_eth/src/esp_eth_phy_ip101.c @@ -297,7 +297,9 @@ err: static esp_err_t ip101_del(esp_eth_phy_t *phy) { phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent); - free(ip101); + if (atomic_fetch_sub(&phy->ref_count, 1) == 1) { + free(ip101); + } return ESP_OK; } @@ -348,6 +350,7 @@ esp_eth_phy_t *esp_eth_phy_new_ip101(const eth_phy_config_t *config) ip101->parent.get_addr = ip101_get_addr; ip101->parent.set_addr = ip101_set_addr; ip101->parent.del = ip101_del; + atomic_init(&ip101->parent.ref_count, 1); return &(ip101->parent); err: diff --git a/components/esp_eth/src/esp_eth_phy_lan8720.c b/components/esp_eth/src/esp_eth_phy_lan8720.c index b685d136be..1afbcdbfb3 100644 --- a/components/esp_eth/src/esp_eth_phy_lan8720.c +++ b/components/esp_eth/src/esp_eth_phy_lan8720.c @@ -341,7 +341,9 @@ err: static esp_err_t lan8720_del(esp_eth_phy_t *phy) { phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent); - free(lan8720); + if (atomic_fetch_sub(&phy->ref_count, 1) == 1) { + free(lan8720); + } return ESP_OK; } @@ -394,6 +396,7 @@ esp_eth_phy_t *esp_eth_phy_new_lan8720(const eth_phy_config_t *config) lan8720->parent.get_addr = lan8720_get_addr; lan8720->parent.set_addr = lan8720_set_addr; lan8720->parent.del = lan8720_del; + atomic_init(&lan8720->parent.ref_count, 1); return &(lan8720->parent); err: diff --git a/components/esp_eth/src/esp_eth_phy_rtl8201.c b/components/esp_eth/src/esp_eth_phy_rtl8201.c index 874449c67c..95152acb72 100644 --- a/components/esp_eth/src/esp_eth_phy_rtl8201.c +++ b/components/esp_eth/src/esp_eth_phy_rtl8201.c @@ -248,7 +248,9 @@ err: static esp_err_t rtl8201_del(esp_eth_phy_t *phy) { phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent); - free(rtl8201); + if (atomic_fetch_sub(&phy->ref_count, 1) == 1) { + free(rtl8201); + } return ESP_OK; } @@ -302,6 +304,7 @@ esp_eth_phy_t *esp_eth_phy_new_rtl8201(const eth_phy_config_t *config) rtl8201->parent.get_addr = rtl8201_get_addr; rtl8201->parent.set_addr = rtl8201_set_addr; rtl8201->parent.del = rtl8201_del; + atomic_init(&rtl8201->parent.ref_count, 1); return &(rtl8201->parent); err: diff --git a/components/esp_eth/test/test_emac.c b/components/esp_eth/test/test_emac.c index 96e10c50db..51378c6da9 100644 --- a/components/esp_eth/test/test_emac.c +++ b/components/esp_eth/test/test_emac.c @@ -170,8 +170,6 @@ TEST_CASE("esp32 ethernet event test", "[ethernet][test_env=UT_T2_Ethernet]") /* wait for connection stop */ bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(ETH_STOP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); - // "check link timer callback" might owned the reference of phy object, make sure it has release it - vTaskDelay(pdMS_TO_TICKS(2000)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); TEST_ESP_OK(esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, eth_event_handler)); @@ -209,8 +207,6 @@ TEST_CASE("esp32 ethernet dhcp test", "[ethernet][test_env=UT_T2_Ethernet]") /* wait for connection stop */ bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(ETH_STOP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); - // "check link timer callback" might owned the reference of phy object, make sure it has release it - vTaskDelay(pdMS_TO_TICKS(2000)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); TEST_ESP_OK(esp_event_handler_unregister(IP_EVENT, IP_EVENT_ETH_GOT_IP, got_ip_event_handler)); @@ -292,8 +288,6 @@ TEST_CASE("esp32 ethernet icmp test", "[ethernet][test_env=UT_T2_Ethernet]") /* wait for connection stop */ bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(ETH_STOP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); - // "check link timer callback" might owned the reference of phy object, make sure it has release it - vTaskDelay(pdMS_TO_TICKS(2000)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); TEST_ESP_OK(esp_event_handler_unregister(IP_EVENT, IP_EVENT_ETH_GOT_IP, got_ip_event_handler));