[riot-notifications] [RIOT-OS/RIOT] boards: slstk3401a: add support (v2) (#8630)

Alexandre Abadie notifications at github.com
Tue Feb 27 15:29:32 CET 2018


aabadie requested changes on this pull request.

Made a pass of review and found minor things.

> @@ -0,0 +1,9 @@
+ifneq (,$(filter saul_default,$(USEMODULE)))
+    USEMODULE += saul_gpio

indentation should be 2 spaces here

> @@ -0,0 +1,14 @@
+# Put defined MCU peripherals here (in alphabetical order)
+FEATURES_PROVIDED += periph_adc
+FEATURES_PROVIDED += periph_gpio
+FEATURES_PROVIDED += periph_i2c
+FEATURES_PROVIDED += periph_rtc
+FEATURES_PROVIDED += periph_rtt
+FEATURES_PROVIDED += periph_spi
+FEATURES_PROVIDED += periph_timer
+FEATURES_PROVIDED += periph_uart
+
+# The board MPU family (used for grouping by the CI system)
+FEATURES_MCU_GROUP = cortex_m4_2
+
+include $(RIOTCPU)/efm32/Makefile.features

just comparing to nucleos and I think if a '-' at the beginning of the line is required

> + * Connection to the on-board Sharp Memory LCD (LS013B7DH03).
+ * @{
+ */
+#define DISP_SPI            SPI_DEV(0)
+#define DISP_COM_PIN        GPIO_PIN(PD, 13)
+#define DISP_CS_PIN         GPIO_PIN(PD, 14)
+#define DISP_EN_PIN         GPIO_PIN(PD, 15)
+/** @} */
+
+/**
+ * @name    Temperature sensor configuration
+ *
+ * Connection to the on-board temperature/humidity sensor (Si7021).
+ * @{
+ */
+#ifndef SI7021_ENABLED

I don't think this is the way to go. Just bind this to the fact that the module is loaded or not, e.g. use `MODULE_SI7021`, or even `MODULE_SI70XX`

> + * Connection to the on-board Sharp Memory LCD (LS013B7DH03).
+ * @{
+ */
+#define DISP_SPI            SPI_DEV(0)
+#define DISP_COM_PIN        GPIO_PIN(PD, 13)
+#define DISP_CS_PIN         GPIO_PIN(PD, 14)
+#define DISP_EN_PIN         GPIO_PIN(PD, 15)
+/** @} */
+
+/**
+ * @name    Temperature sensor configuration
+ *
+ * Connection to the on-board temperature/humidity sensor (Si7021).
+ * @{
+ */
+#ifndef SI7021_ENABLED

So in fact this part is not needed.

> +
+#include "board.h"
+#include "board_common.h"
+
+#include "periph/gpio.h"
+
+void board_init(void)
+{
+    /* initialize the CPU */
+    cpu_init();
+
+    /* perform common board initialization */
+    board_common_init();
+
+    /* initialize the Si7021 sensor */
+#if SI7021_ENABLED

use:
```c
#ifdef MODULE_SI70XX
```
instead. This flag is set automatically when one adds USEMODULE += si7021 in the application makefile.

-- 
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/8630#pullrequestreview-99700418
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20180227/a98670d1/attachment.html>


More information about the notifications mailing list