From 3c8bab02851d9b72e9ec43f80af69e812a7ac14f Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Mon, 20 Dec 2021 12:45:24 +0100 Subject: [PATCH 1/2] Fixed ESP32 EMAC driver `insufficient TX buffer size` which could followed esp_eth_stop and esp_eth_start sequence --- components/esp_eth/src/esp_eth.c | 7 +++++ components/esp_eth/src/esp_eth_mac_esp32.c | 15 ++++++++-- components/hal/esp32/emac_hal.c | 17 ++++++++--- components/hal/esp32/include/hal/emac.h | 35 +++++++++++++--------- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 307f51b7b5..55af49fd13 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -323,6 +323,13 @@ esp_err_t esp_eth_transmit(esp_eth_handle_t hdl, void *buf, size_t length) { esp_err_t ret = ESP_OK; esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl; + + if (atomic_load(ð_driver->fsm) != ESP_ETH_FSM_START) { + ret = ESP_ERR_INVALID_STATE; + ESP_LOGD(TAG, "Ethernet is not started"); + goto err; + } + ETH_CHECK(buf, "can't set buf to null", err, ESP_ERR_INVALID_ARG); ETH_CHECK(length, "buf length can't be zero", err, ESP_ERR_INVALID_ARG); ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG); diff --git a/components/esp_eth/src/esp_eth_mac_esp32.c b/components/esp_eth/src/esp_eth_mac_esp32.c index 9569a7fabe..5ded2fbcec 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/components/esp_eth/src/esp_eth_mac_esp32.c @@ -47,7 +47,7 @@ static const char *TAG = "emac_esp32"; } while (0) #define PHY_OPERATION_TIMEOUT_US (1000) - +#define MAC_STOP_TIMEOUT_MS (100) #define FLOW_CONTROL_LOW_WATER_MARK (CONFIG_ETH_DMA_RX_BUFFER_NUM / 3) #define FLOW_CONTROL_HIGH_WATER_MARK (FLOW_CONTROL_LOW_WATER_MARK * 2) @@ -388,6 +388,7 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac) static esp_err_t emac_esp32_start(esp_eth_mac_t *mac) { emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent); + emac_hal_reset_desc_chain(&emac->hal); emac_hal_start(&emac->hal); return ESP_OK; } @@ -395,8 +396,16 @@ static esp_err_t emac_esp32_start(esp_eth_mac_t *mac) 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; + esp_err_t ret = ESP_OK; + int32_t to = 0; + do { + if ((ret = emac_hal_stop(&emac->hal)) == ESP_OK) { + break; + } + to += 20; + vTaskDelay(pdMS_TO_TICKS(20)); + } while (to < MAC_STOP_TIMEOUT_MS); + return ret; } static esp_err_t emac_esp32_del(esp_eth_mac_t *mac) diff --git a/components/hal/esp32/emac_hal.c b/components/hal/esp32/emac_hal.c index 5ba3c4aec9..cdc2b4c514 100644 --- a/components/hal/esp32/emac_hal.c +++ b/components/hal/esp32/emac_hal.c @@ -184,6 +184,8 @@ void emac_hal_reset_desc_chain(emac_hal_context_t *hal) /* init tx chain */ for (int i = 0; i < CONFIG_ETH_DMA_TX_BUFFER_NUM; i++) { + /* Set Own bit of the Tx descriptor Status: CPU */ + hal->tx_desc[i].TDES0.Own = 0; /* Set Second Address Chained bit */ hal->tx_desc[i].TDES0.SecondAddressChained = 1; hal->tx_desc[i].TDES1.TransmitBuffer1Size = CONFIG_ETH_DMA_BUFFER_SIZE; @@ -427,27 +429,34 @@ void emac_hal_start(emac_hal_context_t *hal) hal->dma_regs->dmastatus.val = 0xFFFFFFFF; } -void emac_hal_stop(emac_hal_context_t *hal) +esp_err_t emac_hal_stop(emac_hal_context_t *hal) { typeof(hal->dma_regs->dmaoperation_mode) opm = hal->dma_regs->dmaoperation_mode; typeof(hal->mac_regs->gmacconfig) cfg = hal->mac_regs->gmacconfig; - /* Flush Transmit FIFO */ - opm.flush_tx_fifo = 1; /* Stop DMA transmission */ opm.start_stop_transmission_command = 0; /* Stop DMA reception */ opm.start_stop_rx = 0; + + hal->dma_regs->dmaoperation_mode = opm; + + if (hal->mac_regs->emacdebug.mactfcs != 0x0) { + /* Previous transmit in progress */ + return ESP_ERR_INVALID_STATE; + } + /* Disable receive state machine of the MAC for reception from the MII */ cfg.rx = 0; /* Disable transmit state machine of the MAC for transmission on the MII */ cfg.tx = 0; - hal->dma_regs->dmaoperation_mode = opm; hal->mac_regs->gmacconfig = cfg; /* Disable Ethernet MAC and DMA Interrupt */ hal->dma_regs->dmain_en.val = 0x0; + + return ESP_OK; } uint32_t emac_hal_get_tx_desc_owner(emac_hal_context_t *hal) diff --git a/components/hal/esp32/include/hal/emac.h b/components/hal/esp32/include/hal/emac.h index ec0c1ab3b9..81708549ba 100644 --- a/components/hal/esp32/include/hal/emac.h +++ b/components/hal/esp32/include/hal/emac.h @@ -1,16 +1,8 @@ -// Copyright 2019 Espressif Systems (Shanghai) PTE LTD -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +/* + * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ #pragma once @@ -381,9 +373,24 @@ uint32_t emac_hal_get_phy_data(emac_hal_context_t *hal); void emac_hal_set_address(emac_hal_context_t *hal, uint8_t *mac_addr); + +/** + * @brief Starts EMAC Transmission & Reception + * + * @param hal EMAC HAL context infostructure + */ void emac_hal_start(emac_hal_context_t *hal); -void emac_hal_stop(emac_hal_context_t *hal); +/** + * @brief Stops EMAC Transmission & Reception + * + * @param hal EMAC HAL context infostructure + * @return + * - ESP_OK: succeed + * - ESP_ERR_INVALID_STATE: previous frame transmission is not completed. When this error occurs, + * wait and reapeat the EMAC stop again. + */ +esp_err_t emac_hal_stop(emac_hal_context_t *hal); uint32_t emac_hal_get_tx_desc_owner(emac_hal_context_t *hal); From 8f1f424390d3ec8e17bfc76640c9c73663ddd6bf Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Mon, 7 Mar 2022 08:36:28 +0100 Subject: [PATCH 2/2] esp_eth: EMAC start/stop optimization --- components/esp_eth/src/esp_eth.c | 2 -- components/esp_eth/src/esp_eth_mac_esp32.c | 14 ++++++++------ components/hal/esp32/emac_hal.c | 17 ++++++++++------- components/hal/esp32/include/hal/emac.h | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 55af49fd13..48a25093db 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -262,7 +262,6 @@ 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); esp_eth_phy_t *phy = eth_driver->phy; - esp_eth_mac_t *mac = eth_driver->mac; // check if driver has started esp_eth_fsm_t expected_fsm = ESP_ETH_FSM_STOP; if (!atomic_compare_exchange_strong(ð_driver->fsm, &expected_fsm, ESP_ETH_FSM_START)) { @@ -271,7 +270,6 @@ esp_err_t esp_eth_start(esp_eth_handle_t hdl) goto err; } ETH_CHECK(phy->negotiate(phy) == ESP_OK, "phy negotiation failed", err, ESP_FAIL); - ETH_CHECK(mac->start(mac) == ESP_OK, "start mac failed", err, ESP_FAIL); ETH_CHECK(esp_event_post(ETH_EVENT, ETHERNET_EVENT_START, ð_driver, sizeof(esp_eth_driver_t *), 0) == ESP_OK, "send ETHERNET_EVENT_START event failed", err, ESP_FAIL); ETH_CHECK(phy->get_link(phy) == ESP_OK, "phy get link status failed", err, ESP_FAIL); diff --git a/components/esp_eth/src/esp_eth_mac_esp32.c b/components/esp_eth/src/esp_eth_mac_esp32.c index 5ded2fbcec..e72d6897d2 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/components/esp_eth/src/esp_eth_mac_esp32.c @@ -47,7 +47,7 @@ static const char *TAG = "emac_esp32"; } while (0) #define PHY_OPERATION_TIMEOUT_US (1000) -#define MAC_STOP_TIMEOUT_MS (100) +#define MAC_STOP_TIMEOUT_US (250) #define FLOW_CONTROL_LOW_WATER_MARK (CONFIG_ETH_DMA_RX_BUFFER_NUM / 3) #define FLOW_CONTROL_HIGH_WATER_MARK (FLOW_CONTROL_LOW_WATER_MARK * 2) @@ -77,6 +77,8 @@ typedef struct { static esp_err_t esp_emac_alloc_driver_obj(const eth_mac_config_t *config, emac_esp32_t **emac_out_hdl, void **out_descriptors); static void esp_emac_free_driver_obj(emac_esp32_t *emac, void *descriptors); +static esp_err_t emac_esp32_start(esp_eth_mac_t *mac); +static esp_err_t emac_esp32_stop(esp_eth_mac_t *mac); static esp_err_t emac_esp32_set_mediator(esp_eth_mac_t *mac, esp_eth_mediator_t *eth) { @@ -163,11 +165,11 @@ static esp_err_t emac_esp32_set_link(esp_eth_mac_t *mac, eth_link_t link) switch (link) { case ETH_LINK_UP: MAC_CHECK(esp_intr_enable(emac->intr_hdl) == ESP_OK, "enable interrupt failed", err, ESP_FAIL); - emac_hal_start(&emac->hal); + emac_esp32_start(mac); break; case ETH_LINK_DOWN: MAC_CHECK(esp_intr_disable(emac->intr_hdl) == ESP_OK, "disable interrupt failed", err, ESP_FAIL); - emac_hal_stop(&emac->hal); + emac_esp32_stop(mac); break; default: MAC_CHECK(false, "unknown link status", err, ESP_ERR_INVALID_ARG); @@ -402,9 +404,9 @@ static esp_err_t emac_esp32_stop(esp_eth_mac_t *mac) if ((ret = emac_hal_stop(&emac->hal)) == ESP_OK) { break; } - to += 20; - vTaskDelay(pdMS_TO_TICKS(20)); - } while (to < MAC_STOP_TIMEOUT_MS); + to += 25; + esp_rom_delay_us(25); + } while (to < MAC_STOP_TIMEOUT_US); return ret; } diff --git a/components/hal/esp32/emac_hal.c b/components/hal/esp32/emac_hal.c index cdc2b4c514..4484eeb24e 100644 --- a/components/hal/esp32/emac_hal.c +++ b/components/hal/esp32/emac_hal.c @@ -436,22 +436,25 @@ esp_err_t emac_hal_stop(emac_hal_context_t *hal) /* Stop DMA transmission */ opm.start_stop_transmission_command = 0; - /* Stop DMA reception */ - opm.start_stop_rx = 0; - hal->dma_regs->dmaoperation_mode = opm; - if (hal->mac_regs->emacdebug.mactfcs != 0x0) { /* Previous transmit in progress */ return ESP_ERR_INVALID_STATE; } + /* Disable transmit state machine of the MAC for transmission on the MII */ + cfg.tx = 0; + hal->mac_regs->gmacconfig = cfg; /* Disable receive state machine of the MAC for reception from the MII */ cfg.rx = 0; - /* Disable transmit state machine of the MAC for transmission on the MII */ - cfg.tx = 0; - hal->mac_regs->gmacconfig = cfg; + if (hal->mac_regs->emacdebug.mtlrfrcs != 0x0) { + /* Previous receive copy in progress */ + return ESP_ERR_INVALID_STATE; + } + /* Stop DMA reception */ + opm.start_stop_rx = 0; + hal->dma_regs->dmaoperation_mode = opm; /* Disable Ethernet MAC and DMA Interrupt */ hal->dma_regs->dmain_en.val = 0x0; diff --git a/components/hal/esp32/include/hal/emac.h b/components/hal/esp32/include/hal/emac.h index 81708549ba..d1f0d712cb 100644 --- a/components/hal/esp32/include/hal/emac.h +++ b/components/hal/esp32/include/hal/emac.h @@ -387,7 +387,7 @@ void emac_hal_start(emac_hal_context_t *hal); * @param hal EMAC HAL context infostructure * @return * - ESP_OK: succeed - * - ESP_ERR_INVALID_STATE: previous frame transmission is not completed. When this error occurs, + * - ESP_ERR_INVALID_STATE: previous frame transmission/reception is not completed. When this error occurs, * wait and reapeat the EMAC stop again. */ esp_err_t emac_hal_stop(emac_hal_context_t *hal);