From 5bb28c7f10ebd1036302cf7ac0b24a7a233de2aa Mon Sep 17 00:00:00 2001 From: Damien George Date: Mon, 3 Oct 2016 12:39:31 +1100 Subject: [PATCH] extmod/machine_spi: Simplify SPI xfer function to only take one buf len. There is no need to take src_len and dest_len arguments. The case of reading-only with a single output byte (originally src_len=1, dest_len>1) is now handled by using the output buffer as the input buffer, and using memset to fill the output byte into this buffer. This simplifies the implementations of the spi_transfer protocol function. --- esp8266/modpybhspi.c | 32 ++++++++++---------------------- esp8266/modpybspi.c | 15 +++++---------- extmod/machine_spi.c | 25 ++++++++++--------------- extmod/machine_spi.h | 2 +- stmhal/spi.c | 11 ++--------- 5 files changed, 28 insertions(+), 57 deletions(-) diff --git a/esp8266/modpybhspi.c b/esp8266/modpybhspi.c index c1cd7f662..10a090269 100644 --- a/esp8266/modpybhspi.c +++ b/esp8266/modpybhspi.c @@ -50,23 +50,23 @@ typedef struct _pyb_hspi_obj_t { } pyb_hspi_obj_t; -STATIC void hspi_transfer(mp_obj_base_t *self_in, size_t src_len, const uint8_t *src_buf, size_t dest_len, uint8_t *dest_buf) { +STATIC void hspi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) { (void)self_in; - if (dest_len == 0) { + if (dest == NULL) { // fast case when we only need to write data size_t chunk_size = 1024; - size_t count = src_len / chunk_size; + size_t count = len / chunk_size; size_t i = 0; for (size_t j = 0; j < count; ++j) { for (size_t k = 0; k < chunk_size; ++k) { - spi_tx8fast(HSPI, src_buf[i]); + spi_tx8fast(HSPI, src[i]); ++i; } ets_loop_iter(); } - while (i < src_len) { - spi_tx8fast(HSPI, src_buf[i]); + while (i < len) { + spi_tx8fast(HSPI, src[i]); ++i; } } else { @@ -74,29 +74,17 @@ STATIC void hspi_transfer(mp_obj_base_t *self_in, size_t src_len, const uint8_t // Process data in chunks, let the pending tasks run in between size_t chunk_size = 1024; // TODO this should depend on baudrate - size_t count = dest_len / chunk_size; + size_t count = len / chunk_size; size_t i = 0; for (size_t j = 0; j < count; ++j) { for (size_t k = 0; k < chunk_size; ++k) { - uint32_t data_out; - if (src_len == 1) { - data_out = src_buf[0]; - } else { - data_out = src_buf[i]; - } - dest_buf[i] = spi_transaction(HSPI, 0, 0, 0, 0, 8, data_out, 8, 0); + dest[i] = spi_transaction(HSPI, 0, 0, 0, 0, 8, src[i], 8, 0); ++i; } ets_loop_iter(); } - while (i < dest_len) { - uint32_t data_out; - if (src_len == 1) { - data_out = src_buf[0]; - } else { - data_out = src_buf[i]; - } - dest_buf[i] = spi_transaction(HSPI, 0, 0, 0, 0, 8, data_out, 8, 0); + while (i < len) { + dest[i] = spi_transaction(HSPI, 0, 0, 0, 0, 8, src[i], 8, 0); ++i; } } diff --git a/esp8266/modpybspi.c b/esp8266/modpybspi.c index 4c4b843c5..b9650954f 100644 --- a/esp8266/modpybspi.c +++ b/esp8266/modpybspi.c @@ -47,17 +47,12 @@ typedef struct _pyb_spi_obj_t { mp_hal_pin_obj_t miso; } pyb_spi_obj_t; -STATIC void mp_hal_spi_transfer(mp_obj_base_t *self_in, size_t src_len, const uint8_t *src_buf, size_t dest_len, uint8_t *dest_buf) { +STATIC void mp_hal_spi_transfer(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) { pyb_spi_obj_t *self = (pyb_spi_obj_t*)self_in; // only MSB transfer is implemented uint32_t delay_half = 500000 / self->baudrate + 1; - for (size_t i = 0; i < src_len || i < dest_len; ++i) { - uint8_t data_out; - if (src_len == 1) { - data_out = src_buf[0]; - } else { - data_out = src_buf[i]; - } + for (size_t i = 0; i < len; ++i) { + uint8_t data_out = src[i]; uint8_t data_in = 0; for (int j = 0; j < 8; ++j, data_out <<= 1) { mp_hal_pin_write(self->mosi, (data_out >> 7) & 1); @@ -77,8 +72,8 @@ STATIC void mp_hal_spi_transfer(mp_obj_base_t *self_in, size_t src_len, const ui ets_delay_us(delay_half); } } - if (dest_len != 0) { - dest_buf[i] = data_in; + if (dest != NULL) { + dest[i] = data_in; } // make sure pending tasks have a chance to run ets_loop_iter(); diff --git a/extmod/machine_spi.c b/extmod/machine_spi.c index 6b6202a22..1c64cf511 100644 --- a/extmod/machine_spi.c +++ b/extmod/machine_spi.c @@ -25,26 +25,24 @@ */ #include +#include #include "py/runtime.h" #include "extmod/machine_spi.h" #if MICROPY_PY_MACHINE_SPI -STATIC void mp_machine_spi_transfer(mp_obj_t self, size_t slen, const uint8_t *src, size_t dlen, uint8_t *dest) { +STATIC void mp_machine_spi_transfer(mp_obj_t self, size_t len, const void *src, void *dest) { 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, slen, src, dlen, dest); + spi_p->transfer(s, len, src, dest); } STATIC mp_obj_t mp_machine_spi_read(size_t n_args, const mp_obj_t *args) { - uint8_t write_byte = 0; - if (n_args == 3) { - write_byte = mp_obj_get_int(args[2]); - } vstr_t vstr; vstr_init_len(&vstr, mp_obj_get_int(args[1])); - mp_machine_spi_transfer(args[0], 1, &write_byte, vstr.len, (uint8_t*)vstr.buf); + 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); 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); @@ -52,11 +50,8 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_read_obj, 2, 3, mp_machine_sp 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); - uint8_t write_byte = 0; - if (n_args == 3) { - write_byte = mp_obj_get_int(args[2]); - } - mp_machine_spi_transfer(args[0], 1, &write_byte, bufinfo.len, (uint8_t*)bufinfo.buf); + 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); return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_readinto_obj, 2, 3, mp_machine_spi_readinto); @@ -64,7 +59,7 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_machine_spi_readinto_obj, 2, 3, mp_machin STATIC mp_obj_t mp_machine_spi_write(mp_obj_t self, mp_obj_t wr_buf) { 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, 0, NULL); + mp_machine_spi_transfer(self, src.len, (const uint8_t*)src.buf, NULL); return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_2(mp_machine_spi_write_obj, mp_machine_spi_write); @@ -75,9 +70,9 @@ STATIC mp_obj_t mp_machine_spi_write_readinto(mp_obj_t self, mp_obj_t wr_buf, mp mp_buffer_info_t dest; mp_get_buffer_raise(rd_buf, &dest, MP_BUFFER_WRITE); if (src.len != dest.len) { - nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_ValueError, "buffers must be the same length")); + mp_raise_ValueError("buffers must be the same length"); } - mp_machine_spi_transfer(self, src.len, (const uint8_t*)src.buf, dest.len, (uint8_t*)dest.buf); + mp_machine_spi_transfer(self, src.len, src.buf, dest.buf); return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_3(mp_machine_spi_write_readinto_obj, mp_machine_spi_write_readinto); diff --git a/extmod/machine_spi.h b/extmod/machine_spi.h index 26d716fc1..35a911d91 100644 --- a/extmod/machine_spi.h +++ b/extmod/machine_spi.h @@ -31,7 +31,7 @@ // SPI protocol typedef struct _mp_machine_spi_p_t { - void (*transfer)(mp_obj_base_t *obj, size_t slen, const uint8_t *src, size_t dlen, uint8_t *dest); + void (*transfer)(mp_obj_base_t *obj, size_t len, const uint8_t *src, uint8_t *dest); } mp_machine_spi_p_t; MP_DECLARE_CONST_FUN_OBJ(mp_machine_spi_read_obj); diff --git a/stmhal/spi.c b/stmhal/spi.c index 85f170920..05b9b629c 100644 --- a/stmhal/spi.c +++ b/stmhal/spi.c @@ -401,15 +401,8 @@ STATIC void spi_transfer(mp_obj_base_t *self_in, size_t src_len, const uint8_t * } } -STATIC void spi_transfer_machine(mp_obj_base_t *self_in, size_t src_len, const uint8_t *src_buf, size_t dest_len, uint8_t *dest_buf) { - if (src_len == 1 && dest_len > 1) { - // this catches read and readinto - // copy the single output byte to the dest buffer and use that as source - memset(dest_buf, src_buf[0], dest_len); - src_len = dest_len; - src_buf = dest_buf; - } - spi_transfer(self_in, src_len, src_buf, dest_len, dest_buf, 100); +STATIC void spi_transfer_machine(mp_obj_base_t *self_in, size_t len, const uint8_t *src, uint8_t *dest) { + spi_transfer(self_in, len, src, dest == NULL ? 0 : len, dest, 100); } /******************************************************************************/