Unverified Commit 7ba9bdcb authored by Douglas Anderson's avatar Douglas Anderson Committed by Mark Brown
Browse files

spi: spi-geni-qcom: Don't keep a local state variable 



The variable "cur_mcmd" kept track of our current state (idle, xfer,
cs, cancel).  We don't really need it, so get rid of it.  Instead:
* Use separate condition variables for "chip select done", "cancel
  done", and "abort done".  This is important so that if a "done"
  comes through (perhaps some previous interrupt finally came through)
  it can't confuse the cancel/abort function.
* Use the "done" interrupt only for when a chip select or transfer is
  done and we can tell the difference by looking at whether "cur_xfer"
  is NULL.

This is mostly a no-op change.  However, it is possible it could fix
an issue where a super delayed interrupt for a cancel command could
have confused our waiting for an abort command.

Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
Reviewed-by: default avatarStephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20200618080459.v4.5.Ib1e6855405fc9c99916ab7c7dee84d73a8bf3d68@changeid


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 902481a7

drivers/spi/spi-geni-qcom.c

+27 −28
Original line number Diff line number Diff line
@@ -63,13 +63,6 @@ 
#define TIMESTAMP_AFTER		BIT(3)

#define POST_CMD_DELAY		BIT(4)



enum spi_m_cmd_opcode {

	CMD_NONE,

	CMD_XFER,

	CMD_CS,

	CMD_CANCEL,

};



struct spi_geni_master {

	struct geni_se se;

	struct device *dev;
@@ -81,10 +74,11 @@ struct spi_geni_master { 
	unsigned int tx_rem_bytes;

	unsigned int rx_rem_bytes;

	const struct spi_transfer *cur_xfer;

	struct completion xfer_done;

	struct completion cs_done;

	struct completion cancel_done;

	struct completion abort_done;

	unsigned int oversampling;

	spinlock_t lock;

	enum spi_m_cmd_opcode cur_mcmd;

	int irq;

};
@@ -126,20 +120,23 @@ static void handle_fifo_timeout(struct spi_master *spi, 
	struct geni_se *se = &mas->se;



	spin_lock_irq(&mas->lock);

	reinit_completion(&mas->xfer_done);

	mas->cur_mcmd = CMD_CANCEL;

	geni_se_cancel_m_cmd(se);

	reinit_completion(&mas->cancel_done);

	writel(0, se->base + SE_GENI_TX_WATERMARK_REG);

	mas->cur_xfer = NULL;

	mas->tx_rem_bytes = mas->rx_rem_bytes = 0;

	geni_se_cancel_m_cmd(se);

	spin_unlock_irq(&mas->lock);

	time_left = wait_for_completion_timeout(&mas->xfer_done, HZ);



	time_left = wait_for_completion_timeout(&mas->cancel_done, HZ);

	if (time_left)

		return;



	spin_lock_irq(&mas->lock);

	reinit_completion(&mas->xfer_done);

	reinit_completion(&mas->abort_done);

	geni_se_abort_m_cmd(se);

	spin_unlock_irq(&mas->lock);

	time_left = wait_for_completion_timeout(&mas->xfer_done, HZ);



	time_left = wait_for_completion_timeout(&mas->abort_done, HZ);

	if (!time_left)

		dev_err(mas->dev, "Failed to cancel/abort m_cmd\n");

}
@@ -156,15 +153,14 @@ static void spi_geni_set_cs(struct spi_device *slv, bool set_flag) 
		set_flag = !set_flag;



	spin_lock_irq(&mas->lock);

	reinit_completion(&mas->xfer_done);

	mas->cur_mcmd = CMD_CS;

	reinit_completion(&mas->cs_done);

	if (set_flag)

		geni_se_setup_m_cmd(se, SPI_CS_ASSERT, 0);

	else

		geni_se_setup_m_cmd(se, SPI_CS_DEASSERT, 0);

	spin_unlock_irq(&mas->lock);



	time_left = wait_for_completion_timeout(&mas->xfer_done, HZ);

	time_left = wait_for_completion_timeout(&mas->cs_done, HZ);

	if (!time_left)

		handle_fifo_timeout(spi, NULL);
@@ -383,7 +379,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer, 
		mas->rx_rem_bytes = xfer->len;

	}

	writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);

	mas->cur_mcmd = CMD_XFER;



	/*

	 * Lock around right before we start the transfer since our
@@ -520,11 +515,13 @@ static irqreturn_t geni_spi_isr(int irq, void *data) 
		geni_spi_handle_tx(mas);



	if (m_irq & M_CMD_DONE_EN) {

		if (mas->cur_mcmd == CMD_XFER)

		if (mas->cur_xfer) {

			spi_finalize_current_transfer(spi);

		else if (mas->cur_mcmd == CMD_CS)

			complete(&mas->xfer_done);

		mas->cur_mcmd = CMD_NONE;

			mas->cur_xfer = NULL;

		} else {

			complete(&mas->cs_done);

		}



		/*

		 * If this happens, then a CMD_DONE came before all the Tx

		 * buffer bytes were sent out. This is unusual, log this
@@ -546,10 +543,10 @@ static irqreturn_t geni_spi_isr(int irq, void *data) 
				mas->rx_rem_bytes, mas->cur_bits_per_word);

	}



	if ((m_irq & M_CMD_CANCEL_EN) || (m_irq & M_CMD_ABORT_EN)) {

		mas->cur_mcmd = CMD_NONE;

		complete(&mas->xfer_done);

	}

	if (m_irq & M_CMD_CANCEL_EN)

		complete(&mas->cancel_done);

	if (m_irq & M_CMD_ABORT_EN)

		complete(&mas->abort_done);



	/*

	 * It's safe or a good idea to Ack all of our our interrupts at the
@@ -617,7 +614,9 @@ static int spi_geni_probe(struct platform_device *pdev) 
	spi->handle_err = handle_fifo_timeout;

	spi->set_cs = spi_geni_set_cs;



	init_completion(&mas->xfer_done);

	init_completion(&mas->cs_done);

	init_completion(&mas->cancel_done);

	init_completion(&mas->abort_done);

	spin_lock_init(&mas->lock);

	pm_runtime_enable(dev);

CRA Git | Maintained and supported by SUSTech CRA and CCSE