From 78f791d4d5072f084196d015ad8ddd682d6fd853 Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Mon, 6 Nov 2023 14:43:26 +0100 Subject: [PATCH 1/2] feat(esp_eth): added ioctl option to read/write PHY registers LAN87xx: Added extra delay after setting PHY speed --- components/esp_eth/include/esp_eth_driver.h | 13 ++++++++++++- components/esp_eth/src/esp_eth.c | 20 ++++++++++++++++++-- components/esp_eth/src/esp_eth_phy_lan87xx.c | 16 ++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/components/esp_eth/include/esp_eth_driver.h b/components/esp_eth/include/esp_eth_driver.h index f6f358f015..068a911dfb 100644 --- a/components/esp_eth/include/esp_eth_driver.h +++ b/components/esp_eth/include/esp_eth_driver.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -121,6 +121,15 @@ typedef struct { esp_err_t (*write_phy_reg)(esp_eth_handle_t eth_handle, uint32_t phy_addr, uint32_t phy_reg, uint32_t reg_value); } esp_eth_config_t; +/** + * @brief Data structure to Read/Write PHY register via ioctl API + * + */ +typedef struct { + uint32_t reg_addr; /*!< PHY register address */ + uint32_t *reg_value_p; /*!< Pointer to a memory where the register value is read/written */ +} esp_eth_phy_reg_rw_data_t; + /** * @brief Command list for ioctl API * @@ -139,6 +148,8 @@ typedef enum { ETH_CMD_G_DUPLEX_MODE, /*!< Get Duplex mode */ ETH_CMD_S_DUPLEX_MODE, /*!< Set Duplex mode */ ETH_CMD_S_PHY_LOOPBACK, /*!< Set PHY loopback */ + ETH_CMD_READ_PHY_REG, /*!< Read PHY register */ + ETH_CMD_WRITE_PHY_REG, /*!< Write PHY register */ ETH_CMD_CUSTOM_MAC_CMDS = 0x0FFF, // Offset for start of MAC custom commands ETH_CMD_CUSTOM_PHY_CMDS = 0x1FFF, // Offset for start of PHY custom commands diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index 90cc8a229f..6590ae15e9 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -466,7 +466,23 @@ esp_err_t esp_eth_ioctl(esp_eth_handle_t hdl, esp_eth_io_cmd_t cmd, void *data) case ETH_CMD_S_PHY_LOOPBACK: ESP_GOTO_ON_FALSE(data, ESP_ERR_INVALID_ARG, err, TAG, "can't set loopback to null"); ESP_GOTO_ON_ERROR(phy->loopback(phy, *(bool *)data), err, TAG, "configuration of phy loopback mode failed"); - + break; + case ETH_CMD_READ_PHY_REG: + uint32_t phy_addr; + ESP_GOTO_ON_FALSE(data, ESP_ERR_INVALID_ARG, err, TAG, "invalid register read/write info"); + esp_eth_phy_reg_rw_data_t *phy_r_data = (esp_eth_phy_reg_rw_data_t *)data; + ESP_GOTO_ON_FALSE(phy_r_data->reg_value_p, ESP_ERR_INVALID_ARG, err, TAG, "can't read registers to null"); + ESP_GOTO_ON_ERROR(phy->get_addr(phy, &phy_addr), err, TAG, "get phy address failed"); + ESP_GOTO_ON_ERROR(eth_driver->mediator.phy_reg_read(ð_driver->mediator, + phy_addr, phy_r_data->reg_addr, phy_r_data->reg_value_p), err, TAG, "failed to read PHY register"); + break; + case ETH_CMD_WRITE_PHY_REG: + ESP_GOTO_ON_FALSE(data, ESP_ERR_INVALID_ARG, err, TAG, "invalid register read/write info"); + esp_eth_phy_reg_rw_data_t *phy_w_data = (esp_eth_phy_reg_rw_data_t *)data; + ESP_GOTO_ON_FALSE(phy_w_data->reg_value_p, ESP_ERR_INVALID_ARG, err, TAG, "can't write registers from null"); + ESP_GOTO_ON_ERROR(phy->get_addr(phy, &phy_addr), err, TAG, "get phy address failed"); + ESP_GOTO_ON_ERROR(eth_driver->mediator.phy_reg_write(ð_driver->mediator, + phy_addr, phy_w_data->reg_addr, *(phy_w_data->reg_value_p)), err, TAG, "failed to write PHY register"); break; default: if (phy->custom_ioctl != NULL && cmd >= ETH_CMD_CUSTOM_PHY_CMDS) { diff --git a/components/esp_eth/src/esp_eth_phy_lan87xx.c b/components/esp_eth/src/esp_eth_phy_lan87xx.c index 621f0b7478..b63242d660 100644 --- a/components/esp_eth/src/esp_eth_phy_lan87xx.c +++ b/components/esp_eth/src/esp_eth_phy_lan87xx.c @@ -6,6 +6,8 @@ #include #include #include +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" #include "esp_log.h" #include "esp_check.h" #include "esp_eth_phy_802_3.h" @@ -317,6 +319,19 @@ err: return ret; } +static esp_err_t lan87xx_set_speed(esp_eth_phy_t *phy, eth_speed_t speed) +{ + esp_err_t ret = ESP_OK; + phy_802_3_t *phy_802_3 = esp_eth_phy_into_phy_802_3(phy); + + /* It was observed that a delay needs to be introduced after setting speed and prior driver's start. + Otherwise, the very first read of PHY registers is not valid data (0xFFFF's). */ + ESP_GOTO_ON_ERROR(esp_eth_phy_802_3_set_speed(phy_802_3, speed), err, TAG, "set speed failed"); + vTaskDelay(pdMS_TO_TICKS(10)); +err: + return ret; +} + static esp_err_t lan87xx_init(esp_eth_phy_t *phy) { esp_err_t ret = ESP_OK; @@ -359,6 +374,7 @@ esp_eth_phy_t *esp_eth_phy_new_lan87xx(const eth_phy_config_t *config) lan87xx->phy_802_3.parent.get_link = lan87xx_get_link; lan87xx->phy_802_3.parent.autonego_ctrl = lan87xx_autonego_ctrl; lan87xx->phy_802_3.parent.loopback = lan87xx_loopback; + lan87xx->phy_802_3.parent.set_speed = lan87xx_set_speed; return &lan87xx->phy_802_3.parent; err: From 383bb46298cfb3e078b334630ed63b9610edb8d4 Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Mon, 6 Nov 2023 14:43:48 +0100 Subject: [PATCH 2/2] ci(esp_eth): addressed LAN8720 errata in CI test --- .../esp_eth/test_apps/main/Kconfig.projbuild | 6 ++-- .../test_apps/main/esp_eth_test_apps.c | 36 +++++++++++++++++-- .../test_apps/main/esp_eth_test_common.c | 4 +-- .../test_apps/sdkconfig.ci.default_ksz8041 | 2 +- .../test_apps/sdkconfig.ci.default_lan8720 | 2 +- 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/components/esp_eth/test_apps/main/Kconfig.projbuild b/components/esp_eth/test_apps/main/Kconfig.projbuild index b9ac8825db..61b15841e7 100644 --- a/components/esp_eth/test_apps/main/Kconfig.projbuild +++ b/components/esp_eth/test_apps/main/Kconfig.projbuild @@ -30,10 +30,10 @@ menu "esp_eth TEST_APPS Configuration" config TARGET_ETH_PHY_DEVICE_IP101 bool "IP101" - config TARGET_ETH_PHY_DEVICE_LAN87XX + config TARGET_ETH_PHY_DEVICE_LAN8720 bool "LAN8720" - config TARGET_ETH_PHY_DEVICE_KSZ80XX - bool "KSZ80xx" + config TARGET_ETH_PHY_DEVICE_KSZ8041 + bool "KSZ8041" config TARGET_ETH_PHY_DEVICE_RTL8201 bool "RTL8201" config TARGET_ETH_PHY_DEVICE_DP83848 diff --git a/components/esp_eth/test_apps/main/esp_eth_test_apps.c b/components/esp_eth/test_apps/main/esp_eth_test_apps.c index 3532df46c4..9a58b7b5fe 100644 --- a/components/esp_eth/test_apps/main/esp_eth_test_apps.c +++ b/components/esp_eth/test_apps/main/esp_eth_test_apps.c @@ -164,6 +164,23 @@ TEST_CASE("ethernet io speed/duplex/autonegotiation", "[ethernet]") // set new speed TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_S_SPEED, &speed)); +// *** 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 = ®_val; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®)); + reg_val |= 0x8000; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_WRITE_PHY_REG, ®)); + uint32_t reg_val_act; + reg.reg_value_p = ®_val_act; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®)); + TEST_ASSERT_EQUAL(reg_val, reg_val_act); +#endif + // start the driver and wait for connection establish esp_eth_start(eth_handle); bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(ETH_CONNECT_TIMEOUT_MS)); @@ -242,6 +259,19 @@ TEST_CASE("ethernet io speed/duplex/autonegotiation", "[ethernet]") esp_eth_stop(eth_handle); auto_nego_en = true; esp_eth_ioctl(eth_handle, ETH_CMD_S_AUTONEGO, &auto_nego_en); + +// *** LAN8720 deviation *** +// Rationale: See above +#ifdef CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720 + reg.reg_value_p = ®_val; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®)); + reg_val &= ~0x8000; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_WRITE_PHY_REG, ®)); + reg.reg_value_p = ®_val_act; + TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®)); + TEST_ASSERT_EQUAL(reg_val, reg_val_act); +#endif + esp_eth_start(eth_handle); bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(ETH_CONNECT_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT); @@ -328,7 +358,7 @@ TEST_CASE("ethernet io loopback", "[ethernet]") ESP_LOGI(TAG, "Test with %s Mbps %s duplex.", expected_speed == ETH_SPEED_10M ? "10" : "100", expected_duplex == ETH_DUPLEX_HALF ? "half" : "full"); // *** KSZ80XX, KSZ8851SNL and DM9051 deviation *** // Rationale: do not support loopback at 10 Mbps -#if defined(CONFIG_TARGET_ETH_PHY_DEVICE_KSZ80XX) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_DM9051) +#if defined(CONFIG_TARGET_ETH_PHY_DEVICE_KSZ8041) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_DM9051) if ((expected_speed == ETH_SPEED_10M)) { TEST_ASSERT_EQUAL(ESP_ERR_INVALID_STATE, esp_eth_ioctl(eth_handle, ETH_CMD_S_SPEED, &expected_speed)); continue; @@ -378,7 +408,7 @@ TEST_CASE("ethernet io loopback", "[ethernet]") // *** RTL8201, DP83848 and LAN87xx deviation *** // Rationale: do not support autonegotiation with loopback enabled. #if defined(CONFIG_TARGET_ETH_PHY_DEVICE_RTL8201) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_DP83848) || \ - defined(CONFIG_TARGET_ETH_PHY_DEVICE_LAN87XX) + defined(CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720) TEST_ASSERT_EQUAL(ESP_ERR_INVALID_STATE, esp_eth_ioctl(eth_handle, ETH_CMD_S_AUTONEGO, &auto_nego_en)); goto cleanup; #endif @@ -397,7 +427,7 @@ TEST_CASE("ethernet io loopback", "[ethernet]") TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); // *** W5500, LAN87xx, RTL8201 and DP83848 deviation *** // Rationale: in those cases 'goto cleanup' is used to skip part of the test code. Incasing in #if block is done to prevent unused label error -#if defined(CONFIG_TARGET_ETH_PHY_DEVICE_W5500) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_LAN87XX) || \ +#if defined(CONFIG_TARGET_ETH_PHY_DEVICE_W5500) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720) || \ defined(CONFIG_TARGET_ETH_PHY_DEVICE_RTL8201) || defined(CONFIG_TARGET_ETH_PHY_DEVICE_DP83848) cleanup: #endif diff --git a/components/esp_eth/test_apps/main/esp_eth_test_common.c b/components/esp_eth/test_apps/main/esp_eth_test_common.c index 0a04d9561e..69f5577abd 100644 --- a/components/esp_eth/test_apps/main/esp_eth_test_common.c +++ b/components/esp_eth/test_apps/main/esp_eth_test_common.c @@ -96,9 +96,9 @@ esp_eth_phy_t *phy_init(eth_phy_config_t *phy_config) phy_config->phy_addr = ESP_ETH_PHY_ADDR_AUTO; #if CONFIG_TARGET_ETH_PHY_DEVICE_IP101 phy = esp_eth_phy_new_ip101(phy_config); -#elif CONFIG_TARGET_ETH_PHY_DEVICE_LAN87XX +#elif CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720 phy = esp_eth_phy_new_lan87xx(phy_config); -#elif CONFIG_TARGET_ETH_PHY_DEVICE_KSZ80XX +#elif CONFIG_TARGET_ETH_PHY_DEVICE_KSZ8041 phy = esp_eth_phy_new_ksz80xx(phy_config); #elif CONFIG_TARGET_ETH_PHY_DEVICE_RTL8201 phy = esp_eth_phy_new_rtl8201(phy_config); diff --git a/components/esp_eth/test_apps/sdkconfig.ci.default_ksz8041 b/components/esp_eth/test_apps/sdkconfig.ci.default_ksz8041 index ecb0066f5e..6a6547adf7 100644 --- a/components/esp_eth/test_apps/sdkconfig.ci.default_ksz8041 +++ b/components/esp_eth/test_apps/sdkconfig.ci.default_ksz8041 @@ -6,4 +6,4 @@ CONFIG_ETH_USE_ESP32_EMAC=y CONFIG_ESP_TASK_WDT=n CONFIG_TARGET_USE_INTERNAL_ETHERNET=y -CONFIG_TARGET_ETH_PHY_DEVICE_KSZ80XX=y +CONFIG_TARGET_ETH_PHY_DEVICE_KSZ8041=y diff --git a/components/esp_eth/test_apps/sdkconfig.ci.default_lan8720 b/components/esp_eth/test_apps/sdkconfig.ci.default_lan8720 index 1539599da0..c13b8b5f2c 100644 --- a/components/esp_eth/test_apps/sdkconfig.ci.default_lan8720 +++ b/components/esp_eth/test_apps/sdkconfig.ci.default_lan8720 @@ -6,6 +6,6 @@ CONFIG_ETH_USE_ESP32_EMAC=y CONFIG_ESP_TASK_WDT_EN=n CONFIG_TARGET_USE_INTERNAL_ETHERNET=y -CONFIG_TARGET_ETH_PHY_DEVICE_LAN87XX=y +CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720=y CONFIG_ETH_RMII_CLK_OUTPUT=y CONFIG_ETH_RMII_CLK_OUT_GPIO=17