From 0cde8bc26de3620645221667b3afcac865fa8aa9 Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Thu, 23 Oct 2025 15:30:15 +0200 Subject: [PATCH] fix(esp_eth): made LAN8720 test write register function more robust --- .../test_apps/main/esp_eth_test_apps.c | 108 +++++++++++++----- 1 file changed, 79 insertions(+), 29 deletions(-) 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 69eec14e3a..3ded93a323 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 @@ -94,6 +94,82 @@ TEST_CASE("ethernet io test", "[ethernet]") extra_cleanup(); } +#ifdef CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720 +esp_err_t set_phy_reg_bits(esp_eth_handle_t eth_handle, uint32_t reg_addr, uint32_t bitmask, uint32_t max_attempts) +{ + esp_eth_phy_reg_rw_data_t reg = { + .reg_addr = reg_addr, + .reg_value_p = NULL + }; + uint32_t reg_value, reg_value_rb; + + for (uint32_t i = 0; i < max_attempts; i++) { + reg.reg_value_p = ®_value; + esp_err_t ret = esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + reg_value |= bitmask; + ret = esp_eth_ioctl(eth_handle, ETH_CMD_WRITE_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + reg.reg_value_p = ®_value_rb; + ret = esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + // Check if the write was successful + if ((reg_value_rb & bitmask) == bitmask) { + return ESP_OK; + } + // Add delay only if not the last attempt + if (i < max_attempts - 1) { + ESP_LOGW(TAG, "Setting PHY register %04"PRIx32" failed, retrying... (attempt %"PRIu32" of %"PRIu32")", reg_addr, i + 1, max_attempts); + vTaskDelay(pdMS_TO_TICKS(10)); + } + } + return ESP_ERR_TIMEOUT; +} + +esp_err_t clear_phy_reg_bits(esp_eth_handle_t eth_handle, uint32_t reg_addr, uint32_t bitmask, uint32_t max_attempts) +{ + esp_eth_phy_reg_rw_data_t reg = { + .reg_addr = reg_addr, + .reg_value_p = NULL + }; + uint32_t reg_value, reg_value_rb; + + for (uint32_t i = 0; i < max_attempts; i++) { + reg.reg_value_p = ®_value; + esp_err_t ret = esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + reg_value &= ~bitmask; + ret = esp_eth_ioctl(eth_handle, ETH_CMD_WRITE_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + reg.reg_value_p = ®_value_rb; + ret = esp_eth_ioctl(eth_handle, ETH_CMD_READ_PHY_REG, ®); + if (ret != ESP_OK) { + return ret; + } + // Check if the write was successful + if ((reg_value_rb & bitmask) == 0) { + return ESP_OK; + } + // Add delay only if not the last attempt + if (i < max_attempts - 1) { + ESP_LOGW(TAG, "Clearing PHY register %04"PRIx32" failed, retrying... (attempt %"PRIu32" of %"PRIu32")", reg_addr, i + 1, max_attempts); + vTaskDelay(pdMS_TO_TICKS(10)); + } + } + return ESP_ERR_TIMEOUT; +} +#endif // CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720 + // This test expects autonegotiation to be enabled on the other node. TEST_CASE("ethernet io speed/duplex/autonegotiation", "[ethernet]") { @@ -168,17 +244,7 @@ TEST_CASE("ethernet io speed/duplex/autonegotiation", "[ethernet]") // 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); + TEST_ESP_OK(set_phy_reg_bits(eth_handle, 27, 0x8000, 3)); #endif // start the driver and wait for connection establish @@ -263,13 +329,7 @@ TEST_CASE("ethernet io speed/duplex/autonegotiation", "[ethernet]") // *** 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); + TEST_ESP_OK(clear_phy_reg_bits(eth_handle, 27, 0x8000, 3)); #endif esp_eth_start(eth_handle); @@ -565,17 +625,7 @@ TEST_CASE("ethernet start/stop stress test with IP stack", "[ethernet]") // 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); + TEST_ESP_OK(set_phy_reg_bits(eth_handle, 27, 0x8000, 3)); #endif } for (int i = 0; i < 10; i++) {