[PATCH 2/2] spi: spl022: switch to use default spi_transfer_one_message()
From: Nam Cao
Date: Wed Nov 29 2023 - 08:32:34 EST
Except for polling mode, this driver's transfer_one_message() makes use
of interrupt handler and tasklet. This is problematic because
spi_transfer_delay_exec(), who may sleep, is called in interrupt handler
and tasklet. This causes the following warning:
BUG: sleeping function called from invalid context at drivers/spi/spi.c:1428
Switch to use the default spi_transfer_one_message() instead, which
calls spi_transfer_delay_exec() appropriately.
Signed-off-by: Nam Cao <namcao@xxxxxxxxxxxxx>
---
Tested with polling mode and interrupt mode, NOT with DMA mode.
Support with testing very appreciated!
drivers/spi/spi-pl022.c | 372 +++++++---------------------------------
1 file changed, 66 insertions(+), 306 deletions(-)
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index d1b6110b38fc..1e3bd6f3303a 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -338,7 +338,6 @@ struct vendor_data {
* @clk: outgoing clock "SPICLK" for the SPI bus
* @host: SPI framework hookup
* @host_info: controller-specific data from machine setup
- * @pump_transfers: Tasklet used in Interrupt Transfer mode
* @cur_msg: Pointer to current spi_message being processed
* @cur_transfer: Pointer to current spi_transfer
* @cur_chip: pointer to current clients chip(assigned from controller_state)
@@ -372,9 +371,6 @@ struct pl022 {
struct clk *clk;
struct spi_controller *host;
struct pl022_ssp_controller *host_info;
- /* Message per-transfer pump */
- struct tasklet_struct pump_transfers;
- struct spi_message *cur_msg;
struct spi_transfer *cur_transfer;
struct chip_data *cur_chip;
bool next_msg_cs_active;
@@ -437,93 +433,23 @@ struct chip_data {
* (vendor extension). Each of the 5 LSB in the register controls one chip
* select signal.
*/
-static void internal_cs_control(struct pl022 *pl022, u32 command)
+static void internal_cs_control(struct pl022 *pl022, bool enable)
{
u32 tmp;
tmp = readw(SSP_CSR(pl022->virtbase));
- if (command == SSP_CHIP_SELECT)
+ if (enable)
tmp &= ~BIT(pl022->cur_cs);
else
tmp |= BIT(pl022->cur_cs);
writew(tmp, SSP_CSR(pl022->virtbase));
}
-static void pl022_cs_control(struct pl022 *pl022, u32 command)
+static void pl022_cs_control(struct spi_device *spi, bool enable)
{
+ struct pl022 *pl022 = spi_controller_get_devdata(spi->controller);
if (pl022->vendor->internal_cs_ctrl)
- internal_cs_control(pl022, command);
- else if (pl022->cur_gpiod)
- /*
- * This needs to be inverted since with GPIOLIB in
- * control, the inversion will be handled by
- * GPIOLIB's active low handling. The "command"
- * passed into this function will be SSP_CHIP_SELECT
- * which is enum:ed to 0, so we need the inverse
- * (1) to activate chip select.
- */
- gpiod_set_value(pl022->cur_gpiod, !command);
-}
-
-/**
- * giveback - current spi_message is over, schedule next message and call
- * callback of this message. Assumes that caller already
- * set message->status; dma and pio irqs are blocked
- * @pl022: SSP driver private data structure
- */
-static void giveback(struct pl022 *pl022)
-{
- struct spi_transfer *last_transfer;
- pl022->next_msg_cs_active = false;
-
- last_transfer = list_last_entry(&pl022->cur_msg->transfers,
- struct spi_transfer, transfer_list);
-
- /* Delay if requested before any change in chip select */
- /*
- * FIXME: This runs in interrupt context.
- * Is this really smart?
- */
- spi_transfer_delay_exec(last_transfer);
-
- if (!last_transfer->cs_change) {
- struct spi_message *next_msg;
-
- /*
- * cs_change was not set. We can keep the chip select
- * enabled if there is message in the queue and it is
- * for the same spi device.
- *
- * We cannot postpone this until pump_messages, because
- * after calling msg->complete (below) the driver that
- * sent the current message could be unloaded, which
- * could invalidate the cs_control() callback...
- */
- /* get a pointer to the next message, if any */
- next_msg = spi_get_next_queued_message(pl022->host);
-
- /*
- * see if the next and current messages point
- * to the same spi device.
- */
- if (next_msg && next_msg->spi != pl022->cur_msg->spi)
- next_msg = NULL;
- if (!next_msg || pl022->cur_msg->state == STATE_ERROR)
- pl022_cs_control(pl022, SSP_CHIP_DESELECT);
- else
- pl022->next_msg_cs_active = true;
-
- }
-
- pl022->cur_msg = NULL;
- pl022->cur_transfer = NULL;
- pl022->cur_chip = NULL;
-
- /* disable the SPI/SSP operation */
- writew((readw(SSP_CR1(pl022->virtbase)) &
- (~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase));
-
- spi_finalize_current_message(pl022->host);
+ internal_cs_control(pl022, enable);
}
/**
@@ -757,30 +683,6 @@ static void readwriter(struct pl022 *pl022)
*/
}
-/**
- * next_transfer - Move to the Next transfer in the current spi message
- * @pl022: SSP driver private data structure
- *
- * This function moves though the linked list of spi transfers in the
- * current spi message and returns with the state of current spi
- * message i.e whether its last transfer is done(STATE_DONE) or
- * Next transfer is ready(STATE_RUNNING)
- */
-static void *next_transfer(struct pl022 *pl022)
-{
- struct spi_message *msg = pl022->cur_msg;
- struct spi_transfer *trans = pl022->cur_transfer;
-
- /* Move to next transfer */
- if (trans->transfer_list.next != &msg->transfers) {
- pl022->cur_transfer =
- list_entry(trans->transfer_list.next,
- struct spi_transfer, transfer_list);
- return STATE_RUNNING;
- }
- return STATE_DONE;
-}
-
/*
* This DMA functionality is only compiled in if we have
* access to the generic DMA devices/DMA engine.
@@ -800,7 +702,6 @@ static void unmap_free_dma_scatter(struct pl022 *pl022)
static void dma_callback(void *data)
{
struct pl022 *pl022 = data;
- struct spi_message *msg = pl022->cur_msg;
BUG_ON(!pl022->sgt_rx.sgl);
@@ -845,13 +746,7 @@ static void dma_callback(void *data)
unmap_free_dma_scatter(pl022);
- /* Update total bytes transferred */
- msg->actual_length += pl022->cur_transfer->len;
- /* Move to next transfer */
- msg->state = next_transfer(pl022);
- if (msg->state != STATE_DONE && pl022->cur_transfer->cs_change)
- pl022_cs_control(pl022, SSP_CHIP_DESELECT);
- tasklet_schedule(&pl022->pump_transfers);
+ spi_finalize_current_transfer(pl022->host);
}
static void setup_dma_scatter(struct pl022 *pl022,
@@ -1189,6 +1084,9 @@ static int pl022_dma_autoprobe(struct pl022 *pl022)
static void terminate_dma(struct pl022 *pl022)
{
+ if (!pl022->dma_running)
+ return;
+
struct dma_chan *rxchan = pl022->dma_rx_channel;
struct dma_chan *txchan = pl022->dma_tx_channel;
@@ -1200,8 +1098,7 @@ static void terminate_dma(struct pl022 *pl022)
static void pl022_dma_remove(struct pl022 *pl022)
{
- if (pl022->dma_running)
- terminate_dma(pl022);
+ terminate_dma(pl022);
if (pl022->dma_tx_channel)
dma_release_channel(pl022->dma_tx_channel);
if (pl022->dma_rx_channel)
@@ -1225,6 +1122,10 @@ static inline int pl022_dma_probe(struct pl022 *pl022)
return 0;
}
+static inline void terminate_dma(struct pl022 *pl022)
+{
+}
+
static inline void pl022_dma_remove(struct pl022 *pl022)
{
}
@@ -1246,16 +1147,7 @@ static inline void pl022_dma_remove(struct pl022 *pl022)
static irqreturn_t pl022_interrupt_handler(int irq, void *dev_id)
{
struct pl022 *pl022 = dev_id;
- struct spi_message *msg = pl022->cur_msg;
u16 irq_status = 0;
-
- if (unlikely(!msg)) {
- dev_err(&pl022->adev->dev,
- "bad message state in interrupt handler");
- /* Never fail */
- return IRQ_HANDLED;
- }
-
/* Read the Interrupt Status Register */
irq_status = readw(SSP_MIS(pl022->virtbase));
@@ -1287,10 +1179,8 @@ static irqreturn_t pl022_interrupt_handler(int irq, void *dev_id)
writew(CLEAR_ALL_INTERRUPTS, SSP_ICR(pl022->virtbase));
writew((readw(SSP_CR1(pl022->virtbase)) &
(~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase));
- msg->state = STATE_ERROR;
-
- /* Schedule message queue handler */
- tasklet_schedule(&pl022->pump_transfers);
+ pl022->cur_transfer->error |= SPI_TRANS_FAIL_IO;
+ spi_finalize_current_transfer(pl022->host);
return IRQ_HANDLED;
}
@@ -1318,13 +1208,7 @@ static irqreturn_t pl022_interrupt_handler(int irq, void *dev_id)
"number of bytes on a 16bit bus?)\n",
(u32) (pl022->rx - pl022->rx_end));
}
- /* Update total bytes transferred */
- msg->actual_length += pl022->cur_transfer->len;
- /* Move to next transfer */
- msg->state = next_transfer(pl022);
- if (msg->state != STATE_DONE && pl022->cur_transfer->cs_change)
- pl022_cs_control(pl022, SSP_CHIP_DESELECT);
- tasklet_schedule(&pl022->pump_transfers);
+ spi_finalize_current_transfer(pl022->host);
return IRQ_HANDLED;
}
@@ -1361,98 +1245,20 @@ static int set_up_next_transfer(struct pl022 *pl022,
return 0;
}
-/**
- * pump_transfers - Tasklet function which schedules next transfer
- * when running in interrupt or DMA transfer mode.
- * @data: SSP driver private data structure
- *
- */
-static void pump_transfers(unsigned long data)
+static int do_interrupt_dma_transfer(struct pl022 *pl022)
{
- struct pl022 *pl022 = (struct pl022 *) data;
- struct spi_message *message = NULL;
- struct spi_transfer *transfer = NULL;
- struct spi_transfer *previous = NULL;
-
- /* Get current state information */
- message = pl022->cur_msg;
- transfer = pl022->cur_transfer;
-
- /* Handle for abort */
- if (message->state == STATE_ERROR) {
- message->status = -EIO;
- giveback(pl022);
- return;
- }
-
- /* Handle end of message */
- if (message->state == STATE_DONE) {
- message->status = 0;
- giveback(pl022);
- return;
- }
-
- /* Delay if requested at end of transfer before CS change */
- if (message->state == STATE_RUNNING) {
- previous = list_entry(transfer->transfer_list.prev,
- struct spi_transfer,
- transfer_list);
- /*
- * FIXME: This runs in interrupt context.
- * Is this really smart?
- */
- spi_transfer_delay_exec(previous);
-
- /* Reselect chip select only if cs_change was requested */
- if (previous->cs_change)
- pl022_cs_control(pl022, SSP_CHIP_SELECT);
- } else {
- /* STATE_START */
- message->state = STATE_RUNNING;
- }
-
- if (set_up_next_transfer(pl022, transfer)) {
- message->state = STATE_ERROR;
- message->status = -EIO;
- giveback(pl022);
- return;
- }
- /* Flush the FIFOs and let's go! */
- flush(pl022);
-
- if (pl022->cur_chip->enable_dma) {
- if (configure_dma(pl022)) {
- dev_dbg(&pl022->adev->dev,
- "configuration of DMA failed, fall back to interrupt mode\n");
- goto err_config_dma;
- }
- return;
- }
-
-err_config_dma:
- /* enable all interrupts except RX */
- writew(ENABLE_ALL_INTERRUPTS & ~SSP_IMSC_MASK_RXIM, SSP_IMSC(pl022->virtbase));
-}
+ int ret;
-static void do_interrupt_dma_transfer(struct pl022 *pl022)
-{
/*
* Default is to enable all interrupts except RX -
* this will be enabled once TX is complete
*/
u32 irqflags = (u32)(ENABLE_ALL_INTERRUPTS & ~SSP_IMSC_MASK_RXIM);
- /* Enable target chip, if not already active */
- if (!pl022->next_msg_cs_active)
- pl022_cs_control(pl022, SSP_CHIP_SELECT);
+ ret = set_up_next_transfer(pl022, pl022->cur_transfer);
+ if (ret)
+ return ret;
- if (set_up_next_transfer(pl022, pl022->cur_transfer)) {
- /* Error path */
- pl022->cur_msg->state = STATE_ERROR;
- pl022->cur_msg->status = -EIO;
- giveback(pl022);
- return;
- }
/* If we're using DMA, set up DMA here */
if (pl022->cur_chip->enable_dma) {
/* Configure DMA transfer */
@@ -1469,6 +1275,7 @@ static void do_interrupt_dma_transfer(struct pl022 *pl022)
writew((readw(SSP_CR1(pl022->virtbase)) | SSP_CR1_MASK_SSE),
SSP_CR1(pl022->virtbase));
writew(irqflags, SSP_IMSC(pl022->virtbase));
+ return 1;
}
static void print_current_status(struct pl022 *pl022)
@@ -1495,111 +1302,67 @@ static void print_current_status(struct pl022 *pl022)
}
-static void do_polling_transfer(struct pl022 *pl022)
+static int do_polling_transfer(struct pl022 *pl022)
{
- struct spi_message *message = NULL;
- struct spi_transfer *transfer = NULL;
- struct spi_transfer *previous = NULL;
+ int ret;
unsigned long time, timeout;
- message = pl022->cur_msg;
-
- while (message->state != STATE_DONE) {
- /* Handle for abort */
- if (message->state == STATE_ERROR)
- break;
- transfer = pl022->cur_transfer;
-
- /* Delay if requested at end of transfer */
- if (message->state == STATE_RUNNING) {
- previous =
- list_entry(transfer->transfer_list.prev,
- struct spi_transfer, transfer_list);
- spi_transfer_delay_exec(previous);
- if (previous->cs_change)
- pl022_cs_control(pl022, SSP_CHIP_SELECT);
- } else {
- /* STATE_START */
- message->state = STATE_RUNNING;
- if (!pl022->next_msg_cs_active)
- pl022_cs_control(pl022, SSP_CHIP_SELECT);
- }
-
- /* Configuration Changing Per Transfer */
- if (set_up_next_transfer(pl022, transfer)) {
- /* Error path */
- message->state = STATE_ERROR;
- break;
- }
- /* Flush FIFOs and enable SSP */
- flush(pl022);
- writew((readw(SSP_CR1(pl022->virtbase)) | SSP_CR1_MASK_SSE),
- SSP_CR1(pl022->virtbase));
-
- dev_dbg(&pl022->adev->dev, "polling transfer ongoing ...\n");
-
- timeout = jiffies + msecs_to_jiffies(SPI_POLLING_TIMEOUT);
- while (pl022->tx < pl022->tx_end || pl022->rx < pl022->rx_end) {
- time = jiffies;
- readwriter(pl022);
- if (time_after(time, timeout)) {
- dev_warn(&pl022->adev->dev,
- "%s: timeout!\n", __func__);
- message->state = STATE_TIMEOUT;
- print_current_status(pl022);
- goto out;
- }
- cpu_relax();
+ /* Configuration Changing Per Transfer */
+ ret = set_up_next_transfer(pl022, pl022->cur_transfer);
+ if (ret)
+ return ret;
+ /* Flush FIFOs and enable SSP */
+ flush(pl022);
+ writew((readw(SSP_CR1(pl022->virtbase)) | SSP_CR1_MASK_SSE),
+ SSP_CR1(pl022->virtbase));
+
+ dev_dbg(&pl022->adev->dev, "polling transfer ongoing ...\n");
+
+ timeout = jiffies + msecs_to_jiffies(SPI_POLLING_TIMEOUT);
+ while (pl022->tx < pl022->tx_end || pl022->rx < pl022->rx_end) {
+ time = jiffies;
+ readwriter(pl022);
+ if (time_after(time, timeout)) {
+ dev_warn(&pl022->adev->dev,
+ "%s: timeout!\n", __func__);
+ print_current_status(pl022);
+ return -ETIMEDOUT;
}
-
- /* Update total byte transferred */
- message->actual_length += pl022->cur_transfer->len;
- /* Move to next transfer */
- message->state = next_transfer(pl022);
- if (message->state != STATE_DONE
- && pl022->cur_transfer->cs_change)
- pl022_cs_control(pl022, SSP_CHIP_DESELECT);
+ cpu_relax();
}
-out:
- /* Handle end of message */
- if (message->state == STATE_DONE)
- message->status = 0;
- else if (message->state == STATE_TIMEOUT)
- message->status = -EAGAIN;
- else
- message->status = -EIO;
- giveback(pl022);
- return;
+ return 0;
}
-static int pl022_transfer_one_message(struct spi_controller *host,
- struct spi_message *msg)
+static int pl022_transfer_one(struct spi_controller *host, struct spi_device *spi,
+ struct spi_transfer *transfer)
{
struct pl022 *pl022 = spi_controller_get_devdata(host);
- /* Initial message state */
- pl022->cur_msg = msg;
- msg->state = STATE_START;
-
- pl022->cur_transfer = list_entry(msg->transfers.next,
- struct spi_transfer, transfer_list);
+ pl022->cur_transfer = transfer;
/* Setup the SPI using the per chip configuration */
- pl022->cur_chip = spi_get_ctldata(msg->spi);
- pl022->cur_cs = spi_get_chipselect(msg->spi, 0);
+ pl022->cur_chip = spi_get_ctldata(spi);
+ pl022->cur_cs = spi_get_chipselect(spi, 0);
/* This is always available but may be set to -ENOENT */
- pl022->cur_gpiod = spi_get_csgpiod(msg->spi, 0);
+ pl022->cur_gpiod = spi_get_csgpiod(spi, 0);
restore_state(pl022);
flush(pl022);
if (pl022->cur_chip->xfer_type == POLLING_TRANSFER)
- do_polling_transfer(pl022);
+ return do_polling_transfer(pl022);
else
- do_interrupt_dma_transfer(pl022);
+ return do_interrupt_dma_transfer(pl022);
+}
- return 0;
+static void pl022_handle_err(struct spi_controller *ctlr, struct spi_message *message)
+{
+ struct pl022 *pl022 = spi_controller_get_devdata(ctlr);
+
+ terminate_dma(pl022);
+ writew(DISABLE_ALL_INTERRUPTS, SSP_IMSC(pl022->virtbase));
+ writew(CLEAR_ALL_INTERRUPTS, SSP_ICR(pl022->virtbase));
}
static int pl022_unprepare_transfer_hardware(struct spi_controller *host)
@@ -2138,7 +1901,9 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
host->cleanup = pl022_cleanup;
host->setup = pl022_setup;
host->auto_runtime_pm = true;
- host->transfer_one_message = pl022_transfer_one_message;
+ host->transfer_one = pl022_transfer_one;
+ host->set_cs = pl022_cs_control;
+ host->handle_err = pl022_handle_err;
host->unprepare_transfer_hardware = pl022_unprepare_transfer_hardware;
host->rt = platform_info->rt;
host->dev.of_node = dev->of_node;
@@ -2175,10 +1940,6 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
goto err_no_clk;
}
- /* Initialize transfer pump */
- tasklet_init(&pl022->pump_transfers, pump_transfers,
- (unsigned long)pl022);
-
/* Disable SSP */
writew((readw(SSP_CR1(pl022->virtbase)) & (~SSP_CR1_MASK_SSE)),
SSP_CR1(pl022->virtbase));
@@ -2261,7 +2022,6 @@ pl022_remove(struct amba_device *adev)
pl022_dma_remove(pl022);
amba_release_regions(adev);
- tasklet_disable(&pl022->pump_transfers);
}
#ifdef CONFIG_PM_SLEEP
--
2.39.2