Merge #19487 #19781

19487: nanocoap: implement extended tokens (RFC 8974) r=benpicco a=benpicco



19781: cpu/nrf{53,9160}: add pwm support r=benpicco a=dylad

### Contribution description

This PR moves the nRF52 PWM driver to `cpu/nrf5x_common` to allow nRF9160 and nRF53 to use this driver.
IP is identical on these families.

I didn't test on nRF9160DK and I didn't test if there is any regression on nRF52-based board as I don't have any so tests are welcome !
However it works fine on nRF53-based board.


### Testing procedure

Flash the `tests/periph/pwm` test application on `nrf5340dk` or `nrf9160dk`.
You can then use the `osci` command to make the onboard LEDs "breath".
You can also attach an oscilloscope and/or logic analyzer to watch the signal.


### Issues/PRs references
~~Based on #19769~~

Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
Co-authored-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

boards/nrf5340dk-app/Kconfig

@@ -11,6 +11,7 @@ config BOARD_NRF5340DK_APP
     bool
     default y
     select CPU_MODEL_NRF5340_APP
+    select HAS_PERIPH_PWM
     select HAS_PERIPH_TIMER
     select HAS_PERIPH_UART
     select HAS_PERIPH_UART_HW_FC

boards/nrf5340dk-app/Makefile.features

@@ -2,6 +2,7 @@ CPU_MODEL = nrf5340_app
 CPU = nrf53
 
 # Put defined MCU peripherals here (in alphabetical order)
+FEATURES_PROVIDED += periph_pwm
 FEATURES_PROVIDED += periph_timer
 FEATURES_PROVIDED += periph_uart
 FEATURES_PROVIDED += periph_uart_hw_fc

boards/nrf5340dk-app/include/periph_conf.h

@@ -75,6 +75,25 @@ static const uart_conf_t uart_config[] = {
 #define UART_NUMOF          ARRAY_SIZE(uart_config) /**< UART configuration NUMOF */
 /** @} */
 
+/**
+ * @name   PWM configuration
+ * @{
+ */
+static const pwm_conf_t pwm_config[] = {
+    {
+        .dev = NRF_PWM0_S,
+        .pin = {
+                    LED0_PIN,
+                    LED1_PIN,
+                    LED2_PIN,
+                    LED3_PIN
+               }
+    },
+};
+
+#define PWM_NUMOF           ARRAY_SIZE(pwm_config)
+/** @} */
+
 #ifdef __cplusplus
 }
 #endif

boards/nrf9160dk/Kconfig

@@ -12,6 +12,7 @@ config BOARD_NRF9160DK
     default y
     select CPU_MODEL_NRF9160
     select HAS_PERIPH_I2C
+    select HAS_PERIPH_PWM
     select HAS_PERIPH_SPI
     select HAS_PERIPH_TIMER
     select HAS_PERIPH_UART

boards/nrf9160dk/Makefile.features

@@ -3,6 +3,7 @@ CPU = nrf9160
 
 # Put defined MCU peripherals here (in alphabetical order)
 FEATURES_PROVIDED += periph_i2c
+FEATURES_PROVIDED += periph_pwm
 FEATURES_PROVIDED += periph_spi
 FEATURES_PROVIDED += periph_timer
 FEATURES_PROVIDED += periph_uart

boards/nrf9160dk/include/periph_conf.h

@@ -116,9 +116,27 @@ static const uart_conf_t uart_config[] = {
 #define UART_0_ISR          (isr_uarte0_spim0_spis0_twim0_twis0) /**< UART0_IRQ */
 #define UART_1_ISR          (isr_uarte1_spim1_spis1_twim1_twis1) /**< UART1_IRQ */
 
-#define UART_NUMOF          ARRAY_SIZE(uart_config) /**< UART confgiguration NUMOF */
+#define UART_NUMOF          ARRAY_SIZE(uart_config) /**< UART configuration NUMOF */
 /** @} */
 
+/**
+ * @name   PWM configuration
+ * @{
+ */
+static const pwm_conf_t pwm_config[] = {
+    {
+        .dev = NRF_PWM0_S,
+        .pin = {
+                    LED0_PIN,
+                    LED1_PIN,
+                    LED2_PIN,
+                    LED3_PIN
+               }
+    },
+};
+
+#define PWM_NUMOF           ARRAY_SIZE(pwm_config)
+
 #ifdef __cplusplus
 }
 #endif

cpu/nrf52/include/periph_cpu.h

@@ -153,54 +153,6 @@ typedef struct {
 #define i2c_pin_scl(dev) i2c_config[dev].scl
 /** @} */
 
-/**
- * @name    The PWM unit on the nRF52 supports 4 channels per device
- */
-#define PWM_CHANNELS        (4U)
-
-/**
- * @name    Generate PWM mode values
- *
- * To encode the PWM mode, we use two bit:
- * - bit  0: select up or up-and-down counting
- * - bit 15: select polarity
- */
-#define PWM_MODE(ud, pol)   (ud | (pol << 15))
-
-/**
- * @brief   Override the PWM mode definitions
- * @{
- */
-#define HAVE_PWM_MODE_T
-typedef enum {
-    PWM_LEFT       = PWM_MODE(0, 1),    /**< left aligned PWM */
-    PWM_RIGHT      = PWM_MODE(0, 0),    /**< right aligned PWM */
-    PWM_CENTER     = PWM_MODE(1, 1),    /**< not supported */
-    PWM_CENTER_INV = PWM_MODE(1, 0)     /**< not supported */
-} pwm_mode_t;
-/** @} */
-
-/**
- * @brief   PWM configuration options
- *
- * Each device supports up to 4 channels. If you want to use less than 4
- * channels, just set the unused pins to GPIO_UNDEF.
- *
- * @note    define unused pins only from right to left, so the defined channels
- *          always start with channel 0 to x and the undefined ones are from x+1
- *          to PWM_CHANNELS.
- *
- * @warning All the channels not in active use must be set to GPIO_UNDEF; just
- *          initializing fewer members of pin would insert a 0 value, which
- *          would be interpreted as the P0.00 pin that's then driven.
- */
-#if defined(PWM_PRESENT) || DOXYGEN
-typedef struct {
-    NRF_PWM_Type *dev;                  /**< PWM device descriptor */
-    gpio_t pin[PWM_CHANNELS];           /**< PWM out pins */
-} pwm_conf_t;
-#endif
-
 /**
  * @brief   Size of the UART TX buffer for non-blocking mode.
  */

cpu/nrf53/include/periph_cpu.h

@@ -26,6 +26,15 @@
 extern "C" {
 #endif
 
+/**
+ * @brief    Peripheral clocks speed
+ */
+#define PERIPH_CLOCK_1MHZ   MHZ(1)          /**<  1MHz peripheral clock */
+#define PERIPH_CLOCK_16MHZ  MHZ(16)         /**< 16MHz peripheral clock */
+#define PERIPH_CLOCK_32MHZ  MHZ(32)         /**< 32MHz peripheral clock */
+#define PERIPH_CLOCK_64MHZ  MHZ(64)         /**< 64MHz peripheral clock */
+#define PERIPH_CLOCK        PERIPH_CLOCK_16MHZ /**< For driver compatibility */
+
 #ifndef DOXYGEN
 /**
  * @brief Wrapper to fix differences between nRF families vendor files

cpu/nrf5x_common/include/periph_cpu_common.h

@@ -302,6 +302,55 @@ typedef struct {
 
 #endif /* ndef CPU_MODEL_NRF52832XXAA && ndef CPU_FAM_NRF51 */
 
+#if !defined(CPU_FAM_NRF51) && !defined(DOXYGEN)
+/**
+ * @brief    The PWM unit on the nRF52, nRF53 and nRF9160
+ *          supports 4 channels per device
+ */
+#define PWM_CHANNELS        (4U)
+
+/**
+ * @brief    Generate PWM mode values
+ *
+ * To encode the PWM mode, we use two bit:
+ * - bit  0: select up or up-and-down counting
+ * - bit 15: select polarity
+ */
+#define PWM_MODE(ud, pol)   (ud | (pol << 15))
+
+/**
+ * @brief   Override the PWM mode definitions
+ */
+#define HAVE_PWM_MODE_T
+typedef enum {
+    PWM_LEFT       = PWM_MODE(0, 1),    /**< left aligned PWM */
+    PWM_RIGHT      = PWM_MODE(0, 0),    /**< right aligned PWM */
+    PWM_CENTER     = PWM_MODE(1, 1),    /**< not supported */
+    PWM_CENTER_INV = PWM_MODE(1, 0)     /**< not supported */
+} pwm_mode_t;
+
+/**
+ * @brief   PWM configuration options
+ *
+ * Each device supports up to 4 channels. If you want to use less than 4
+ * channels, just set the unused pins to GPIO_UNDEF.
+ *
+ * @note    define unused pins only from right to left, so the defined channels
+ *          always start with channel 0 to x and the undefined ones are from x+1
+ *          to PWM_CHANNELS.
+ *
+ * @warning All the channels not in active use must be set to GPIO_UNDEF; just
+ *          initializing fewer members of pin would insert a 0 value, which
+ *          would be interpreted as the P0.00 pin that's then driven.
+ */
+#if defined(PWM_PRESENT)
+typedef struct {
+    NRF_PWM_Type *dev;                  /**< PWM device descriptor */
+    gpio_t pin[PWM_CHANNELS];           /**< PWM out pins */
+} pwm_conf_t;
+#endif
+#endif /* ndef CPU_FAM_NRF51 */
+
 #ifdef __cplusplus
 }
 #endif

cpu/nrf5x_common/periph/Makefile

@@ -7,6 +7,14 @@ ifneq (,$(filter periph_i2c,$(USEMODULE)))
   endif
 endif
 
+# Select the specific implementation for `periph_pwm`
+# nRF51 has its own PWM driver variant in its periph driver folder
+ifneq (,$(filter periph_pwm,$(USEMODULE)))
+  ifneq (,$(filter $(CPU_FAM),nrf52 nrf53 nrf9160))
+    SRC += pwm_nrfxx.c
+  endif
+endif
+
 # Select the specific implementation for `periph_spi`
 ifneq (,$(filter periph_spi,$(USEMODULE)))
   ifneq (,$(filter $(CPU_FAM),nrf52 nrf9160))

cpu/nrf52/periph/pwm.c → cpu/nrf5x_common/periph/pwm_nrfxx.c

@@ -7,7 +7,7 @@
  */
 
 /**
- * @ingroup     cpu_nrf52
+ * @ingroup     cpu_nrf5x_common
  * @{
  *
  * @file

sys/include/net/nanocoap.h

@@ -92,6 +92,7 @@
 #include "iolist.h"
 #include "macros/utils.h"
 #include "net/coap.h"
+#include "modules.h"
 #else
 #include "coap.h"
 #include <arpa/inet.h>
@@ -513,15 +514,37 @@ static inline unsigned coap_get_id(const coap_pkt_t *pkt)
 /**
  * @brief   Get a message's token length [in byte]
  *
+ * If the `nanocoap_token_ext` module is enabled, this will include
+ * the extended token length.
+ *
  * @param[in]   pkt   CoAP packet
  *
  * @returns     length of token in the given message (0-8 byte)
  */
 static inline unsigned coap_get_token_len(const coap_pkt_t *pkt)
 {
-    return (pkt->hdr->ver_t_tkl & 0xf);
+    uint8_t tkl = pkt->hdr->ver_t_tkl & 0xf;
+
+    if (!IS_USED(MODULE_NANOCOAP_TOKEN_EXT)) {
+        return tkl;
+    }
+
+    void *ext = pkt->hdr + 1;
+    switch (tkl) {
+    case 13:
+        return tkl + *(uint8_t *)ext;
+    case 14:
+        return tkl + 255 + byteorder_bebuftohs(ext);
+    case 15:
+        assert(0);
+        /* fall-through */
+    default:
+        return tkl;
+    }
 }
 
+static inline uint8_t *coap_hdr_data_ptr(const coap_hdr_t *hdr);
+
 /**
  * @brief   Get pointer to a message's token
  *
@@ -531,7 +554,7 @@ static inline unsigned coap_get_token_len(const coap_pkt_t *pkt)
  */
 static inline void *coap_get_token(const coap_pkt_t *pkt)
 {
-    return (uint8_t*)pkt->hdr + sizeof(coap_hdr_t);
+    return coap_hdr_data_ptr(pkt->hdr);
 }
 
 /**
@@ -587,6 +610,35 @@ static inline unsigned coap_get_ver(const coap_pkt_t *pkt)
     return (pkt->hdr->ver_t_tkl & 0x60) >> 6;
 }
 
+/**
+ * @brief   Get the size of the extended Token length field
+ *          (RFC 8974)
+ *
+ * @note    This requires the `nanocoap_token_ext` module to be enabled
+ *
+ * @param[in]   hdr   CoAP header
+ *
+ * @returns     number of bytes used for extended token length
+ */
+static inline uint8_t coap_hdr_tkl_ext_len(const coap_hdr_t *hdr)
+{
+    if (!IS_USED(MODULE_NANOCOAP_TOKEN_EXT)) {
+        return 0;
+    }
+
+    switch (hdr->ver_t_tkl & 0xf) {
+    case 13:
+        return 1;
+    case 14:
+        return 2;
+    case 15:
+        assert(0);
+        /* fall-through */
+    default:
+        return 0;
+    }
+}
+
 /**
  * @brief   Get the start of data after the header
  *
@@ -596,7 +648,7 @@ static inline unsigned coap_get_ver(const coap_pkt_t *pkt)
  */
 static inline uint8_t *coap_hdr_data_ptr(const coap_hdr_t *hdr)
 {
-    return ((uint8_t *)hdr) + sizeof(coap_hdr_t);
+    return ((uint8_t *)hdr) + sizeof(coap_hdr_t) + coap_hdr_tkl_ext_len(hdr);
 }
 
 /**

sys/net/application_layer/nanocoap/nanocoap.c

@@ -98,7 +98,13 @@ int coap_parse(coap_pkt_t *pkt, uint8_t *buf, size_t len)
         return -EBADMSG;
     }
 
-    if (coap_get_token_len(pkt) > COAP_TOKEN_LENGTH_MAX) {
+    if (IS_USED(MODULE_NANOCOAP_TOKEN_EXT)) {
+        if ((pkt->hdr->ver_t_tkl & 0xf) == 15) {
+            DEBUG("nanocoap: token length is reserved value 15,"
+                  "invalid for extended token length field.\n");
+            return -EBADMSG;
+        }
+    } else if (coap_get_token_len(pkt) > COAP_TOKEN_LENGTH_MAX) {
         DEBUG("nanocoap: token length invalid\n");
         return -EBADMSG;
     }
@@ -586,18 +592,39 @@ ssize_t coap_build_hdr(coap_hdr_t *hdr, unsigned type, uint8_t *token,
                        size_t token_len, unsigned code, uint16_t id)
 {
     assert(!(type & ~0x3));
-    assert(!(token_len & ~0x1f));
+
+    uint16_t tkl_ext;
+    uint8_t tkl_ext_len, tkl;
+
+    if (token_len > 268 && IS_USED(MODULE_NANOCOAP_TOKEN_EXT)) {
+        tkl_ext_len = 2;
+        tkl_ext = htons(token_len - 269); /* 269 = 255 + 14 */
+        tkl = 14;
+    }
+    else if (token_len > 12 && IS_USED(MODULE_NANOCOAP_TOKEN_EXT)) {
+        tkl_ext_len = 1;
+        tkl_ext = token_len - 13;
+        tkl = 13;
+    }
+    else {
+        tkl = token_len;
+        tkl_ext_len = 0;
+    }
 
     memset(hdr, 0, sizeof(coap_hdr_t));
-    hdr->ver_t_tkl = (COAP_V1 << 6) | (type << 4) | token_len;
+    hdr->ver_t_tkl = (COAP_V1 << 6) | (type << 4) | tkl;
     hdr->code = code;
     hdr->id = htons(id);
 
+    if (tkl_ext_len) {
+        memcpy(hdr + 1, &tkl_ext, tkl_ext_len);
+    }
+
     if (token_len) {
         memcpy(coap_hdr_data_ptr(hdr), token, token_len);
     }
 
-    return sizeof(coap_hdr_t) + token_len;
+    return sizeof(coap_hdr_t) + token_len + tkl_ext_len;
 }
 
 void coap_pkt_init(coap_pkt_t *pkt, uint8_t *buf, size_t len, size_t header_len)

tests/unittests/tests-nanocoap/Makefile.include

@@ -1 +1,2 @@
 USEMODULE += nanocoap
+USEMODULE += nanocoap_token_ext