Linux-SPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 00/12] NXP DSPI bugfixes and support for LS1028A
@ 2020-03-14 22:43 Vladimir Oltean
  2020-03-14 22:43 ` [PATCH v3 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

This series addresses a few issues that were missed during the previous
series "[PATCH 00/12] TCFQ to XSPI migration for NXP DSPI driver", on
SoCs other than LS1021A and LS1043A. DMA mode has been completely broken
by that series, and XSPI mode never worked on little-endian controllers.

Then it introduces support for the LS1028A chip, whose compatible has
recently been documented here:

https://lore.kernel.org/linux-devicetree/20200218171418.18297-1-michael-QKn5cuLxLXY@public.gmane.org/

The device tree for the LS1028A SoC is extended with DMA channels
definition, such that even though the default operating mode is XSPI,
one can simply change DSPI_XSPI_MODE to DSPI_DMA_MODE in the
devtype_data structure of the driver and use that instead.

I don't expect the "fixes" patches to reach very far down the stable
pipe, since there has been pretty heavy refactoring in this driver.

For testing, benchmarking and debugging, the mikroBUS connector on the
LS1028A-RDB is made available via spidev.

Vladimir Oltean (12):
  spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR
  spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA
  spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode
  spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode
  spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight
  spi: spi-fsl-dspi: Replace interruptible wait queue with a simple
    completion
  spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA
    mode
  spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code
    path
  spi: spi-fsl-dspi: Move invariant configs out of
    dspi_transfer_one_message
  spi: spi-fsl-dspi: Add support for LS1028A
  arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers
  arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS

 .../boot/dts/freescale/fsl-ls1028a-rdb.dts    |  14 ++
 .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi |   6 +
 drivers/spi/spi-fsl-dspi.c                    | 227 +++++++++---------
 3 files changed, 138 insertions(+), 109 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR
  2020-03-14 22:43 [PATCH v3 00/12] NXP DSPI bugfixes and support for LS1028A Vladimir Oltean
@ 2020-03-14 22:43 ` Vladimir Oltean
  2020-03-14 22:43 ` [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion Vladimir Oltean
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie
  Cc: linux-spi, linux-kernel, shawnguo, robh+dt, mark.rutland,
	devicetree, eha, angelo, andrew.smirnov, gustavo, weic, mhosny,
	michael, peng.ma

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The SPI_MCR_PCSIS macro assumes that the controller has a number of chip
select signals equal to 6. That is not always the case, but actually is
described through the driver-specific "spi-num-chipselects" device tree
binding. LS1028A for example only has 4 chip selects.

Don't write to the upper bits of the PCSIS field, which are reserved in
the reference manual.

Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v4:
None.

Changes in v3:
None.

Changes in v2:
Remove duplicate phrase in commit message.

 drivers/spi/spi-fsl-dspi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 50e3382f0c50..6ca35881881b 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -22,7 +22,7 @@
 
 #define SPI_MCR				0x00
 #define SPI_MCR_MASTER			BIT(31)
-#define SPI_MCR_PCSIS			(0x3F << 16)
+#define SPI_MCR_PCSIS(x)		((x) << 16)
 #define SPI_MCR_CLR_TXF			BIT(11)
 #define SPI_MCR_CLR_RXF			BIT(10)
 #define SPI_MCR_XSPI			BIT(3)
@@ -1200,7 +1200,10 @@ static const struct regmap_config dspi_xspi_regmap_config[] = {
 
 static void dspi_init(struct fsl_dspi *dspi)
 {
-	unsigned int mcr = SPI_MCR_PCSIS;
+	unsigned int mcr;
+
+	/* Set idle states for all chip select signals to high */
+	mcr = SPI_MCR_PCSIS(GENMASK(dspi->ctlr->num_chipselect - 1, 0));
 
 	if (dspi->devtype_data->trans_mode == DSPI_XSPI_MODE)
 		mcr |= SPI_MCR_XSPI;
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

In XSPI mode, the 32-bit PUSHR register can be written to separately:
the higher 16 bits are for commands and the lower 16 bits are for data.

This has nicely been hacked around, by defining a second regmap with a
width of 16 bits, and effectively splitting a 32-bit register into 2
16-bit ones, from the perspective of this regmap_pushr.

The problem is the assumption about the controller's endianness. If the
controller is little endian (such as anything post-LS1046A), then the
first 2 bytes, in the order imposed by memory layout, will actually hold
the TXDATA, and the last 2 bytes will hold the CMD.

So take the controller's endianness into account when performing split
writes to PUSHR. The obvious and simple solution would have been to call
regmap_get_val_endian(), but that is an internal regmap function and we
don't want to change regmap just for this. Therefore, we just re-read
the "big-endian" device tree property.

Fixes: 58ba07ec79e6 ("spi: spi-fsl-dspi: Add support for XSPI mode registers")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
None.

Changes in v3:
None.

Changes in v2:
Parse "big-endian" device tree bindings instead of taking the decision
based on compatible SoC.

 drivers/spi/spi-fsl-dspi.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 6ca35881881b..be717776dd98 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -103,10 +103,6 @@
 #define SPI_FRAME_BITS(bits)		SPI_CTAR_FMSZ((bits) - 1)
 #define SPI_FRAME_EBITS(bits)		SPI_CTARE_FMSZE(((bits) - 1) >> 4)
 
-/* Register offsets for regmap_pushr */
-#define PUSHR_CMD			0x0
-#define PUSHR_TX			0x2
-
 #define DMA_COMPLETION_TIMEOUT		msecs_to_jiffies(3000)
 
 struct chip_data {
@@ -240,6 +236,13 @@ struct fsl_dspi {
 
 	int					words_in_flight;
 
+	/*
+	 * Offsets for CMD and TXDATA within SPI_PUSHR when accessed
+	 * individually (in XSPI mode)
+	 */
+	int					pushr_cmd;
+	int					pushr_tx;
+
 	void (*host_to_dev)(struct fsl_dspi *dspi, u32 *txdata);
 	void (*dev_to_host)(struct fsl_dspi *dspi, u32 rxdata);
 };
@@ -673,12 +676,12 @@ static void dspi_pushr_cmd_write(struct fsl_dspi *dspi, u16 cmd)
 	 */
 	if (dspi->len > dspi->oper_word_size)
 		cmd |= SPI_PUSHR_CMD_CONT;
-	regmap_write(dspi->regmap_pushr, PUSHR_CMD, cmd);
+	regmap_write(dspi->regmap_pushr, dspi->pushr_cmd, cmd);
 }
 
 static void dspi_pushr_txdata_write(struct fsl_dspi *dspi, u16 txdata)
 {
-	regmap_write(dspi->regmap_pushr, PUSHR_TX, txdata);
+	regmap_write(dspi->regmap_pushr, dspi->pushr_tx, txdata);
 }
 
 static void dspi_xspi_write(struct fsl_dspi *dspi, int cnt, bool eoq)
@@ -1259,6 +1262,7 @@ static int dspi_probe(struct platform_device *pdev)
 	struct fsl_dspi *dspi;
 	struct resource *res;
 	void __iomem *base;
+	bool big_endian;
 
 	ctlr = spi_alloc_master(&pdev->dev, sizeof(struct fsl_dspi));
 	if (!ctlr)
@@ -1284,6 +1288,7 @@ static int dspi_probe(struct platform_device *pdev)
 
 		/* Only Coldfire uses platform data */
 		dspi->devtype_data = &devtype_data[MCF5441X];
+		big_endian = true;
 	} else {
 
 		ret = of_property_read_u32(np, "spi-num-chipselects", &cs_num);
@@ -1305,6 +1310,15 @@ static int dspi_probe(struct platform_device *pdev)
 			ret = -EFAULT;
 			goto out_ctlr_put;
 		}
+
+		big_endian = of_device_is_big_endian(np);
+	}
+	if (big_endian) {
+		dspi->pushr_cmd = 0;
+		dspi->pushr_tx = 2;
+	} else {
+		dspi->pushr_cmd = 2;
+		dspi->pushr_tx = 0;
 	}
 
 	if (dspi->devtype_data->trans_mode == DSPI_XSPI_MODE)
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2020-03-14 22:43   ` [PATCH v3 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode Vladimir Oltean
                     ` (6 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

In DMA mode, dspi_setup_accel does not get called, which results in the
dspi->oper_word_size variable (which is used by dspi_dma_xfer) to not be
initialized properly.

Because oper_word_size is zero, a few calculations end up being
incorrect, and the DMA transfer eventually times out instead of sending
anything on the wire.

Set up native transfers (or 8-on-16 acceleration) using dspi_setup_accel
for DMA mode too.

Also take the opportunity and simplify the DMA buffer handling a little
bit.

Fixes: 6c1c26ecd9a3 ("spi: spi-fsl-dspi: Accelerate transfers using larger word size if possible")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
Rebased on top of "spi: spi-fsl-dspi: fix DMA mapping".
Stopped uselessly writing to SPI_CTAR in dspi_transfer_one_message,
since we already do that in dspi_setup_accel which we now call.
Update message->actual_length before submitting the DMA transfer.

Changes in v3:
Pretty much re-did the patch. Before, dspi_setup_accel was called just
once at the beginning of dspi_dma_xfer. Now it is called in the while
loop. Everything else is just refactoring that follows along.

Changes in v2:
None.

 drivers/spi/spi-fsl-dspi.c | 86 ++++++++++++++------------------------
 1 file changed, 32 insertions(+), 54 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index be717776dd98..8f2b73cc6ed7 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -119,7 +119,6 @@ struct fsl_dspi_devtype_data {
 	enum dspi_trans_mode	trans_mode;
 	u8			max_clock_factor;
 	int			fifo_size;
-	int			dma_bufsize;
 };
 
 enum {
@@ -138,7 +137,6 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
 	[VF610] = {
 		.trans_mode		= DSPI_DMA_MODE,
 		.max_clock_factor	= 2,
-		.dma_bufsize		= 4096,
 		.fifo_size		= 4,
 	},
 	[LS1021A] = {
@@ -167,19 +165,16 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
 	},
 	[LS2080A] = {
 		.trans_mode		= DSPI_DMA_MODE,
-		.dma_bufsize		= 8,
 		.max_clock_factor	= 8,
 		.fifo_size		= 4,
 	},
 	[LS2085A] = {
 		.trans_mode		= DSPI_DMA_MODE,
-		.dma_bufsize		= 8,
 		.max_clock_factor	= 8,
 		.fifo_size		= 4,
 	},
 	[LX2160A] = {
 		.trans_mode		= DSPI_DMA_MODE,
-		.dma_bufsize		= 8,
 		.max_clock_factor	= 8,
 		.fifo_size		= 4,
 	},
@@ -191,9 +186,6 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
 };
 
 struct fsl_dspi_dma {
-	/* Length of transfer in words of dspi->fifo_size */
-	u32					curr_xfer_len;
-
 	u32					*tx_dma_buf;
 	struct dma_chan				*chan_tx;
 	dma_addr_t				tx_dma_phys;
@@ -352,7 +344,7 @@ static void dspi_rx_dma_callback(void *arg)
 	int i;
 
 	if (dspi->rx) {
-		for (i = 0; i < dma->curr_xfer_len; i++)
+		for (i = 0; i < dspi->words_in_flight; i++)
 			dspi_push_rx(dspi, dspi->dma->rx_dma_buf[i]);
 	}
 
@@ -366,12 +358,12 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 	int time_left;
 	int i;
 
-	for (i = 0; i < dma->curr_xfer_len; i++)
+	for (i = 0; i < dspi->words_in_flight; i++)
 		dspi->dma->tx_dma_buf[i] = dspi_pop_tx_pushr(dspi);
 
 	dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
 					dma->tx_dma_phys,
-					dma->curr_xfer_len *
+					dspi->words_in_flight *
 					DMA_SLAVE_BUSWIDTH_4_BYTES,
 					DMA_MEM_TO_DEV,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -389,7 +381,7 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 
 	dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
 					dma->rx_dma_phys,
-					dma->curr_xfer_len *
+					dspi->words_in_flight *
 					DMA_SLAVE_BUSWIDTH_4_BYTES,
 					DMA_DEV_TO_MEM,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -437,46 +429,42 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 	return 0;
 }
 
+static void dspi_setup_accel(struct fsl_dspi *dspi);
+
 static int dspi_dma_xfer(struct fsl_dspi *dspi)
 {
 	struct spi_message *message = dspi->cur_msg;
 	struct device *dev = &dspi->pdev->dev;
-	struct fsl_dspi_dma *dma = dspi->dma;
-	int curr_remaining_bytes;
-	int bytes_per_buffer;
 	int ret = 0;
 
-	curr_remaining_bytes = dspi->len;
-	bytes_per_buffer = dspi->devtype_data->dma_bufsize /
-			   dspi->devtype_data->fifo_size;
-	while (curr_remaining_bytes) {
-		/* Check if current transfer fits the DMA buffer */
-		dma->curr_xfer_len = curr_remaining_bytes /
-				     dspi->oper_word_size;
-		if (dma->curr_xfer_len > bytes_per_buffer)
-			dma->curr_xfer_len = bytes_per_buffer;
+	/*
+	 * dspi->len gets decremented by dspi_pop_tx_pushr in
+	 * dspi_next_xfer_dma_submit
+	 */
+	while (dspi->len) {
+		/* Figure out operational bits-per-word for this chunk */
+		dspi_setup_accel(dspi);
+
+		dspi->words_in_flight = dspi->len / dspi->oper_word_size;
+		if (dspi->words_in_flight > dspi->devtype_data->fifo_size)
+			dspi->words_in_flight = dspi->devtype_data->fifo_size;
+
+		message->actual_length += dspi->words_in_flight *
+					  dspi->oper_word_size;
 
 		ret = dspi_next_xfer_dma_submit(dspi);
 		if (ret) {
 			dev_err(dev, "DMA transfer failed\n");
-			goto exit;
-
-		} else {
-			const int len = dma->curr_xfer_len *
-					dspi->oper_word_size;
-			curr_remaining_bytes -= len;
-			message->actual_length += len;
-			if (curr_remaining_bytes < 0)
-				curr_remaining_bytes = 0;
+			break;
 		}
 	}
 
-exit:
 	return ret;
 }
 
 static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
 {
+	int dma_bufsize = dspi->devtype_data->fifo_size * 2;
 	struct device *dev = &dspi->pdev->dev;
 	struct dma_slave_config cfg;
 	struct fsl_dspi_dma *dma;
@@ -501,16 +489,16 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
 	}
 
 	dma->tx_dma_buf = dma_alloc_coherent(dma->chan_tx->device->dev,
-					     dspi->devtype_data->dma_bufsize,
-					     &dma->tx_dma_phys, GFP_KERNEL);
+					     dma_bufsize, &dma->tx_dma_phys,
+					     GFP_KERNEL);
 	if (!dma->tx_dma_buf) {
 		ret = -ENOMEM;
 		goto err_tx_dma_buf;
 	}
 
 	dma->rx_dma_buf = dma_alloc_coherent(dma->chan_rx->device->dev,
-					     dspi->devtype_data->dma_bufsize,
-					     &dma->rx_dma_phys, GFP_KERNEL);
+					     dma_bufsize, &dma->rx_dma_phys,
+					     GFP_KERNEL);
 	if (!dma->rx_dma_buf) {
 		ret = -ENOMEM;
 		goto err_rx_dma_buf;
@@ -547,12 +535,10 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
 
 err_slave_config:
 	dma_free_coherent(dma->chan_rx->device->dev,
-			  dspi->devtype_data->dma_bufsize,
-			  dma->rx_dma_buf, dma->rx_dma_phys);
+			  dma_bufsize, dma->rx_dma_buf, dma->rx_dma_phys);
 err_rx_dma_buf:
 	dma_free_coherent(dma->chan_tx->device->dev,
-			  dspi->devtype_data->dma_bufsize,
-			  dma->tx_dma_buf, dma->tx_dma_phys);
+			  dma_bufsize, dma->tx_dma_buf, dma->tx_dma_phys);
 err_tx_dma_buf:
 	dma_release_channel(dma->chan_tx);
 err_tx_channel:
@@ -566,6 +552,7 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
 
 static void dspi_release_dma(struct fsl_dspi *dspi)
 {
+	int dma_bufsize = dspi->devtype_data->fifo_size * 2;
 	struct fsl_dspi_dma *dma = dspi->dma;
 
 	if (!dma)
@@ -573,15 +560,13 @@ static void dspi_release_dma(struct fsl_dspi *dspi)
 
 	if (dma->chan_tx) {
 		dma_unmap_single(dma->chan_tx->device->dev, dma->tx_dma_phys,
-				 dspi->devtype_data->dma_bufsize,
-				 DMA_TO_DEVICE);
+				 dma_bufsize, DMA_TO_DEVICE);
 		dma_release_channel(dma->chan_tx);
 	}
 
 	if (dma->chan_rx) {
 		dma_unmap_single(dma->chan_rx->device->dev, dma->rx_dma_phys,
-				 dspi->devtype_data->dma_bufsize,
-				 DMA_FROM_DEVICE);
+				 dma_bufsize, DMA_FROM_DEVICE);
 		dma_release_channel(dma->chan_rx);
 	}
 }
@@ -833,7 +818,7 @@ static void dspi_setup_accel(struct fsl_dspi *dspi)
 	dspi->oper_word_size = DIV_ROUND_UP(dspi->oper_bits_per_word, 8);
 
 	/*
-	 * Update CTAR here (code is common for both EOQ and XSPI modes).
+	 * Update CTAR here (code is common for EOQ, XSPI and DMA modes).
 	 * We will update CTARE in the portion specific to XSPI, when we
 	 * also know the preload value (DTCP).
 	 */
@@ -960,13 +945,6 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 		regmap_update_bits(dspi->regmap, SPI_MCR,
 				   SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF,
 				   SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF);
-		/*
-		 * Static CTAR setup for modes that don't dynamically adjust it
-		 * via dspi_setup_accel (aka for DMA)
-		 */
-		regmap_write(dspi->regmap, SPI_CTAR(0),
-			     dspi->cur_chip->ctar_val |
-			     SPI_FRAME_BITS(transfer->bits_per_word));
 
 		spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,
 				       dspi->progress, !dspi->irq);
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2020-03-14 22:43   ` [PATCH v3 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight Vladimir Oltean
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

If dspi->words_in_flight is populated with the hardware FIFO size,
then in dspi_fifo_read it will attempt to read more data at the end of a
buffer that is not a multiple of 16 bytes in length. It will probably
time out attempting to do so.

So limit the num_fifo_entries variable to the actual number of FIFO
entries that is going to be used.

Fixes: d59c90a2400f ("spi: spi-fsl-dspi: Convert TCFQ users to XSPI FIFO mode")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 8f2b73cc6ed7..51224b772680 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -739,13 +739,16 @@ static void dspi_eoq_fifo_write(struct fsl_dspi *dspi)
 	int num_fifo_entries = dspi->devtype_data->fifo_size;
 	u16 xfer_cmd = dspi->tx_cmd;
 
+	if (num_fifo_entries * dspi->oper_word_size > dspi->len)
+		num_fifo_entries = dspi->len / dspi->oper_word_size;
+
 	dspi->words_in_flight = num_fifo_entries;
 
 	/* Fill TX FIFO with as many transfers as possible */
-	while (dspi->len && num_fifo_entries--) {
+	while (num_fifo_entries--) {
 		dspi->tx_cmd = xfer_cmd;
 		/* Request EOQF for last transfer in FIFO */
-		if (dspi->len == dspi->oper_word_size || num_fifo_entries == 0)
+		if (num_fifo_entries == 0)
 			dspi->tx_cmd |= SPI_PUSHR_CMD_EOQ;
 		/* Write combined TX FIFO and CMD FIFO entry */
 		dspi_pushr_write(dspi);
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
       [not found]     ` <20200314224340.1544-6-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2020-03-14 22:43   ` [PATCH v3 08/12] spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code path Vladimir Oltean
                     ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

dspi->words_in_flight is a variable populated in the *_write functions
and used in the dspi_fifo_read function. It is also used in
dspi_fifo_write, immediately after transmission, to update the
message->actual_length variable used by higher layers such as spi-mem
for integrity checking.

But it may happen that the IRQ which calls dspi_fifo_read to be
triggered before the updating of message->actual_length takes place. In
that case, dspi_fifo_read will decrement dspi->words_in_flight to -1,
and that will cause an invalid modification of message->actual_length.

Make the simplest fix possible: don't decrement the actual shared
variable in dspi->words_in_flight from dspi_fifo_read, but actually a
copy of it which is on stack.

Suggested-by: Michael Walle <michael-QKn5cuLxLXY@public.gmane.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 51224b772680..3ac004aa2abd 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -765,8 +765,10 @@ static u32 dspi_popr_read(struct fsl_dspi *dspi)
 
 static void dspi_fifo_read(struct fsl_dspi *dspi)
 {
+	int num_fifo_entries = dspi->words_in_flight;
+
 	/* Read one FIFO entry and push to rx buffer */
-	while (dspi->words_in_flight--)
+	while (num_fifo_entries--)
 		dspi_push_rx(dspi, dspi_popr_read(dspi));
 }
 
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
  2020-03-14 22:43 [PATCH v3 00/12] NXP DSPI bugfixes and support for LS1028A Vladimir Oltean
  2020-03-14 22:43 ` [PATCH v3 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
@ 2020-03-14 22:43 ` Vladimir Oltean
  2020-03-16 12:26   ` Mark Brown
  2020-03-14 22:43 ` [PATCH v3 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode Vladimir Oltean
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  3 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie
  Cc: linux-spi, linux-kernel, shawnguo, robh+dt, mark.rutland,
	devicetree, eha, angelo, andrew.smirnov, gustavo, weic, mhosny,
	michael, peng.ma

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Currently the driver puts the process in interruptible sleep waiting for
the interrupt train to finish transfer to/from the tx_buf and rx_buf.

But exiting the process with ctrl-c may make the kernel panic: the
wait_event_interruptible call will return -ERESTARTSYS, which a proper
driver implementation is perhaps supposed to handle, but nonetheless
this one doesn't, and aborts the transfer altogether.

Actually when the task is interrupted, there is still a high chance that
the dspi_interrupt is still triggering. And if dspi_transfer_one_message
returns execution all the way to the spi_device driver, that can free
the spi_message and spi_transfer structures, leaving the interrupts to
access a freed tx_buf and rx_buf.

hexdump -C /dev/mtd0
00000000  00 75 68 75 0a ff ff ff  ff ff ff ff ff ff ff ff
|.uhu............|
00000010  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff
|................|
*
^C[   38.495955] fsl-dspi 2120000.spi: Waiting for transfer to complete failed!
[   38.503097] spi_master spi2: failed to transfer one message from queue
[   38.509729] Unable to handle kernel paging request at virtual address ffff800095ab3377
[   38.517676] Mem abort info:
[   38.520474]   ESR = 0x96000045
[   38.523533]   EC = 0x25: DABT (current EL), IL = 32 bits
[   38.528861]   SET = 0, FnV = 0
[   38.531921]   EA = 0, S1PTW = 0
[   38.535067] Data abort info:
[   38.537952]   ISV = 0, ISS = 0x00000045
[   38.541797]   CM = 0, WnR = 1
[   38.544771] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000082621000
[   38.551494] [ffff800095ab3377] pgd=00000020fffff003, p4d=00000020fffff003, pud=0000000000000000
[   38.560229] Internal error: Oops: 96000045 [#1] PREEMPT SMP
[   38.565819] Modules linked in:
[   38.568882] CPU: 0 PID: 2729 Comm: hexdump Not tainted 5.6.0-rc4-next-20200306-00052-gd8730cdc8a0b-dirty #193
[   38.578834] Hardware name: Kontron SMARC-sAL28 (Single PHY) on SMARC Eval 2.0 carrier (DT)
[   38.587129] pstate: 20000085 (nzCv daIf -PAN -UAO)
[   38.591941] pc : ktime_get_real_ts64+0x3c/0x110
[   38.596487] lr : spi_take_timestamp_pre+0x40/0x90
[   38.601203] sp : ffff800010003d90
[   38.604525] x29: ffff800010003d90 x28: ffff80001200e000
[   38.609854] x27: ffff800011da9000 x26: ffff002079c40400
[   38.615184] x25: ffff8000117fe018 x24: ffff800011daa1a0
[   38.620513] x23: ffff800015ab3860 x22: ffff800095ab3377
[   38.625841] x21: 000000000000146e x20: ffff8000120c3000
[   38.631170] x19: ffff0020795f6e80 x18: ffff800011da9948
[   38.636498] x17: 0000000000000000 x16: 0000000000000000
[   38.641826] x15: ffff800095ab3377 x14: 0720072007200720
[   38.647155] x13: 0720072007200765 x12: 0775076507750771
[   38.652483] x11: 0720076d076f0772 x10: 0000000000000040
[   38.657812] x9 : ffff8000108e2100 x8 : ffff800011dcabe8
[   38.663139] x7 : 0000000000000000 x6 : ffff800015ab3a60
[   38.668468] x5 : 0000000007200720 x4 : ffff800095ab3377
[   38.673796] x3 : 0000000000000000 x2 : 0000000000000ab0
[   38.679125] x1 : ffff800011daa000 x0 : 0000000000000026
[   38.684454] Call trace:
[   38.686905]  ktime_get_real_ts64+0x3c/0x110
[   38.691100]  spi_take_timestamp_pre+0x40/0x90
[   38.695470]  dspi_fifo_write+0x58/0x2c0
[   38.699315]  dspi_interrupt+0xbc/0xd0
[   38.702987]  __handle_irq_event_percpu+0x78/0x2c0
[   38.707706]  handle_irq_event_percpu+0x3c/0x90
[   38.712161]  handle_irq_event+0x4c/0xd0
[   38.716008]  handle_fasteoi_irq+0xbc/0x170
[   38.720115]  generic_handle_irq+0x2c/0x40
[   38.724135]  __handle_domain_irq+0x68/0xc0
[   38.728243]  gic_handle_irq+0xc8/0x160
[   38.732000]  el1_irq+0xb8/0x180
[   38.735149]  spi_nor_spimem_read_data+0xe0/0x140
[   38.739779]  spi_nor_read+0xc4/0x120
[   38.743364]  mtd_read_oob+0xa8/0xc0
[   38.746860]  mtd_read+0x4c/0x80
[   38.750007]  mtdchar_read+0x108/0x2a0
[   38.753679]  __vfs_read+0x20/0x50
[   38.757002]  vfs_read+0xa4/0x190
[   38.760237]  ksys_read+0x6c/0xf0
[   38.763471]  __arm64_sys_read+0x20/0x30
[   38.767319]  el0_svc_common.constprop.3+0x90/0x160
[   38.772125]  do_el0_svc+0x28/0x90
[   38.775449]  el0_sync_handler+0x118/0x190
[   38.779468]  el0_sync+0x140/0x180
[   38.782793] Code: 91000294 1400000f d50339bf f9405e80 (f90002c0)
[   38.788910] ---[ end trace 55da560db4d6bef7 ]---
[   38.793540] Kernel panic - not syncing: Fatal exception in interrupt
[   38.799914] SMP: stopping secondary CPUs
[   38.803849] Kernel Offset: disabled
[   38.807344] CPU features: 0x10002,20006008
[   38.811451] Memory Limit: none
[   38.814513] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

So it is clear that the "interruptible" part isn't handled correctly.
When the process receives a signal, one could either attempt a clean
abort (which appears to be difficult with this hardware) or just keep
restarting the sleep until the wait queue really completes. But checking
in a loop for -ERESTARTSYS is a bit too complicated for this driver, so
just make the sleep uninterruptible, to avoid all that nonsense.

The wait queue was actually restructured as a completion, after polling
other drivers for the most "popular" approach.

Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
Reported-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v4:
Removed the dspi_disable_interrupts and dspi_enable_interrupts approach
and replaced it with this method.

Changes in v3:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 3ac004aa2abd..78c891e441cd 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -218,8 +218,7 @@ struct fsl_dspi {
 	u16					tx_cmd;
 	const struct fsl_dspi_devtype_data	*devtype_data;
 
-	wait_queue_head_t			waitq;
-	u32					waitflags;
+	struct completion			xfer_done;
 
 	struct fsl_dspi_dma			*dma;
 
@@ -899,10 +898,8 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 	if (!(spi_sr & (SPI_SR_EOQF | SPI_SR_CMDTCF)))
 		return IRQ_NONE;
 
-	if (dspi_rxtx(dspi) == 0) {
-		dspi->waitflags = 1;
-		wake_up_interruptible(&dspi->waitq);
-	}
+	if (dspi_rxtx(dspi) == 0)
+		complete(&dspi->xfer_done);
 
 	return IRQ_HANDLED;
 }
@@ -982,13 +979,9 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 				status = dspi_poll(dspi);
 			} while (status == -EINPROGRESS);
 		} else if (trans_mode != DSPI_DMA_MODE) {
-			status = wait_event_interruptible(dspi->waitq,
-							  dspi->waitflags);
-			dspi->waitflags = 0;
+			wait_for_completion(&dspi->xfer_done);
+			reinit_completion(&dspi->xfer_done);
 		}
-		if (status)
-			dev_err(&dspi->pdev->dev,
-				"Waiting for transfer to complete failed!\n");
 
 		spi_transfer_delay_exec(transfer);
 	}
@@ -1368,7 +1361,7 @@ static int dspi_probe(struct platform_device *pdev)
 		goto out_clk_put;
 	}
 
-	init_waitqueue_head(&dspi->waitq);
+	init_completion(&dspi->xfer_done);
 
 poll_mode:
 
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode
  2020-03-14 22:43 [PATCH v3 00/12] NXP DSPI bugfixes and support for LS1028A Vladimir Oltean
  2020-03-14 22:43 ` [PATCH v3 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
  2020-03-14 22:43 ` [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion Vladimir Oltean
@ 2020-03-14 22:43 ` Vladimir Oltean
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  3 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie
  Cc: linux-spi, linux-kernel, shawnguo, robh+dt, mark.rutland,
	devicetree, eha, angelo, andrew.smirnov, gustavo, weic, mhosny,
	michael, peng.ma

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The driver does not create the dspi->dma structure unless operating in
DSPI_DMA_MODE, so it makes sense to check for that.

Fixes: f4b323905d8b ("spi: Introduce dspi_slave_abort() function for NXP's dspi SPI driver")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v4:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 78c891e441cd..a8aef5690c5d 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1201,8 +1201,10 @@ static int dspi_slave_abort(struct spi_master *master)
 	 * Terminate all pending DMA transactions for the SPI working
 	 * in SLAVE mode.
 	 */
-	dmaengine_terminate_sync(dspi->dma->chan_rx);
-	dmaengine_terminate_sync(dspi->dma->chan_tx);
+	if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
+		dmaengine_terminate_sync(dspi->dma->chan_rx);
+		dmaengine_terminate_sync(dspi->dma->chan_tx);
+	}
 
 	/* Clear the internal DSPI RX and TX FIFO buffers */
 	regmap_update_bits(dspi->regmap, SPI_MCR,
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 08/12] spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code path
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (3 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message Vladimir Oltean
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

Interrupts are not necessary for DMA functionality, since the completion
event is provided by the DMA driver.

But if the driver fails to request the IRQ defined in the device tree,
it will call dspi_poll which would make the driver hang waiting for data
to become available in the RX FIFO.

Fixes: c55be3059159 ("spi: spi-fsl-dspi: Use poll mode in case the platform IRQ is missing")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index a8aef5690c5d..eab4929330e1 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -974,13 +974,15 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 			goto out;
 		}
 
-		if (!dspi->irq) {
-			do {
-				status = dspi_poll(dspi);
-			} while (status == -EINPROGRESS);
-		} else if (trans_mode != DSPI_DMA_MODE) {
-			wait_for_completion(&dspi->xfer_done);
-			reinit_completion(&dspi->xfer_done);
+		if (trans_mode != DSPI_DMA_MODE) {
+			if (dspi->irq) {
+				wait_for_completion(&dspi->xfer_done);
+				reinit_completion(&dspi->xfer_done);
+			} else {
+				do {
+					status = dspi_poll(dspi);
+				} while (status == -EINPROGRESS);
+			}
 		}
 
 		spi_transfer_delay_exec(transfer);
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (4 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 08/12] spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code path Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 10/12] spi: spi-fsl-dspi: Add support for LS1028A Vladimir Oltean
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

The operating mode (DMA, XSPI, EOQ) is not going to change across the
lifetime of the device. So it makes no sense to keep writing to SPI_RSER
on each message. Move this configuration to dspi_init instead.

Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
Patch is new.

 drivers/spi/spi-fsl-dspi.c | 55 ++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index eab4929330e1..86255d38ffcf 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -909,7 +909,6 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 {
 	struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);
 	struct spi_device *spi = message->spi;
-	enum dspi_trans_mode trans_mode;
 	struct spi_transfer *transfer;
 	int status = 0;
 
@@ -951,30 +950,11 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 		spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,
 				       dspi->progress, !dspi->irq);
 
-		trans_mode = dspi->devtype_data->trans_mode;
-		switch (trans_mode) {
-		case DSPI_EOQ_MODE:
-			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
-			dspi_fifo_write(dspi);
-			break;
-		case DSPI_XSPI_MODE:
-			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_CMDTCFE);
-			dspi_fifo_write(dspi);
-			break;
-		case DSPI_DMA_MODE:
-			regmap_write(dspi->regmap, SPI_RSER,
-				     SPI_RSER_TFFFE | SPI_RSER_TFFFD |
-				     SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+		if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
 			status = dspi_dma_xfer(dspi);
-			break;
-		default:
-			dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
-				trans_mode);
-			status = -EINVAL;
-			goto out;
-		}
+		} else {
+			dspi_fifo_write(dspi);
 
-		if (trans_mode != DSPI_DMA_MODE) {
 			if (dspi->irq) {
 				wait_for_completion(&dspi->xfer_done);
 				reinit_completion(&dspi->xfer_done);
@@ -984,11 +964,12 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
 				} while (status == -EINPROGRESS);
 			}
 		}
+		if (status)
+			break;
 
 		spi_transfer_delay_exec(transfer);
 	}
 
-out:
 	message->status = status;
 	spi_finalize_current_message(ctlr);
 
@@ -1179,7 +1160,7 @@ static const struct regmap_config dspi_xspi_regmap_config[] = {
 	},
 };
 
-static void dspi_init(struct fsl_dspi *dspi)
+static int dspi_init(struct fsl_dspi *dspi)
 {
 	unsigned int mcr;
 
@@ -1193,6 +1174,26 @@ static void dspi_init(struct fsl_dspi *dspi)
 
 	regmap_write(dspi->regmap, SPI_MCR, mcr);
 	regmap_write(dspi->regmap, SPI_SR, SPI_SR_CLEAR);
+
+	switch (dspi->devtype_data->trans_mode) {
+	case DSPI_EOQ_MODE:
+		regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
+		break;
+	case DSPI_XSPI_MODE:
+		regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_CMDTCFE);
+		break;
+	case DSPI_DMA_MODE:
+		regmap_write(dspi->regmap, SPI_RSER,
+			     SPI_RSER_TFFFE | SPI_RSER_TFFFD |
+			     SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+		break;
+	default:
+		dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
+			dspi->devtype_data->trans_mode);
+		return -EINVAL;
+	}
+
+	return 0;
 }
 
 static int dspi_slave_abort(struct spi_master *master)
@@ -1348,7 +1349,9 @@ static int dspi_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_ctlr_put;
 
-	dspi_init(dspi);
+	ret = dspi_init(dspi);
+	if (ret)
+		goto out_clk_put;
 
 	dspi->irq = platform_get_irq(pdev, 0);
 	if (dspi->irq <= 0) {
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 10/12] spi: spi-fsl-dspi: Add support for LS1028A
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (5 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 11/12] arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

This is similar to the DSPI instantiation on LS1028A, except that:
 - The A-011218 erratum has been fixed, so DMA works
 - The endianness is different, which has implications on XSPI mode

Some benchmarking with the following command:

spidev_test --device /dev/spidev2.0 --bpw 8 --size 256 --cpha --iter 10000000 --speed 20000000

shows that in DMA mode, it can achieve around 2400 kbps, and in XSPI
mode, the same command goes up to 4700 kbps. This is somewhat to be
expected, since the DMA buffer size is extremely small at 8 bytes, the
winner becomes whomever can prepare the buffers for transmission
quicker, and DMA mode has higher overhead there. So XSPI FIFO mode has
been chosen as the operating mode for this chip.

Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
None.

Changes in v3:
Removed the dma_bufsize variable (obsoleted by 03/12).

Changes in v2:
Switch to DSPI_XSPI_MODE.

 drivers/spi/spi-fsl-dspi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 86255d38ffcf..b6d7e6f383ec 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -124,6 +124,7 @@ struct fsl_dspi_devtype_data {
 enum {
 	LS1021A,
 	LS1012A,
+	LS1028A,
 	LS1043A,
 	LS1046A,
 	LS2080A,
@@ -151,6 +152,11 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
 		.max_clock_factor	= 8,
 		.fifo_size		= 16,
 	},
+	[LS1028A] = {
+		.trans_mode		= DSPI_XSPI_MODE,
+		.max_clock_factor	= 8,
+		.fifo_size		= 4,
+	},
 	[LS1043A] = {
 		/* Has A-011218 DMA erratum */
 		.trans_mode		= DSPI_XSPI_MODE,
@@ -1059,6 +1065,9 @@ static const struct of_device_id fsl_dspi_dt_ids[] = {
 	}, {
 		.compatible = "fsl,ls1012a-dspi",
 		.data = &devtype_data[LS1012A],
+	}, {
+		.compatible = "fsl,ls1028a-dspi",
+		.data = &devtype_data[LS1028A],
 	}, {
 		.compatible = "fsl,ls1043a-dspi",
 		.data = &devtype_data[LS1043A],
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 11/12] arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (6 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 10/12] spi: spi-fsl-dspi: Add support for LS1028A Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  2020-03-14 22:43   ` [PATCH v3 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

LS1028A has a functional connection to the eDMA module. Even if the
spi-fsl-dspi.c driver is not using DMA for LS1028A now, define the slots
in the DMAMUX for connecting the eDMA channels to the 3 DSPI
controllers.

Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
None.

Changes in v3:
None.

Changes in v2:
None.

 arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 515e0a1b934f..18155273a46e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -298,6 +298,8 @@
 			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
 			clock-names = "dspi";
 			clocks = <&clockgen 4 1>;
+			dmas = <&edma0 0 62>, <&edma0 0 60>;
+			dma-names = "tx", "rx";
 			spi-num-chipselects = <4>;
 			little-endian;
 			status = "disabled";
@@ -311,6 +313,8 @@
 			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
 			clock-names = "dspi";
 			clocks = <&clockgen 4 1>;
+			dmas = <&edma0 0 58>, <&edma0 0 56>;
+			dma-names = "tx", "rx";
 			spi-num-chipselects = <4>;
 			little-endian;
 			status = "disabled";
@@ -324,6 +328,8 @@
 			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
 			clock-names = "dspi";
 			clocks = <&clockgen 4 1>;
+			dmas = <&edma0 0 54>, <&edma0 0 2>;
+			dma-names = "tx", "rx";
 			spi-num-chipselects = <3>;
 			little-endian;
 			status = "disabled";
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
       [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (7 preceding siblings ...)
  2020-03-14 22:43   ` [PATCH v3 11/12] arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers Vladimir Oltean
@ 2020-03-14 22:43   ` Vladimir Oltean
  8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-14 22:43 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>

For debugging, it is useful to have access to the DSPI controller
signals. On the reference design board, these are exported to either the
mikroBUS1 or mikroBUS2 connector (according to the CPLD register
BRDCFG3[SPI3]).

Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v4:
None.

Changes in v3:
None.

Changes in v2:
Change compatible string for spidev node.

 arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 6d05b76c2c7a..0d27b5667b8c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -83,6 +83,20 @@
 	};
 };
 
+&dspi2 {
+	bus-num = <2>;
+	status = "okay";
+
+	/* mikroBUS1 */
+	spidev@0 {
+		compatible = "rohm,dh2228fv";
+		spi-max-frequency = <20000000>;
+		fsl,spi-cs-sck-delay = <100>;
+		fsl,spi-sck-cs-delay = <100>;
+		reg = <0>;
+	};
+};
+
 &esdhc {
 	sd-uhs-sdr104;
 	sd-uhs-sdr50;
-- 
2.17.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
  2020-03-14 22:43 ` [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion Vladimir Oltean
@ 2020-03-16 12:26   ` Mark Brown
       [not found]     ` <20200316122613.GE5010-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Brown @ 2020-03-16 12:26 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-spi, linux-kernel, shawnguo, robh+dt, mark.rutland,
	devicetree, eha, angelo, andrew.smirnov, gustavo, weic, mhosny,
	michael, peng.ma

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Sun, Mar 15, 2020 at 12:43:34AM +0200, Vladimir Oltean wrote:

> The wait queue was actually restructured as a completion, after polling
> other drivers for the most "popular" approach.

> Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")

Fixes should generally go at the start of the series to make sure that
they can be applied without any dependencies on the rest of the series
and sent to mainline before the merge window.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
       [not found]     ` <20200316122613.GE5010-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2020-03-16 12:29       ` Vladimir Oltean
       [not found]         ` <CA+h21hqRV+HmAz4QGyH9ZtcFWzeCKczitcn+mfTdwAC7ZobdDw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-16 12:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, lkml, Shawn Guo, Rob Herring,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal,
	angelo-BIYBQhTR83Y, andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
	peng.ma-3arQi8VN3Tc

Hi Mark,

On Mon, 16 Mar 2020 at 14:26, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Sun, Mar 15, 2020 at 12:43:34AM +0200, Vladimir Oltean wrote:
>
> > The wait queue was actually restructured as a completion, after polling
> > other drivers for the most "popular" approach.
>
> > Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
>
> Fixes should generally go at the start of the series to make sure that
> they can be applied without any dependencies on the rest of the series
> and sent to mainline before the merge window.

Correct, the real problem is that I forgot to add a Fixes: tag for
patch 5. I'll do that now.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight
       [not found]     ` <20200314224340.1544-6-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2020-03-16 12:35       ` Vladimir Oltean
  0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-16 12:35 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
	mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
	peng.ma-3arQi8VN3Tc

Mar 15, 2020 12:44:02 AM Vladimir Oltean <olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:

> From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
>
> dspi->words_in_flight is a variable populated in the *_write functions
> and used in the dspi_fifo_read function. It is also used in
> dspi_fifo_write, immediately after transmission, to update the
> message->actual_length variable used by higher layers such as spi-mem
> for integrity checking.
>
> But it may happen that the IRQ which calls dspi_fifo_read to be
> triggered before the updating of message->actual_length takes place. In
> that case, dspi_fifo_read will decrement dspi->words_in_flight to -1,
> and that will cause an invalid modification of message->actual_length.
>
> Make the simplest fix possible: don't decrement the actual shared
> variable in dspi->words_in_flight from dspi_fifo_read, but actually a
> copy of it which is on stack.
>
> Suggested-by: Michael Walle <michael-QKn5cuLxLXY@public.gmane.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
> ---
> Changes in v4:
> Patch is new.
>
> drivers/spi/spi-fsl-dspi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index 51224b772680..3ac004aa2abd 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -765,8 +765,10 @@ static u32 dspi_popr_read(struct fsl_dspi *dspi)
>
> static void dspi_fifo_read(struct fsl_dspi *dspi)
> {
> + int num_fifo_entries = dspi->words_in_flight;
> +
> /* Read one FIFO entry and push to rx buffer */
> - while (dspi->words_in_flight--)
> + while (num_fifo_entries--)
> dspi_push_rx(dspi, dspi_popr_read(dspi));
> }
>
> --
> 2.17.1
>
>

Fixes: d59c90a2400f ("spi: spi-fsl-dspi: Convert TCFQ users to XSPI FIFO mode")

Patchwork should know to pick up this tag.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
       [not found]         ` <CA+h21hqRV+HmAz4QGyH9ZtcFWzeCKczitcn+mfTdwAC7ZobdDw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2020-03-16 12:49           ` Mark Brown
  2020-03-16 13:00             ` Vladimir Oltean
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Brown @ 2020-03-16 12:49 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, lkml, Shawn Guo, Rob Herring,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Esben Haabendal,
	angelo-BIYBQhTR83Y, andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
	Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
	peng.ma-3arQi8VN3Tc

[-- Attachment #1: Type: text/plain, Size: 455 bytes --]

On Mon, Mar 16, 2020 at 02:29:09PM +0200, Vladimir Oltean wrote:

> Correct, the real problem is that I forgot to add a Fixes: tag for
> patch 5. I'll do that now.

OK.  The series otherwise looked fine but I'll wait for testing.
Michael, if there's issues remaining it might be good to get some
Tested-bys for the patches prior to whatever's broken so we can get
those fixes in (but obviously verifying that is work so only if you 
have time).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
  2020-03-16 12:49           ` Mark Brown
@ 2020-03-16 13:00             ` Vladimir Oltean
       [not found]               ` <CA+h21hpoHGuDwpOqtWJFO7+0mQVUrmcBLW7nnGq6dt3QU5axfw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-16 13:00 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, lkml, Shawn Guo, Rob Herring, Mark Rutland,
	devicetree, Esben Haabendal, angelo, andrew.smirnov,
	Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
	peng.ma

On Mon, 16 Mar 2020 at 14:49, Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Mar 16, 2020 at 02:29:09PM +0200, Vladimir Oltean wrote:
>
> > Correct, the real problem is that I forgot to add a Fixes: tag for
> > patch 5. I'll do that now.
>
> OK.  The series otherwise looked fine but I'll wait for testing.
> Michael, if there's issues remaining it might be good to get some
> Tested-bys for the patches prior to whatever's broken so we can get
> those fixes in (but obviously verifying that is work so only if you
> have time).

This time I verified with a protocol analyzer all transfer lengths
from 1 all the way to 256, with this script:

#!/bin/bash

buf=''

for i in $(seq 0 255); do
»       buf="${buf}\x$(printf '%02x' ${i})"
»       spidev_test --device /dev/spidev2.0 --bpw 8 --cpha --speed
5000000 -p "${buf}"
done

It looked fine as far as I could tell, and also the problems
surrounding Ctrl-C are no longer present. Nonetheless it would be good
if Michael could confirm, but I know that he's very busy too so it's
understandable if he can no longer spend time on this.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
       [not found]               ` <CA+h21hpoHGuDwpOqtWJFO7+0mQVUrmcBLW7nnGq6dt3QU5axfw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2020-03-16 13:25                 ` Michael Walle
  2020-03-16 16:23                   ` Vladimir Oltean
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Walle @ 2020-03-16 13:25 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, lkml, Shawn Guo,
	Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Esben Haabendal, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w, Gustavo A. R. Silva,
	Wei Chen, Mohamed Hosny, peng.ma-3arQi8VN3Tc

Am 2020-03-16 14:00, schrieb Vladimir Oltean:
> On Mon, 16 Mar 2020 at 14:49, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> 
>> On Mon, Mar 16, 2020 at 02:29:09PM +0200, Vladimir Oltean wrote:
>> 
>> > Correct, the real problem is that I forgot to add a Fixes: tag for
>> > patch 5. I'll do that now.
>> 
>> OK.  The series otherwise looked fine but I'll wait for testing.
>> Michael, if there's issues remaining it might be good to get some
>> Tested-bys for the patches prior to whatever's broken so we can get
>> those fixes in (but obviously verifying that is work so only if you
>> have time).

I'm just about to test it. While my former "cat /dev/mtdN > /dev/null"
is working. I had the impression that it was slower, so I tried to test
it with dd now and a known chunk size.. only to find out that it is
still not working:

# dmesg|grep spi
[    1.894891] spi-nor spi1.0: w25q128fw (16384 Kbytes)
..
# time cat /dev/mtd0 > /dev/null
real    0m 30.73s
user    0m 0.00s
sys     0m 1.02s
# dd if=/dev/mtd0 of=/dev/null bs=64
262144+0 records in
262144+0 records out
# dd if=/dev/mtd0 of=/dev/null bs=64
262144+0 records in
262144+0 records out
# dd if=/dev/mtd0 of=/dev/null bs=64
dd: /dev/mtd0: Input/output error

I also wanted to test how it behaves if there are multiple processes
access the /dev/mtdN device. I haven't found the time to dig into
the call chain if see if there is any locking. Because what happens
if transfer_one_message() is called twice at the same time from two
different processes?

> 
> This time I verified with a protocol analyzer all transfer lengths
> from 1 all the way to 256, with this script:
> 
> #!/bin/bash
> 
> buf=''
> 
> for i in $(seq 0 255); do
> »       buf="${buf}\x$(printf '%02x' ${i})"
> »       spidev_test --device /dev/spidev2.0 --bpw 8 --cpha --speed
> 5000000 -p "${buf}"
> done
> 
> It looked fine as far as I could tell, and also the problems
> surrounding Ctrl-C are no longer present. Nonetheless it would be good
> if Michael could confirm, but I know that he's very busy too so it's
> understandable if he can no longer spend time on this.

I'm working on it ;)

-michael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
  2020-03-16 13:25                 ` Michael Walle
@ 2020-03-16 16:23                   ` Vladimir Oltean
       [not found]                     ` <CA+h21hqt7Xe1LrSDsCVS8zqunQp2tKGhmHDraMirxL595go4nA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-16 16:23 UTC (permalink / raw)
  To: Michael Walle
  Cc: Mark Brown, linux-spi, lkml, Shawn Guo, Rob Herring,
	Mark Rutland, devicetree, Esben Haabendal, angelo,
	andrew.smirnov, Gustavo A. R. Silva, Wei Chen, Mohamed Hosny,
	peng.ma

On Mon, 16 Mar 2020 at 15:25, Michael Walle <michael@walle.cc> wrote:
>
> Am 2020-03-16 14:00, schrieb Vladimir Oltean:
> > On Mon, 16 Mar 2020 at 14:49, Mark Brown <broonie@kernel.org> wrote:
> >>
> >> On Mon, Mar 16, 2020 at 02:29:09PM +0200, Vladimir Oltean wrote:
> >>
> >> > Correct, the real problem is that I forgot to add a Fixes: tag for
> >> > patch 5. I'll do that now.
> >>
> >> OK.  The series otherwise looked fine but I'll wait for testing.
> >> Michael, if there's issues remaining it might be good to get some
> >> Tested-bys for the patches prior to whatever's broken so we can get
> >> those fixes in (but obviously verifying that is work so only if you
> >> have time).
>
> I'm just about to test it. While my former "cat /dev/mtdN > /dev/null"
> is working. I had the impression that it was slower, so I tried to test
> it with dd now and a known chunk size.. only to find out that it is
> still not working:
>
> # dmesg|grep spi
> [    1.894891] spi-nor spi1.0: w25q128fw (16384 Kbytes)
> ..
> # time cat /dev/mtd0 > /dev/null
> real    0m 30.73s
> user    0m 0.00s
> sys     0m 1.02s
> # dd if=/dev/mtd0 of=/dev/null bs=64
> 262144+0 records in
> 262144+0 records out
> # dd if=/dev/mtd0 of=/dev/null bs=64
> 262144+0 records in
> 262144+0 records out
> # dd if=/dev/mtd0 of=/dev/null bs=64
> dd: /dev/mtd0: Input/output error

I don't really have a SPI flash connected to DSPI on any LS1028A board.
Is this DMA or XSPI mode?

>
> I also wanted to test how it behaves if there are multiple processes
> access the /dev/mtdN device. I haven't found the time to dig into
> the call chain if see if there is any locking. Because what happens
> if transfer_one_message() is called twice at the same time from two
> different processes?
>

There is a mutex on the SPI bus, and therefore all variants of the
.transfer() call are operating under this lock protection, which
simplifies things quite a bit.

> >
> > This time I verified with a protocol analyzer all transfer lengths
> > from 1 all the way to 256, with this script:
> >
> > #!/bin/bash
> >
> > buf=''
> >
> > for i in $(seq 0 255); do
> > »       buf="${buf}\x$(printf '%02x' ${i})"
> > »       spidev_test --device /dev/spidev2.0 --bpw 8 --cpha --speed
> > 5000000 -p "${buf}"
> > done
> >
> > It looked fine as far as I could tell, and also the problems
> > surrounding Ctrl-C are no longer present. Nonetheless it would be good
> > if Michael could confirm, but I know that he's very busy too so it's
> > understandable if he can no longer spend time on this.
>
> I'm working on it ;)
>
> -michael

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
       [not found]                     ` <CA+h21hqt7Xe1LrSDsCVS8zqunQp2tKGhmHDraMirxL595go4nA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2020-03-16 17:15                       ` Michael Walle
       [not found]                         ` <8c22cb70b7c0acb6769e0c68540ab523-QKn5cuLxLXY@public.gmane.org>
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Walle @ 2020-03-16 17:15 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, lkml, Shawn Guo,
	Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Esben Haabendal, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w, Gustavo A. R. Silva,
	Wei Chen, Mohamed Hosny, peng.ma-3arQi8VN3Tc

Am 2020-03-16 17:23, schrieb Vladimir Oltean:
> On Mon, 16 Mar 2020 at 15:25, Michael Walle <michael-QKn5cuLxLXY@public.gmane.org> wrote:
>> 
>> Am 2020-03-16 14:00, schrieb Vladimir Oltean:
>> > On Mon, 16 Mar 2020 at 14:49, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >>
>> >> On Mon, Mar 16, 2020 at 02:29:09PM +0200, Vladimir Oltean wrote:
>> >>
>> >> > Correct, the real problem is that I forgot to add a Fixes: tag for
>> >> > patch 5. I'll do that now.
>> >>
>> >> OK.  The series otherwise looked fine but I'll wait for testing.
>> >> Michael, if there's issues remaining it might be good to get some
>> >> Tested-bys for the patches prior to whatever's broken so we can get
>> >> those fixes in (but obviously verifying that is work so only if you
>> >> have time).
>> 
>> I'm just about to test it. While my former "cat /dev/mtdN > /dev/null"
>> is working. I had the impression that it was slower, so I tried to 
>> test
>> it with dd now and a known chunk size.. only to find out that it is
>> still not working:
>> 
>> # dmesg|grep spi
>> [    1.894891] spi-nor spi1.0: w25q128fw (16384 Kbytes)
>> ..
>> # time cat /dev/mtd0 > /dev/null
>> real    0m 30.73s
>> user    0m 0.00s
>> sys     0m 1.02s
>> # dd if=/dev/mtd0 of=/dev/null bs=64
>> 262144+0 records in
>> 262144+0 records out
>> # dd if=/dev/mtd0 of=/dev/null bs=64
>> 262144+0 records in
>> 262144+0 records out
>> # dd if=/dev/mtd0 of=/dev/null bs=64
>> dd: /dev/mtd0: Input/output error
> 
> I don't really have a SPI flash connected to DSPI on any LS1028A board.

I'm already debugging it again.

> Is this DMA or XSPI mode?

XSPI mode. DMA mode looked good for now.

>> 
>> I also wanted to test how it behaves if there are multiple processes
>> access the /dev/mtdN device. I haven't found the time to dig into
>> the call chain if see if there is any locking. Because what happens
>> if transfer_one_message() is called twice at the same time from two
>> different processes?
>> 
> 
> There is a mutex on the SPI bus, and therefore all variants of the
> .transfer() call are operating under this lock protection, which
> simplifies things quite a bit.

Ok, thanks.

-michael

>> >
>> > This time I verified with a protocol analyzer all transfer lengths
>> > from 1 all the way to 256, with this script:
>> >
>> > #!/bin/bash
>> >
>> > buf=''
>> >
>> > for i in $(seq 0 255); do
>> > »       buf="${buf}\x$(printf '%02x' ${i})"
>> > »       spidev_test --device /dev/spidev2.0 --bpw 8 --cpha --speed
>> > 5000000 -p "${buf}"
>> > done
>> >
>> > It looked fine as far as I could tell, and also the problems
>> > surrounding Ctrl-C are no longer present. Nonetheless it would be good
>> > if Michael could confirm, but I know that he's very busy too so it's
>> > understandable if he can no longer spend time on this.
>> 
>> I'm working on it ;)
>> 
>> -michael
> 
> Thanks,
> -Vladimir

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion
       [not found]                         ` <8c22cb70b7c0acb6769e0c68540ab523-QKn5cuLxLXY@public.gmane.org>
@ 2020-03-17 11:24                           ` Michael Walle
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Walle @ 2020-03-17 11:24 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, lkml, Shawn Guo,
	Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Esben Haabendal, angelo-BIYBQhTR83Y,
	andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w, Gustavo A. R. Silva,
	Wei Chen, Mohamed Hosny, peng.ma-3arQi8VN3Tc

Am 2020-03-16 18:15, schrieb Michael Walle:
>>> dd: /dev/mtd0: Input/output error
>> 
>> I don't really have a SPI flash connected to DSPI on any LS1028A 
>> board.
> 
> I'm already debugging it again.

Ok now maybe a more experienced kernel developer could have a look at 
this.

(1) the error is EIO because the actual_length doesn't match the 
expected
     length
(2) I've put trace_printk() where the actual_length is modified:
     in dspi_fifo_write()

         /* Update total number of bytes that were transferred */
         bytes_sent = dspi->words_in_flight * dspi->oper_word_size;
         msg->actual_length += bytes_sent;
         trace_printk("msg=%px actual_length=%d bytes_sent=%d 
word_in_flight=%d oper_word_size=%d\n",
                         msg, msg->actual_length,
                         bytes_sent, dspi->words_in_flight,
                         dspi->oper_word_size);
         dspi->progress += bytes_sent / DIV_ROUND_UP(xfer->bits_per_word, 
8);

    At the start of the foreach transfer loop in 
dspi_transfer_one_message():

         list_for_each_entry(transfer, &message->transfers, 
transfer_list) {
                 trace_printk("%d transfer->length=%d\n", i++, 
transfer->len);
                 dspi->cur_transfer = transfer;

    And at the end of this function:


         }
         trace_printk("returning actual_length=%d\n", 
message->actual_length);

         message->status = status;
         spi_finalize_current_message(ctlr);

         return status;

(3) The trace is attached at the end of this mail. There is a good case 
and the
     bad one. What I've noticed (besides returning the wrong count, eg 
13, which
     seems to be the first one) is the preempt-depth which is always none 
in the
     bad case. In all transfers before, the preempt-depth is 1. The flags 
are
     usually "d.h1", sometimes "d.H1" and in case of the error "d.H.".
     I also don't know why the fifo_write() with actual_length=13 is 
traced after
     all the ones printed inside the interrupt handler. Also something 
seems to
     be off with the words_in_flight and oper_word_size in the bad case.

     So besides the flags.. I still think there is a race between the
     dspi_fifo_write() call in transfer_one_message and the interrupt 
handler
     for the shared data like words_in_flight and oper_word_size.


good one:
               dd-1259  [000] ....  3383.499863: 
dspi_transfer_one_message: 0 transfer->length=1
               dd-1259  [000] ....  3383.499886: dspi_fifo_write: 
msg=ffff800011973840 actual_length=1 bytes_sent=1 word_in_flight=1 
oper_word_size=1
               dd-1259  [000] ....  3383.499891: 
dspi_transfer_one_message: 1 transfer->length=3
               dd-1259  [000] ....  3383.499915: dspi_fifo_write: 
msg=ffff800011973840 actual_length=4 bytes_sent=3 word_in_flight=3 
oper_word_size=1
               dd-1259  [000] ....  3383.499920: 
dspi_transfer_one_message: 2 transfer->length=1
               dd-1259  [000] ....  3383.499940: dspi_fifo_write: 
msg=ffff800011973840 actual_length=5 bytes_sent=1 word_in_flight=1 
oper_word_size=1
               dd-1259  [000] ....  3383.499946: 
dspi_transfer_one_message: 3 transfer->length=64
               dd-1259  [000] d.h1  3383.499973: dspi_fifo_write: 
msg=ffff800011973840 actual_length=21 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.499993: dspi_fifo_write: 
msg=ffff800011973840 actual_length=29 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.500013: dspi_fifo_write: 
msg=ffff800011973840 actual_length=37 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.500032: dspi_fifo_write: 
msg=ffff800011973840 actual_length=45 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.500051: dspi_fifo_write: 
msg=ffff800011973840 actual_length=53 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.500070: dspi_fifo_write: 
msg=ffff800011973840 actual_length=61 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.h1  3383.500089: dspi_fifo_write: 
msg=ffff800011973840 actual_length=69 bytes_sent=8 word_in_flight=4 
oper_word_size=2
               dd-1259  [000] ....  3383.500107: dspi_fifo_write: 
msg=ffff800011973840 actual_length=13 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] ....  3383.500111: 
dspi_transfer_one_message: returning actual_length=69

bad one:
               dd-1259  [000] ....  3383.500149: 
dspi_transfer_one_message: 0 transfer->length=1
               dd-1259  [000] ....  3383.500172: dspi_fifo_write: 
msg=ffff800011973840 actual_length=1 bytes_sent=1 word_in_flight=1 
oper_word_size=1
               dd-1259  [000] ....  3383.500178: 
dspi_transfer_one_message: 1 transfer->length=3
               dd-1259  [000] ....  3383.500202: dspi_fifo_write: 
msg=ffff800011973840 actual_length=4 bytes_sent=3 word_in_flight=3 
oper_word_size=1
               dd-1259  [000] ....  3383.500207: 
dspi_transfer_one_message: 2 transfer->length=1
               dd-1259  [000] ....  3383.500227: dspi_fifo_write: 
msg=ffff800011973840 actual_length=5 bytes_sent=1 word_in_flight=1 
oper_word_size=1
               dd-1259  [000] ....  3383.500232: 
dspi_transfer_one_message: 3 transfer->length=64
               dd-1259  [000] d.H.  3383.500286: dspi_fifo_write: 
msg=ffff800011973840 actual_length=13 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500307: dspi_fifo_write: 
msg=ffff800011973840 actual_length=21 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500327: dspi_fifo_write: 
msg=ffff800011973840 actual_length=29 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500347: dspi_fifo_write: 
msg=ffff800011973840 actual_length=37 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500366: dspi_fifo_write: 
msg=ffff800011973840 actual_length=45 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500385: dspi_fifo_write: 
msg=ffff800011973840 actual_length=53 bytes_sent=8 word_in_flight=2 
oper_word_size=4
               dd-1259  [000] d.H.  3383.500404: dspi_fifo_write: 
msg=ffff800011973840 actual_length=61 bytes_sent=8 word_in_flight=4 
oper_word_size=2
               dd-1259  [000] ....  3383.500436: dspi_fifo_write: 
msg=ffff800011973840 actual_length=13 bytes_sent=8 word_in_flight=4 
oper_word_size=2
               dd-1259  [000] ....  3383.500441: 
dspi_transfer_one_message: returning actual_length=13


-michael

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-14 22:43 [PATCH v3 00/12] NXP DSPI bugfixes and support for LS1028A Vladimir Oltean
2020-03-14 22:43 ` [PATCH v3 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
2020-03-14 22:43 ` [PATCH v3 06/12] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion Vladimir Oltean
2020-03-16 12:26   ` Mark Brown
     [not found]     ` <20200316122613.GE5010-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2020-03-16 12:29       ` Vladimir Oltean
     [not found]         ` <CA+h21hqRV+HmAz4QGyH9ZtcFWzeCKczitcn+mfTdwAC7ZobdDw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-03-16 12:49           ` Mark Brown
2020-03-16 13:00             ` Vladimir Oltean
     [not found]               ` <CA+h21hpoHGuDwpOqtWJFO7+0mQVUrmcBLW7nnGq6dt3QU5axfw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-03-16 13:25                 ` Michael Walle
2020-03-16 16:23                   ` Vladimir Oltean
     [not found]                     ` <CA+h21hqt7Xe1LrSDsCVS8zqunQp2tKGhmHDraMirxL595go4nA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-03-16 17:15                       ` Michael Walle
     [not found]                         ` <8c22cb70b7c0acb6769e0c68540ab523-QKn5cuLxLXY@public.gmane.org>
2020-03-17 11:24                           ` Michael Walle
2020-03-14 22:43 ` [PATCH v3 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode Vladimir Oltean
     [not found] ` <20200314224340.1544-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-14 22:43   ` [PATCH v3 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight Vladimir Oltean
     [not found]     ` <20200314224340.1544-6-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-16 12:35       ` Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 08/12] spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code path Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 10/12] spi: spi-fsl-dspi: Add support for LS1028A Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 11/12] arm64: dts: ls1028a: Specify the DMA channels for the DSPI controllers Vladimir Oltean
2020-03-14 22:43   ` [PATCH v3 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean

Linux-SPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-spi/0 linux-spi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-spi linux-spi/ https://lore.kernel.org/linux-spi \
		linux-spi@vger.kernel.org
	public-inbox-index linux-spi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-spi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git