drivers: spi_xlnx_axi_quadspi: Reduce IRQ work
This driver could end up doing a great deal of work inside the ISR when
large SPI transfers were in use, which could cause significant IRQ
latency. For the normal, non-async SPI transfer case, use events to
signal the calling thread to complete the work rather than performing
FIFO transfers inside the ISR.
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
diff --git a/drivers/spi/Kconfig.xlnx b/drivers/spi/Kconfig.xlnx
index 5d3f454..4ce0b67 100644
--- a/drivers/spi/Kconfig.xlnx
+++ b/drivers/spi/Kconfig.xlnx
@@ -7,5 +7,6 @@
bool "Xilinx AXI Quad SPI driver"
default y
depends on DT_HAS_XLNX_XPS_SPI_2_00_A_ENABLED
+ select EVENTS
help
Enable Xilinx AXI Quad SPI v3.2 driver.
diff --git a/drivers/spi/spi_xlnx_axi_quadspi.c b/drivers/spi/spi_xlnx_axi_quadspi.c
index c42166c..4c551b6 100644
--- a/drivers/spi/spi_xlnx_axi_quadspi.c
+++ b/drivers/spi/spi_xlnx_axi_quadspi.c
@@ -11,6 +11,7 @@
#include <zephyr/sys/sys_io.h>
#include <zephyr/logging/log.h>
#include <zephyr/irq.h>
+#include <zephyr/kernel.h>
LOG_MODULE_REGISTER(xlnx_quadspi, CONFIG_SPI_LOG_LEVEL);
#include "spi_context.h"
@@ -94,6 +95,7 @@
struct xlnx_quadspi_data {
struct spi_context ctx;
+ struct k_event dtr_empty;
};
static inline uint32_t xlnx_quadspi_read32(const struct device *dev,
@@ -227,7 +229,7 @@
return 0;
}
-static void xlnx_quadspi_start_tx(const struct device *dev)
+static bool xlnx_quadspi_start_tx(const struct device *dev)
{
const struct xlnx_quadspi_config *config = dev->config;
struct xlnx_quadspi_data *data = dev->data;
@@ -237,6 +239,7 @@
uint32_t spisr;
uint32_t dtr = 0U;
uint32_t fifo_avail_words = config->fifo_size ? config->fifo_size : 1;
+ bool complete = false;
if (!spi_context_tx_on(ctx) && !spi_context_rx_on(ctx)) {
/* All done, de-assert slave select */
@@ -250,7 +253,8 @@
}
spi_context_complete(ctx, dev, 0);
- return;
+ complete = true;
+ return complete;
}
if (!IS_ENABLED(CONFIG_SPI_SLAVE) || !spi_context_is_slave(ctx)) {
@@ -318,6 +322,7 @@
SPICR_OFFSET);
spi_context_complete(ctx, dev, -ENOTSUP);
+ complete = true;
}
if (!IS_ENABLED(CONFIG_SPI_SLAVE) || !spi_context_is_slave(ctx)) {
@@ -325,6 +330,47 @@
spicr &= ~(SPICR_MASTER_XFER_INH);
xlnx_quadspi_write32(dev, spicr, SPICR_OFFSET);
}
+ return complete;
+}
+
+static void xlnx_quadspi_read_fifo(const struct device *dev)
+{
+ const struct xlnx_quadspi_config *config = dev->config;
+ struct xlnx_quadspi_data *data = dev->data;
+ struct spi_context *ctx = &data->ctx;
+ uint32_t spisr = xlnx_quadspi_read32(dev, SPISR_OFFSET);
+ /* RX FIFO occupancy register only exists if FIFO is implemented */
+ uint32_t rx_fifo_words = config->fifo_size ?
+ xlnx_quadspi_read32(dev, SPI_RX_FIFO_OCR_OFFSET) + 1 : 1;
+
+ /* Read RX data */
+ while (!(spisr & SPISR_RX_EMPTY)) {
+ uint32_t drr = xlnx_quadspi_read32(dev, SPI_DRR_OFFSET);
+
+ if (spi_context_rx_buf_on(ctx)) {
+ switch (config->num_xfer_bytes) {
+ case 1:
+ UNALIGNED_PUT(drr, (uint8_t *)ctx->rx_buf);
+ break;
+ case 2:
+ UNALIGNED_PUT(drr, (uint16_t *)ctx->rx_buf);
+ break;
+ case 4:
+ UNALIGNED_PUT(drr, (uint32_t *)ctx->rx_buf);
+ break;
+ default:
+ __ASSERT(0, "unsupported num_xfer_bytes");
+ }
+ }
+
+ spi_context_update_rx(ctx, config->num_xfer_bytes, 1);
+
+ if (--rx_fifo_words == 0) {
+ spisr = xlnx_quadspi_read32(dev, SPISR_OFFSET);
+ rx_fifo_words = config->fifo_size ?
+ xlnx_quadspi_read32(dev, SPI_RX_FIFO_OCR_OFFSET) + 1 : 1;
+ }
+ }
}
static int xlnx_quadspi_transceive(const struct device *dev,
@@ -352,7 +398,27 @@
xlnx_quadspi_cs_control(dev, true);
- xlnx_quadspi_start_tx(dev);
+ while (true) {
+ k_event_clear(&data->dtr_empty, 1);
+ bool complete = xlnx_quadspi_start_tx(dev);
+
+ if (complete || async) {
+ break;
+ }
+
+ /**
+ * 20ms should be long enough for 256 byte FIFO at any
+ * reasonable clock speed.
+ */
+ if (!k_event_wait(&data->dtr_empty, 1, false,
+ K_MSEC(20 + CONFIG_SPI_COMPLETION_TIMEOUT_TOLERANCE))) {
+ /* Timeout */
+ LOG_ERR("DTR empty timeout");
+ spi_context_complete(ctx, dev, -ETIMEDOUT);
+ break;
+ }
+ xlnx_quadspi_read_fifo(dev);
+ }
ret = spi_context_wait_for_completion(ctx);
out:
@@ -405,9 +471,7 @@
static void xlnx_quadspi_isr(const struct device *dev)
{
- const struct xlnx_quadspi_config *config = dev->config;
struct xlnx_quadspi_data *data = dev->data;
- struct spi_context *ctx = &data->ctx;
uint32_t ipisr;
/* Acknowledge interrupt */
@@ -415,46 +479,20 @@
xlnx_quadspi_write32(dev, ipisr, IPISR_OFFSET);
if (ipisr & IPIXR_DTR_EMPTY) {
- uint32_t spisr = xlnx_quadspi_read32(dev, SPISR_OFFSET);
- /* RX FIFO occupancy register only exists if FIFO is implemented */
- uint32_t rx_fifo_words = config->fifo_size ?
- xlnx_quadspi_read32(dev, SPI_RX_FIFO_OCR_OFFSET) + 1 : 1;
-
- /* Read RX data */
- while (!(spisr & SPISR_RX_EMPTY)) {
- uint32_t drr = xlnx_quadspi_read32(dev, SPI_DRR_OFFSET);
-
- if (spi_context_rx_buf_on(ctx)) {
- switch (config->num_xfer_bytes) {
- case 1:
- UNALIGNED_PUT(drr,
- (uint8_t *)ctx->rx_buf);
- break;
- case 2:
- UNALIGNED_PUT(drr,
- (uint16_t *)ctx->rx_buf);
- break;
- case 4:
- UNALIGNED_PUT(drr,
- (uint32_t *)ctx->rx_buf);
- break;
- default:
- __ASSERT(0,
- "unsupported num_xfer_bytes");
- }
- }
-
- spi_context_update_rx(ctx, config->num_xfer_bytes, 1);
-
- if (--rx_fifo_words == 0) {
- spisr = xlnx_quadspi_read32(dev, SPISR_OFFSET);
- rx_fifo_words = config->fifo_size ?
- xlnx_quadspi_read32(dev, SPI_RX_FIFO_OCR_OFFSET) + 1 : 1;
- }
+ /**
+ * For async mode, we need to read the RX FIFO and refill the TX FIFO
+ * if needed here.
+ * For sync mode, we do this in the caller's context to avoid doing too much
+ * work in the ISR, so just post the event.
+ */
+#ifdef CONFIG_SPI_ASYNC
+ if (ctx->asynchronous) {
+ xlnx_quadspi_read_fifo(dev);
+ xlnx_quadspi_start_tx(dev);
+ return;
}
-
- /* Start next TX */
- xlnx_quadspi_start_tx(dev);
+#endif
+ k_event_post(&data->dtr_empty, 1);
} else {
LOG_WRN("unhandled interrupt, ipisr = 0x%08x", ipisr);
}
@@ -514,6 +552,8 @@
const struct xlnx_quadspi_config *config = dev->config;
struct xlnx_quadspi_data *data = dev->data;
+ k_event_init(&data->dtr_empty);
+
/* Reset controller */
xlnx_quadspi_write32(dev, SRR_SOFTRESET_MAGIC, SRR_OFFSET);
