Skip to content

Commit

Permalink
Merge branch 'feature/no_return_Q_from_isr_for_slave' into 'master'
Browse files Browse the repository at this point in the history 
spi_slave: add new flag SPI_SLAVE_NO_RETURN_RESULT to bypass return descriptor from ISR

Closes IDF-5971

See merge request espressif/esp-idf!20164
  • Loading branch information
@wanckl
wanckl committed Sep 23, 2022
2 parents 4cc763f + ee6d3de commit e352382
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 11 deletions.
1 change: 1 addition & 0 deletions components/driver/include/driver/spi_master.h
View file
Expand Up @@ -224,6 +224,7 @@ esp_err_t spi_device_queue_trans(spi_device_handle_t handle, spi_transaction_t *
out.
* @return
* - ESP_ERR_INVALID_ARG if parameter is invalid
* - ESP_ERR_NOT_SUPPORTED if flag `SPI_DEVICE_NO_RETURN_RESULT` is set
* - ESP_ERR_TIMEOUT if there was no completed transaction before ticks_to_wait expired
* - ESP_OK on success
*/
Expand Down
3 changes: 2 additions & 1 deletion components/driver/include/driver/spi_slave.h
View file
Expand Up @@ -23,7 +23,7 @@ extern "C"
#define SPI_SLAVE_TXBIT_LSBFIRST (1<<0) ///< Transmit command/address/data LSB first instead of the default MSB first
#define SPI_SLAVE_RXBIT_LSBFIRST (1<<1) ///< Receive data LSB first instead of the default MSB first
#define SPI_SLAVE_BIT_LSBFIRST (SPI_SLAVE_TXBIT_LSBFIRST|SPI_SLAVE_RXBIT_LSBFIRST) ///< Transmit and receive LSB first

#define SPI_SLAVE_NO_RETURN_RESULT (1<<2) ///< Don't return the descriptor to the host on completion (use `post_trans_cb` to notify instead)

typedef struct spi_slave_transaction_t spi_slave_transaction_t;
typedef void(*slave_transaction_cb_t)(spi_slave_transaction_t *trans);
Expand Down Expand Up @@ -158,6 +158,7 @@ esp_err_t spi_slave_queue_trans(spi_host_device_t host, const spi_slave_transact
* out.
* @return
* - ESP_ERR_INVALID_ARG if parameter is invalid
* - ESP_ERR_NOT_SUPPORTED if flag `SPI_SLAVE_NO_RETURN_RESULT` is set
* - ESP_OK on success
*/
esp_err_t spi_slave_get_trans_result(spi_host_device_t host, spi_slave_transaction_t **trans_desc, TickType_t ticks_to_wait);
Expand Down
21 changes: 16 additions & 5 deletions components/driver/spi_master.c
View file
Expand Up @@ -381,10 +381,16 @@ esp_err_t spi_bus_add_device(spi_host_device_t host_id, const spi_device_interfa

//Allocate queues, set defaults
dev->trans_queue = xQueueCreate(dev_config->queue_size, sizeof(spi_trans_priv_t));
dev->ret_queue = xQueueCreate(dev_config->queue_size, sizeof(spi_trans_priv_t));
if (!dev->trans_queue || !dev->ret_queue) {
if (!dev->trans_queue) {
goto nomem;
}
//ret_queue nolonger needed if use flag SPI_DEVICE_NO_RETURN_RESULT
if (!(dev_config->flags & SPI_DEVICE_NO_RETURN_RESULT)) {
dev->ret_queue = xQueueCreate(dev_config->queue_size, sizeof(spi_trans_priv_t));
if (!dev->ret_queue) {
goto nomem;
}
}

//We want to save a copy of the dev config in the dev struct.
memcpy(&dev->cfg, dev_config, sizeof(spi_device_interface_config_t));
Expand Down Expand Up @@ -445,15 +451,17 @@ esp_err_t spi_bus_remove_device(spi_device_handle_t handle)
//catch design errors and aren't meant to be triggered during normal operation.
SPI_CHECK(uxQueueMessagesWaiting(handle->trans_queue)==0, "Have unfinished transactions", ESP_ERR_INVALID_STATE);
SPI_CHECK(handle->host->cur_cs == DEV_NUM_MAX || handle->host->device[handle->host->cur_cs] != handle, "Have unfinished transactions", ESP_ERR_INVALID_STATE);
SPI_CHECK(uxQueueMessagesWaiting(handle->ret_queue)==0, "Have unfinished transactions", ESP_ERR_INVALID_STATE);
if (handle->ret_queue) {
SPI_CHECK(uxQueueMessagesWaiting(handle->ret_queue)==0, "Have unfinished transactions", ESP_ERR_INVALID_STATE);
}

//return
int spics_io_num = handle->cfg.spics_io_num;
if (spics_io_num >= 0) spicommon_cs_free_io(spics_io_num);

//Kill queues
vQueueDelete(handle->trans_queue);
vQueueDelete(handle->ret_queue);
if (handle->trans_queue) vQueueDelete(handle->trans_queue);
if (handle->ret_queue) vQueueDelete(handle->ret_queue);
spi_bus_lock_unregister_dev(handle->dev_lock);

assert(handle->host->device[handle->id] == handle);
Expand Down Expand Up @@ -852,6 +860,9 @@ esp_err_t SPI_MASTER_ATTR spi_device_get_trans_result(spi_device_handle_t handle
spi_trans_priv_t trans_buf;
SPI_CHECK(handle!=NULL, "invalid dev handle", ESP_ERR_INVALID_ARG);

//if SPI_DEVICE_NO_RETURN_RESULT is set, ret_queue will always be empty
SPI_CHECK(!(handle->cfg.flags & SPI_DEVICE_NO_RETURN_RESULT), "API not Supported!", ESP_ERR_NOT_SUPPORTED);

//use the interrupt, block until return
r=xQueueReceive(handle->ret_queue, (void*)&trans_buf, ticks_to_wait);
if (!r) {
Expand Down
25 changes: 20 additions & 5 deletions components/driver/spi_slave.c
View file
Expand Up @@ -125,6 +125,11 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b
#endif
SPI_CHECK(slave_config->spics_io_num < 0 || GPIO_IS_VALID_GPIO(slave_config->spics_io_num), "spics pin invalid", ESP_ERR_INVALID_ARG);

//Check post_trans_cb status when `SPI_SLAVE_NO_RETURN_RESULT` flag is set.
if(slave_config->flags & SPI_SLAVE_NO_RETURN_RESULT) {
SPI_CHECK(slave_config->post_trans_cb != NULL, "use feature flag 'SPI_SLAVE_NO_RETURN_RESULT' but no post_trans_cb function sets", ESP_ERR_INVALID_ARG);
}

spi_chan_claimed=spicommon_periph_claim(host, "spi slave");
SPI_CHECK(spi_chan_claimed, "host already in use", ESP_ERR_INVALID_STATE);

Expand Down Expand Up @@ -183,11 +188,17 @@ esp_err_t spi_slave_initialize(spi_host_device_t host, const spi_bus_config_t *b

//Create queues
spihost[host]->trans_queue = xQueueCreate(slave_config->queue_size, sizeof(spi_slave_transaction_t *));
spihost[host]->ret_queue = xQueueCreate(slave_config->queue_size, sizeof(spi_slave_transaction_t *));
if (!spihost[host]->trans_queue || !spihost[host]->ret_queue) {
if (!spihost[host]->trans_queue) {
ret = ESP_ERR_NO_MEM;
goto cleanup;
}
if(!(slave_config->flags & SPI_SLAVE_NO_RETURN_RESULT)) {
spihost[host]->ret_queue = xQueueCreate(slave_config->queue_size, sizeof(spi_slave_transaction_t *));
if (!spihost[host]->ret_queue) {
ret = ESP_ERR_NO_MEM;
goto cleanup;
}
}

int flags = bus_config->intr_flags | ESP_INTR_FLAG_INTRDISABLED;
err = esp_intr_alloc(spicommon_irqsource_for_host(host), flags, spi_intr, (void *)spihost[host], &spihost[host]->intr);
Expand Down Expand Up @@ -298,6 +309,9 @@ esp_err_t SPI_SLAVE_ATTR spi_slave_get_trans_result(spi_host_device_t host, spi_
BaseType_t r;
SPI_CHECK(is_valid_host(host), "invalid host", ESP_ERR_INVALID_ARG);
SPI_CHECK(spihost[host], "host not slave", ESP_ERR_INVALID_ARG);
//if SPI_SLAVE_NO_RETURN_RESULT is set, ret_queue will always be empty
SPI_CHECK(!(spihost[host]->cfg.flags & SPI_SLAVE_NO_RETURN_RESULT), "API not Supported!", ESP_ERR_NOT_SUPPORTED);

r = xQueueReceive(spihost[host]->ret_queue, (void *)trans_desc, ticks_to_wait);
if (!r) return ESP_ERR_TIMEOUT;
return ESP_OK;
Expand Down Expand Up @@ -349,9 +363,10 @@ static void SPI_SLAVE_ISR_ATTR spi_intr(void *arg)
spicommon_dmaworkaround_req_reset(host->tx_dma_chan, spi_slave_restart_after_dmareset, host);
}
if (host->cfg.post_trans_cb) host->cfg.post_trans_cb(host->cur_trans);
//Okay, transaction is done.
//Return transaction descriptor.
xQueueSendFromISR(host->ret_queue, &host->cur_trans, &do_yield);

if(!(host->cfg.flags & SPI_SLAVE_NO_RETURN_RESULT)) {
xQueueSendFromISR(host->ret_queue, &host->cur_trans, &do_yield);
}
host->cur_trans = NULL;
}
if (use_dma) {
Expand Down

0 comments on commit e352382

Please sign in to comment.