[riot-notifications] [RIOT-OS/RIOT] cpu/nrf52: initial kconfig modeling (no netif) (#16837)

Leandro Lanzieri notifications at github.com
Tue Sep 21 11:42:30 CEST 2021


@leandrolanzieri commented on this pull request.

Thanks for this! Some initial thoughts

> +config MODULE_SAUL_DEFAULT
+    select MODULE_SAUL_NRF_TEMPERATURE

`MODULE_SAUL_NRF_TEMPERATURE` is already defined in `drivers/saul/Kconfig`.
Maybe we can follow here the 'feature selects module` approach?

```
config HAVE_SAUL_NRF_TEMPERATURE
    bool
    select MODULE_SAUL_NRF_TEMPERATURE if MODULE_SAUL_DEFAULT && HAS_PERIPH_TEMPERATURE
    help
        Indicates that a SAUL wrapper to the temperature peripheral is present.
```

> @@ -13,4 +13,15 @@ config BOARD_COMMON_NRF52XXXDK
     select HAS_PERIPH_UART
     select HAS_VDD_LC_FILTER_REG1
 
+if TEST_KCONFIG
+
+config MODULE_BOARDS_COMMON_NRF52XXDK
+    bool
+    default y
+    select MODULE_SAUL_GPIO if MODULE_SAUL_DEFAULT && HAS_PERIPH_GPIO

Enabling `MODULE_SAUL_GPIO` when `MODULE_SAUL_DEFAULT` is done already in `drivers/saul/Kconfig` if `HAVE_SAUL_GPIO` is provided.

> +config HAVE_LIS2DH12
+    bool
+    select MODULE_LIS2DH12 if MODULE_SAUL_DEFAULT
+    help
+        Indicates that a lisdh12 is present
+

I think this feature should be placed in `drivers/lisdh12/Kconfig` and selected by the boards that have the device.

> +choice
+    bool "Backend"
+    depends on MODULE_NETDEV_DEFAULT
+    default NRF802154
+    default NRFBLE
+
+config NRF802154
+    bool "nrf802154"
+    select MODULE_NRF802154
+
+config NRFBLE
+    bool "nrfble"
+    select MODULE_NRFBLE
+
+config NRFMIN
+    bool "nrfmin"
+    select MODULE_NRFMIN
+
+endchoice

According to what I see in the `Makefile.dep`, the selection of these modules depends on `CPU_MODEL`, I think it might have to do with having `HAS_RADIO_NRF802154` but I'm not sure, as this is done by model name so far.

I think it makes sense to have a symbol like jose proposes, which selects a module that will enable this choice. I'm thinking of something in the direction of:
```
config HAVE_NRF52_RADIO
    bool
    select MODULE_NRF52_RADIO if MODULE_NETDEV_DEFAULT
    help
        Indicates that an NRF52 radio is present.

menuconfig MODULE_NRF52_RADIO
    bool "NRF52 radio driver"
    depends on HAVE_NRF52_RADIO
    depends on TEST_KCONFIG

if MODULE_NRF52_RADIO

choice
    bool "NRF52 radio backend"

config NRF802154
    bool "nrf802154"
    select MODULE_NRF802154

config NRFBLE
    bool "nrfble"
    select MODULE_NRFBLE

config NRFMIN
    bool "nrfmin"
    select MODULE_NRFMIN

endchoice

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/16837#pullrequestreview-759383044
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210921/c6bd359a/attachment-0001.htm>


More information about the notifications mailing list