spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

PATCH In review Sylwester Nawrocki 3 participants 19 messages Apr 17, 2018 – Apr 18, 2018 1 message not in the archive

Subsystems (3)

SPI Arm Samsung Exynos Samsung SPI drivers

Share & tools

A message that is not in the archive (sent off-list or not yet received): CGME20180417143022epcas2p1185d48998a1659ce6580180ea6725607@epcas2p1.samsung.com

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #1 lore raw

PATCH v2 1/8 spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

Currently timeout errors in polling I/O mode transfer are silently ignored. Fix it by returning an error when we time out waiting on the RX FIFO level to reach the transfer length.

Reviewed-by Andi Shyti <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/spi/spi-s3c64xx.c +2 -0

index baa3a9fa2638..9a990bc3138a 100644
@@ -505,6 +505,8 @@ static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
 		status = readl(regs + S3C64XX_SPI_STATUS);
 	} while (RX_FIFO_LVL(status, sdd) < xfer->len && --val);
 
+	if (!val)
+		return -EIO;
 
 	/* If it was only Tx */
 	if (!xfer->rx_buf) {

Signature · 1 line

2.14.2

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #2 lore raw

PATCH v2 2/8 spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

The tgl_spi pointer is now unused so remove it.

Reviewed-by Andi Shyti <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 2 --
 1 file changed, 2 deletions(-)

drivers/spi/spi-s3c64xx.c +0 -2

index 9a990bc3138a..f0e28adbbc32 100644
@@ -156,7 +156,6 @@ struct s3c64xx_spi_port_config {
  * @ioclk: Pointer to the i/o clock between master and slave
  * @master: Pointer to the SPI Protocol master.
  * @cntrlr_info: Platform specific data for the controller this driver manages.
- * @tgl_spi: Pointer to the last CS left untoggled by the cs_change hint.
  * @lock: Controller specific lock.
  * @state: Set of FLAGS to indicate status.
  * @rx_dmach: Controller's DMA channel for Rx.

@@ -177,7 +176,6 @@ struct s3c64xx_spi_driver_data {
 	struct platform_device          *pdev;
 	struct spi_master               *master;
 	struct s3c64xx_spi_info  *cntrlr_info;
-	struct spi_device               *tgl_spi;
 	spinlock_t                      lock;
 	unsigned long                   sfr_start;
 	struct completion               xfer_completion;

Signature · 1 line

2.14.2

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #4 lore raw

PATCH v2 4/8 spi: spi-s3c64xx: Drop unused enable_datapath() function argument

In reply to Sylwester Nawrocki

The spi pointer argument is not used now so remove it.

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/spi/spi-s3c64xx.c +2 -3

index 4c27426bcb40..5d94915c2738 100644
@@ -349,8 +349,7 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 }
 
 static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				struct spi_device *spi,
-				struct spi_transfer *xfer, int dma_mode)
+			    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;

@@ -667,7 +666,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;
 
-	enable_datapath(sdd, spi, xfer, use_dma);
+	enable_datapath(sdd, xfer, use_dma);
 
 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);

Signature · 1 line

2.14.2

andi at etezian.org

2018-04-17 18:09 UTC #5 lore raw

PATCH v2 4/8 spi: spi-s3c64xx: Drop unused enable_datapath() function argument

Hi Sylwester,

Quoted 3 lines · Sylwester Nawrocki

The spi pointer argument is not used now so remove it.
Signed-off-by Sylwester Nawrocki <[email protected]>

Thanks for splitting it!

Reviewed-by Andi Shyti <[email protected]>

Andi

Mark Brown

maintainer

2018-04-18 11:40 UTC #6 lore raw

Applied "spi: spi-s3c64xx: Drop unused enable_datapath() function argument" to the spi tree

In reply to Sylwester Nawrocki

The patch

spi: spi-s3c64xx: Drop unused enable_datapath() function argument

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 80aa0d746c10e14ac14ca03b2bc8b26a0f75c5e4 Mon Sep 17 00:00:00 2001

From: Sylwester Nawrocki <[email protected]>
Date: Tue, 17 Apr 2018 16:29:50 +0200
Subject: [PATCH] spi: spi-s3c64xx: Drop unused enable_datapath() function
argument

The spi pointer argument is not used now so remove it.

Signed-off-by Sylwester Nawrocki <[email protected]>

Reviewed-by Andi Shyti <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/spi/spi-s3c64xx.c +2 -3

index 54fde5c80374..38a5e9ce5f11 100644
@@ -349,8 +349,7 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 }
 
 static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				struct spi_device *spi,
-				struct spi_transfer *xfer, int dma_mode)
+			    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;

@@ -667,7 +666,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;
 
-	enable_datapath(sdd, spi, xfer, use_dma);
+	enable_datapath(sdd, xfer, use_dma);
 
 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);

Signature · 1 line

2.17.0

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #7 lore raw

PATCH v2 5/8 spi: spi-s3c64xx: Add missing s3c64xx_ prefix to function names

In reply to Sylwester Nawrocki

Add a s3c64xx_ prefix to remaining generic function names so it is clear the code is part of the driver when grepping or looking at debug logs.

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/spi/spi-s3c64xx.c +12 ~~-12~~

index 5d94915c2738..b9b406138675 100644
@@ -188,7 +188,7 @@ struct s3c64xx_spi_driver_data {
 	unsigned int			port_id;
 };
 
-static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
+static void s3c64xx_flush_fifo(struct s3c64xx_spi_driver_data *sdd)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long loops;

@@ -348,8 +348,8 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 	return xfer->len > (FIFO_LVL_MASK(sdd) >> 1) + 1;
 }
 
-static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-			    struct spi_transfer *xfer, int dma_mode)
+static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
+				    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;

@@ -439,8 +439,8 @@ static u32 s3c64xx_spi_wait_for_timeout(struct s3c64xx_spi_driver_data *sdd,
 	return RX_FIFO_LVL(status, sdd);
 }
 
-static int wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
-			struct spi_transfer *xfer)
+static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
+				struct spi_transfer *xfer)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long val;

@@ -482,8 +482,8 @@ static int wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
 	return 0;
 }
 
-static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
-			struct spi_transfer *xfer)
+static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
+				struct spi_transfer *xfer)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long val;

@@ -666,7 +666,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;
 
-	enable_datapath(sdd, xfer, use_dma);
+	s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);

@@ -674,9 +674,9 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	spin_unlock_irqrestore(&sdd->lock, flags);
 
 	if (use_dma)
-		status = wait_for_dma(sdd, xfer);
+		status = s3c64xx_wait_for_dma(sdd, xfer);
 	else
-		status = wait_for_pio(sdd, xfer);
+		status = s3c64xx_wait_for_pio(sdd, xfer);
 
 	if (status) {
 		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",

@@ -694,7 +694,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				dmaengine_terminate_all(sdd->rx_dma.ch);
 		}
 	} else {
-		flush_fifo(sdd);
+		s3c64xx_flush_fifo(sdd);
 	}
 
 	return status;

@@ -928,7 +928,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
 	val |= (S3C64XX_SPI_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
 	writel(val, regs + S3C64XX_SPI_MODE_CFG);
 
-	flush_fifo(sdd);
+	s3c64xx_flush_fifo(sdd);
 }
 
 #ifdef CONFIG_OF

Signature · 1 line

2.14.2

andi at etezian.org

2018-04-17 18:11 UTC #8 lore raw

PATCH v2 5/8 spi: spi-s3c64xx: Add missing s3c64xx_ prefix to function names

Hi Sylwester,

Quoted 5 lines · Sylwester Nawrocki

Add a s3c64xx_ prefix to remaining generic function names so it is clear
the code is part of the driver when grepping or looking at debug logs.
Signed-off-by Sylwester Nawrocki <[email protected]>

thank you :)

Reviewed-by Andi Shyti <[email protected]>

Andi

Mark Brown

maintainer

2018-04-18 11:40 UTC #9 lore raw

Applied "spi: spi-s3c64xx: Add missing s3c64xx_ prefix to function names" to the spi tree

In reply to Sylwester Nawrocki

The patch

spi: spi-s3c64xx: Add missing s3c64xx_ prefix to function names

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 3655d30c054816331820842b0a1db75f58aeadc9 Mon Sep 17 00:00:00 2001

From: Sylwester Nawrocki <[email protected]>
Date: Tue, 17 Apr 2018 16:29:51 +0200
Subject: [PATCH] spi: spi-s3c64xx: Add missing s3c64xx_ prefix to function
names

Add a s3c64xx_ prefix to remaining generic function names so it is clear the code is part of the driver when grepping or looking at debug logs.

Signed-off-by Sylwester Nawrocki <[email protected]>

Reviewed-by Andi Shyti <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/spi/spi-s3c64xx.c +12 ~~-12~~

index 38a5e9ce5f11..db33bc9b4147 100644
@@ -188,7 +188,7 @@ struct s3c64xx_spi_driver_data {
 	unsigned int			port_id;
 };
 
-static void flush_fifo(struct s3c64xx_spi_driver_data *sdd)
+static void s3c64xx_flush_fifo(struct s3c64xx_spi_driver_data *sdd)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long loops;

@@ -348,8 +348,8 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 	return xfer->len > (FIFO_LVL_MASK(sdd) >> 1) + 1;
 }
 
-static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-			    struct spi_transfer *xfer, int dma_mode)
+static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
+				    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;

@@ -439,8 +439,8 @@ static u32 s3c64xx_spi_wait_for_timeout(struct s3c64xx_spi_driver_data *sdd,
 	return RX_FIFO_LVL(status, sdd);
 }
 
-static int wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
-			struct spi_transfer *xfer)
+static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
+				struct spi_transfer *xfer)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long val;

@@ -482,8 +482,8 @@ static int wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
 	return 0;
 }
 
-static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
-			struct spi_transfer *xfer)
+static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
+				struct spi_transfer *xfer)
 {
 	void __iomem *regs = sdd->regs;
 	unsigned long val;

@@ -666,7 +666,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;
 
-	enable_datapath(sdd, xfer, use_dma);
+	s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);

@@ -674,9 +674,9 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	spin_unlock_irqrestore(&sdd->lock, flags);
 
 	if (use_dma)
-		status = wait_for_dma(sdd, xfer);
+		status = s3c64xx_wait_for_dma(sdd, xfer);
 	else
-		status = wait_for_pio(sdd, xfer);
+		status = s3c64xx_wait_for_pio(sdd, xfer);
 
 	if (status) {
 		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",

@@ -694,7 +694,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				dmaengine_terminate_all(sdd->rx_dma.ch);
 		}
 	} else {
-		flush_fifo(sdd);
+		s3c64xx_flush_fifo(sdd);
 	}
 
 	return status;

@@ -928,7 +928,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
 	val |= (S3C64XX_SPI_TRAILCNT << S3C64XX_SPI_TRAILCNT_OFF);
 	writel(val, regs + S3C64XX_SPI_MODE_CFG);
 
-	flush_fifo(sdd);
+	s3c64xx_flush_fifo(sdd);
 }
 
 #ifdef CONFIG_OF

Signature · 1 line

2.17.0

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #10 lore raw

PATCH v2 6/8 spi: spi-s3c64xx: Fix indentation in the register offset definitions

In reply to Sylwester Nawrocki

Change indentation so register address offset and register bit definitions are aligned to same column.

Reviewed-by Andi Shyti <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

Signed-off-by Sylwester Nawrocki <[email protected]>

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

drivers/spi/spi-s3c64xx.c +8 -8

index b9b406138675..db33bc9b4147 100644
@@ -28,15 +28,15 @@
 
 #define S3C64XX_SPI_CH_CFG		0x00
 #define S3C64XX_SPI_CLK_CFG		0x04
-#define S3C64XX_SPI_MODE_CFG	0x08
-#define S3C64XX_SPI_SLAVE_SEL	0x0C
+#define S3C64XX_SPI_MODE_CFG		0x08
+#define S3C64XX_SPI_SLAVE_SEL		0x0C
 #define S3C64XX_SPI_INT_EN		0x10
 #define S3C64XX_SPI_STATUS		0x14
 #define S3C64XX_SPI_TX_DATA		0x18
 #define S3C64XX_SPI_RX_DATA		0x1C
-#define S3C64XX_SPI_PACKET_CNT	0x20
-#define S3C64XX_SPI_PENDING_CLR	0x24
-#define S3C64XX_SPI_SWAP_CFG	0x28
+#define S3C64XX_SPI_PACKET_CNT		0x20
+#define S3C64XX_SPI_PENDING_CLR		0x24
+#define S3C64XX_SPI_SWAP_CFG		0x28
 #define S3C64XX_SPI_FB_CLK		0x2C
 
 #define S3C64XX_SPI_CH_HS_EN		(1<<6)	/* High Speed Enable */

@@ -77,9 +77,9 @@
 #define S3C64XX_SPI_INT_TX_FIFORDY_EN		(1<<0)
 
 #define S3C64XX_SPI_ST_RX_OVERRUN_ERR		(1<<5)
-#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR	(1<<4)
+#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR		(1<<4)
 #define S3C64XX_SPI_ST_TX_OVERRUN_ERR		(1<<3)
-#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR	(1<<2)
+#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR		(1<<2)
 #define S3C64XX_SPI_ST_RX_FIFORDY		(1<<1)
 #define S3C64XX_SPI_ST_TX_FIFORDY		(1<<0)

@@ -100,7 +100,7 @@
 #define S3C64XX_SPI_SWAP_TX_BIT			(1<<1)
 #define S3C64XX_SPI_SWAP_TX_EN			(1<<0)
 
-#define S3C64XX_SPI_FBCLK_MSK		(3<<0)
+#define S3C64XX_SPI_FBCLK_MSK			(3<<0)
 
 #define FIFO_LVL_MASK(i) ((i)->port_conf->fifo_lvl_mask[i->port_id])
 #define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & \

Signature · 1 line

2.14.2

Mark Brown

maintainer

2018-04-18 10:47 UTC #11 lore raw

PATCH v2 6/8 spi: spi-s3c64xx: Fix indentation in the register offset definitions

Quoted 2 lines · Sylwester Nawrocki

Change indentation so register address offset and register bit definitions are aligned to same column.

This doesn't apply against current code, please check and resend.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180418/3a28d070/attachment.sig>

Sylwester Nawrocki

author

2018-04-18 12:17 UTC #12 lore raw

PATCH v2 6/8 spi: spi-s3c64xx: Fix indentation in the register offset definitions

Quoted 5 lines · Mark Brown

Quoted 2 lines · Sylwester Nawrocki
Change indentation so register address offset and register bit definitions are aligned to same column.
This doesn't apply against current code, please check and resend.

It seems there is no need to resend now as all patches from the series are in your for-4.18 branch.

Signature · 2 lines

Thanks,
Sylwester

Mark Brown

maintainer

2018-04-18 13:49 UTC #13 lore raw

PATCH v2 6/8 spi: spi-s3c64xx: Fix indentation in the register offset definitions

Quoted 6 lines · Sylwester Nawrocki

Quoted 1 line · Mark Brown
This doesn't apply against current code, please check and resend.
It seems there is no need to resend now as all patches from the series are in your for-4.18 branch.

I applied a bunch of the series yesterday, it looks like something that
was applied then.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180418/12d8e2f6/attachment.sig>

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #14 lore raw

PATCH v2 7/8 spi: spi-s3c64xx: Use local variable for FIFO length

In reply to Sylwester Nawrocki

More references to fifo_len are added in subsequent patch.

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/spi/spi-s3c64xx.c +3 -3

index db33bc9b4147..27cd8c59eabf 100644
@@ -634,6 +634,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				    struct spi_transfer *xfer)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
+	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
 	int status;
 	u32 speed;
 	u8 bpw;

@@ -655,9 +656,8 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 
 	/* Polling method for xfers not bigger than FIFO capacity */
 	use_dma = 0;
-	if (!is_polling(sdd) &&
-	    (sdd->rx_dma.ch && sdd->tx_dma.ch &&
-	     (xfer->len > ((FIFO_LVL_MASK(sdd) >> 1) + 1))))
+	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
+	    sdd->rx_dma.ch && sdd->tx_dma.ch)
 		use_dma = 1;
 
 	spin_lock_irqsave(&sdd->lock, flags);

Signature · 1 line

2.14.2

andi at etezian.org

2018-04-17 18:14 UTC #15 lore raw

PATCH v2 7/8 spi: spi-s3c64xx: Use local variable for FIFO length

Hi Sylwester,

Quoted 3 lines · Sylwester Nawrocki

More references to fifo_len are added in subsequent patch.
Signed-off-by Sylwester Nawrocki <[email protected]>

thanks also for this! It's trivial, but I like it better on a separate patch.

Reviewed-by Andi Shyti <[email protected]>

Andi

Mark Brown

maintainer

2018-04-18 11:40 UTC #16 lore raw

Applied "spi: spi-s3c64xx: Use local variable for FIFO length" to the spi tree

In reply to Sylwester Nawrocki

The patch

spi: spi-s3c64xx: Use local variable for FIFO length

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From f6364e66c66135f40709635849620411f3f8a2ff Mon Sep 17 00:00:00 2001

From: Sylwester Nawrocki <[email protected]>
Date: Tue, 17 Apr 2018 16:29:53 +0200
Subject: [PATCH] spi: spi-s3c64xx: Use local variable for FIFO length

More references to fifo_len are added in subsequent patch.

Signed-off-by Sylwester Nawrocki <[email protected]>

Reviewed-by Andi Shyti <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/spi/spi-s3c64xx.c +3 -3

index db33bc9b4147..27cd8c59eabf 100644
@@ -634,6 +634,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				    struct spi_transfer *xfer)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
+	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
 	int status;
 	u32 speed;
 	u8 bpw;

@@ -655,9 +656,8 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 
 	/* Polling method for xfers not bigger than FIFO capacity */
 	use_dma = 0;
-	if (!is_polling(sdd) &&
-	    (sdd->rx_dma.ch && sdd->tx_dma.ch &&
-	     (xfer->len > ((FIFO_LVL_MASK(sdd) >> 1) + 1))))
+	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
+	    sdd->rx_dma.ch && sdd->tx_dma.ch)
 		use_dma = 1;
 
 	spin_lock_irqsave(&sdd->lock, flags);

Signature · 1 line

2.17.0

Sylwester Nawrocki

author

2018-04-17 14:29 UTC #17 lore raw

PATCH v2 8/8 spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

In reply to Sylwester Nawrocki

Some variants of the SPI controller have no DMA support, in such case SPI transfers longer than the FIFO length are not currently properly handled by the driver. Fix it by doing multiple transfers in the s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds the FIFO size.

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 99 +++++++++++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 33 deletions(-)

drivers/spi/spi-s3c64xx.c +66 ~~-33~~

index 27cd8c59eabf..755ab2dc6969 100644
@@ -635,11 +635,14 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
 	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
+	const void *tx_buf = NULL;
+	void *rx_buf = NULL;
+	int target_len = 0, origin_len = 0;
+	int use_dma = 0;
 	int status;
 	u32 speed;
 	u8 bpw;
 	unsigned long flags;
-	int use_dma;
 
 	reinit_completion(&sdd->xfer_completion);

@@ -654,47 +657,77 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 		s3c64xx_spi_config(sdd);
 	}
 
-	/* Polling method for xfers not bigger than FIFO capacity */
-	use_dma = 0;
 	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
-	    sdd->rx_dma.ch && sdd->tx_dma.ch)
+	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
 		use_dma = 1;
 
-	spin_lock_irqsave(&sdd->lock, flags);
+	} else if (is_polling(sdd) && xfer->len > fifo_len) {
+		tx_buf = xfer->tx_buf;
+		rx_buf = xfer->rx_buf;
+		origin_len = xfer->len;
 
-	/* Pending only which is to be done */
-	sdd->state &= ~RXBUSY;
-	sdd->state &= ~TXBUSY;
+		target_len = xfer->len;
+		if (xfer->len > fifo_len)
+			xfer->len = fifo_len;
+	}
+
+	do {
+		spin_lock_irqsave(&sdd->lock, flags);
 
-	s3c64xx_enable_datapath(sdd, xfer, use_dma);
+		/* Pending only which is to be done */
+		sdd->state &= ~RXBUSY;
+		sdd->state &= ~TXBUSY;
 
-	/* Start the signals */
-	s3c64xx_spi_set_cs(spi, true);
+		s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
-	spin_unlock_irqrestore(&sdd->lock, flags);
+		/* Start the signals */
+		s3c64xx_spi_set_cs(spi, true);
 
-	if (use_dma)
-		status = s3c64xx_wait_for_dma(sdd, xfer);
-	else
-		status = s3c64xx_wait_for_pio(sdd, xfer);
-
-	if (status) {
-		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
-			xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
-			(sdd->state & RXBUSY) ? 'f' : 'p',
-			(sdd->state & TXBUSY) ? 'f' : 'p',
-			xfer->len);
-
-		if (use_dma) {
-			if (xfer->tx_buf != NULL
-			    && (sdd->state & TXBUSY))
-				dmaengine_terminate_all(sdd->tx_dma.ch);
-			if (xfer->rx_buf != NULL
-			    && (sdd->state & RXBUSY))
-				dmaengine_terminate_all(sdd->rx_dma.ch);
+		spin_unlock_irqrestore(&sdd->lock, flags);
+
+		if (use_dma)
+			status = s3c64xx_wait_for_dma(sdd, xfer);
+		else
+			status = s3c64xx_wait_for_pio(sdd, xfer);
+
+		if (status) {
+			dev_err(&spi->dev,
+				"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
+				xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
+				(sdd->state & RXBUSY) ? 'f' : 'p',
+				(sdd->state & TXBUSY) ? 'f' : 'p',
+				xfer->len);
+
+			if (use_dma) {
+				if (xfer->tx_buf && (sdd->state & TXBUSY))
+					dmaengine_terminate_all(sdd->tx_dma.ch);
+				if (xfer->rx_buf && (sdd->state & RXBUSY))
+					dmaengine_terminate_all(sdd->rx_dma.ch);
+			}
+		} else {
+			s3c64xx_flush_fifo(sdd);
 		}
-	} else {
-		s3c64xx_flush_fifo(sdd);
+		if (target_len > 0) {
+			target_len -= xfer->len;
+
+			if (xfer->tx_buf)
+				xfer->tx_buf += xfer->len;
+
+			if (xfer->rx_buf)
+				xfer->rx_buf += xfer->len;
+
+			if (target_len > fifo_len)
+				xfer->len = fifo_len;
+			else
+				xfer->len = target_len;
+		}
+	} while (target_len > 0);
+
+	if (origin_len) {
+		/* Restore original xfer buffers and length */
+		xfer->tx_buf = tx_buf;
+		xfer->rx_buf = rx_buf;
+		xfer->len = origin_len;
 	}
 
 	return status;

Signature · 1 line

2.14.2

andi at etezian.org

2018-04-17 18:27 UTC #18 lore raw

PATCH v2 8/8 spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

Hi Sylwester,

Quoted 7 lines · Sylwester Nawrocki

Some variants of the SPI controller have no DMA support, in such case SPI transfers longer than the FIFO length are not currently properly handled by the driver. Fix it by doing multiple transfers in the s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds the FIFO size.
Signed-off-by Sylwester Nawrocki <[email protected]>

thank you! it looks good.

Unfortunately I don't have right now the device with me to test it, in any case you can add

Reviewed-by Andi Shyti <[email protected]>

Thanks,
Andi

Mark Brown

maintainer

2018-04-18 11:40 UTC #19 lore raw

Applied "spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode" to the spi tree

In reply to Sylwester Nawrocki

The patch

spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

has been applied to the spi tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From 0af7af7da6510f12595b81ba98bb15dd7ffed1f3 Mon Sep 17 00:00:00 2001

From: Sylwester Nawrocki <[email protected]>
Date: Tue, 17 Apr 2018 16:29:54 +0200
Subject: [PATCH] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO
mode

Signed-off-by Sylwester Nawrocki <[email protected]>

Reviewed-by Andi Shyti <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 drivers/spi/spi-s3c64xx.c | 99 ++++++++++++++++++++++++++-------------
 1 file changed, 66 insertions(+), 33 deletions(-)

drivers/spi/spi-s3c64xx.c +66 ~~-33~~

index 27cd8c59eabf..755ab2dc6969 100644
@@ -635,11 +635,14 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
 	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
+	const void *tx_buf = NULL;
+	void *rx_buf = NULL;
+	int target_len = 0, origin_len = 0;
+	int use_dma = 0;
 	int status;
 	u32 speed;
 	u8 bpw;
 	unsigned long flags;
-	int use_dma;
 
 	reinit_completion(&sdd->xfer_completion);

@@ -654,47 +657,77 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 		s3c64xx_spi_config(sdd);
 	}
 
-	/* Polling method for xfers not bigger than FIFO capacity */
-	use_dma = 0;
 	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
-	    sdd->rx_dma.ch && sdd->tx_dma.ch)
+	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
 		use_dma = 1;
 
-	spin_lock_irqsave(&sdd->lock, flags);
+	} else if (is_polling(sdd) && xfer->len > fifo_len) {
+		tx_buf = xfer->tx_buf;
+		rx_buf = xfer->rx_buf;
+		origin_len = xfer->len;
 
-	/* Pending only which is to be done */
-	sdd->state &= ~RXBUSY;
-	sdd->state &= ~TXBUSY;
+		target_len = xfer->len;
+		if (xfer->len > fifo_len)
+			xfer->len = fifo_len;
+	}
+
+	do {
+		spin_lock_irqsave(&sdd->lock, flags);
 
-	s3c64xx_enable_datapath(sdd, xfer, use_dma);
+		/* Pending only which is to be done */
+		sdd->state &= ~RXBUSY;
+		sdd->state &= ~TXBUSY;
 
-	/* Start the signals */
-	s3c64xx_spi_set_cs(spi, true);
+		s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
-	spin_unlock_irqrestore(&sdd->lock, flags);
+		/* Start the signals */
+		s3c64xx_spi_set_cs(spi, true);
 
-	if (use_dma)
-		status = s3c64xx_wait_for_dma(sdd, xfer);
-	else
-		status = s3c64xx_wait_for_pio(sdd, xfer);
-
-	if (status) {
-		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
-			xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
-			(sdd->state & RXBUSY) ? 'f' : 'p',
-			(sdd->state & TXBUSY) ? 'f' : 'p',
-			xfer->len);
-
-		if (use_dma) {
-			if (xfer->tx_buf != NULL
-			    && (sdd->state & TXBUSY))
-				dmaengine_terminate_all(sdd->tx_dma.ch);
-			if (xfer->rx_buf != NULL
-			    && (sdd->state & RXBUSY))
-				dmaengine_terminate_all(sdd->rx_dma.ch);
+		spin_unlock_irqrestore(&sdd->lock, flags);
+
+		if (use_dma)
+			status = s3c64xx_wait_for_dma(sdd, xfer);
+		else
+			status = s3c64xx_wait_for_pio(sdd, xfer);
+
+		if (status) {
+			dev_err(&spi->dev,
+				"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
+				xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
+				(sdd->state & RXBUSY) ? 'f' : 'p',
+				(sdd->state & TXBUSY) ? 'f' : 'p',
+				xfer->len);
+
+			if (use_dma) {
+				if (xfer->tx_buf && (sdd->state & TXBUSY))
+					dmaengine_terminate_all(sdd->tx_dma.ch);
+				if (xfer->rx_buf && (sdd->state & RXBUSY))
+					dmaengine_terminate_all(sdd->rx_dma.ch);
+			}
+		} else {
+			s3c64xx_flush_fifo(sdd);
 		}
-	} else {
-		s3c64xx_flush_fifo(sdd);
+		if (target_len > 0) {
+			target_len -= xfer->len;
+
+			if (xfer->tx_buf)
+				xfer->tx_buf += xfer->len;
+
+			if (xfer->rx_buf)
+				xfer->rx_buf += xfer->len;
+
+			if (target_len > fifo_len)
+				xfer->len = fifo_len;
+			else
+				xfer->len = target_len;
+		}
+	} while (target_len > 0);
+
+	if (origin_len) {
+		/* Restore original xfer buffers and length */
+		xfer->tx_buf = tx_buf;
+		xfer->rx_buf = rx_buf;
+		xfer->len = origin_len;
 	}
 
 	return status;

Signature · 1 line

2.17.0