[riot-notifications] [RIOT-OS/RIOT] boards: Adding board support for the Seeeduino XIAO (#16469)

benpicco notifications at github.com
Wed May 26 11:09:28 CEST 2021


@benpicco commented on this pull request.

looks good, some nits

> +#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+config BOARD
+    default "seeeduino_XIAO" if BOARD_SEEEDUINO_XIAO
+
+config BOARD_SEEEDUINO_XIAO
+    bool
+    default y
+    select CPU_MODEL_SAMD21G18A
+    select HAS_HIGHLEVEL_STDIO
+    select HAS_PERIPH_ADC
+    select HAS_PERIPH_I2C
+   # select HAS_PERIPH_PWM not implemented yet

just remove that line, PWM is implemented for the CPU, it just lacks the board configuration. 

> + * @author      Franz Freitag <franz.freitag at st.ovgu.de>
+ * @author	Justus Krebs <justus.krebs at st.ovgu.de>

alignment 

> + * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     boards_seeeduino_xiao
+ * @{
+ *
+ * @file
+ * @brief       Board specific implementations for the Seeeduino XIAO board
+ *
+ * @author      Franz Freitag <franz.freitag at st.ovgu.de>
+ * @author	Justus Krebs <justus.krebs at st.ovgu.de>
+ * @author	Nick Weiler <nick.weiler at st.ovgu.de>
+ * @author	Benjamin Valentin <benpicco at googlemail.com>

I don't even have this board :wink: 

> + * @author	Nick Weiler <nick.weiler at st.ovgu.de>
+ * @author	Benjamin Valentin <benpicco at googlemail.com>
+ *
+ * @}
+ */
+
+#include "cpu.h"
+#include "board.h"
+#include "mtd.h"
+#include "mtd_spi_nor.h"
+#include "periph/gpio.h"
+#include "periph/spi.h"
+#include "timex.h"
+
+#ifdef MODULE_MTD
+/* GD25Q32C */

Is this the chip that's used?
Have you tried `tests/mtd_raw`?

> +    .clk = SEEEDUINO_XIAO_NOR_SPI_CLK,
+    .flag = SEEEDUINO_XIAO_NOR_FLAGS,
+    .spi = SEEEDUINO_XIAO_NOR_SPI_DEV,
+    .mode = SEEEDUINO_XIAO_NOR_SPI_MODE,
+    .cs = SEEEDUINO_XIAO_NOR_SPI_CS,
+    .wp = GPIO_UNDEF,
+    .hold = GPIO_UNDEF,
+    .addr_width = 3,
+};
+
+static mtd_spi_nor_t seeeduino_XIAO_nor_dev = {
+    .base = {
+        .driver = &mtd_spi_nor_driver,
+        .page_size = SEEEDUINO_XIAO_NOR_PAGE_SIZE,
+        .pages_per_sector = SEEEDUINO_XIAO_NOR_PAGES_PER_SECTOR,
+        .sector_count = SEEEDUINO_XIAO_NOR_SECTOR_COUNT,

```suggestion
```
You can remove this line to enable auto-detection of the flash chip size 

> +        .page_size = SEEEDUINO_XIAO_NOR_PAGE_SIZE,
+        .pages_per_sector = SEEEDUINO_XIAO_NOR_PAGES_PER_SECTOR,
+        .sector_count = SEEEDUINO_XIAO_NOR_SECTOR_COUNT,
+    },
+    .params = &_seeeduino_XIAO_nor_params,
+};
+
+mtd_dev_t *mtd0 = (mtd_dev_t *)&seeeduino_XIAO_nor_dev;
+#endif /* MODULE_MTD */
+
+void board_init(void)
+{
+    /* initialize the CPU */
+    cpu_init();
+
+    /* initialize the on-board red LEDs */

later on you list them as blue and yellow :wink: 

> +};
+
+#define ADC_NUMOF           ARRAY_SIZE(adc_channels)
+/** @} */
+
+/**
+ * @name USB peripheral configuration
+ * @{
+ */
+static const sam0_common_usb_config_t sam_usbdev_config[] = {
+    {
+        .dm     = GPIO_PIN(PA, 24),
+        .dp     = GPIO_PIN(PA, 25),
+        .d_mux  = GPIO_MUX_G,
+        .device = &USB->DEVICE,
+        .gclk_src = SAM0_GCLK_MAIN,

I'd go with

```suggestion
        .gclk_src = SAM0_GCLK_48MHZ,
```

just to be sure USB is always sourced from a 48 MHz clock even if a different CPU frequency is configured. 

-- 
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/16469#pullrequestreview-668773032
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210526/f526ca1a/attachment.htm>


More information about the notifications mailing list