*/*spi.c

*/*spi.h

Support for odd bit lengths in SPI, per discussion at micropython#5225

Author: Eric Poulsen

drivers/bus/softspi.c

@@ -4,6 +4,7 @@
  * The MIT License (MIT)
  *
  * Copyright (c) 2016-2018 Damien P. George
+ * Copyright (c) 2019 "Eric Poulsen" <eric@zyxod.com>
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -25,6 +26,7 @@
  */
 
 #include "drivers/bus/spi.h"
+#include "py/runtime.h"
 
 int mp_soft_spi_ioctl(void *self_in, uint32_t cmd) {
     mp_soft_spi_obj_t *self = (mp_soft_spi_obj_t*)self_in;
@@ -44,38 +46,63 @@ int mp_soft_spi_ioctl(void *self_in, uint32_t cmd) {
     return 0;
 }
 
-void mp_soft_spi_transfer(void *self_in, size_t len, const uint8_t *src, uint8_t *dest) {
+void mp_soft_spi_transfer(void *self_in, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits) {
     mp_soft_spi_obj_t *self = (mp_soft_spi_obj_t*)self_in;
     uint32_t delay_half = self->delay_half;
 
+    bits = bits ? bits : self->bits;
+    if (bits == 0) {
+        // Shoudln't be possible with soft SPI, but
+        // just in case.
+        mp_raise_ValueError("bits cannot be 0");
+    }
+
+    int bytesPerChunk = (bits + 7) / 8;
+    // round length down as needed (possibly to zero)
+    len = len / bytesPerChunk * bytesPerChunk;
+    if(len == 0) {
+        return;
+    }
+
+    uint8_t remBits = bits;
+
     // only MSB transfer is implemented
 
     // If a port defines MICROPY_HW_SOFTSPI_MIN_DELAY, and the configured
     // delay_half is equal to this value, then the software SPI implementation
     // will run as fast as possible, limited only by CPU speed and GPIO time.
     #ifdef MICROPY_HW_SOFTSPI_MIN_DELAY
-    if (delay_half == MICROPY_HW_SOFTSPI_MIN_DELAY) {
-        for (size_t i = 0; i < len; ++i) {
-            uint8_t data_out = src[i];
+    if (delay_half == MICROPY_HW_SOFTSPI_MIN_DELAY)
+    {
+        while (len--) {
+            uint8_t bitsThisByte = remBits < 8 ? remBits : 8;
+            uint8_t data_out = *src++;
             uint8_t data_in = 0;
-            for (int j = 0; j < 8; ++j, data_out <<= 1) {
+
+            remBits -= bitsThisByte;
+            for (; bitsThisByte; --bitsThisByte, data_out <<= 1) {
                 mp_hal_pin_write(self->mosi, (data_out >> 7) & 1);
                 mp_hal_pin_write(self->sck, 1 - self->polarity);
                 data_in = (data_in << 1) | mp_hal_pin_read(self->miso);
                 mp_hal_pin_write(self->sck, self->polarity);
             }
+            remBits = remBits ? remBits : bits;
             if (dest != NULL) {
-                dest[i] = data_in;
+                *dest++ = data_in;
             }
         }
         return;
     }
-    #endif
+#endif
 
-    for (size_t i = 0; i < len; ++i) {
-        uint8_t data_out = src[i];
+
+    while(len--) {
+        uint8_t bitsThisByte = remBits < 8 ? remBits : 8;
+        uint8_t data_out = *src++;
         uint8_t data_in = 0;
-        for (int j = 0; j < 8; ++j, data_out <<= 1) {
+
+        remBits -= bitsThisByte;
+        for(;bitsThisByte; --bitsThisByte, data_out <<= 1) {
             mp_hal_pin_write(self->mosi, (data_out >> 7) & 1);
             if (self->phase == 0) {
                 mp_hal_delay_us_fast(delay_half);
@@ -93,8 +120,9 @@ void mp_soft_spi_transfer(void *self_in, size_t len, const uint8_t *src, uint8_t
                 mp_hal_delay_us_fast(delay_half);
             }
         }
+        remBits = remBits ? remBits : bits;
         if (dest != NULL) {
-            dest[i] = data_in;
+            *dest++ = data_in;
         }
     }
 }

drivers/bus/spi.h

@@ -35,13 +35,14 @@ enum {
 
 typedef struct _mp_spi_proto_t {
     int (*ioctl)(void *self, uint32_t cmd);
-    void (*transfer)(void *self, size_t len, const uint8_t *src, uint8_t *dest);
+    void (*transfer)(void *self, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits);
 } mp_spi_proto_t;
 
 typedef struct _mp_soft_spi_obj_t {
     uint32_t delay_half; // microsecond delay for half SCK period
     uint8_t polarity;
     uint8_t phase;
+    uint8_t bits;
     mp_hal_pin_obj_t sck;
     mp_hal_pin_obj_t mosi;
     mp_hal_pin_obj_t miso;
@@ -50,6 +51,6 @@ typedef struct _mp_soft_spi_obj_t {
 extern const mp_spi_proto_t mp_soft_spi_proto;
 
 int mp_soft_spi_ioctl(void *self, uint32_t cmd);
-void mp_soft_spi_transfer(void *self, size_t len, const uint8_t *src, uint8_t *dest);
+void mp_soft_spi_transfer(void *self, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits);
 
 #endif // MICROPY_INCLUDED_DRIVERS_BUS_SPI_H

drivers/memory/spiflash.c

@@ -69,7 +69,7 @@ STATIC void mp_spiflash_write_cmd_data(mp_spiflash_t *self, uint8_t cmd, size_t
     if (c->bus_kind == MP_SPIFLASH_BUS_SPI) {
         // Note: len/data are unused for standard SPI
         mp_hal_pin_write(c->bus.u_spi.cs, 0);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 1, &cmd, NULL);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 1, &cmd, NULL, 8);
         mp_hal_pin_write(c->bus.u_spi.cs, 1);
     } else {
         c->bus.u_qspi.proto->write_cmd_data(c->bus.u_qspi.data, cmd, len, data);
@@ -81,9 +81,9 @@ STATIC void mp_spiflash_write_cmd_addr_data(mp_spiflash_t *self, uint8_t cmd, ui
     if (c->bus_kind == MP_SPIFLASH_BUS_SPI) {
         uint8_t buf[4] = {cmd, addr >> 16, addr >> 8, addr};
         mp_hal_pin_write(c->bus.u_spi.cs, 0);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 4, buf, NULL);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 4, buf, NULL, 8);
         if (len) {
-            c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, src, NULL);
+            c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, src, NULL, 8);
         }
         mp_hal_pin_write(c->bus.u_spi.cs, 1);
     } else {
@@ -96,8 +96,8 @@ STATIC uint32_t mp_spiflash_read_cmd(mp_spiflash_t *self, uint8_t cmd, size_t le
     if (c->bus_kind == MP_SPIFLASH_BUS_SPI) {
         uint32_t buf;
         mp_hal_pin_write(c->bus.u_spi.cs, 0);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 1, &cmd, NULL);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, (void*)&buf, (void*)&buf);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 1, &cmd, NULL, 8);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, (void*)&buf, (void*)&buf, 8);
         mp_hal_pin_write(c->bus.u_spi.cs, 1);
         return buf;
     } else {
@@ -110,8 +110,8 @@ STATIC void mp_spiflash_read_data(mp_spiflash_t *self, uint32_t addr, size_t len
     if (c->bus_kind == MP_SPIFLASH_BUS_SPI) {
         uint8_t buf[4] = {CMD_READ, addr >> 16, addr >> 8, addr};
         mp_hal_pin_write(c->bus.u_spi.cs, 0);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 4, buf, NULL);
-        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, dest, dest);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, 4, buf, NULL, 8);
+        c->bus.u_spi.proto->transfer(c->bus.u_spi.data, len, dest, dest, 8);
         mp_hal_pin_write(c->bus.u_spi.cs, 1);
     } else {
         c->bus.u_qspi.proto->read_cmd_qaddr_qdata(c->bus.u_qspi.data, CMD_C4READ, addr, len, dest);

extmod/machine_spi.c

@@ -81,50 +81,59 @@ STATIC mp_obj_t machine_spi_deinit(mp_obj_t self) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(machine_spi_deinit_obj, machine_spi_deinit);
 
-STATIC void mp_machine_spi_transfer(mp_obj_t self, size_t len, const void *src, void *dest) {
+STATIC void mp_machine_spi_transfer(mp_obj_t self, size_t len, const void *src, void *dest, uint8_t bits) {
     mp_obj_base_t *s = (mp_obj_base_t*)MP_OBJ_TO_PTR(self);
     mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)s->type->protocol;
-    spi_p->transfer(s, len, src, dest);
+    spi_p->transfer(s, len, src, dest, bits);
 }
 
 STATIC mp_obj_t mp_machine_spi_read(size_t n_args, const mp_obj_t *args) {
     vstr_t vstr;
     vstr_init_len(&vstr, mp_obj_get_int(args[1]));
-    memset(vstr.buf, n_args == 3 ? mp_obj_get_int(args[2]) : 0, vstr.len);
-    mp_machine_spi_transfer(args[0], vstr.len, vstr.buf, vstr.buf);
+    memset(vstr.buf, n_args >= 3 ? mp_obj_get_int(args[2]) : 0, vstr.len);
+    uint8_t bits = n_args == 4 ? mp_obj_get_int(args[3]) : 0;
+    mp_machine_spi_transfer(args[0], vstr.len, vstr.buf, vstr.buf, bits);
     return mp_obj_new_str_from_vstr(&mp_type_bytes, &vstr);
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_read_obj, 2, 3, mp_machine_spi_read);
+MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_read_obj, 2, 4, mp_machine_spi_read);
 
 STATIC mp_obj_t mp_machine_spi_readinto(size_t n_args, const mp_obj_t *args) {
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[1], &bufinfo, MP_BUFFER_WRITE);
-    memset(bufinfo.buf, n_args == 3 ? mp_obj_get_int(args[2]) : 0, bufinfo.len);
-    mp_machine_spi_transfer(args[0], bufinfo.len, bufinfo.buf, bufinfo.buf);
+    memset(bufinfo.buf, n_args >= 3 ? mp_obj_get_int(args[2]) : 0, bufinfo.len);
+    uint8_t bits = n_args == 4 ? mp_obj_get_int(args[3]) : 0;
+    mp_machine_spi_transfer(args[0], bufinfo.len, bufinfo.buf, bufinfo.buf, bits);
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_readinto_obj, 2, 3, mp_machine_spi_readinto);
+MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_readinto_obj, 2, 4, mp_machine_spi_readinto);
 
-STATIC mp_obj_t mp_machine_spi_write(mp_obj_t self, mp_obj_t wr_buf) {
+STATIC mp_obj_t mp_machine_spi_write(size_t n_args, const mp_obj_t *args) {
+    mp_obj_t self = args[0];
+    mp_obj_t wr_buf = args[1];
+    uint8_t bits = n_args == 3 ? mp_obj_get_int(args[2]) : 0;
     mp_buffer_info_t src;
     mp_get_buffer_raise(wr_buf, &src, MP_BUFFER_READ);
-    mp_machine_spi_transfer(self, src.len, (const uint8_t*)src.buf, NULL);
+    mp_machine_spi_transfer(self, src.len, (const uint8_t*)src.buf, NULL, bits);
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_2(mp_machine_spi_write_obj, mp_machine_spi_write);
+MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_write_obj, 2, 3, mp_machine_spi_write);
 
-STATIC mp_obj_t mp_machine_spi_write_readinto(mp_obj_t self, mp_obj_t wr_buf, mp_obj_t rd_buf) {
+STATIC mp_obj_t mp_machine_spi_write_readinto(size_t n_args, const mp_obj_t *args) {
+    mp_obj_t self = args[0];
+    mp_obj_t wr_buf = args[1];
+    mp_obj_t rd_buf = args[2];
+    uint8_t bits = n_args == 4 ? mp_obj_get_int(args[3]) : 0;
     mp_buffer_info_t src;
     mp_get_buffer_raise(wr_buf, &src, MP_BUFFER_READ);
     mp_buffer_info_t dest;
     mp_get_buffer_raise(rd_buf, &dest, MP_BUFFER_WRITE);
     if (src.len != dest.len) {
         mp_raise_ValueError("buffers must be the same length");
     }
-    mp_machine_spi_transfer(self, src.len, src.buf, dest.buf);
+    mp_machine_spi_transfer(self, src.len, src.buf, dest.buf, bits);
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_3(mp_machine_spi_write_readinto_obj, mp_machine_spi_write_readinto);
+MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_write_readinto_obj, 3, 4, mp_machine_spi_write_readinto);
 
 STATIC const mp_rom_map_elem_t machine_spi_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_init), MP_ROM_PTR(&machine_spi_init_obj) },
@@ -201,8 +210,9 @@ STATIC mp_obj_t mp_machine_soft_spi_make_new(const mp_obj_type_t *type, size_t n
     self->spi.delay_half = baudrate_to_delay_half(args[ARG_baudrate].u_int);
     self->spi.polarity = args[ARG_polarity].u_int;
     self->spi.phase = args[ARG_phase].u_int;
-    if (args[ARG_bits].u_int != 8) {
-        mp_raise_ValueError("bits must be 8");
+    self->spi.bits = args[ARG_bits].u_int;
+    if (self->spi.bits == 0) {
+        mp_raise_ValueError("bits must be > 0");
     }
     if (args[ARG_firstbit].u_int != MICROPY_PY_MACHINE_SPI_MSB) {
         mp_raise_ValueError("firstbit must be MSB");
@@ -260,9 +270,9 @@ STATIC void mp_machine_soft_spi_init(mp_obj_base_t *self_in, size_t n_args, cons
     mp_soft_spi_ioctl(&self->spi, MP_SPI_IOCTL_INIT);
 }
 
-STATIC void mp_machine_soft_spi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) {
+STATIC void mp_machine_soft_spi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits) {
     mp_machine_soft_spi_obj_t *self = (mp_machine_soft_spi_obj_t*)self_in;
-    mp_soft_spi_transfer(&self->spi, len, src, dest);
+    mp_soft_spi_transfer(&self->spi, len, src, dest, bits);
 }
 
 const mp_machine_spi_p_t mp_machine_soft_spi_p = {

extmod/machine_spi.h

@@ -34,7 +34,7 @@
 typedef struct _mp_machine_spi_p_t {
     void (*init)(mp_obj_base_t *obj, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args);
     void (*deinit)(mp_obj_base_t *obj); // can be NULL
-    void (*transfer)(mp_obj_base_t *obj, size_t len, const uint8_t *src, uint8_t *dest);
+    void (*transfer)(mp_obj_base_t *obj, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits);
 } mp_machine_spi_p_t;
 
 typedef struct _mp_machine_soft_spi_obj_t {
@@ -47,10 +47,9 @@ extern const mp_obj_type_t mp_machine_soft_spi_type;
 extern const mp_obj_dict_t mp_machine_spi_locals_dict;
 
 mp_obj_t mp_machine_spi_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args);
-
 MP_DECLARE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_read_obj);
 MP_DECLARE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_readinto_obj);
-MP_DECLARE_CONST_FUN_OBJ_2(mp_machine_spi_write_obj);
-MP_DECLARE_CONST_FUN_OBJ_3(mp_machine_spi_write_readinto_obj);
+MP_DECLARE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_write_obj);
+MP_DECLARE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_write_readinto_obj);
 
 #endif // MICROPY_INCLUDED_EXTMOD_MACHINE_SPI_H

ports/esp32/machine_hw_spi.c

@@ -237,54 +237,68 @@ STATIC void machine_hw_spi_deinit(mp_obj_base_t *self_in) {
     }
 }
 
-STATIC void machine_hw_spi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) {
+STATIC void machine_hw_spi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits) {
     machine_hw_spi_obj_t *self = MP_OBJ_TO_PTR(self_in);
 
     if (self->state == MACHINE_HW_SPI_STATE_DEINIT) {
         mp_raise_msg(&mp_type_OSError, "transfer on deinitialized SPI");
         return;
     }
 
-    struct spi_transaction_t transaction = { 0 };
-
-    // Round to nearest whole set of bits
-    int bits_to_send = len * 8 / self->bits * self->bits;
+    bits = bits ? bits : self->bits;
+    bool wholeBytes = bits % 8 == 0;
+    if (bits == 0) {
+        mp_raise_ValueError("bits cannot be 0");
+    }
 
 
-    if (len <= 4) {
-        if (src != NULL) {
-            memcpy(&transaction.tx_data, src, len);
-        }
+    int bytesPerChunk = (bits + 7) / 8;
+    // round length down as needed (possibly to zero)
+    len = len / bytesPerChunk * bytesPerChunk;
+    if(len == 0) {
+        return;
+    }
 
-        transaction.flags = SPI_TRANS_USE_TXDATA | SPI_TRANS_USE_RXDATA;
-        transaction.length = bits_to_send;
-        spi_device_transmit(self->spi, &transaction);
+    struct spi_transaction_t transaction;
+    while(len) {
 
-        if (dest != NULL) {
-            memcpy(dest, &transaction.rx_data, len);
-        }
-    } else {
-        int offset = 0;
-        int bits_remaining = bits_to_send;
+        memset(&transaction, 0, sizeof(transaction));
+        if ((bits <= 32 && !wholeBytes) || len <= 4) {
+            if (src != NULL) {
+                memcpy(&transaction.tx_data, src, bytesPerChunk);
+            }
 
-        while (bits_remaining) {
-            memset(&transaction, 0, sizeof(transaction));
+            transaction.flags = SPI_TRANS_USE_TXDATA | SPI_TRANS_USE_RXDATA;
+            transaction.length = bits;
+            spi_device_transmit(self->spi, &transaction);
 
+            if (dest != NULL) {
+                memcpy(dest, &transaction.rx_data, bytesPerChunk);
+            }
+            src += bytesPerChunk;
+            len -= bytesPerChunk;
+        } else {
+            if (wholeBytes) {
+                transaction.length = len * 8 / bits * bits;
+            } else {
+                transaction.length = bits;
+            }
             transaction.length =
-                bits_remaining > MP_HW_SPI_MAX_XFER_BITS ? MP_HW_SPI_MAX_XFER_BITS : bits_remaining;
+                transaction.length > MP_HW_SPI_MAX_XFER_BITS
+                    ? MP_HW_SPI_MAX_XFER_BITS
+                    : transaction.length;
 
+            size_t bytesSent = (transaction.length + 7) / 8;
             if (src != NULL) {
-                transaction.tx_buffer = src + offset;
+                transaction.tx_buffer = src;
+                src += bytesSent;
             }
             if (dest != NULL) {
-                transaction.rx_buffer = dest + offset;
+                transaction.rx_buffer = dest;
+                dest += bytesSent;
             }
-
+            len -= bytesSent;
             spi_device_transmit(self->spi, &transaction);
-            bits_remaining -= transaction.length;
-
-            // doesn't need ceil(); loop ends when bits_remaining is 0
-            offset += transaction.length / 8;
         }
     }
 }

ports/esp8266/machine_hspi.c

@@ -48,7 +48,7 @@ typedef struct _machine_hspi_obj_t {
     uint8_t phase;
 } machine_hspi_obj_t;
 
-STATIC void machine_hspi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) {
+STATIC void machine_hspi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest, uint8_t bits) {
     (void)self_in;
 
     if (dest == NULL) {