From: marex@denx•de (Marek Vasut)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
Date: Thu, 30 Jul 2015 13:24:05 +0200 [thread overview]
Message-ID: <201507301324.05721.marex@denx.de> (raw)
In-Reply-To: <CAOMqctRPOv3_YZqS-g+SuuMT5UpOJMFs1OtBuZAhLR_SZnfTyw@mail.gmail.com>
On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote:
> On 27 July 2015 at 19:43, Marek Vasut <marex@denx•de> wrote:
> > On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote:
> >> On 24 July 2015 at 10:34, Marek Vasut <marex@denx•de> wrote:
> >> > On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote:
> >> Ok, so here is some summary.
> >>
> >> I have a NOR flash attached to a s3c64xx SPI controller with 64byte
> >> fifo and a pl330 dma controller.
> >>
> >> Normally DMA controller is used for transfers that do not fit into the
> >> FIFO.
> >>
> >> I tried adding the flash memory ID to the spi-nor driver table and
> >> adding a DT node for it.
> >>
> >> The flash is rated at 120MHz so I used that speed but the ID was
> >> bit-shifted and identification failed. There is DT property
> >> samsung,spi-feedback-delay for addressing this and at 120MHz it must
> >> be 2 or 3 on this board. 40MHz works with default value 0.
> >>
> >> The next step after identification worked was to try reading the flash
> >> content. For this the DMA controller is used because data is
> >> transferred in blocks larger than 64 bytes. When reading the whole 4MB
> >> flash the transfer failed silently. I got a 4MB file of all ones or
> >> all zeroes.
> >>
> >> It turns out that
> >>
> >> - the pl330 locks up when transfering large amount of data.
> >>
> >> Specifically, the maximum power of 2 sized transfer at 120MHz is 128
> >> bytes and 64k at 40MHz. Transferring more than this results in the
> >> pl330 locking up and never signalling completion of the transfer.
> >
> > Hypothesis:
> > I think this might just be that the controller didn't catch all the
> > inbound clock ticks and thus counted less inbound data than it was
> > set up to receive. The controller thus waits for more data.
>
> The flash chip can transfer data as long as you keep the clock going,
> especially when you transfer 64k off a 4M flash.
>
> The read command is bound just by stopping the clock. There is always
> more data to be had.
Sure, if you keep clocking the chip, the data will keep going. Is the
chip being clocked ?
Doesn't the PL330 have some kind of a counter register where you can check
how much data were received so far ? That should be sufficient to verify
this hypothesis of mine.
> >> Data
> >> is left in FIFO which causes subsequent commands to fail since garbage
> >> is returned instead of command reply. Also subsequent read may cause
> >> I/O error or or return an empty page depending on the garbage around.
> >
> > So the driver for the DMA controller might need to be augmented to handle
> > this case -- add a timeout and in case DMA times out, drain the FIFO to
> > make sure subsequent transfers do not fail.
>
> There is no way to add timeout in the DMA driver since it does not
> know the SPI clock.
>
> The timeout is handled by the SPI driver and it should be possible to
> augment it to drain the FIFO when DMA fails so long as FIFO level is
> readable somewhere.
If the DMA doesn't complete in certain amount of time, it should time out
I'd say. Don't you think ?
> >> - the I/O errors are not checked in spi-nor at all which leads to
> >> silent data corruption.
> >>
> >> On a suggestion that this may improve reliability I changed the
> >> s3c64xx driver to use DMA for all transfers. This caused
> >> identification to fail in spin-nor because the ID was prefixed with
> >> extra 00 byte. Testing with spidev confirmed that everything is
> >> prefixed with extra 00.
> >
> > The determinism of this is in fact excellent news.
> >
> >> Also when the DMA controller locked up no
> >> transfers were possible anymore. When DMA was not used for sending
> >> commands the controller would recover on next command. I made the
> >> option to always use DMA configurable and it turns out that the
> >> returned data is prefixed with 00 only when no transfer without DMA
> >> was ever made. Loading the spi-nor driver with the dma-only option off
> >> and then with dma-only option on results in correct operation. Only
> >> loading the dma-only driver first causes the 00 prefix.
> >
> > Can we conclude that the PIO codepath somehow programs a register
> > somewhere which gets rid of this 0x00 prefix ? If so, this should then
> > also be part of the DMA codepath, or even better, this should be set in
> > probe() somewhere.
>
> Yes, it looks like it.
Did you find what this could be please ?
next prev parent reply other threads:[~2015-07-30 11:24 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-03 21:26 [PATCH 00/11] Enable access to SPI NOR flash on Samsung Snow board Michal Suchanek
2015-06-03 21:26 ` [PATCH 01/11] ARM: dt: Add SPI CS " Michal Suchanek
2015-06-04 2:05 ` Krzysztof Kozlowski
2015-06-04 6:52 ` Javier Martinez Canillas
2015-06-03 21:26 ` [PATCH 05/11] mtd: mtdpart: Do not fail mtd probe when parsing partitions fails Michal Suchanek
2015-06-03 21:26 ` [PATCH 03/11] mtd: add debug prints to mtdpart partition parser Michal Suchanek
2015-06-03 21:26 ` [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist Michal Suchanek
2015-06-03 22:58 ` Marek Vasut
2015-06-04 4:54 ` Michal Suchanek
2015-06-04 15:28 ` Marek Vasut
2015-06-04 15:40 ` Michal Suchanek
2015-06-05 14:13 ` Marek Vasut
2015-06-23 18:26 ` Brian Norris
2015-06-03 21:26 ` [PATCH 02/11] mtd: spi-nor: Add GD25LQ32C 1.8V SPI NOR flash ID Michal Suchanek
2015-06-03 21:26 ` [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size Michal Suchanek
2015-06-03 23:03 ` Marek Vasut
2015-06-04 4:31 ` Michal Suchanek
2015-06-04 15:15 ` Marek Vasut
2015-06-04 6:42 ` Geert Uytterhoeven
2015-06-04 8:31 ` Michal Suchanek
2015-06-04 17:15 ` Richard Cochran
2015-07-15 9:45 ` Michal Suchanek
2015-07-15 11:52 ` Marek Vasut
2015-07-15 15:59 ` Brian Norris
2015-07-15 17:15 ` Marek Vasut
2015-07-16 1:19 ` Brian Norris
2015-07-16 1:44 ` Marek Vasut
2015-07-19 19:01 ` Michal Suchanek
2015-07-21 4:29 ` Vinod Koul
2015-07-21 8:14 ` Michal Suchanek
2015-07-22 4:49 ` Vinod Koul
2015-07-22 7:30 ` Michal Suchanek
2015-07-22 7:33 ` Marek Vasut
2015-07-22 7:45 ` Michal Suchanek
2015-07-22 7:58 ` Marek Vasut
2015-07-22 8:18 ` Michal Suchanek
2015-07-22 8:24 ` Marek Vasut
2015-07-22 8:38 ` Michal Suchanek
2015-07-22 9:01 ` Marek Vasut
2015-07-23 16:46 ` Michal Suchanek
2015-07-23 17:03 ` Michal Suchanek
2015-07-24 8:34 ` Marek Vasut
2015-07-24 11:20 ` Michal Suchanek
2015-07-27 9:46 ` Michal Suchanek
2015-07-27 17:43 ` Marek Vasut
2015-07-27 20:43 ` Michal Suchanek
2015-07-30 11:24 ` Marek Vasut [this message]
2015-07-30 12:18 ` Michal Suchanek
2015-07-30 12:33 ` Marek Vasut
2015-06-03 21:26 ` [PATCH 07/11] mtd: spi-nor: rework write loop Michal Suchanek
2015-06-03 21:26 ` [PATCH 09/11] dma: pl330: fix wording in mcbufsz message Michal Suchanek
2015-06-04 2:10 ` Krzysztof Kozlowski
2015-06-08 11:07 ` Vinod Koul
2015-06-03 21:26 ` [PATCH 06/11] mtd: spi-nor: rework spi nor read and write Michal Suchanek
2015-06-03 21:26 ` [PATCH 10/11] spi: add more debug prints in s3c64xx Michal Suchanek
2015-06-03 23:04 ` Marek Vasut
2015-06-04 9:16 ` Mark Brown
2015-06-04 9:30 ` Geert Uytterhoeven
2015-06-04 9:42 ` Mark Brown
2015-06-04 9:33 ` Michal Suchanek
2015-06-04 10:26 ` Mark Brown
2015-06-04 10:52 ` Michal Suchanek
2015-06-04 10:56 ` Mark Brown
2015-06-03 21:26 ` [PATCH 11/11] dt: Exynos: add Snow SPI NOR node Michal Suchanek
2015-06-04 2:04 ` Krzysztof Kozlowski
2015-06-04 15:20 ` Marek Vasut
2015-06-17 12:19 ` Pavel Machek
2015-06-03 22:53 ` [PATCH 00/11] Enable access to SPI NOR flash on Samsung Snow board Marek Vasut
2015-06-04 4:21 ` Michal Suchanek
2015-06-04 15:29 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201507301324.05721.marex@denx.de \
--to=marex@denx$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox