[riot-notifications] [RIOT-OS/RIOT] cpu/saml1x: add support for SAML10 and SAML11 MCUs (Cortex-M23) (#10653)

Alexandre Abadie notifications at github.com
Sun Jan 13 16:29:45 CET 2019


aabadie requested changes on this pull request.

Thanks for updating this PR @dylad !

I just reviewed/tried (saml11-xpro) it again and found some problems:
- `gpio_params.h` is missing but `saul_gpio` is included with the `saul_default` module
- the gpio driver is broken with interrupts
- I found the saml21 and saml1x peripheral implementations very close. I won't ask you to factorize in this PR though, but I see room for improvement here.
- `tests/periph_rtc` and `tests/periph_rtt` doesn't work: the first one is stuck and doesn't print anything, the second one doesn't print "many hellos". Maybe just remove for now the support for these periphs ?
- `tests/periph_flashpage` doesn't build, it needs to be adapted.

I haven't test I2C and SPI but others I tested (cpuid, timer, adc) work.

There are other minor stuff see below.



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

missing newline

> @@ -0,0 +1,4 @@
+MODULE = board
+DIRS = $(RIOTBOARD)/common/saml1x
+
+include $(RIOTBASE)/Makefile.base

missing newline

> @@ -0,0 +1,9 @@
+FEATURES_PROVIDED += periph_adc

It would make more sense to move these provided features to the common boards. They the same for saml11-xpro.

> @@ -99,7 +99,11 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
     if ((rx_cb) && (uart_config[uart].rx_pin != GPIO_UNDEF)) {
         uart_ctx[uart].rx_cb = rx_cb;
         uart_ctx[uart].arg = arg;
+#if defined (CPU_FAM_SAML10) || defined (CPU_FAM_SAML11)

```suggestion
#if defined (CPU_SAML1X)
```

> @@ -0,0 +1,10 @@
+# define the module that is build
+MODULE = cpu
+
+# add a list of subdirectories, that should also be build
+DIRS = periph $(RIOTCPU)/cortexm_common $(RIOTCPU)/sam0_common
+
+# (file triggers compiler bug. see #5775)
+SRC_NOLTO += vectors.c
+
+include $(RIOTBASE)/Makefile.base

missing newline

> @@ -0,0 +1 @@
+-include $(RIOTCPU)/sam0_common/Makefile.features

newline

> @@ -0,0 +1 @@
+include $(RIOTMAKE)/periph.mk

newline

> +#endif
+
+/**
+ * @brief   Mapping of pins to EXTI lines, -1 means not EXTI possible
+ */
+static const int8_t exti_config[1][32] = {
+    { 0,  1,  2,  3,  4,  5,  6,  7, -1, 0,  1,  2, -1, -1, 3, 4,
+      5,  6,  7,  0, -1, -1,  1,  2,  3, 4, -1,  5, -1, -1, 6, 7},
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* PERIPH_CPU_H */
+/** @} */

newline, and maybe in other places (I won't check all of them)

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

The `gpio_params.h` is missing in the include directory. Maybe a mistake when factorizing the code ? It doesn't build `examples/default`

-- 
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/10653#pullrequestreview-191977379
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190113/3b814e52/attachment.html>


More information about the notifications mailing list