linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
@ 2014-05-08 14:30 Mika Westerberg
  2014-05-09 10:33 ` Mark Brown
  2014-05-12 21:06 ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Mika Westerberg @ 2014-05-08 14:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Brown, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh, Mika Westerberg

In case we are doing DMA transfer and the size of the buffer is not multiple
of 4 bytes the driver truncates that to 4-byte boundary and tries to handle
remaining bytes using PIO.

Or that is what it tried to do. What actually happens is that it calls
ALIGN() to the buffer size which aligns it to the next 4-byte boundary
(doesn't truncate). Doing this results 1-3 bytes extra to be transferred.
Furthermore we handle remaining bytes using PIO which results one extra
byte to be transferred. In worst case the driver transfers 4 extra bytes.

While investigating this it turned out that the DMA hardware doesn't even
have such limitation so we can solve this by dropping the code that tries
to handle unaligned bytes.

Reported-by: Chiau Ee Chew <chiau.ee.chew@intel.com>
Reported-by: Hock Leong Kweh <hock.leong.kweh@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-dma.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-dma.c b/drivers/spi/spi-pxa2xx-dma.c
index 713af4806f26..f6759dc0153b 100644
--- a/drivers/spi/spi-pxa2xx-dma.c
+++ b/drivers/spi/spi-pxa2xx-dma.c
@@ -29,18 +29,6 @@ static int pxa2xx_spi_map_dma_buffer(struct driver_data *drv_data,
 	struct sg_table *sgt;
 	void *buf, *pbuf;
 
-	/*
-	 * Some DMA controllers have problems transferring buffers that are
-	 * not multiple of 4 bytes. So we truncate the transfer so that it
-	 * is suitable for such controllers, and handle the trailing bytes
-	 * manually after the DMA completes.
-	 *
-	 * REVISIT: It would be better if this information could be
-	 * retrieved directly from the DMA device in a similar way than
-	 * ->copy_align etc. is done.
-	 */
-	len = ALIGN(drv_data->len, 4);
-
 	if (dir == DMA_TO_DEVICE) {
 		dmadev = drv_data->tx_chan->device->dev;
 		sgt = &drv_data->tx_sgt;
@@ -144,12 +132,8 @@ static void pxa2xx_spi_dma_transfer_complete(struct driver_data *drv_data,
 		if (!error) {
 			pxa2xx_spi_unmap_dma_buffers(drv_data);
 
-			/* Handle the last bytes of unaligned transfer */
 			drv_data->tx += drv_data->tx_map_len;
-			drv_data->write(drv_data);
-
 			drv_data->rx += drv_data->rx_map_len;
-			drv_data->read(drv_data);
 
 			msg->actual_length += drv_data->len;
 			msg->state = pxa2xx_spi_next_transfer(drv_data);
-- 
2.0.0.rc2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
  2014-05-08 14:30 [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes Mika Westerberg
@ 2014-05-09 10:33 ` Mark Brown
  2014-05-09 11:21   ` Mika Westerberg
  2014-05-12 21:06 ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-05-09 10:33 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-kernel, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh

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

On Thu, May 08, 2014 at 05:30:31PM +0300, Mika Westerberg wrote:
> In case we are doing DMA transfer and the size of the buffer is not multiple
> of 4 bytes the driver truncates that to 4-byte boundary and tries to handle
> remaining bytes using PIO.

...

> While investigating this it turned out that the DMA hardware doesn't even
> have such limitation so we can solve this by dropping the code that tries
> to handle unaligned bytes.

Is this definitely the case for all of the IPs using this driver?  It
seems like something which might have been present in actual PXA
implemenetations but got fixed in later revisons used with x86.  Equally
well the current code is clearly broken either way so I'm not sure that
problems with older systems should be a barrier to merging the patch but
it seems better to check.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
  2014-05-09 10:33 ` Mark Brown
@ 2014-05-09 11:21   ` Mika Westerberg
  2014-05-09 11:27     ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2014-05-09 11:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh

On Fri, May 09, 2014 at 11:33:15AM +0100, Mark Brown wrote:
> On Thu, May 08, 2014 at 05:30:31PM +0300, Mika Westerberg wrote:
> > In case we are doing DMA transfer and the size of the buffer is not multiple
> > of 4 bytes the driver truncates that to 4-byte boundary and tries to handle
> > remaining bytes using PIO.
> 
> ...
> 
> > While investigating this it turned out that the DMA hardware doesn't even
> > have such limitation so we can solve this by dropping the code that tries
> > to handle unaligned bytes.
> 
> Is this definitely the case for all of the IPs using this driver?  It
> seems like something which might have been present in actual PXA
> implemenetations but got fixed in later revisons used with x86.  Equally
> well the current code is clearly broken either way so I'm not sure that
> problems with older systems should be a barrier to merging the patch but
> it seems better to check.

This code came with x86 LPSS implementation originally. The PXA one,
which lives in a different file (spi-pxa2xx-pxadma.c) didn't have any
such checks AFAIK.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
  2014-05-09 11:21   ` Mika Westerberg
@ 2014-05-09 11:27     ` Mark Brown
  2014-05-12  7:08       ` Mika Westerberg
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-05-09 11:27 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-kernel, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh

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

On Fri, May 09, 2014 at 02:21:05PM +0300, Mika Westerberg wrote:
> On Fri, May 09, 2014 at 11:33:15AM +0100, Mark Brown wrote:

> > Is this definitely the case for all of the IPs using this driver?  It
> > seems like something which might have been present in actual PXA
> > implemenetations but got fixed in later revisons used with x86.  Equally
> > well the current code is clearly broken either way so I'm not sure that
> > problems with older systems should be a barrier to merging the patch but
> > it seems better to check.

> This code came with x86 LPSS implementation originally. The PXA one,
> which lives in a different file (spi-pxa2xx-pxadma.c) didn't have any
> such checks AFAIK.

OK, that should be fine then.  The PXA platforms should be being
converted over to use this file as part of the dmaengine transition on
that platform so we can't assume it's Intel specific even though it was
originally written for x86.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
  2014-05-09 11:27     ` Mark Brown
@ 2014-05-12  7:08       ` Mika Westerberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2014-05-12  7:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh

On Fri, May 09, 2014 at 12:27:12PM +0100, Mark Brown wrote:
> On Fri, May 09, 2014 at 02:21:05PM +0300, Mika Westerberg wrote:
> > On Fri, May 09, 2014 at 11:33:15AM +0100, Mark Brown wrote:
> 
> > > Is this definitely the case for all of the IPs using this driver?  It
> > > seems like something which might have been present in actual PXA
> > > implemenetations but got fixed in later revisons used with x86.  Equally
> > > well the current code is clearly broken either way so I'm not sure that
> > > problems with older systems should be a barrier to merging the patch but
> > > it seems better to check.
> 
> > This code came with x86 LPSS implementation originally. The PXA one,
> > which lives in a different file (spi-pxa2xx-pxadma.c) didn't have any
> > such checks AFAIK.
> 
> OK, that should be fine then.  The PXA platforms should be being
> converted over to use this file as part of the dmaengine transition on
> that platform so we can't assume it's Intel specific even though it was
> originally written for x86.

Right and it isn't. However, this code (the one I'm removing with this
patch) was developed on early Haswell machines where we encountered this
restriction. With the hardware that is shipping the restriction is gone
so I don't see much point keeping it there.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes
  2014-05-08 14:30 [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes Mika Westerberg
  2014-05-09 10:33 ` Mark Brown
@ 2014-05-12 21:06 ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-12 21:06 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-kernel, Eric Miao, Russell King, Haojian Zhuang,
	Chiau Ee Chew, Hock Leong Kweh

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

On Thu, May 08, 2014 at 05:30:31PM +0300, Mika Westerberg wrote:
> In case we are doing DMA transfer and the size of the buffer is not multiple
> of 4 bytes the driver truncates that to 4-byte boundary and tries to handle
> remaining bytes using PIO.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-05-12 21:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-08 14:30 [PATCH] spi/pxa2xx: Prevent DMA from transferring too many bytes Mika Westerberg
2014-05-09 10:33 ` Mark Brown
2014-05-09 11:21   ` Mika Westerberg
2014-05-09 11:27     ` Mark Brown
2014-05-12  7:08       ` Mika Westerberg
2014-05-12 21:06 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).