Hi all,
I finally found the bug I saw months ago, before "[PATCH v8 3/8]
spi/spi-atmel: add dmaengine support" was merged.
Here it is:
When the ioctl SPI_IOC_MESSAGE is used with small and big buffers,
the big RX buffer is corrupted with bytes from the big TX buffer.
(Small means size < DMA_MIN_BYTES, Big means size > DMA_MIN_BYTES)
I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
It fills 3 TX buffers with 0xAA pattern: a small, a big and a small again.
It reads in return 3 RX buffers.
The MISO pin has to be on 3.3v.
It checks if the received buffers are filled with 0xFF (they should
be, as MISO is high).
And I've got a lot of buffers filled partially with 0xAA bytes.
BUT, if you apply this patch:
---8<----
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 83cf609..cde42a4 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -187,7 +187,7 @@
/* use PIO for small transfers, avoiding DMA setup/teardown overhead and
* cache operations; better heuristics consider wordsize and bitrate.
*/
-#define DMA_MIN_BYTES 16
+#define DMA_MIN_BYTES 0
struct atmel_spi_dma {
struct dma_chan *chan_rx;
---8<----
There's no error any more.
So there's something wrong happening when switching from/to pio
transfer to/from DMA.
We didn't saw that at the time of the merge because we did the test
with a loop on MISO/MOSI,
so the RX buffer was corrupted with identical bytes.
Best regards,
Richard.
On Tue, Oct 08, 2013 at 12:44:16PM +0200, Richard Genoud wrote:
> I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
Looks like the attachment got forgotten?
2013/10/8 Mark Brown <[email protected]>:
> On Tue, Oct 08, 2013 at 12:44:16PM +0200, Richard Genoud wrote:
>
>> I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
>
> Looks like the attachment got forgotten?
arg !!!
thanks !
here it is.
2013/10/8 Mark Brown <[email protected]>:
> On Tue, Oct 08, 2013 at 12:44:16PM +0200, Richard Genoud wrote:
>
>> I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
>
> Looks like the attachment got forgotten?
arg !!!
thanks !
here it is. (I hope !! Since this morning, I have a *very*
discontinued internet acces with small shooting windows)
On Tue, Oct 08, 2013 at 02:52:58PM +0200, Richard Genoud wrote:
> thanks !
> here it is. (I hope !! Since this morning, I have a *very*
> discontinued internet acces with small shooting windows)
Got it.
2013/10/8 Richard Genoud <[email protected]>:
> Hi all,
>
> I finally found the bug I saw months ago, before "[PATCH v8 3/8]
> spi/spi-atmel: add dmaengine support" was merged.
>
> Here it is:
>
> When the ioctl SPI_IOC_MESSAGE is used with small and big buffers,
> the big RX buffer is corrupted with bytes from the big TX buffer.
> (Small means size < DMA_MIN_BYTES, Big means size > DMA_MIN_BYTES)
>
> I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
>
> It fills 3 TX buffers with 0xAA pattern: a small, a big and a small again.
> It reads in return 3 RX buffers.
> The MISO pin has to be on 3.3v.
>
> It checks if the received buffers are filled with 0xFF (they should
> be, as MISO is high).
> And I've got a lot of buffers filled partially with 0xAA bytes.
>
> BUT, if you apply this patch:
> ---8<----
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 83cf609..cde42a4 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -187,7 +187,7 @@
> /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
> * cache operations; better heuristics consider wordsize and bitrate.
> */
> -#define DMA_MIN_BYTES 16
> +#define DMA_MIN_BYTES 0
>
> struct atmel_spi_dma {
> struct dma_chan *chan_rx;
> ---8<----
> There's no error any more.
> So there's something wrong happening when switching from/to pio
> transfer to/from DMA.
>
> We didn't saw that at the time of the merge because we did the test
> with a loop on MISO/MOSI,
> so the RX buffer was corrupted with identical bytes.
>
> Best regards,
> Richard.
Hi Wenyou,
Did you have the opportunity to give it a test ?
Thread: https://lkml.org/lkml/2013/10/8/329
Richard.
Hi Richard,
> -----Original Message-----
> From: Richard Genoud [mailto:[email protected]]
> Sent: 2013年10月17日 19:01
> To: Yang, Wenyou
> Cc: [email protected]; [email protected]; Ferre, Nicolas;
> Mark Brown
> Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with
> SPI_IOC_MESSAGE
>
> 2013/10/8 Richard Genoud <[email protected]>:
> > Hi all,
> >
> > I finally found the bug I saw months ago, before "[PATCH v8 3/8]
> > spi/spi-atmel: add dmaengine support" was merged.
> >
> > Here it is:
> >
> > When the ioctl SPI_IOC_MESSAGE is used with small and big buffers,
> > the big RX buffer is corrupted with bytes from the big TX buffer.
> > (Small means size < DMA_MIN_BYTES, Big means size >
> DMA_MIN_BYTES)
> >
> > I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
> >
> > It fills 3 TX buffers with 0xAA pattern: a small, a big and a small again.
> > It reads in return 3 RX buffers.
> > The MISO pin has to be on 3.3v.
> >
> > It checks if the received buffers are filled with 0xFF (they should
> > be, as MISO is high).
> > And I've got a lot of buffers filled partially with 0xAA bytes.
> >
> > BUT, if you apply this patch:
> > ---8<----
> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> > index 83cf609..cde42a4 100644
> > --- a/drivers/spi/spi-atmel.c
> > +++ b/drivers/spi/spi-atmel.c
> > @@ -187,7 +187,7 @@
> > /* use PIO for small transfers, avoiding DMA setup/teardown overhead
> and
> > * cache operations; better heuristics consider wordsize and bitrate.
> > */
> > -#define DMA_MIN_BYTES 16
> > +#define DMA_MIN_BYTES 0
> >
> > struct atmel_spi_dma {
> > struct dma_chan *chan_rx;
> > ---8<----
> > There's no error any more.
> > So there's something wrong happening when switching from/to pio
> > transfer to/from DMA.
> >
> > We didn't saw that at the time of the merge because we did the test
> > with a loop on MISO/MOSI,
> > so the RX buffer was corrupted with identical bytes.
> >
> > Best regards,
> > Richard.
>
> Hi Wenyou,
>
> Did you have the opportunity to give it a test ?
>
> Thread: https://lkml.org/lkml/2013/10/8/329
I tried to reproduce it with this thread, but I didn't
Which slave device did you use?
>
> Richard.
Best Regards,
Wenyou Yang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
2013/10/21 Yang, Wenyou <[email protected]>:
> Hi Richard,
>
>> -----Original Message-----
>> From: Richard Genoud [mailto:[email protected]]
>> Sent: 2013年10月17日 19:01
>> To: Yang, Wenyou
>> Cc: [email protected]; [email protected]; Ferre, Nicolas;
>> Mark Brown
>> Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with
>> SPI_IOC_MESSAGE
>>
>> 2013/10/8 Richard Genoud <[email protected]>:
>> > Hi all,
>> >
>> > I finally found the bug I saw months ago, before "[PATCH v8 3/8]
>> > spi/spi-atmel: add dmaengine support" was merged.
>> >
>> > Here it is:
>> >
>> > When the ioctl SPI_IOC_MESSAGE is used with small and big buffers,
>> > the big RX buffer is corrupted with bytes from the big TX buffer.
>> > (Small means size < DMA_MIN_BYTES, Big means size >
>> DMA_MIN_BYTES)
>> >
>> > I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
>> >
>> > It fills 3 TX buffers with 0xAA pattern: a small, a big and a small again.
>> > It reads in return 3 RX buffers.
>> > The MISO pin has to be on 3.3v.
>> >
>> > It checks if the received buffers are filled with 0xFF (they should
>> > be, as MISO is high).
>> > And I've got a lot of buffers filled partially with 0xAA bytes.
>> >
>> > BUT, if you apply this patch:
>> > ---8<----
>> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
>> > index 83cf609..cde42a4 100644
>> > --- a/drivers/spi/spi-atmel.c
>> > +++ b/drivers/spi/spi-atmel.c
>> > @@ -187,7 +187,7 @@
>> > /* use PIO for small transfers, avoiding DMA setup/teardown overhead
>> and
>> > * cache operations; better heuristics consider wordsize and bitrate.
>> > */
>> > -#define DMA_MIN_BYTES 16
>> > +#define DMA_MIN_BYTES 0
>> >
>> > struct atmel_spi_dma {
>> > struct dma_chan *chan_rx;
>> > ---8<----
>> > There's no error any more.
>> > So there's something wrong happening when switching from/to pio
>> > transfer to/from DMA.
>> >
>> > We didn't saw that at the time of the merge because we did the test
>> > with a loop on MISO/MOSI,
>> > so the RX buffer was corrupted with identical bytes.
>> >
>> > Best regards,
>> > Richard.
>>
>> Hi Wenyou,
>>
>> Did you have the opportunity to give it a test ?
>>
>> Thread: https://lkml.org/lkml/2013/10/8/329
> I tried to reproduce it with this thread, but I didn't
> Which slave device did you use?
Hi Wenyou,
I'm not using any slave device, I'm just setting the MISO pin to 3.3V,
then I execute the C code ( https://lkml.org/lkml/2013/10/8/329 )
So, we should only receive 0xFF in the buffer.
But i'm getting :
aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff
aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff
Richard.
> -----Original Message-----
> From: Richard Genoud [mailto:[email protected]]
> Sent: 2013年10月21日 16:26
> To: Yang, Wenyou
> Cc: [email protected]; [email protected]; Ferre, Nicolas;
> Mark Brown
> Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with
> SPI_IOC_MESSAGE
>
> 2013/10/21 Yang, Wenyou <[email protected]>:
> > Hi Richard,
> >
> >> -----Original Message-----
> >> From: Richard Genoud [mailto:[email protected]]
> >> Sent: 2013年10月17日 19:01
> >> To: Yang, Wenyou
> >> Cc: [email protected]; [email protected]; Ferre,
> Nicolas;
> >> Mark Brown
> >> Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with
> >> SPI_IOC_MESSAGE
> >>
> >> 2013/10/8 Richard Genoud <[email protected]>:
> >> > Hi all,
> >> >
> >> > I finally found the bug I saw months ago, before "[PATCH v8 3/8]
> >> > spi/spi-atmel: add dmaengine support" was merged.
> >> >
> >> > Here it is:
> >> >
> >> > When the ioctl SPI_IOC_MESSAGE is used with small and big buffers,
> >> > the big RX buffer is corrupted with bytes from the big TX buffer.
> >> > (Small means size < DMA_MIN_BYTES, Big means size >
> >> DMA_MIN_BYTES)
> >> >
> >> > I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x )
> >> >
> >> > It fills 3 TX buffers with 0xAA pattern: a small, a big and a small again.
> >> > It reads in return 3 RX buffers.
> >> > The MISO pin has to be on 3.3v.
> >> >
> >> > It checks if the received buffers are filled with 0xFF (they should
> >> > be, as MISO is high).
> >> > And I've got a lot of buffers filled partially with 0xAA bytes.
> >> >
> >> > BUT, if you apply this patch:
> >> > ---8<----
> >> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> >> > index 83cf609..cde42a4 100644
> >> > --- a/drivers/spi/spi-atmel.c
> >> > +++ b/drivers/spi/spi-atmel.c
> >> > @@ -187,7 +187,7 @@
> >> > /* use PIO for small transfers, avoiding DMA setup/teardown
> overhead
> >> and
> >> > * cache operations; better heuristics consider wordsize and
> bitrate.
> >> > */
> >> > -#define DMA_MIN_BYTES 16
> >> > +#define DMA_MIN_BYTES 0
> >> >
> >> > struct atmel_spi_dma {
> >> > struct dma_chan *chan_rx;
> >> > ---8<----
> >> > There's no error any more.
> >> > So there's something wrong happening when switching from/to pio
> >> > transfer to/from DMA.
> >> >
> >> > We didn't saw that at the time of the merge because we did the test
> >> > with a loop on MISO/MOSI,
> >> > so the RX buffer was corrupted with identical bytes.
> >> >
> >> > Best regards,
> >> > Richard.
> >>
> >> Hi Wenyou,
> >>
> >> Did you have the opportunity to give it a test ?
> >>
> >> Thread: https://lkml.org/lkml/2013/10/8/329
> > I tried to reproduce it with this thread, but I didn't
> > Which slave device did you use?
>
> Hi Wenyou,
>
> I'm not using any slave device, I'm just setting the MISO pin to 3.3V,
> then I execute the C code ( https://lkml.org/lkml/2013/10/8/329 )
> So, we should only receive 0xFF in the buffer.
> But i'm getting :
> aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff
> aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ff ff ff ff ff ff ff ff
>
Hi, Richard,
The bug is reproduced.
Tested on at91sam9g25ek, Linux-3.6.9.
> Richard.
Best Regards,
Wenyou Yang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?