Linux SPI subsystem development
 help / color / mirror / Atom feed
From: "Nuno Sá" <nuno.sa@analog.com>
To: Fei Xie <fei.xie@horizon.auto>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Mark Brown <broonie@kernel.org>,
	linux-spi@vger.kernel.org, linux-mtd@lists.infradead.org
Subject: Re: [RFC PATCH 0/4] spi: cadence-xspi: add ACMD PIO support for NAND and NOR
Date: Mon, 28 Sep 2026 16:02:27 +0100	[thread overview]
Message-ID: <arp-E1MDFLfV1ASq@nsa> (raw)
In-Reply-To: <20260928055005.3987624-1-fei.xie@horizon.auto>

On Mon, Sep 28, 2026 at 01:50:05PM +0800, Fei Xie wrote:

Hi Fei,

> Hi Nuno, Miquel,
> 
> Thanks for asking for benchmarks. I backported the 32/64-bit STIG SDMA
> transfer changes from the patch Nuno pointed to [1] to our vendor 6.1
> kernel, and compared that STIG path against the ACMD SPI NAND path from
> this RFC. I have not applied Nuno's separate ACMD read implementation;
> the figures below are for the implementation in this RFC.
> 
> The device is a GD5F4GM8RE SPI NAND (2 KiB pages, 128 KiB eraseblocks)
> on a Horizon J6B board, running at 80 MHz in quad SDR mode. Both paths
> used the same board PHY configuration. STIG needed one additional dummy
> clock for the EBh read-cache command on this board to return correct
> data at 80 MHz. This is a local test adjustment, not part of the posted
> RFC or [1]; I excluded the incorrect STIG reads from the comparison.
> 
> I ran flash_speed -c 5 -d /dev/mtd18 three times with each mode. The
> numbers below are the arithmetic means, in KiB/s:
> 
>                                STIG      ACMD PIO
>   eraseblock write speed      3685          3555
>   eraseblock read speed      12468         14222
>   page write speed            3671          3510
>   page read speed            11931         13716
>   erase speed                40000         37647

So this goes along with my expectations. I think we can all agree that
PROGRAM + ERASE is not worth the trouble? We even loose performance...

> 
> For a separate sequential-read test, I erased and programmed the first
> 64 MiB with known random data, then checked that each mode read back
> the complete area byte-for-byte. Both SHA-256 hashes matched the source;
> the reported ECC failure counts were zero. I then ran:

What do you mean by sequential read? Does your flash supports continuous
rads (or buffered mode I think). If so, I would expect much better
number for the ACMD case. But that can be the issue of having both the
controller and the chip trying to walk pages.

> 
>   /var/busybox/time -f 'real=%e user=%U sys=%S cpu=%P' \
>     dd if=/dev/mtd18 of=/dev/null bs=1M count=64
> 
>                           STIG              ACMD PIO
>   elapsed (three runs)    5.30/5.28/5.26 s  4.66/4.65/4.65 s
>   mean throughput         12.12 MiB/s       13.75 MiB/s
>   mean system time        3.55 s            1.06 s
>   process CPU usage       67%               22%
> 
> On this NAND, ACMD improves the 64 MiB read throughput by about 13.5%
> and reduces the reading process's CPU usage by 45 percentage points.
> The flash_speed results show a similar gain for reads, including
> single-page reads. I do not see a program or erase throughput benefit:
> ACMD is slightly slower for both in this setup. These are SPI NAND
> results only; I have not benchmarked the NOR path on this board.

So CPU results are the bing thing here. I guess we need to decide if
it's worth the amount of complexity it requires for the whole thing to
work. As I mentioned, if the chip has continuous mode and we try to
read anything bigger than page size we should let the nand chip to
walk the pages and that means the behavior we have today (IMHO of
course):

0x13 (STIG) + wait + read from cache (ACMD)

If we're only reading a single page then the logic in this series would
payoff (slight performance gain + significant drop in cpu load). One simple
way for doing this as a starting point would be:

* Nand has cont mode -> xspi controller does nothing special
* Nand has no cont mode -> Aggregate the xfers together.

Doing something dynamic would be much more complex I guess. With the
above reasoning, having some mem_info() kind of spi_mem callback to tell
us things like geometry, continuous mode support, etc would be enough to
handle the above I think. The reasoning is that reading a nand page is
always like spinand_load_page_op() (opcode 0x13) + spinand_wait() +
spinand_read_from_cache_op() so we could probably aggregate everything
in the last call on the controller side without complicating the nand core
logic.

But Miquel knows better if the above could be a viable thing to do.

- Nuno Sá

> 
> This is hardware validation of a 6.1 backport, not a claim that the RFC
> was tested on a current upstream kernel. I agree the read and CPU gains
> need to be weighed against the controller/MTD layering concerns raised
> in review.
> 
> Thanks,
> Fei
> 
> [1] https://lore.kernel.org/linux-spi/178056886874.53724.4850286391745939707.b4-ty@b4/
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2026-09-28 15:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21  9:36 [RFC PATCH 0/4] spi: cadence-xspi: add ACMD PIO support for NAND and NOR Fei Xie
2026-09-21  9:36 ` [RFC PATCH 1/4] dt-bindings: spi: cdns,xspi: add SPI NAND compatible Fei Xie
2026-09-21 14:01   ` Mark Brown
2026-09-23 12:54   ` Krzysztof Kozlowski
2026-09-21  9:36 ` [RFC PATCH 2/4] spi: cadence-xspi: add ACMD support for SPI NAND Fei Xie
2026-09-21 14:51   ` Mark Brown
2026-09-23  6:12     ` Fei Xie
2026-09-24 18:55       ` Mark Brown
2026-09-25  9:45         ` Miquel Raynal
2026-09-25 10:14           ` Nuno Sá
2026-09-25 10:23             ` Miquel Raynal
2026-09-25 11:31               ` Nuno Sá
2026-09-25 11:59               ` Mark Brown
2026-09-25 12:59                 ` Miquel Raynal
2026-09-25 16:38                   ` Mark Brown
2026-09-23 12:37     ` Nuno Sá
2026-09-21  9:37 ` [RFC PATCH 3/4] spi: cadence-xspi: factor out reusable ACMD helpers Fei Xie
2026-09-21  9:37 ` [RFC PATCH 4/4] spi: cadence-xspi: add ACMD support for SPI NOR Fei Xie
2026-09-23 12:24 ` [RFC PATCH 0/4] spi: cadence-xspi: add ACMD PIO support for NAND and NOR Nuno Sá
2026-09-28  5:50   ` Fei Xie
2026-09-28 15:02     ` Nuno Sá [this message]
2026-10-04 14:37       ` Miquel Raynal
2026-10-06  6:50         ` Nuno Sá
2026-10-07  4:48           ` Jisheng Zhang
2026-10-07  6:12             ` Nuno Sá
2026-10-08 13:55               ` Jisheng Zhang
2026-10-09  4:30                 ` Jisheng Zhang
2026-10-09  6:17                   ` Nuno Sá

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=arp-E1MDFLfV1ASq@nsa \
    --to=nuno.sa@analog.com \
    --cc=broonie@kernel.org \
    --cc=fei.xie@horizon.auto \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    /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