[riot-notifications] [RIOT-OS/RIOT] added SPI support for Nucleo-F767ZI (#11142)

Alexandre Abadie notifications at github.com
Tue May 21 11:33:01 CEST 2019


aabadie requested changes on this pull request.

According to the datasheet the configuration proposed here is correct but could be improved for usability with the Arduino compatible pinout.

Please update the commit message according to [our contributing guidelines](https://github.com/RIOT-OS/RIOT/blob/master/CONTRIBUTING.md#general-rules): `boards/nucleo-f767zi: add spi config`

> +    {       /* for APB2 @ 60000000Hz */
+        7,  /* -> 234375Hz */
+        6,  /* -> 468750Hz */
+        5,  /* -> 937500Hz */
+        3,  /* -> 3750000Hz */
+        2   /* -> 7500000Hz */
+    }
+};
+
+static const spi_conf_t spi_config[] = {
+    {
+        .dev      = SPI1,
+        .mosi_pin = GPIO_PIN(PORT_A, 7),
+        .miso_pin = GPIO_PIN(PORT_A, 6),
+        .sclk_pin = GPIO_PIN(PORT_A, 5),
+        .cs_pin   = GPIO_PIN(PORT_A, 4),

To be compatible with Arduino pinout, this should be PD14 (D10) or just leave it as GPIO_UNDEF and let the developer set it via a custom driver configuration.

> +
+static const spi_conf_t spi_config[] = {
+    {
+        .dev      = SPI1,
+        .mosi_pin = GPIO_PIN(PORT_A, 7),
+        .miso_pin = GPIO_PIN(PORT_A, 6),
+        .sclk_pin = GPIO_PIN(PORT_A, 5),
+        .cs_pin   = GPIO_PIN(PORT_A, 4),
+        .af       = GPIO_AF5,
+        .rccmask  = RCC_APB2ENR_SPI1EN,
+        .apbbus   = APB2
+    },
+    {
+        .dev      = SPI2,
+        .mosi_pin = GPIO_PIN(PORT_B, 15),
+        .miso_pin = GPIO_PIN(PORT_C, 2),

Why not simply use PB14 for this one ?

-- 
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/11142#pullrequestreview-239924697
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190521/d19a52bb/attachment-0001.html>


More information about the notifications mailing list