diff mbox series

[4/4] spi: s3c64xx: Fix large transfers with DMA

Message ID 20220916113951.228398-5-vincent.whitchurch@axis.com
State New
Headers show
Series spi: Fix DMA bugs in (not only) spi-s3c64xx | expand

Commit Message

Vincent Whitchurch Sept. 16, 2022, 11:39 a.m. UTC
The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
value is 65535.  Asking the driver to transfer a larger size currently
leads to the DMA transfer timing out.  Fix this by splitting the
transfer as needed.

With this, the len>64 KiB tests in spi-loopback-test pass.

(Note that len==64 KiB tests work even without this patch for some reason.
 The driver programs 0 to the COUNT_VALUE field in that case, but it's
 unclear if it's by design, since the hardware documentation doesn't say
 anything about the behaviour when COUNT_VALUE == 0, so play it safe and
 split at 65535.)

Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/spi/spi-s3c64xx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Krzysztof Kozlowski Sept. 19, 2022, 9:43 a.m. UTC | #1
On 16/09/2022 13:39, Vincent Whitchurch wrote:
> The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> value is 65535.  Asking the driver to transfer a larger size currently
> leads to the DMA transfer timing out.  Fix this by splitting the
> transfer as needed.
> 
> With this, the len>64 KiB tests in spi-loopback-test pass.
> 
> (Note that len==64 KiB tests work even without this patch for some reason.
>  The driver programs 0 to the COUNT_VALUE field in that case, but it's
>  unclear if it's by design, since the hardware documentation doesn't say
>  anything about the behaviour when COUNT_VALUE == 0, so play it safe and
>  split at 65535.)
> 
> Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver")
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Mark Brown Sept. 19, 2022, 2:38 p.m. UTC | #2
On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote:
> The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> value is 65535.  Asking the driver to transfer a larger size currently
> leads to the DMA transfer timing out.  Fix this by splitting the
> transfer as needed.

The driver should just set max_transfer_size and let the core
handle this.
Vincent Whitchurch Sept. 26, 2022, 1:42 p.m. UTC | #3
On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote:
> On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote:
> > The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> > value is 65535.  Asking the driver to transfer a larger size currently
> > leads to the DMA transfer timing out.  Fix this by splitting the
> > transfer as needed.
> 
> The driver should just set max_transfer_size and let the core
> handle this.

Hmm, AFAICS, the core doesn't actually do anything with
max_transfer_size?  It's only used from spi-mem and a handful of other
clients via spi_max_transfer_size().
Mark Brown Sept. 26, 2022, 1:52 p.m. UTC | #4
On Mon, Sep 26, 2022 at 03:42:30PM +0200, Vincent Whitchurch wrote:
> On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote:

> > The driver should just set max_transfer_size and let the core
> > handle this.

> Hmm, AFAICS, the core doesn't actually do anything with
> max_transfer_size?  It's only used from spi-mem and a handful of other
> clients via spi_max_transfer_size().

Then we should fix the core to handle it properly.
diff mbox series

Patch

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 7f346866614a..85e1d1f90109 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -701,6 +701,16 @@  static int s3c64xx_spi_prepare_message(struct spi_master *master,
 	struct spi_device *spi = msg->spi;
 	struct s3c64xx_spi_csinfo *cs = spi->controller_data;
 
+	if (master->can_dma) {
+		int ret;
+
+		/* Limited by size of PACKET_CNT.COUNT_VALUE. */
+		ret = spi_split_transfers_maxsize(master, msg, 65535,
+						  GFP_KERNEL | GFP_DMA);
+		if (ret)
+			return ret;
+	}
+
 	/* Configure feedback delay */
 	if (!cs)
 		/* No delay if not defined */