qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Bin Meng <bmeng.cn@gmail.com>
To: "Alistair Francis" <alistair.francis@wdc.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Francisco Iglesias" <frasse.iglesias@gmail.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	"Marcin Krzeminski" <marcin.krzeminski@nokia.com>,
	"Andrew Jeffery" <andrew@aj.id.au>,
	"Bin Meng" <bin.meng@windriver.com>,
	"Havard Skinnemoen" <hskinnemoen@google.com>,
	"Max Reitz" <mreitz@redhat.com>,
	"Tyrone Ting" <kfting@nuvoton.com>,
	qemu-arm@nongnu.org, "Cédric Le Goater" <clg@kaod.org>,
	"Joe Komlodi" <komlodi@xilinx.com>,
	"Joel Stanley" <joel@jms.id.au>
Subject: [PATCH 0/9] hw/block: m25p80: Fix the mess of dummy bytes needed for fast read commands
Date: Thu, 14 Jan 2021 23:08:53 +0800	[thread overview]
Message-ID: <20210114150902.11515-1-bmeng.cn@gmail.com> (raw)

From: Bin Meng <bin.meng@windriver.com>

The m25p80 model uses s->needed_bytes to indicate how many follow-up
bytes are expected to be received after it receives a command. For
example, depending on the address mode, either 3-byte address or
4-byte address is needed.

For fast read family commands, some dummy cycles are required after
sending the address bytes, and the dummy cycles need to be counted
in s->needed_bytes. This is where the mess began.

As the variable name (needed_bytes) indicates, the unit is in byte.
It is not in bit, or cycle. However for some reason the model has
been using the number of dummy cycles for s->needed_bytes. The right
approach is to convert the number of dummy cycles to bytes based on
the SPI protocol, for example, 6 dummy cycles for the Fast Read Quad
I/O (EBh) should be converted to 3 bytes per the formula (6 * 4 / 8).

Things get complicated when interacting with different SPI or QSPI
flash controllers. There are major two cases:

- Dummy bytes prepared by drivers, and wrote to the controller fifo.
  For such case, driver will calculate the correct number of dummy
  bytes and write them into the tx fifo. Fixing the m25p80 model will
  fix flashes working with such controllers.
- Dummy bytes not prepared by drivers. Drivers just tell the hardware
  the dummy cycle configuration via some registers, and hardware will
  automatically generate dummy cycles for us. Fixing the m25p80 model
  is not enough, and we will need to fix the SPI/QSPI models for such
  controllers.

This series fixes the mess in the m25p80 from the flash side first,
followed by fixes to 3 known SPI controller models that fall into
the 2nd case above.

Please note, I have no way to verify patch 7/8/9 because:

* There is no public datasheet available for the SoC / SPI controller
* There is no QEMU docs, or details that tell people how to boot either
  U-Boot or Linux kernel to verify the functionality

These 3 patches are very likely to be wrong. Hence I would like to ask
help from the original author who wrote these SPI controller models
to help testing, or completely rewrite these 3 patches to fix things.
Thanks!

Patch 6 is unvalidated with QEMU, mainly because there is no doc to
tell people how to boot anything to test. But I have some confidence
based on my read of the ZynqMP manual, as well as some experimental
testing on a real ZCU102 board.

Other flash patches can be tested with the SiFive SPI series:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=222391

Cherry-pick patch 16 and 17 from the series above, and switch to
different flash model to test with the following command:

$ qemu-system-riscv64 -nographic -M sifive_u -m 2G -smp 5 -kernel u-boot

I've picked up two for testing:

QEMU flash: "sst25vf032b"

  U-Boot 2020.10 (Jan 14 2021 - 21:55:59 +0800)

  CPU:   rv64imafdcsu
  Model: SiFive HiFive Unleashed A00
  DRAM:  2 GiB
  MMC:
  Loading Environment from SPIFlash... SF: Detected sst25vf032b with page size 256 Bytes, erase size 4 KiB, total 4 MiB
  *** Warning - bad CRC, using default environment

  In:    serial@10010000
  Out:   serial@10010000
  Err:   serial@10010000
  Net:   failed to get gemgxl_reset reset

  Warning: ethernet@10090000 MAC addresses don't match:
  Address in DT is                52:54:00:12:34:56
  Address in environment is       70:b3:d5:92:f0:01
  eth0: ethernet@10090000
  Hit any key to stop autoboot:  0
  => sf probe
  SF: Detected sst25vf032b with page size 256 Bytes, erase size 4 KiB,
  total 4 MiB
  => sf test 1ff000 1000
  SPI flash test:
  0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
  1 check: 10 ticks, 400 KiB/s 3.200 Mbps
  2 write: 170 ticks, 23 KiB/s 0.184 Mbps
  3 read: 9 ticks, 444 KiB/s 3.552 Mbps
  Test passed
  0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
  1 check: 10 ticks, 400 KiB/s 3.200 Mbps
  2 write: 170 ticks, 23 KiB/s 0.184 Mbps
  3 read: 9 ticks, 444 KiB/s 3.552 Mbps

QEMU flash: "mx66u51235f"

  U-Boot 2020.10 (Jan 14 2021 - 21:55:59 +0800)

  CPU:   rv64imafdcsu
  Model: SiFive HiFive Unleashed A00
  DRAM:  2 GiB
  MMC:
  Loading Environment from SPIFlash... SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB, total 64 MiB
  *** Warning - bad CRC, using default environment

  In:    serial@10010000
  Out:   serial@10010000
  Err:   serial@10010000
  Net:   failed to get gemgxl_reset reset

  Warning: ethernet@10090000 MAC addresses don't match:
  Address in DT is                52:54:00:12:34:56
  Address in environment is       70:b3:d5:92:f0:01
  eth0: ethernet@10090000
  Hit any key to stop autoboot:  0
  => sf probe
  SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB, total 64 MiB
  => sf test 0 8000
  SPI flash test:
  0 erase: 1 ticks, 32000 KiB/s 256.000 Mbps
  1 check: 80 ticks, 400 KiB/s 3.200 Mbps
  2 write: 83 ticks, 385 KiB/s 3.080 Mbps
  3 read: 79 ticks, 405 KiB/s 3.240 Mbps
  Test passed
  0 erase: 1 ticks, 32000 KiB/s 256.000 Mbps
  1 check: 80 ticks, 400 KiB/s 3.200 Mbps
  2 write: 83 ticks, 385 KiB/s 3.080 Mbps
  3 read: 79 ticks, 405 KiB/s 3.240 Mbps

I am sure there will be bugs, and I have not tested all flashes affected.
But I want to send out this series for an early discussion and comments.
I will continue my testing.


Bin Meng (9):
  hw/block: m25p80: Fix the number of dummy bytes needed for Windbond
    flashes
  hw/block: m25p80: Fix the number of dummy bytes needed for
    Numonyx/Micron flashes
  hw/block: m25p80: Fix the number of dummy bytes needed for Macronix
    flashes
  hw/block: m25p80: Fix the number of dummy bytes needed for Spansion
    flashes
  hw/block: m25p80: Support fast read for SST flashes
  hw/ssi: xilinx_spips: Fix generic fifo dummy cycle handling
  Revert "aspeed/smc: Fix number of dummy cycles for FAST_READ_4
    command"
  Revert "aspeed/smc: snoop SPI transfers to fake dummy cycles"
  hw/ssi: npcm7xx_fiu: Correct the dummy cycle emulation logic

 include/hw/ssi/aspeed_smc.h |   3 -
 hw/block/m25p80.c           | 153 ++++++++++++++++++++++++++++--------
 hw/ssi/aspeed_smc.c         | 116 +--------------------------
 hw/ssi/npcm7xx_fiu.c        |   8 +-
 hw/ssi/xilinx_spips.c       |  29 ++++++-
 5 files changed, 153 insertions(+), 156 deletions(-)

-- 
2.25.1



             reply	other threads:[~2021-01-14 15:17 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14 15:08 Bin Meng [this message]
2021-01-14 15:08 ` [PATCH 1/9] hw/block: m25p80: Fix the number of dummy bytes needed for Windbond flashes Bin Meng
2021-01-14 15:08 ` [PATCH 2/9] hw/block: m25p80: Fix the number of dummy bytes needed for Numonyx/Micron flashes Bin Meng
2021-01-14 15:08 ` [PATCH 3/9] hw/block: m25p80: Fix the number of dummy bytes needed for Macronix flashes Bin Meng
2021-01-14 15:08 ` [PATCH 4/9] hw/block: m25p80: Fix the number of dummy bytes needed for Spansion flashes Bin Meng
2021-01-14 15:08 ` [PATCH 5/9] hw/block: m25p80: Support fast read for SST flashes Bin Meng
2021-01-14 15:08 ` [PATCH 6/9] hw/ssi: xilinx_spips: Fix generic fifo dummy cycle handling Bin Meng
2021-01-14 15:09 ` [PATCH 7/9] Revert "aspeed/smc: Fix number of dummy cycles for FAST_READ_4 command" Bin Meng
2021-01-14 15:09 ` [PATCH 8/9] Revert "aspeed/smc: snoop SPI transfers to fake dummy cycles" Bin Meng
2021-01-14 15:09 ` [PATCH 9/9] hw/ssi: npcm7xx_fiu: Correct the dummy cycle emulation logic Bin Meng
2021-01-14 17:12   ` Havard Skinnemoen via
2021-01-14 15:59 ` [PATCH 0/9] hw/block: m25p80: Fix the mess of dummy bytes needed for fast read commands Cédric Le Goater
2021-01-14 16:12 ` no-reply
2021-01-14 18:13 ` Francisco Iglesias
2021-01-15  2:07   ` Bin Meng
2021-01-15  3:29     ` Havard Skinnemoen via
2021-01-15 13:54       ` Bin Meng
2021-01-15 12:26     ` Francisco Iglesias
2021-01-15 14:38       ` Bin Meng
2021-01-18 10:05         ` Francisco Iglesias
2021-01-18 12:32           ` Bin Meng
2021-01-19 13:01             ` Francisco Iglesias
2021-01-20 14:20               ` Bin Meng
2021-01-21  8:50                 ` Francisco Iglesias
2021-01-21  8:59                   ` Bin Meng
2021-01-21 10:01                     ` Francisco Iglesias
2021-01-21 14:18                     ` Francisco Iglesias
2021-02-08 14:41                       ` Bin Meng
2021-02-08 15:30                         ` Edgar E. Iglesias
2021-02-09  9:35                           ` Francisco Iglesias
2021-04-23  6:45                         ` Bin Meng
2021-04-27  5:56                           ` Alistair Francis
2021-04-27  8:54                             ` Francisco Iglesias
2021-04-27 14:32                               ` Cédric Le Goater
2021-04-28 13:12                                 ` Bin Meng
2021-04-28 13:54                                   ` Cédric Le Goater

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=20210114150902.11515-1-bmeng.cn@gmail.com \
    --to=bmeng.cn@gmail.com \
    --cc=alistair.francis@wdc.com \
    --cc=andrew@aj.id.au \
    --cc=bin.meng@windriver.com \
    --cc=clg@kaod.org \
    --cc=f4bug@amsat.org \
    --cc=frasse.iglesias@gmail.com \
    --cc=hskinnemoen@google.com \
    --cc=joel@jms.id.au \
    --cc=kfting@nuvoton.com \
    --cc=komlodi@xilinx.com \
    --cc=kwolf@redhat.com \
    --cc=marcin.krzeminski@nokia.com \
    --cc=mreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.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).