RE: [PATCH v8 00/13] add ecspi ERR009165 for i.mx6/7 soc family

[Robin Gong <yibin.gong@nxp.com>]

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

On 2020/06/03 Matthias Schiffer <matthias.schiffer@ew.tq-group.com> wrote:
> On Wed, 2020-06-03 at 09:50 +0000, Robin Gong wrote:
> > On 2020/06/03 Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > wrote:
> >  > On Thu, 2020-05-21 at 04:34 +0800, Robin Gong wrote:
> > > > There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO
> > > > transfer to be send twice in DMA mode. Please get more information
> > > > from:
> > > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> > > > .
> > > >
> > >
> > >
> nxp.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&amp;data=02%7C01%7C
> > > yibin.g
> > > >
> > >
> > >
> ong%40nxp.com%7C4621358b9be04a79d2d508d80798835b%7C686ea1d3bc2b
> > > 4c6fa92
> > > >
> > >
> > >
> cd99c5c301635%7C0%7C1%7C637267698912634476&amp;sdata=hR66H1hP%
> > > 2Fqb6OXe
> > > > w9wpXizY8DiNfZZ1KLwu3Kty87jc%3D&amp;reserved=0. The workaround
> is
> > > > adding new sdma ram script which works in XCH  mode as PIO inside
> > > > sdma instead of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0.
> > > > The issue
> > >
> > > should be exist on all legacy i.mx6/7 soc family before i.mx6ul.
> > > > NXP fix this design issue from i.mx6ul, so newer chips including
> > > > i.mx6ul/ 6ull/6sll do not need this workaroud anymore. All other
> > > > i.mx6/7/8 chips still need this workaroud. This patch set add new
> > > > 'fsl,imx6ul-ecspi'
> > > > for ecspi driver and 'ecspi_fixed' in sdma driver to choose if
> > > > need errata or not.
> > > > The first two reverted patches should be the same issue, though,
> > > > it seems 'fixed' by changing to other shp script. Hope Sean or
> > > > Sascha could have the chance to test this patch set if could fix
> > > > their issues.
> > > > Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not
> > > > work on i.mx8mm because the event id is zero.
> > > >
> > > > PS:
> > > >    Please get sdma firmware from below linux-firmware and copy it
> > > > to your local rootfs /lib/firmware/imx/sdma.
> > >
> > >
> > > Hello Robin,
> > >
> > > we have tried out this series, and there seems to be an issue with
> > > the
> > > PIO fallback. We are testing on an i.MX6Q board, and our kernel is
> > > a
> > > mostly-unmodified 5.4, on which we backported all SDMA patches from
> > > next-20200602 (imx-sdma.c is identical to next-20200602 version),
> > > and
> > > then applied this whole series.
> > >
> > > We build the SDMA driver as a kernel module, which is loaded by
> > > udev,
> > > so the root filesystem is ready and the SDMA firmware can be
> > > loaded.
> > > The behaviour we're seeing is the following:
> > >
> > > 1. As long as the SDMA driver is not loaded, initializing spi_imx
> > > will
> > > be deferred
> > > 2. imx_sdma is loaded. The SDMA firmware is not yet loaded at this
> > > point
> > > 3. spi_imx is initialized and an SPI-NOR flash is probed. To load
> > > the
> > > BFPT, the driver will attempt to use DMA; this will fail with
> > > EINVAL as
> > > long as the SDMA firmware is not ready, so the fallback to PIO
> > > happens
> > > (4. SDMA firmware is ready, subsequent SPI transfers use DMA)
> > >
> > > The problem happens in step 3: Whenever the driver falls back to
> > > PIO,
> > > the received data is corrupt. The behaviour is specific to the
> > > fallback: When I disable DMA completely via spi_imx.use_dma, or
> > > when
> > > the timing is lucky and the SDMA firmware gets loaded before the
> > > flash
> > > is probed, no corruption can be observed.
> >
> > Thanks Matthias, would you like post log?
> >
> 
> I have attached the following log files:
> - pio.log: DMA disabled via module parameter
> - dma.log: "lucky" timing, SDMA firmware loaded before SPI-NOR probe
> - fallback.log: DMA->PIO fallback
> 
> The logs include some additional log messages:
> - Return value of spi_imx_dma_transfer() before PIO fallback
> - SPI-NOR SFPT dump
> 
> It can be seen that the BFPT data is identical in pio.log and dma.log,
> and differs almost completely in fallback.log. The corrupted data seems
> to be random, or uninitialized memory; it differs with every boot.
Would you please have a try with the attached patch? Thanks.

[-- Attachment #2: 0006-spi-imx-add-dma_sync_sg_for_device-after-fallback-fr.patch --]
[-- Type: application/octet-stream, Size: 1526 bytes --]

From 08becec165b15663fafea52e3dc6ed5482ad3652 Mon Sep 17 00:00:00 2001
From: Robin Gong <yibin.gong@nxp.com>
Date: Fri, 5 Jun 2020 08:57:19 +0800
Subject: [PATCH v9 06/14] spi: imx: add dma_sync_sg_for_device after fallback
 from dma

In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to be
taken care cache since they have already been  mapped by dma_map_sg() in
spi.c.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
 drivers/spi/spi-imx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b7a85e3..c51cd3a 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1456,6 +1456,13 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
 		return -ETIMEDOUT;
 	}
 
+	if (transfer->rx_sg.sgl) {
+		struct device *rx_dev = spi->controller->dma_rx->device->dev;
+
+		dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
+				       transfer->rx_sg.nents, DMA_TO_DEVICE);
+	}
+
 	return transfer->len;
 }
 
@@ -1521,10 +1528,15 @@ static int spi_imx_transfer(struct spi_device *spi,
 	 * firmware may not be updated as ERR009165 required.
 	 */
 	if (spi_imx->usedma) {
+		struct device *tx_dev = spi->controller->dma_tx->device->dev;
+
 		ret = spi_imx_dma_transfer(spi_imx, transfer);
 		if (ret != -EINVAL)
 			return ret;
 
+		dma_sync_sg_for_device(tx_dev, transfer->tx_sg.sgl,
+				       transfer->tx_sg.nents, DMA_FROM_DEVICE);
+
 		spi_imx->devtype_data->disable_dma(spi_imx);
 
 		spi_imx->usedma = false;
-- 
2.7.4