From a7cf50eb03d483a6395038bf6aa9006f86f9e3c9 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 11:32:20 +0200 Subject: [PATCH 1/8] drivers/stm32_eth: Code style fixes - Drop incorrect use of `volatile` - Fix missing spaces and indention --- drivers/stm32_eth/stm32_eth.c | 38 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/stm32_eth/stm32_eth.c b/drivers/stm32_eth/stm32_eth.c index 9dc85a1db1..072402a958 100644 --- a/drivers/stm32_eth/stm32_eth.c +++ b/drivers/stm32_eth/stm32_eth.c @@ -39,14 +39,14 @@ int stm32_eth_send(const struct iolist *iolist); int stm32_eth_get_rx_status_owned(void); static void _isr(netdev_t *netdev) { - if(stm32_eth_get_rx_status_owned()) { + if (stm32_eth_get_rx_status_owned()) { netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); } } void isr_eth(void) { - volatile unsigned tmp = ETH->DMASR; + unsigned tmp = ETH->DMASR; if ((tmp & ETH_DMASR_TS)) { ETH->DMASR = ETH_DMASR_TS | ETH_DMASR_NIS; @@ -70,7 +70,7 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info) { (void)info; (void)netdev; - if(!stm32_eth_get_rx_status_owned()){ + if (!stm32_eth_get_rx_status_owned()){ mutex_lock(&_rx); } int ret = stm32_eth_receive_blocking((char *)buf, len); @@ -104,14 +104,14 @@ static int _set(netdev_t *dev, netopt_t opt, const void *value, size_t max_len) int res = -1; switch (opt) { - case NETOPT_ADDRESS: - assert(max_len >= ETHERNET_ADDR_LEN); - stm32_eth_set_mac((char *)value); - res = ETHERNET_ADDR_LEN; - break; - default: - res = netdev_eth_set(dev, opt, value, max_len); - break; + case NETOPT_ADDRESS: + assert(max_len >= ETHERNET_ADDR_LEN); + stm32_eth_set_mac((char *)value); + res = ETHERNET_ADDR_LEN; + break; + default: + res = netdev_eth_set(dev, opt, value, max_len); + break; } return res; @@ -122,14 +122,14 @@ static int _get(netdev_t *dev, netopt_t opt, void *value, size_t max_len) int res = -1; switch (opt) { - case NETOPT_ADDRESS: - assert(max_len >= ETHERNET_ADDR_LEN); - stm32_eth_get_mac((char *)value); - res = ETHERNET_ADDR_LEN; - break; - default: - res = netdev_eth_get(dev, opt, value, max_len); - break; + case NETOPT_ADDRESS: + assert(max_len >= ETHERNET_ADDR_LEN); + stm32_eth_get_mac((char *)value); + res = ETHERNET_ADDR_LEN; + break; + default: + res = netdev_eth_get(dev, opt, value, max_len); + break; } return res; From a5dbec33d998cacb6fb76d48fccc72b1ab173818 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 11:37:23 +0200 Subject: [PATCH 2/8] cpu/stm32/periph_eth: Cleanup & fix DMA descriptor - Add missing `volatile` to DMA descriptor, as memory is also accessed by the DMA without knowledge of the compiler - Dropped `__attribute__((packed))` from DMA descriptor - The DMA descriptor fields need to be aligned on word boundries to properly function - The compiler can now more efficiently access the fields (safes ~300 B ROM) - Moved the DMA descriptor struct and the flags to `periph_cpu.h` - This allows Doxygen documentation being build for it - Those types and fields are needed for a future PTP implementation - Renamed DMA descriptor flags - They now reflect to which field in the DMA descriptor they refer to, so that confusion is avoided - Added documentation to the DMA descriptor and the corresponding flags --- cpu/stm32/include/periph_cpu.h | 91 ++++++++++++++++++++++++++++++++++ cpu/stm32/periph/eth.c | 59 +++++++--------------- 2 files changed, 109 insertions(+), 41 deletions(-) diff --git a/cpu/stm32/include/periph_cpu.h b/cpu/stm32/include/periph_cpu.h index b3fad57d19..f93b6e94dc 100644 --- a/cpu/stm32/include/periph_cpu.h +++ b/cpu/stm32/include/periph_cpu.h @@ -1017,6 +1017,97 @@ typedef struct { supported by all modes. */ } eth_conf_t; +/** + * @brief Layout of enhanced RX/TX DMA descriptor + * + * @note Don't confuse this with the normal RX/TX descriptor format. + * @warning The content of the status and control bits is different for RX and + * TX DMA descriptors + */ +typedef struct eth_dma_desc { + volatile uint32_t status; /**< Mostly status bits, some control bits */ + volatile uint32_t control; /**< Control bits */ + char * volatile buffer_addr; /**< RX/TX buffer */ + struct eth_dma_desc * volatile desc_next; /**< Address of next DMA descriptor */ + volatile uint32_t reserved1_ext; /**< RX: Extended status, TX: reserved */ + volatile uint32_t reserved2; /**< Reserved for future use */ + /** + * @brief Sub-second part of PTP timestamp of transmitted / sent frame + * + * For TX: If PTP timestamping is enabled and the TTSE bit in the + * transmit descriptor word 0 (struct eth_dma_desc::status) is set, the + * MAC will store the PTP timestamp of when the Start of Frame Delimiter + * was sent. The TTSS bit is send by the hardware if the timestamp was + * correctly set. + * + * For RX: If PTP timestamping is enabled, the timestamp of all received + * frames is captured. + */ + volatile uint32_t ts_low; + volatile uint32_t ts_high; /**< Second part of PTP timestamp */ +} edma_desc_t; + +/** + * @name Flags in the status word of the Ethernet enhanced RX DMA descriptor + * @{ + */ +#define RX_DESC_STAT_LS (BIT8) /**< If set, descriptor is the last of a frame */ +#define RX_DESC_STAT_FS (BIT9) /**< If set, descriptor is the first of a frame */ +/** + * @brief Frame length + * + * The length of the frame in host memory order including CRC. Only valid if + * @ref RX_DESC_STAT_LS is set and @ref RX_DESC_STAT_DE is not set. + */ +#define RX_DESC_STAT_FL (0x3FFF0000) /* bits 16-29 */ +#define RX_DESC_STAT_DE (BIT14) /**< If set, a frame too large to fit buffers given by descriptors was received */ +#define RX_DESC_STAT_OWN (BIT31) /**< If set, descriptor is owned by DMA, otherwise by CPU */ +/** @} */ +/** + * @name Flags in the control word of the Ethernet enhanced RX DMA descriptor + * @{ + */ +/** + * @brief Indicates if RDES3 points to the next DMA descriptor (1), or to a second buffer (0) + * + * If the bit is set, RDES3 (@ref edma_desc_t::desc_next) will point to the + * next DMA descriptor rather than to a second frame-segment buffer. This is + * always set by the driver + */ +#define RX_DESC_CTRL_RCH (BIT14) +/** @} */ +/** + * @name Flags in the status word of the Ethernet enhanced TX DMA descriptor + * @{ + */ +#define TX_DESC_STAT_TTSS (BIT17) /**< If set, the descriptor contains a valid PTP timestamp */ +/** + * @brief Indicates if TDES3 points to the next DMA descriptor (1), or to a second buffer (0) + * + * If the bit is set, TDES3 (@ref edma_desc_t::desc_next) will point to the + * next DMA descriptor rather than to a second frame-segment buffer. This is + * always set by the driver + */ +#define TX_DESC_STAT_TCH (BIT20) +/** + * @brief Checksum insertion control + * + * | Value | Meaning | + * |:------ |:----------------------------------------------------------------------------- | + * | `0b00` | Checksum insertion disabled | + * | `0b01` | Calculate and insert checksum in IPv4 header | + * | `0b10` | Calculate and insert IPv4 checksum, insert pre-calculated payload checksum | + * | `0b11 | Calculated and insert both IPv4 and payload checksum | + */ +#define TX_DESC_STAT_CIC (BIT22 | BIT23) +#define TX_DESC_STAT_TTSE (BIT25) /**< If set, an PTP timestamp is added to the descriptor after TX completed */ +#define TX_DESC_STAT_FS (BIT28) /**< If set, buffer contains first segment of frame to transmit */ +#define TX_DESC_STAT_LS (BIT29) /**< If set, buffer contains last segment of frame to transmit */ +#define TX_DESC_STAT_IC (BIT30) /**< If set, trigger IRQ on completion */ +#define TX_DESC_STAT_OWN (BIT31) /**< If set, descriptor is owned by DMA, otherwise by CPU */ +/** @} */ + + /** * @name Ethernet PHY Common Registers * @{ diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 9ea48c42b0..3392c41d14 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -19,6 +19,7 @@ */ #include +#include "bitarithm.h" #include "mutex.h" #include "luid.h" @@ -30,7 +31,6 @@ #define ENABLE_DEBUG (0) #include "debug.h" - /* Set the value of the divider with the clock configured */ #if !defined(CLOCK_CORECLOCK) || CLOCK_CORECLOCK < (20000000U) #error This peripheral requires a CORECLOCK of at least 20MHz @@ -46,31 +46,6 @@ #define CLOCK_RANGE ETH_MACMIIAR_CR_Div102 #endif /* CLOCK_CORECLOCK < (20000000U) */ -/* Internal flags for the DMA descriptors */ -#define DESC_OWN (0x80000000) -#define RX_DESC_FL (0x3FFF0000) -#define RX_DESC_FS (0x00000200) -#define RX_DESC_LS (0x00000100) -#define RX_DESC_RCH (0x00004000) -#define TX_DESC_TCH (0x00100000) -#define TX_DESC_IC (0x40000000) -#define TX_DESC_CIC (0x00C00000) -#define TX_DESC_LS (0x20000000) -#define TX_DESC_FS (0x10000000) - -struct eth_dma_desc { - uint32_t status; - uint32_t control; - char *buffer_addr; - struct eth_dma_desc *desc_next; - uint32_t reserved1_ext; - uint32_t reserved2; - uint32_t ts_low; - uint32_t ts_high; -} __attribute__((packed)); - -typedef struct eth_dma_desc edma_desc_t; - /* Descriptors */ static edma_desc_t rx_desc[ETH_RX_BUFFER_COUNT]; static edma_desc_t tx_desc[ETH_TX_BUFFER_COUNT]; @@ -142,8 +117,8 @@ static void _init_buffer(void) { int i; for (i = 0; i < ETH_RX_BUFFER_COUNT; i++) { - rx_desc[i].status = DESC_OWN; - rx_desc[i].control = RX_DESC_RCH | (ETH_RX_BUFFER_SIZE & 0x0fff); + rx_desc[i].status = RX_DESC_STAT_OWN; + rx_desc[i].control = RX_DESC_CTRL_RCH | (ETH_RX_BUFFER_SIZE & 0x0fff); rx_desc[i].buffer_addr = &rx_buffer[i][0]; if((i+1) < ETH_RX_BUFFER_COUNT) { rx_desc[i].desc_next = &rx_desc[i + 1]; @@ -152,7 +127,7 @@ static void _init_buffer(void) rx_desc[i - 1].desc_next = &rx_desc[0]; for (i = 0; i < ETH_TX_BUFFER_COUNT; i++) { - tx_desc[i].status = TX_DESC_TCH | TX_DESC_CIC; + tx_desc[i].status = TX_DESC_STAT_TCH | TX_DESC_STAT_CIC; tx_desc[i].buffer_addr = &tx_buffer[i][0]; if ((i + 1) < ETH_RX_BUFFER_COUNT) { tx_desc[i].desc_next = &tx_desc[i + 1]; @@ -164,8 +139,8 @@ static void _init_buffer(void) rx_curr = &rx_desc[0]; tx_curr = &tx_desc[0]; - ETH->DMARDLAR = (uint32_t)rx_curr; - ETH->DMATDLAR = (uint32_t)tx_curr; + ETH->DMARDLAR = (uintptr_t)rx_curr; + ETH->DMATDLAR = (uintptr_t)tx_curr; } int stm32_eth_init(void) @@ -262,7 +237,7 @@ int stm32_eth_send(const struct iolist *iolist) } /* block until there's an available descriptor */ - while (tx_curr->status & DESC_OWN) { + while (tx_curr->status & TX_DESC_STAT_OWN) { DEBUG("stm32_eth: not avail\n"); } @@ -271,8 +246,10 @@ int stm32_eth_send(const struct iolist *iolist) dma_acquire(eth_config.dma); for (; iolist; iolist = iolist->iol_next) { - ret += dma_transfer(eth_config.dma, eth_config.dma_chan, iolist->iol_base, - tx_curr->buffer_addr+ret, iolist->iol_len, DMA_MEM_TO_MEM, DMA_INC_BOTH_ADDR); + ret += dma_transfer( + eth_config.dma, eth_config.dma_chan, + iolist->iol_base, tx_curr->buffer_addr + ret, iolist->iol_len, + DMA_MEM_TO_MEM, DMA_INC_BOTH_ADDR); } dma_release(eth_config.dma); @@ -283,11 +260,11 @@ int stm32_eth_send(const struct iolist *iolist) tx_curr->control = (len & 0x1fff); /* set flags for first and last frames */ - tx_curr->status |= TX_DESC_FS; - tx_curr->status |= TX_DESC_LS | TX_DESC_IC; + tx_curr->status |= TX_DESC_STAT_FS; + tx_curr->status |= TX_DESC_STAT_LS | TX_DESC_STAT_IC; /* give the descriptors to the DMA */ - tx_curr->status |= DESC_OWN; + tx_curr->status |= TX_DESC_STAT_OWN; tx_curr = tx_curr->desc_next; /* start tx */ @@ -305,7 +282,7 @@ static int _try_receive(char *data, int max_len, int block) for (int i = 0; i < ETH_RX_BUFFER_COUNT && len == 0; i++) { /* try receiving, if the block is set, simply wait for the rest of * the packet to complete, otherwise just break */ - while (p->status & DESC_OWN) { + while (p->status & RX_DESC_STAT_OWN) { if (!block) { break; } @@ -313,7 +290,7 @@ static int _try_receive(char *data, int max_len, int block) /* amount of data to copy */ copy = ETH_RX_BUFFER_SIZE; - if (p->status & (RX_DESC_LS | RX_DESC_FL)) { + if (p->status & (RX_DESC_STAT_LS | RX_DESC_STAT_FL)) { len = ((p->status >> 16) & 0x3FFF) - 4; copy = len - copied; } @@ -327,7 +304,7 @@ static int _try_receive(char *data, int max_len, int block) else if (max_len < copy) { len = -1; } - p->status = DESC_OWN; + p->status = RX_DESC_STAT_OWN; } p = p->desc_next; } @@ -351,7 +328,7 @@ int stm32_eth_receive_blocking(char *data, unsigned max_len) int stm32_eth_get_rx_status_owned(void) { - return (!(rx_curr->status & DESC_OWN)); + return (!(rx_curr->status & RX_DESC_STAT_OWN)); } void stm32_eth_isr_eth_wkup(void) From 53375f04bf79af72646462d0cf8cfc089fc24019 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 11:44:08 +0200 Subject: [PATCH 3/8] cpu/stm32/periph_eth: Optimize / fix flush - Added missing wait for TX flush - Grouped access to the same registers of the Ethernet PHY to reduce accesses. (The compiler won't optimize accesses to `volatile`, as defined in the C standard.) --- cpu/stm32/periph/eth.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 3392c41d14..3cdb06d03b 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -210,13 +210,15 @@ int stm32_eth_init(void) NVIC_EnableIRQ(ETH_IRQn); ETH->DMAIER |= ETH_DMAIER_NISE | ETH_DMAIER_TIE | ETH_DMAIER_RIE; - /* enable */ - ETH->MACCR |= ETH_MACCR_TE; + /* enable transmitter and receiver */ + ETH->MACCR |= ETH_MACCR_TE | ETH_MACCR_RE; + /* flush transmit FIFO */ ETH->DMAOMR |= ETH_DMAOMR_FTF; - ETH->MACCR |= ETH_MACCR_RE; + /* wait for FIFO flushing to complete */ + while (ETH->DMAOMR & ETH_DMAOMR_FTF) { } - ETH->DMAOMR |= ETH_DMAOMR_ST; - ETH->DMAOMR |= ETH_DMAOMR_SR; + /* enable DMA TX and RX */ + ETH->DMAOMR |= ETH_DMAOMR_ST | ETH_DMAOMR_SR; /* configure speed, do it at the end so the PHY had time to * reset */ From 932c311ee27507ec4f3030e90c0049f0903a3ddf Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 17:43:01 +0200 Subject: [PATCH 4/8] cpu/stm32/periph_eth: Fix RX logic If any incoming frame is bigger than a single DMA buffer, the Ethernet DMA will split the content and use multiple DMA buffers instead. But only the DMA descriptor of the last Ethernet frame segment will contain the frame length. Previously, the frame length calculation, reassembly of the frame, and the freeing of DMA descriptors was completely broken and only worked in case the received frame was small enough to fit into one DMA buffer. This is now fixed, so that smaller DMA buffers can safely be used now. Additionally the interface was simplified: Previously two receive flavors were implemented, with only one ever being used. None of those function was public due to missing declarations in headers. The unused interface was dropped and the remaining was streamlined to better fit the use case. --- cpu/stm32/include/periph_cpu.h | 5 ++ cpu/stm32/periph/eth.c | 114 +++++++++++++++++++++------------ drivers/stm32_eth/stm32_eth.c | 4 +- 3 files changed, 80 insertions(+), 43 deletions(-) diff --git a/cpu/stm32/include/periph_cpu.h b/cpu/stm32/include/periph_cpu.h index f93b6e94dc..bdace91256 100644 --- a/cpu/stm32/include/periph_cpu.h +++ b/cpu/stm32/include/periph_cpu.h @@ -1080,6 +1080,10 @@ typedef struct eth_dma_desc { * @name Flags in the status word of the Ethernet enhanced TX DMA descriptor * @{ */ +#define TX_DESC_STAT_UF (BIT1) /**< If set, an underflow occurred while sending */ +#define TX_DESC_STAT_EC (BIT8) /**< If set, TX was aborted due to excessive collisions (half-duplex only) */ +#define TX_DESC_STAT_NC (BIT10) /**< If set, no carrier was detected (TX aborted) */ +#define TX_DESC_STAT_ES (BIT15) /**< If set, one or more error occurred */ #define TX_DESC_STAT_TTSS (BIT17) /**< If set, the descriptor contains a valid PTP timestamp */ /** * @brief Indicates if TDES3 points to the next DMA descriptor (1), or to a second buffer (0) @@ -1089,6 +1093,7 @@ typedef struct eth_dma_desc { * always set by the driver */ #define TX_DESC_STAT_TCH (BIT20) +#define TX_DESC_STAT_TER (BIT21) /**< If set, DMA will return to first descriptor in ring afterwards */ /** * @brief Checksum insertion control * diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 3cdb06d03b..93fa145670 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2016 TriaGnoSys GmbH + * 2020 Otto-von-Guericke-Universität Magdeburg * * This file is subject to the terms and conditions of the GNU Lesser General * Public License v2.1. See the file LICENSE in the top level directory for more @@ -14,9 +15,11 @@ * @brief Low-level ETH driver implementation * * @author Víctor Ariño + * @author Marian Buschsieweke * * @} */ +#include #include #include "bitarithm.h" @@ -46,6 +49,8 @@ #define CLOCK_RANGE ETH_MACMIIAR_CR_Div102 #endif /* CLOCK_CORECLOCK < (20000000U) */ +#define MIN(a, b) (((a) <= (b)) ? (a) : (b)) + /* Descriptors */ static edma_desc_t rx_desc[ETH_RX_BUFFER_COUNT]; static edma_desc_t tx_desc[ETH_TX_BUFFER_COUNT]; @@ -115,7 +120,7 @@ void stm32_eth_set_mac(const char *mac) /** Initialization of the DMA descriptors to be used */ static void _init_buffer(void) { - int i; + size_t i; for (i = 0; i < ETH_RX_BUFFER_COUNT; i++) { rx_desc[i].status = RX_DESC_STAT_OWN; rx_desc[i].control = RX_DESC_CTRL_RCH | (ETH_RX_BUFFER_SIZE & 0x0fff); @@ -274,58 +279,85 @@ int stm32_eth_send(const struct iolist *iolist) return ret; } -static int _try_receive(char *data, int max_len, int block) +static ssize_t get_rx_frame_size(void) { - int copy, len = 0; - int copied = 0; - int drop = (data || max_len > 0); - - edma_desc_t *p = rx_curr; - for (int i = 0; i < ETH_RX_BUFFER_COUNT && len == 0; i++) { - /* try receiving, if the block is set, simply wait for the rest of - * the packet to complete, otherwise just break */ - while (p->status & RX_DESC_STAT_OWN) { - if (!block) { - break; - } + edma_desc_t *i = rx_curr; + uint32_t status; + while (1) { + /* Wait until DMA gave up control over descriptor */ + while ((status = i->status) & RX_DESC_STAT_OWN) { } + DEBUG("stm32_eth: get_rx_frame_size(): FS=%c, LS=%c, DE=%c, FL=%lu\n", + (status & RX_DESC_STAT_FS) ? '1' : '0', + (status & RX_DESC_STAT_LS) ? '1' : '0', + (status & RX_DESC_STAT_DE) ? '1' : '0', + ((status >> 16) & 0x3fff) - ETHERNET_FCS_LEN); + if (status & RX_DESC_STAT_LS) { + break; } - - /* amount of data to copy */ - copy = ETH_RX_BUFFER_SIZE; - if (p->status & (RX_DESC_STAT_LS | RX_DESC_STAT_FL)) { - len = ((p->status >> 16) & 0x3FFF) - 4; - copy = len - copied; - } - - if (drop) { - /* copy the data if possible */ - if (data && max_len >= copy) { - memcpy(data, p->buffer_addr, copy); - max_len -= copy; - } - else if (max_len < copy) { - len = -1; - } - p->status = RX_DESC_STAT_OWN; - } - p = p->desc_next; + i = i->desc_next; } - if (drop) { - rx_curr = p; + if (status & RX_DESC_STAT_DE) { + return -1; } - return len; + /* bits 16-29 contain the frame length including 4 B frame check sequence */ + return ((status >> 16) & 0x3fff) - ETHERNET_FCS_LEN; } -int stm32_eth_try_receive(char *data, unsigned max_len) +static void drop_frame_and_update_rx_curr(void) { - return _try_receive(data, max_len, 0); + while (1) { + uint32_t old_status = rx_curr->status; + /* hand over old descriptor to DMA */ + rx_curr->status = RX_DESC_STAT_OWN; + rx_curr = rx_curr->desc_next; + if (old_status & RX_DESC_STAT_LS) { + /* reached last DMA descriptor of frame ==> done */ + return; + } + } } -int stm32_eth_receive_blocking(char *data, unsigned max_len) +int stm32_eth_receive(void *buf, size_t max_len) { - return _try_receive(data, max_len, 1); + char *data = buf; + /* Determine the size of received frame. The frame might span multiple + * DMA buffers */ + ssize_t size = get_rx_frame_size(); + + if (size == -1) { + DEBUG("stm32_eth: Received frame was too large for DMA buffer(s)\n"); + drop_frame_and_update_rx_curr(); + return -EOVERFLOW; + } + + if (!buf) { + if (max_len) { + DEBUG("stm32_eth: Dropping frame as requested by upper layer\n"); + drop_frame_and_update_rx_curr(); + } + return size; + } + + if (max_len < (size_t)size) { + DEBUG("stm32_eth: Buffer provided by upper layer is too small\n"); + drop_frame_and_update_rx_curr(); + return -ENOBUFS; + } + + size_t remain = size; + while (remain) { + size_t chunk = MIN(remain, ETH_RX_BUFFER_SIZE); + memcpy(data, rx_curr->buffer_addr, chunk); + data += chunk; + remain -= chunk; + /* Hand over descriptor to DMA */ + rx_curr->status = RX_DESC_STAT_OWN; + rx_curr = rx_curr->desc_next; + } + + return size; } int stm32_eth_get_rx_status_owned(void) diff --git a/drivers/stm32_eth/stm32_eth.c b/drivers/stm32_eth/stm32_eth.c index 072402a958..9091438337 100644 --- a/drivers/stm32_eth/stm32_eth.c +++ b/drivers/stm32_eth/stm32_eth.c @@ -34,7 +34,7 @@ netdev_t *_netdev; void stm32_eth_set_mac(const char *mac); void stm32_eth_get_mac(char *out); int stm32_eth_init(void); -int stm32_eth_receive_blocking(char *data, unsigned max_len); +int stm32_eth_receive(void *data, size_t max_len); int stm32_eth_send(const struct iolist *iolist); int stm32_eth_get_rx_status_owned(void); @@ -73,7 +73,7 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info) if (!stm32_eth_get_rx_status_owned()){ mutex_lock(&_rx); } - int ret = stm32_eth_receive_blocking((char *)buf, len); + int ret = stm32_eth_receive(buf, len); DEBUG("stm32_eth_netdev: _recev: %d\n", ret); return ret; From 51fe77afa4ec6b07af55c405c9a85b774c166499 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 17:53:06 +0200 Subject: [PATCH 5/8] cpu/stm32/periph_eth: configurable buffer size 1. Move buffer configuration from boards to cpu/stm32 2. Allow overwriting buffer configuration - If the default configuration ever needs touching, this will be due to a use case and should be done by the application rather than the board 3. Reduce default RX buffer size - Now that handling of frames split up into multiple DMA descriptors works, we can make use of this Note: With the significantly smaller RX buffers the driver will now perform much worse when receiving data at maximum throughput. But as long as frames are small (which is to be expected for IoT or boarder gateway scenarios) the performance should not be affected. --- boards/nucleo-f207zg/include/periph_conf.h | 6 ------ boards/nucleo-f767zi/include/periph_conf.h | 6 ------ cpu/stm32/periph/eth.c | 25 ++++++++++++++++++++++ 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/boards/nucleo-f207zg/include/periph_conf.h b/boards/nucleo-f207zg/include/periph_conf.h index 11027824fa..b79b321436 100644 --- a/boards/nucleo-f207zg/include/periph_conf.h +++ b/boards/nucleo-f207zg/include/periph_conf.h @@ -274,12 +274,6 @@ static const eth_conf_t eth_config = { } }; -#define ETH_RX_BUFFER_COUNT (4) -#define ETH_TX_BUFFER_COUNT (4) - -#define ETH_RX_BUFFER_SIZE (1524) -#define ETH_TX_BUFFER_SIZE (1524) - #define ETH_DMA_ISR isr_dma2_stream0 /** @} */ diff --git a/boards/nucleo-f767zi/include/periph_conf.h b/boards/nucleo-f767zi/include/periph_conf.h index 4c5398cd51..c6589560e4 100644 --- a/boards/nucleo-f767zi/include/periph_conf.h +++ b/boards/nucleo-f767zi/include/periph_conf.h @@ -178,12 +178,6 @@ static const eth_conf_t eth_config = { } }; -#define ETH_RX_BUFFER_COUNT (4) -#define ETH_TX_BUFFER_COUNT (4) - -#define ETH_RX_BUFFER_SIZE (1524) -#define ETH_TX_BUFFER_SIZE (1524) - #define ETH_DMA_ISR isr_dma2_stream0 /** @} */ diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 93fa145670..75431fbc2a 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -49,6 +49,31 @@ #define CLOCK_RANGE ETH_MACMIIAR_CR_Div102 #endif /* CLOCK_CORECLOCK < (20000000U) */ +/* Default DMA buffer setup */ +#ifndef ETH_RX_BUFFER_COUNT +#define ETH_RX_BUFFER_COUNT (6U) +#endif +#ifndef ETH_TX_BUFFER_COUNT +#define ETH_TX_BUFFER_COUNT (4U) +#endif +#ifndef ETH_TX_BUFFER_SIZE +#define ETH_TX_BUFFER_SIZE (1524U) +#endif +#ifndef ETH_RX_BUFFER_SIZE +#define ETH_RX_BUFFER_SIZE (256U) +#endif + +#if (ETH_RX_BUFFER_SIZE % 16) != 0 +/* For compatibility with 128bit memory interfaces, the buffer size needs to + * be a multiple of 16 Byte. For 64 bit memory interfaces need the size to be + * a multiple of 8 Byte, for 32 bit a multiple of 4 byte is sufficient. */ +#warning "ETH_RX_BUFFER_SIZE is not a multiple of 16. (See comment above.)" +#endif + +#if ETH_RX_BUFFER_COUNT * ETH_RX_BUFFER_SIZE < 1524U +#warning "Total RX buffers lower than MTU, you won't receive huge frames!" +#endif + #define MIN(a, b) (((a) <= (b)) ? (a) : (b)) /* Descriptors */ From 28ed07d6e33e5718ca3094c2c4cfaadad9864995 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Fri, 24 Jul 2020 23:32:42 +0200 Subject: [PATCH 6/8] cpu/stm32/periph_eth: zero-copy TX (-6 KiB RAM) The Ethernet DMA is capable of collecting a frame from multiple chunks, just like the send function of the netdev interface passes. The send function was rewritten to just set up the Ethernet DMA up to collect the outgoing frame while sending. As a result, the send function blocks until the frame is sent to keep control over the buffers. This frees 6 KiB of RAM previously used for TX buffers. --- cpu/stm32/periph/eth.c | 140 +++++++++++++++++----------------- drivers/stm32_eth/stm32_eth.c | 40 +++------- 2 files changed, 77 insertions(+), 103 deletions(-) diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 75431fbc2a..40ec778e36 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -50,14 +50,11 @@ #endif /* CLOCK_CORECLOCK < (20000000U) */ /* Default DMA buffer setup */ -#ifndef ETH_RX_BUFFER_COUNT -#define ETH_RX_BUFFER_COUNT (6U) +#ifndef ETH_RX_DESCRIPTOR_COUNT +#define ETH_RX_DESCRIPTOR_COUNT (6U) #endif -#ifndef ETH_TX_BUFFER_COUNT -#define ETH_TX_BUFFER_COUNT (4U) -#endif -#ifndef ETH_TX_BUFFER_SIZE -#define ETH_TX_BUFFER_SIZE (1524U) +#ifndef ETH_TX_DESCRIPTOR_COUNT +#define ETH_TX_DESCRIPTOR_COUNT (8U) #endif #ifndef ETH_RX_BUFFER_SIZE #define ETH_RX_BUFFER_SIZE (256U) @@ -70,21 +67,22 @@ #warning "ETH_RX_BUFFER_SIZE is not a multiple of 16. (See comment above.)" #endif -#if ETH_RX_BUFFER_COUNT * ETH_RX_BUFFER_SIZE < 1524U +#if ETH_RX_DESCRIPTOR_COUNT * ETH_RX_BUFFER_SIZE < 1524U #warning "Total RX buffers lower than MTU, you won't receive huge frames!" #endif #define MIN(a, b) (((a) <= (b)) ? (a) : (b)) -/* Descriptors */ -static edma_desc_t rx_desc[ETH_RX_BUFFER_COUNT]; -static edma_desc_t tx_desc[ETH_TX_BUFFER_COUNT]; -static edma_desc_t *rx_curr; -static edma_desc_t *tx_curr; +/* Synchronization between IRQ and thread context */ +mutex_t stm32_eth_tx_completed = MUTEX_INIT_LOCKED; -/* Buffers */ -static char rx_buffer[ETH_RX_BUFFER_COUNT][ETH_RX_BUFFER_SIZE]; -static char tx_buffer[ETH_TX_BUFFER_COUNT][ETH_TX_BUFFER_SIZE]; +/* Descriptors */ +static edma_desc_t rx_desc[ETH_RX_DESCRIPTOR_COUNT]; +static edma_desc_t tx_desc[ETH_TX_DESCRIPTOR_COUNT]; +static edma_desc_t *rx_curr; + +/* RX Buffers */ +static char rx_buffer[ETH_RX_DESCRIPTOR_COUNT][ETH_RX_BUFFER_SIZE]; /** Read or write a phy register, to write the register ETH_MACMIIAR_MW is to * be passed as the higher nibble of the value */ @@ -146,31 +144,25 @@ void stm32_eth_set_mac(const char *mac) static void _init_buffer(void) { size_t i; - for (i = 0; i < ETH_RX_BUFFER_COUNT; i++) { + for (i = 0; i < ETH_RX_DESCRIPTOR_COUNT; i++) { rx_desc[i].status = RX_DESC_STAT_OWN; rx_desc[i].control = RX_DESC_CTRL_RCH | (ETH_RX_BUFFER_SIZE & 0x0fff); rx_desc[i].buffer_addr = &rx_buffer[i][0]; - if((i+1) < ETH_RX_BUFFER_COUNT) { + if ((i + 1) < ETH_RX_DESCRIPTOR_COUNT) { rx_desc[i].desc_next = &rx_desc[i + 1]; } } rx_desc[i - 1].desc_next = &rx_desc[0]; - for (i = 0; i < ETH_TX_BUFFER_COUNT; i++) { - tx_desc[i].status = TX_DESC_STAT_TCH | TX_DESC_STAT_CIC; - tx_desc[i].buffer_addr = &tx_buffer[i][0]; - if ((i + 1) < ETH_RX_BUFFER_COUNT) { - tx_desc[i].desc_next = &tx_desc[i + 1]; - } + for (i = 0; i < ETH_TX_DESCRIPTOR_COUNT - 1; i++) { + tx_desc[i].desc_next = &tx_desc[i + 1]; } - - tx_desc[i - 1].desc_next = &tx_desc[0]; + tx_desc[ETH_RX_DESCRIPTOR_COUNT - 1].desc_next = &tx_desc[0]; rx_curr = &rx_desc[0]; - tx_curr = &tx_desc[0]; ETH->DMARDLAR = (uintptr_t)rx_curr; - ETH->DMATDLAR = (uintptr_t)tx_curr; + ETH->DMATDLAR = (uintptr_t)&tx_desc[0]; } int stm32_eth_init(void) @@ -259,49 +251,58 @@ int stm32_eth_init(void) int stm32_eth_send(const struct iolist *iolist) { - unsigned len = iolist_size(iolist); - int ret = 0; + unsigned bytes_to_send = iolist_size(iolist); + /* Input must not be bigger than maximum allowed frame length */ + assert(bytes_to_send <= ETHERNET_FRAME_LEN); + /* This API is not thread safe, check that no other thread is sending */ + assert(!(tx_desc[0].status & TX_DESC_STAT_OWN)); + /* We cannot send more chunks than allocated descriptors */ + assert(iolist_count(iolist) <= ETH_TX_DESCRIPTOR_COUNT); - /* safety check */ - if (len > ETH_TX_BUFFER_SIZE) { - DEBUG("stm32_eth: Error iolist_size > ETH_TX_BUFFER_SIZE\n"); - return -1; + for (unsigned i = 0; iolist; iolist = iolist->iol_next, i++) { + tx_desc[i].control = iolist->iol_len; + tx_desc[i].buffer_addr = iolist->iol_base; + uint32_t status = TX_DESC_STAT_IC | TX_DESC_STAT_TCH | TX_DESC_STAT_CIC + | TX_DESC_STAT_OWN; + if (!i) { + /* fist chunk */ + status |= TX_DESC_STAT_FS; + } + if (!iolist->iol_next) { + /* last chunk */ + status |= TX_DESC_STAT_LS | TX_DESC_STAT_TER; + } + tx_desc[i].status = status; } - /* block until there's an available descriptor */ - while (tx_curr->status & TX_DESC_STAT_OWN) { - DEBUG("stm32_eth: not avail\n"); - } - - /* clear status field */ - tx_curr->status &= 0x0fffffff; - - dma_acquire(eth_config.dma); - for (; iolist; iolist = iolist->iol_next) { - ret += dma_transfer( - eth_config.dma, eth_config.dma_chan, - iolist->iol_base, tx_curr->buffer_addr + ret, iolist->iol_len, - DMA_MEM_TO_MEM, DMA_INC_BOTH_ADDR); - } - - dma_release(eth_config.dma); - if (ret < 0) { - DEBUG("stm32_eth: Failure in dma_transfer\n"); - return ret; - } - tx_curr->control = (len & 0x1fff); - - /* set flags for first and last frames */ - tx_curr->status |= TX_DESC_STAT_FS; - tx_curr->status |= TX_DESC_STAT_LS | TX_DESC_STAT_IC; - - /* give the descriptors to the DMA */ - tx_curr->status |= TX_DESC_STAT_OWN; - tx_curr = tx_curr->desc_next; - - /* start tx */ + /* start TX */ ETH->DMATPDR = 0; - return ret; + /* await completion */ + DEBUG("stm32_eth: Started to send %u B via DMA\n", bytes_to_send); + mutex_lock(&stm32_eth_tx_completed); + DEBUG("stm32_eth: TX completed\n"); + + /* Error check */ + unsigned i = 0; + while (1) { + uint32_t status = tx_desc[i].status; + DEBUG("TX desc %u status: ES=%c, UF=%c, EC=%c, NC=%c, FS=%c, LS=%c\n", + i, + (status & TX_DESC_STAT_ES) ? '1' : '0', + (status & TX_DESC_STAT_UF) ? '1' : '0', + (status & TX_DESC_STAT_EC) ? '1' : '0', + (status & TX_DESC_STAT_NC) ? '1' : '0', + (status & TX_DESC_STAT_FS) ? '1' : '0', + (status & TX_DESC_STAT_LS) ? '1' : '0'); + if (status & TX_DESC_STAT_ES) { + return -1; + } + if (status & TX_DESC_STAT_LS) { + break; + } + i++; + } + return (int)bytes_to_send; } static ssize_t get_rx_frame_size(void) @@ -385,11 +386,6 @@ int stm32_eth_receive(void *buf, size_t max_len) return size; } -int stm32_eth_get_rx_status_owned(void) -{ - return (!(rx_curr->status & RX_DESC_STAT_OWN)); -} - void stm32_eth_isr_eth_wkup(void) { cortexm_isr_end(); diff --git a/drivers/stm32_eth/stm32_eth.c b/drivers/stm32_eth/stm32_eth.c index 9091438337..5f406a943a 100644 --- a/drivers/stm32_eth/stm32_eth.c +++ b/drivers/stm32_eth/stm32_eth.c @@ -27,21 +27,17 @@ #include "debug.h" #include -static mutex_t _tx = MUTEX_INIT; -static mutex_t _rx = MUTEX_INIT; netdev_t *_netdev; +extern mutex_t stm32_eth_tx_completed; void stm32_eth_set_mac(const char *mac); void stm32_eth_get_mac(char *out); int stm32_eth_init(void); int stm32_eth_receive(void *data, size_t max_len); int stm32_eth_send(const struct iolist *iolist); -int stm32_eth_get_rx_status_owned(void); static void _isr(netdev_t *netdev) { - if (stm32_eth_get_rx_status_owned()) { - netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); - } + netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); } void isr_eth(void) @@ -49,20 +45,19 @@ void isr_eth(void) unsigned tmp = ETH->DMASR; if ((tmp & ETH_DMASR_TS)) { - ETH->DMASR = ETH_DMASR_TS | ETH_DMASR_NIS; - mutex_unlock(&_tx); + ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_TS; + DEBUG("isr_eth: TX completed\n"); + mutex_unlock(&stm32_eth_tx_completed); } if ((tmp & ETH_DMASR_RS)) { - ETH->DMASR = ETH_DMASR_RS | ETH_DMASR_NIS; - mutex_unlock(&_rx); + ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_RS; + DEBUG("isr_eth: RX completed\n"); if (_netdev) { netdev_trigger_event_isr(_netdev); } } - /* printf("r:%x\n\n", tmp); */ - cortexm_isr_end(); } @@ -70,32 +65,15 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info) { (void)info; (void)netdev; - if (!stm32_eth_get_rx_status_owned()){ - mutex_lock(&_rx); - } - int ret = stm32_eth_receive(buf, len); - DEBUG("stm32_eth_netdev: _recev: %d\n", ret); - - return ret; + return stm32_eth_receive(buf, len); } static int _send(netdev_t *netdev, const struct iolist *iolist) { (void)netdev; - int ret = 0; - if(stm32_eth_get_rx_status_owned()) { - mutex_lock(&_tx); - } netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); - ret = stm32_eth_send(iolist); - DEBUG("stm32_eth_netdev: _send: %d %d\n", ret, iolist_size(iolist)); - if (ret < 0) - { - netdev->event_callback(netdev, NETDEV_EVENT_TX_MEDIUM_BUSY); - return ret; - } + int ret = stm32_eth_send(iolist); netdev->event_callback(netdev, NETDEV_EVENT_TX_COMPLETE); - return ret; } From 8d8af31e39ba5cf8dd5c33a24f82e1e86b672d0a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Sun, 26 Jul 2020 22:05:18 +0200 Subject: [PATCH 7/8] driver/stm32_eth: Integrate into periph_eth The stm32_eth driver was build on top of the internal API periph_eth, which was unused anywhere. (Additionally, with two obscure exceptions, no functions where declared in headers, making them pretty hard to use anyway.) The separation of the driver into two layers incurs overhead, but does not result in cleaner structure or reuse of code. Thus, this artificial separation was dropped. --- cpu/stm32/Makefile.dep | 7 ++ cpu/stm32/include/periph_cpu.h | 23 ------ cpu/stm32/periph/eth.c | 117 ++++++++++++++++++++++++---- drivers/stm32_eth/Makefile | 1 - drivers/stm32_eth/Makefile.dep | 5 -- drivers/stm32_eth/doc.txt | 13 ---- drivers/stm32_eth/stm32_eth.c | 135 --------------------------------- makefiles/pseudomodules.inc.mk | 3 +- 8 files changed, 112 insertions(+), 192 deletions(-) delete mode 100644 drivers/stm32_eth/Makefile delete mode 100644 drivers/stm32_eth/Makefile.dep delete mode 100644 drivers/stm32_eth/doc.txt delete mode 100644 drivers/stm32_eth/stm32_eth.c diff --git a/cpu/stm32/Makefile.dep b/cpu/stm32/Makefile.dep index b39081a74f..ea8843d6ea 100644 --- a/cpu/stm32/Makefile.dep +++ b/cpu/stm32/Makefile.dep @@ -16,6 +16,13 @@ ifneq (,$(filter periph_uart_nonblocking,$(USEMODULE))) USEMODULE += tsrb endif +ifneq (,$(filter stm32_eth,$(USEMODULE))) + FEATURES_REQUIRED += periph_eth + USEMODULE += netdev_eth + USEMODULE += iolist + USEMODULE += luid +endif + ifneq (STM32F030x4, $(CPU_LINE)) # Retrieve vendor CMSIS headers from ST cmsis_device_ repositories on # GitHub, https://github.com/STMicroelectronics diff --git a/cpu/stm32/include/periph_cpu.h b/cpu/stm32/include/periph_cpu.h index bdace91256..3a7ec7cd37 100644 --- a/cpu/stm32/include/periph_cpu.h +++ b/cpu/stm32/include/periph_cpu.h @@ -1227,29 +1227,6 @@ typedef struct eth_dma_desc { #define ANER_LP_AN_ABLE (0x0001) /** @} */ -#ifdef MODULE_STM32_ETH -/** - * @brief Read a PHY register - * - * @param[in] addr address of the PHY to read - * @param[in] reg register to be read - * - * @return value in the register, or <=0 on error - */ -int32_t stm32_eth_phy_read(uint16_t addr, uint8_t reg); - -/** - * @brief Write a PHY register - * - * @param[in] addr address of the PHY to write - * @param[in] reg register to be written - * @param[in] value value to write into the register - * - * @return 0 in case of success or <=0 on error - */ -int32_t stm32_eth_phy_write(uint16_t addr, uint8_t reg, uint16_t value); -#endif /* MODULE_STM32_ETH */ - #ifdef __cplusplus } #endif diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 40ec778e36..606ee11844 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -23,12 +23,11 @@ #include #include "bitarithm.h" -#include "mutex.h" -#include "luid.h" - #include "iolist.h" +#include "luid.h" +#include "mutex.h" #include "net/ethernet.h" - +#include "net/netdev/eth.h" #include "periph/gpio.h" #define ENABLE_DEBUG (0) @@ -84,6 +83,9 @@ static edma_desc_t *rx_curr; /* RX Buffers */ static char rx_buffer[ETH_RX_DESCRIPTOR_COUNT][ETH_RX_BUFFER_SIZE]; +/* Netdev used in RIOT's API to upper layer */ +netdev_t *_netdev; + /** Read or write a phy register, to write the register ETH_MACMIIAR_MW is to * be passed as the higher nibble of the value */ static unsigned _rw_phy(unsigned addr, unsigned reg, unsigned value) @@ -105,18 +107,17 @@ static unsigned _rw_phy(unsigned addr, unsigned reg, unsigned value) return (ETH->MACMIIDR & 0x0000ffff); } -int32_t stm32_eth_phy_read(uint16_t addr, uint8_t reg) +static inline int32_t _phy_read(uint16_t addr, uint8_t reg) { return _rw_phy(addr, reg, 0); } -int32_t stm32_eth_phy_write(uint16_t addr, uint8_t reg, uint16_t value) +static inline void _phy_write(uint16_t addr, uint8_t reg, uint16_t value) { _rw_phy(addr, reg, (value & 0xffff) | (ETH_MACMIIAR_MW << 16)); - return 0; } -void stm32_eth_get_mac(char *out) +static void stm32_eth_get_mac(char *out) { unsigned t; @@ -133,7 +134,7 @@ void stm32_eth_get_mac(char *out) /** Set the mac address. The peripheral supports up to 4 MACs but only one is * implemented */ -void stm32_eth_set_mac(const char *mac) +static void stm32_eth_set_mac(const char *mac) { ETH->MACA0HR &= 0xffff0000; ETH->MACA0HR |= ((mac[5] << 8) | mac[4]); @@ -165,8 +166,9 @@ static void _init_buffer(void) ETH->DMATDLAR = (uintptr_t)&tx_desc[0]; } -int stm32_eth_init(void) +static int stm32_eth_init(netdev_t *netdev) { + (void)netdev; /* enable APB2 clock */ RCC->APB2ENR |= RCC_APB2ENR_SYSCFGEN; @@ -199,7 +201,7 @@ int stm32_eth_init(void) /* configure the PHY (standard for all PHY's) */ /* if there's no PHY, this has no effect */ - stm32_eth_phy_write(eth_config.phy_addr, PHY_BMCR, BMCR_RESET); + _phy_write(eth_config.phy_addr, PHY_BMCR, BMCR_RESET); /* speed from conf */ ETH->MACCR |= (ETH_MACCR_ROD | ETH_MACCR_IPCO | ETH_MACCR_APCS | @@ -244,13 +246,15 @@ int stm32_eth_init(void) /* configure speed, do it at the end so the PHY had time to * reset */ - stm32_eth_phy_write(eth_config.phy_addr, PHY_BMCR, eth_config.speed); + _phy_write(eth_config.phy_addr, PHY_BMCR, eth_config.speed); return 0; } -int stm32_eth_send(const struct iolist *iolist) +static int stm32_eth_send(netdev_t *netdev, const struct iolist *iolist) { + (void)netdev; + netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); unsigned bytes_to_send = iolist_size(iolist); /* Input must not be bigger than maximum allowed frame length */ assert(bytes_to_send <= ETHERNET_FRAME_LEN); @@ -294,7 +298,10 @@ int stm32_eth_send(const struct iolist *iolist) (status & TX_DESC_STAT_NC) ? '1' : '0', (status & TX_DESC_STAT_FS) ? '1' : '0', (status & TX_DESC_STAT_LS) ? '1' : '0'); + /* The Error Summary (ES) bit is set, if any error during TX occurred */ if (status & TX_DESC_STAT_ES) { + /* TODO: Report better event to reflect error */ + netdev->event_callback(netdev, NETDEV_EVENT_TX_COMPLETE); return -1; } if (status & TX_DESC_STAT_LS) { @@ -302,6 +309,7 @@ int stm32_eth_send(const struct iolist *iolist) } i++; } + netdev->event_callback(netdev, NETDEV_EVENT_TX_COMPLETE); return (int)bytes_to_send; } @@ -345,8 +353,11 @@ static void drop_frame_and_update_rx_curr(void) } } -int stm32_eth_receive(void *buf, size_t max_len) +static int stm32_eth_recv(netdev_t *netdev, void *buf, size_t max_len, + void *info) { + (void)info; + (void)netdev; char *data = buf; /* Determine the size of received frame. The frame might span multiple * DMA buffers */ @@ -390,3 +401,81 @@ void stm32_eth_isr_eth_wkup(void) { cortexm_isr_end(); } + +static void stm32_eth_isr(netdev_t *netdev) { + netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); +} + +void isr_eth(void) +{ + unsigned tmp = ETH->DMASR; + + if ((tmp & ETH_DMASR_TS)) { + ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_TS; + DEBUG("isr_eth: TX completed\n"); + mutex_unlock(&stm32_eth_tx_completed); + } + + if ((tmp & ETH_DMASR_RS)) { + ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_RS; + DEBUG("isr_eth: RX completed\n"); + if (_netdev) { + netdev_trigger_event_isr(_netdev); + } + } + + cortexm_isr_end(); +} + +static int stm32_eth_set(netdev_t *dev, netopt_t opt, + const void *value, size_t max_len) +{ + int res = -1; + + switch (opt) { + case NETOPT_ADDRESS: + assert(max_len >= ETHERNET_ADDR_LEN); + stm32_eth_set_mac((char *)value); + res = ETHERNET_ADDR_LEN; + break; + default: + res = netdev_eth_set(dev, opt, value, max_len); + break; + } + + return res; +} + +static int stm32_eth_get(netdev_t *dev, netopt_t opt, + void *value, size_t max_len) +{ + int res = -1; + + switch (opt) { + case NETOPT_ADDRESS: + assert(max_len >= ETHERNET_ADDR_LEN); + stm32_eth_get_mac((char *)value); + res = ETHERNET_ADDR_LEN; + break; + default: + res = netdev_eth_get(dev, opt, value, max_len); + break; + } + + return res; +} + +static const netdev_driver_t netdev_driver_stm32f4eth = { + .send = stm32_eth_send, + .recv = stm32_eth_recv, + .init = stm32_eth_init, + .isr = stm32_eth_isr, + .get = stm32_eth_get, + .set = stm32_eth_set, +}; + +void stm32_eth_netdev_setup(netdev_t *netdev) +{ + _netdev = netdev; + netdev->driver = &netdev_driver_stm32f4eth; +} diff --git a/drivers/stm32_eth/Makefile b/drivers/stm32_eth/Makefile deleted file mode 100644 index 48422e909a..0000000000 --- a/drivers/stm32_eth/Makefile +++ /dev/null @@ -1 +0,0 @@ -include $(RIOTBASE)/Makefile.base diff --git a/drivers/stm32_eth/Makefile.dep b/drivers/stm32_eth/Makefile.dep deleted file mode 100644 index a6efd51ab2..0000000000 --- a/drivers/stm32_eth/Makefile.dep +++ /dev/null @@ -1,5 +0,0 @@ -FEATURES_REQUIRED += periph_eth -FEATURES_REQUIRED += periph_dma -USEMODULE += netdev_eth -USEMODULE += iolist -USEMODULE += luid diff --git a/drivers/stm32_eth/doc.txt b/drivers/stm32_eth/doc.txt deleted file mode 100644 index 0bff9e38df..0000000000 --- a/drivers/stm32_eth/doc.txt +++ /dev/null @@ -1,13 +0,0 @@ -/* - * - * This file is subject to the terms and conditions of the GNU Lesser - * General Public License v2.1. See the file LICENSE in the top level - * directory for more details. - */ - -/** -@defgroup drivers_stm32_common Driver for stm32 ethernet -@ingroup drivers_netdev -@brief Device Driver for STM32 Ethernet - -*/ diff --git a/drivers/stm32_eth/stm32_eth.c b/drivers/stm32_eth/stm32_eth.c deleted file mode 100644 index 5f406a943a..0000000000 --- a/drivers/stm32_eth/stm32_eth.c +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (C) 2016 TriaGnoSys GmbH - * - * This file is subject to the terms and conditions of the GNU Lesser General - * Public License v2.1. See the file LICENSE in the top level directory for more - * details. - */ - -/** - * @ingroup drivers_stm32_common - * @{ - * - * @file - * @brief Netdev wrapper for stm32 ethernet - * - * @author Víctor Ariño - * - * @} - */ - -#include "periph_conf.h" -#include "mutex.h" -#include "net/netdev/eth.h" -#include "net/ethernet.h" -#include "iolist.h" -#define ENABLE_DEBUG (0) -#include "debug.h" - -#include -netdev_t *_netdev; -extern mutex_t stm32_eth_tx_completed; - -void stm32_eth_set_mac(const char *mac); -void stm32_eth_get_mac(char *out); -int stm32_eth_init(void); -int stm32_eth_receive(void *data, size_t max_len); -int stm32_eth_send(const struct iolist *iolist); - -static void _isr(netdev_t *netdev) { - netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); -} - -void isr_eth(void) -{ - unsigned tmp = ETH->DMASR; - - if ((tmp & ETH_DMASR_TS)) { - ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_TS; - DEBUG("isr_eth: TX completed\n"); - mutex_unlock(&stm32_eth_tx_completed); - } - - if ((tmp & ETH_DMASR_RS)) { - ETH->DMASR = ETH_DMASR_NIS | ETH_DMASR_RS; - DEBUG("isr_eth: RX completed\n"); - if (_netdev) { - netdev_trigger_event_isr(_netdev); - } - } - - cortexm_isr_end(); -} - -static int _recv(netdev_t *netdev, void *buf, size_t len, void *info) -{ - (void)info; - (void)netdev; - return stm32_eth_receive(buf, len); -} - -static int _send(netdev_t *netdev, const struct iolist *iolist) -{ - (void)netdev; - netdev->event_callback(netdev, NETDEV_EVENT_TX_STARTED); - int ret = stm32_eth_send(iolist); - netdev->event_callback(netdev, NETDEV_EVENT_TX_COMPLETE); - return ret; -} - -static int _set(netdev_t *dev, netopt_t opt, const void *value, size_t max_len) -{ - int res = -1; - - switch (opt) { - case NETOPT_ADDRESS: - assert(max_len >= ETHERNET_ADDR_LEN); - stm32_eth_set_mac((char *)value); - res = ETHERNET_ADDR_LEN; - break; - default: - res = netdev_eth_set(dev, opt, value, max_len); - break; - } - - return res; -} - -static int _get(netdev_t *dev, netopt_t opt, void *value, size_t max_len) -{ - int res = -1; - - switch (opt) { - case NETOPT_ADDRESS: - assert(max_len >= ETHERNET_ADDR_LEN); - stm32_eth_get_mac((char *)value); - res = ETHERNET_ADDR_LEN; - break; - default: - res = netdev_eth_get(dev, opt, value, max_len); - break; - } - - return res; -} - -static int _init(netdev_t *netdev) -{ - (void)netdev; - return stm32_eth_init(); -} - -static const netdev_driver_t netdev_driver_stm32f4eth = { - .send = _send, - .recv = _recv, - .init = _init, - .isr = _isr, - .get = _get, - .set = _set, -}; - -void stm32_eth_netdev_setup(netdev_t *netdev) -{ - _netdev = netdev; - netdev->driver = &netdev_driver_stm32f4eth; -} diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk index 4cf00c3ab4..eecd7260a5 100644 --- a/makefiles/pseudomodules.inc.mk +++ b/makefiles/pseudomodules.inc.mk @@ -107,9 +107,10 @@ PSEUDOMODULES += sock_tcp PSEUDOMODULES += sock_udp PSEUDOMODULES += soft_uart_modecfg PSEUDOMODULES += stdin -PSEUDOMODULES += stdio_ethos PSEUDOMODULES += stdio_cdc_acm +PSEUDOMODULES += stdio_ethos PSEUDOMODULES += stdio_uart_rx +PSEUDOMODULES += stm32_eth PSEUDOMODULES += suit_transport_% PSEUDOMODULES += wakaama_objects_% PSEUDOMODULES += wifi_enterprise From 4fcf37c162e9a0e60696a77a5165f4c6c0891dad Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Mon, 27 Jul 2020 09:33:53 +0200 Subject: [PATCH 8/8] cpu/stm32/periph_eth: Handle lost & spurious IRQs Fixes https://github.com/RIOT-OS/RIOT/issues/13496 --- cpu/stm32/include/periph_cpu.h | 1 + cpu/stm32/periph/eth.c | 75 ++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/cpu/stm32/include/periph_cpu.h b/cpu/stm32/include/periph_cpu.h index 3a7ec7cd37..b3578f9b19 100644 --- a/cpu/stm32/include/periph_cpu.h +++ b/cpu/stm32/include/periph_cpu.h @@ -1061,6 +1061,7 @@ typedef struct eth_dma_desc { */ #define RX_DESC_STAT_FL (0x3FFF0000) /* bits 16-29 */ #define RX_DESC_STAT_DE (BIT14) /**< If set, a frame too large to fit buffers given by descriptors was received */ +#define RX_DESC_STAT_ES (BIT14) /**< If set, an error occurred during RX */ #define RX_DESC_STAT_OWN (BIT31) /**< If set, descriptor is owned by DMA, otherwise by CPU */ /** @} */ /** diff --git a/cpu/stm32/periph/eth.c b/cpu/stm32/periph/eth.c index 606ee11844..211dcdad1f 100644 --- a/cpu/stm32/periph/eth.c +++ b/cpu/stm32/periph/eth.c @@ -93,7 +93,7 @@ static unsigned _rw_phy(unsigned addr, unsigned reg, unsigned value) unsigned tmp; while (ETH->MACMIIAR & ETH_MACMIIAR_MB) {} - DEBUG("stm32_eth: rw_phy %x (%x): %x\n", addr, reg, value); + DEBUG("[stm32_eth] rw_phy %x (%x): %x\n", addr, reg, value); tmp = (ETH->MACMIIAR & ETH_MACMIIAR_CR) | ETH_MACMIIAR_MB; tmp |= (((addr & 0x1f) << 11) | ((reg & 0x1f) << 6)); @@ -103,7 +103,7 @@ static unsigned _rw_phy(unsigned addr, unsigned reg, unsigned value) ETH->MACMIIAR = tmp; while (ETH->MACMIIAR & ETH_MACMIIAR_MB) {} - DEBUG("stm32_eth: %lx\n", ETH->MACMIIDR); + DEBUG("[stm32_eth] %lx\n", ETH->MACMIIDR); return (ETH->MACMIIDR & 0x0000ffff); } @@ -282,9 +282,9 @@ static int stm32_eth_send(netdev_t *netdev, const struct iolist *iolist) /* start TX */ ETH->DMATPDR = 0; /* await completion */ - DEBUG("stm32_eth: Started to send %u B via DMA\n", bytes_to_send); + DEBUG("[stm32_eth] Started to send %u B via DMA\n", bytes_to_send); mutex_lock(&stm32_eth_tx_completed); - DEBUG("stm32_eth: TX completed\n"); + DEBUG("[stm32_eth] TX completed\n"); /* Error check */ unsigned i = 0; @@ -313,28 +313,36 @@ static int stm32_eth_send(netdev_t *netdev, const struct iolist *iolist) return (int)bytes_to_send; } -static ssize_t get_rx_frame_size(void) +static int get_rx_frame_size(void) { edma_desc_t *i = rx_curr; uint32_t status; while (1) { /* Wait until DMA gave up control over descriptor */ - while ((status = i->status) & RX_DESC_STAT_OWN) { } - DEBUG("stm32_eth: get_rx_frame_size(): FS=%c, LS=%c, DE=%c, FL=%lu\n", + if ((status = i->status) & RX_DESC_STAT_OWN) { + DEBUG("[stm32_eth] RX not completed (spurious interrupt?)\n"); + return -EAGAIN; + } + DEBUG("[stm32_eth] get_rx_frame_size(): FS=%c, LS=%c, ES=%c, DE=%c, FL=%lu\n", (status & RX_DESC_STAT_FS) ? '1' : '0', (status & RX_DESC_STAT_LS) ? '1' : '0', + (status & RX_DESC_STAT_ES) ? '1' : '0', (status & RX_DESC_STAT_DE) ? '1' : '0', ((status >> 16) & 0x3fff) - ETHERNET_FCS_LEN); + if (status & RX_DESC_STAT_DE) { + DEBUG("[stm32_eth] Overflow during RX\n"); + return -EOVERFLOW; + } + if (status & RX_DESC_STAT_ES) { + DEBUG("[stm32_eth] Error during RX\n"); + return -EIO; + } if (status & RX_DESC_STAT_LS) { break; } i = i->desc_next; } - if (status & RX_DESC_STAT_DE) { - return -1; - } - /* bits 16-29 contain the frame length including 4 B frame check sequence */ return ((status >> 16) & 0x3fff) - ETHERNET_FCS_LEN; } @@ -346,39 +354,63 @@ static void drop_frame_and_update_rx_curr(void) /* hand over old descriptor to DMA */ rx_curr->status = RX_DESC_STAT_OWN; rx_curr = rx_curr->desc_next; - if (old_status & RX_DESC_STAT_LS) { - /* reached last DMA descriptor of frame ==> done */ + if (old_status & (RX_DESC_STAT_LS | RX_DESC_STAT_ES)) { + /* reached either last DMA descriptor of frame or error ==> done */ return; } } } +static void handle_lost_rx_irqs(void) +{ + edma_desc_t *iter = rx_curr; + while (1) { + uint32_t status = iter->status; + if (status & RX_DESC_STAT_OWN) { + break; + } + if (status & RX_DESC_STAT_LS) { + DEBUG("[stm32_eth] Lost RX IRQ, sending event to upper layer now\n"); + /* we use the ISR event for this, as the upper layer calls recv() + * right away on an NETDEV_EVENT_RX_COMPLETE. Because there could be + * potentially quite a lot of received frames in the queue, we might + * risk a stack overflow if we would send an NETDEV_EVENT_RX_COMPLETE + */ + netdev_trigger_event_isr(_netdev); + break; + } + iter = iter->desc_next; + } +} + static int stm32_eth_recv(netdev_t *netdev, void *buf, size_t max_len, - void *info) + void *info) { (void)info; (void)netdev; char *data = buf; /* Determine the size of received frame. The frame might span multiple * DMA buffers */ - ssize_t size = get_rx_frame_size(); + int size = get_rx_frame_size(); - if (size == -1) { - DEBUG("stm32_eth: Received frame was too large for DMA buffer(s)\n"); - drop_frame_and_update_rx_curr(); - return -EOVERFLOW; + if (size < 0) { + if (size != -EAGAIN) { + DEBUG("[stm32_eth] Dropping frame due to error\n"); + drop_frame_and_update_rx_curr(); + } + return size; } if (!buf) { if (max_len) { - DEBUG("stm32_eth: Dropping frame as requested by upper layer\n"); + DEBUG("[stm32_eth] Dropping frame as requested by upper layer\n"); drop_frame_and_update_rx_curr(); } return size; } if (max_len < (size_t)size) { - DEBUG("stm32_eth: Buffer provided by upper layer is too small\n"); + DEBUG("[stm32_eth] Buffer provided by upper layer is too small\n"); drop_frame_and_update_rx_curr(); return -ENOBUFS; } @@ -394,6 +426,7 @@ static int stm32_eth_recv(netdev_t *netdev, void *buf, size_t max_len, rx_curr = rx_curr->desc_next; } + handle_lost_rx_irqs(); return size; }