Skip to content

Commit

Permalink
Fix AVR SPI parameter configuration, remove timeouts due to sync prot…
Browse files Browse the repository at this point in the history
…ocol. (qmk#8775)
  • Loading branch information
tzarc authored and kujawakk committed Apr 21, 2020
1 parent 0292119 commit 922dfae
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 84 deletions.
31 changes: 12 additions & 19 deletions docs/spi_driver.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ No special setup is required - just connect the `SS`, `SCK`, `MOSI` and `MISO` p
You may use more than one slave select pin, not just the `SS` pin. This is useful when you have multiple devices connected and need to communicate with them individually.
`SPI_SS_PIN` can be passed to `spi_start()` to refer to `SS`.

## ARM Configuration
## ChibiOS/ARM Configuration

ARM support for this driver is not ready yet. Check back later!

Expand All @@ -28,7 +28,7 @@ Initialize the SPI driver. This function must be called only once, before any of

---

### `void spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint8_t divisor)`
### `bool spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint16_t divisor)`

Start an SPI transaction.

Expand All @@ -48,44 +48,41 @@ Start an SPI transaction.
|`2` |Leading edge falling|Sample on leading edge |
|`3` |Leading edge falling|Sample on trailing edge|

- `uint8_t divisor`
- `uint16_t divisor`
The SPI clock divisor, will be rounded up to the nearest power of two. This number can be calculated by dividing the MCU's clock speed by the desired SPI clock speed. For example, an MCU running at 8 MHz wanting to talk to an SPI device at 4 MHz would set the divisor to `2`.

#### Return Value

`false` if the supplied parameters are invalid or the SPI peripheral is already in use, or `true`.

---

### `spi_status_t spi_write(uint8_t data, uint16_t timeout)`
### `spi_status_t spi_write(uint8_t data)`

Write a byte to the selected SPI device.

#### Arguments

- `uint8_t data`
The byte to write.
- `uint16_t timeout`
The amount of time to wait, in milliseconds, before timing out.

#### Return Value

`SPI_STATUS_TIMEOUT` if the timeout period elapses, or `SPI_STATUS_SUCCESS`.

---

### `spi_status_t spi_read(uint16_t timeout)`
### `spi_status_t spi_read(void)`

Read a byte from the selected SPI device.

#### Arguments

- `uint16_t timeout`
The amount of time to wait, in milliseconds, before timing out.

#### Return Value

`SPI_STATUS_TIMEOUT` if the timeout period elapses, or the byte read from the device.

---

### `spi_status_t spi_transmit(const uint8_t *data, uint16_t length, uint16_t timeout)`
### `spi_status_t spi_transmit(const uint8_t *data, uint16_t length)`

Send multiple bytes to the selected SPI device.

Expand All @@ -95,16 +92,14 @@ Send multiple bytes to the selected SPI device.
A pointer to the data to write from.
- `uint16_t length`
The number of bytes to write. Take care not to overrun the length of `data`.
- `uint16_t timeout`
The amount of time to wait, in milliseconds, before timing out.

#### Return Value

`SPI_STATUS_TIMEOUT` if the timeout period elapses, `SPI_STATUS_SUCCESS` on success, or `SPI_STATUS_ERROR` otherwise.

---

### `spi_status_t spi_receive(uint8_t *data, uint16_t length, uint16_t timeout)`
### `spi_status_t spi_receive(uint8_t *data, uint16_t length)`

Receive multiple bytes from the selected SPI device.

Expand All @@ -114,12 +109,10 @@ Receive multiple bytes from the selected SPI device.
A pointer to the buffer to read into.
- `uint16_t length`
The number of bytes to read. Take care not to overrun the length of `data`.
- `uint16_t timeout`
The amount of time to wait, in milliseconds, before timing out.

#### Return Value

`SPI_STATUS_TIMEOUT` if the timeout period elapses, `SPI_STATUS_SUCCESS` on success, or `SPI_STATUS_ERROR` otherwise.
`SPI_STATUS_TIMEOUT` if the internal transmission timeout period elapses, `SPI_STATUS_SUCCESS` on success, or `SPI_STATUS_ERROR` otherwise.

---

Expand Down
123 changes: 68 additions & 55 deletions drivers/avr/spi_master.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
# define SPI_MISO_PIN B4
#endif

#ifndef SPI_TIMEOUT
# define SPI_TIMEOUT 100
#endif

static pin_t currentSlavePin = NO_PIN;
static uint8_t currentSlaveConfig = 0;
static bool currentSlave2X = false;
Expand All @@ -47,100 +51,109 @@ void spi_init(void) {
SPCR = (_BV(SPE) | _BV(MSTR));
}

void spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint8_t divisor) {
if (currentSlavePin == NO_PIN && slavePin != NO_PIN) {
if (lsbFirst) {
currentSlaveConfig |= _BV(DORD);
}
bool spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint16_t divisor) {
if (currentSlavePin != NO_PIN || slavePin == NO_PIN) {
return false;
}

switch (mode) {
case 1:
currentSlaveConfig |= _BV(CPHA);
break;
case 2:
currentSlaveConfig |= _BV(CPOL);
break;
case 3:
currentSlaveConfig |= (_BV(CPOL) | _BV(CPHA));
break;
}
currentSlaveConfig = 0;

uint8_t roundedDivisor = 1;
while (roundedDivisor < divisor) {
roundedDivisor <<= 1;
}
if (lsbFirst) {
currentSlaveConfig |= _BV(DORD);
}

switch (roundedDivisor) {
case 16:
currentSlaveConfig |= _BV(SPR0);
break;
case 64:
currentSlaveConfig |= _BV(SPR1);
break;
case 128:
currentSlaveConfig |= (_BV(SPR1) | _BV(SPR0));
break;
case 2:
currentSlave2X = true;
break;
case 8:
currentSlave2X = true;
currentSlaveConfig |= _BV(SPR0);
break;
case 32:
currentSlave2X = true;
currentSlaveConfig |= _BV(SPR1);
break;
}
switch (mode) {
case 1:
currentSlaveConfig |= _BV(CPHA);
break;
case 2:
currentSlaveConfig |= _BV(CPOL);
break;
case 3:
currentSlaveConfig |= (_BV(CPOL) | _BV(CPHA));
break;
}

SPSR |= currentSlaveConfig;
currentSlavePin = slavePin;
setPinOutput(currentSlavePin);
writePinLow(currentSlavePin);
uint16_t roundedDivisor = 1;
while (roundedDivisor < divisor) {
roundedDivisor <<= 1;
}

switch (roundedDivisor) {
case 16:
currentSlaveConfig |= _BV(SPR0);
break;
case 64:
currentSlaveConfig |= _BV(SPR1);
break;
case 128:
currentSlaveConfig |= (_BV(SPR1) | _BV(SPR0));
break;
case 2:
currentSlave2X = true;
break;
case 8:
currentSlave2X = true;
currentSlaveConfig |= _BV(SPR0);
break;
case 32:
currentSlave2X = true;
currentSlaveConfig |= _BV(SPR1);
break;
}

SPCR |= currentSlaveConfig;
if (currentSlave2X) {
SPSR |= _BV(SPI2X);
}
currentSlavePin = slavePin;
setPinOutput(currentSlavePin);
writePinLow(currentSlavePin);

return true;
}

spi_status_t spi_write(uint8_t data, uint16_t timeout) {
spi_status_t spi_write(uint8_t data) {
SPDR = data;

uint16_t timeout_timer = timer_read();
while (!(SPSR & _BV(SPIF))) {
if ((timeout != SPI_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) {
if ((timer_read() - timeout_timer) >= SPI_TIMEOUT) {
return SPI_STATUS_TIMEOUT;
}
}

return SPDR;
}

spi_status_t spi_read(uint16_t timeout) {
spi_status_t spi_read() {
SPDR = 0x00; // Dummy

uint16_t timeout_timer = timer_read();
while (!(SPSR & _BV(SPIF))) {
if ((timeout != SPI_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) {
if ((timer_read() - timeout_timer) >= SPI_TIMEOUT) {
return SPI_STATUS_TIMEOUT;
}
}

return SPDR;
}

spi_status_t spi_transmit(const uint8_t *data, uint16_t length, uint16_t timeout) {
spi_status_t spi_transmit(const uint8_t *data, uint16_t length) {
spi_status_t status = SPI_STATUS_ERROR;

for (uint16_t i = 0; i < length; i++) {
status = spi_write(data[i], timeout);
status = spi_write(data[i]);
}

return status;
}

spi_status_t spi_receive(uint8_t *data, uint16_t length, uint16_t timeout) {
spi_status_t spi_receive(uint8_t *data, uint16_t length) {
spi_status_t status = SPI_STATUS_ERROR;

for (uint16_t i = 0; i < length; i++) {
status = spi_read(timeout);
status = spi_read();

if (status > 0) {
data[i] = status;
Expand All @@ -155,9 +168,9 @@ void spi_stop(void) {
setPinOutput(currentSlavePin);
writePinHigh(currentSlavePin);
currentSlavePin = NO_PIN;
SPSR &= ~(_BV(SPI2X));
SPCR &= ~(currentSlaveConfig);
currentSlaveConfig = 0;
SPSR = 0;
currentSlave2X = false;
}
}
10 changes: 5 additions & 5 deletions drivers/avr/spi_master.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ extern "C" {
#endif
void spi_init(void);

void spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint8_t divisor);
bool spi_start(pin_t slavePin, bool lsbFirst, uint8_t mode, uint16_t divisor);

spi_status_t spi_write(uint8_t data, uint16_t timeout);
spi_status_t spi_write(uint8_t data);

spi_status_t spi_read(uint16_t timeout);
spi_status_t spi_read(void);

spi_status_t spi_transmit(const uint8_t *data, uint16_t length, uint16_t timeout);
spi_status_t spi_transmit(const uint8_t *data, uint16_t length);

spi_status_t spi_receive(uint8_t *data, uint16_t length, uint16_t timeout);
spi_status_t spi_receive(uint8_t *data, uint16_t length);

void spi_stop(void);
#ifdef __cplusplus
Expand Down
10 changes: 5 additions & 5 deletions tmk_core/protocol/lufa/adafruit_ble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ static bool sdep_send_pkt(const struct sdep_msg *msg, uint16_t timeout) {
bool ready = false;

do {
ready = spi_write(msg->type, 100) != SdepSlaveNotReady;
ready = spi_write(msg->type) != SdepSlaveNotReady;
if (ready) {
break;
}
Expand All @@ -165,7 +165,7 @@ static bool sdep_send_pkt(const struct sdep_msg *msg, uint16_t timeout) {

if (ready) {
// Slave is ready; send the rest of the packet
spi_transmit(&msg->cmd_low, sizeof(*msg) - (1 + sizeof(msg->payload)) + msg->len, 100);
spi_transmit(&msg->cmd_low, sizeof(*msg) - (1 + sizeof(msg->payload)) + msg->len);
success = true;
}

Expand Down Expand Up @@ -205,7 +205,7 @@ static bool sdep_recv_pkt(struct sdep_msg *msg, uint16_t timeout) {

do {
// Read the command type, waiting for the data to be ready
msg->type = spi_read(100);
msg->type = spi_read();
if (msg->type == SdepSlaveNotReady || msg->type == SdepSlaveOverflow) {
// Release it and let it initialize
spi_stop();
Expand All @@ -215,11 +215,11 @@ static bool sdep_recv_pkt(struct sdep_msg *msg, uint16_t timeout) {
}

// Read the rest of the header
spi_receive(&msg->cmd_low, sizeof(*msg) - (1 + sizeof(msg->payload)), 100);
spi_receive(&msg->cmd_low, sizeof(*msg) - (1 + sizeof(msg->payload)));

// and get the payload if there is any
if (msg->len <= SdepMaxPayload) {
spi_receive(msg->payload, msg->len, 100);
spi_receive(msg->payload, msg->len);
}
success = true;
break;
Expand Down

0 comments on commit 922dfae

Please sign in to comment.