Skip to content

fix read for case (bits <= 32 && !wholeBytes) || len <= 4)) #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mirko wants to merge 2 commits into
base: fix-5225-upstream
Choose a base branch
from
Open

fix read for case (bits <= 32 && !wholeBytes) || len <= 4)) #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
494ecaf
*/*spi.c
Feb 21, 2020
5c2230c
fix read for case `(bits <= 32 && !wholeBytes) || len <= 4))`
Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 39 additions & 11 deletions drivers/bus/softspi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions drivers/bus/spi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
14 changes: 7 additions & 7 deletions drivers/memory/spiflash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand Down
46 changes: 28 additions & 18 deletions extmod/machine_spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) },
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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 = {
Expand Down
7 changes: 3 additions & 4 deletions extmod/machine_spi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
71 changes: 43 additions & 28 deletions ports/esp32/machine_hw_spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,54 +237,69 @@ 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);
dest += 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;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ports/esp8266/machine_hspi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading