From 57ef88a91f00f215fe5c21acc78ae36fd49a61c7 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 29 Nov 2019 14:49:02 +0800 Subject: [PATCH 1/3] ethernet: add pm lock --- components/esp_eth/src/esp_eth_mac_esp32.c | 24 +++++++++++++++++++ .../api-reference/system/power_management.rst | 2 +- .../api-reference/system/power_management.rst | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/components/esp_eth/src/esp_eth_mac_esp32.c b/components/esp_eth/src/esp_eth_mac_esp32.c index 03a5b53bb..7f0ffdd4b 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/components/esp_eth/src/esp_eth_mac_esp32.c @@ -19,6 +19,7 @@ #include "esp_attr.h" #include "esp_log.h" #include "esp_eth.h" +#include "esp_pm.h" #include "esp_system.h" #include "esp_heap_caps.h" #include "esp_intr_alloc.h" @@ -57,6 +58,9 @@ typedef struct { uint8_t *rx_buf[CONFIG_ETH_DMA_RX_BUFFER_NUM]; uint8_t *tx_buf[CONFIG_ETH_DMA_TX_BUFFER_NUM]; bool isr_need_yield; +#ifdef CONFIG_PM_ENABLE + esp_pm_lock_handle_t pm_lock; +#endif } emac_esp32_t; static esp_err_t emac_esp32_set_mediator(esp_eth_mac_t *mac, esp_eth_mediator_t *eth) @@ -314,6 +318,9 @@ static esp_err_t emac_esp32_init(esp_eth_mac_t *mac) MAC_CHECK(esp_read_mac(emac->addr, ESP_MAC_ETH) == ESP_OK, "fetch ethernet mac address failed", err, ESP_FAIL); /* set MAC address to emac register */ emac_hal_set_address(&emac->hal, emac->addr); +#ifdef CONFIG_PM_ENABLE + esp_pm_lock_acquire(emac->pm_lock); +#endif return ESP_OK; err: eth->on_state_changed(eth, ETH_STATE_DEINIT, NULL); @@ -325,6 +332,9 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac) { emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent); esp_eth_mediator_t *eth = emac->eth; +#ifdef CONFIG_PM_ENABLE + esp_pm_lock_release(emac->pm_lock); +#endif emac_hal_stop(&emac->hal); eth->on_state_changed(eth, ETH_STATE_DEINIT, NULL); periph_module_disable(PERIPH_EMAC_MODULE); @@ -335,6 +345,11 @@ 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); +#ifdef CONFIG_PM_ENABLE + if (emac->pm_lock) { + esp_pm_lock_delete(emac->pm_lock); + } +#endif vTaskDelete(emac->rx_task_hdl); int i = 0; for (i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) { @@ -409,6 +424,10 @@ esp_eth_mac_t *esp_eth_mac_new_esp32(const eth_mac_config_t *config) 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, NULL); +#ifdef CONFIG_PM_ENABLE + MAC_CHECK(esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "emac_esp32", &emac->pm_lock) == ESP_OK, + "create pm lock failed", err, NULL); +#endif /* 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); @@ -429,6 +448,11 @@ err: for (int i = 0; i < CONFIG_ETH_DMA_RX_BUFFER_NUM; i++) { free(emac->rx_buf[i]); } +#ifdef CONFIG_PM_ENABLE + if (emac->pm_lock) { + esp_pm_lock_delete(emac->pm_lock); + } +#endif free(emac); } if (descriptors) { diff --git a/docs/en/api-reference/system/power_management.rst b/docs/en/api-reference/system/power_management.rst index a10b9b03b..6a965f47f 100644 --- a/docs/en/api-reference/system/power_management.rst +++ b/docs/en/api-reference/system/power_management.rst @@ -125,7 +125,7 @@ Currently, the following peripheral drivers are aware of DFS and will use the `` The following drivers will hold the ``ESP_PM_APB_FREQ_MAX`` lock while the driver is enabled: - **SPI slave**: between calls to :cpp:func:`spi_slave_initialize` and :cpp:func:`spi_slave_free`. -- **Ethernet**: between calls to :cpp:func:`esp_eth_enable` and :cpp:func:`esp_eth_disable`. +- **Ethernet**: between calls to :cpp:func:`esp_eth_driver_install` and :cpp:func:`esp_eth_driver_uninstall`. - **WiFi**: between calls to :cpp:func:`esp_wifi_start` and :cpp:func:`esp_wifi_stop`. If modem sleep is enabled, the lock will be released for the periods of time when radio is disabled. - **Bluetooth**: between calls to :cpp:func:`esp_bt_controller_enable` and :cpp:func:`esp_bt_controller_disable`. If Bluetooth modem sleep is enabled, the ``ESP_PM_APB_FREQ_MAX`` lock will be released for the periods of time when radio is disabled. However the ``ESP_PM_NO_LIGHT_SLEEP`` lock will still be held, unless :ref:`CONFIG_BTDM_LOW_POWER_CLOCK` option is set to "External 32kHz crystal". - **CAN**: between calls to :cpp:func:`can_driver_install` and :cpp:func:`can_driver_uninstall`. diff --git a/docs/zh_CN/api-reference/system/power_management.rst b/docs/zh_CN/api-reference/system/power_management.rst index a37a97be4..350020e87 100644 --- a/docs/zh_CN/api-reference/system/power_management.rst +++ b/docs/zh_CN/api-reference/system/power_management.rst @@ -111,7 +111,7 @@ ESP32 支持下表中所述的三种电源管理锁。 启用以下驱动程序时，将占用 ``ESP_PM_APB_FREQ_MAX`` 锁： - **SPI slave**：从调用 :cpp:func:`spi_slave_initialize` 至 :cpp:func:`spi_slave_free` 期间。 -- **Ethernet**：从调用 :cpp:func:`esp_eth_enable` 至 :cpp:func:`esp_eth_disable` 期间。 +- **Ethernet**：从调用 :cpp:func:`esp_eth_driver_install` 至 :cpp:func:`esp_eth_driver_uninstall` 期间。 - **WiFi**：从调用 :cpp:func:`esp_wifi_start` 至 :cpp:func:`esp_wifi_stop` 期间。如果启用了调制解调器睡眠模式，广播关闭时将释放此管理锁。 - **Bluetooth**：从调用 :cpp:func:`esp_bt_controller_enable` 至 :cpp:func:`esp_bt_controller_disable` 期间。如果启用了蓝牙调制解调器，广播关闭时将释放此管理锁。但依然占用 ``ESP_PM_NO_LIGHT_SLEEP`` 锁。 - **CAN**：从调用 :cpp:func:`can_driver_install` 至 :cpp:func:`can_driver_uninstall` 期间。 From ac11545e0a6456ad0db2fd01586b0556726d8377 Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 3 Dec 2019 15:37:34 +0800 Subject: [PATCH 2/3] ethernet: warning when double start/stop --- components/esp_eth/include/esp_eth.h | 2 ++ components/esp_eth/src/esp_eth.c | 27 ++++++++++++++++--- components/esp_eth/src/esp_eth_phy_dm9051.c | 1 + components/esp_eth/src/esp_eth_phy_dp83848.c | 1 + components/esp_eth/src/esp_eth_phy_ip101.c | 1 + components/esp_eth/src/esp_eth_phy_lan8720.c | 1 + components/esp_eth/src/esp_eth_phy_rtl8201.c | 1 + .../tcpip_adapter/tcpip_adapter_compat.c | 1 - .../protocol_examples_common/connect.c | 1 + 9 files changed, 31 insertions(+), 5 deletions(-) diff --git a/components/esp_eth/include/esp_eth.h b/components/esp_eth/include/esp_eth.h index db0124eaf..34ee91594 100644 --- a/components/esp_eth/include/esp_eth.h +++ b/components/esp_eth/include/esp_eth.h @@ -142,6 +142,7 @@ esp_err_t esp_eth_driver_uninstall(esp_eth_handle_t hdl); * @return * - ESP_OK: start esp_eth driver successfully * - ESP_ERR_INVALID_ARG: start esp_eth driver failed because of some invalid argument +* - ESP_ERR_INVALID_STATE: start esp_eth driver failed because driver has started already * - ESP_FAIL: start esp_eth driver failed because some other error occurred */ esp_err_t esp_eth_start(esp_eth_handle_t hdl); @@ -155,6 +156,7 @@ esp_err_t esp_eth_start(esp_eth_handle_t hdl); * @return * - ESP_OK: stop esp_eth driver successfully * - ESP_ERR_INVALID_ARG: stop esp_eth driver failed because of some invalid argument +* - ESP_ERR_INVALID_STATE: stop esp_eth driver failed because driver has not started yet * - ESP_FAIL: stop esp_eth driver failed because some other error occurred */ esp_err_t esp_eth_stop(esp_eth_handle_t hdl); diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 8b1f90e32..1c87b8a60 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -34,6 +34,8 @@ static const char *TAG = "esp_eth"; ESP_EVENT_DEFINE_BASE(ETH_EVENT); +#define ESP_ETH_FLAGS_STARTED (1<<0) + /** * @brief The Ethernet driver mainly consists of PHY, MAC and * the mediator who will handle the request/response from/to MAC, PHY and Users. @@ -54,6 +56,7 @@ typedef struct { eth_link_t link; atomic_int ref_count; void *priv; + uint32_t flags; esp_err_t (*stack_input)(esp_eth_handle_t eth_handle, uint8_t *buffer, uint32_t length, void *priv); 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); @@ -236,6 +239,14 @@ esp_err_t esp_eth_start(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 + if (eth_driver->flags & ESP_ETH_FLAGS_STARTED) { + 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); ETH_CHECK(esp_event_post(ETH_EVENT, ETHERNET_EVENT_START, ð_driver, sizeof(eth_driver), 0) == ESP_OK, @@ -252,10 +263,18 @@ esp_err_t esp_eth_stop(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); - 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, ð_driver, sizeof(eth_driver), 0) == ESP_OK, - "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL); + // 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, ð_driver, sizeof(eth_driver), 0) == ESP_OK, + "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL); + } else { + ESP_LOGW(TAG, "driver not started yet"); + ret = ESP_ERR_INVALID_STATE; + goto err; + } return ESP_OK; err: return ret; diff --git a/components/esp_eth/src/esp_eth_phy_dm9051.c b/components/esp_eth/src/esp_eth_phy_dm9051.c index 655967a4b..c3775f204 100644 --- a/components/esp_eth/src/esp_eth_phy_dm9051.c +++ b/components/esp_eth/src/esp_eth_phy_dm9051.c @@ -150,6 +150,7 @@ err: static esp_err_t dm9051_reset(esp_eth_phy_t *phy) { phy_dm9051_t *dm9051 = __containerof(phy, phy_dm9051_t, parent); + dm9051->link_status = ETH_LINK_DOWN; 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, diff --git a/components/esp_eth/src/esp_eth_phy_dp83848.c b/components/esp_eth/src/esp_eth_phy_dp83848.c index 0c1cd99cd..fbb41391a 100644 --- a/components/esp_eth/src/esp_eth_phy_dp83848.c +++ b/components/esp_eth/src/esp_eth_phy_dp83848.c @@ -156,6 +156,7 @@ err: static esp_err_t dp83848_reset(esp_eth_phy_t *phy) { phy_dp83848_t *dp83848 = __containerof(phy, phy_dp83848_t, parent); + dp83848->link_status = ETH_LINK_DOWN; 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, diff --git a/components/esp_eth/src/esp_eth_phy_ip101.c b/components/esp_eth/src/esp_eth_phy_ip101.c index 307dca6eb..6b16c2188 100644 --- a/components/esp_eth/src/esp_eth_phy_ip101.c +++ b/components/esp_eth/src/esp_eth_phy_ip101.c @@ -196,6 +196,7 @@ err: static esp_err_t ip101_reset(esp_eth_phy_t *phy) { phy_ip101_t *ip101 = __containerof(phy, phy_ip101_t, parent); + ip101->link_status = ETH_LINK_DOWN; 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, diff --git a/components/esp_eth/src/esp_eth_phy_lan8720.c b/components/esp_eth/src/esp_eth_phy_lan8720.c index 82ae78b1d..d7d62bb78 100644 --- a/components/esp_eth/src/esp_eth_phy_lan8720.c +++ b/components/esp_eth/src/esp_eth_phy_lan8720.c @@ -238,6 +238,7 @@ err: static esp_err_t lan8720_reset(esp_eth_phy_t *phy) { phy_lan8720_t *lan8720 = __containerof(phy, phy_lan8720_t, parent); + lan8720->link_status = ETH_LINK_DOWN; 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, diff --git a/components/esp_eth/src/esp_eth_phy_rtl8201.c b/components/esp_eth/src/esp_eth_phy_rtl8201.c index 7ec624f82..814563fdc 100644 --- a/components/esp_eth/src/esp_eth_phy_rtl8201.c +++ b/components/esp_eth/src/esp_eth_phy_rtl8201.c @@ -147,6 +147,7 @@ err: static esp_err_t rtl8201_reset(esp_eth_phy_t *phy) { phy_rtl8201_t *rtl8201 = __containerof(phy, phy_rtl8201_t, parent); + rtl8201->link_status = ETH_LINK_DOWN; 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, diff --git a/components/tcpip_adapter/tcpip_adapter_compat.c b/components/tcpip_adapter/tcpip_adapter_compat.c index 1d2dee5b1..7a81fbb13 100644 --- a/components/tcpip_adapter/tcpip_adapter_compat.c +++ b/components/tcpip_adapter/tcpip_adapter_compat.c @@ -133,7 +133,6 @@ esp_err_t tcpip_adapter_compat_start_eth(void *eth_driver) if (esp_netif) { esp_netif_attach(esp_netif, esp_eth_new_netif_glue(eth_driver)); } - esp_eth_start(eth_driver); } return ESP_OK; } diff --git a/examples/common_components/protocol_examples_common/connect.c b/examples/common_components/protocol_examples_common/connect.c index f33aa7051..b3765d878 100644 --- a/examples/common_components/protocol_examples_common/connect.c +++ b/examples/common_components/protocol_examples_common/connect.c @@ -287,6 +287,7 @@ static void stop(void) ESP_ERROR_CHECK(esp_event_handler_unregister(IP_EVENT, IP_EVENT_GOT_IP6, &on_got_ipv6)); ESP_ERROR_CHECK(esp_event_handler_unregister(ETH_EVENT, ETHERNET_EVENT_CONNECTED, &on_eth_event)); #endif + ESP_ERROR_CHECK(esp_eth_stop(s_eth_handle)); ESP_ERROR_CHECK(esp_eth_driver_uninstall(s_eth_handle)); ESP_ERROR_CHECK(s_phy->del(s_phy)); ESP_ERROR_CHECK(s_mac->del(s_mac)); From cf161b1c8343aef21ae7d4d0e4281fd4db155ccb Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 3 Dec 2019 16:30:01 +0800 Subject: [PATCH 3/3] ethernet: add start/stop stress test --- components/esp_eth/test/test_emac.c | 53 +++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/components/esp_eth/test/test_emac.c b/components/esp_eth/test/test_emac.c index 07fc324d7..e70fbb558 100644 --- a/components/esp_eth/test/test_emac.c +++ b/components/esp_eth/test/test_emac.c @@ -238,6 +238,59 @@ TEST_CASE("esp32 ethernet dhcp test", "[ethernet][test_env=UT_T2_Ethernet]") vEventGroupDelete(eth_event_group); } +TEST_CASE("esp32 ethernet start/stop stress test", "[ethernet][test_env=UT_T2_Ethernet][timeout=240]") +{ + EventBits_t bits = 0; + EventGroupHandle_t eth_event_group = xEventGroupCreate(); + TEST_ASSERT(eth_event_group != NULL); + test_case_uses_tcpip(); + TEST_ESP_OK(esp_event_loop_create_default()); + // create TCP/IP netif + esp_netif_config_t netif_cfg = ESP_NETIF_DEFAULT_ETH(); + esp_netif_t *eth_netif = esp_netif_new(&netif_cfg); + // set default handlers to do layer 3 (and up) stuffs + TEST_ESP_OK(esp_eth_set_default_handlers(eth_netif)); + // register user defined event handers + TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, ð_event_handler, eth_event_group)); + TEST_ESP_OK(esp_event_handler_register(IP_EVENT, IP_EVENT_ETH_GOT_IP, &got_ip_event_handler, eth_event_group)); + eth_mac_config_t mac_config = ETH_MAC_DEFAULT_CONFIG(); + esp_eth_mac_t *mac = esp_eth_mac_new_esp32(&mac_config); + eth_phy_config_t phy_config = ETH_PHY_DEFAULT_CONFIG(); + esp_eth_phy_t *phy = esp_eth_phy_new_ip101(&phy_config); + esp_eth_config_t eth_config = ETH_DEFAULT_CONFIG(mac, phy); + esp_eth_handle_t eth_handle = NULL; + // install Ethernet driver + TEST_ESP_OK(esp_eth_driver_install(ð_config, ð_handle)); + // combine driver with netif + void *glue = esp_eth_new_netif_glue(eth_handle); + TEST_ESP_OK(esp_netif_attach(eth_netif, glue)); + + for (int i = 0; i < 10; i++) { + // start Ethernet driver + TEST_ESP_OK(esp_eth_start(eth_handle)); + /* wait for IP lease */ + bits = xEventGroupWaitBits(eth_event_group, ETH_GOT_IP_BIT, true, true, pdMS_TO_TICKS(ETH_GET_IP_TIMEOUT_MS)); + TEST_ASSERT((bits & ETH_GOT_IP_BIT) == ETH_GOT_IP_BIT); + // stop Ethernet driver + TEST_ESP_OK(esp_eth_stop(eth_handle)); + /* 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); + } + + TEST_ESP_OK(esp_eth_del_netif_glue(glue)); + /* driver should be uninstalled within 2 seconds */ + TEST_ESP_OK(test_uninstall_driver(eth_handle, 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)); + TEST_ESP_OK(esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, eth_event_handler)); + TEST_ESP_OK(esp_eth_clear_default_handlers(eth_netif)); + esp_netif_destroy(eth_netif); + TEST_ESP_OK(esp_event_loop_delete_default()); + vEventGroupDelete(eth_event_group); +} + TEST_CASE("esp32 ethernet icmp test", "[ethernet][test_env=UT_T2_Ethernet]") { EventBits_t bits = 0;