Merge #19703 #19724 #19735

19703: cpu/sam0_eth: interrupt based link detection/auto-negotiation r=benpicco a=benpicco 19724: dist/tools/openocd: add OPENOCD_SERVER_ADDRESS variable r=benpicco a=fabian18 19735: nrf5x_common: Clear I2C periph shorts r=benpicco a=bergzand ### Contribution description The I2C peripheral's shortcuts are used with the read and write register to automatically stop the I2C transaction or to continue with the next stage. With simple I2C read and write bytes these shorts are not used, but are also not cleared by the function in all cases, causing it to use the shortcut configuration set by a previous function call. This patch ensures that the shorts are always set by the read and write functions ### Testing procedure Should be possible to spot with a logic analyzer and the I2C periph test. Maybe the HIL test can also detect it :) ### Issues/PRs references None Co-authored-by: Benjamin Valentin <[email protected]> Co-authored-by: Fabian Hüßler <[email protected]> Co-authored-by: Koen Zandberg <[email protected]>
RIOT-OS · Jun 14, 2023 · 829af7c · 829af7c
4 parents 61d7c40 + 2a255ff + 17d0e58 + e5d25e0
commit 829af7c
Show file tree

Hide file tree

Showing 8 changed files with 213 additions and 30 deletions.
diff --git a/cpu/nrf5x_common/periph/i2c_nrf52_nrf9160.c b/cpu/nrf5x_common/periph/i2c_nrf52_nrf9160.c
@@ -251,6 +251,9 @@ int i2c_read_bytes(i2c_t dev, uint16_t addr, void *data, size_t len,
     if (!(flags & I2C_NOSTOP)) {
         bus(dev)->SHORTS = TWIM_SHORTS_LASTRX_STOP_Msk;
     }
+    else {
+        bus(dev)->SHORTS = 0;
+    }
     /* Start transmission */
     bus(dev)->TASKS_STARTRX = 1;
 
@@ -309,6 +312,9 @@ int i2c_write_bytes(i2c_t dev, uint16_t addr, const void *data, size_t len,
     if (!(flags & I2C_NOSTOP)) {
         bus(dev)->SHORTS = TWIM_SHORTS_LASTTX_STOP_Msk;
     }
+    else {
+        bus(dev)->SHORTS = 0;
+    }
     bus(dev)->TASKS_STARTTX = 1;
 
     return finish(dev);

diff --git a/cpu/sam0_common/Makefile.dep b/cpu/sam0_common/Makefile.dep
@@ -19,5 +19,6 @@ ifneq (,$(filter sam0_eth,$(USEMODULE)))
   USEMODULE += netdev_legacy_api
   USEMODULE += netopt
   FEATURES_REQUIRED += periph_eth
+  FEATURES_REQUIRED += periph_gpio_irq
 endif
 include $(RIOTCPU)/cortexm_common/Makefile.dep
diff --git a/cpu/sam0_common/periph/eth.c b/cpu/sam0_common/periph/eth.c
@@ -19,6 +19,7 @@
  */
 
 #include "iolist.h"
+#include "mii.h"
 #include "net/eui48.h"
 #include "net/ethernet.h"
 #include "net/netdev/eth.h"
@@ -120,6 +121,36 @@ static void _disable_clock(void)
     MCLK->APBCMASK.reg &= ~MCLK_APBCMASK_GMAC;
 }
 
+unsigned sam0_read_phy(uint8_t phy, uint8_t addr)
+{
+    if (_is_sleeping) {
+        return 0;
+    }
+
+    GMAC->MAN.reg = GMAC_MAN_REGA(addr) | GMAC_MAN_PHYA(phy)
+                  | GMAC_MAN_CLTTO      | GMAC_MAN_WTN(0x2)
+                  | GMAC_MAN_OP(PHY_READ_OP);
+
+    /* Wait for operation completion */
+    while (!GMAC->NSR.bit.IDLE) {}
+    /* return content of shift register */
+    return (GMAC->MAN.reg & GMAC_MAN_DATA_Msk);
+}
+
+void sam0_write_phy(uint8_t phy, uint8_t addr, uint16_t data)
+{
+    if (_is_sleeping) {
+        return;
+    }
+
+    GMAC->MAN.reg = GMAC_MAN_REGA(addr) | GMAC_MAN_PHYA(phy)
+                  | GMAC_MAN_WTN(0x2)   | GMAC_MAN_OP(PHY_WRITE_OP)
+                  | GMAC_MAN_CLTTO      | GMAC_MAN_DATA(data);
+
+    /* Wait for operation completion */
+    while (!GMAC->NSR.bit.IDLE) {}
+}
+
 void sam0_eth_poweron(void)
 {
     _enable_clock();
@@ -128,6 +159,8 @@ void sam0_eth_poweron(void)
     /* enable PHY */
     gpio_set(sam_gmac_config[0].rst_pin);
     _is_sleeping = false;
+
+    while (MII_BMCR_RESET & sam0_read_phy(0, MII_BMCR)) {}
 }
 
 void sam0_eth_poweroff(void)
@@ -165,28 +198,6 @@ static void _init_desc_buf(void)
     GMAC->TBQB.reg = (uint32_t) tx_desc;
 }
 
-unsigned sam0_read_phy(uint8_t phy, uint8_t addr)
-{
-    GMAC->MAN.reg = GMAC_MAN_REGA(addr) | GMAC_MAN_PHYA(phy)
-                  | GMAC_MAN_CLTTO      | GMAC_MAN_WTN(0x2)
-                  | GMAC_MAN_OP(PHY_READ_OP);
-
-    /* Wait for operation completion */
-    while (!GMAC->NSR.bit.IDLE) {}
-    /* return content of shift register */
-    return (GMAC->MAN.reg & GMAC_MAN_DATA_Msk);
-}
-
-void sam0_write_phy(uint8_t phy, uint8_t addr, uint16_t data)
-{
-    GMAC->MAN.reg = GMAC_MAN_REGA(addr) | GMAC_MAN_PHYA(phy)
-                  | GMAC_MAN_WTN(0x2)   | GMAC_MAN_OP(PHY_WRITE_OP)
-                  | GMAC_MAN_CLTTO      | GMAC_MAN_DATA(data);
-
-    /* Wait for operation completion */
-    while (!GMAC->NSR.bit.IDLE) {}
-}
-
 void sam0_eth_set_mac(const eui48_t *mac)
 {
     GMAC->Sa[0].SAT.reg = ((mac->uint8[5] << 8)  | mac->uint8[4]);
@@ -374,11 +385,10 @@ int sam0_eth_init(void)
     /* Enable needed interrupts */
     GMAC->IER.reg = GMAC_IER_RCOMP;
 
-    /* Set TxBase-100-FD by default */
-    /* TODO: implement auto negotiation */
-    GMAC->NCFGR.reg = GMAC_NCFGR_SPD | GMAC_NCFGR_FD | GMAC_NCFGR_MTIHEN
+    GMAC->NCFGR.reg = GMAC_NCFGR_MTIHEN
                     | GMAC_NCFGR_RXCOEN | GMAC_NCFGR_MAXFS | GMAC_NCFGR_CAF
-                    | GMAC_NCFGR_LFERD | GMAC_NCFGR_RFCS | GMAC_NCFGR_CLK(3);
+                    | GMAC_NCFGR_LFERD | GMAC_NCFGR_RFCS | GMAC_NCFGR_CLK(3)
+                    | GMAC_NCFGR_DBW(1);
 
     /* Enable all multicast addresses */
     GMAC->HRB.reg = 0xffffffff;

diff --git a/cpu/sam0_common/sam0_eth/Kconfig b/cpu/sam0_common/sam0_eth/Kconfig
@@ -10,6 +10,7 @@ config MODULE_SAM0_ETH
     depends on TEST_KCONFIG
     depends on CPU_COMMON_SAM0
     depends on HAS_PERIPH_ETH
+    select MODULE_PERIPH_GPIO_IRQ
     select MODULE_NETDEV_ETH
     select MODULE_NETDEV_LEGACY_API
     select MODULE_NETOPT

diff --git a/cpu/sam0_common/sam0_eth/eth-netdev.c b/cpu/sam0_common/sam0_eth/eth-netdev.c
@@ -20,20 +20,29 @@
 #include <string.h>
 
 #include "iolist.h"
+#include "mii.h"
 #include "net/gnrc/netif/ethernet.h"
 #include "net/gnrc.h"
 #include "net/ethernet.h"
 #include "net/netdev/eth.h"
 #include "net/eui_provider.h"
 
 #include "periph/gpio.h"
+#include "ztimer.h"
 
 #include "sam0_eth_netdev.h"
 
 #define ENABLE_DEBUG 0
 #include "debug.h"
 #include "log.h"
 
+/**
+ * @brief   Link auto-negotiation timeout
+ */
+#ifndef CONFIG_SAM0_ETH_LINK_TIMEOUT_MS
+#define CONFIG_SAM0_ETH_LINK_TIMEOUT_MS         (5 * MS_PER_SEC)
+#endif
+
 /* Internal helpers */
 extern int sam0_eth_init(void);
 extern void sam0_eth_poweron(void);
@@ -44,26 +53,130 @@ extern bool sam0_eth_has_queued_pkt(void);
 extern void sam0_eth_set_mac(const eui48_t *mac);
 extern void sam0_eth_get_mac(char *out);
 extern void sam0_clear_rx_buffers(void);
+extern unsigned sam0_read_phy(uint8_t phy, uint8_t addr);
+extern void sam0_write_phy(uint8_t phy, uint8_t addr, uint16_t data);
+static void _restart_an(void *ctx);
 
 /* SAM0 CPUs only have one GMAC IP, so it is safe to
 statically defines one in this file */
 static sam0_eth_netdev_t _sam0_eth_dev;
 
+/* auto-negotiation timeout timer */
+static ztimer_t _phy_tim = { .callback = _restart_an };
+/* PHY interrupt status register */
+static uint16_t _phy_irq;
+
+static inline bool _get_link_status(void)
+{
+    return sam0_read_phy(0, MII_BMSR) & MII_BMSR_LINK;
+}
+
+static void _restart_an(void *ctx)
+{
+    (void)ctx;
+    sam0_write_phy(0, MII_IRQ, MII_IRQ_EN_LPA_ACK);
+    sam0_write_phy(0, MII_BMCR, MII_BMCR_AN_RESTART | MII_BMCR_AN_ENABLE |
+                                MII_BMCR_SPEED_100 | MII_BMCR_FULL_DPLX);
+}
+
+static void _phy_isr(void *ctx)
+{
+    (void)ctx;
+
+    _phy_irq = sam0_read_phy(0, MII_IRQ);
+
+    netdev_trigger_event_isr(_sam0_eth_dev.netdev);
+}
+
+static void _handle_phy_irq(uint16_t irq)
+{
+    netdev_t *netdev = _sam0_eth_dev.netdev;
+
+    if (irq & MII_IRQ_LINK_DOWN) {
+        DEBUG_PUTS("[sam0_eth]: link down");
+
+        if (IS_USED(MODULE_ZTIMER_MSEC)) {
+            ztimer_remove(ZTIMER_MSEC, &_phy_tim);
+        }
+        /* only listen for link partner ACK events now */
+        sam0_write_phy(0, MII_IRQ, MII_IRQ_EN_LPA_ACK);
+
+        netdev->event_callback(netdev, NETDEV_EVENT_LINK_DOWN);
+        return;
+    }
+    if (irq & MII_IRQ_LINK_UP) {
+        DEBUG_PUTS("[sam0_eth]: link up");
+
+        uint16_t adv = sam0_read_phy(0, MII_ADVERTISE);
+        uint16_t lpa = sam0_read_phy(0, MII_LPA);
+
+        if (IS_USED(MODULE_ZTIMER_MSEC)) {
+            ztimer_remove(ZTIMER_MSEC, &_phy_tim);
+        }
+
+        uint32_t ncfgr = GMAC->NCFGR.reg & ~(GMAC_NCFGR_FD | GMAC_NCFGR_MTIHEN);
+
+        if ((adv & MII_ADVERTISE_100) && (lpa & MII_LPA_100)) {
+            /* 100 Mbps */
+            ncfgr |= GMAC_NCFGR_SPD;
+        }
+        if ((adv & MII_ADVERTISE_10_F) && (lpa & MII_LPA_10_F)) {
+            /* full duplex */
+            ncfgr |= GMAC_NCFGR_FD;
+        }
+
+        GMAC->NCFGR.reg = ncfgr;
+        netdev->event_callback(netdev, NETDEV_EVENT_LINK_UP);
+        return;
+    }
+
+    if (irq & MII_IRQ_LPA_ACK) {
+        DEBUG_PUTS("[sam0_eth]: link partner present");
+
+        /* if we don't succeed, restart auto-negotiation in 5s */
+        if (IS_USED(MODULE_ZTIMER_MSEC)) {
+            ztimer_set(ZTIMER_MSEC, &_phy_tim, CONFIG_SAM0_ETH_LINK_TIMEOUT_MS);
+        }
+
+        /* we only care about link up / down events now */
+        sam0_write_phy(0, MII_IRQ, MII_IRQ_EN_LINK_UP | MII_IRQ_EN_LINK_DOWN);
+        return;
+    }
+
+    DEBUG("[sam0_eth]: unexpected PHY IRQ: %x\n", irq);
+}
+
+static inline void _setup_phy_irq(gpio_cb_t cb, void *arg)
+{
+    gpio_init_int(sam_gmac_config[0].int_pin, GPIO_IN, GPIO_FALLING, cb, arg);
+}
+
 static int _sam0_eth_init(netdev_t *netdev)
 {
     sam0_eth_init();
     eui48_t hwaddr;
     netdev_eui48_get(netdev, &hwaddr);
     sam0_eth_set_mac(&hwaddr);
 
-    /* signal link UP */
-    netdev->event_callback(netdev, NETDEV_EVENT_LINK_UP);
+    /* wait for PHY to be ready */
+    while (MII_BMCR_RESET & sam0_read_phy(0, MII_BMCR)) {}
+
+    _setup_phy_irq(_phy_isr, NULL);
+    _restart_an(NULL);
 
     return 0;
 }
 
 static void _sam0_eth_isr(netdev_t *netdev)
 {
+    if (_phy_irq) {
+        uint16_t tmp = _phy_irq;
+        _phy_irq = 0;
+
+        _handle_phy_irq(tmp);
+        return;
+    }
+
     netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE);
     return;
 }
@@ -106,6 +219,11 @@ static int _sam0_eth_get(netdev_t *netdev, netopt_t opt, void *val, size_t max_l
             sam0_eth_get_mac((char *)val);
             res = ETHERNET_ADDR_LEN;
             break;
+        case NETOPT_LINK:
+            assert(max_len == sizeof(netopt_enable_t));
+            *(netopt_enable_t *)val = _get_link_status();
+            res = sizeof(netopt_enable_t);
+            break;
         default:
             res = netdev_eth_get(netdev, opt, val, max_len);
             break;
@@ -118,10 +236,16 @@ static int _set_state(netopt_state_t state)
 {
     switch (state) {
         case NETOPT_STATE_SLEEP:
+            if (IS_USED(MODULE_ZTIMER_MSEC)) {
+                ztimer_remove(ZTIMER_MSEC, &_phy_tim);
+            }
             sam0_eth_poweroff();
+            _sam0_eth_dev.netdev->event_callback(_sam0_eth_dev.netdev,
+                                                 NETDEV_EVENT_LINK_DOWN);
             break;
         case NETOPT_STATE_IDLE:
             sam0_eth_poweron();
+            _restart_an(NULL);
             break;
         default:
             return -ENOTSUP;

diff --git a/dist/tools/openocd/openocd.sh b/dist/tools/openocd/openocd.sh
@@ -20,6 +20,7 @@
 #                      `OPENOCD="~/openocd/src/openocd -s ~/openocd/tcl"`.
 # OPENOCD_CONFIG:      OpenOCD configuration file name,
 #                      default: "${BOARDSDIR}/${BOARD}/dist/openocd.cfg"
+# OPENOCD_SERVER_ADDRESS: OpenOCD server bind address, default: "localhost"
 #
 # The script supports the following actions:
 #
@@ -102,14 +103,16 @@
 : ${OPENOCD_ADAPTER_INIT:=}
 # If set to 1 'reset_config' will use 'connect_assert_srst' before 'flash' or 'reset.
 : ${OPENOCD_RESET_USE_CONNECT_ASSERT_SRST:=}
+# Default bind address for OpenOCD
+: ${OPENOCD_SERVER_ADDRESS:=localhost}
 # The setsid command is needed so that Ctrl+C in GDB doesn't kill OpenOCD
 : ${SETSID:=setsid}
 # GDB command, usually a separate command for each platform (e.g. arm-none-eabi-gdb)
 : ${GDB:=gdb}
 # Debugger client command, can be used to wrap GDB in a front-end
 : ${DBG:=${GDB}}
 # Default debugger flags,
-: ${DBG_DEFAULT_FLAGS:=-q -ex \"tar ext :$(( GDB_PORT + GDB_PORT_CORE_OFFSET ))\"}
+: ${DBG_DEFAULT_FLAGS:=-q -ex \"tar ext ${OPENOCD_SERVER_ADDRESS}:$(( GDB_PORT + GDB_PORT_CORE_OFFSET ))\"}
 # Extra debugger flags, added by the user
 : ${DBG_EXTRA_FLAGS:=}
 # Debugger flags, will be passed to sh -c, remember to escape any quotation signs.
@@ -142,7 +145,7 @@
 
 # default terminal frontend
 _OPENOCD_TERMPROG=${RIOTTOOLS}/pyterm/pyterm
-_OPENOCD_TERMFLAGS="-ts ${RTT_PORT} ${PYTERMFLAGS}"
+_OPENOCD_TERMFLAGS="-ts ${OPENOCD_SERVER_ADDRESS}:${RTT_PORT} ${PYTERMFLAGS}"
 
 #
 # Examples of alternative debugger configurations
@@ -391,6 +394,7 @@ do_debug() {
             ${OPENOCD_ADAPTER_INIT} \
             -f '${OPENOCD_CONFIG}' \
             ${OPENOCD_EXTRA_INIT} \
+            -c 'bindto ${OPENOCD_SERVER_ADDRESS}' \
             -c 'tcl_port ${TCL_PORT}' \
             -c 'telnet_port ${TELNET_PORT}' \
             -c 'gdb_port ${GDB_PORT}' \
@@ -410,6 +414,7 @@ do_debugserver() {
             ${OPENOCD_ADAPTER_INIT} \
             -f '${OPENOCD_CONFIG}' \
             ${OPENOCD_EXTRA_INIT} \
+            -c 'bindto ${OPENOCD_SERVER_ADDRESS}' \
             -c 'tcl_port ${TCL_PORT}' \
             -c 'telnet_port ${TELNET_PORT}' \
             -c 'gdb_port ${GDB_PORT}' \
@@ -459,6 +464,7 @@ do_term() {
             ${OPENOCD_ADAPTER_INIT} \
             -f '${OPENOCD_CONFIG}' \
             ${OPENOCD_EXTRA_INIT} \
+            -c 'bindto ${OPENOCD_SERVER_ADDRESS}' \
             -c 'tcl_port 0' \
             -c 'telnet_port 0' \
             -c 'gdb_port 0' \