On 2020/05/13 Sascha Hauer [off-list ref] wrote:

On Wed, May 13, 2020 at 09:05:33AM +0000, Robin Gong wrote:

quoted

On 2020/05/13 Sascha Hauer [off-list ref] wrote:d

quoted

quoted

 drivers/spi/spi-imx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index

f4f28a4..70df8e6 100644

--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c

@@ -585,8 +585,8 @@ static int mx51_ecspi_prepare_transfer(struct

spi_imx_data *spi_imx,

quoted

 	ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk);
 	spi_imx->spi_bus_clk = clk;

-	if (spi_imx->usedma)
-		ctrl |= MX51_ECSPI_CTRL_SMC;
+	/* ERR009165: work in XHC mode as PIO */
+	ctrl &= ~MX51_ECSPI_CTRL_SMC;

 	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);

@@ -617,7 +617,7 @@ static void mx51_setup_wml(struct spi_imx_data

*spi_imx)

quoted

 	 * and enable DMA request.
 	 */
 	writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) |
-		MX51_ECSPI_DMA_TX_WML(spi_imx->wml) |
+		MX51_ECSPI_DMA_TX_WML(0) |
 		MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) |
 		MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN |
 		MX51_ECSPI_DMA_RXTDEN, spi_imx->base +

MX51_ECSPI_DMA);

quoted

quoted

@@ -1171,7

quoted

+1171,11 @@ static int spi_imx_dma_configure(struct spi_master
+*master)
 	tx.direction = DMA_MEM_TO_DEV;
 	tx.dst_addr = spi_imx->base_phys + MXC_CSPITXDATA;
 	tx.dst_addr_width = buswidth;
-	tx.dst_maxburst = spi_imx->wml;
+	/*
+	 * For ERR009165 with tx_wml = 0 could enlarge burst size to fifo size
+	 * to speed up fifo filling as possible.
+	 */
+	tx.dst_maxburst = spi_imx->devtype_data->fifo_size;

In the next patch this is changed again to:

+       if (spi_imx->devtype_data->tx_glitch_fixed)
+               tx.dst_maxburst = spi_imx->wml;
+       else
+               tx.dst_maxburst = spi_imx->devtype_data->fifo_size;

So with tx_glitch_fixed we end up with tx.dst_maxburst being the
same as two patches before which is rather confusing. Better
introduce tx_glitch_fixed in this patch, or maybe even merge this patch and

the next one.

quoted

Sorry confused you, I should repleace 'tx_wml=0' in the above comments
with ' TX_THRESHOLD=0', which means tx transfer dma have to wait all
the tx data in tx fifo transferred with ERR009165 rather than
generically 'tx_wml' (for example --half fifo size used as
TX_THRESHOLD). Obviously TX_THRESHOLD=0 would down performance, so
enlarge dst_maxburst to fifo size as PIO with ERR009165. After
ERR009165 fixed at HW level. TX_THRESHOLD could be used as common
'spi_imx->wml' so change it back. Will add more detail information in
v8.

I am not confused, I meant the patches are confusing. What you are doing
is:

No patch:
	tx.dst_maxburst = a;

1st patch
	tx.dst_maxburst = b;

2nd patch:

	if (foo)
		tx.dst_maxburst = a;
	else
		tx.dst_maxburst = b;

It would be better readable and understandable if you did that in one patch,
because that would directly say "Under certain conditions we have to choose a,
otherwise b". That's much better than changing "a" to "b" and then to "a or b"

Okay, I'll merge those 2 changes into the next 08/13.