| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20200616034044.v3.5.Ib1e6855405fc9c99916ab7c7dee84d73a8bf3d68@changeid>
Date: Tue, 16 Jun 2020 03:40:50 -0700
From: Douglas Anderson <dianders@...omium.org>
To: Mark Brown <broonie@...nel.org>
Cc: Alok Chauhan <alokc@...eaurora.org>, skakit@...eaurora.org,
swboyd@...omium.org, Douglas Anderson <dianders@...omium.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-spi@...r.kernel.org
Subject: [PATCH v3 5/5] 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: Douglas Anderson <dianders@...omium.org>
---
Changes in v3:
- ("spi: spi-geni-qcom: Don't keep a local state variable") new in v3.
Changes in v2: None
drivers/spi/spi-geni-qcom.c | 55 ++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 63a62548b078..6feea88d63ac 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -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);
--
2.27.0.290.gba653c62da-goog
Powered by blists - more mailing lists