fix(esp_eth): Fixed Ethernet link reset when Ethernet is stopped

This commit is contained in:
Ondrej Kosta 2024-02-16 12:34:01 +01:00
parent b1992f1b84
commit 3d90c7755f
8 changed files with 130 additions and 18 deletions

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2019-2021 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -128,6 +128,19 @@ struct esp_eth_phy_s {
*/
esp_err_t (*get_link)(esp_eth_phy_t *phy);
/**
* @brief Set Ethernet PHY link status
*
* @param[in] phy: Ethernet PHY instance
* @param[in] link new link status
*
* @return
* - ESP_OK: set Ethernet PHY link status successfully
* - ESP_FAIL: set Ethernet PHY link status failed because some error occurred
*
*/
esp_err_t (*set_link)(esp_eth_phy_t *phy, eth_link_t link);
/**
* @brief Power control of Ethernet PHY
*

View File

@ -139,6 +139,16 @@ esp_err_t esp_eth_phy_802_3_set_speed(phy_802_3_t *phy_802_3, eth_speed_t speed)
*/
esp_err_t esp_eth_phy_802_3_set_duplex(phy_802_3_t *phy_802_3, eth_duplex_t duplex);
/**
* @brief Set Ethernet PHY link status
*
* @param phy_802_3 IEEE 802.3 PHY object infostructure
* @param link new link status
* @return
* - ESP_OK: Ethernet PHY link set successfuly
*/
esp_err_t esp_eth_phy_802_3_set_link(phy_802_3_t *phy_802_3, eth_link_t link);
/**
* @brief Initialize Ethernet PHY
*

View File

@ -303,6 +303,7 @@ 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;
ESP_GOTO_ON_FALSE(eth_driver, ESP_ERR_INVALID_ARG, err, TAG, "ethernet driver handle can't be null");
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_START;
@ -312,10 +313,16 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl)
eth_link_t expected_link = ETH_LINK_UP;
if (atomic_compare_exchange_strong(&eth_driver->link, &expected_link, ETH_LINK_DOWN)){
// MAC is stopped by setting link down
ESP_GOTO_ON_ERROR(mac->set_link(mac, ETH_LINK_DOWN), err, TAG, "ethernet mac set link failed");
ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_DISCONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
// to ensure backwards compatibility, check if set_link function is not NULL
if (phy->set_link != NULL) {
// MAC is stopped by setting link down at PHY layer
ESP_GOTO_ON_ERROR(phy->set_link(phy, ETH_LINK_DOWN), err, TAG, "ethernet phy reset link failed");
} else {
// MAC is stopped by setting link down at MAC layer
ESP_GOTO_ON_ERROR(mac->set_link(mac, ETH_LINK_DOWN), err, TAG, "ethernet mac set link failed");
ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_DISCONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
TAG, "send ETHERNET_EVENT_DISCONNECTED event failed");
}
}
ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_STOP, &eth_driver, sizeof(esp_eth_driver_t *), 0),

View File

@ -156,10 +156,12 @@ static esp_err_t emac_esp32_set_link(esp_eth_mac_t *mac, eth_link_t link)
case ETH_LINK_UP:
ESP_GOTO_ON_ERROR(esp_intr_enable(emac->intr_hdl), err, TAG, "enable interrupt failed");
emac_esp32_start(mac);
ESP_LOGD(TAG, "emac started");
break;
case ETH_LINK_DOWN:
ESP_GOTO_ON_ERROR(esp_intr_disable(emac->intr_hdl), err, TAG, "disable interrupt failed");
emac_esp32_stop(mac);
ESP_LOGD(TAG, "emac stopped");
break;
default:
ESP_GOTO_ON_FALSE(false, ESP_ERR_INVALID_ARG, err, TAG, "unknown link status");

View File

@ -87,6 +87,12 @@ static esp_err_t set_duplex(esp_eth_phy_t *phy, eth_duplex_t duplex)
return esp_eth_phy_802_3_set_duplex(phy_802_3, duplex);
}
static esp_err_t set_link(esp_eth_phy_t *phy, eth_link_t link)
{
phy_802_3_t *phy_802_3 = esp_eth_phy_into_phy_802_3(phy);
return esp_eth_phy_802_3_set_link(phy_802_3, link);
}
static esp_err_t init(esp_eth_phy_t *phy)
{
phy_802_3_t *phy_802_3 = esp_eth_phy_into_phy_802_3(phy);
@ -344,6 +350,20 @@ err:
return ret;
}
esp_err_t esp_eth_phy_802_3_set_link(phy_802_3_t *phy_802_3, eth_link_t link)
{
esp_err_t ret = ESP_OK;
esp_eth_mediator_t *eth = phy_802_3->eth;
if (phy_802_3->link_status != link) {
phy_802_3->link_status = link;
// link status changed, inmiedately report to upper layers
ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)phy_802_3->link_status), err, TAG, "change link failed");
}
err:
return ret;
}
esp_err_t esp_eth_phy_802_3_init(phy_802_3_t *phy_802_3)
{
return esp_eth_phy_802_3_basic_phy_init(phy_802_3);
@ -491,6 +511,7 @@ esp_err_t esp_eth_phy_802_3_obj_config_init(phy_802_3_t *phy_802_3, const eth_ph
phy_802_3->parent.set_speed = set_speed;
phy_802_3->parent.set_duplex = set_duplex;
phy_802_3->parent.del = del;
phy_802_3->parent.set_link = set_link;
phy_802_3->parent.get_link = NULL;
phy_802_3->parent.custom_ioctl = NULL;

View File

@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: MIT
*
* SPDX-FileContributor: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileContributor: 2021-2024 Espressif Systems (Shanghai) CO LTD
*/
#include <stdlib.h>
#include "esp_check.h"
@ -223,6 +223,21 @@ static esp_err_t phy_ksz8851_get_link(esp_eth_phy_t *phy)
return ksz8851_update_link_duplex_speed(ksz8851);
}
static esp_err_t phy_ksz8851_set_link(esp_eth_phy_t *phy, eth_link_t link)
{
esp_err_t ret = ESP_OK;
phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent);
esp_eth_mediator_t *eth = ksz8851->eth;
if (ksz8851->link_status != link) {
ksz8851->link_status = link;
// link status changed, inmiedately report to upper layers
ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)ksz8851->link_status), err, TAG, "change link failed");
}
err:
return ret;
}
static esp_err_t phy_ksz8851_set_addr(esp_eth_phy_t *phy, uint32_t addr)
{
phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent);
@ -361,6 +376,7 @@ esp_eth_phy_t *esp_eth_phy_new_ksz8851snl(const eth_phy_config_t *config)
ksz8851->parent.deinit = phy_ksz8851_deinit;
ksz8851->parent.autonego_ctrl = phy_ksz8851_autonego_ctrl;
ksz8851->parent.get_link = phy_ksz8851_get_link;
ksz8851->parent.set_link = phy_ksz8851_set_link;
ksz8851->parent.pwrctl = phy_ksz8851_pwrctl;
ksz8851->parent.set_addr = phy_ksz8851_set_addr;
ksz8851->parent.get_addr = phy_ksz8851_get_addr;

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -115,6 +115,21 @@ err:
return ret;
}
static esp_err_t w5500_set_link(esp_eth_phy_t *phy, eth_link_t link)
{
esp_err_t ret = ESP_OK;
phy_w5500_t *w5500 = __containerof(phy, phy_w5500_t, parent);
esp_eth_mediator_t *eth = w5500->eth;
if (w5500->link_status != link) {
w5500->link_status = link;
// link status changed, inmiedately report to upper layers
ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)w5500->link_status), err, TAG, "change link failed");
}
err:
return ret;
}
static esp_err_t w5500_reset(esp_eth_phy_t *phy)
{
esp_err_t ret = ESP_OK;
@ -362,6 +377,7 @@ esp_eth_phy_t *esp_eth_phy_new_w5500(const eth_phy_config_t *config)
w5500->parent.set_mediator = w5500_set_mediator;
w5500->parent.autonego_ctrl = w5500_autonego_ctrl;
w5500->parent.get_link = w5500_get_link;
w5500->parent.set_link = w5500_set_link;
w5500->parent.pwrctl = w5500_pwrctl;
w5500->parent.get_addr = w5500_get_addr;
w5500->parent.set_addr = w5500_set_addr;

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
@ -416,17 +416,44 @@ TEST_CASE("esp32 ethernet start/stop stress test", "[ethernet][test_env=UT_T2_Et
TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_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));
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);
for(int j = 0; j < 2; j++) {
// run the start/stop test with disabled auto-negotiation
if (j > 0) {
ESP_LOGI(TAG, "Run with auto-negotiation disabled...");
bool auto_nego_en = false;
bool exp_autoneg_en;
TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_S_AUTONEGO, &auto_nego_en));
TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_G_AUTONEGO, &exp_autoneg_en));
TEST_ASSERT_EQUAL(false, exp_autoneg_en);
// *** LAN8720 deviation ***
// Rationale: When the device is in manual 100BASE-TX or 10BASE-T modes with Auto-MDIX enabled, the PHY does not link to a
// link partner that is configured for auto-negotiation. See LAN8720 errata for more details.
#ifdef CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720
esp_eth_phy_reg_rw_data_t reg;
uint32_t reg_val;
reg.reg_addr = 27;
reg.reg_value_p = &reg_val;
TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, &reg));
reg_val |= 0x8000;
TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_WRITE_PHY_REG, &reg));
uint32_t reg_val_act;
reg.reg_value_p = &reg_val_act;
TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, &reg));
TEST_ASSERT_EQUAL(reg_val, reg_val_act);
#endif
}
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));