From 1e674f1d208577883137b415a401d557208f0fbb Mon Sep 17 00:00:00 2001 From: David Cermak Date: Wed, 21 Apr 2021 20:31:11 +0200 Subject: [PATCH] esp_eth: Update KSZ8851SNL driver to use global error checkers Also updated KSZ8851SNL per internal code review: * Removed Link status change interrupt as it's handled with polling * Added auto negotiation timeout * Updated typedefs, moved types to appropriate source, updated components/esp_eth/src/ksz8851.h to use only inherent device types * Applied IDF code formatting * Updated header file order to include first the most generic to more specific --- .../esp_eth/src/esp_eth_mac_ksz8851snl.c | 306 +++++++++--------- .../esp_eth/src/esp_eth_phy_ksz8851snl.c | 131 ++++---- components/esp_eth/src/ksz8851.h | 28 -- 3 files changed, 238 insertions(+), 227 deletions(-) diff --git a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c index 6f8b588946..53ac760014 100644 --- a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c +++ b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c @@ -17,24 +17,31 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -#include "ksz8851.h" - -#include "driver/gpio.h" -#include "esp_log.h" -#include "esp_rom_gpio.h" #include +#include "esp_log.h" +#include "esp_check.h" +#include "driver/gpio.h" +#include "esp_rom_gpio.h" +#include "driver/spi_master.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "freertos/semphr.h" +#include "esp_eth.h" +#include "ksz8851.h" -static const char *TAG = "ksz8851snl-mac"; -#define MAC_CHECK(a, str, goto_tag, ret_value, ...) \ - do { \ - if (!(a)) { \ - ESP_LOGE(TAG, "%s(%d): " str, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - ret = ret_value; \ - goto goto_tag; \ - } \ - } while (0) +typedef struct { + esp_eth_mac_t parent; + esp_eth_mediator_t *eth; + spi_device_handle_t spi_hdl; + SemaphoreHandle_t spi_lock; + TaskHandle_t rx_task_hdl; + uint32_t sw_reset_timeout_ms; + int int_gpio_num; + uint8_t *rx_buffer; + uint8_t *tx_buffer; +} emac_ksz8851snl_t; typedef enum { KSZ8851_SPI_COMMAND_READ_REG = 0x0U, @@ -43,19 +50,23 @@ typedef enum { KSZ8851_SPI_COMMAND_WRITE_FIFO = 0x3U, } ksz8851_spi_commands_t; -static const unsigned KSZ8851_SPI_COMMAND_BITS = 2U; -static const unsigned KSZ8851_SPI_ADDR_SHIFT = 2U; -static const unsigned KSZ8851_SPI_BYTE_MASK_SHIFT = 8U + KSZ8851_SPI_ADDR_SHIFT; -static const unsigned KSZ8851_SPI_LOCK_TIMEOUT_MS = 500U; - typedef enum { KSZ8851_QMU_PACKET_LENGTH = 2000U, KSZ8851_QMU_PACKET_PADDING = 16U, } ksz8851_qmu_packet_size_t; -static const uint16_t RXDTTR_VALUE = 0x03E8U; -static const uint16_t RXDBCTR_VALUE = 0x1000U; -static const uint16_t RXFCT_VALUE = 0X0001U; + +static const char *TAG = "ksz8851snl-mac"; + +static const unsigned KSZ8851_SPI_COMMAND_BITS = 2U; +static const unsigned KSZ8851_SPI_ADDR_SHIFT = 2U; +static const unsigned KSZ8851_SPI_BYTE_MASK_SHIFT = 8U + KSZ8851_SPI_ADDR_SHIFT; +static const unsigned KSZ8851_SPI_LOCK_TIMEOUT_MS = 500U; + +static const uint16_t RXDTTR_INIT_VALUE = 0x03E8U; +static const uint16_t RXDBCTR_INIT_VALUE = 0x1000U; +static const uint16_t RXFCT_INIT_VALUE = 0X0001U; + IRAM_ATTR static void ksz8851_isr_handler(void *arg) { @@ -80,8 +91,8 @@ static inline bool ksz8851_mutex_unlock(emac_ksz8851snl_t *emac) static esp_err_t ksz8851_read_reg(emac_ksz8851snl_t *emac, uint32_t address, uint16_t *value) { esp_err_t ret = ESP_OK; - MAC_CHECK(value != NULL, "out pointer must not be null", err, ESP_ERR_INVALID_ARG); - MAC_CHECK((address & ~KSZ8851_VALID_ADDRESS_MASK) == 0U, "address is out of bounds", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(value != NULL, ESP_ERR_INVALID_ARG, err, TAG, "out pointer must not be null"); + ESP_GOTO_ON_FALSE((address & ~KSZ8851_VALID_ADDRESS_MASK) == 0U, ESP_ERR_INVALID_ARG, err, TAG, "address is out of bounds"); const unsigned data_size = 16U; // NOTE(v.chistyakov): bits // NOTE(v.chistyakov): select upper or lower word inside a dword @@ -114,7 +125,7 @@ err: static esp_err_t ksz8851_write_reg(emac_ksz8851snl_t *emac, uint32_t address, uint16_t value) { esp_err_t ret = ESP_OK; - MAC_CHECK((address & ~KSZ8851_VALID_ADDRESS_MASK) == 0U, "address is out of bounds", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE((address & ~KSZ8851_VALID_ADDRESS_MASK) == 0U, ESP_ERR_INVALID_ARG, err, TAG, "address is out of bounds"); ESP_LOGV(TAG, "writing reg 0x%02x = 0x%04x", address, value); const unsigned data_size = 16U; // NOTE(v.chistyakov): bits @@ -149,9 +160,9 @@ static esp_err_t ksz8851_set_bits(emac_ksz8851snl_t *emac, uint32_t address, uin { esp_err_t ret = ESP_OK; uint16_t old; - MAC_CHECK(ksz8851_read_reg(emac, address, &old) == ESP_OK, "failed to read reg 0x%x", err, ESP_FAIL, address); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, address, &old), err, TAG, "failed to read reg 0x%x", address); old |= value; - MAC_CHECK(ksz8851_write_reg(emac, address, old) == ESP_OK, "failed to write reg 0x%x", err, ESP_FAIL, address); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, address, old), err, TAG, "failed to write reg 0x%x", address); err: return ret; } @@ -160,9 +171,9 @@ static esp_err_t ksz8851_clear_bits(emac_ksz8851snl_t *emac, uint32_t address, u { esp_err_t ret = ESP_OK; uint16_t old; - MAC_CHECK(ksz8851_read_reg(emac, address, &old) == ESP_OK, "failed to read reg 0x%x", err, ESP_FAIL, address); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, address, &old), err, TAG, "failed to read reg 0x%x", address); old &= ~value; - MAC_CHECK(ksz8851_write_reg(emac, address, old) == ESP_OK, "failed to write reg 0x%x", err, ESP_FAIL, address); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, address, old), err, TAG, "failed to write reg 0x%x", address); err: return ret; } @@ -170,7 +181,7 @@ err: static esp_err_t emac_ksz8851_set_mediator(esp_eth_mac_t *mac, esp_eth_mediator_t *eth) { esp_err_t ret = ESP_OK; - MAC_CHECK(eth, "mediator can not be null", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(eth, ESP_ERR_INVALID_ARG, err, TAG, "mediator can not be null"); emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); emac->eth = eth; return ESP_OK; @@ -178,6 +189,62 @@ err: return ret; } +static esp_err_t init_soft_reset(emac_ksz8851snl_t *emac) +{ + esp_err_t ret = ESP_OK; + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_PMECR, PMECR_WAKEUP_TO_NORMAL), err, TAG, "PMECR write failed"); + + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_GRR, GRR_GLOBAL_SOFT_RESET), err, TAG, "GRR write failed"); + vTaskDelay(pdMS_TO_TICKS(emac->sw_reset_timeout_ms)); + + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_GRR, GRR_GLOBAL_SOFT_RESET), err, TAG, "GRR write failed"); + vTaskDelay(pdMS_TO_TICKS(emac->sw_reset_timeout_ms)); + return ESP_OK; +err: + return ret; +} + +static esp_err_t init_verify_chipid(emac_ksz8851snl_t *emac) +{ + uint16_t id; + esp_err_t ret = ESP_OK; + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_CIDER, &id), err, TAG, "CIDER read failed"); + uint8_t family_id = (id & CIDER_FAMILY_ID_MASK) >> CIDER_FAMILY_ID_SHIFT; + uint8_t chip_id = (id & CIDER_CHIP_ID_MASK) >> CIDER_CHIP_ID_SHIFT; + uint8_t revision = (id & CIDER_REVISION_ID_MASK) >> CIDER_REVISION_ID_SHIFT; + ESP_LOGI(TAG, "Family ID = 0x%x\t Chip ID = 0x%x\t Revision ID = 0x%x", family_id, chip_id, revision); + ESP_GOTO_ON_FALSE(family_id == CIDER_KSZ8851SNL_FAMILY_ID, ESP_FAIL, err, TAG, "wrong family id"); + ESP_GOTO_ON_FALSE(chip_id == CIDER_KSZ8851SNL_CHIP_ID, ESP_FAIL, err, TAG, "wrong chip id"); + return ESP_OK; +err: + return ret; +} + +static esp_err_t init_set_defaults(emac_ksz8851snl_t *emac) +{ + esp_err_t ret = ESP_OK; + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_TXFDPR, TXFDPR_TXFPAI), err, TAG, "TXFDPR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_TXCR, + TXCR_TXFCE | TXCR_TXPE | TXCR_TXCE | TXCR_TCGICMP | TXCR_TCGIP | TXCR_TCGTCP), err, TAG, "TXCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXFDPR, RXFDPR_RXFPAI), err, TAG, "RXFDPR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXFCTR, RXFCT_INIT_VALUE), err, TAG, "RXFCTR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXDTTR, RXDTTR_INIT_VALUE), err, TAG, "RXDTTR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXDBCTR, RXDBCTR_INIT_VALUE), err, TAG, "RXDBCTR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXCR1, + RXCR1_RXUDPFCC | RXCR1_RXTCPFCC | RXCR1_RXIPFCC | RXCR1_RXPAFMA | RXCR1_RXFCE | RXCR1_RXBE | RXCR1_RXUE), err, TAG, "RXCR1 write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXCR2, + (4 << RXCR2_SRDBL_SHIFT) | RXCR2_IUFFP | RXCR2_RXIUFCEZ | RXCR2_UDPLFE | RXCR2_RXICMPFCC), err, TAG, "RXCR2 write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_RXIPHTOE | RXQCR_RXFCTE | RXQCR_ADRFE), err, TAG, "RXQCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX), err, TAG, "P1CR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_RESTART_AN), err, TAG, "P1CR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_ISR, ISR_ALL), err, TAG, "ISR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_IER, IER_TXIE | IER_RXIE | IER_LDIE | IER_SPIBEIE | IER_RXOIE), err, TAG, "IER write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_TXQCR, TXQCR_AETFE), err, TAG, "TXQCR write failed"); + return ESP_OK; +err: + return ret; +} + static esp_err_t emac_ksz8851_init(esp_eth_mac_t *mac) { esp_err_t ret = ESP_OK; @@ -189,58 +256,17 @@ static esp_err_t emac_ksz8851_init(esp_eth_mac_t *mac) gpio_set_intr_type(emac->int_gpio_num, GPIO_INTR_NEGEDGE); // NOTE(v.chistyakov): active low gpio_intr_enable(emac->int_gpio_num); gpio_isr_handler_add(emac->int_gpio_num, ksz8851_isr_handler, emac); - MAC_CHECK(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL) == ESP_OK, "lowlevel init failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL), err, TAG, "lowlevel init failed"); + // NOTE(v.chistyakov): soft reset - { - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_PMECR, PMECR_WAKEUP_TO_NORMAL) == ESP_OK, "PMECR write failed", err, - ESP_FAIL); + ESP_GOTO_ON_ERROR(init_soft_reset(emac), err, TAG, "soft reset failed"); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_GRR, GRR_GLOBAL_SOFT_RESET) == ESP_OK, "GRR write failed", err, ESP_FAIL); - vTaskDelay(pdMS_TO_TICKS(emac->sw_reset_timeout_ms)); - - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_GRR, GRR_GLOBAL_SOFT_RESET) == ESP_OK, "GRR write failed", err, - ESP_FAIL); - vTaskDelay(pdMS_TO_TICKS(emac->sw_reset_timeout_ms)); - } // NOTE(v.chistyakov): verify chip id - { - uint16_t id; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_CIDER, &id) == ESP_OK, "CIDER read failed", err, ESP_FAIL); - uint8_t family_id = (id & CIDER_FAMILY_ID_MASK) >> CIDER_FAMILY_ID_SHIFT; - uint8_t chip_id = (id & CIDER_CHIP_ID_MASK) >> CIDER_CHIP_ID_SHIFT; - uint8_t revision = (id & CIDER_REVISION_ID_MASK) >> CIDER_REVISION_ID_SHIFT; - ESP_LOGI(TAG, "Family ID = 0x%x\t Chip ID = 0x%x\t Revision ID = 0x%x", family_id, chip_id, revision); - MAC_CHECK(family_id == CIDER_KSZ8851SNL_FAMILY_ID, "wrong family id", err, ESP_FAIL); - MAC_CHECK(chip_id == CIDER_KSZ8851SNL_CHIP_ID, "wrong chip id", err, ESP_FAIL); - } + ESP_GOTO_ON_ERROR(init_verify_chipid(emac), err, TAG, "verify chip id failed"); + // NOTE(v.chistyakov): set default values - { - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_TXFDPR, TXFDPR_TXFPAI) == ESP_OK, "TXFDPR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_TXCR, - TXCR_TXFCE | TXCR_TXPE | TXCR_TXCE | TXCR_TCGICMP | TXCR_TCGIP | TXCR_TCGTCP) == ESP_OK, - "TXCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXFDPR, RXFDPR_RXFPAI) == ESP_OK, "RXFDPR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXFCTR, RXFCT_VALUE) == ESP_OK, "RXFCTR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXDTTR, RXDTTR_VALUE) == ESP_OK, "RXDTTR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXDBCTR, RXDBCTR_VALUE) == ESP_OK, "RXDBCTR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXCR1, - RXCR1_RXUDPFCC | RXCR1_RXTCPFCC | RXCR1_RXIPFCC | RXCR1_RXPAFMA | RXCR1_RXFCE | - RXCR1_RXBE | RXCR1_RXUE) == ESP_OK, - "RXCR1 write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXCR2, - (4 << RXCR2_SRDBL_SHIFT) | RXCR2_IUFFP | RXCR2_RXIUFCEZ | RXCR2_UDPLFE | - RXCR2_RXICMPFCC) == ESP_OK, - "RXCR2 write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_RXIPHTOE | RXQCR_RXFCTE | RXQCR_ADRFE) == ESP_OK, - "RXQCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX) == ESP_OK, "P1CR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_RESTART_AN) == ESP_OK, "P1CR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_ISR, ISR_ALL) == ESP_OK, "ISR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_IER, - IER_LCIE | IER_TXIE | IER_RXIE | IER_LDIE | IER_SPIBEIE | IER_RXOIE) == ESP_OK, - "IER write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_TXQCR, TXQCR_AETFE) == ESP_OK, "TXQCR write failed", err, ESP_FAIL); - } + ESP_GOTO_ON_ERROR(init_set_defaults(emac), err, TAG, "set defaults after init failed"); + ESP_LOGD(TAG, "MAC initialized"); return ESP_OK; err: @@ -267,8 +293,8 @@ static esp_err_t emac_ksz8851_start(esp_eth_mac_t *mac) { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_TXCR, TXCR_TXE) == ESP_OK, "TXCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXCR1, RXCR1_RXE) == ESP_OK, "RXCR1 write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_TXCR, TXCR_TXE), err, TAG, "TXCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXCR1, RXCR1_RXE), err, TAG, "RXCR1 write failed"); ESP_LOGD(TAG, "MAC started"); err: return ret; @@ -278,8 +304,8 @@ static esp_err_t emac_ksz8851_stop(esp_eth_mac_t *mac) { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_TXCR, TXCR_TXE) == ESP_OK, "TXCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_RXCR1, RXCR1_RXE) == ESP_OK, "RXCR1 write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_TXCR, TXCR_TXE), err, TAG, "TXCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_RXCR1, RXCR1_RXE), err, TAG, "RXCR1 write failed"); ESP_LOGD(TAG, "MAC stopped"); err: return ret; @@ -296,14 +322,14 @@ static esp_err_t emac_ksz8851snl_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint return ESP_ERR_TIMEOUT; } - MAC_CHECK(length <= KSZ8851_QMU_PACKET_LENGTH, "packet is too big", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(length <= KSZ8851_QMU_PACKET_LENGTH, ESP_ERR_INVALID_ARG, err, TAG, "packet is too big"); // NOTE(v.chistyakov): 4 bytes header + length aligned to 4 bytes unsigned transmit_length = 4U + ((length + 3U) & ~0x3U); uint16_t free_space; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_TXMIR, &free_space) == ESP_OK, "TXMIR read failed", err, ESP_FAIL); - MAC_CHECK(transmit_length <= free_space, "TXQ free space (%d) < send length (%d)", err, ESP_FAIL, free_space, - transmit_length); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_TXMIR, &free_space), err, TAG, "TXMIR read failed"); + ESP_GOTO_ON_FALSE(transmit_length <= free_space, ESP_FAIL, err, TAG, "TXQ free space (%d) < send length (%d)", free_space, + transmit_length); emac->tx_buffer[0] = ++s_frame_id & TXSR_TXFID_MASK; emac->tx_buffer[1] = 0x80U; @@ -321,17 +347,17 @@ static esp_err_t emac_ksz8851snl_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint }; uint16_t ier; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_IER, &ier) == ESP_OK, "IER read failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_IER, 0) == ESP_OK, "IER write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_IER, &ier), err, TAG, "IER read failed"); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_IER, 0), err, TAG, "IER write failed"); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_SDA) == ESP_OK, "RXQCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_SDA), err, TAG, "RXQCR write failed"); if (spi_device_polling_transmit(emac->spi_hdl, &trans.base) != ESP_OK) { ESP_LOGE(TAG, "%s(%d): spi transmit failed", __FUNCTION__, __LINE__); ret = ESP_FAIL; } - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_RXQCR, RXQCR_SDA) == ESP_OK, "RXQCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_RXQCR, RXQCR_SDA), err, TAG, "RXQCR write failed"); - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_IER, ier) == ESP_OK, "IER write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_IER, ier), err, TAG, "IER write failed"); err: ksz8851_mutex_unlock(emac); return ret; @@ -345,20 +371,20 @@ static esp_err_t emac_ksz8851_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t return ESP_ERR_TIMEOUT; } - MAC_CHECK(buf, "receive buffer can not be null", err, ESP_ERR_INVALID_ARG); - MAC_CHECK(length, "receive buffer length can not be null", err, ESP_ERR_INVALID_ARG); - MAC_CHECK(*length > 0U, "receive buffer length must be greater than zero", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(buf, ESP_ERR_INVALID_ARG, err, TAG, "receive buffer can not be null"); + ESP_GOTO_ON_FALSE(length, ESP_ERR_INVALID_ARG, err, TAG, "receive buffer length can not be null"); + ESP_GOTO_ON_FALSE(*length > 0U, ESP_ERR_INVALID_ARG, err, TAG, "receive buffer length must be greater than zero"); uint16_t header_status; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_RXFHSR, &header_status) == ESP_OK, "RXFHSR read failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_RXFHSR, &header_status), err, TAG, "RXFHSR read failed"); uint16_t byte_count; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_RXFHBCR, &byte_count) == ESP_OK, "RXFHBCR read failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_RXFHBCR, &byte_count), err, TAG, "RXFHBCR read failed"); byte_count &= RXFHBCR_RXBC_MASK; // NOTE(v.chistyakov): do not include 2 bytes padding at the beginning and 4 bytes CRC at the end const unsigned frame_size = byte_count - 6U; - MAC_CHECK(frame_size <= *length, "frame size is greater than length", err, ESP_FAIL); + ESP_GOTO_ON_FALSE(frame_size <= *length, ESP_FAIL, err, TAG, "frame size is greater than length"); if (header_status & RXFHSR_RXFV) { // NOTE(v.chistyakov): 4 dummy + 4 header + alignment @@ -372,14 +398,13 @@ static esp_err_t emac_ksz8851_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t .address_bits = 6U, }; - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_RXFDPR, RXFDPR_RXFP_MASK) == ESP_OK, "RXFDPR write failed", err, - ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_SDA) == ESP_OK, "RXQCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_RXFDPR, RXFDPR_RXFP_MASK), err, TAG, "RXFDPR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_SDA), err, TAG, "RXQCR write failed"); if (spi_device_polling_transmit(emac->spi_hdl, &trans.base) != ESP_OK) { ESP_LOGE(TAG, "%s(%d): spi transmit failed", __FUNCTION__, __LINE__); ret = ESP_FAIL; } - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_RXQCR, RXQCR_SDA) == ESP_OK, "RXQCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_RXQCR, RXQCR_SDA), err, TAG, "RXQCR write failed"); // NOTE(v.chistyakov): skip 4 dummy, 4 header, 2 padding memcpy(buf, emac->rx_buffer + 10U, frame_size); @@ -388,7 +413,7 @@ static esp_err_t emac_ksz8851_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t } else if (header_status & (RXFHSR_RXCE | RXFHSR_RXRF | RXFHSR_RXFTL | RXFHSR_RXMR | RXFHSR_RXUDPFCS | RXFHSR_RXTCPFCS | RXFHSR_RXIPFCS | RXFHSR_RXICMPFCS)) { // NOTE(v.chistyakov): RRXEF is a self-clearing bit - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_RRXEF) == ESP_OK, "RXQCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXQCR, RXQCR_RRXEF), err, TAG, "RXQCR write failed"); *length = 0U; } err: @@ -400,9 +425,9 @@ static esp_err_t emac_ksz8851_read_phy_reg(esp_eth_mac_t *mac, uint32_t phy_addr { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(reg_value, "reg_value can not be null", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(reg_value, ESP_ERR_INVALID_ARG, err, TAG, "reg_value can not be null"); uint16_t tmp_val; - MAC_CHECK(ksz8851_read_reg(emac, phy_reg, &tmp_val) == ESP_OK, "read PHY register failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, phy_reg, &tmp_val), err, TAG, "read PHY register failed"); *reg_value = tmp_val; err: return ret; @@ -412,8 +437,7 @@ static esp_err_t emac_ksz8851_write_phy_reg(esp_eth_mac_t *mac, uint32_t phy_add { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(ksz8851_write_reg(emac, phy_reg, (uint16_t)reg_value) == ESP_OK, "write PHY register failed", err, - ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, phy_reg, (uint16_t)reg_value), err, TAG, "write PHY register failed"); err: return ret; } @@ -422,13 +446,13 @@ static esp_err_t emac_ksz8851_set_addr(esp_eth_mac_t *mac, uint8_t *addr) { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(addr, "addr can not be null", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(addr, ESP_ERR_INVALID_ARG, err, TAG, "addr can not be null"); uint16_t MARL = addr[5] | ((uint16_t)(addr[4]) << 8); uint16_t MARM = addr[3] | ((uint16_t)(addr[2]) << 8); uint16_t MARH = addr[1] | ((uint16_t)(addr[0]) << 8); - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_MARL, MARL) == ESP_OK, "MARL write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_MARM, MARM) == ESP_OK, "MARM write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_MARH, MARH) == ESP_OK, "MARH write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_MARL, MARL), err, TAG, "MARL write failed"); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_MARM, MARM), err, TAG, "MARM write failed"); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_MARH, MARH), err, TAG, "MARH write failed"); ESP_LOGD(TAG, "set MAC address to %02x:%02x:%02x:%02x:%02x:%02x", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]); err: @@ -439,11 +463,11 @@ static esp_err_t emac_ksz8851_get_addr(esp_eth_mac_t *mac, uint8_t *addr) { esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); - MAC_CHECK(addr, "addr can not be null", err, ESP_ERR_INVALID_ARG); + ESP_GOTO_ON_FALSE(addr, ESP_ERR_INVALID_ARG, err, TAG, "addr can not be null"); uint16_t MARL, MARM, MARH; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_MARL, &MARL) == ESP_OK, "MARL read failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_MARM, &MARM) == ESP_OK, "MARM read failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_MARH, &MARH) == ESP_OK, "MARH read failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_MARL, &MARL), err, TAG, "MARL read failed"); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_MARM, &MARM), err, TAG, "MARM read failed"); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_MARH, &MARH), err, TAG, "MARH read failed"); addr[0] = (MARH >> 8) & 0xFF; addr[1] = MARH & 0xFF; addr[2] = (MARM >> 8) & 0xFF; @@ -460,14 +484,14 @@ static esp_err_t emac_ksz8851_set_speed(esp_eth_mac_t *mac, eth_speed_t speed) emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); switch (speed) { case ETH_SPEED_100M: - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_FORCE_SPEED) == ESP_OK, "P1CR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_FORCE_SPEED), err, TAG, "P1CR write failed"); ESP_LOGD(TAG, "set speed to 100M"); break; case ETH_SPEED_10M: - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_SPEED) == ESP_OK, "P1CR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_SPEED), err, TAG, "P1CR write failed"); ESP_LOGD(TAG, "set speed to 10M"); break; - default: MAC_CHECK(false, "unknown speed", err, ESP_ERR_INVALID_ARG); break; + default: ESP_GOTO_ON_FALSE(false, ESP_ERR_INVALID_ARG, err, TAG, "unknown speed"); break; } err: return ret; @@ -479,14 +503,14 @@ static esp_err_t emac_ksz8851_set_duplex(esp_eth_mac_t *mac, eth_duplex_t duplex emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); switch (duplex) { case ETH_DUPLEX_FULL: - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX) == ESP_OK, "P1CR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX), err, TAG, "P1CR write failed"); ESP_LOGD(TAG, "set duplex to full"); break; case ETH_DUPLEX_HALF: - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX) == ESP_OK, "P1CR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_P1CR, P1CR_FORCE_DUPLEX), err, TAG, "P1CR write failed"); ESP_LOGD(TAG, "set duplex to half"); break; - default: MAC_CHECK(false, "unknown duplex", err, ESP_ERR_INVALID_ARG); break; + default: ESP_GOTO_ON_FALSE(false, ESP_ERR_INVALID_ARG, err, TAG, "unknown duplex"); break; } err: return ret; @@ -497,14 +521,14 @@ static esp_err_t emac_ksz8851_set_link(esp_eth_mac_t *mac, eth_link_t link) esp_err_t ret = ESP_OK; switch (link) { case ETH_LINK_UP: - MAC_CHECK(mac->start(mac) == ESP_OK, "ksz8851 start failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(mac->start(mac), err, TAG, "ksz8851 start failed"); ESP_LOGD(TAG, "link is up"); break; case ETH_LINK_DOWN: - MAC_CHECK(mac->stop(mac) == ESP_OK, "ksz8851 stop failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(mac->stop(mac), err, TAG, "ksz8851 stop failed"); ESP_LOGD(TAG, "link is down"); break; - default: MAC_CHECK(false, "unknown link status", err, ESP_ERR_INVALID_ARG); break; + default: ESP_GOTO_ON_FALSE(false, ESP_ERR_INVALID_ARG, err, TAG, "unknown link status"); break; } err: return ret; @@ -515,7 +539,7 @@ static esp_err_t emac_ksz8851_set_promiscuous(esp_eth_mac_t *mac, bool enable) esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); uint16_t rxcr1; - MAC_CHECK(ksz8851_read_reg(emac, KSZ8851_RXCR1, &rxcr1) == ESP_OK, "RXCR1 read failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_read_reg(emac, KSZ8851_RXCR1, &rxcr1), err, TAG, "RXCR1 read failed"); if (enable) { // NOTE(v.chistyakov): set promiscuous mode ESP_LOGD(TAG, "setting promiscuous mode"); @@ -527,7 +551,7 @@ static esp_err_t emac_ksz8851_set_promiscuous(esp_eth_mac_t *mac, bool enable) rxcr1 |= RXCR1_RXPAFMA; rxcr1 &= ~(RXCR1_RXINVF | RXCR1_RXAE | RXCR1_RXMAFMA); } - MAC_CHECK(ksz8851_write_reg(emac, KSZ8851_RXCR1, rxcr1) == ESP_OK, "RXCR1 write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_write_reg(emac, KSZ8851_RXCR1, rxcr1), err, TAG, "RXCR1 write failed"); err: return ret; } @@ -537,12 +561,12 @@ static esp_err_t emac_ksz8851_enable_flow_ctrl(esp_eth_mac_t *mac, bool enable) esp_err_t ret = ESP_OK; emac_ksz8851snl_t *emac = __containerof(mac, emac_ksz8851snl_t, parent); if (enable) { - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_TXCR, TXCR_TXFCE) == ESP_OK, "TXCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_set_bits(emac, KSZ8851_RXCR1, RXCR1_RXFCE) == ESP_OK, "RXCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_TXCR, TXCR_TXFCE), err, TAG, "TXCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_set_bits(emac, KSZ8851_RXCR1, RXCR1_RXFCE), err, TAG, "RXCR write failed"); ESP_LOGD(TAG, "flow control enabled"); } else { - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_TXCR, TXCR_TXFCE) == ESP_OK, "TXCR write failed", err, ESP_FAIL); - MAC_CHECK(ksz8851_clear_bits(emac, KSZ8851_RXCR1, RXCR1_RXFCE) == ESP_OK, "RXCR write failed", err, ESP_FAIL); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_TXCR, TXCR_TXFCE), err, TAG, "TXCR write failed"); + ESP_GOTO_ON_ERROR(ksz8851_clear_bits(emac, KSZ8851_RXCR1, RXCR1_RXFCE), err, TAG, "RXCR write failed"); ESP_LOGD(TAG, "flow control disabled"); } err: @@ -574,11 +598,8 @@ static void emac_ksz8851snl_task(void *arg) uint16_t interrupt_status; ksz8851_read_reg(emac, KSZ8851_ISR, &interrupt_status); - ksz8851_write_reg(emac, KSZ8851_ISR, ISR_ALL); + ksz8851_write_reg(emac, KSZ8851_ISR, interrupt_status); - if (interrupt_status & ISR_LCIS) { - ESP_LOGD(TAG, "Link Change Interrupt"); - } if (interrupt_status & ISR_TXIS) { ESP_LOGD(TAG, "TX Interrupt"); } @@ -651,11 +672,11 @@ esp_eth_mac_t *esp_eth_mac_new_ksz8851snl(const eth_ksz8851snl_config_t *ksz8851 esp_eth_mac_t *ret = NULL; emac_ksz8851snl_t *emac = NULL; - MAC_CHECK(ksz8851snl_config && mac_config, "arguments can not be null", err, NULL); - MAC_CHECK(ksz8851snl_config->int_gpio_num >= 0, "invalid interrupt gpio number", err, NULL); + ESP_GOTO_ON_FALSE(ksz8851snl_config && mac_config, NULL, err, TAG, "arguments can not be null"); + ESP_GOTO_ON_FALSE(ksz8851snl_config->int_gpio_num >= 0, NULL, err, TAG, "invalid interrupt gpio number"); emac = calloc(1, sizeof(emac_ksz8851snl_t)); - MAC_CHECK(emac, "no mem for MAC instance", err, NULL); + ESP_GOTO_ON_FALSE(emac, NULL, err, TAG, "no mem for MAC instance"); emac->sw_reset_timeout_ms = mac_config->sw_reset_timeout_ms; emac->int_gpio_num = ksz8851snl_config->int_gpio_num; @@ -679,21 +700,21 @@ esp_eth_mac_t *esp_eth_mac_new_ksz8851snl(const eth_ksz8851snl_config_t *ksz8851 emac->parent.set_peer_pause_ability = emac_ksz8851_set_peer_pause_ability; emac->parent.del = emac_ksz8851_del; emac->spi_lock = xSemaphoreCreateRecursiveMutex(); - MAC_CHECK(emac->spi_lock, "create lock failed", err, NULL); + ESP_GOTO_ON_FALSE(emac->spi_lock, NULL, err, TAG, "create lock failed"); emac->rx_buffer = NULL; emac->tx_buffer = NULL; emac->rx_buffer = heap_caps_malloc(KSZ8851_QMU_PACKET_LENGTH + KSZ8851_QMU_PACKET_PADDING, MALLOC_CAP_DMA); emac->tx_buffer = heap_caps_malloc(KSZ8851_QMU_PACKET_LENGTH + KSZ8851_QMU_PACKET_PADDING, MALLOC_CAP_DMA); - MAC_CHECK(emac->rx_buffer, "RX buffer allocation failed", err, NULL); - MAC_CHECK(emac->tx_buffer, "TX buffer allocation failed", err, NULL); + ESP_GOTO_ON_FALSE(emac->rx_buffer, NULL, err, TAG, "RX buffer allocation failed"); + ESP_GOTO_ON_FALSE(emac->tx_buffer, NULL, err, TAG, "TX buffer allocation failed"); BaseType_t core_num = tskNO_AFFINITY; if (mac_config->flags & ETH_MAC_FLAG_PIN_TO_CORE) { core_num = cpu_hal_get_core_id(); } BaseType_t xReturned = xTaskCreatePinnedToCore(emac_ksz8851snl_task, "ksz8851snl_tsk", mac_config->rx_task_stack_size, - emac, mac_config->rx_task_prio, &emac->rx_task_hdl, core_num); - MAC_CHECK(xReturned == pdPASS, "create ksz8851 task failed", err, NULL); + emac, mac_config->rx_task_prio, &emac->rx_task_hdl, core_num); + ESP_GOTO_ON_FALSE(xReturned == pdPASS, NULL, err, TAG, "create ksz8851 task failed"); return &(emac->parent); err: @@ -704,9 +725,6 @@ err: if (emac->spi_lock) { vSemaphoreDelete(emac->spi_lock); } - if (emac->spi_lock) { - vSemaphoreDelete(emac->spi_lock); - } // NOTE(v.chistyakov): safe to call with NULL heap_caps_free(emac->rx_buffer); heap_caps_free(emac->tx_buffer); diff --git a/components/esp_eth/src/esp_eth_phy_ksz8851snl.c b/components/esp_eth/src/esp_eth_phy_ksz8851snl.c index c8db737227..ab5cf497ea 100644 --- a/components/esp_eth/src/esp_eth_phy_ksz8851snl.c +++ b/components/esp_eth/src/esp_eth_phy_ksz8851snl.c @@ -17,33 +17,42 @@ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -#include "ksz8851.h" - -#include "driver/gpio.h" -#include "esp_heap_caps.h" -#include "esp_log.h" -#include "esp_rom_gpio.h" #include -#include +#include "esp_check.h" +#include "esp_heap_caps.h" +#include "esp_log.h" +#include "driver/gpio.h" +#include "esp_rom_gpio.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "esp_eth.h" +#include "ksz8851.h" + + +typedef struct { + esp_eth_phy_t parent; + esp_eth_mediator_t *eth; + int32_t addr; + uint32_t reset_timeout_ms; + uint32_t autonego_timeout_ms; + eth_link_t link_status; + int reset_gpio_num; +} phy_ksz8851snl_t; + static const char *TAG = "ksz8851snl-phy"; -#define PHY_CHECK(a, str, goto_tag, ...) \ - do { \ - if (!(a)) { \ - ESP_LOGE(TAG, "%s(%d): " str, __FUNCTION__, __LINE__, ##__VA_ARGS__); \ - goto goto_tag; \ - } \ - } while (0) + static esp_err_t ksz8851_update_link_duplex_speed(phy_ksz8851snl_t *ksz8851) { + esp_err_t ret = ESP_OK; esp_eth_mediator_t *eth = ksz8851->eth; eth_speed_t speed = ETH_SPEED_10M; eth_duplex_t duplex = ETH_DUPLEX_HALF; uint32_t status; - PHY_CHECK(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1SR, &status) == ESP_OK, "P1SR read failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1SR, &status), err, TAG, "P1SR read failed"); eth_link_t link = (status & P1SR_LINK_GOOD) ? ETH_LINK_UP : ETH_LINK_DOWN; if (ksz8851->link_status != link) { if (link == ETH_LINK_UP) { @@ -61,40 +70,41 @@ static esp_err_t ksz8851_update_link_duplex_speed(phy_ksz8851snl_t *ksz8851) duplex = ETH_DUPLEX_HALF; ESP_LOGD(TAG, "duplex half"); } - PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed) == ESP_OK, "change speed failed", err); - PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex) == ESP_OK, "change duplex failed", err); + ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_SPEED, (void *)speed), err, TAG, "change speed failed"); + ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_DUPLEX, (void *)duplex), err, TAG, "change duplex failed"); } - PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link) == ESP_OK, "change link failed", err); + ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)link), err, TAG, "change link failed"); ksz8851->link_status = link; } return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_set_mediator(esp_eth_phy_t *phy, esp_eth_mediator_t *eth) { - PHY_CHECK(eth, "mediator can not be null", err); + esp_err_t ret = ESP_OK; + ESP_GOTO_ON_FALSE(eth, ESP_ERR_INVALID_ARG, err, TAG, "mediator can not be null"); phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); ksz8851->eth = eth; return ESP_OK; err: - return ESP_ERR_INVALID_ARG; + return ret; } static esp_err_t phy_ksz8851_reset(esp_eth_phy_t *phy) { + esp_err_t ret = ESP_OK; phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); ksz8851->link_status = ETH_LINK_DOWN; esp_eth_mediator_t *eth = ksz8851->eth; ESP_LOGD(TAG, "soft reset"); // NOTE(v.chistyakov): PHY_RESET bit is self-clearing - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PHYRR, PHYRR_PHY_RESET) == ESP_OK, "PHYRR write failed", - err); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PHYRR, PHYRR_PHY_RESET), err, TAG, "PHYRR write failed"); vTaskDelay(pdMS_TO_TICKS(ksz8851->reset_timeout_ms)); return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_reset_hw(esp_eth_phy_t *phy) @@ -114,64 +124,74 @@ static esp_err_t phy_ksz8851_reset_hw(esp_eth_phy_t *phy) static esp_err_t phy_ksz8851_pwrctl(esp_eth_phy_t *phy, bool enable) { + esp_err_t ret = ESP_OK; phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); esp_eth_mediator_t *eth = ksz8851->eth; if (enable) { ESP_LOGD(TAG, "normal mode"); - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PMECR, PMECR_PME_MODE_POWER_SAVING) == ESP_OK, - "PMECR write failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PMECR, PMECR_PME_MODE_POWER_SAVING), err, TAG, "PMECR write failed"); } else { ESP_LOGD(TAG, "power saving mode"); - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PMECR, PMECR_PME_MODE_NORMAL) == ESP_OK, - "PMECR write failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_PMECR, PMECR_PME_MODE_NORMAL), err, TAG, "PMECR write failed"); } return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_init(esp_eth_phy_t *phy) { + esp_err_t ret = ESP_OK; ESP_LOGD(TAG, "initializing PHY"); - PHY_CHECK(phy_ksz8851_pwrctl(phy, true) == ESP_OK, "power control failed", err); - PHY_CHECK(phy_ksz8851_reset(phy) == ESP_OK, "reset failed", err); + ESP_GOTO_ON_ERROR(phy_ksz8851_pwrctl(phy, true), err, TAG, "power control failed"); + ESP_GOTO_ON_ERROR(phy_ksz8851_reset(phy), err, TAG, "reset failed"); return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_deinit(esp_eth_phy_t *phy) { + esp_err_t ret = ESP_OK; ESP_LOGD(TAG, "deinitializing PHY"); - PHY_CHECK(phy_ksz8851_pwrctl(phy, false) == ESP_OK, "power control failed", err); + ESP_GOTO_ON_ERROR(phy_ksz8851_pwrctl(phy, false), err, TAG, "power control failed"); return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_negotiate(esp_eth_phy_t *phy) { + esp_err_t ret = ESP_OK; phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); esp_eth_mediator_t *eth = ksz8851->eth; ESP_LOGD(TAG, "restart negotiation"); uint32_t control; - PHY_CHECK(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1CR, &control) == ESP_OK, "P1CR read failed", err); - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1CR, control | P1CR_RESTART_AN) == ESP_OK, - "P1CR write failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1CR, &control), err, TAG, "P1CR read failed"); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1CR, control | P1CR_RESTART_AN), err, TAG, "P1CR write failed"); - vTaskDelay(pdMS_TO_TICKS(ksz8851->autonego_timeout_ms)); uint32_t status; - PHY_CHECK(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1SR, &status) == ESP_OK, "P1SR read failed", err); - PHY_CHECK(status & P1SR_AN_DONE, "auto-negotiation failed", err); - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1CR, control) == ESP_OK, "P1CR write failed", err); + unsigned to; + for (to = 0; to < ksz8851->autonego_timeout_ms / 10; to++) { + vTaskDelay(pdMS_TO_TICKS(10)); + ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1SR, &status), err, TAG, "P1SR read failed"); + if (status & P1SR_AN_DONE) { + break; + } + } + if (to >= ksz8851->autonego_timeout_ms / 10) { + ESP_LOGW(TAG, "Ethernet PHY auto negotiation timeout"); + } - PHY_CHECK(ksz8851_update_link_duplex_speed(ksz8851) == ESP_OK, "update link duplex speed failed", err); - ESP_LOGD(TAG, "negotiation succeded"); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1CR, control), err, TAG, "P1CR write failed"); + + ESP_GOTO_ON_ERROR(ksz8851_update_link_duplex_speed(ksz8851), err, TAG, "update link duplex speed failed"); + ESP_LOGD(TAG, "negotiation succeeded"); return ESP_OK; err: ESP_LOGD(TAG, "negotiation failed"); - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_get_link(esp_eth_phy_t *phy) @@ -190,33 +210,33 @@ static esp_err_t phy_ksz8851_set_addr(esp_eth_phy_t *phy, uint32_t addr) static esp_err_t phy_ksz8851_get_addr(esp_eth_phy_t *phy, uint32_t *addr) { - PHY_CHECK(addr, "addr can not be null", err); + esp_err_t ret = ESP_OK; + ESP_GOTO_ON_FALSE(addr, ESP_ERR_INVALID_ARG, err, TAG, "addr can not be null"); phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); *addr = ksz8851->addr; return ESP_OK; err: - return ESP_ERR_INVALID_ARG; + return ret; } static esp_err_t phy_ksz8851_advertise_pause_ability(esp_eth_phy_t *phy, uint32_t ability) { + esp_err_t ret = ESP_OK; phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent); esp_eth_mediator_t *eth = ksz8851->eth; uint32_t anar; - PHY_CHECK(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1ANAR, &anar) == ESP_OK, "P1ANAR read failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1ANAR, &anar), err, TAG, "P1ANAR read failed"); if (ability) { - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1ANAR, anar | P1ANAR_PAUSE) == ESP_OK, - "P1ANAR write failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1ANAR, anar | P1ANAR_PAUSE), err, TAG, "P1ANAR write failed"); ESP_LOGD(TAG, "start advertising pause ability"); } else { - PHY_CHECK(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1ANAR, anar & ~P1ANAR_PAUSE) == ESP_OK, - "P1ANAR write failed", err); + ESP_GOTO_ON_ERROR(eth->phy_reg_write(eth, ksz8851->addr, KSZ8851_P1ANAR, anar & ~P1ANAR_PAUSE), err, TAG, "P1ANAR write failed"); ESP_LOGD(TAG, "stop advertising pause ability"); } return ESP_OK; err: - return ESP_FAIL; + return ret; } static esp_err_t phy_ksz8851_del(esp_eth_phy_t *phy) @@ -229,9 +249,10 @@ static esp_err_t phy_ksz8851_del(esp_eth_phy_t *phy) esp_eth_phy_t *esp_eth_phy_new_ksz8851snl(const eth_phy_config_t *config) { - PHY_CHECK(config, "config can not be null", err); + esp_eth_phy_t *ret = NULL; + ESP_GOTO_ON_FALSE(config, NULL, err, TAG, "config can not be null"); phy_ksz8851snl_t *ksz8851 = calloc(1, sizeof(phy_ksz8851snl_t)); - PHY_CHECK(ksz8851, "no mem for PHY instance", err); + ESP_GOTO_ON_FALSE(ksz8851, NULL, err, TAG, "no mem for PHY instance"); ksz8851->addr = config->phy_addr; ksz8851->reset_timeout_ms = config->reset_timeout_ms; ksz8851->reset_gpio_num = config->reset_gpio_num; @@ -251,5 +272,5 @@ esp_eth_phy_t *esp_eth_phy_new_ksz8851snl(const eth_phy_config_t *config) ksz8851->parent.del = phy_ksz8851_del; return &(ksz8851->parent); err: - return NULL; + return ret; } diff --git a/components/esp_eth/src/ksz8851.h b/components/esp_eth/src/ksz8851.h index 4cb86acd4b..44c1087b6c 100644 --- a/components/esp_eth/src/ksz8851.h +++ b/components/esp_eth/src/ksz8851.h @@ -18,13 +18,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. #pragma once -#include "driver/spi_master.h" -#include "esp_eth.h" -#include "freertos/FreeRTOS.h" -#include "freertos/semphr.h" -#include "freertos/task.h" -#include typedef enum { KSZ8851_CCR = 0x08, ///< Chip Configuration Register @@ -366,25 +360,3 @@ typedef enum { P1SR_PARTNER_10BT_FULL_DUPLEX_CAPABILITY = 0x0002U, ///< RO Partner 10BT full-duplex capability P1SR_PARTNER_10BT_HALF_DUPLEX_CAPABILITY = 0x0001U, ///< RO Partner 10BT half-duplex capability } ksz8851_register_bits_t; - -typedef struct { - esp_eth_mac_t parent; - esp_eth_mediator_t *eth; - spi_device_handle_t spi_hdl; - SemaphoreHandle_t spi_lock; - TaskHandle_t rx_task_hdl; - uint32_t sw_reset_timeout_ms; - int int_gpio_num; - uint8_t *rx_buffer; - uint8_t *tx_buffer; -} emac_ksz8851snl_t; - -typedef struct { - esp_eth_phy_t parent; - esp_eth_mediator_t *eth; - int32_t addr; - uint32_t reset_timeout_ms; - uint32_t autonego_timeout_ms; - eth_link_t link_status; - int reset_gpio_num; -} phy_ksz8851snl_t;