From e07023b9c709fc8ea14857fda83b2a80bf1ab400 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 8 May 2020 21:44:30 +0800 Subject: [PATCH 1/3] ethernet: esp_eth_stop API should stop emac hardware --- components/esp_eth/include/esp_eth_mac.h | 24 ++++++++++++++++++++ components/esp_eth/src/esp_eth.c | 2 ++ components/esp_eth/src/esp_eth_mac_dm9051.c | 18 +++++++++------ components/esp_eth/src/esp_eth_mac_esp32.c | 16 +++++++++++++ components/esp_eth/src/esp_eth_mac_openeth.c | 14 ++++++++++++ components/esp_eth/src/esp_eth_phy_dm9051.c | 5 ++++ 6 files changed, 72 insertions(+), 7 deletions(-) diff --git a/components/esp_eth/include/esp_eth_mac.h b/components/esp_eth/include/esp_eth_mac.h index 00094d8b0..c72a4197b 100644 --- a/components/esp_eth/include/esp_eth_mac.h +++ b/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 * diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 64025c4bd..6c80bb2c5 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -267,6 +267,8 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl) // check if driver has started if (eth_driver->flags & ESP_ETH_FLAGS_STARTED) { eth_driver->flags &= ~ESP_ETH_FLAGS_STARTED; + 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, ð_driver, sizeof(eth_driver), 0) == ESP_OK, diff --git a/components/esp_eth/src/esp_eth_mac_dm9051.c b/components/esp_eth/src/esp_eth_mac_dm9051.c index 79eb2aaad..0a37aa5ab 100644 --- a/components/esp_eth/src/esp_eth_mac_dm9051.c +++ b/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; diff --git a/components/esp_eth/src/esp_eth_mac_esp32.c b/components/esp_eth/src/esp_eth_mac_esp32.c index b1d28e0e5..ab9f0d449 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/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; diff --git a/components/esp_eth/src/esp_eth_mac_openeth.c b/components/esp_eth/src/esp_eth_mac_openeth.c index 654195d70..c28ca1cbd 100644 --- a/components/esp_eth/src/esp_eth_mac_openeth.c +++ b/components/esp_eth/src/esp_eth_mac_openeth.c @@ -319,6 +319,18 @@ static esp_err_t emac_opencores_deinit(esp_eth_mac_t *mac) return ESP_OK; } +static esp_err_t emac_opencores_start(esp_eth_mac_t *mac) +{ + openeth_enable(); + return ESP_OK; +} + +static esp_err_t emac_opencores_stop(esp_eth_mac_t *mac) +{ + openeth_disable(); + return ESP_OK; +} + static esp_err_t emac_opencores_del(esp_eth_mac_t *mac) { emac_opencores_t *emac = __containerof(mac, emac_opencores_t, parent); @@ -366,6 +378,8 @@ esp_eth_mac_t *esp_eth_mac_new_openeth(const eth_mac_config_t *config) emac->parent.set_mediator = emac_opencores_set_mediator; emac->parent.init = emac_opencores_init; emac->parent.deinit = emac_opencores_deinit; + emac->parent.start = emac_opencores_start; + emac->parent.stop = emac_opencores_stop; emac->parent.del = emac_opencores_del; emac->parent.write_phy_reg = emac_opencores_write_phy_reg; emac->parent.read_phy_reg = emac_opencores_read_phy_reg; diff --git a/components/esp_eth/src/esp_eth_phy_dm9051.c b/components/esp_eth/src/esp_eth_phy_dm9051.c index 286a7baa5..8dbe3876d 100644 --- a/components/esp_eth/src/esp_eth_phy_dm9051.c +++ b/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; From 63dae581767274d7ff08bdd238d5daf722c5ae1e Mon Sep 17 00:00:00 2001 From: morris Date: Sat, 9 May 2020 12:49:29 +0800 Subject: [PATCH 2/3] ethernet: better control start/stop/uninstall/install --- components/esp_eth/src/esp_eth.c | 44 +++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 6c80bb2c5..a895b5b09 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/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 @@ -56,7 +59,7 @@ typedef struct { eth_link_t link; atomic_int ref_count; void *priv; - 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, 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); @@ -175,6 +178,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(ð_driver->ref_count, 1); + atomic_init(ð_driver->fsm, ESP_ETH_FSM_STOP); eth_driver->mac = mac; eth_driver->phy = phy; eth_driver->link = ETH_LINK_DOWN; @@ -221,9 +225,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(ð_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(ð_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(ð_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); @@ -241,12 +256,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(ð_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); @@ -265,19 +280,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; - 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, ð_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(ð_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, ð_driver, sizeof(eth_driver), 0) == ESP_OK, + "send ETHERNET_EVENT_STOP event failed", err, ESP_FAIL); return ESP_OK; err: return ret; From 222ac1dd60c246694b65f3d7eb37b34dd517f4e9 Mon Sep 17 00:00:00 2001 From: morris Date: Wed, 13 May 2020 16:03:00 +0800 Subject: [PATCH 3/3] ethernet: fix potential task watch dog timeout --- components/esp_eth/src/esp_eth_mac_esp32.c | 4 ++-- 1 file changed, 2 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 ab9f0d449..778ca422b 100644 --- a/components/esp_eth/src/esp_eth_mac_esp32.c +++ b/components/esp_eth/src/esp_eth_mac_esp32.c @@ -244,8 +244,8 @@ static void emac_esp32_rx_task(void *arg) uint8_t *buffer = NULL; uint32_t length = 0; while (1) { - // block indefinitely until some task notifies me - ulTaskNotifyTake(pdFALSE, portMAX_DELAY); + // block indefinitely until got notification from underlay event + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); do { length = ETH_MAX_PACKET_SIZE; buffer = malloc(length);