[PATCH v5 0/8] mtd: spi-nor: read while write support

* [PATCH v5 0/8] mtd: spi-nor: read while write support
@ 2023-03-28 15:40 Miquel Raynal
  2023-03-28 15:40 ` [PATCH v5 1/8] mtd: spi-nor: Introduce the concept of bank Miquel Raynal
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Miquel Raynal @ 2023-03-28 15:40 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Pratyush Yadav, Michael Walle, linux-mtd
  Cc: Julien Su, Jaime Liao, Jaime Liao, Alvin Zhou, Thomas Petazzoni,
	Michal Simek, linux-arm-kernel, Miquel Raynal

Hello folks,

Here is a new version of this series trying to bring a little bit of
parallelism to support SPI-NOR Read While Write feature on parts
supporting it and featuring several banks.

I have received some hardware to make it work, so since the RFC, the
series has been updated to fix my mistakes, but the overall idea is the
same.

There is nothing Macronix specific in the implementation, the operations
and opcodes are exactly the same as before. The only difference being:
we may consider the chip usable when it is in the busy state during a
write or an erase. Any chip with an internal split allowing to perform
parallel operations might possibly leverage the benefits of this
implementation.

The first patches are just refactoring and preparation work, there is
almost no functional change, it's just a way to prepare the introduction
of the new locking mechanism and hopefully provide the cleanest and
simplest diff possible for this new feature. The actual change is all
contained in "mtd: spi-nor: Enhance locking to support reads while
writes". The logic is described in the commit log and copy/pasted here
for clarity:

"
    On devices featuring several banks, the Read While Write (RWW) feature
    is here to improve the overall performance when performing parallel
    reads and writes at different locations (different banks). The
    following constraints have to be taken into account:
    1#: A single operation can be performed in a given bank.
    2#: Only a single program or erase operation can happen on the entire
        chip (common hardware limitation to limit costs)
    3#: Reads must remain serialized even though reads crossing bank
    	boundaries are allowed.
    4#: The I/O bus is unique and thus is the most constrained resource,
        all spi-nor operations requiring access to the spi bus (through
        the spi controller) must be serialized until the bus exchanges
        are over. So we must ensure a single operation can be "sent" at
        a time.
    5#: Any other operation that would not be either a read or a write or an
        erase is considered requiring access to the full chip and cannot be
        parallelized, we then need to ensure the full chip is in the idle
        state when this occurs.

    All these constraints can easily be managed with a proper locking model:
    1#: Is enforced by a bitfield of the in-use banks, so that only a single
        operation can happen in a specific bank at any time.
    2#: Is handled by the ongoing_pe boolean which is set before any write
        or erase, and is released only at the very end of the
        operation. This way, no other destructive operation on the chip can
        start during this time frame.
    3#: An ongoing_rd boolean allows to track the ongoing reads, so that
        only one can be performed at a time.
    4#: An ongoing_io boolean is introduced in order to capture and
        serialize bus accessed. This is the one being released "sooner"
        than before, because we only need to protect the chip against
        other SPI accesses during the I/O phase, which for the
        destructive operations is the beginning of the operation (when
        we send the command cycles and possibly the data), while the
        second part of the operation (the erase delay or the
        programmation delay) is when we can do something else in another
        bank.
    5#: Is handled by the three booleans presented above, if any of them is
        set, the chip is not yet ready for the operation and must wait.

    All these internal variables are protected by the existing lock, so that
    changes in this structure are atomic. The serialization is handled with
    a wait queue."

Here is now a benchmark with a Macronix MX25UW51245G with 4 banks and RWW
support:

     // Testing the two accesses in the same bank
     $ flash_speed -b0 -k0 -c1 -d /dev/mtd0
     [...]
     testing read while write latency
     read while write took 7ms, read ended after 7ms

     // Testing the two accesses within different banks
     $ flash_speed -b0 -k4096 -c1 -d /dev/mtd0
     [...]
     testing read while write latency
     testing read while write latency
     read while write took 7ms, read ended after 5ms

Parallel accesses have been validated with io_paral. A slight increase
of the time spent on this test has however been noticed. With my
configuration, over a limited number of blocks, the overall operation
took 22 min without any RWW changes up to 27 min with these changes,
maybe due to the number of additional scheduling situations involved.

ubiformat /dev/mtd1 -y && ubiattach -p /dev/mtd1 && io_paral /dev/ubi0

Here is a branch with the mtd-utils patch bringing support for this
additional "-k" parameter in flash_speed (for the second block to use
during RWW testing), used to get the above results:
https://github.com/miquelraynal/mtd-utils/compare/master...rww

Finally, here are two speed tests with and without the RWW bit set.

* Speed test with RWW enabled:
# flash_speed -d -c 100 /dev/mtd1
not NAND flash, assume page size is 512 bytes.
scanning for bad eraseblocks
scanned 100 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 251 KiB/s
testing eraseblock read speed
eraseblock read speed is 1869 KiB/s
testing page write speed
page write speed is 247 KiB/s
testing page read speed
page read speed is 902 KiB/s
testing 2 page write speed
2 page write speed is 250 KiB/s
testing 2 page read speed
2 page read speed is 1282 KiB/s
Testing erase speed
erase speed is 157 KiB/s
Testing 2x multi-block erase speed
2x multi-block erase speed is 160 KiB/s
Testing 4x multi-block erase speed
4x multi-block erase speed is 161 KiB/s
Testing 8x multi-block erase speed
8x multi-block erase speed is 161 KiB/s
Testing 16x multi-block erase speed
16x multi-block erase speed is 161 KiB/s
Testing 32x multi-block erase speed
32x multi-block erase speed is 161 KiB/s
Testing 64x multi-block erase speed
64x multi-block erase speed is 161 KiB/s
finished

* Speedtest without the RWW bit:
# flash_speed -d -c100 /dev/mtd1
not NAND flash, assume page size is 512 bytes.
scanning for bad eraseblocks
scanned 100 eraseblocks, 0 are bad
testing eraseblock write speed
eraseblock write speed is 249 KiB/s
testing eraseblock read speed
eraseblock read speed is 1869 KiB/s
testing page write speed
page write speed is 246 KiB/s
testing page read speed
page read speed is 907 KiB/s
testing 2 page write speed
2 page write speed is 248 KiB/s
testing 2 page read speed
2 page read speed is 1286 KiB/s
Testing erase speed
erase speed is 158 KiB/s
Testing 2x multi-block erase speed
2x multi-block erase speed is 160 KiB/s
Testing 4x multi-block erase speed
4x multi-block erase speed is 161 KiB/s
Testing 8x multi-block erase speed
8x multi-block erase speed is 161 KiB/s
Testing 16x multi-block erase speed
16x multi-block erase speed is 160 KiB/s
Testing 32x multi-block erase speed
32x multi-block erase speed is 161 KiB/s
Testing 64x multi-block erase speed
64x multi-block erase speed is 160 KiB/s
finished

The differences are well within the imprecision of the measure, so no
performance impact are to be expected on chips without RWW support.

Cheers,
Miquèl

Changes in v5:
* Made the bank calculations independent from the number of sectors and
  their size.
* Reordered the preparation vs. locking step in a few drivers which I
  missed in my first attempt to do the conversion.
* Converted the definition on the Macronix NOR chip to use SFDP tables.
* Move the RWW SPI-NOR flag to the regular flag list, as it is not
  specific to the "no_sfdp" situation.
* Created a SPI_NOR_F_* flag and set it in the core when relevant.
* Really improved the logic when enabling the RWW SPI NOR core flag so
  we don't have to make any further checks beside looking at a single
  bit.
* Improved commit messages and adapted them to the latest changes.
* Changed numerous function parameters and added inermediate local
  variables as requested by Tudor.
* Changed variable types.
* Added information about the spi-nor chip being introduced as well as
  additional benchmarks.

Changes in v4:
* Dropped patch 1/9 which got applied.
* s/SPI-NOR/SPI NOR/
* Turned n_banks into an u8 and moved it below in the struct to avoid
  padding.
* Updated the S3AN_INFO macro to set n_banks to 1 by default.
* Renamed the lock and prep helper to follow the order of each
  operation.
* Reworded a commit log to fit the recent changes upstream.

Changes in v3:
* Fix the bank offsets calculations by providing the same values when
  locking and when unlocking (might be changed by the functions themselves
  without use noticing).
* I completely changed the way the locking works because there was a new
  constraint: reads cannot be interrupted and status reads cannot happen
  during a read. Hence, as the multi-locks design was starting to be too
  messy, I changed the implementation to use a bunch of variables to
  track the read while write state, protected by the main spi-nor
  lock. If the internal state does not allow the operation, a sleep
  starts in a queue, until the threads are woken up after a state
  update. I know it is very verbose, I am open to suggestions.

Miquel Raynal (8):
  mtd: spi-nor: Introduce the concept of bank
  mtd: spi-nor: Add a macro to define more banks
  mtd: spi-nor: Reorder the preparation vs. locking steps
  mtd: spi-nor: Separate preparation and locking
  mtd: spi-nor: Prepare the introduction of a new locking mechanism
  mtd: spi-nor: Add a RWW flag
  mtd: spi-nor: Enhance locking to support reads while writes
  mtd: spi-nor: macronix: Add support for mx25uw51245g with RWW

 drivers/mtd/spi-nor/core.c     | 413 +++++++++++++++++++++++++++++++--
 drivers/mtd/spi-nor/core.h     |  25 +-
 drivers/mtd/spi-nor/debugfs.c  |   1 +
 drivers/mtd/spi-nor/macronix.c |   4 +
 drivers/mtd/spi-nor/otp.c      |   8 +-
 drivers/mtd/spi-nor/sst.c      |   2 +-
 drivers/mtd/spi-nor/swp.c      |   6 +-
 drivers/mtd/spi-nor/xilinx.c   |   1 +
 include/linux/mtd/spi-nor.h    |  13 ++
 9 files changed, 437 insertions(+), 36 deletions(-)

-- 
2.34.1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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