Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1670507 > unrolled thread

[PATCH v3 00/15] spi: qup: Fixes and add support for >64k transfers

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/15] spi: qup: Fixes and add support for >64k transfers Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
    [PATCH v3 11/15] spi: qup: allow block mode to generate multiple transactions Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
    [PATCH v3 06/15] spi: qup: Fix error handling in spi_qup_prep_sg Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
    [PATCH v3 14/15] spi: qup: Ensure done detection Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
    [PATCH v3 15/15] spi: qup: support for qup v1 dma Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
      Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma Rob Herring <robh@kernel.org> - 2017-06-23 23:50 +0200
        Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-27 11:40 +0200
        Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma Andy Gross <andy.gross@linaro.org> - 2017-06-28 08:50 +0200
    [PATCH v3 07/15] spi: qup: Fix transaction done signaling Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:20 +0200
    [PATCH v3 05/15] spi: qup: Place the QUP in run mode before DMA Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:30 +0200
    [PATCH v3 04/15] spi: qup: Add completion timeout Varadarajan Narayanan <varada@codeaurora.org> - 2017-06-20 11:30 +0200

#1670507 — [PATCH v3 00/15] spi: qup: Fixes and add support for >64k transfers

v3:
	Fix git bisect-ability issues in
		spi: qup: Add completion structures for DMA
		spi: qup: Add completion timeout
		spi: qup: Place the QUP in run mode before DMA
		spi: qup: Fix transaction done signaling

v2:
	Incorporate feedback from Andy Gross, Sricharan, Stanimir Varbanov

	Modified the QUP-v1 dma completion sequence to QUP-v2 as per feedback.

	Removed code that used controller->xfer to identify extra interrupts,
	since with the fixes done to handle i/o completion we don't see
	extra interrupts.

v1:
	This series fixes some existing issues in the code for both
	interrupt and dma mode. Patches 1 - 11 are the fixes.
	Random failures/timeout are observed without these fixes.
	Also, the current driver does not support block transfers > 64K
	and the driver quietly fails. Patches 12 - 18 add support for this
	in both interrupt and dma mode.

	The entire series has been tested on ipq4019 with
	SPI-NOR flash for block sizes > 64k.

Varadarajan Narayanan (15):
  spi: qup: Enable chip select support
  spi: qup: Setup DMA mode correctly
  spi: qup: Add completion structures for DMA
  spi: qup: Add completion timeout
  spi: qup: Place the QUP in run mode before DMA
  spi: qup: Fix error handling in spi_qup_prep_sg
  spi: qup: Fix transaction done signaling
  spi: qup: Do block sized read/write in block mode
  spi: qup: refactor spi_qup_io_config into two functions
  spi: qup: call io_config in mode specific function
  spi: qup: allow block mode to generate multiple transactions
  spi: qup: refactor spi_qup_prep_sg
  spi: qup: allow multiple DMA transactions per spi xfer
  spi: qup: Ensure done detection
  spi: qup: support for qup v1 dma

 .../devicetree/bindings/spi/qcom,spi-qup.txt       |   6 +
 drivers/spi/spi-qup.c                              | 630 ++++++++++++++-------
 2 files changed, 441 insertions(+), 195 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [next] | [standalone]

#1670510 — [PATCH v3 11/15] spi: qup: allow block mode to generate multiple transactions

This let's you write more to the SPI bus than 64K-1 which is important
if the block size of a SPI device is >= 64K or some other device wants
to do something larger.

This has the benefit of completely removing spi_message from the spi-qup
transactions

Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 128 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 80 insertions(+), 48 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 37b51e3..026f25e 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -120,7 +120,7 @@
 
 #define SPI_NUM_CHIPSELECTS		4
 
-#define SPI_MAX_DMA_XFER		(SZ_64K - 64)
+#define SPI_MAX_XFER			(SZ_64K - 64)
 
 /* high speed mode is when bus rate is greater then 26MHz */
 #define SPI_HS_MIN_RATE			26000000
@@ -151,6 +151,8 @@ struct spi_qup {
 	int			n_words;
 	int			tx_bytes;
 	int			rx_bytes;
+	const u8		*tx_buf;
+	u8			*rx_buf;
 	int			qup_v1;
 
 	int			mode;
@@ -175,6 +177,12 @@ static inline bool spi_qup_is_dma_xfer(int mode)
 	return false;
 }
 
+/* get's the transaction size length */
+static inline unsigned int spi_qup_len(struct spi_qup *controller)
+{
+	return controller->n_words * controller->w_size;
+}
+
 static inline bool spi_qup_is_valid_state(struct spi_qup *controller)
 {
 	u32 opstate = readl_relaxed(controller->base + QUP_STATE);
@@ -227,10 +235,9 @@ static int spi_qup_set_state(struct spi_qup *controller, u32 state)
 	return 0;
 }
 
-static void spi_qup_read_from_fifo(struct spi_qup *controller,
-	struct spi_transfer *xfer, u32 num_words)
+static void spi_qup_read_from_fifo(struct spi_qup *controller, u32 num_words)
 {
-	u8 *rx_buf = xfer->rx_buf;
+	u8 *rx_buf = controller->rx_buf;
 	int i, shift, num_bytes;
 	u32 word;
 
@@ -238,8 +245,9 @@ static void spi_qup_read_from_fifo(struct spi_qup *controller,
 
 		word = readl_relaxed(controller->base + QUP_INPUT_FIFO);
 
-		num_bytes = min_t(int, xfer->len - controller->rx_bytes,
-					controller->w_size);
+		num_bytes = min_t(int, spi_qup_len(controller) -
+				       controller->rx_bytes,
+				       controller->w_size);
 
 		if (!rx_buf) {
 			controller->rx_bytes += num_bytes;
@@ -260,13 +268,12 @@ static void spi_qup_read_from_fifo(struct spi_qup *controller,
 	}
 }
 
-static void spi_qup_read(struct spi_qup *controller,
-			    struct spi_transfer *xfer)
+static void spi_qup_read(struct spi_qup *controller)
 {
 	u32 remainder, words_per_block, num_words;
 	bool is_block_mode = controller->mode == QUP_IO_M_MODE_BLOCK;
 
-	remainder = DIV_ROUND_UP(xfer->len - controller->rx_bytes,
+	remainder = DIV_ROUND_UP(spi_qup_len(controller) - controller->rx_bytes,
 				 controller->w_size);
 	words_per_block = controller->in_blk_sz >> 2;
 
@@ -287,7 +294,7 @@ static void spi_qup_read(struct spi_qup *controller,
 		}
 
 		/* read up to the maximum transfer size available */
-		spi_qup_read_from_fifo(controller, xfer, num_words);
+		spi_qup_read_from_fifo(controller, num_words);
 
 		remainder -= num_words;
 
@@ -309,18 +316,18 @@ static void spi_qup_read(struct spi_qup *controller,
 
 }
 
-static void spi_qup_write_to_fifo(struct spi_qup *controller,
-	struct spi_transfer *xfer, u32 num_words)
+static void spi_qup_write_to_fifo(struct spi_qup *controller, u32 num_words)
 {
-	const u8 *tx_buf = xfer->tx_buf;
+	const u8 *tx_buf = controller->tx_buf;
 	int i, num_bytes;
 	u32 word, data;
 
 	for (; num_words; num_words--) {
 		word = 0;
 
-		num_bytes = min_t(int, xfer->len - controller->tx_bytes,
-				    controller->w_size);
+		num_bytes = min_t(int, spi_qup_len(controller) -
+				       controller->tx_bytes,
+				       controller->w_size);
 		if (tx_buf)
 			for (i = 0; i < num_bytes; i++) {
 				data = tx_buf[controller->tx_bytes + i];
@@ -338,13 +345,12 @@ static void spi_qup_dma_done(void *data)
 	complete(data);
 }
 
-static void spi_qup_write(struct spi_qup *controller,
-			    struct spi_transfer *xfer)
+static void spi_qup_write(struct spi_qup *controller)
 {
 	bool is_block_mode = controller->mode == QUP_IO_M_MODE_BLOCK;
 	u32 remainder, words_per_block, num_words;
 
-	remainder = DIV_ROUND_UP(xfer->len - controller->tx_bytes,
+	remainder = DIV_ROUND_UP(spi_qup_len(controller) - controller->tx_bytes,
 				 controller->w_size);
 	words_per_block = controller->out_blk_sz >> 2;
 
@@ -364,7 +370,7 @@ static void spi_qup_write(struct spi_qup *controller,
 			num_words = 1;
 		}
 
-		spi_qup_write_to_fifo(controller, xfer, num_words);
+		spi_qup_write_to_fifo(controller, num_words);
 
 		remainder -= num_words;
 
@@ -469,36 +475,62 @@ static int spi_qup_do_pio(struct spi_device *spi, struct spi_transfer *xfer,
 {
 	struct spi_master *master = spi->master;
 	struct spi_qup *qup = spi_master_get_devdata(master);
-	int ret;
+	int ret, n_words, iterations, offset = 0;
 
-	ret = spi_qup_io_config(spi, xfer);
-	if (ret)
-		return ret;
+	n_words = qup->n_words;
+	iterations = n_words / SPI_MAX_XFER; /* round down */
+	qup->rx_buf = xfer->rx_buf;
+	qup->tx_buf = xfer->tx_buf;
 
-	ret = spi_qup_set_state(qup, QUP_STATE_RUN);
-	if (ret) {
-		dev_warn(qup->dev, "cannot set RUN state\n");
-		return ret;
-	}
+	do {
+		if (iterations)
+			qup->n_words = SPI_MAX_XFER;
+		else
+			qup->n_words = n_words % SPI_MAX_XFER;
 
-	ret = spi_qup_set_state(qup, QUP_STATE_PAUSE);
-	if (ret) {
-		dev_warn(qup->dev, "cannot set PAUSE state\n");
-		return ret;
-	}
+		if (qup->tx_buf && offset)
+			qup->tx_buf = xfer->tx_buf + offset * SPI_MAX_XFER;
 
-	if (qup->mode == QUP_IO_M_MODE_FIFO)
-		spi_qup_write(qup, xfer);
+		if (qup->rx_buf && offset)
+			qup->rx_buf = xfer->rx_buf + offset * SPI_MAX_XFER;
 
-	ret = spi_qup_set_state(qup, QUP_STATE_RUN);
-	if (ret) {
-		dev_warn(qup->dev, "%s(%d): cannot set RUN state\n",
-				__func__, __LINE__);
-		return ret;
-	}
+		/*
+		 * if the transaction is small enough, we need
+		 * to fallback to FIFO mode
+		 */
+		if (qup->n_words <= (qup->in_fifo_sz / sizeof(u32)))
+			qup->mode = QUP_IO_M_MODE_FIFO;
 
-	if (!wait_for_completion_timeout(&qup->done, timeout))
-		return -ETIMEDOUT;
+		ret = spi_qup_io_config(spi, xfer);
+		if (ret)
+			return ret;
+
+		ret = spi_qup_set_state(qup, QUP_STATE_RUN);
+		if (ret) {
+			dev_warn(qup->dev, "cannot set RUN state\n");
+			return ret;
+		}
+
+		ret = spi_qup_set_state(qup, QUP_STATE_PAUSE);
+		if (ret) {
+			dev_warn(qup->dev, "cannot set PAUSE state\n");
+			return ret;
+		}
+
+		if (qup->mode == QUP_IO_M_MODE_FIFO)
+			spi_qup_write(qup);
+
+		ret = spi_qup_set_state(qup, QUP_STATE_RUN);
+		if (ret) {
+			dev_warn(qup->dev, "cannot set RUN state\n");
+			return ret;
+		}
+
+		if (!wait_for_completion_timeout(&qup->done, timeout))
+			return -ETIMEDOUT;
+
+		offset++;
+	} while (iterations--);
 
 	return 0;
 }
@@ -506,7 +538,6 @@ static int spi_qup_do_pio(struct spi_device *spi, struct spi_transfer *xfer,
 static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 {
 	struct spi_qup *controller = dev_id;
-	struct spi_transfer *xfer = controller->xfer;
 	u32 opflags, qup_err, spi_err;
 	int error = 0;
 
@@ -543,10 +574,10 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
 	} else {
 		if (opflags & QUP_OP_IN_SERVICE_FLAG)
-			spi_qup_read(controller, xfer);
+			spi_qup_read(controller);
 
 		if (opflags & QUP_OP_OUT_SERVICE_FLAG)
-			spi_qup_write(controller, xfer);
+			spi_qup_write(controller);
 	}
 
 	if ((opflags & QUP_OP_MAX_INPUT_DONE_FLAG) || error)
@@ -756,7 +787,8 @@ static int spi_qup_transfer_one(struct spi_master *master,
 		return ret;
 
 	timeout = DIV_ROUND_UP(xfer->speed_hz, MSEC_PER_SEC);
-	timeout = DIV_ROUND_UP(xfer->len * 8, timeout);
+	timeout = DIV_ROUND_UP(min_t(unsigned long, SPI_MAX_XFER,
+				     xfer->len) * 8, timeout);
 	timeout = 100 * msecs_to_jiffies(timeout);
 
 	if (spi_qup_is_dma_xfer(controller->mode))
@@ -961,7 +993,7 @@ static int spi_qup_probe(struct platform_device *pdev)
 	master->dev.of_node = pdev->dev.of_node;
 	master->auto_runtime_pm = true;
 	master->dma_alignment = dma_get_cache_alignment();
-	master->max_dma_len = SPI_MAX_DMA_XFER;
+	master->max_dma_len = SPI_MAX_XFER;
 
 	platform_set_drvdata(pdev, master);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1670511 — [PATCH v3 06/15] spi: qup: Fix error handling in spi_qup_prep_sg

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 0683e47..e2b4a50 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -310,8 +310,8 @@ static int spi_qup_prep_sg(struct spi_master *master, struct spi_transfer *xfer,
 	}
 
 	desc = dmaengine_prep_slave_sg(chan, sgl, nents, dir, flags);
-	if (!desc)
-		return -EINVAL;
+	if (IS_ERR_OR_NULL(desc))
+		return desc ? PTR_ERR(desc) : -EINVAL;
 
 	desc->callback = callback;
 	desc->callback_param = data;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1670512 — [PATCH v3 14/15] spi: qup: Ensure done detection

This patch fixes an issue where a SPI transaction has completed, but the
done condition is missed.  This occurs because at the time of interrupt the
MAX_INPUT_DONE_FLAG is not asserted.  However, in the process of reading
blocks of data from the FIFO, the last portion of data comes in.

The opflags read at the beginning of the irq handler no longer matches the
current opflag state.  To get around this condition, the block read
function should update the opflags so that done detection is correct after
the return.

Signed-off-by: Andy Gross <agross@codeaurora.org>
Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index a39a0d2..ca5206e 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -268,7 +268,7 @@ static void spi_qup_read_from_fifo(struct spi_qup *controller, u32 num_words)
 	}
 }
 
-static void spi_qup_read(struct spi_qup *controller)
+static void spi_qup_read(struct spi_qup *controller, u32 *opflags)
 {
 	u32 remainder, words_per_block, num_words;
 	bool is_block_mode = controller->mode == QUP_IO_M_MODE_BLOCK;
@@ -307,10 +307,12 @@ static void spi_qup_read(struct spi_qup *controller)
 
 	/*
 	 * Due to extra stickiness of the QUP_OP_IN_SERVICE_FLAG during block
-	 * mode reads, it has to be cleared again at the very end
+	 * reads, it has to be cleared again at the very end.  However, be sure
+	 * to refresh opflags value because MAX_INPUT_DONE_FLAG may now be
+	 * present and this is used to determine if transaction is complete
 	 */
-	if (is_block_mode && spi_qup_is_flag_set(controller,
-				QUP_OP_MAX_INPUT_DONE_FLAG))
+	*opflags = readl_relaxed(controller->base + QUP_OPERATIONAL);
+	if (is_block_mode && *opflags & QUP_OP_MAX_INPUT_DONE_FLAG)
 		writel_relaxed(QUP_OP_IN_SERVICE_FLAG,
 			       controller->base + QUP_OPERATIONAL);
 
@@ -617,7 +619,7 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
 	} else {
 		if (opflags & QUP_OP_IN_SERVICE_FLAG)
-			spi_qup_read(controller);
+			spi_qup_read(controller, &opflags);
 
 		if (opflags & QUP_OP_OUT_SERVICE_FLAG)
 			spi_qup_write(controller);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1670513 — [PATCH v3 15/15] spi: qup: support for qup v1 dma

Currently the QUP Version v1 does not work with DMA so added
the support for the same.

1. It uses ADM DMA which requires TX and RX CRCI
2. DMA channel initialization need to be done after setting
   block size for having valid values in maxburst
3. QUP mode should be DMOV instead of BAM.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 .../devicetree/bindings/spi/qcom,spi-qup.txt       |  6 ++++
 drivers/spi/spi-qup.c                              | 35 +++++++++++++++++-----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
index 5c09077..e754181 100644
--- a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
+++ b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
@@ -38,6 +38,12 @@ Optional properties:
 - dma-names:    Names for the dma channels, if present. There must be at
                 least one channel named "tx" for transmit and named "rx" for
                 receive.
+- qcom,tx-crci: Identificator for Client Rate Control Interface (CRCI) to be
+		used with TX DMA channel. Required when using DMA for
+		transmission with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
+- qcom,rx-crci: Identificator for Client Rate Control Interface (CRCI) to be
+		used with RX DMA channel. Required when using DMA for
+		receiving with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
 
 SPI slave nodes must be children of the SPI master node and can contain
 properties described in Documentation/devicetree/bindings/spi/spi-bus.txt
diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index ca5206e..b608efa3 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -658,7 +658,8 @@ static int spi_qup_io_prep(struct spi_device *spi, struct spi_transfer *xfer)
 	else if (spi->master->can_dma &&
 		 spi->master->can_dma(spi->master, spi, xfer) &&
 		 spi->master->cur_msg_mapped)
-		controller->mode = QUP_IO_M_MODE_BAM;
+		controller->mode = controller->qup_v1 ? QUP_IO_M_MODE_DMOV :
+							QUP_IO_M_MODE_BAM;
 	else
 		controller->mode = QUP_IO_M_MODE_BLOCK;
 
@@ -697,6 +698,7 @@ static int spi_qup_io_config(struct spi_device *spi, struct spi_transfer *xfer)
 		writel_relaxed(0, controller->base + QUP_MX_OUTPUT_CNT);
 		break;
 	case QUP_IO_M_MODE_BAM:
+	case QUP_IO_M_MODE_DMOV:
 		reinit_completion(&controller->txc);
 		reinit_completion(&controller->rxc);
 		writel_relaxed(controller->n_words,
@@ -901,6 +903,7 @@ static int spi_qup_init_dma(struct spi_master *master, resource_size_t base)
 	struct dma_slave_config *rx_conf = &spi->rx_conf,
 				*tx_conf = &spi->tx_conf;
 	struct device *dev = spi->dev;
+	u32 tx_crci = 0, rx_crci = 0;
 	int ret;
 
 	/* allocate dma resources, if available */
@@ -914,16 +917,34 @@ static int spi_qup_init_dma(struct spi_master *master, resource_size_t base)
 		goto err_tx;
 	}
 
+	if (spi->qup_v1) {
+		ret = of_property_read_u32(dev->of_node, "qcom,tx-crci",
+					   &tx_crci);
+		if (ret) {
+			dev_err(dev, "missing property qcom,tx-crci\n");
+			goto err;
+		}
+
+		ret = of_property_read_u32(dev->of_node, "qcom,rx-crci",
+					   &rx_crci);
+		if (ret) {
+			dev_err(dev, "missing property qcom,rx-crci\n");
+			goto err;
+		}
+	}
+
 	/* set DMA parameters */
 	rx_conf->direction = DMA_DEV_TO_MEM;
 	rx_conf->device_fc = 1;
 	rx_conf->src_addr = base + QUP_INPUT_FIFO;
 	rx_conf->src_maxburst = spi->in_blk_sz;
+	rx_conf->slave_id = rx_crci;
 
 	tx_conf->direction = DMA_MEM_TO_DEV;
 	tx_conf->device_fc = 1;
 	tx_conf->dst_addr = base + QUP_OUTPUT_FIFO;
 	tx_conf->dst_maxburst = spi->out_blk_sz;
+	tx_conf->slave_id = tx_crci;
 
 	ret = dmaengine_slave_config(master->dma_rx, rx_conf);
 	if (ret) {
@@ -1050,12 +1071,6 @@ static int spi_qup_probe(struct platform_device *pdev)
 	controller->cclk = cclk;
 	controller->irq = irq;
 
-	ret = spi_qup_init_dma(master, res->start);
-	if (ret == -EPROBE_DEFER)
-		goto error;
-	else if (!ret)
-		master->can_dma = spi_qup_can_dma;
-
 	/* set v1 flag if device is version 1 */
 	if (of_device_is_compatible(dev->of_node, "qcom,spi-qup-v1.1.1"))
 		controller->qup_v1 = 1;
@@ -1092,6 +1107,12 @@ static int spi_qup_probe(struct platform_device *pdev)
 		 controller->in_blk_sz, controller->in_fifo_sz,
 		 controller->out_blk_sz, controller->out_fifo_sz);
 
+	ret = spi_qup_init_dma(master, res->start);
+	if (ret == -EPROBE_DEFER)
+		goto error;
+	else if (!ret)
+		master->can_dma = spi_qup_can_dma;
+
 	writel_relaxed(1, base + QUP_SW_RESET);
 
 	ret = spi_qup_set_state(controller, QUP_STATE_RESET);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1673894 — Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma

On Tue, Jun 20, 2017 at 02:40:57PM +0530, Varadarajan Narayanan wrote:
> Currently the QUP Version v1 does not work with DMA so added
> the support for the same.
> 
> 1. It uses ADM DMA which requires TX and RX CRCI
> 2. DMA channel initialization need to be done after setting
>    block size for having valid values in maxburst
> 3. QUP mode should be DMOV instead of BAM.
> 
> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  .../devicetree/bindings/spi/qcom,spi-qup.txt       |  6 ++++
>  drivers/spi/spi-qup.c                              | 35 +++++++++++++++++-----
>  2 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> index 5c09077..e754181 100644
> --- a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> @@ -38,6 +38,12 @@ Optional properties:
>  - dma-names:    Names for the dma channels, if present. There must be at
>                  least one channel named "tx" for transmit and named "rx" for
>                  receive.
> +- qcom,tx-crci: Identificator for Client Rate Control Interface (CRCI) to be

Identificator is not a word.

This sounds like something that should be a cell in the dmas property.

> +		used with TX DMA channel. Required when using DMA for
> +		transmission with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
> +- qcom,rx-crci: Identificator for Client Rate Control Interface (CRCI) to be
> +		used with RX DMA channel. Required when using DMA for
> +		receiving with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
>  
>  SPI slave nodes must be children of the SPI master node and can contain
>  properties described in Documentation/devicetree/bindings/spi/spi-bus.txt

[toc] | [prev] | [next] | [standalone]

#1675455 — Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma

Rob,

On Fri, Jun 23, 2017 at 04:49:23PM -0500, Rob Herring wrote:
> On Tue, Jun 20, 2017 at 02:40:57PM +0530, Varadarajan Narayanan wrote:
> > Currently the QUP Version v1 does not work with DMA so added
> > the support for the same.
> >
> > 1. It uses ADM DMA which requires TX and RX CRCI
> > 2. DMA channel initialization need to be done after setting
> >    block size for having valid values in maxburst
> > 3. QUP mode should be DMOV instead of BAM.
> >
> > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> > Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> > ---
> >  .../devicetree/bindings/spi/qcom,spi-qup.txt       |  6 ++++
> >  drivers/spi/spi-qup.c                              | 35 +++++++++++++++++-----
> >  2 files changed, 34 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > index 5c09077..e754181 100644
> > --- a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > +++ b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > @@ -38,6 +38,12 @@ Optional properties:
> >  - dma-names:    Names for the dma channels, if present. There must be at
> >                  least one channel named "tx" for transmit and named "rx" for
> >                  receive.
> > +- qcom,tx-crci: Identificator for Client Rate Control Interface (CRCI) to be
>
> Identificator is not a word.
>
> This sounds like something that should be a cell in the dmas property.

I will hold this patch for now. This depends on the ADM driver, which
is not upstreamed yet. Will incorporate your feedback and re-post the
patch later.

Thanks
Varada

> > +		used with TX DMA channel. Required when using DMA for
> > +		transmission with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
> > +- qcom,rx-crci: Identificator for Client Rate Control Interface (CRCI) to be
> > +		used with RX DMA channel. Required when using DMA for
> > +		receiving with QUP Version 1 i.e qcom,spi-qup-v1.1.1.
> >
> >  SPI slave nodes must be children of the SPI master node and can contain
> >  properties described in Documentation/devicetree/bindings/spi/spi-bus.txt

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1676408 — Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma

On Fri, Jun 23, 2017 at 04:49:23PM -0500, Rob Herring wrote:
> On Tue, Jun 20, 2017 at 02:40:57PM +0530, Varadarajan Narayanan wrote:
> > Currently the QUP Version v1 does not work with DMA so added
> > the support for the same.
> > 
> > 1. It uses ADM DMA which requires TX and RX CRCI
> > 2. DMA channel initialization need to be done after setting
> >    block size for having valid values in maxburst
> > 3. QUP mode should be DMOV instead of BAM.
> > 
> > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> > Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> > ---
> >  .../devicetree/bindings/spi/qcom,spi-qup.txt       |  6 ++++
> >  drivers/spi/spi-qup.c                              | 35 +++++++++++++++++-----
> >  2 files changed, 34 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > index 5c09077..e754181 100644
> > --- a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > +++ b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> > @@ -38,6 +38,12 @@ Optional properties:
> >  - dma-names:    Names for the dma channels, if present. There must be at
> >                  least one channel named "tx" for transmit and named "rx" for
> >                  receive.
> > +- qcom,tx-crci: Identificator for Client Rate Control Interface (CRCI) to be
> 
> Identificator is not a word.
> 
> This sounds like something that should be a cell in the dmas property.

Yes, the CRCI should be part of the dma cells for the ADM.  That would make a
channel + crci work as a virtual channel that is backed by a hardware channel.

The only thing that has to be dealt with is the protocol difference between the
BAM and ADM dma blocks, which is what the v1 compatible tells us.


Regards,
Andy

[toc] | [prev] | [next] | [standalone]

#1670514 — [PATCH v3 07/15] spi: qup: Fix transaction done signaling

Wait to signal done until we get all of the interrupts we are expecting
to get for a transaction.  If we don't wait for the input done flag, we
can be in between transactions when the done flag comes in and this can
mess up the next transaction.

While here cleaning up the code which sets controller->xfer = NULL and
restores it in the ISR. This looks to be some debug code which is not
required.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index e2b4a50..2bd5115 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -407,29 +407,16 @@ static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer,
 static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 {
 	struct spi_qup *controller = dev_id;
-	struct spi_transfer *xfer;
+	struct spi_transfer *xfer = controller->xfer;
 	u32 opflags, qup_err, spi_err;
-	unsigned long flags;
 	int error = 0;
 
-	spin_lock_irqsave(&controller->lock, flags);
-	xfer = controller->xfer;
-	controller->xfer = NULL;
-	spin_unlock_irqrestore(&controller->lock, flags);
-
 	qup_err = readl_relaxed(controller->base + QUP_ERROR_FLAGS);
 	spi_err = readl_relaxed(controller->base + SPI_ERROR_FLAGS);
 	opflags = readl_relaxed(controller->base + QUP_OPERATIONAL);
 
 	writel_relaxed(qup_err, controller->base + QUP_ERROR_FLAGS);
 	writel_relaxed(spi_err, controller->base + SPI_ERROR_FLAGS);
-	writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
-
-	if (!xfer) {
-		dev_err_ratelimited(controller->dev, "unexpected irq %08x %08x %08x\n",
-				    qup_err, spi_err, opflags);
-		return IRQ_HANDLED;
-	}
 
 	if (qup_err) {
 		if (qup_err & QUP_ERROR_OUTPUT_OVER_RUN)
@@ -453,7 +440,9 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 		error = -EIO;
 	}
 
-	if (!spi_qup_is_dma_xfer(controller->mode)) {
+	if (spi_qup_is_dma_xfer(controller->mode)) {
+		writel_relaxed(opflags, controller->base + QUP_OPERATIONAL);
+	} else {
 		if (opflags & QUP_OP_IN_SERVICE_FLAG)
 			spi_qup_fifo_read(controller, xfer);
 
@@ -461,12 +450,7 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id)
 			spi_qup_fifo_write(controller, xfer);
 	}
 
-	spin_lock_irqsave(&controller->lock, flags);
-	controller->error = error;
-	controller->xfer = xfer;
-	spin_unlock_irqrestore(&controller->lock, flags);
-
-	if (controller->rx_bytes == xfer->len || error)
+	if ((opflags & QUP_OP_MAX_INPUT_DONE_FLAG) || error)
 		complete(&controller->done);
 
 	return IRQ_HANDLED;
@@ -666,7 +650,6 @@ static int spi_qup_transfer_one(struct spi_master *master,
 exit:
 	spi_qup_set_state(controller, QUP_STATE_RESET);
 	spin_lock_irqsave(&controller->lock, flags);
-	controller->xfer = NULL;
 	if (!ret)
 		ret = controller->error;
 	spin_unlock_irqrestore(&controller->lock, flags);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1670525 — [PATCH v3 05/15] spi: qup: Place the QUP in run mode before DMA

Signed-off-by: Andy Gross <andy.gross@linaro.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index d3ccf53..0683e47 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -336,6 +336,14 @@ static int spi_qup_do_dma(struct spi_master *master, struct spi_transfer *xfer,
 	struct spi_qup *qup = spi_master_get_devdata(master);
 	int ret;
 
+	/* before issuing the descriptors, set the QUP to run */
+	ret = spi_qup_set_state(qup, QUP_STATE_RUN);
+	if (ret) {
+		dev_warn(qup->dev, "%s(%d): cannot set RUN state\n",
+				__func__, __LINE__);
+		return ret;
+	}
+
 	if (xfer->rx_buf) {
 		ret = spi_qup_prep_sg(master, xfer, DMA_DEV_TO_MEM,
 					spi_qup_dma_done, &qup->rxc);
@@ -383,6 +391,13 @@ static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer,
 
 	spi_qup_fifo_write(qup, xfer);
 
+	ret = spi_qup_set_state(qup, QUP_STATE_RUN);
+	if (ret) {
+		dev_warn(qup->dev, "%s(%d): cannot set RUN state\n",
+				__func__, __LINE__);
+		return ret;
+	}
+
 	if (!wait_for_completion_timeout(&qup->done, timeout))
 		return -ETIMEDOUT;
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [next] | [standalone]

#1670528 — [PATCH v3 04/15] spi: qup: Add completion timeout

Add i/o completion timeout for DMA and PIO modes.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/spi/spi-qup.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 59e799f..d3ccf53 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -330,7 +330,8 @@ static void spi_qup_dma_terminate(struct spi_master *master,
 		dmaengine_terminate_all(master->dma_rx);
 }
 
-static int spi_qup_do_dma(struct spi_master *master, struct spi_transfer *xfer)
+static int spi_qup_do_dma(struct spi_master *master, struct spi_transfer *xfer,
+			  unsigned long timeout)
 {
 	struct spi_qup *qup = spi_master_get_devdata(master);
 	int ret;
@@ -353,10 +354,17 @@ static int spi_qup_do_dma(struct spi_master *master, struct spi_transfer *xfer)
 		dma_async_issue_pending(master->dma_tx);
 	}
 
+	if (xfer->rx_buf && !wait_for_completion_timeout(&qup->rxc, timeout))
+		return -ETIMEDOUT;
+
+	if (xfer->tx_buf && !wait_for_completion_timeout(&qup->txc, timeout))
+		return -ETIMEDOUT;
+
 	return 0;
 }
 
-static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer)
+static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer,
+			  unsigned long timeout)
 {
 	struct spi_qup *qup = spi_master_get_devdata(master);
 	int ret;
@@ -375,6 +383,9 @@ static int spi_qup_do_pio(struct spi_master *master, struct spi_transfer *xfer)
 
 	spi_qup_fifo_write(qup, xfer);
 
+	if (!wait_for_completion_timeout(&qup->done, timeout))
+		return -ETIMEDOUT;
+
 	return 0;
 }
 
@@ -618,7 +629,6 @@ static int spi_qup_transfer_one(struct spi_master *master,
 	timeout = DIV_ROUND_UP(xfer->len * 8, timeout);
 	timeout = 100 * msecs_to_jiffies(timeout);
 
-
 	spin_lock_irqsave(&controller->lock, flags);
 	controller->xfer     = xfer;
 	controller->error    = 0;
@@ -629,10 +639,10 @@ static int spi_qup_transfer_one(struct spi_master *master,
 	if (spi_qup_is_dma_xfer(controller->mode)) {
 		reinit_completion(&controller->rxc);
 		reinit_completion(&controller->txc);
-		ret = spi_qup_do_dma(master, xfer);
+		ret = spi_qup_do_dma(master, xfer, timeout);
 	} else {
 		reinit_completion(&controller->done);
-		ret = spi_qup_do_pio(master, xfer);
+		ret = spi_qup_do_pio(master, xfer, timeout);
 	}
 
 	if (ret)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web