Merge branch 'bugfix/lan8720_ci_speed_v5.2' into 'release/v5.2'

feat(esp_eth): added ioctl option to read/write PHY registers - v5.2

See merge request espressif/esp-idf!27098

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

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
  */
@@ -229,7 +229,7 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_
     ESP_GOTO_ON_ERROR(phy->init(phy), err, TAG, "init phy failed");
     // get default status of PHY autonegotiation (ultimately may also indicate if it is supported)
     ESP_GOTO_ON_ERROR(phy->autonego_ctrl(phy, ESP_ETH_PHY_AUTONEGO_G_STAT, &eth_driver->auto_nego_en),
-                        err, TAG, "get autonegotiation status failed");
+                      err, TAG, "get autonegotiation status failed");
     ESP_LOGD(TAG, "new ethernet driver @%p", eth_driver);
     *out_hdl = eth_driver;
 
@@ -311,11 +311,11 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl)
     ESP_GOTO_ON_ERROR(esp_timer_stop(eth_driver->check_link_timer), err, TAG, "stop link timer failed");
 
     eth_link_t expected_link = ETH_LINK_UP;
-    if (atomic_compare_exchange_strong(&eth_driver->link, &expected_link, ETH_LINK_DOWN)){
+    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,
-                            TAG, "send ETHERNET_EVENT_DISCONNECTED event failed");
+                          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),
@@ -466,8 +466,27 @@ 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(&eth_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: {
+        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_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(&eth_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) {
             ret = phy->custom_ioctl(phy, cmd, data);

components/esp_eth/src/esp_eth_phy_lan87xx.c

@@ -6,6 +6,8 @@
 #include <string.h>
 #include <stdlib.h>
 #include <sys/cdefs.h>
+#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 @@ static esp_err_t lan87xx_loopback(esp_eth_phy_t *phy, bool enable)
     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:

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

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 = &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
+
     // 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 = &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));
+    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
+
     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

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);

components/esp_eth/test_apps/pytest_esp_eth.py

@@ -214,6 +214,7 @@ def test_esp_eth_ip101(dut: IdfDut) -> None:
 # ----------- LAN8720 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_lan8720
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_lan8720',
 ], indirect=True)
@@ -226,6 +227,7 @@ def test_esp_eth_lan8720(dut: IdfDut) -> None:
 # ----------- RTL8201 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_rtl8201
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_rtl8201',
 ], indirect=True)
@@ -238,6 +240,7 @@ def test_esp_eth_rtl8201(dut: IdfDut) -> None:
 # ----------- KSZ8041 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_ksz8041
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_ksz8041',
 ], indirect=True)
@@ -250,6 +253,7 @@ def test_esp_eth_ksz8041(dut: IdfDut) -> None:
 # ----------- DP83848 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_dp83848
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_dp83848',
 ], indirect=True)
@@ -262,6 +266,7 @@ def test_esp_eth_dp83848(dut: IdfDut) -> None:
 # ----------- W5500 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_w5500
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_w5500',
 ], indirect=True)
@@ -274,6 +279,7 @@ def test_esp_eth_w5500(dut: IdfDut) -> None:
 # ----------- KSZ8851SNL -----------
 @pytest.mark.esp32
 @pytest.mark.eth_ksz8851snl
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_ksz8851snl',
 ], indirect=True)
@@ -286,6 +292,7 @@ def test_esp_eth_ksz8851snl(dut: IdfDut) -> None:
 # ----------- DM9051 -----------
 @pytest.mark.esp32
 @pytest.mark.eth_dm9051
+@pytest.mark.nightly_run
 @pytest.mark.parametrize('config', [
     'default_dm9051',
 ], indirect=True)

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

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