[riot-notifications] [RIOT-OS/RIOT] drivers/sx126x: Add support for Nucleo -WL55JC (#16579)

José Alamos notifications at github.com
Wed Jul 7 11:02:51 CEST 2021


@jia200x requested changes on this pull request.

Last round of changes.
Also, some commits should be squashed (e.g Kconfig updates should belong to the HW debug commits, and all commits that enable the variant should be one too).

> @@ -1,4 +1,10 @@
 ifneq (,$(filter stdio_uart,$(USEMODULE)))
   FEATURES_REQUIRED += periph_lpuart
 endif
+ifneq (,$(filter netdev_default,$(USEMODULE)))
+  USEMODULE += sx126x_stm32wl
+endif
+ifneq (,$(filter sx126x_stm32wl,$(USEMODULE)))

This is not a board dependency but rather periph/driver. Please move it to the corresponding Makefile.dep under `drivers`

> @@ -182,6 +186,7 @@ int sx126x_init(sx126x_t *dev)
         SX126X_IRQ_TX_DONE |
         SX126X_IRQ_RX_DONE |
         SX126X_IRQ_PREAMBLE_DETECTED |
+        SX126X_IRQ_SYNC_WORD_VALID |

This has no other use right now more than printing a DEBUG message (there's no netdev event associated to it).
I'm not sure if we need it here. If so, it should be at least on it's own commit.

> + *
+ * @file        board.c
+ * @brief       Board specific implementations for the Nucleo-wl55jc board
+ *
+ *
+ * @author      Akshai M <akshai.m at fu-berlin.de>
+ *
+ * @}
+ */
+
+#include <stdio.h>
+
+#include "board.h"
+#include "periph/gpio.h"
+
+#if IS_USED(MODULE_SX126X_RF_SWITCH)

```suggestion
#if IS_USED(MODULE_SX126X_STM32WL)
```

> +
+#include <stdio.h>
+
+#include "board.h"
+#include "periph/gpio.h"
+
+#if IS_USED(MODULE_SX126X_RF_SWITCH)
+#include "sx126x.h"
+#endif
+
+void board_init(void)
+{
+    /* initialize the CPU */
+    board_common_nucleo_init();
+
+    if (IS_USED(MODULE_SX126X_RF_SWITCH)) {

This is a bit tricky, since the feature actually depends on `MODULE_SX126X_RF_SWITCH`. But for the scope of the file, I think it would make more sense to depend on `MODULE_SX126X_STM32WL`
```suggestion
    if (IS_USED(MODULE_SX126X_STM32WL)) {
```

> +
+    if (IS_USED(MODULE_SX126X_RF_SWITCH)) {
+        /* Initialize the GPIO control for RF 3-port switch (SP3T) */
+        gpio_init(FE_CTRL1, GPIO_OUT);
+        gpio_init(FE_CTRL2, GPIO_OUT);
+        gpio_init(FE_CTRL3, GPIO_OUT);
+    }
+}
+
+/**
+ * @brief Callback to set RF switch mode
+ *
+ * This function sets the GPIO's wired to the SP3T RF Switch. Nucleo-WL55JC
+ * supports three modes of operation.
+ */
+#if IS_USED(MODULE_SX126X_RF_SWITCH)

```suggestion
#if IS_USED(MODULE_SX126X_STM32WL)
```

> @@ -31,6 +31,10 @@ extern "C" {
  * @{
  */
 #define SX126X_PARAM_SPI                    (SPI_DEV(0))
+
+#if IS_USED(MODULE_SX126X_RF_SWITCH)

```suggestion
#if IS_USED(MODULE_SX126X_STM32WL)
```

> +#ifndef SX126X_PARAM_SET_RF_MODE_CB
+#define SX126X_PARAM_SET_RF_MODE_CB         NULL
+#else
+extern void SX126X_PARAM_SET_RF_MODE_CB(sx126x_t *dev, sx126x_rf_mode_t rf_mode);
+#endif

I just realized this is not necessary since this file includes `board.h`.
```suggestion
#ifndef SX126X_PARAM_SET_RF_MODE_CB
#define SX126X_PARAM_SET_RF_MODE_CB         NULL
#endif
```

> @@ -79,7 +85,9 @@ extern "C" {
                                     .busy_pin = SX126X_PARAM_BUSY,      \
                                     .dio1_pin = SX126X_PARAM_DIO1,      \
                                     .type     = SX126X_PARAM_TYPE,      \
-                                    .regulator = SX126X_PARAM_REGULATOR }
+                                    .regulator = SX126X_PARAM_REGULATOR, \
+                                    .set_rf_mode = SX126X_PARAM_SET_RF_MODE_CB }

this should be guarded:
```suggestion
#if IS_USED(MODULE_SX126X_RF_SWITCH)
                                    .set_rf_mode = SX126X_PARAM_SET_RF_MODE_CB \
#endif
                                    }
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16579#pullrequestreview-700721468
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210707/142cf716/attachment.htm>


More information about the notifications mailing list