From eec6de57ff9fc69690dac5574da229c85ac1c6e3 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 9 Nov 2018 17:33:44 +0800 Subject: [PATCH] ethernet: multi-call failure in esp_eth_init Because of incomplete state machine, ethernet driver will broken if esp_eth_init is called twice. Detailed information here: https://ezredmine.espressif.cn:8765/issues/27332 --- components/ethernet/Kconfig | 2 +- components/ethernet/emac_main.c | 51 +++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/components/ethernet/Kconfig b/components/ethernet/Kconfig index 4657b53d4..d379a5598 100644 --- a/components/ethernet/Kconfig +++ b/components/ethernet/Kconfig @@ -21,7 +21,7 @@ config DMA_TX_BUF_NUM config EMAC_L2_TO_L3_RX_BUF_MODE bool "Enable copy between Layer2 and Layer3" - default n + default y help If this options is selected, a copy of each received buffer will be created when passing it from the Ethernet MAC (L2) to the IP stack (L3). Otherwise, IP stack diff --git a/components/ethernet/emac_main.c b/components/ethernet/emac_main.c index de6db0f9f..b70682b56 100644 --- a/components/ethernet/emac_main.c +++ b/components/ethernet/emac_main.c @@ -1066,8 +1066,19 @@ esp_err_t IRAM_ATTR emac_post(emac_sig_t sig, emac_par_t par) } esp_err_t esp_eth_init(eth_config_t *config) +{ + esp_event_set_default_eth_handlers(); + return esp_eth_init_internal(config); +} + +esp_err_t esp_eth_init_internal(eth_config_t *config) { int i = 0; + esp_err_t ret = ESP_OK; + if (emac_config.emac_status != EMAC_RUNTIME_NOT_INIT) { + goto _initialised; + } + /* dynamically alloc memory for ethernet dma */ emac_dma_rx_chain_buf = (dma_extended_desc_t *)heap_caps_malloc(sizeof(dma_extended_desc_t) * DMA_RX_BUF_NUM, MALLOC_CAP_DMA); emac_dma_tx_chain_buf = (dma_extended_desc_t *)heap_caps_malloc(sizeof(dma_extended_desc_t) * DMA_TX_BUF_NUM, MALLOC_CAP_DMA); @@ -1077,16 +1088,6 @@ esp_err_t esp_eth_init(eth_config_t *config) for (i = 0; i < DMA_TX_BUF_NUM; i++) { emac_dma_tx_buf[i] = (uint8_t *)heap_caps_malloc(DMA_TX_BUF_SIZE, MALLOC_CAP_DMA); } - esp_event_set_default_eth_handlers(); - return esp_eth_init_internal(config); -} - -esp_err_t esp_eth_init_internal(eth_config_t *config) -{ - esp_err_t ret = ESP_OK; - if (emac_config.emac_status != EMAC_RUNTIME_NOT_INIT) { - goto _exit; - } emac_init_default_data(); @@ -1097,7 +1098,7 @@ esp_err_t esp_eth_init_internal(eth_config_t *config) ret = emac_verify_args(); if (ret != ESP_OK) { - goto _exit; + goto _verify_err; } emac_config.emac_phy_power_enable(true); @@ -1110,7 +1111,7 @@ esp_err_t esp_eth_init_internal(eth_config_t *config) if (esp_spiram_is_initialized()) { ESP_LOGE(TAG, "GPIO16 and GPIO17 has been occupied by PSRAM, Only ETH_CLOCK_GPIO_IN is supported!"); ret = ESP_FAIL; - goto _exit; + goto _verify_err; } else { ESP_LOGW(TAG, "GPIO16/17 is used for clock of EMAC, Please Make Sure you're not using PSRAM."); } @@ -1173,15 +1174,36 @@ esp_err_t esp_eth_init_internal(eth_config_t *config) emac_config.emac_status = EMAC_RUNTIME_INIT; -_exit: + return ESP_OK; + +_verify_err: + free(emac_dma_rx_chain_buf); + free(emac_dma_tx_chain_buf); + emac_dma_rx_chain_buf = NULL; + emac_dma_tx_chain_buf = NULL; + for (i = 0; i < DMA_RX_BUF_NUM; i++) { + free(emac_dma_rx_buf[i]); + emac_dma_rx_buf[i] = NULL; + } + for (i = 0; i < DMA_TX_BUF_NUM; i++) { + free(emac_dma_tx_buf[i]); + emac_dma_tx_buf[i] = NULL; + } +_initialised: return ret; } esp_err_t esp_eth_deinit(void) { - esp_err_t ret = ESP_FAIL; + esp_err_t ret = ESP_OK; int i = 0; + if (emac_config.emac_status == EMAC_RUNTIME_NOT_INIT) { + goto _exit; + } + if (emac_config.emac_status == EMAC_RUNTIME_START) { + esp_eth_disable(); + } if (!emac_task_hdl) { ret = ESP_ERR_INVALID_STATE; goto _exit; @@ -1213,7 +1235,6 @@ esp_err_t esp_eth_deinit(void) emac_dma_tx_buf[i] = NULL; } esp_intr_free(eth_intr_handle); - ret = ESP_OK; _exit: return ret; }