[PATCH 0/8] spi: sh-msiof: Import some BSP patches

[PATCH 0/8] spi: sh-msiof: Import some BSP patches

[Dirk Behme]

Pick some patches and fixes from Renesas v4.9/rcar-3.5.8 BSP to make spi
on RCar3 more reliably.

Patches are done against renesas-drivers-2017-09-05-v4.13.

Should we consider

spi: sh-msiof: Fix DMA transfer size check
spi: sh-msiof: Fix MSIOF address for DMAC

for -stable?

Both have been introduced with

b0d0ce8b6b91a0f6f99045b6019
spi: sh-msiof: Add DMA support
v3.17

Best regards

Dirk

[PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

This patch is for debug of transfer between master and slave.
Since the slave needs to complete a preparation in data transfer
before the master working, the sleep wait is put before
the data transfer of the master.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/Kconfig        | 20 ++++++++++++++++++++
 drivers/spi/spi-sh-msiof.c | 15 +++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index a75f2a2cf780..0139ecf8f42e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -600,6 +600,26 @@ config SPI_SH_MSIOF
 	help
 	  SPI driver for SuperH and SH Mobile MSIOF blocks.
 
+config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
+	bool "Transfer Synchronization Debug support for MSIOF"
+	depends on SPI_SH_MSIOF
+	default n
+	help
+	  In data transfer, the slave needs to have completed
+	  a transfer preparation before the master.
+	  As a test environment, it was to be able to put a sleep wait
+	  before the data transfer of the master.
+
+config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
+	int "Master of sleep latency (msec time)"
+	default 1
+	depends on SPI_SH_MSIOF && SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
+	help
+	  Select Sleep latency of the previous data transfer
+	  at the time of master mode.
+	  Examples:
+	    N => N msec
+
 config SPI_SH
 	tristate "SuperH SPI controller"
 	depends on SUPERH || COMPILE_TEST
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 0eb1e9583485..2b4d3a520176 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -41,6 +41,10 @@ struct sh_msiof_chipdata {
 	u16 min_div;
 };
 
+#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
+#define TRANSFAR_SYNC_DELAY (CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP)
+#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
+
 struct sh_msiof_spi_priv {
 	struct spi_master *master;
 	void __iomem *mapbase;
@@ -910,6 +914,11 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 		if (tx_buf)
 			copy32(p->tx_dma_page, tx_buf, l / 4);
 
+#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
+		if (p->mode == SPI_MSIOF_MASTER)
+			msleep(TRANSFAR_SYNC_DELAY);
+#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
+
 		ret = sh_msiof_dma_once(p, tx_buf, rx_buf, l);
 		if (ret == -EAGAIN) {
 			pr_warn_once("%s %s: DMA not available, falling back to PIO\n",
@@ -983,6 +992,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 	words = len / bytes_per_word;
 
 	while (words > 0) {
+
+#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
+		if (p->mode == SPI_MSIOF_MASTER)
+			msleep(TRANSFAR_SYNC_DELAY);
+#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
+
 		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo, tx_buf, rx_buf,
 					   words, bits);
 		if (n < 0)
-- 
2.14.1

[PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

DMA supports 32-bit words only,
even if BITLEN1 of SITMDR2 register is 16bit.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 2b4d3a520176..f9300fdf41e5 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 				break;
 			copy32 = copy_bswap32;
 		} else if (bits <= 16) {
-			if (l & 1)
+			if (l & 3)
 				break;
 			copy32 = copy_wswap32;
 		} else {
-- 
2.14.1

[PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>

MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.

Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index f9300fdf41e5..24b49d3ca9a8 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -1145,10 +1145,7 @@ static int sh_msiof_request_dma(struct sh_msiof_spi_priv *p)
 		return 0;
 	}
 
-	/* The DMA engine uses the second register set, if present */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!res)
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	master = p->master;
 	master->dma_tx = sh_msiof_request_dma_chan(dev, DMA_MEM_TO_DEV,
-- 
2.14.1

[PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

From: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.

Signed-off-by: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
---
 drivers/spi/spi-sh-msiof.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index f9300fdf41e5..24b49d3ca9a8 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -1145,10 +1145,7 @@ static int sh_msiof_request_dma(struct sh_msiof_spi_priv *p)
 		return 0;
 	}
 
-	/* The DMA engine uses the second register set, if present */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!res)
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	master = p->master;
 	master->dma_tx = sh_msiof_request_dma_chan(dev, DMA_MEM_TO_DEV,
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] spi: sh-msiof: Fix DMA completion

[Dirk Behme]

From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>

When reception DMA completes before transmission DMA, next transmission
DMA may not be able to start. This patch adds wait_for_completion_timeout()
to both of reception DMA and transmission DMA.

If the driver waits only for the Rx DMA completion, the Tx DMA completion
thread of DMA Engine may be still processing.

Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
[reword commit message]
Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
[adjust context]
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 53 +++++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 24b49d3ca9a8..660b03ed6770 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -52,6 +52,7 @@ struct sh_msiof_spi_priv {
 	struct platform_device *pdev;
 	struct sh_msiof_spi_info *info;
 	struct completion done;
+	struct completion done_dma_tx, done_dma_rx;
 	unsigned int tx_fifo_size;
 	unsigned int rx_fifo_size;
 	unsigned int min_div;
@@ -621,7 +622,8 @@ static int sh_msiof_slave_abort(struct spi_master *master)
 	return 0;
 }
 
-static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p)
+static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p,
+					const void *tx, void *rx)
 {
 	if (spi_controller_is_slave(p->master)) {
 		if (wait_for_completion_interruptible(&p->done) ||
@@ -630,10 +632,22 @@ static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p)
 			return -EINTR;
 		}
 	} else {
-		if (!wait_for_completion_timeout(&p->done, HZ)) {
-			dev_err(&p->pdev->dev, "timeout\n");
-			return -ETIMEDOUT;
+		if (tx) {
+			if (!wait_for_completion_timeout(&p->done_dma_tx,
+							 HZ)) {
+				dev_err(&p->pdev->dev, "Tx DMA timeout\n");
+				return -ETIMEDOUT;
+			}
 		}
+		if (rx) {
+			if (!wait_for_completion_timeout(&p->done_dma_rx,
+							 HZ)) {
+				dev_err(&p->pdev->dev, "Rx DMA timeout\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		sh_msiof_write(p, IER, 0);
 	}
 
 	return 0;
@@ -680,7 +694,7 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 	}
 
 	/* wait for tx fifo to be emptied / rx fifo to be filled */
-	ret = sh_msiof_wait_for_completion(p);
+	ret = sh_msiof_wait_for_completion(p, tx_buf, rx_buf);
 	if (ret)
 		goto stop_reset;
 
@@ -707,12 +721,18 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 	return ret;
 }
 
-static void sh_msiof_dma_complete(void *arg)
+static void sh_msiof_tx_dma_complete(void *arg)
 {
 	struct sh_msiof_spi_priv *p = arg;
 
-	sh_msiof_write(p, IER, 0);
-	complete(&p->done);
+	complete(&p->done_dma_tx);
+}
+
+static void sh_msiof_rx_dma_complete(void *arg)
+{
+	struct sh_msiof_spi_priv *p = arg;
+
+	complete(&p->done_dma_rx);
 }
 
 static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
@@ -732,7 +752,7 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 		if (!desc_rx)
 			return -EAGAIN;
 
-		desc_rx->callback = sh_msiof_dma_complete;
+		desc_rx->callback = sh_msiof_rx_dma_complete;
 		desc_rx->callback_param = p;
 		cookie = dmaengine_submit(desc_rx);
 		if (dma_submit_error(cookie))
@@ -751,13 +771,8 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 			goto no_dma_tx;
 		}
 
-		if (rx) {
-			/* No callback */
-			desc_tx->callback = NULL;
-		} else {
-			desc_tx->callback = sh_msiof_dma_complete;
-			desc_tx->callback_param = p;
-		}
+		desc_tx->callback = sh_msiof_tx_dma_complete;
+		desc_tx->callback_param = p;
 		cookie = dmaengine_submit(desc_tx);
 		if (dma_submit_error(cookie)) {
 			ret = cookie;
@@ -774,6 +789,8 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 	sh_msiof_write(p, IER, ier_bits);
 
 	reinit_completion(&p->done);
+	reinit_completion(&p->done_dma_tx);
+	reinit_completion(&p->done_dma_rx);
 	p->slave_aborted = false;
 
 	/* Now start DMA */
@@ -789,7 +806,7 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 	}
 
 	/* wait for tx fifo to be emptied / rx fifo to be filled */
-	ret = sh_msiof_wait_for_completion(p);
+	ret = sh_msiof_wait_for_completion(p, tx, rx);
 	if (ret)
 		goto stop_reset;
 
@@ -1258,6 +1275,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	p->min_div = chipdata->min_div;
 
 	init_completion(&p->done);
+	init_completion(&p->done_dma_tx);
+	init_completion(&p->done_dma_rx);
 
 	p->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(p->clk)) {
-- 
2.14.1

[PATCH 5/8] spi: sh-msiof: Wait for Tx FIFO empty after DMA

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

When Tx DMA is only used, Tx FIFO is still not empty after DMA callback.
This patch waits for sweeping data out of the Tx FIFO.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
[adjust context]
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 660b03ed6770..a960e8da123d 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -638,6 +638,17 @@ static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p,
 				dev_err(&p->pdev->dev, "Tx DMA timeout\n");
 				return -ETIMEDOUT;
 			}
+			if (!rx) {
+				sh_msiof_write(p, IER, IER_TEOFE);
+
+				/* wait for tx fifo to be emptied */
+				if (!wait_for_completion_timeout(&p->done,
+								 HZ)) {
+					dev_err(&p->pdev->dev,
+					"Tx fifo to be emptied timeout\n");
+					return -ETIMEDOUT;
+				}
+			}
 		}
 		if (rx) {
 			if (!wait_for_completion_timeout(&p->done_dma_rx,
@@ -805,7 +816,7 @@ static int sh_msiof_dma_once(struct sh_msiof_spi_priv *p, const void *tx,
 		goto stop_dma;
 	}
 
-	/* wait for tx fifo to be emptied / rx fifo to be filled */
+	/* wait for Tx/Rx DMA completion */
 	ret = sh_msiof_wait_for_completion(p, tx, rx);
 	if (ret)
 		goto stop_reset;
-- 
2.14.1

[PATCH 6/8] spi: sh-msiof: Add MSIOF parent clock changing function for R-Car Gen3

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index a960e8da123d..2c53fc3f73af 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -188,6 +188,14 @@ struct sh_msiof_spi_priv {
 #define IER_RFOVFE	0x00000008 /* Receive FIFO Overflow Enable */
 
 
+static int msiof_rcar_is_gen3(struct device *dev)
+{
+	struct device_node *node = dev->of_node;
+
+	return of_device_is_compatible(node, "renesas,msiof-r8a7795") ||
+		of_device_is_compatible(node, "renesas,msiof-r8a7796");
+}
+
 static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
 {
 	switch (reg_offs) {
@@ -1252,6 +1260,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id;
 	struct sh_msiof_spi_info *info;
 	struct sh_msiof_spi_priv *p;
+	struct clk *ref_clk;
+	u32 clk_rate = 0;
 	int i;
 	int ret;
 
@@ -1352,6 +1362,17 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
+	if (msiof_rcar_is_gen3(&master->dev)) {
+		ref_clk = devm_clk_get(&pdev->dev, "msiof_ref_clk");
+		if (!IS_ERR(ref_clk))
+			clk_rate = clk_get_rate(ref_clk);
+		if (clk_rate) {
+			clk_prepare_enable(p->clk);
+			clk_set_rate(p->clk, clk_rate);
+			clk_disable_unprepare(p->clk);
+		}
+	}
+
 	return 0;
 
  err2:
-- 
2.14.1

[PATCH 7/8] spi: sh-msiof: Fix gpio function

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

This patch adds a function to control chip select by GPIO.
In order to use this patch, it is necessary to define it to
devicetree. <refer Documentation/devicetree/bindings/spi/spi-bus.txt>

<devicetree example>

&pfc {
        ...
        /* MSIOF_SYMC Pin delete. */
        msiof1_pins: spi2 {
                /* The definition of sync, ss1 and ss2 are
                   unnecessary because of using GPIO as chip
                   select. */
                groups = "msiof1_clk_c",
                                "msiof1_rxd_c",  "msiof1_txd_c";
                function = "msiof1";
        };
        ...
};

&msiof1 {
        pinctrl-0 = <&msiof1_pins>;
        pinctrl-names = "default";
        cs-gpios = <&gpio6 21 GPIO_ACTIVE_LOW>,
                    <&gpio6 27 GPIO_ACTIVE_LOW>;
        status = "okay";

        spidev@0 {
                ...
                reg = <0>;
		...
        };
        spidev@1 {
                ...
                reg = <1>;
		...
        };
};

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 2c53fc3f73af..fdad8d852602 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -541,6 +541,7 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 {
 	struct device_node	*np = spi->master->dev.of_node;
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
+	int ret;
 
 	pm_runtime_get_sync(&p->pdev->dev);
 
@@ -559,8 +560,12 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 				  !!(spi->mode & SPI_LSB_FIRST),
 				  !!(spi->mode & SPI_CS_HIGH));
 
-	if (spi->cs_gpio >= 0)
-		gpio_set_value(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
+	if (gpio_is_valid(spi->cs_gpio)) {
+		ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
+		if (!ret)
+			gpio_direction_output(spi->cs_gpio,
+					!(spi->mode & SPI_CS_HIGH));
+	}
 
 
 	pm_runtime_put(&p->pdev->dev);
-- 
2.14.1

[PATCH 7/8] spi: sh-msiof: Fix gpio function

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

This patch adds a function to control chip select by GPIO.
In order to use this patch, it is necessary to define it to
devicetree. <refer Documentation/devicetree/bindings/spi/spi-bus.txt>

<devicetree example>

&pfc {
        ...
        /* MSIOF_SYMC Pin delete. */
        msiof1_pins: spi2 {
                /* The definition of sync, ss1 and ss2 are
                   unnecessary because of using GPIO as chip
                   select. */
                groups = "msiof1_clk_c",
                                "msiof1_rxd_c",  "msiof1_txd_c";
                function = "msiof1";
        };
        ...
};

&msiof1 {
        pinctrl-0 = <&msiof1_pins>;
        pinctrl-names = "default";
        cs-gpios = <&gpio6 21 GPIO_ACTIVE_LOW>,
                    <&gpio6 27 GPIO_ACTIVE_LOW>;
        status = "okay";

        spidev@0 {
                ...
                reg = <0>;
		...
        };
        spidev@1 {
                ...
                reg = <1>;
		...
        };
};

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
---
 drivers/spi/spi-sh-msiof.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 2c53fc3f73af..fdad8d852602 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -541,6 +541,7 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 {
 	struct device_node	*np = spi->master->dev.of_node;
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
+	int ret;
 
 	pm_runtime_get_sync(&p->pdev->dev);
 
@@ -559,8 +560,12 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
 				  !!(spi->mode & SPI_LSB_FIRST),
 				  !!(spi->mode & SPI_CS_HIGH));
 
-	if (spi->cs_gpio >= 0)
-		gpio_set_value(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
+	if (gpio_is_valid(spi->cs_gpio)) {
+		ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
+		if (!ret)
+			gpio_direction_output(spi->cs_gpio,
+					!(spi->mode & SPI_CS_HIGH));
+	}
 
 
 	pm_runtime_put(&p->pdev->dev);
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] spi: sh-msiof: Add registers reset

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>

Reset register before starting transfer.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 drivers/spi/spi-sh-msiof.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index fdad8d852602..e8aebd406477 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
 #define CTR_TFSE	0x00004000 /* Transmit Frame Sync Signal Output Enable */
 #define CTR_TXE		0x00000200 /* Transmit Enable */
 #define CTR_RXE		0x00000100 /* Receive Enable */
+#define CTR_TXRST	0x00000002 /* Transmit Reset */
+#define CTR_RXRST	0x00000001 /* Receive Reset */
 
 /* FCTR */
 #define FCTR_TFWM_MASK	0xe0000000 /* Transmit FIFO Watermark */
@@ -254,6 +256,25 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
+{
+	u32 mask = CTR_TXRST | CTR_RXRST;
+	u32 data;
+	int k;
+
+	data = sh_msiof_read(p, CTR);
+	data |= mask;
+
+	sh_msiof_write(p, CTR, data);
+
+	for (k = 100; k > 0; k--) {
+		if (!(sh_msiof_read(p, CTR) & mask))
+			break;
+
+		udelay(10);
+	}
+}
+
 static struct {
 	unsigned short div;
 	unsigned short brdv;
@@ -924,6 +945,9 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 	bool swab;
 	int ret;
 
+	/* reset registers */
+	sh_msiof_spi_reset_regs(p);
+
 	/* setup clocks (clock already enabled in chipselect()) */
 	if (!spi_controller_is_slave(p->master))
 		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
-- 
2.14.1

[PATCH 8/8] spi: sh-msiof: Add registers reset

[Dirk Behme]

From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Reset register before starting transfer.

Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
---
 drivers/spi/spi-sh-msiof.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index fdad8d852602..e8aebd406477 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -130,6 +130,8 @@ struct sh_msiof_spi_priv {
 #define CTR_TFSE	0x00004000 /* Transmit Frame Sync Signal Output Enable */
 #define CTR_TXE		0x00000200 /* Transmit Enable */
 #define CTR_RXE		0x00000100 /* Receive Enable */
+#define CTR_TXRST	0x00000002 /* Transmit Reset */
+#define CTR_RXRST	0x00000001 /* Receive Reset */
 
 /* FCTR */
 #define FCTR_TFWM_MASK	0xe0000000 /* Transmit FIFO Watermark */
@@ -254,6 +256,25 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
+{
+	u32 mask = CTR_TXRST | CTR_RXRST;
+	u32 data;
+	int k;
+
+	data = sh_msiof_read(p, CTR);
+	data |= mask;
+
+	sh_msiof_write(p, CTR, data);
+
+	for (k = 100; k > 0; k--) {
+		if (!(sh_msiof_read(p, CTR) & mask))
+			break;
+
+		udelay(10);
+	}
+}
+
 static struct {
 	unsigned short div;
 	unsigned short brdv;
@@ -924,6 +945,9 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 	bool swab;
 	int ret;
 
+	/* reset registers */
+	sh_msiof_spi_reset_regs(p);
+
 	/* setup clocks (clock already enabled in chipselect()) */
 	if (!spi_controller_is_slave(p->master))
 		sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk), t->speed_hz);
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>
> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>
> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

NAKed-by: Geert Uytterhoeven <geert+renesas@glider.be>

The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
manuals. It still affects e.g. R-Mobile APE6.

(I'll reply to the other patches when I find some time).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

On 06.09.2017 11:22, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>
>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>
>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> 
> NAKed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
> manuals. It still affects e.g. R-Mobile APE6.


Do we have a nice run time detection for that?


Best regards

Dirk

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

On 06.09.2017 11:22, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>
>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>
>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> 
> NAKed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> 
> The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
> manuals. It still affects e.g. R-Mobile APE6.


Do we have a nice run time detection for that?


Best regards

Dirk
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 12:09 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 06.09.2017 11:22, Geert Uytterhoeven wrote:
>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>>
>>> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>>
>>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>>
>>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>
>>
>> NAKed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
>> manuals. It still affects e.g. R-Mobile APE6.
>
> Do we have a nice run time detection for that?

The current driver code just looks at the DTS, cfr. the comment:

    /* The DMA engine uses the second register set, if present */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

On 06.09.2017 12:42, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 12:09 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 06.09.2017 11:22, Geert Uytterhoeven wrote:
>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>>
>>>> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>>>
>>>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>>>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>>>
>>>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>
>>>
>>> NAKed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>> The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
>>> manuals. It still affects e.g. R-Mobile APE6.
>>
>> Do we have a nice run time detection for that?
> 
> The current driver code just looks at the DTS, cfr. the comment:
> 
>      /* The DMA engine uses the second register set, if present */


Ok, yes, thanks.

So this patch is completely superfluous, even in the BSP, correct?

Will drop it for the next round, hoping that it doesn't result in too 
much rebase hassle ;)

Best regards

Dirk

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Dirk Behme]

On 06.09.2017 12:42, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 12:09 PM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> On 06.09.2017 11:22, Geert Uytterhoeven wrote:
>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>> wrote:
>>>>
>>>> From: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>
>>>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>>>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>>>
>>>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>
>>>
>>> NAKed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>>
>>> The MSIOF Base Address for DMAC was removed only from the R-Car Gen2/Gen3
>>> manuals. It still affects e.g. R-Mobile APE6.
>>
>> Do we have a nice run time detection for that?
> 
> The current driver code just looks at the DTS, cfr. the comment:
> 
>      /* The DMA engine uses the second register set, if present */


Ok, yes, thanks.

So this patch is completely superfluous, even in the BSP, correct?

Will drop it for the next round, hoping that it doesn't result in too 
much rebase hassle ;)

Best regards

Dirk

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] spi: sh-msiof: Fix MSIOF address for DMAC

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 12:59 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 06.09.2017 12:42, Geert Uytterhoeven wrote:
>> On Wed, Sep 6, 2017 at 12:09 PM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 06.09.2017 11:22, Geert Uytterhoeven wrote:
>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>> wrote:
>>>>> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>>>>
>>>>> MSIOF Base Address H'E6xx can be accessed by CPU and DMAC.
>>>>> MSIOF Base Address H'E7xx for DMAC was removed from H/W manual.
>>>>>
>>>>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>
>>>> NAKed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>
>>>> The MSIOF Base Address for DMAC was removed only from the R-Car
>>>> Gen2/Gen3
>>>> manuals. It still affects e.g. R-Mobile APE6.
>>>
>>> Do we have a nice run time detection for that?
>>
>> The current driver code just looks at the DTS, cfr. the comment:
>>
>>      /* The DMA engine uses the second register set, if present */
>
> Ok, yes, thanks.
>
> So this patch is completely superfluous, even in the BSP, correct?

Correct. But it keeps on popping up, I think this is the third time someone
tried to submit it upstream :-(

> Will drop it for the next round, hoping that it doesn't result in too much
> rebase hassle ;)

OK, thx!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/8] spi: sh-msiof: Wait for Tx FIFO empty after DMA

[Sergei Shtylyov]

Hello!

On 09/06/2017 10:05 AM, Dirk Behme wrote:

> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> 
> When Tx DMA is only used, Tx FIFO is still not empty after DMA callback.
> This patch waits for sweeping data out of the Tx FIFO.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> [adjust context]
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>   drivers/spi/spi-sh-msiof.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 660b03ed6770..a960e8da123d 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -638,6 +638,17 @@ static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p,
>   				dev_err(&p->pdev->dev, "Tx DMA timeout\n");
>   				return -ETIMEDOUT;
>   			}
> +			if (!rx) {
> +				sh_msiof_write(p, IER, IER_TEOFE);
> +
> +				/* wait for tx fifo to be emptied */
> +				if (!wait_for_completion_timeout(&p->done,
> +								 HZ)) {
> +					dev_err(&p->pdev->dev,
> +					"Tx fifo to be emptied timeout\n");

    Please indent this line properly, messages can violate 80-column limit.

> +					return -ETIMEDOUT;
> +				}
> +			}
>   		}
>   		if (rx) {
>   			if (!wait_for_completion_timeout(&p->done_dma_rx,
[...]

MBR, Sergei

Re: [PATCH 8/8] spi: sh-msiof: Add registers reset

[Sergei Shtylyov]

On 09/06/2017 10:05 AM, Dirk Behme wrote:

> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> 
> Reset register before starting transfer.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>   drivers/spi/spi-sh-msiof.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index fdad8d852602..e8aebd406477 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
[...]
> @@ -254,6 +256,25 @@ static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
>   	return IRQ_HANDLED;
>   }
>   
> +static void sh_msiof_spi_reset_regs(struct sh_msiof_spi_priv *p)
> +{
> +	u32 mask = CTR_TXRST | CTR_RXRST;

    *const*?

> +	u32 data;
> +	int k;
> +
> +	data = sh_msiof_read(p, CTR);
> +	data |= mask;
> +

    Don't think empty line is needed here.

> +	sh_msiof_write(p, CTR, data);
> +
> +	for (k = 100; k > 0; k--) {
> +		if (!(sh_msiof_read(p, CTR) & mask))
> +			break;
> +
> +		udelay(10);
> +	}
> +}
> +
>   static struct {
>   	unsigned short div;
>   	unsigned short brdv;
[...]

MBR, Sergei

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Vladimir Zapolskiy]

Hi Dirk,

On 09/06/2017 10:05 AM, Dirk Behme wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> 
> This patch is for debug of transfer between master and slave.
> Since the slave needs to complete a preparation in data transfer
> before the master working, the sleep wait is put before
> the data transfer of the master.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  drivers/spi/Kconfig        | 20 ++++++++++++++++++++
>  drivers/spi/spi-sh-msiof.c | 15 +++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index a75f2a2cf780..0139ecf8f42e 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -600,6 +600,26 @@ config SPI_SH_MSIOF
>  	help
>  	  SPI driver for SuperH and SH Mobile MSIOF blocks.
>  
> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +	bool "Transfer Synchronization Debug support for MSIOF"
> +	depends on SPI_SH_MSIOF
> +	default n

Drop 'default n', it is the default per se.

> +	help
> +	  In data transfer, the slave needs to have completed
> +	  a transfer preparation before the master.
> +	  As a test environment, it was to be able to put a sleep wait
> +	  before the data transfer of the master.
> +
> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
> +	int "Master of sleep latency (msec time)"

Master of sleep latency? Probably reformulation is wanted.

> +	default 1

In addition please define and add a valid 'range' option.

> +	depends on SPI_SH_MSIOF && SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG

Dependence on SPI_SH_MSIOF is inherited.

> +	help
> +	  Select Sleep latency of the previous data transfer
> +	  at the time of master mode.
> +	  Examples:
> +	    N => N msec
> +
>  config SPI_SH
>  	tristate "SuperH SPI controller"
>  	depends on SUPERH || COMPILE_TEST
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 0eb1e9583485..2b4d3a520176 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -41,6 +41,10 @@ struct sh_msiof_chipdata {
>  	u16 min_div;
>  };
>  
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +#define TRANSFAR_SYNC_DELAY (CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP)

typo, s/TRANSFAR/TRANSFER/

Parenthesis around CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
are not needed.

> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> +
>  struct sh_msiof_spi_priv {
>  	struct spi_master *master;
>  	void __iomem *mapbase;
> @@ -910,6 +914,11 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>  		if (tx_buf)
>  			copy32(p->tx_dma_page, tx_buf, l / 4);
>  
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +		if (p->mode == SPI_MSIOF_MASTER)
> +			msleep(TRANSFAR_SYNC_DELAY);
> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */

If SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG is unset, you may set TRANSFAR_SYNC_DELAY
to 0 and get rid of #ifdefs in the code.

	if (p->mode == SPI_MSIOF_MASTER && TRANSFAR_SYNC_DELAY)
		msleep(TRANSFAR_SYNC_DELAY);

> +
>  		ret = sh_msiof_dma_once(p, tx_buf, rx_buf, l);
>  		if (ret == -EAGAIN) {
>  			pr_warn_once("%s %s: DMA not available, falling back to PIO\n",
> @@ -983,6 +992,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>  	words = len / bytes_per_word;
>  
>  	while (words > 0) {
> +
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +		if (p->mode == SPI_MSIOF_MASTER)
> +			msleep(TRANSFAR_SYNC_DELAY);
> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> +
>  		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo, tx_buf, rx_buf,
>  					   words, bits);
>  		if (n < 0)
> 

In general I don't think it makes any sense to incluide this change.

--
With best wishes,
Vladimir

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Vladimir Zapolskiy]

Hi Dirk,

On 09/06/2017 10:05 AM, Dirk Behme wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> 
> This patch is for debug of transfer between master and slave.
> Since the slave needs to complete a preparation in data transfer
> before the master working, the sleep wait is put before
> the data transfer of the master.
> 
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/spi/Kconfig        | 20 ++++++++++++++++++++
>  drivers/spi/spi-sh-msiof.c | 15 +++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index a75f2a2cf780..0139ecf8f42e 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -600,6 +600,26 @@ config SPI_SH_MSIOF
>  	help
>  	  SPI driver for SuperH and SH Mobile MSIOF blocks.
>  
> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +	bool "Transfer Synchronization Debug support for MSIOF"
> +	depends on SPI_SH_MSIOF
> +	default n

Drop 'default n', it is the default per se.

> +	help
> +	  In data transfer, the slave needs to have completed
> +	  a transfer preparation before the master.
> +	  As a test environment, it was to be able to put a sleep wait
> +	  before the data transfer of the master.
> +
> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
> +	int "Master of sleep latency (msec time)"

Master of sleep latency? Probably reformulation is wanted.

> +	default 1

In addition please define and add a valid 'range' option.

> +	depends on SPI_SH_MSIOF && SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG

Dependence on SPI_SH_MSIOF is inherited.

> +	help
> +	  Select Sleep latency of the previous data transfer
> +	  at the time of master mode.
> +	  Examples:
> +	    N => N msec
> +
>  config SPI_SH
>  	tristate "SuperH SPI controller"
>  	depends on SUPERH || COMPILE_TEST
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 0eb1e9583485..2b4d3a520176 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -41,6 +41,10 @@ struct sh_msiof_chipdata {
>  	u16 min_div;
>  };
>  
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +#define TRANSFAR_SYNC_DELAY (CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP)

typo, s/TRANSFAR/TRANSFER/

Parenthesis around CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
are not needed.

> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> +
>  struct sh_msiof_spi_priv {
>  	struct spi_master *master;
>  	void __iomem *mapbase;
> @@ -910,6 +914,11 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>  		if (tx_buf)
>  			copy32(p->tx_dma_page, tx_buf, l / 4);
>  
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +		if (p->mode == SPI_MSIOF_MASTER)
> +			msleep(TRANSFAR_SYNC_DELAY);
> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */

If SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG is unset, you may set TRANSFAR_SYNC_DELAY
to 0 and get rid of #ifdefs in the code.

	if (p->mode == SPI_MSIOF_MASTER && TRANSFAR_SYNC_DELAY)
		msleep(TRANSFAR_SYNC_DELAY);

> +
>  		ret = sh_msiof_dma_once(p, tx_buf, rx_buf, l);
>  		if (ret == -EAGAIN) {
>  			pr_warn_once("%s %s: DMA not available, falling back to PIO\n",
> @@ -983,6 +992,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>  	words = len / bytes_per_word;
>  
>  	while (words > 0) {
> +
> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> +		if (p->mode == SPI_MSIOF_MASTER)
> +			msleep(TRANSFAR_SYNC_DELAY);
> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> +
>  		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo, tx_buf, rx_buf,
>  					   words, bits);
>  		if (n < 0)
> 

In general I don't think it makes any sense to incluide this change.

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Dirk Behme]

On 07.09.2017 09:04, Vladimir Zapolskiy wrote:
> Hi Dirk,
> 
> On 09/06/2017 10:05 AM, Dirk Behme wrote:
>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>
>> This patch is for debug of transfer between master and slave.
>> Since the slave needs to complete a preparation in data transfer
>> before the master working, the sleep wait is put before
>> the data transfer of the master.
>>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>   drivers/spi/Kconfig        | 20 ++++++++++++++++++++
>>   drivers/spi/spi-sh-msiof.c | 15 +++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> index a75f2a2cf780..0139ecf8f42e 100644
>> --- a/drivers/spi/Kconfig
>> +++ b/drivers/spi/Kconfig
>> @@ -600,6 +600,26 @@ config SPI_SH_MSIOF
>>   	help
>>   	  SPI driver for SuperH and SH Mobile MSIOF blocks.
>>   
>> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +	bool "Transfer Synchronization Debug support for MSIOF"
>> +	depends on SPI_SH_MSIOF
>> +	default n
> 
> Drop 'default n', it is the default per se.
> 
>> +	help
>> +	  In data transfer, the slave needs to have completed
>> +	  a transfer preparation before the master.
>> +	  As a test environment, it was to be able to put a sleep wait
>> +	  before the data transfer of the master.
>> +
>> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
>> +	int "Master of sleep latency (msec time)"
> 
> Master of sleep latency? Probably reformulation is wanted.
> 
>> +	default 1
> 
> In addition please define and add a valid 'range' option.
> 
>> +	depends on SPI_SH_MSIOF && SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> 
> Dependence on SPI_SH_MSIOF is inherited.
> 
>> +	help
>> +	  Select Sleep latency of the previous data transfer
>> +	  at the time of master mode.
>> +	  Examples:
>> +	    N => N msec
>> +
>>   config SPI_SH
>>   	tristate "SuperH SPI controller"
>>   	depends on SUPERH || COMPILE_TEST
>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>> index 0eb1e9583485..2b4d3a520176 100644
>> --- a/drivers/spi/spi-sh-msiof.c
>> +++ b/drivers/spi/spi-sh-msiof.c
>> @@ -41,6 +41,10 @@ struct sh_msiof_chipdata {
>>   	u16 min_div;
>>   };
>>   
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +#define TRANSFAR_SYNC_DELAY (CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP)
> 
> typo, s/TRANSFAR/TRANSFER/
> 
> Parenthesis around CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
> are not needed.
> 
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
>> +
>>   struct sh_msiof_spi_priv {
>>   	struct spi_master *master;
>>   	void __iomem *mapbase;
>> @@ -910,6 +914,11 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>>   		if (tx_buf)
>>   			copy32(p->tx_dma_page, tx_buf, l / 4);
>>   
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +		if (p->mode == SPI_MSIOF_MASTER)
>> +			msleep(TRANSFAR_SYNC_DELAY);
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> 
> If SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG is unset, you may set TRANSFAR_SYNC_DELAY
> to 0 and get rid of #ifdefs in the code.
> 
> 	if (p->mode == SPI_MSIOF_MASTER && TRANSFAR_SYNC_DELAY)
> 		msleep(TRANSFAR_SYNC_DELAY);
> 
>> +
>>   		ret = sh_msiof_dma_once(p, tx_buf, rx_buf, l);
>>   		if (ret == -EAGAIN) {
>>   			pr_warn_once("%s %s: DMA not available, falling back to PIO\n",
>> @@ -983,6 +992,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>>   	words = len / bytes_per_word;
>>   
>>   	while (words > 0) {
>> +
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +		if (p->mode == SPI_MSIOF_MASTER)
>> +			msleep(TRANSFAR_SYNC_DELAY);
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
>> +
>>   		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo, tx_buf, rx_buf,
>>   					   words, bits);
>>   		if (n < 0)
>>
> 
> In general I don't think it makes any sense to incluide this change.


Would be fine with me.

Geert: Do you agree to drop this, too?

Best regards

Dirk

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Dirk Behme]

On 07.09.2017 09:04, Vladimir Zapolskiy wrote:
> Hi Dirk,
> 
> On 09/06/2017 10:05 AM, Dirk Behme wrote:
>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>
>> This patch is for debug of transfer between master and slave.
>> Since the slave needs to complete a preparation in data transfer
>> before the master working, the sleep wait is put before
>> the data transfer of the master.
>>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>> ---
>>   drivers/spi/Kconfig        | 20 ++++++++++++++++++++
>>   drivers/spi/spi-sh-msiof.c | 15 +++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> index a75f2a2cf780..0139ecf8f42e 100644
>> --- a/drivers/spi/Kconfig
>> +++ b/drivers/spi/Kconfig
>> @@ -600,6 +600,26 @@ config SPI_SH_MSIOF
>>   	help
>>   	  SPI driver for SuperH and SH Mobile MSIOF blocks.
>>   
>> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +	bool "Transfer Synchronization Debug support for MSIOF"
>> +	depends on SPI_SH_MSIOF
>> +	default n
> 
> Drop 'default n', it is the default per se.
> 
>> +	help
>> +	  In data transfer, the slave needs to have completed
>> +	  a transfer preparation before the master.
>> +	  As a test environment, it was to be able to put a sleep wait
>> +	  before the data transfer of the master.
>> +
>> +config SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
>> +	int "Master of sleep latency (msec time)"
> 
> Master of sleep latency? Probably reformulation is wanted.
> 
>> +	default 1
> 
> In addition please define and add a valid 'range' option.
> 
>> +	depends on SPI_SH_MSIOF && SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
> 
> Dependence on SPI_SH_MSIOF is inherited.
> 
>> +	help
>> +	  Select Sleep latency of the previous data transfer
>> +	  at the time of master mode.
>> +	  Examples:
>> +	    N => N msec
>> +
>>   config SPI_SH
>>   	tristate "SuperH SPI controller"
>>   	depends on SUPERH || COMPILE_TEST
>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>> index 0eb1e9583485..2b4d3a520176 100644
>> --- a/drivers/spi/spi-sh-msiof.c
>> +++ b/drivers/spi/spi-sh-msiof.c
>> @@ -41,6 +41,10 @@ struct sh_msiof_chipdata {
>>   	u16 min_div;
>>   };
>>   
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +#define TRANSFAR_SYNC_DELAY (CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP)
> 
> typo, s/TRANSFAR/TRANSFER/
> 
> Parenthesis around CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG_MSLEEP
> are not needed.
> 
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
>> +
>>   struct sh_msiof_spi_priv {
>>   	struct spi_master *master;
>>   	void __iomem *mapbase;
>> @@ -910,6 +914,11 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>>   		if (tx_buf)
>>   			copy32(p->tx_dma_page, tx_buf, l / 4);
>>   
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +		if (p->mode == SPI_MSIOF_MASTER)
>> +			msleep(TRANSFAR_SYNC_DELAY);
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
> 
> If SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG is unset, you may set TRANSFAR_SYNC_DELAY
> to 0 and get rid of #ifdefs in the code.
> 
> 	if (p->mode == SPI_MSIOF_MASTER && TRANSFAR_SYNC_DELAY)
> 		msleep(TRANSFAR_SYNC_DELAY);
> 
>> +
>>   		ret = sh_msiof_dma_once(p, tx_buf, rx_buf, l);
>>   		if (ret == -EAGAIN) {
>>   			pr_warn_once("%s %s: DMA not available, falling back to PIO\n",
>> @@ -983,6 +992,12 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>>   	words = len / bytes_per_word;
>>   
>>   	while (words > 0) {
>> +
>> +#ifdef CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG
>> +		if (p->mode == SPI_MSIOF_MASTER)
>> +			msleep(TRANSFAR_SYNC_DELAY);
>> +#endif /* CONFIG_SPI_SH_MSIOF_TRANSFER_SYNC_DEBUG */
>> +
>>   		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo, tx_buf, rx_buf,
>>   					   words, bits);
>>   		if (n < 0)
>>
> 
> In general I don't think it makes any sense to incluide this change.


Would be fine with me.

Geert: Do you agree to drop this, too?

Best regards

Dirk
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> This patch is for debug of transfer between master and slave.
> Since the slave needs to complete a preparation in data transfer
> before the master working, the sleep wait is put before
> the data transfer of the master.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

IMHO this is a pure debug patch, not intended for upstream.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 8/8] spi: sh-msiof: Add registers reset

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> Reset register before starting transfer.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Why is this needed?
What does it fix?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 8/8] spi: sh-msiof: Add registers reset

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> Reset register before starting transfer.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>

Why is this needed?
What does it fix?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] spi: sh-msiof: Fix gpio function

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> This patch adds a function to control chip select by GPIO.
> In order to use this patch, it is necessary to define it to
> devicetree. <refer Documentation/devicetree/bindings/spi/spi-bus.txt>
>
> <devicetree example>
>
> &pfc {
>         ...
>         /* MSIOF_SYMC Pin delete. */
>         msiof1_pins: spi2 {
>                 /* The definition of sync, ss1 and ss2 are
>                    unnecessary because of using GPIO as chip
>                    select. */
>                 groups = "msiof1_clk_c",
>                                 "msiof1_rxd_c",  "msiof1_txd_c";
>                 function = "msiof1";
>         };
>         ...
> };
>
> &msiof1 {
>         pinctrl-0 = <&msiof1_pins>;
>         pinctrl-names = "default";
>         cs-gpios = <&gpio6 21 GPIO_ACTIVE_LOW>,
>                     <&gpio6 27 GPIO_ACTIVE_LOW>;
>         status = "okay";
>
>         spidev@0 {
>                 ...
>                 reg = <0>;
>                 ...
>         };
>         spidev@1 {
>                 ...
>                 reg = <1>;
>                 ...
>         };
> };
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  drivers/spi/spi-sh-msiof.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 2c53fc3f73af..fdad8d852602 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -541,6 +541,7 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
>  {
>         struct device_node      *np = spi->master->dev.of_node;
>         struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
> +       int ret;
>
>         pm_runtime_get_sync(&p->pdev->dev);
>
> @@ -559,8 +560,12 @@ static int sh_msiof_spi_setup(struct spi_device *spi)
>                                   !!(spi->mode & SPI_LSB_FIRST),
>                                   !!(spi->mode & SPI_CS_HIGH));
>
> -       if (spi->cs_gpio >= 0)
> -               gpio_set_value(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> +       if (gpio_is_valid(spi->cs_gpio)) {
> +               ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
> +               if (!ret)
> +                       gpio_direction_output(spi->cs_gpio,
> +                                       !(spi->mode & SPI_CS_HIGH));
> +       }

The GPIO is never freed, and .setup() is called multiple times.

In addition, sh_msiof_spi_setup() writes to hardware registers.
Hence if multiple SPI slaves are present (e.g. hardware CS plus GPIO CS,
or multiple GPIO CSses), SPI transfers to a slave may use some configuration
from the previous call to .setup() for another SPI slave.

>         pm_runtime_put(&p->pdev->dev);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Dirk Behme]

On 07.09.2017 10:11, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>
>> This patch is for debug of transfer between master and slave.
>> Since the slave needs to complete a preparation in data transfer
>> before the master working, the sleep wait is put before
>> the data transfer of the master.
>>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> 
> IMHO this is a pure debug patch, not intended for upstream.


Dropped for v2.


Thanks

Dirk

Re: [PATCH 1/8] spi: sh-msiof: Add sleep before master transfer for test

[Dirk Behme]

On 07.09.2017 10:11, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>
>> This patch is for debug of transfer between master and slave.
>> Since the slave needs to complete a preparation in data transfer
>> before the master working, the sleep wait is put before
>> the data transfer of the master.
>>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> 
> IMHO this is a pure debug patch, not intended for upstream.


Dropped for v2.


Thanks

Dirk

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> DMA supports 32-bit words only,
> even if BITLEN1 of SITMDR2 register is 16bit.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  drivers/spi/spi-sh-msiof.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 2b4d3a520176..f9300fdf41e5 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>                                 break;
>                         copy32 = copy_bswap32;
>                 } else if (bits <= 16) {
> -                       if (l & 1)
> +                       if (l & 3)
>                                 break;
>                         copy32 = copy_wswap32;
>                 } else {

Looks OK, as l is in bytes, not 16-bit words.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

On 07.09.2017 10:31, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>
>> DMA supports 32-bit words only,
>> even if BITLEN1 of SITMDR2 register is 16bit.
>>
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>   drivers/spi/spi-sh-msiof.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>> index 2b4d3a520176..f9300fdf41e5 100644
>> --- a/drivers/spi/spi-sh-msiof.c
>> +++ b/drivers/spi/spi-sh-msiof.c
>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master *master,
>>                                  break;
>>                          copy32 = copy_bswap32;
>>                  } else if (bits <= 16) {
>> -                       if (l & 1)
>> +                       if (l & 3)
>>                                  break;
>>                          copy32 = copy_wswap32;
>>                  } else {
> 
> Looks OK, as l is in bytes, not 16-bit words.
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>


What do you think about CCing this to -stable?

Best regards

Dirk

Re: [PATCH 4/8] spi: sh-msiof: Fix DMA completion

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>
> When reception DMA completes before transmission DMA, next transmission
> DMA may not be able to start. This patch adds wait_for_completion_timeout()
> to both of reception DMA and transmission DMA.
>
> If the driver waits only for the Rx DMA completion, the Tx DMA completion
> thread of DMA Engine may be still processing.
>
> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
> [reword commit message]
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> [adjust context]
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Is this still needed after "[PATCH 5/8] spi: sh-msiof: Wait for Tx
FIFO empty after DMA"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 4/8] spi: sh-msiof: Fix DMA completion

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
> From: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> When reception DMA completes before transmission DMA, next transmission
> DMA may not be able to start. This patch adds wait_for_completion_timeout()
> to both of reception DMA and transmission DMA.
>
> If the driver waits only for the Rx DMA completion, the Tx DMA completion
> thread of DMA Engine may be still processing.
>
> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> [reword commit message]
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> [adjust context]
> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>

Is this still needed after "[PATCH 5/8] spi: sh-msiof: Wait for Tx
FIFO empty after DMA"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] spi: sh-msiof: Wait for Tx FIFO empty after DMA

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> When Tx DMA is only used, Tx FIFO is still not empty after DMA callback.
> This patch waits for sweeping data out of the Tx FIFO.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> [adjust context]
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  drivers/spi/spi-sh-msiof.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 660b03ed6770..a960e8da123d 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -638,6 +638,17 @@ static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p,
>                                 dev_err(&p->pdev->dev, "Tx DMA timeout\n");
>                                 return -ETIMEDOUT;
>                         }
> +                       if (!rx) {

May the issue also happen is TX and RX are used?
See also "[PATCH 4/8] spi: sh-msiof: Fix DMA completion".

> +                               sh_msiof_write(p, IER, IER_TEOFE);
> +
> +                               /* wait for tx fifo to be emptied */
> +                               if (!wait_for_completion_timeout(&p->done,
> +                                                                HZ)) {
> +                                       dev_err(&p->pdev->dev,
> +                                       "Tx fifo to be emptied timeout\n");
> +                                       return -ETIMEDOUT;
> +                               }
> +                       }
>                 }
>                 if (rx) {
>                         if (!wait_for_completion_timeout(&p->done_dma_rx,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/8] spi: sh-msiof: Wait for Tx FIFO empty after DMA

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>
> When Tx DMA is only used, Tx FIFO is still not empty after DMA callback.
> This patch waits for sweeping data out of the Tx FIFO.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> [adjust context]
> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/spi/spi-sh-msiof.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 660b03ed6770..a960e8da123d 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -638,6 +638,17 @@ static int sh_msiof_wait_for_completion(struct sh_msiof_spi_priv *p,
>                                 dev_err(&p->pdev->dev, "Tx DMA timeout\n");
>                                 return -ETIMEDOUT;
>                         }
> +                       if (!rx) {

May the issue also happen is TX and RX are used?
See also "[PATCH 4/8] spi: sh-msiof: Fix DMA completion".

> +                               sh_msiof_write(p, IER, IER_TEOFE);
> +
> +                               /* wait for tx fifo to be emptied */
> +                               if (!wait_for_completion_timeout(&p->done,
> +                                                                HZ)) {
> +                                       dev_err(&p->pdev->dev,
> +                                       "Tx fifo to be emptied timeout\n");
> +                                       return -ETIMEDOUT;
> +                               }
> +                       }
>                 }
>                 if (rx) {
>                         if (!wait_for_completion_timeout(&p->done_dma_rx,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/8] spi: sh-msiof: Add MSIOF parent clock changing function for R-Car Gen3

[Geert Uytterhoeven]

Hi Dirk,

On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  drivers/spi/spi-sh-msiof.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index a960e8da123d..2c53fc3f73af 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -188,6 +188,14 @@ struct sh_msiof_spi_priv {
>  #define IER_RFOVFE     0x00000008 /* Receive FIFO Overflow Enable */
>
>
> +static int msiof_rcar_is_gen3(struct device *dev)
> +{
> +       struct device_node *node = dev->of_node;
> +
> +       return of_device_is_compatible(node, "renesas,msiof-r8a7795") ||
> +               of_device_is_compatible(node, "renesas,msiof-r8a7796");

Things like this should be done by adding a flag to struct sh_msiof_chipdata
instead.

> +}
> +
>  static u32 sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
>  {
>         switch (reg_offs) {
> @@ -1252,6 +1260,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>         const struct of_device_id *of_id;
>         struct sh_msiof_spi_info *info;
>         struct sh_msiof_spi_priv *p;
> +       struct clk *ref_clk;
> +       u32 clk_rate = 0;
>         int i;
>         int ret;
>
> @@ -1352,6 +1362,17 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>                 goto err2;
>         }
>
> +       if (msiof_rcar_is_gen3(&master->dev)) {
> +               ref_clk = devm_clk_get(&pdev->dev, "msiof_ref_clk");

Where is this clock coming from?
The MSIOF DT bindings specify a single clock only.

> +               if (!IS_ERR(ref_clk))
> +                       clk_rate = clk_get_rate(ref_clk);
> +               if (clk_rate) {
> +                       clk_prepare_enable(p->clk);
> +                       clk_set_rate(p->clk, clk_rate);
> +                       clk_disable_unprepare(p->clk);
> +               }
> +       }
> +

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>
>>> DMA supports 32-bit words only,
>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>
>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>> ---
>>>   drivers/spi/spi-sh-msiof.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>> index 2b4d3a520176..f9300fdf41e5 100644
>>> --- a/drivers/spi/spi-sh-msiof.c
>>> +++ b/drivers/spi/spi-sh-msiof.c
>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>> *master,
>>>                                  break;
>>>                          copy32 = copy_bswap32;
>>>                  } else if (bits <= 16) {
>>> -                       if (l & 1)
>>> +                       if (l & 3)
>>>                                  break;
>>>                          copy32 = copy_wswap32;
>>>                  } else {
>>
>>
>> Looks OK, as l is in bytes, not 16-bit words.
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> What do you think about CCing this to -stable?

Sounds OK. Have you tested this?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>> wrote:
>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>
>>> DMA supports 32-bit words only,
>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>
>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>> ---
>>>   drivers/spi/spi-sh-msiof.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>> index 2b4d3a520176..f9300fdf41e5 100644
>>> --- a/drivers/spi/spi-sh-msiof.c
>>> +++ b/drivers/spi/spi-sh-msiof.c
>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>> *master,
>>>                                  break;
>>>                          copy32 = copy_bswap32;
>>>                  } else if (bits <= 16) {
>>> -                       if (l & 1)
>>> +                       if (l & 3)
>>>                                  break;
>>>                          copy32 = copy_wswap32;
>>>                  } else {
>>
>>
>> Looks OK, as l is in bytes, not 16-bit words.
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>
> What do you think about CCing this to -stable?

Sounds OK. Have you tested this?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] spi: sh-msiof: Fix DMA completion

[Dirk Behme]

On 07.09.2017 10:33, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> From: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>>
>> When reception DMA completes before transmission DMA, next transmission
>> DMA may not be able to start. This patch adds wait_for_completion_timeout()
>> to both of reception DMA and transmission DMA.
>>
>> If the driver waits only for the Rx DMA completion, the Tx DMA completion
>> thread of DMA Engine may be still processing.
>>
>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt@renesas.com>
>> [reword commit message]
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>> [adjust context]
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> 
> Is this still needed after "[PATCH 5/8] spi: sh-msiof: Wait for Tx
> FIFO empty after DMA"?


Do you have a proposal for nice test cases you like to see covered?


Best regards

Dirk

Re: [PATCH 4/8] spi: sh-msiof: Fix DMA completion

[Dirk Behme]

On 07.09.2017 10:33, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>
>> When reception DMA completes before transmission DMA, next transmission
>> DMA may not be able to start. This patch adds wait_for_completion_timeout()
>> to both of reception DMA and transmission DMA.
>>
>> If the driver waits only for the Rx DMA completion, the Tx DMA completion
>> thread of DMA Engine may be still processing.
>>
>> Signed-off-by: Ryo Kataoka <ryo.kataoka.wt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> [reword commit message]
>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> [adjust context]
>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
> 
> Is this still needed after "[PATCH 5/8] spi: sh-msiof: Wait for Tx
> FIFO empty after DMA"?


Do you have a proposal for nice test cases you like to see covered?


Best regards

Dirk




--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

On 07.09.2017 10:39, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>
>>>> DMA supports 32-bit words only,
>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>
>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>> ---
>>>>    drivers/spi/spi-sh-msiof.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>>> *master,
>>>>                                   break;
>>>>                           copy32 = copy_bswap32;
>>>>                   } else if (bits <= 16) {
>>>> -                       if (l & 1)
>>>> +                       if (l & 3)
>>>>                                   break;
>>>>                           copy32 = copy_wswap32;
>>>>                   } else {
>>>
>>>
>>> Looks OK, as l is in bytes, not 16-bit words.
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> What do you think about CCing this to -stable?
> 
> Sounds OK. Have you tested this?


I tested all 8 patches together if they fix the issues I observed with 
plain mainline.

Best regards

Dirk

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>> wrote:
>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>
>>>>> DMA supports 32-bit words only,
>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>
>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>> ---
>>>>>    drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>>>> *master,
>>>>>                                   break;
>>>>>                           copy32 = copy_bswap32;
>>>>>                   } else if (bits <= 16) {
>>>>> -                       if (l & 1)
>>>>> +                       if (l & 3)
>>>>>                                   break;
>>>>>                           copy32 = copy_wswap32;
>>>>>                   } else {
>>>>
>>>>
>>>>
>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>
>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>> What do you think about CCing this to -stable?
>>
>> Sounds OK. Have you tested this?
>
> I tested all 8 patches together if they fix the issues I observed with plain
> mainline.

Could you please elaborate about the issues you observed with plain mainline?
It may help to identify which patches are responsible for fixing them.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

On 07.09.2017 10:59, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>> wrote:
>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>
>>>>>> DMA supports 32-bit words only,
>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>
>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>> ---
>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>>>>> *master,
>>>>>>                                    break;
>>>>>>                            copy32 = copy_bswap32;
>>>>>>                    } else if (bits <= 16) {
>>>>>> -                       if (l & 1)
>>>>>> +                       if (l & 3)
>>>>>>                                    break;
>>>>>>                            copy32 = copy_wswap32;
>>>>>>                    } else {
>>>>>
>>>>>
>>>>>
>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>
>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>
>>>> What do you think about CCing this to -stable?
>>>
>>> Sounds OK. Have you tested this?
>>
>> I tested all 8 patches together if they fix the issues I observed with plain
>> mainline.
> 
> Could you please elaborate about the issues you observed with plain mainline?
> It may help to identify which patches are responsible for fixing them.


I've been told that an easy test case is to just cat random data (any 
mid sized file) into SPI. E.g.:

# cat /proc/cpuinfo > /dev/spidev32764.2
[  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
[  504.551265] spidev spi32764.2: SPI transfer failed: -110
[  504.556625] spi_master spi32764: failed to transfer one message from 
queue
[  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
[  504.571177] spidev spi32764.2: SPI transfer failed: -110
[  504.576528] spi_master spi32764: failed to transfer one message from 
queue
cat: write error: Connection timed out

done on plain 4.13-rc6.

Best regards

Dirk

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

On 07.09.2017 10:59, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>> wrote:
>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>> wrote:
>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>>
>>>>>> DMA supports 32-bit words only,
>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>
>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>> ---
>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct spi_master
>>>>>> *master,
>>>>>>                                    break;
>>>>>>                            copy32 = copy_bswap32;
>>>>>>                    } else if (bits <= 16) {
>>>>>> -                       if (l & 1)
>>>>>> +                       if (l & 3)
>>>>>>                                    break;
>>>>>>                            copy32 = copy_wswap32;
>>>>>>                    } else {
>>>>>
>>>>>
>>>>>
>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>
>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>>>
>>>> What do you think about CCing this to -stable?
>>>
>>> Sounds OK. Have you tested this?
>>
>> I tested all 8 patches together if they fix the issues I observed with plain
>> mainline.
> 
> Could you please elaborate about the issues you observed with plain mainline?
> It may help to identify which patches are responsible for fixing them.


I've been told that an easy test case is to just cat random data (any 
mid sized file) into SPI. E.g.:

# cat /proc/cpuinfo > /dev/spidev32764.2
[  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
[  504.551265] spidev spi32764.2: SPI transfer failed: -110
[  504.556625] spi_master spi32764: failed to transfer one message from 
queue
[  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
[  504.571177] spidev spi32764.2: SPI transfer failed: -110
[  504.576528] spi_master spi32764: failed to transfer one message from 
queue
cat: write error: Connection timed out

done on plain 4.13-rc6.

Best regards

Dirk

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>> wrote:
>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>>> wrote:
>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>> DMA supports 32-bit words only,
>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>
>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>>> ---
>>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>> spi_master
>>>>>>> *master,
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_bswap32;
>>>>>>>                    } else if (bits <= 16) {
>>>>>>> -                       if (l & 1)
>>>>>>> +                       if (l & 3)
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_wswap32;
>>>>>>>                    } else {
>>>>>>
>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>
>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>
>>>>> What do you think about CCing this to -stable?
>>>>
>>>> Sounds OK. Have you tested this?
>>>
>>> I tested all 8 patches together if they fix the issues I observed with
>>> plain
>>> mainline.
>>
>> Could you please elaborate about the issues you observed with plain
>> mainline?
>> It may help to identify which patches are responsible for fixing them.
>
> I've been told that an easy test case is to just cat random data (any mid
> sized file) into SPI. E.g.:
>
> # cat /proc/cpuinfo > /dev/spidev32764.2
> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
> [  504.556625] spi_master spi32764: failed to transfer one message from
> queue
> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
> [  504.576528] spi_master spi32764: failed to transfer one message from
> queue
> cat: write error: Connection timed out
>
> done on plain 4.13-rc6.

Thank you, that's very valuable information!
We'll give it a try...

Note that this test does not exercise the code path affected by this particular
patch ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>> wrote:
>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>> wrote:
>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>> wrote:
>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>>> DMA supports 32-bit words only,
>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>
>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>>> ---
>>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>> spi_master
>>>>>>> *master,
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_bswap32;
>>>>>>>                    } else if (bits <= 16) {
>>>>>>> -                       if (l & 1)
>>>>>>> +                       if (l & 3)
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_wswap32;
>>>>>>>                    } else {
>>>>>>
>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>
>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>>>>
>>>>> What do you think about CCing this to -stable?
>>>>
>>>> Sounds OK. Have you tested this?
>>>
>>> I tested all 8 patches together if they fix the issues I observed with
>>> plain
>>> mainline.
>>
>> Could you please elaborate about the issues you observed with plain
>> mainline?
>> It may help to identify which patches are responsible for fixing them.
>
> I've been told that an easy test case is to just cat random data (any mid
> sized file) into SPI. E.g.:
>
> # cat /proc/cpuinfo > /dev/spidev32764.2
> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
> [  504.556625] spi_master spi32764: failed to transfer one message from
> queue
> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
> [  504.576528] spi_master spi32764: failed to transfer one message from
> queue
> cat: write error: Connection timed out
>
> done on plain 4.13-rc6.

Thank you, that's very valuable information!
We'll give it a try...

Note that this test does not exercise the code path affected by this particular
patch ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 11:12 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>> wrote:
>>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>>>> wrote:
>>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>>> DMA supports 32-bit words only,
>>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>>
>>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>>>> ---
>>>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>>> spi_master
>>>>>>>> *master,
>>>>>>>>                                    break;
>>>>>>>>                            copy32 = copy_bswap32;
>>>>>>>>                    } else if (bits <= 16) {
>>>>>>>> -                       if (l & 1)
>>>>>>>> +                       if (l & 3)
>>>>>>>>                                    break;
>>>>>>>>                            copy32 = copy_wswap32;
>>>>>>>>                    } else {
>>>>>>>
>>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>>
>>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>
>>>>>> What do you think about CCing this to -stable?
>>>>>
>>>>> Sounds OK. Have you tested this?
>>>>
>>>> I tested all 8 patches together if they fix the issues I observed with
>>>> plain
>>>> mainline.
>>>
>>> Could you please elaborate about the issues you observed with plain
>>> mainline?
>>> It may help to identify which patches are responsible for fixing them.
>>
>> I've been told that an easy test case is to just cat random data (any mid
>> sized file) into SPI. E.g.:
>>
>> # cat /proc/cpuinfo > /dev/spidev32764.2
>> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
>> [  504.556625] spi_master spi32764: failed to transfer one message from
>> queue
>> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
>> [  504.576528] spi_master spi32764: failed to transfer one message from
>> queue
>> cat: write error: Connection timed out
>>
>> done on plain 4.13-rc6.
>
> Thank you, that's very valuable information!
> We'll give it a try...

After some investigation, this is caused by DMA completion triggering
too early for TX-only transfers.  This should indeed be fixed by "[PATCH 5/8]
spi: sh-msiof: Wait for Tx FIFO empty after DMA" you extracted from the BSP.

As that patch needs changes to apply (a) on current spi/for-next and (b)
without other patches from your series, I reworked it into "[PATCH] spi:
sh-msiof: Fix timeout failures for TX-only DMA transfers".

As for the other patches from your series:
  - Upstream already has commit 36735783fdb599c9 ("spi: sh-msiof: Fix DMA
    transfer size check"),
  - spi/for-next has commit b8761434bdec32fa ("spi: sh-msiof: Implement
    cs-gpios configuration").
The remaining were either rejected, retracted, or it is unclear which problem
(if any) they are really fixing.

Do you agree?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Sep 7, 2017 at 11:12 AM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>> wrote:
>>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>> wrote:
>>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>>> wrote:
>>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>>>> DMA supports 32-bit words only,
>>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>>
>>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>>>>>>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>>>>>> ---
>>>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>>> spi_master
>>>>>>>> *master,
>>>>>>>>                                    break;
>>>>>>>>                            copy32 = copy_bswap32;
>>>>>>>>                    } else if (bits <= 16) {
>>>>>>>> -                       if (l & 1)
>>>>>>>> +                       if (l & 3)
>>>>>>>>                                    break;
>>>>>>>>                            copy32 = copy_wswap32;
>>>>>>>>                    } else {
>>>>>>>
>>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>>
>>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>>>>>
>>>>>> What do you think about CCing this to -stable?
>>>>>
>>>>> Sounds OK. Have you tested this?
>>>>
>>>> I tested all 8 patches together if they fix the issues I observed with
>>>> plain
>>>> mainline.
>>>
>>> Could you please elaborate about the issues you observed with plain
>>> mainline?
>>> It may help to identify which patches are responsible for fixing them.
>>
>> I've been told that an easy test case is to just cat random data (any mid
>> sized file) into SPI. E.g.:
>>
>> # cat /proc/cpuinfo > /dev/spidev32764.2
>> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
>> [  504.556625] spi_master spi32764: failed to transfer one message from
>> queue
>> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
>> [  504.576528] spi_master spi32764: failed to transfer one message from
>> queue
>> cat: write error: Connection timed out
>>
>> done on plain 4.13-rc6.
>
> Thank you, that's very valuable information!
> We'll give it a try...

After some investigation, this is caused by DMA completion triggering
too early for TX-only transfers.  This should indeed be fixed by "[PATCH 5/8]
spi: sh-msiof: Wait for Tx FIFO empty after DMA" you extracted from the BSP.

As that patch needs changes to apply (a) on current spi/for-next and (b)
without other patches from your series, I reworked it into "[PATCH] spi:
sh-msiof: Fix timeout failures for TX-only DMA transfers".

As for the other patches from your series:
  - Upstream already has commit 36735783fdb599c9 ("spi: sh-msiof: Fix DMA
    transfer size check"),
  - spi/for-next has commit b8761434bdec32fa ("spi: sh-msiof: Implement
    cs-gpios configuration").
The remaining were either rejected, retracted, or it is unclear which problem
(if any) they are really fixing.

Do you agree?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Dirk Behme]

On 03.01.2018 18:25, Geert Uytterhoeven wrote:
> Hi Dirk,
> 
> On Thu, Sep 7, 2017 at 11:12 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>>>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>> wrote:
>>>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>>> wrote:
>>>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>>>>> wrote:
>>>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>>>> DMA supports 32-bit words only,
>>>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@renesas.com>
>>>>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>>>>> ---
>>>>>>>>>      drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>>>      1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>>>> spi_master
>>>>>>>>> *master,
>>>>>>>>>                                     break;
>>>>>>>>>                             copy32 = copy_bswap32;
>>>>>>>>>                     } else if (bits <= 16) {
>>>>>>>>> -                       if (l & 1)
>>>>>>>>> +                       if (l & 3)
>>>>>>>>>                                     break;
>>>>>>>>>                             copy32 = copy_wswap32;
>>>>>>>>>                     } else {
>>>>>>>>
>>>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>>>
>>>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>>>
>>>>>>> What do you think about CCing this to -stable?
>>>>>>
>>>>>> Sounds OK. Have you tested this?
>>>>>
>>>>> I tested all 8 patches together if they fix the issues I observed with
>>>>> plain
>>>>> mainline.
>>>>
>>>> Could you please elaborate about the issues you observed with plain
>>>> mainline?
>>>> It may help to identify which patches are responsible for fixing them.
>>>
>>> I've been told that an easy test case is to just cat random data (any mid
>>> sized file) into SPI. E.g.:
>>>
>>> # cat /proc/cpuinfo > /dev/spidev32764.2
>>> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
>>> [  504.556625] spi_master spi32764: failed to transfer one message from
>>> queue
>>> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
>>> [  504.576528] spi_master spi32764: failed to transfer one message from
>>> queue
>>> cat: write error: Connection timed out
>>>
>>> done on plain 4.13-rc6.
>>
>> Thank you, that's very valuable information!
>> We'll give it a try...
> 
> After some investigation, this is caused by DMA completion triggering
> too early for TX-only transfers.  This should indeed be fixed by "[PATCH 5/8]
> spi: sh-msiof: Wait for Tx FIFO empty after DMA" you extracted from the BSP.
> 
> As that patch needs changes to apply (a) on current spi/for-next and (b)
> without other patches from your series, I reworked it into "[PATCH] spi:
> sh-msiof: Fix timeout failures for TX-only DMA transfers".
> 
> As for the other patches from your series:
>    - Upstream already has commit 36735783fdb599c9 ("spi: sh-msiof: Fix DMA
>      transfer size check"),
>    - spi/for-next has commit b8761434bdec32fa ("spi: sh-msiof: Implement
>      cs-gpios configuration").
> The remaining were either rejected, retracted, or it is unclear which problem
> (if any) they are really fixing.


First, many thanks for looking into this!


> Do you agree?


If there are no more issues, then I agree ;) So lets rephrase that a 
little: We'll test what we have, now. And in case we find any remaining 
issues, we'll come back with more details, then. What will help us to 
look at the remaining patches.

I apologize already now in case testing will take some time ;)

Anyhow, many thanks again for looking into this.

Best regards

Dirk

Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Jan 4, 2018 at 8:19 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 03.01.2018 18:25, Geert Uytterhoeven wrote:
>> On Thu, Sep 7, 2017 at 11:12 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>>>>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>>> wrote:
>>>>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>>>>> Sounds OK. Have you tested this?
>>>>>>
>>>>>> I tested all 8 patches together if they fix the issues I observed with
>>>>>> plain
>>>>>> mainline.
>>>>>
>>>>> Could you please elaborate about the issues you observed with plain
>>>>> mainline?
>>>>> It may help to identify which patches are responsible for fixing them.
>>>>
>>>> I've been told that an easy test case is to just cat random data (any
>>>> mid
>>>> sized file) into SPI. E.g.:
>>>>
>>>> # cat /proc/cpuinfo > /dev/spidev32764.2
>>>> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>>> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
>>>> [  504.556625] spi_master spi32764: failed to transfer one message from
>>>> queue
>>>> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>>> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
>>>> [  504.576528] spi_master spi32764: failed to transfer one message from
>>>> queue
>>>> cat: write error: Connection timed out
>>>>
>>>> done on plain 4.13-rc6.
>>>
>>> Thank you, that's very valuable information!
>>> We'll give it a try...
>>
>> After some investigation, this is caused by DMA completion triggering
>> too early for TX-only transfers.  This should indeed be fixed by "[PATCH
>> 5/8]
>> spi: sh-msiof: Wait for Tx FIFO empty after DMA" you extracted from the
>> BSP.
>>
>> As that patch needs changes to apply (a) on current spi/for-next and (b)
>> without other patches from your series, I reworked it into "[PATCH] spi:
>> sh-msiof: Fix timeout failures for TX-only DMA transfers".
>>
>> As for the other patches from your series:
>>    - Upstream already has commit 36735783fdb599c9 ("spi: sh-msiof: Fix DMA
>>      transfer size check"),
>>    - spi/for-next has commit b8761434bdec32fa ("spi: sh-msiof: Implement
>>      cs-gpios configuration").
>> The remaining were either rejected, retracted, or it is unclear which
>> problem
>> (if any) they are really fixing.
>
> First, many thanks for looking into this!
>
>> Do you agree?
>
> If there are no more issues, then I agree ;) So lets rephrase that a little:
> We'll test what we have, now. And in case we find any remaining issues,
> we'll come back with more details, then. What will help us to look at the
> remaining patches.

Sure, if more issues can be identified, we should look into fixing them as
well.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds