Merge pull request #19792 from kyleb29/bugfix-19787

cpu/stm32/periph_i2c: prevent corrupting AFIO->MAPR

boards/common/blxxxpill/include/board_common.h

@@ -56,14 +56,6 @@ extern "C" {
 #define XTIMER_BACKOFF      (19)
 /** @} */
 
-/* The boards debug header only exports  SWD, so JTAG-only pins PA15, PB3(*),
- * and PB4 can be remapped as regular GPIOs instead. (Note: PB3 is also used as
- * SWO.  The user needs to take care to not enable SWO with the debugger while
- * at the same time PB3 is used as GPIO. But RIOT does not use SWO in any case,
- * so if a user adds this feature in her/his own code, she/he should be well
- * aware of this.)
- */
-#define STM32F1_DISABLE_JTAG    /**< Disable JTAG to allow pins being used as GPIOs */
 #ifdef __cplusplus
 }
 #endif

boards/nucleo-f103rb/Makefile.include

@@ -1,9 +1,2 @@
 # load the common Makefile.include for Nucleo boards
 include $(RIOTBOARD)/common/nucleo64/Makefile.include
-
-# On-board debugger uses SWD, so JTAG-only pins PA15, PB3(*), and PB4 can be
-# remapped as regular GPIOs instead. (Note: PB3 is also used as SWO. The user
-# needs to take care to not enable SWO with the debugger while at the same time
-# PB3 is used as GPIO. But RIOT does not use SWO in any case, so if a user adds
-# this feature in her/his own code, she/he should be well aware of this.)
-CFLAGS += -DSTM32F1_DISABLE_JTAG

cpu/stm32/cpu_init.c

@@ -333,6 +333,26 @@ void _wlx5xx_init_subghz_debug_pins(void)
 #endif
 }
 
+static void swj_init(void)
+{
+#if defined(CPU_FAM_STM32F1)
+    /* Only if the selected SWJ config differs from the reset value, we
+     * actually need to do something. Since both sides are compile time
+     * constants, this hole code gets optimized out by default */
+    if (CONFIG_AFIO_MAPR_SWJ_CFG != SWJ_CFG_FULL_SWJ) {
+        /* The remapping periph clock must first be enabled */
+        RCC->APB2ENR |= RCC_APB2ENR_AFIOEN;
+        /* Handling of the MAPR register is a bit involved due to the
+         * write-only nature of the SWJ_CFG field, which returns undefined
+         * garbage on read. `afio_mapr_read()` will read the current MAPR
+         * value, but clear the SWF_CFG vield. `afio_mapr_wriote()` will then
+         * write the value read back, but apply the `SWF_CFG` configuration
+         * from `CONFIG_AFIO_MAPR_SWJ_CFG` first.*/
+        afio_mapr_write(afio_mapr_read());
+    }
+#endif
+}
+
 void cpu_init(void)
 {
     /* initialize the Cortex-M core */
@@ -362,10 +382,7 @@ void cpu_init(void)
     /* initialize stdio prior to periph_init() to allow use of DEBUG() there */
     early_init();
 
-#ifdef STM32F1_DISABLE_JTAG
-    RCC->APB2ENR |= RCC_APB2ENR_AFIOEN;
-    AFIO->MAPR |= AFIO_MAPR_SWJ_CFG_JTAGDISABLE;
-#endif
+    swj_init();
 
     /* trigger static peripheral initialization */
     periph_init();

cpu/stm32/include/periph/f1/periph_cpu.h

@@ -107,6 +107,67 @@ enum {
 };
 /** @} */
 
+/**
+ * @brief   Possible values of the `SWJ_CFG` field in the AFIO->MAPR register
+ *
+ * @details This wraps the vendor header file preprocessor macros into a
+ *          C language `enum`.
+ */
+typedef enum {
+     /**
+      * @brief  Both JTAG-DP and SW-DP enabled, reset state
+      */
+    SWJ_CFG_FULL_SWJ    = 0,
+    /**
+     * @brief   Both JTAG-DP and SW-DP enabled, but NJTRST disabled and pin
+     *          usable as GPIO
+     */
+    SWJ_CFG_NO_NJTRST   = AFIO_MAPR_SWJ_CFG_NOJNTRST,
+    /**
+     * @brief   Only SW-DP enabled, JTAG pins usable as GPIOS
+     */
+    SWJ_CFG_NO_JTAG_DP  = AFIO_MAPR_SWJ_CFG_JTAGDISABLE,
+    /**
+     * @brief   Neither JTAG-DP nor SW-DP enabled, JTAG and SWD pins usable as
+     *          GPIOS
+     */
+    SWJ_CFG_DISABLED    = AFIO_MAPR_SWJ_CFG_DISABLE,
+} afio_mapr_swj_cfg_t;
+
+#ifndef CONFIG_AFIO_MAPR_SWJ_CFG
+/**
+ * @brief   By default, disable JTAG and keep only SWD
+ *
+ * This frees the JTAG pins for use as regular GPIOs. We do not support flashing
+ * or debugging via JTAG anyway, so there is nothing lost except for a few bytes
+ * of ROM to initialize the `SWJ_CFG` register.
+ */
+#define CONFIG_AFIO_MAPR_SWJ_CFG    SWJ_CFG_NO_JTAG_DP
+#endif
+
+/**
+ * @brief   Read the current value of the AFIO->MAPR register reproducibly
+ *
+ * This will explicitly clear the write-only `SWJ_CFG` field [26:24], as the
+ * values read back are undefined.
+ */
+static inline uint32_t afio_mapr_read(void)
+{
+    return AFIO->MAPR & (~(AFIO_MAPR_SWJ_CFG_Msk));
+}
+
+/**
+ * @brief   Write to the AFIO->MAPR register apply the SWJ configuration
+ *          specified via @ref CONFIG_AFIO_MAPR_SWJ_CFG
+ *
+ * @pre     @p new_value has all bits in the range [26:24] cleared (the
+ *          `SWJ_CFG` field).
+ */
+static inline void afio_mapr_write(uint32_t new_value)
+{
+    AFIO->MAPR = CONFIG_AFIO_MAPR_SWJ_CFG | new_value;
+}
+
 #ifdef __cplusplus
 }
 #endif

cpu/stm32/periph/i2c_2.c

@@ -120,7 +120,7 @@ static void _init_pins(i2c_t dev)
         /* The remapping periph clock must first be enabled */
         RCC->APB2ENR |= RCC_APB2ENR_AFIOEN;
         /* Then the remap can occur */
-        AFIO->MAPR |= AFIO_MAPR_I2C1_REMAP;
+        afio_mapr_write(afio_mapr_read() | AFIO_MAPR_I2C1_REMAP);
     }
     gpio_init_af(i2c_config[dev].scl_pin, GPIO_AF_OUT_OD);
     gpio_init_af(i2c_config[dev].sda_pin, GPIO_AF_OUT_OD);