linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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: Wed, 22 Jul 2015 10:24:05 +0200	[thread overview]
Message-ID: <201507221024.05496.marex@denx.de> (raw)
In-Reply-To: <CAOMqctQ13JE+GHyj+hU4XL3Uz4+Yt_X6BFU=NO8jXcpRtzMyzQ@mail.gmail.com>

On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote:
> On 22 July 2015 at 09:58, Marek Vasut <marex@denx.de> wrote:
> > On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote:
> >> On 22 July 2015 at 09:33, Marek Vasut <marex@denx.de> wrote:
> >> > On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote:
> >> >> On 22 July 2015 at 06:49, Vinod Koul <vinod.koul@intel.com> wrote:
> >> >> > On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote:
> >> >> >> > Or alternatively we could publish the limitations of the channel
> >> >> >> > using capabilities so SPI knows I have a dmaengine channel and
> >> >> >> > it can transfer max N length transfers so would be able to
> >> >> >> > break rather than guessing it or coding in DT. Yes it may come
> >> >> >> > from DT but that should be dmaengine driver rather than client
> >> >> >> > driver :)
> >> >> >> > 
> >> >> >> > This can be done by dma_get_slave_caps(chan, &caps)
> >> >> >> > 
> >> >> >> > And we add max_length as one more parameter to existing set
> >> >> >> > 
> >> >> >> > Also all this could be handled in generic SPI-dmaengine layer so
> >> >> >> > that individual drivers don't have to code it in
> >> >> >> > 
> >> >> >> > Let me know if this idea is okay, I can push the dmaengine
> >> >> >> > bits...
> >> >> >> 
> >> >> >> It would be ok if there was a fixed limit. However, the limit
> >> >> >> depends on SPI slave settings. Presumably for other buses using
> >> >> >> the dmaengine the limit would depend on the bus or slave settings
> >> >> >> as well. I do not see a sane way of passing this all the way to
> >> >> >> the dmaengine driver.
> >> >> > 
> >> >> > I don't see why this should be client (SPI) dependent. The max
> >> >> > length supported is a dmaengine constraint, typically flowing from
> >> >> > max blocks/length it can transfer. Know this limit can allow
> >> >> > clients to split transfers.
> >> >> 
> >> >> In practice on the board I have the maximum transfer length before it
> >> >> fails depends on SPI bus speed which is set up per slave. I did not
> >> >> try searching the space of possible settings thorougly and settled
> >> >> for a setting that gives reasonable speed and transfer length.
> >> > 
> >> > This looks more like a signal integrity issue though.
> >> 
> >> It certainly does on the surface. However, when wrong data is
> >> delivered over the SPI bus (such as when I use wrong phase setting)
> >> the SPI controller happily delivers wrong data over PIO.
> >> 
> >> The failure I am seeing is that the pl330 DMA program which repeatedly
> >> waits for data from the SPI controller never finishes the read loop
> >> and does not signal the interrupt. It seems it also leaves some data
> >> in a FIFO somewhere so next command on the flash returns garbage and
> >> fails.
> > 
> > I observed something similar on MXS (mx28) SPI block. Do you use mixed
> > PIO/DMA mode perhaps ?
> 
> The SPI driver uses PIO for short transfers and DMA for transfers
> longer than the controller FIFO. This seems to be the standard way to
> do things.It works flawlessly so long as submitting overly long DMA
> programs is avoided.

Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation
issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just
give pure DMA a go to see if the thing stabilizes somehow.

> > Do you have the option to connect a bus analyzer?
> > I can probably offer you some tools, I'm in Prague ...
> 
> The flash chip is accessible when removing the bottom cover. It is
> VSOP8 package slightly lower than SOP8 so attaching clips to it might
> be a bit problematic. That's the only accessible part. Everything
> other than SPI is inside the SoC.

That might be doable, though you might want to try the above thing first.

> Since SPI has no verification whatsoever the chip might be completely
> dead and you can still read fine all zeroes or all ones when
> attempting a read from it. I observed this behaviour when I used a
> flash chip in a socket and it was not firmly seated. It was with a
> different SPI controller, though.

You should run into issues as soon as the SPI NOR framework tries to read
status register, no ?

  reply	other threads:[~2015-07-22  8: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 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 03/11] mtd: add debug prints to mtdpart partition parser Michal Suchanek
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 05/11] mtd: mtdpart: Do not fail mtd probe when parsing partitions fails 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 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 [this message]
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
2015-07-30 12:18                                                     ` Michal Suchanek
2015-07-30 12:33                                                       ` Marek Vasut
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 07/11] mtd: spi-nor: rework write loop Michal Suchanek
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 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 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=201507221024.05496.marex@denx.de \
    --to=marex@denx.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).