Commit abb4b46c authored by David Lechner's avatar David Lechner Committed by Mark Brown

spi: axi-spi-engine: move msg finalization out of irq handler

As a general principal, it is best to do as little as possible in an
interrupt handler. This patch reworks the AXI SPI Engine driver to move
timer_delete_sync() and spi_finalize_current_message() out of the
interrupt handler. Instead, spi_finalize_current_message() is moved to
the transfer_one_message function (similar to nearly all other SPI
controllers). A completion is now used to wait for the sync interrupt
that indicates that the message is complete. The watchdog timer is no
longer needed since we can use the wait_for_completion_timeout()
function to wait for the message to complete with the same effect.

As a bonus, these changes also improve throughput of the SPI controller.
For example, this was tested on a ZynqMP with a 80MHz SCLK reading 4
byte samples from an ADC. The max measured throughput increased from
26k to 28k samples per second.
Reviewed-by: default avatarNuno Sa <nuno.sa@analog.com>
Signed-off-by: default avatarDavid Lechner <dlechner@baylibre.com>
Link: https://lore.kernel.org/r/20240207-axi-spi-engine-round-2-1-v2-2-40c0b4e85352@baylibre.comSigned-off-by: default avatarMark Brown <broonie@kernel.org>
parent 531860e1
......@@ -6,6 +6,7 @@
*/
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/fpga/adi-axi-common.h>
#include <linux/interrupt.h>
#include <linux/io.h>
......@@ -13,7 +14,6 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/spi/spi.h>
#include <linux/timer.h>
#define SPI_ENGINE_REG_RESET 0x40
......@@ -109,9 +109,7 @@ struct spi_engine {
spinlock_t lock;
void __iomem *base;
struct timer_list watchdog_timer;
struct spi_controller *controller;
struct completion msg_complete;
unsigned int int_enable;
};
......@@ -483,11 +481,9 @@ static irqreturn_t spi_engine_irq(int irq, void *devid)
if (pending & SPI_ENGINE_INT_SYNC && msg) {
if (completed_id == AXI_SPI_ENGINE_CUR_MSG_SYNC_ID) {
if (timer_delete_sync(&spi_engine->watchdog_timer)) {
msg->status = 0;
msg->actual_length = msg->frame_length;
spi_finalize_current_message(host);
}
msg->status = 0;
msg->actual_length = msg->frame_length;
complete(&spi_engine->msg_complete);
disable_int |= SPI_ENGINE_INT_SYNC;
}
}
......@@ -558,7 +554,7 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
unsigned int int_enable = 0;
unsigned long flags;
mod_timer(&spi_engine->watchdog_timer, jiffies + msecs_to_jiffies(5000));
reinit_completion(&spi_engine->msg_complete);
spin_lock_irqsave(&spi_engine->lock, flags);
......@@ -580,21 +576,16 @@ static int spi_engine_transfer_one_message(struct spi_controller *host,
spi_engine->int_enable = int_enable;
spin_unlock_irqrestore(&spi_engine->lock, flags);
return 0;
}
static void spi_engine_timeout(struct timer_list *timer)
{
struct spi_engine *spi_engine = from_timer(spi_engine, timer, watchdog_timer);
struct spi_controller *host = spi_engine->controller;
if (WARN_ON(!host->cur_msg))
return;
if (!wait_for_completion_timeout(&spi_engine->msg_complete,
msecs_to_jiffies(5000))) {
dev_err(&host->dev,
"Timeout occurred while waiting for transfer to complete. Hardware is probably broken.\n");
msg->status = -ETIMEDOUT;
}
dev_err(&host->dev,
"Timeout occurred while waiting for transfer to complete. Hardware is probably broken.\n");
host->cur_msg->status = -ETIMEDOUT;
spi_finalize_current_message(host);
return msg->status;
}
static void spi_engine_release_hw(void *p)
......@@ -625,8 +616,7 @@ static int spi_engine_probe(struct platform_device *pdev)
spi_engine = spi_controller_get_devdata(host);
spin_lock_init(&spi_engine->lock);
timer_setup(&spi_engine->watchdog_timer, spi_engine_timeout, TIMER_IRQSAFE);
spi_engine->controller = host;
init_completion(&spi_engine->msg_complete);
spi_engine->clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
if (IS_ERR(spi_engine->clk))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment