[riot-notifications] [RIOT-OS/RIOT] nrf52: Implement EasyDMA-based SPI peripheral implemenation (#14057)

benpicco notifications at github.com
Tue May 12 00:44:09 CEST 2020


@benpicco commented on this pull request.

Looking good - some small comments.

> +}
+
+void spi_transfer_bytes(spi_t bus, spi_cs_t cs, bool cont,
+                        const void *out, void *in, size_t len)
+{
+    const uint8_t *out_buf = out;
+    uint8_t *in_buf = in;
+
+    assert(out_buf || in_buf);
+
+    if (cs != SPI_CS_UNDEF) {
+        gpio_clear((gpio_t)cs);
+    }
+
+    /* Enable the workaround when the length is only 1 byte */
+#ifdef CPU_MODEL_NRF52832XXAA

Better have some

```C
#ifdef CPU_MODEL_NRF52832XXAA
#define ERRATA58_WORKAROUND (1)
#endif
```

at the start of the file

> +void spi_transfer_bytes(spi_t bus, spi_cs_t cs, bool cont,
+                        const void *out, void *in, size_t len)
+{
+    const uint8_t *out_buf = out;
+    uint8_t *in_buf = in;
+
+    assert(out_buf || in_buf);
+
+    if (cs != SPI_CS_UNDEF) {
+        gpio_clear((gpio_t)cs);
+    }
+
+    /* Enable the workaround when the length is only 1 byte */
+#ifdef CPU_MODEL_NRF52832XXAA
+    size_t _len = len;
+    if (_len == 1) {

We can reduce the amount of `ifdef`s by relying on the optimizer 

```suggestion
    if (ERRATA58_WORKAROUND &&
        _len == 1) {
```


> +    if (_len == 1) {
+        _enable_workaround(bus);
+    }
+#endif
+
+    /* Enable IRQ */
+    dev(bus)->INTENSET = SPIM_INTENSET_END_Msk;
+
+    do {
+        size_t transfer_len = len > UINT8_MAX ? UINT8_MAX : len;
+        _transfer(bus, out_buf, in_buf, transfer_len);
+        /* Block until the irq releases the mutex, then lock it again for the
+         * next transfer */
+        mutex_lock(&busy[bus]);
+        out_buf += out_buf ? transfer_len : 0;
+        in_buf += in_buf? transfer_len : 0;

```suggestion
        in_buf += in_buf ? transfer_len : 0;
```

> +#ifdef CPU_MODEL_NRF52840XXAA
+void isr_spi3(void)

`NRF52811XXAA` also defines `isr_spi0`

-- 
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/14057#pullrequestreview-409557407
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200511/5bd44d21/attachment.htm>


More information about the notifications mailing list