* [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition
@ 2026-05-29 16:05 Miquel Raynal
2026-05-29 16:05 ` [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support Miquel Raynal
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
Hello,
Following my "software protection" enhancement series, there were
several comments about the need to simplify and reorganize the handling
of the QE bits, which is strongly impacted by the various read/write
status register capabilities.
The first patch is all about this. It is a massive cleanup, everything
is explained inside, the how and the why. This is a commit that must be
tested, so if you have a SPI NOR chip with one of the not yet tested
SFDP QER configuration, please consider applying that patch (just patch
1 is enough) and boot. If your chip is recognized, it's probably all
good.
Then I add support for JESD216 revision F and its new QER field.
Finally, I propose two Winbond chip additions (from the new RV family)
which makes use of these new capabilities.
There will be more chips to come, but that is all I can test at the
moment.
Thanks,
Miquèl
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Miquel Raynal (5):
mtd: spi-nor: Refactor Read Status/Write Status support
mtd: spi-nor: Add support for the new JESD216 rev F QER field
mtd: spi-nor: Move the SFDP header structure to a C header
mtd: spi-nor: winbond: Add support for W25Q02RV-M
mtd: spi-nor: winbond: Add support for W25Q51RV-M
drivers/mtd/spi-nor/atmel.c | 50 +--
drivers/mtd/spi-nor/core.c | 702 +++++++++++----------------------------
drivers/mtd/spi-nor/core.h | 54 ++-
drivers/mtd/spi-nor/debugfs.c | 1 -
drivers/mtd/spi-nor/gigadevice.c | 6 +-
drivers/mtd/spi-nor/issi.c | 3 +-
drivers/mtd/spi-nor/macronix.c | 12 +-
drivers/mtd/spi-nor/otp.c | 16 +-
drivers/mtd/spi-nor/sfdp.c | 55 ++-
drivers/mtd/spi-nor/sfdp.h | 19 ++
drivers/mtd/spi-nor/spansion.c | 16 +-
drivers/mtd/spi-nor/sst.c | 5 +-
drivers/mtd/spi-nor/swp.c | 58 +---
drivers/mtd/spi-nor/winbond.c | 59 +++-
include/linux/mtd/spi-nor.h | 5 +-
15 files changed, 401 insertions(+), 660 deletions(-)
---
base-commit: 5e9b7d093f3f77cb0af4409559e3d139babfb443
change-id: 20260529-winbond-v7-1-spi-nor-rv-addition-cea2f1f158be
prerequisite-change-id: 20251114-winbond-v6-18-rc1-spi-nor-swp-865d36f4f695:v6
prerequisite-patch-id: 41fa5f57751ccb1324c31df8d0c7d2b327fa1a17
prerequisite-patch-id: 31b2eb1f1148054d1cd45e488c0e973e1621f44c
prerequisite-patch-id: b376391b7772443e75b82d9ceb4d6c564371330a
prerequisite-patch-id: 8d5bfc3369cf6080eb60b7a32de5bfbde4475431
prerequisite-patch-id: 6fad316bee00a8f453527a1f338538540b7356a2
prerequisite-patch-id: c16390e544c15e2ae78ab67ab536cc424ac7c620
prerequisite-patch-id: 3b619f843842f6c00048c90a34d615bafa37ca23
prerequisite-patch-id: a3b7f9516e4eb69e4feae624059c5985b189a9b2
prerequisite-patch-id: 90bfb22a3519fad4d490c7016b6fb8137c1ade08
prerequisite-patch-id: c9375e2a9cf35e00e29a3feb8673db6102e2bafd
prerequisite-patch-id: 283bdc0412147df8f87f823ae2f876667dd00b4c
prerequisite-patch-id: 4e324cb821c96ddffd5b68889f7fb4f340df4b0b
prerequisite-patch-id: ff00869a1a045f279657de31495773594b0aca93
prerequisite-patch-id: 594980c7c187274761a6a10b1803c630976d8290
prerequisite-patch-id: 0ead288e109031e5653558e809778b8561862b28
prerequisite-patch-id: a74f5deb8e8df344c35757adae5072964a5289f5
prerequisite-patch-id: 6dab8d535c34e1c1e84a295c70e48c3bc6dac9f5
prerequisite-patch-id: 09b411dfea1e51b8a33ebcf0887260b5d05fdbc5
prerequisite-patch-id: 83aeb811574fc5242b3709335120854423e60173
prerequisite-patch-id: 0366883be8924559d19c4732d99d3b9df4db7dd1
prerequisite-patch-id: c0a80c658297b6dad22be20efa14e53e928a0f80
prerequisite-patch-id: 6883cb379f52c055d4d1cdc4ea798c7b775d0dd8
prerequisite-patch-id: 2fa7c195a618e53ba5181f308a467817e7efcfca
prerequisite-patch-id: e100bc5f348900a1c976932e3dc6482ae7a5dda9
prerequisite-patch-id: ee34be9e4b9a3d2333a22cd7643a292a45de2c0d
prerequisite-patch-id: 3024fbd42edbc8c246805c7533e314e5a1462ea7
prerequisite-patch-id: d8271437833307db86abf5a952bc0227562c9283
prerequisite-patch-id: 2f65c33b74cce2d0b891974ffe68babe0e829894
prerequisite-message-id: <20260529-winbond-v7-1-spi-nor-jv-cleanup-v1-0-87e5d3122244@bootlin.com>
prerequisite-patch-id: 90271b7e055db3079d3897546c3480a285b740c8
prerequisite-patch-id: 600c72d0a18dc003917f774209625f24c37d11d5
prerequisite-patch-id: 2a40afdd885392bab2b93035db111670016b16e5
prerequisite-patch-id: 840643fd3e1c3d725c1db2ec3ec9f056c0b59203
prerequisite-patch-id: 7d7af1e7bcc2efa8194805a58524b63bcdcaf388
prerequisite-patch-id: 010fd4751671a20d249ecb772bea9d16bf58c76d
prerequisite-patch-id: 0a269f09cd1a9c6597533ef6814141ffd7bca37c
prerequisite-patch-id: c11657122b4e8aac7d93fee35bc6bb8afddc84cb
prerequisite-patch-id: a8d07b1ee7c8d91991531baf7b12294e849ce767
prerequisite-patch-id: 7d822caac76012627ed5e8889011088d721a7bd4
prerequisite-patch-id: 1b688642b8795aecfedd833b4606a992795c4007
prerequisite-patch-id: 7c23d56ea0f3cf6edec6e849bc923ff99a8b68e5
prerequisite-patch-id: 0a9d35ce50c6a2c45b1adca4796766e3eeae79c4
prerequisite-patch-id: 6bf63b2dcd7105e1833117a8d7c40b9bbf4b3669
prerequisite-patch-id: 462a6751e3e9074102e8d31b23bf06140f6ee6e6
prerequisite-patch-id: 4e8a8ccadb8ed079d8e5271bab44e03050024adc
prerequisite-patch-id: 94455a1cc1ba7fac00a1be1fa961f1597878fe85
prerequisite-patch-id: 7e731eebba360dd2f5263aea13b697a1488a6283
prerequisite-patch-id: 78dac39c5737a166cfd2b04002a5e0a131ee8940
prerequisite-patch-id: 0dc8b0344e8b29f298c856477bc5acaa61d3967a
prerequisite-patch-id: a46684d4159abe1e3fd8f76c45001ecf52a92973
prerequisite-patch-id: f07c3b16f229881d2e17f4a8addf3e79835bd641
prerequisite-patch-id: b67fe113cd46f03786adcb69eaf86190eac1d33f
prerequisite-patch-id: ea181a86120f14b48e9ec77e467cd2cce8ce208e
prerequisite-patch-id: 20ce5105adc4c7f8ebc6c779b107fd54def4f1f8
prerequisite-patch-id: 34ed2ebf8e08b20e9443fb95591478840a530b36
prerequisite-patch-id: eaeebc9df5221bab68a6cabc64a97e9c2608899b
prerequisite-patch-id: fb220fdab1d5722ef18827e1629f466f32b14a3e
prerequisite-patch-id: f80c10737ffd66f4b4d700149efe2e354ffd7b46
prerequisite-patch-id: b0cb24cc785ea0f871b60effc6b8334c91aaa0f9
Best regards,
--
Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
@ 2026-05-29 16:05 ` Miquel Raynal
2026-07-07 9:43 ` Michael Walle
2026-05-29 16:05 ` [PATCH 2/5] mtd: spi-nor: Add support for the new JESD216 rev F QER field Miquel Raynal
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
SPI NOR has a lot of history. Additions over additions, the subtleties
of the JESD216 specification, their implementations by the manufacturers
and the hardware mistakes have generated a gigantic maze, let's try to
understand what is really needed.
The specification explains the QE (Quad Enable) bitfield as describing
where the bit to enable the Quad capability is, but also how to set
it. Unfortunately, the specification is not precise about what opcodes
are supported exactly in all the cases. There is an introduction that
basically states:
- Opcode 0x05 reads SR1, and then SR2 if another byte is read
- Opcode 0x35 allows to read SR2
- Opcode 0x01 writes SR1, and then SR2 if another byte is written
Then the bitfield, among indicating the location of the QE bit, may
indicate:
- Reading SR1 and SR2 in one operation is not possible (loops over the
content of SR1)
- Reading SR2 directly is possible
- Writing SR1 smashes SR2
One problem with the current implementation, is that it only focuses on
the QE bit. A quad_enable function was created for each case, even
though in practice, the logic was always the same: read, modify, write,
read back and verify. One problem comes when other features need to play
with the status registers, like software block protection or OTP: you
never know how to properly handle the QE bit, nor where it is, nor how
to read/write the Status registers. This lead to
approximations/guessing (in swp.c, otp.c and obviously in legacy
controller drivers like atmel.c) but also to the implementation of a
gazillon of helpers for reading/writing/checking the status registers.
In addition, I believe some design decisions had a negative impact over
the years.
- All possible situations had to be flagged by the core. This is likely
wrong, because we no longer know why we need specific quirks. It was
ineherent to the state of the SPI NOR core before the great cleanup
that had happened the past few years. I believe this creates confusion
in the core today, and we should push this to vendor fixups
instead. As an example, in 2023 Hsin-Yi was facing an issue because
his chip was setting the wrong QER value, leading to RDCR being
prevented, thus falling into a condition blindly setting a random QE
bit (I strongy believe it is done like that for wrong reasons). His
chip actually had RDCR support! The correct fix should have been to
mark the capability in a device fixup instead of handling this in the
core.
Link: https://lore.kernel.org/lkml/CAJMQK-hR0eaO0b4Vd0U8_KAndLyZapqdHjVLAoe42rWi9rdLkA@mail.gmail.com/
- SFDP parsing is over cautious. I believe
BFPT_DWORD15_QER_SR2_BIT1_NO_RD is abusive (nothing states that RDCR is
not supported), and BFPT_DWORD15_QER_SR2_BIT1 is also out of
specification when forcing 16-bit Status writes.
- SNOR_F_HAS_16BIT_SR is only imposing 16bit Status writes, whereas
reads can still be 8-bit wides and even sometimes can only be 8-bit
wide.
- The usage of helpers verifying the writes was also spread for IMHO no
really good reason. Why shouldn't we trust spi operations when it
comes to Status Registers? We do not read back our page reads, so why
status registers should be treated with so much care, if it's not
because we are unsure of what is being done? My proposal includes a
check when it comes to the QE bit (done once) but we don't need these
checks otherwise. If the QE bit was written properly, there are high
chances that the other register accesses will just be fine, no?
Asde from my main quest, I also observed no good reason to ask the
read/write status register callers to use nor->bouncebuf while the
low-level helpers could do it themselves (we are talking about one or
two bytes being copied).
So after these observations, my proposal is the following:
- Create private low level helpers that just read or write a status
register. They are flexible, we can give the opcode (which varies
based on the SFDP QER field) and the length (1 or 2).
- Create public generic accessors which will be used to read/write sr1
and/or sr2. This is where all the cleverness shall be. The helpers use
the available opcodes for a given chip in order to fullfill the
request.
- Provide a single generic ->quad_enable() hook which generically does
all the steps mentioned above (read, modify, write, read back and
verify).
- Create a list of opcodes for all 6 possible situations:
{read, write} {sr1, sr2, sr1 and sr2}. These opcodes are
filled/cleared based on the QER field. An opcode set to 0 indicates
the absence of support (there is no 0x00 opcode in SPI NOR).
I tried my best to analyze the current behavior and to mimic it as much
as possible, but this is a risky cleanup. However, if we go for this, it
will be *much* easier in the future to handle all kind of chip
variations. We won't be limited to a couple of flags anymore, but rather
we'll be able to just disable a read or write capability using a single
line.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
This is a really hard to review patch, it is big, full of changes
everywhere, but I cannot really split it further. The commit log is
really important, it tries to explain the steps I went through when
trying to refactor the whole QE/SR handling for which I received
feedback from Michael, Tudor and Sashiko.
Ideally we could expect test coverage of each case:
- BFPT_DWORD15_QER_NONE: Probably not needed
- BFPT_DWORD15_QER_SR2_BIT1_BUGGY: Expected from Hsin-Yi
- BFPT_DWORD15_QER_SR2_BIT1_NO_RD: Tested with a chip re-enabling RDCR
in a fixup
- BFPT_DWORD15_QER_SR1_BIT6: TBD
- BFPT_DWORD15_QER_SR2_BIT7: TBD
- BFPT_DWORD15_QER_SR2_BIT1: TBD
- BFPT_DWORD15_QER_SR2_BIT1_1B: Done.
There will probably be breakages on older chips. These cannot be
guessed because they are not properly listed in manufacturer
fixups (yet). If we want a cleanup/simplification, we will have to cope
with this risk.
I hope the diff stats will motivate people to have a look and report
their testing.
I will also eagerly monitor Sashiko's output which will probably be
very useful to catch niche weird cases where these changes might break.
---
drivers/mtd/spi-nor/atmel.c | 50 +--
drivers/mtd/spi-nor/core.c | 702 +++++++++++----------------------------
drivers/mtd/spi-nor/core.h | 54 ++-
drivers/mtd/spi-nor/debugfs.c | 1 -
drivers/mtd/spi-nor/gigadevice.c | 6 +-
drivers/mtd/spi-nor/issi.c | 3 +-
drivers/mtd/spi-nor/macronix.c | 12 +-
drivers/mtd/spi-nor/otp.c | 16 +-
drivers/mtd/spi-nor/sfdp.c | 37 ++-
drivers/mtd/spi-nor/spansion.c | 16 +-
drivers/mtd/spi-nor/sst.c | 5 +-
drivers/mtd/spi-nor/swp.c | 58 +---
drivers/mtd/spi-nor/winbond.c | 4 +-
include/linux/mtd/spi-nor.h | 4 -
14 files changed, 326 insertions(+), 642 deletions(-)
diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
index 82c592f0a1e1..124ceb795f56 100644
--- a/drivers/mtd/spi-nor/atmel.c
+++ b/drivers/mtd/spi-nor/atmel.c
@@ -23,18 +23,28 @@ static int at25fs_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
static int at25fs_nor_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
{
+ /* Write 0x00 to the status register to disable write protection */
+ u8 sr = 0;
int ret;
/* We only support unlocking the whole flash array */
if (ofs || len != nor->params->size)
return -EINVAL;
- /* Write 0x00 to the status register to disable write protection */
- ret = spi_nor_write_sr_and_check(nor, 0);
+ ret = spi_nor_write_srs(nor, &sr, NULL);
if (ret)
- dev_dbg(nor->dev, "unable to clear BP bits, WP# asserted?\n");
+ return ret;
- return ret;
+ ret = spi_nor_read_srs(nor, &sr, NULL);
+ if (ret)
+ return ret;
+
+ if (sr) {
+ dev_dbg(nor->dev, "unable to clear BP bits, WP# asserted?\n");
+ return -EIO;
+ }
+
+ return 0;
}
static int at25fs_nor_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
@@ -69,6 +79,7 @@ static const struct spi_nor_fixups at25fs_nor_fixups = {
* Return: 0 on success, -error otherwise.
*/
static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
+
u64 len, bool is_protect)
{
int ret;
@@ -78,19 +89,24 @@ static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
if (ofs || len != nor->params->size)
return -EINVAL;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &sr, NULL);
if (ret)
return ret;
- sr = nor->bouncebuf[0];
-
/* SRWD bit needs to be cleared, otherwise the protection doesn't change */
if (sr & SR_SRWD) {
sr &= ~SR_SRWD;
- ret = spi_nor_write_sr_and_check(nor, sr);
- if (ret) {
- dev_dbg(nor->dev, "unable to clear SRWD bit, WP# asserted?\n");
+ ret = spi_nor_write_srs(nor, &sr, NULL);
+ if (ret)
return ret;
+
+ ret = spi_nor_read_srs(nor, &sr, NULL);
+ if (ret)
+ return ret;
+
+ if (sr & SR_SRWD) {
+ dev_dbg(nor->dev, "unable to clear SRWD bit, WP# asserted?\n");
+ return -EIO;
}
}
@@ -108,14 +124,7 @@ static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
sr &= ~ATMEL_SR_GLOBAL_PROTECT_MASK;
}
- nor->bouncebuf[0] = sr;
-
- /*
- * We cannot use the spi_nor_write_sr_and_check() because this command
- * isn't really setting any bits, instead it is an pseudo command for
- * "Global Unprotect" or "Global Protect"
- */
- return spi_nor_write_sr(nor, nor->bouncebuf, 1);
+ return spi_nor_write_srs(nor, &sr, NULL);
}
static int atmel_nor_global_protect(struct spi_nor *nor, loff_t ofs, u64 len)
@@ -131,16 +140,17 @@ static int atmel_nor_global_unprotect(struct spi_nor *nor, loff_t ofs, u64 len)
static int atmel_nor_is_global_protected(struct spi_nor *nor, loff_t ofs,
u64 len)
{
+ u8 sr;
int ret;
if (ofs >= nor->params->size || (ofs + len) > nor->params->size)
return -EINVAL;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &sr, NULL);
if (ret)
return ret;
- return ((nor->bouncebuf[0] & ATMEL_SR_GLOBAL_PROTECT_MASK) == ATMEL_SR_GLOBAL_PROTECT_MASK);
+ return ((sr & ATMEL_SR_GLOBAL_PROTECT_MASK) == ATMEL_SR_GLOBAL_PROTECT_MASK);
}
static const struct spi_nor_locking_ops atmel_nor_global_protection_ops = {
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index b25d1a870a22..ca66661a7f44 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -443,75 +443,6 @@ int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
return ret;
}
-/**
- * spi_nor_read_sr() - Read the Status Register.
- * @nor: pointer to 'struct spi_nor'.
- * @sr: pointer to a DMA-able buffer where the value of the
- * Status Register will be written. Should be at least 2 bytes.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op = SPI_NOR_RDSR_OP(sr);
-
- if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
- op.addr.nbytes = nor->params->rdsr_addr_nbytes;
- op.dummy.nbytes = nor->params->rdsr_dummy;
- /*
- * We don't want to read only one byte in DTR mode. So,
- * read 2 and then discard the second byte.
- */
- op.data.nbytes = 2;
- }
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDSR, sr,
- 1);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d reading SR\n", ret);
-
- return ret;
-}
-
-/**
- * spi_nor_read_cr() - Read the Configuration Register using the
- * SPINOR_OP_RDCR (35h) command.
- * @nor: pointer to 'struct spi_nor'
- * @cr: pointer to a DMA-able buffer where the value of the
- * Configuration Register will be written.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op = SPI_NOR_RDCR_OP(cr);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDCR, cr,
- 1);
- }
-
- if (ret)
- dev_dbg(nor->dev, "error %d reading CR\n", ret);
-
- return ret;
-}
-
/**
* spi_nor_set_4byte_addr_mode_en4b_ex4b() - Enter/Exit 4-byte address mode
* using SPINOR_OP_EN4B/SPINOR_OP_EX4B. Typically used by
@@ -617,12 +548,13 @@ int spi_nor_set_4byte_addr_mode_brwr(struct spi_nor *nor, bool enable)
int spi_nor_sr_ready(struct spi_nor *nor)
{
int ret;
+ u8 sr;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &sr, NULL);
if (ret)
return ret;
- return !(nor->bouncebuf[0] & SR_WIP);
+ return !(sr & SR_WIP);
}
/**
@@ -784,345 +716,210 @@ int spi_nor_global_block_unlock(struct spi_nor *nor)
}
/**
- * spi_nor_write_sr() - Write the Status Register.
+ * spi_nor_read_srs_ll() - Low-level Status Registers read.
* @nor: pointer to 'struct spi_nor'.
- * @sr: pointer to DMA-able buffer to write to the Status Register.
- * @len: number of bytes to write to the Status Register.
+ * @opcode: opcode for the status register read operation.
+ * @sr: pointer to buffer for storing the contnet of the status registers.
+ * @len: number of status registers to read.
*
* Return: 0 on success, -errno otherwise.
*/
-int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
+static int spi_nor_read_srs_ll(struct spi_nor *nor, u8 opcode, u8 *sr2,
+ unsigned int len)
{
- int ret;
+ int ret, i;
- ret = spi_nor_write_enable(nor);
- if (ret)
- return ret;
+ for (i = 0; i < len; i++)
+ nor->bouncebuf[i] = 0;
if (nor->spimem) {
- struct spi_mem_op op = SPI_NOR_WRSR_OP(sr, len);
+ struct spi_mem_op op = SPI_NOR_RDSR_OP(opcode, nor->bouncebuf, len);
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_WRSR, sr,
- len);
+ ret = spi_nor_controller_ops_read_reg(nor, opcode, nor->bouncebuf, len);
}
- if (ret) {
- dev_dbg(nor->dev, "error %d writing SR\n", ret);
- return ret;
- }
-
- return spi_nor_wait_till_ready(nor);
-}
-
-/**
- * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
- * ensure that the byte written match the received value.
- * @nor: pointer to a 'struct spi_nor'.
- * @sr1: byte value to be written to the Status Register.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
-{
- int ret;
-
- nor->bouncebuf[0] = sr1;
-
- ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- if (nor->bouncebuf[0] != sr1) {
- dev_dbg(nor->dev, "SR1: read back test failed\n");
- return -EIO;
- }
-
- return 0;
-}
-
-/**
- * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
- * Status Register 2 in one shot. Ensure that the byte written in the Status
- * Register 1 match the received value, and that the 16-bit Write did not
- * affect what was already in the Status Register 2.
- * @nor: pointer to a 'struct spi_nor'.
- * @sr1: byte value to be written to the Status Register 1.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
-{
- int ret;
- u8 *sr_cr = nor->bouncebuf;
- u8 cr_written;
-
- /* Make sure we don't overwrite the contents of Status Register 2. */
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- return ret;
- } else if ((spi_nor_get_protocol_width(nor->read_proto) == 4 ||
- spi_nor_get_protocol_width(nor->write_proto) == 4) &&
- nor->params->quad_enable) {
- /*
- * If the Status Register 2 Read command (35h) is not
- * supported, we should at least be sure we don't
- * change the value of the SR2 Quad Enable bit.
- *
- * When the Quad Enable method is set and the buswidth is 4, we
- * can safely assume that the value of the QE bit is one, as a
- * consequence of the nor->params->quad_enable() call.
- *
- * According to the JESD216 revB standard, BFPT DWORDS[15],
- * bits 22:20, the 16-bit Write Status (01h) command is
- * available just for the cases in which the QE bit is
- * described in SR2 at BIT(1).
- */
- sr_cr[1] = SR2_QUAD_EN_BIT1;
- } else {
- sr_cr[1] = 0;
- }
-
- sr_cr[0] = sr1;
-
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, sr_cr);
- if (ret)
- return ret;
-
- if (sr1 != sr_cr[0]) {
- dev_dbg(nor->dev, "SR: Read back test failed\n");
- return -EIO;
- }
-
- if (nor->flags & SNOR_F_NO_READ_CR)
- return 0;
-
- cr_written = sr_cr[1];
-
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- return ret;
-
- if (cr_written != sr_cr[1]) {
- dev_dbg(nor->dev, "CR: read back test failed\n");
- return -EIO;
- }
-
- return 0;
-}
-
-/**
- * spi_nor_write_16bit_cr_and_check() - Write the Status Register 1 and the
- * Configuration Register in one shot. Ensure that the byte written in the
- * Configuration Register match the received value, and that the 16-bit Write
- * did not affect what was already in the Status Register 1.
- * @nor: pointer to a 'struct spi_nor'.
- * @cr: byte value to be written to the Configuration Register.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
-{
- int ret;
- u8 *sr_cr = nor->bouncebuf;
- u8 sr_written;
-
- /* Keep the current value of the Status Register 1. */
- ret = spi_nor_read_sr(nor, sr_cr);
- if (ret)
- return ret;
-
- sr_cr[1] = cr;
-
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- return ret;
-
- sr_written = sr_cr[0];
-
- ret = spi_nor_read_sr(nor, sr_cr);
- if (ret)
- return ret;
-
- if (sr_written != sr_cr[0]) {
- dev_dbg(nor->dev, "SR: Read back test failed\n");
- return -EIO;
- }
-
- if (nor->flags & SNOR_F_NO_READ_CR)
- return 0;
-
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- return ret;
-
- if (cr != sr_cr[1]) {
- dev_dbg(nor->dev, "CR: read back test failed\n");
- return -EIO;
- }
-
- return 0;
-}
-
-/**
- * spi_nor_write_16bit_sr_cr_and_check() - Write the Status Register 1 and the
- * Configuration Register in one shot. Ensure that the bytes written in both
- * registers match the received value.
- * @nor: pointer to a 'struct spi_nor'.
- * @regs: two-byte array with values to be written to the status and
- * configuration registers.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_write_16bit_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
-{
- u8 written_regs[2];
- int ret;
-
- written_regs[0] = regs[0];
- written_regs[1] = regs[1];
- nor->bouncebuf[0] = regs[0];
- nor->bouncebuf[1] = regs[1];
-
- ret = spi_nor_write_sr(nor, nor->bouncebuf, 2);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- if (written_regs[0] != nor->bouncebuf[0]) {
- dev_dbg(nor->dev, "SR: Read back test failed\n");
- return -EIO;
- }
-
- if (nor->flags & SNOR_F_NO_READ_CR)
- return 0;
-
- ret = spi_nor_read_cr(nor, &nor->bouncebuf[1]);
- if (ret)
- return ret;
-
- if (written_regs[1] != nor->bouncebuf[1]) {
- dev_dbg(nor->dev, "CR: read back test failed\n");
- return -EIO;
- }
-
- return 0;
-}
-
-/**
- * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
- * the byte written match the received value without affecting other bits in the
- * Status Register 1 and 2.
- * @nor: pointer to a 'struct spi_nor'.
- * @sr1: byte value to be written to the Status Register.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
-{
- if (nor->flags & SNOR_F_HAS_16BIT_SR)
- return spi_nor_write_16bit_sr_and_check(nor, sr1);
-
- return spi_nor_write_sr1_and_check(nor, sr1);
-}
-
-/**
- * spi_nor_write_sr_cr_and_check() - Write the Status Register 1 and ensure that
- * the byte written match the received value. Same for the Control Register if
- * available.
- * @nor: pointer to a 'struct spi_nor'.
- * @regs: byte array to be written to the registers.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
-{
- if (nor->flags & SNOR_F_HAS_16BIT_SR)
- return spi_nor_write_16bit_sr_cr_and_check(nor, regs);
-
- return spi_nor_write_sr1_and_check(nor, regs[0]);
-}
-
-/**
- * spi_nor_write_sr2() - Write the Status Register 2 using the
- * SPINOR_OP_WRSR2 (3eh) command.
- * @nor: pointer to 'struct spi_nor'.
- * @sr2: pointer to DMA-able buffer to write to the Status Register 2.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
-{
- int ret;
-
- ret = spi_nor_write_enable(nor);
- if (ret)
- return ret;
-
- if (nor->spimem) {
- struct spi_mem_op op = SPI_NOR_WRSR2_OP(sr2);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_WRSR2,
- sr2, 1);
- }
-
- if (ret) {
- dev_dbg(nor->dev, "error %d writing SR2\n", ret);
- return ret;
- }
-
- return spi_nor_wait_till_ready(nor);
-}
-
-/**
- * spi_nor_read_sr2() - Read the Status Register 2 using the
- * SPINOR_OP_RDSR2 (3fh) command.
- * @nor: pointer to 'struct spi_nor'.
- * @sr2: pointer to DMA-able buffer where the value of the
- * Status Register 2 will be written.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
-{
- int ret;
-
- if (nor->spimem) {
- struct spi_mem_op op = SPI_NOR_RDSR2_OP(sr2);
-
- spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDSR2, sr2,
- 1);
- }
+ memcpy(sr2, nor->bouncebuf, len);
if (ret)
- dev_dbg(nor->dev, "error %d reading SR2\n", ret);
+ dev_dbg(nor->dev, "Error %d reading Status Registers\n", ret);
return ret;
}
+/**
+ * spi_nor_write_srs_ll() - Low-level Status Registers write.
+ * @nor: pointer to 'struct spi_nor'.
+ * @opcode: opcode for the status register write operation.
+ * @srs: pointer to status registers buffer to write.
+ * @len: number of status registers to write.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_srs_ll(struct spi_nor *nor, u8 opcode, const u8 *srs,
+ unsigned int len)
+{
+ int ret, i;
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < len; i++)
+ nor->bouncebuf[i] = srs[i];
+
+ if (nor->spimem) {
+ struct spi_mem_op op = SPI_NOR_WRSR_OP(opcode,
+ nor->bouncebuf, len);
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = spi_nor_controller_ops_write_reg(nor, opcode,
+ nor->bouncebuf, len);
+ }
+
+ if (ret) {
+ dev_dbg(nor->dev, "Error %d writing Status Registers\n", ret);
+ return ret;
+ }
+
+ return spi_nor_wait_till_ready(nor);
+}
+
+int spi_nor_read_srs(struct spi_nor *nor, u8 *sr1, u8 *sr2)
+{
+ struct spi_nor_flash_parameter *params = nor->params;
+ int ret;
+
+ if (params->read_srs_opcode &&
+ ((sr1 && !params->read_sr1_opcode) || (sr2 && !params->read_sr2_opcode))) {
+ u8 srs[2] = {};
+
+ ret = spi_nor_read_srs_ll(nor, params->read_srs_opcode, srs, 2);
+ if (ret)
+ return ret;
+
+ if (sr1)
+ *sr1 = srs[0];
+
+ if (sr2)
+ *sr2 = srs[1];
+
+ return 0;
+ }
+
+ if (sr1) {
+ ret = spi_nor_read_srs_ll(nor, params->read_sr1_opcode, sr1, 1);
+ if (ret)
+ return ret;
+ }
+
+ if (sr2) {
+ if (params->read_sr2_opcode) {
+ ret = spi_nor_read_srs_ll(nor, params->read_sr2_opcode, sr2, 1);
+ if (ret)
+ return ret;
+ } else {
+ /* Make sure the QE bit is persistently kept */
+ if (spi_nor_get_protocol_width(nor->read_proto) == 4 ||
+ spi_nor_get_protocol_width(nor->write_proto) == 4)
+ *sr2 = params->qe_mask[1];
+ }
+ }
+
+ return 0;
+}
+
+int spi_nor_write_srs(struct spi_nor *nor, const u8 *sr1, const u8 *sr2)
+{
+ struct spi_nor_flash_parameter *params = nor->params;
+ int ret;
+
+ if (nor->flags & SNOR_F_HAS_16BIT_SR) {
+ u8 srs[2];
+
+ if (!sr1 || !sr2) {
+ ret = spi_nor_read_srs_ll(nor, params->read_srs_opcode, srs, 2);
+ if (ret)
+ return ret;
+ }
+
+ if (sr1)
+ srs[0] = *sr1;
+ if (sr2)
+ srs[1] = *sr2;
+
+ return spi_nor_write_srs_ll(nor, params->write_srs_opcode, srs, 2);
+ }
+
+ if (sr1) {
+ ret = spi_nor_write_srs_ll(nor, params->write_sr1_opcode, sr1, 1);
+ if (ret)
+ return ret;
+ }
+
+ if (sr2 && params->read_sr2_opcode) {
+ ret = spi_nor_write_srs_ll(nor, params->write_sr2_opcode, sr2, 1);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_generic_quad_enable() - Read the status registers,
+ * apply the QE bit mask,
+ * write the status registers,
+ * read them back and check the content.
+ *
+ * @nor: pointer to 'struct spi_nor'
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_generic_quad_enable(struct spi_nor *nor)
+{
+ u8 *qe_mask = nor->params->qe_mask;
+ u8 srs[2] = {}, tmp[2] = {};
+ int ret;
+
+ if (!qe_mask[0] && !qe_mask[1])
+ return 0;
+
+ ret = spi_nor_read_srs(nor, &srs[0], &srs[1]);
+ if (ret)
+ return ret;
+
+ if (srs[0] & qe_mask[0] || srs[1] & qe_mask[1])
+ return 0;
+
+ srs[0] |= qe_mask[0];
+ srs[1] |= qe_mask[1];
+
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ ret = spi_nor_write_srs(nor, &srs[0], &srs[1]);
+ else
+ ret = spi_nor_write_srs(nor,
+ qe_mask[0] ? &srs[0] : NULL,
+ qe_mask[1] ? &srs[1] : NULL);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_srs(nor, &tmp[0], &tmp[1]);
+ if (ret)
+ return ret;
+
+ if (srs[0] != tmp[0] || srs[1] != tmp[1])
+ return -EINVAL;
+
+ return 0;
+}
+
/**
* spi_nor_erase_die() - Erase the entire die.
* @nor: pointer to 'struct spi_nor'.
@@ -1904,106 +1701,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
return ret;
}
-/**
- * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
- * Register 1.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
-{
- int ret;
-
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
- return 0;
-
- nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
-
- return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
-}
-
-/**
- * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
- * Register 2.
- * @nor: pointer to a 'struct spi_nor'.
- *
- * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
-{
- int ret;
-
- if (nor->flags & SNOR_F_NO_READ_CR)
- return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
-
- ret = spi_nor_read_cr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
- return 0;
-
- nor->bouncebuf[0] |= SR2_QUAD_EN_BIT1;
-
- return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
-}
-
-/**
- * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Status Register 2.
- *
- * This is one of the procedures to set the QE bit described in the SFDP
- * (JESD216 rev B) specification but no manufacturer using this procedure has
- * been identified yet, hence the name of the function.
- *
- * Return: 0 on success, -errno otherwise.
- */
-int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
-{
- u8 *sr2 = nor->bouncebuf;
- int ret;
- u8 sr2_written;
-
- /* Check current Quad Enable bit value. */
- ret = spi_nor_read_sr2(nor, sr2);
- if (ret)
- return ret;
- if (*sr2 & SR2_QUAD_EN_BIT7)
- return 0;
-
- /* Update the Quad Enable bit. */
- *sr2 |= SR2_QUAD_EN_BIT7;
-
- ret = spi_nor_write_sr2(nor, sr2);
- if (ret)
- return ret;
-
- sr2_written = *sr2;
-
- /* Read back and check it. */
- ret = spi_nor_read_sr2(nor, sr2);
- if (ret)
- return ret;
-
- if (*sr2 != sr2_written) {
- dev_dbg(nor->dev, "SR2: Read back test failed\n");
- return -EIO;
- }
-
- return 0;
-}
-
static const struct spi_nor_manufacturer *manufacturers[] = {
&spi_nor_atmel,
&spi_nor_eon,
@@ -2494,6 +2191,7 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
{
struct spi_nor_flash_parameter *params = nor->params;
unsigned int cap;
+ u8 opcode;
/* X-X-X modes are not supported yet, mask them all. */
*hwcaps &= ~SNOR_HWCAPS_X_X_X;
@@ -2525,14 +2223,14 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
*hwcaps &= ~BIT(cap);
}
- /* Some SPI controllers might not support CR read opcode. */
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- struct spi_mem_op op = SPI_NOR_RDCR_OP(nor->bouncebuf);
-
+ /* Some SPI controllers might not support reading SR2 */
+ opcode = nor->params->read_sr2_opcode;
+ if (opcode) {
+ struct spi_mem_op op = SPI_NOR_RDSR_OP(opcode, nor->bouncebuf, 1);
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
if (!spi_mem_supports_op(nor->spimem, &op))
- nor->flags |= SNOR_F_NO_READ_CR;
+ nor->params->read_sr2_opcode = 0;
}
}
@@ -3106,11 +2804,14 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
const struct flash_info *info = nor->info;
struct device_node *np = spi_nor_get_flash_node(nor);
- params->quad_enable = spi_nor_sr2_bit1_quad_enable;
- params->otp.org = info->otp;
-
- /* Default to 16-bit Write Status (01h) Command */
+ /* Default to 16-bit Read/Write Status commands */
nor->flags |= SNOR_F_HAS_16BIT_SR;
+ params->read_srs_opcode = SPINOR_OP_RDSR;
+ params->write_srs_opcode = SPINOR_OP_WRSR;
+ params->quad_enable = spi_nor_generic_quad_enable;
+ params->qe_mask[1] = BIT(1);
+
+ params->otp.org = info->otp;
/* Set SPI NOR sizes. */
params->writesize = 1;
@@ -3258,8 +2959,11 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
return 0;
if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
- spi_nor_get_protocol_width(nor->write_proto) == 4))
+ spi_nor_get_protocol_width(nor->write_proto) == 4)) {
+ nor->params->qe_mask[0] = 0;
+ nor->params->qe_mask[1] = 0;
return 0;
+ }
return nor->params->quad_enable(nor);
}
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index ba2d1a862c9d..d4aff914caf5 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -37,36 +37,18 @@
SPI_MEM_OP_NO_DUMMY, \
SPI_MEM_OP_NO_DATA)
-#define SPI_NOR_RDSR_OP(buf) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR, 0), \
+#define SPI_NOR_RDSR_OP(opcode, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0), \
SPI_MEM_OP_NO_ADDR, \
SPI_MEM_OP_NO_DUMMY, \
- SPI_MEM_OP_DATA_IN(1, buf, 0))
+ SPI_MEM_OP_DATA_IN(len, buf, 0))
-#define SPI_NOR_WRSR_OP(buf, len) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 0), \
+#define SPI_NOR_WRSR_OP(opcode, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0), \
SPI_MEM_OP_NO_ADDR, \
SPI_MEM_OP_NO_DUMMY, \
SPI_MEM_OP_DATA_OUT(len, buf, 0))
-#define SPI_NOR_RDSR2_OP(buf) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 0), \
- SPI_MEM_OP_NO_ADDR, \
- SPI_MEM_OP_NO_DUMMY, \
- SPI_MEM_OP_DATA_OUT(1, buf, 0))
-
-#define SPI_NOR_WRSR2_OP(buf) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 0), \
- SPI_MEM_OP_NO_ADDR, \
- SPI_MEM_OP_NO_DUMMY, \
- SPI_MEM_OP_DATA_OUT(1, buf, 0))
-
-#define SPI_NOR_RDCR_OP(buf) \
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR, 0), \
- SPI_MEM_OP_NO_ADDR, \
- SPI_MEM_OP_NO_DUMMY, \
- SPI_MEM_OP_DATA_IN(1, buf, 0))
-
#define SPI_NOR_EN4B_EX4B_OP(enable) \
SPI_MEM_OP(SPI_MEM_OP_CMD(enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B, 0), \
SPI_MEM_OP_NO_ADDR, \
@@ -130,7 +112,6 @@ enum spi_nor_option_flags {
SNOR_F_HAS_4BAIT = BIT(4),
SNOR_F_HAS_LOCK = BIT(5),
SNOR_F_HAS_16BIT_SR = BIT(6),
- SNOR_F_NO_READ_CR = BIT(7),
SNOR_F_HAS_SR_TB_BIT6 = BIT(8),
SNOR_F_HAS_4BIT_BP = BIT(9),
SNOR_F_HAS_SR_BP3_BIT6 = BIT(10),
@@ -375,6 +356,13 @@ struct spi_nor_otp {
* @otp: SPI NOR OTP info.
* @set_octal_dtr: enables or disables SPI NOR octal DTR mode.
* @quad_enable: enables SPI NOR quad mode.
+ * @qe_mask: two bytes mask used to set/clear the QE bit
+ * @read_srs_opcode: opcode used to read SR1 and SR2 in one operation
+ * @read_sr1_opcode: opcode used to read SR1 alone
+ * @read_sr2_opcode: opcode used to read SR2 alone
+ * @write_srs_opcode: opcode used to write SR1 and SR2 in one operation
+ * @write_sr1_opcode: opcode used to write SR1 alone
+ * @write_sr2_opcode: opcode used to write SR2 alone
* @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
* @ready: (optional) flashes might use a different mechanism
* than reading the status register to indicate they
@@ -405,6 +393,13 @@ struct spi_nor_flash_parameter {
int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
int (*quad_enable)(struct spi_nor *nor);
+ u8 qe_mask[2];
+ u8 read_srs_opcode;
+ u8 read_sr1_opcode;
+ u8 read_sr2_opcode;
+ u8 write_srs_opcode;
+ u8 write_sr1_opcode;
+ u8 write_sr2_opcode;
int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
int (*ready)(struct spi_nor *nor);
@@ -633,18 +628,11 @@ int spi_nor_wait_till_ready(struct spi_nor *nor);
int spi_nor_global_block_unlock(struct spi_nor *nor);
int spi_nor_prep_and_lock(struct spi_nor *nor);
void spi_nor_unlock_and_unprep(struct spi_nor *nor);
-int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
-int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
-int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
enum spi_nor_protocol reg_proto);
-int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
int spi_nor_sr_ready(struct spi_nor *nor);
-int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
-int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
-int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
-int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
-int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs);
+int spi_nor_read_srs(struct spi_nor *nor, u8 *sr1, u8 *sr2);
+int spi_nor_write_srs(struct spi_nor *nor, const u8 *sr1, const u8 *sr2);
ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
u8 *buf);
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index fbefcef2d2e1..5d1c31edb417 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -19,7 +19,6 @@ static const char *const snor_f_names[] = {
SNOR_F_NAME(HAS_4BAIT),
SNOR_F_NAME(HAS_LOCK),
SNOR_F_NAME(HAS_16BIT_SR),
- SNOR_F_NAME(NO_READ_CR),
SNOR_F_NAME(HAS_SR_TB_BIT6),
SNOR_F_NAME(HAS_4BIT_BP),
SNOR_F_NAME(HAS_SR_BP3_BIT6),
diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
index ef1edd0add70..4070a692e968 100644
--- a/drivers/mtd/spi-nor/gigadevice.c
+++ b/drivers/mtd/spi-nor/gigadevice.c
@@ -23,8 +23,10 @@ gd25q256_post_bfpt(struct spi_nor *nor,
* GD25Q256E | SFDP_JESD216_MAJOR | SFDP_JESD216B_MINOR
*/
if (bfpt_header->major == SFDP_JESD216_MAJOR &&
- bfpt_header->minor == SFDP_JESD216_MINOR)
- nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+ bfpt_header->minor == SFDP_JESD216_MINOR) {
+ nor->params->qe_mask[0] = BIT(6);
+ nor->params->qe_mask[1] = 0;
+ }
return 0;
}
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 18d9a00aa22e..04db42b5141e 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -131,7 +131,8 @@ static const struct flash_info issi_nor_parts[] = {
static void issi_nor_default_init(struct spi_nor *nor)
{
- nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+ nor->params->qe_mask[0] = BIT(6);
+ nor->params->qe_mask[1] = 0;
}
static const struct spi_nor_fixups issi_fixups = {
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index e97f5cbd9aad..55612237c1ef 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -65,11 +65,12 @@ mx25l3255e_late_init_fixups(struct spi_nor *nor)
/*
* SFDP of MX25L3255E is JESD216, which does not include the Quad
- * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
- * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
- * Therefore, it is necessary to correct this setting by late_init.
+ * Enable bit Requirement in BFPT. As a result, during BFPT parsing
+ * the quad_enable mask is reset. Therefore, it is necessary to
+ * correct this setting by late_init.
*/
- params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+ params->qe_mask[0] = BIT(6);
+ params->qe_mask[1] = 0;
/*
* In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
@@ -314,7 +315,8 @@ static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
static void macronix_nor_default_init(struct spi_nor *nor)
{
- nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+ nor->params->qe_mask[0] = BIT(6);
+ nor->params->qe_mask[1] = 0;
}
static int macronix_nor_late_init(struct spi_nor *nor)
diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
index 7d0b145d78d8..64686b904e24 100644
--- a/drivers/mtd/spi-nor/otp.c
+++ b/drivers/mtd/spi-nor/otp.c
@@ -175,24 +175,24 @@ static int spi_nor_otp_lock_bit_cr(unsigned int region)
*/
int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region)
{
- u8 *cr = nor->bouncebuf;
int ret, lock_bit;
+ u8 sr2;
lock_bit = spi_nor_otp_lock_bit_cr(region);
if (lock_bit < 0)
return lock_bit;
- ret = spi_nor_read_cr(nor, cr);
+ ret = spi_nor_read_srs(nor, NULL, &sr2);
if (ret)
return ret;
/* no need to write the register if region is already locked */
- if (cr[0] & lock_bit)
+ if (sr2 & lock_bit)
return 0;
- cr[0] |= lock_bit;
+ sr2 |= lock_bit;
- return spi_nor_write_16bit_cr_and_check(nor, cr[0]);
+ return spi_nor_write_srs(nor, NULL, &sr2);
}
/**
@@ -207,18 +207,18 @@ int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region)
*/
int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region)
{
- u8 *cr = nor->bouncebuf;
int ret, lock_bit;
+ u8 sr2;
lock_bit = spi_nor_otp_lock_bit_cr(region);
if (lock_bit < 0)
return lock_bit;
- ret = spi_nor_read_cr(nor, cr);
+ ret = spi_nor_read_srs(nor, NULL, &sr2);
if (ret)
return ret;
- return cr[0] & lock_bit;
+ return sr2 & lock_bit;
}
static loff_t spi_nor_otp_region_start(const struct spi_nor *nor, unsigned int region)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 4600983cb579..23b809cc958f 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -561,10 +561,21 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
val >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
params->page_size = 1U << val;
+ /*
+ * The standard declares various read and write status methods, some of
+ * them will be overloaded based on the QER field.
+ */
+ params->read_srs_opcode = SPINOR_OP_RDSR;
+ params->read_sr1_opcode = SPINOR_OP_RDSR;
+ params->read_sr2_opcode = SPINOR_OP_RDCR;
+ params->write_srs_opcode = SPINOR_OP_WRSR;
+ params->write_sr1_opcode = SPINOR_OP_WRSR;
+
/* Quad Enable Requirements. */
switch (bfpt.dwords[SFDP_DWORD(15)] & BFPT_DWORD15_QER_MASK) {
case BFPT_DWORD15_QER_NONE:
- params->quad_enable = NULL;
+ params->qe_mask[0] = 0;
+ params->qe_mask[1] = 0;
break;
case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
@@ -572,23 +583,33 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
* Writing only one byte to the Status Register has the
* side-effect of clearing Status Register 2.
*/
+ params->write_sr1_opcode = 0;
+ fallthrough;
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
/*
* Read Configuration Register (35h) instruction is not
* supported.
*/
- nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
- params->quad_enable = spi_nor_sr2_bit1_quad_enable;
+ params->read_sr2_opcode = 0;
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
+ params->qe_mask[0] = 0;
+ params->qe_mask[1] = BIT(1);
break;
case BFPT_DWORD15_QER_SR1_BIT6:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+ params->qe_mask[0] = BIT(6);
+ params->qe_mask[1] = 0;
break;
case BFPT_DWORD15_QER_SR2_BIT7:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- params->quad_enable = spi_nor_sr2_bit7_quad_enable;
+ params->qe_mask[0] = 0;
+ params->qe_mask[1] = BIT(7);
+ params->read_srs_opcode = 0;
+ params->read_sr2_opcode = SPINOR_OP_RDSR2;
+ params->write_srs_opcode = 0;
+ params->write_sr2_opcode = SPINOR_OP_WRSR2;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
@@ -599,8 +620,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
* assumption of a 16-bit Write Status (01h) command.
*/
nor->flags |= SNOR_F_HAS_16BIT_SR;
-
- params->quad_enable = spi_nor_sr2_bit1_quad_enable;
+ params->write_sr1_opcode = 0;
+ params->read_srs_opcode = 0;
+ params->qe_mask[0] = 0;
+ params->qe_mask[1] = BIT(1);
break;
default:
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 19a0ad145599..2689ea21074c 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -399,8 +399,9 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
nor->bouncebuf);
bool is3byte, is4byte;
int ret;
+ u8 sr;
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[1]);
+ ret = spi_nor_read_srs(nor, &sr, NULL);
if (ret)
return ret;
@@ -408,7 +409,7 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
if (ret)
return ret;
- is3byte = (nor->bouncebuf[0] == nor->bouncebuf[1]);
+ is3byte = (nor->bouncebuf[0] == sr);
op = (struct spi_mem_op)
CYPRESS_NOR_RD_ANY_REG_OP(4, SPINOR_REG_CYPRESS_STR1V, 0,
@@ -417,7 +418,7 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
if (ret)
return ret;
- is4byte = (nor->bouncebuf[0] == nor->bouncebuf[1]);
+ is4byte = (nor->bouncebuf[0] == sr);
if (is3byte == is4byte)
return -EIO;
@@ -1098,13 +1099,14 @@ static const struct flash_info spansion_nor_parts[] = {
static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
{
int ret;
+ u8 sr;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &sr, NULL);
if (ret)
return ret;
- if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
- if (nor->bouncebuf[0] & SR_E_ERR)
+ if (sr & (SR_E_ERR | SR_P_ERR)) {
+ if (sr & SR_E_ERR)
dev_err(nor->dev, "Erase Error occurred\n");
else
dev_err(nor->dev, "Programming Error occurred\n");
@@ -1124,7 +1126,7 @@ static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
return -EIO;
}
- return !(nor->bouncebuf[0] & SR_WIP);
+ return !(sr & SR_WIP);
}
static int spansion_nor_late_init(struct spi_nor *nor)
diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
index db02c14ba16f..15f8b2cbd497 100644
--- a/drivers/mtd/spi-nor/sst.c
+++ b/drivers/mtd/spi-nor/sst.c
@@ -21,16 +21,17 @@ static int sst26vf_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
static int sst26vf_nor_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
{
int ret;
+ u8 cr;
/* We only support unlocking the entire flash array. */
if (ofs != 0 || len != nor->params->size)
return -EINVAL;
- ret = spi_nor_read_cr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, NULL, &cr);
if (ret)
return ret;
- if (!(nor->bouncebuf[0] & SST26VF_CR_BPNV)) {
+ if (!(cr & SST26VF_CR_BPNV)) {
dev_dbg(nor->dev, "Any block has been permanently locked\n");
return -EINVAL;
}
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 235070b215d1..d746cb595b09 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -36,7 +36,7 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
static u8 spi_nor_get_sr_cmp_mask(struct spi_nor *nor)
{
- if (!(nor->flags & SNOR_F_NO_READ_CR) &&
+ if (nor->params->read_sr2_opcode &&
nor->flags & SNOR_F_HAS_SR2_CMP_BIT6)
return SR2_CMP_BIT6;
else
@@ -207,17 +207,9 @@ void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
if (!sr) {
- if (spi_nor_read_sr(nor, nor->bouncebuf))
+ if (spi_nor_read_srs(nor, &sr_cr[0], &sr_cr[1]))
return;
- sr_cr[0] = nor->bouncebuf[0];
-
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- if (spi_nor_read_cr(nor, nor->bouncebuf))
- return;
- }
-
- sr_cr[1] = nor->bouncebuf[0];
sr = sr_cr;
}
@@ -273,20 +265,10 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
int ret;
u8 pow;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &status_old[0], &status_old[1]);
if (ret)
return ret;
- status_old[0] = nor->bouncebuf[0];
-
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- ret = spi_nor_read_cr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- status_old[1] = nor->bouncebuf[0];
- }
-
/* If nothing in our range is unlocked, we don't need to do anything */
if (spi_nor_is_locked_sr(nor, ofs, len, status_old))
return 0;
@@ -378,10 +360,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
(ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
return -EINVAL;
- if (nor->flags & SNOR_F_NO_READ_CR)
- ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
- else
- ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
+ ret = spi_nor_write_srs(nor, &best_status_new[0], &best_status_new[1]);
if (ret)
return ret;
@@ -409,20 +388,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
int ret;
u8 pow;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &status_old[0], &status_old[1]);
if (ret)
return ret;
- status_old[0] = nor->bouncebuf[0];
-
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- ret = spi_nor_read_cr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- status_old[1] = nor->bouncebuf[0];
- }
-
/* If nothing in our range is locked, we don't need to do anything */
if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old))
return 0;
@@ -512,10 +481,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
(ofs_new < ofs_old || (ofs_old + len_old) < (ofs_new + len_new)))
return -EINVAL;
- if (nor->flags & SNOR_F_NO_READ_CR)
- ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
- else
- ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
+ ret = spi_nor_write_srs(nor, &best_status_new[0], &best_status_new[1]);
if (ret)
return ret;
@@ -536,20 +502,10 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
u8 sr_cr[2] = {};
int ret;
- ret = spi_nor_read_sr(nor, nor->bouncebuf);
+ ret = spi_nor_read_srs(nor, &sr_cr[0], &sr_cr[1]);
if (ret)
return ret;
- sr_cr[0] = nor->bouncebuf[0];
-
- if (!(nor->flags & SNOR_F_NO_READ_CR)) {
- ret = spi_nor_read_cr(nor, nor->bouncebuf);
- if (ret)
- return ret;
-
- sr_cr[1] = nor->bouncebuf[0];
- }
-
return spi_nor_is_locked_sr(nor, ofs, len, sr_cr);
}
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 60a7bb1f2ce4..f96a11531a09 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -481,13 +481,13 @@ static int winbond_nor_late_init(struct spi_nor *nor)
* been to declare CR reads as unsupported, whereas the Jedec
* specification doesn't clearly state that. In practice, all these
* chips do support reading back the CR, which is needed for SWP support,
- * so make sure that capability remains enabled (needed for SWP).
+ * so make sure that capability remains enabled.
* In practice, only exclude the old W25X family (JEDEC ID: EF 30 xx)
* which actually does not support this feature.
*/
if (nor->info->id->bytes[0] == 0xef &&
nor->info->id->bytes[1] > 0x30)
- nor->flags &= ~SNOR_F_NO_READ_CR;
+ params->read_sr2_opcode = SPINOR_OP_RDCR;
return 0;
}
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 4b92494827b1..f0b5a1989881 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -113,20 +113,16 @@
#define SR_E_ERR BIT(5)
#define SR_P_ERR BIT(6)
-#define SR1_QUAD_EN_BIT6 BIT(6)
-
#define SR_BP_SHIFT 2
/* Enhanced Volatile Configuration Register bits */
#define EVCR_QUAD_EN_MICRON BIT(7) /* Micron Quad I/O */
/* Status Register 2 bits. */
-#define SR2_QUAD_EN_BIT1 BIT(1)
#define SR2_LB1 BIT(3) /* Security Register Lock Bit 1 */
#define SR2_LB2 BIT(4) /* Security Register Lock Bit 2 */
#define SR2_LB3 BIT(5) /* Security Register Lock Bit 3 */
#define SR2_CMP_BIT6 BIT(6)
-#define SR2_QUAD_EN_BIT7 BIT(7)
/* Supported SPI protocols */
#define SNOR_PROTO_INST_MASK GENMASK(23, 16)
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] mtd: spi-nor: Add support for the new JESD216 rev F QER field
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
2026-05-29 16:05 ` [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support Miquel Raynal
@ 2026-05-29 16:05 ` Miquel Raynal
2026-05-29 16:05 ` [PATCH 3/5] mtd: spi-nor: Move the SFDP header structure to a C header Miquel Raynal
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
Revision F of the JEDEC JESD216 standard includes yet another
supported setting in the QER 3-bit bitfield: 110b.
This setting implies that the QE bit is bit 1 of SR2 (which is an
already supported case) but this time it makes it very clear that both
reading and writing to SR2 is supported, but not with the usual
opcodes. The write operation uses an alternate WRSR2 opcode:
0x31 (instead of 0x3E) and both the reads and writes manipulate a single
register at a time.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/sfdp.c | 9 +++++++++
drivers/mtd/spi-nor/sfdp.h | 8 ++++++++
include/linux/mtd/spi-nor.h | 1 +
3 files changed, 18 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 23b809cc958f..04580f78b62b 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -626,6 +626,15 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
params->qe_mask[1] = BIT(1);
break;
+ case BFPT_DWORD15_QER_SR2_BIT1_1B:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
+ params->qe_mask[0] = 0;
+ params->qe_mask[1] = BIT(1);
+ params->read_srs_opcode = 0;
+ params->write_srs_opcode = 0;
+ params->write_sr2_opcode = SPINOR_OP_WRSR2_ALT;
+ break;
+
default:
dev_dbg(nor->dev, "BFPT QER reserved value used\n");
break;
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index f74a0eb339ea..7e52e00cbae0 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -12,6 +12,7 @@
#define SFDP_JESD216_MINOR 0
#define SFDP_JESD216A_MINOR 5
#define SFDP_JESD216B_MINOR 6
+#define SFDP_JESD216F_MINOR 10
/* SFDP DWORDS are indexed from 1 but C arrays are indexed from 0. */
#define SFDP_DWORD(i) ((i) - 1)
@@ -81,6 +82,12 @@ struct sfdp_bfpt {
* instruction 35h. QE is set via Write Status instruction 01h with
* two data bytes where bit 1 of the second byte is one.
* [...]
+ * (from JESD216 rev F)
+ * - 110b: QE is bit 1 of status register 2. Status register 1 is read using
+ * Read Status instruction 05h. Status register2 is read using
+ * instruction 35h, and status register 3 is read using instruction 15h.
+ * QE is set via Write Status Register instruction 31h with one data
+ * byte.
*/
#define BFPT_DWORD15_QER_MASK GENMASK(22, 20)
#define BFPT_DWORD15_QER_NONE (0x0UL << 20) /* Micron */
@@ -89,6 +96,7 @@ struct sfdp_bfpt {
#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
+#define BFPT_DWORD15_QER_SR2_BIT1_1B (0x6UL << 20) /* Winbond */
#define BFPT_DWORD16_EN4B_MASK GENMASK(31, 24)
#define BFPT_DWORD16_EN4B_ALWAYS_4B BIT(30)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f0b5a1989881..3a55d76ea77d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -25,6 +25,7 @@
#define SPINOR_OP_WRSR 0x01 /* Write status register 1 */
#define SPINOR_OP_RDSR2 0x3f /* Read status register 2 */
#define SPINOR_OP_WRSR2 0x3e /* Write status register 2 */
+#define SPINOR_OP_WRSR2_ALT 0x31 /* Write status register 2 (alternative) */
#define SPINOR_OP_READ 0x03 /* Read data bytes (low frequency) */
#define SPINOR_OP_READ_FAST 0x0b /* Read data bytes (high frequency) */
#define SPINOR_OP_READ_1_1_2 0x3b /* Read data bytes (Dual Output SPI) */
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] mtd: spi-nor: Move the SFDP header structure to a C header
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
2026-05-29 16:05 ` [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support Miquel Raynal
2026-05-29 16:05 ` [PATCH 2/5] mtd: spi-nor: Add support for the new JESD216 rev F QER field Miquel Raynal
@ 2026-05-29 16:05 ` Miquel Raynal
2026-05-29 16:05 ` [PATCH 4/5] mtd: spi-nor: winbond: Add support for W25Q02RV-M Miquel Raynal
2026-05-29 16:05 ` [PATCH 5/5] mtd: spi-nor: winbond: Add support for W25Q51RV-M Miquel Raynal
4 siblings, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
The sfdp_header structure is currently defined in sfdp.c, whereas the
sfdp_parameter_header structure is defined in sfdp.h. In order to ease
the reuse of this structure outside of the sfdp.c file, move this
structure to the sfdp C header (.h).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/spi-nor/sfdp.c | 11 -----------
drivers/mtd/spi-nor/sfdp.h | 11 +++++++++++
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 04580f78b62b..902506e8015e 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -34,17 +34,6 @@
#define SFDP_SIGNATURE 0x50444653U
-struct sfdp_header {
- u32 signature; /* Ox50444653U <=> "SFDP" */
- u8 minor;
- u8 major;
- u8 nph; /* 0-base number of parameter headers */
- u8 unused;
-
- /* Basic Flash Parameter Table. */
- struct sfdp_parameter_header bfpt_header;
-};
-
/* Fast Read settings. */
struct sfdp_bfpt_read {
/* The Fast Read x-y-z hardware capability in params->hwcaps.mask. */
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index 7e52e00cbae0..1e57ace6d83f 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -149,4 +149,15 @@ struct sfdp_parameter_header {
u8 id_msb;
};
+struct sfdp_header {
+ u32 signature; /* Ox50444653U <=> "SFDP" */
+ u8 minor;
+ u8 major;
+ u8 nph; /* 0-base number of parameter headers */
+ u8 unused;
+
+ /* Basic Flash Parameter Table. */
+ struct sfdp_parameter_header bfpt_header;
+};
+
#endif /* __LINUX_MTD_SFDP_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] mtd: spi-nor: winbond: Add support for W25Q02RV-M
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
` (2 preceding siblings ...)
2026-05-29 16:05 ` [PATCH 3/5] mtd: spi-nor: Move the SFDP header structure to a C header Miquel Raynal
@ 2026-05-29 16:05 ` Miquel Raynal
2026-05-29 16:05 ` [PATCH 5/5] mtd: spi-nor: winbond: Add support for W25Q51RV-M Miquel Raynal
4 siblings, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
There is an ID collision with the chip of same density from the JV
family. Both chips are very similar in practice, it is mostly a matter
of electrical differences (mostly power consumption being lower).
As a significant difference, RV chips identify themselves as supporting
the new SFDP (rev F) field which forces an alternate write SR2
opcode (0x31).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
+ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
ef7022
+ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
winbond
+ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
534644500a0101ff00080117800000ff84010102e00000ffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffe520fbffffffff7f44eb086b083b42bbfeffffffffff
0000ffff44eb0c200f5210d800001602a60081e214dde9e378477a757a75
f7a4d55c39f66dffe970f9a50000000000002c0c000000006655ffff0f00
0000060d44bd27ed080dffffffffff8a00fe21ffdcff
+ sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
fe929abb1ba38dd6a8668200051298a8140bfeb56bd6d58f56b86b074a150f12 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
+ cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
Supported read modes by the flash
1S-1S-1S
opcode 0x13
mode cycles 0
dummy cycles 0
1S-1S-2S
opcode 0x3c
mode cycles 0
dummy cycles 8
1S-2S-2S
opcode 0xbc
mode cycles 2
dummy cycles 2
1S-1S-4S
opcode 0x6c
mode cycles 0
dummy cycles 8
1S-4S-4S
opcode 0xec
mode cycles 2
dummy cycles 4
4S-4S-4S
opcode 0xec
mode cycles 2
dummy cycles 4
Supported page program modes by the flash
1S-1S-1S
opcode 0x12
1S-1S-4S
opcode 0x34
+ cat /sys/kernel/debug/spi-nor/spi0.0/params
name (null)
id ef 70 22 00 00 00
size 256 MiB
write size 1
page size 256
address nbytes 4
flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_SR_TB_BIT6 | HAS_4BIT_BP | SOFT_RESET | NO_WP | HAS_SR2_CMP_BIT6
opcodes
read 0xec
dummy cycles 6
erase 0xdc
program 0x34
8D extension repeat
protocols
read 1S-4S-4S
write 1S-1S-4S
register 1S-1S-1S
erase commands
21 (4.00 KiB) [1]
dc (64.0 KiB) [3]
c7 (256 MiB)
sector map
region (in hex) | erase mask | overlaid
------------------+------------+---------
00000000-0fffffff | [ 3] | no
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0fffffff | unlocked | 4096
+ dd 'if=/dev/urandom' 'of=./spi_test' 'bs=1M' 'count=2'
[ 11.527038] random: crng init done
2+0 records in
2+0 records out
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ hexdump spi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0200000
+ sha256sum spi_read
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5 spi_read
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_read
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_test
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5 spi_read
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_test
+ mtd_debug info /dev/mtd0
mtd.type = MTD_NORFLASH
mtd.flags = MTD_CAP_NORFLASH
mtd.size = 268435456 (256M)
mtd.erasesize = 65536 (64K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0
+ alias 'show_sectors=grep -A4 "locked sectors" /sys/kernel/debug/spi-nor/spi0.0/params'
+ flash_lock -u /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x10000000
Lock status: unlocked
Return code: 0
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_read
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_test
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0fffffff | unlocked | 4096
+ flash_lock -l /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x10000000
Lock status: locked
Return code: 1
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_read
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_test
+ dd 'if=/dev/urandom' 'of=./spi_test2' 'bs=1M' 'count=2'
2+0 records in
2+0 records out
+ mtd_debug write /dev/mtd0 0 2097152 spi_test2
Copied 2097152 bytes from spi_test2 to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read2
Copied 2097152 bytes from address 0x00000000 in flash to spi_read2
+ sha256sum spi_read spi_read2 spi_test spi_test2
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_read
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_read2
32e1a8def4cf674413ff0b7a2738e704f03f7bddcaf7ee2d320ab83aa7bf813e spi_test
4b1329df073cd497567d8cb6878aa20af6af71df98cc7c445d31a69eaf0d69f1 spi_test2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0fffffff | locked | 4096
+ flash_lock -u /dev/mtd0
+ cat /sys/class/mtd/mtd0/size
+ size=268435456
+ cat /sys/class/mtd/mtd0/erasesize
+ bs=65536
+ grep unlocked /sys/kernel/debug/spi-nor/spi0.0/params
+ sed -e 's/.*unlocked | //'
+ nsectors=4096
+ ss=65536
+ bps=1
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 268304384 2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0ffdffff | unlocked | 4094
0ffe0000-0fffffff | locked | 2
+ flash_lock -u /dev/mtd0 268304384 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0ffeffff | unlocked | 4095
0fff0000-0fffffff | locked | 1
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 260046848 128
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0f7fffff | unlocked | 3968
0f800000-0fffffff | locked | 128
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 0 2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0001ffff | locked | 2
00020000-0fffffff | unlocked | 4094
+ flash_lock -u /dev/mtd0 65536 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0000ffff | locked | 1
00010000-0fffffff | unlocked | 4095
+ all_but_one=4095
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 65536 4095
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0000ffff | unlocked | 1
00010000-0fffffff | locked | 4095
+ flash_lock -u /dev/mtd0 65536 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0001ffff | unlocked | 2
00020000-0fffffff | locked | 4094
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 0 4095
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0ffeffff | locked | 4095
0fff0000-0fffffff | unlocked | 1
+ flash_lock -u /dev/mtd0 268304384 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0ffdffff | locked | 4094
0ffe0000-0fffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 53 +++++++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index f96a11531a09..04ba8fd44a6a 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -128,7 +128,9 @@ static int winbond_nor_multi_die_ready(struct spi_nor *nor)
}
static int
-winbond_nor_multi_die_post_sfdp_fixups(struct spi_nor *nor)
+winbond_nor_multi_die_post_sfdp_fixups(struct spi_nor *nor,
+ const struct sfdp_parameter_header *bfpt_header,
+ const struct sfdp_bfpt *bfpt)
{
/*
* SFDP supports dice numbers, but this information is only available in
@@ -142,8 +144,45 @@ winbond_nor_multi_die_post_sfdp_fixups(struct spi_nor *nor)
return 0;
}
-static const struct spi_nor_fixups winbond_nor_multi_die_fixups = {
- .post_sfdp = winbond_nor_multi_die_post_sfdp_fixups,
+static bool is_w25qxxrv(struct spi_nor *nor)
+{
+ struct sfdp_header *sfdp_h = (struct sfdp_header *)nor->sfdp->dwords;
+
+ /*
+ * W25QxxRV chips re-use the same ID as the W25QxxJV family.
+ *
+ * Chips are very similar, W25QxxRV brings mostly performance and power
+ * consumption improvements. The RV family does not require the multi
+ * die fixup.
+ *
+ * They can be distinguished based on their SFDP minor revision:
+ * W25QxxJV: JESD216A, minor revision == 05h
+ * W25Q512/01/02JV: JESD216B, minor revision == 06h
+ * W25QxxRV: JESD216F, minor revision >= 0Ah
+ */
+ return sfdp_h->minor >= SFDP_JESD216F_MINOR;
+}
+
+static int w25qxxjv_rv_post_bfpt_fixups(struct spi_nor *nor,
+ const struct sfdp_parameter_header *bfpt_header,
+ const struct sfdp_bfpt *bfpt)
+{
+ int ret;
+
+ if (!is_w25qxxrv(nor)) {
+ /* Only 1Gb and 2Gb variants are affected */
+ if (nor->params->size == SZ_128M || nor->params->size == SZ_256M) {
+ ret = winbond_nor_multi_die_post_sfdp_fixups(nor, bfpt_header, bfpt);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct spi_nor_fixups w25qxxjv_rv_fixups = {
+ .post_bfpt = w25qxxjv_rv_post_bfpt_fixups,
};
static const struct flash_info winbond_nor_parts[] = {
@@ -230,7 +269,7 @@ static const struct flash_info winbond_nor_parts[] = {
.id = SNOR_ID(0xef, 0x40, 0x21),
.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
- .fixups = &winbond_nor_multi_die_fixups,
+ .fixups = &w25qxxjv_rv_fixups,
}, {
.id = SNOR_ID(0xef, 0x50, 0x12),
.name = "w25q20bw",
@@ -317,13 +356,13 @@ static const struct flash_info winbond_nor_parts[] = {
.id = SNOR_ID(0xef, 0x70, 0x21),
.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
- .fixups = &winbond_nor_multi_die_fixups,
+ .fixups = &w25qxxjv_rv_fixups,
}, {
- /* W25Q02JV-M */
+ /* W25Q02JV-M, W25Q02RV-M */
.id = SNOR_ID(0xef, 0x70, 0x22),
.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
- .fixups = &winbond_nor_multi_die_fixups,
+ .fixups = &w25qxxjv_rv_fixups,
}, {
.id = SNOR_ID(0xef, 0x71, 0x19),
.name = "w25m512jv",
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] mtd: spi-nor: winbond: Add support for W25Q51RV-M
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
` (3 preceding siblings ...)
2026-05-29 16:05 ` [PATCH 4/5] mtd: spi-nor: winbond: Add support for W25Q02RV-M Miquel Raynal
@ 2026-05-29 16:05 ` Miquel Raynal
4 siblings, 0 replies; 7+ messages in thread
From: Miquel Raynal @ 2026-05-29 16:05 UTC (permalink / raw)
To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel, Miquel Raynal
There is an ID collision with the chip of same density from the JV
family. Both chips are very similar in practice, it is mostly a matter
of electrical differences (mostly power consumption being lower).
As a significant difference, RV chips identify themselves as supporting
the new SFDP (rev F) field which forces an alternate write SR2
opcode (0x31).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
+ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
ef4020
+ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
winbond
+ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
534644500a0101ff00080117800000ff84010102e00000ffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffe520f3ffffffff1f44eb086b083b42bbfeffffffffff
0000ffff44eb0c200f5210d800001602a60081e214dde96376337a757a75
f7a4d55c39f66dffe970f9a50000000000002c0c0000000066ffffff0000
00000000000000000000ffffffffff0a00fe21ffdcff
+ sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
74d21fecc7faa297b3f77f1136b1dc53f78b2a432629e226675b6384153fd9f9 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
+ cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
Supported read modes by the flash
1S-1S-1S
opcode 0x13
mode cycles 0
dummy cycles 0
1S-1S-2S
opcode 0x3c
mode cycles 0
dummy cycles 8
1S-2S-2S
opcode 0xbc
mode cycles 2
dummy cycles 2
1S-1S-4S
opcode 0x6c
mode cycles 0
dummy cycles 8
1S-4S-4S
opcode 0xec
mode cycles 2
dummy cycles 4
4S-4S-4S
opcode 0xec
mode cycles 2
dummy cycles 4
Supported page program modes by the flash
1S-1S-1S
opcode 0x12
1S-1S-4S
opcode 0x34
+ cat /sys/kernel/debug/spi-nor/spi0.0/params
name (null)
id ef 40 20 00 00 00
size 64.0 MiB
write size 1
page size 256
address nbytes 4
flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_SR_TB_BIT6 | HAS_4BIT_BP | SOFT_RESET | NO_WP | HAS_SR2_CMP_BIT6
opcodes
read 0xec
dummy cycles 6
erase 0xdc
program 0x34
8D extension repeat
protocols
read 1S-4S-4S
write 1S-1S-4S
register 1S-1S-1S
erase commands
21 (4.00 KiB) [1]
dc (64.0 KiB) [3]
c7 (64.0 MiB)
sector map
region (in hex) | erase mask | overlaid
------------------+------------+---------
00000000-03ffffff | [ 3] | no
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03ffffff | unlocked | 1024
+ dd 'if=/dev/urandom' 'of=./spi_test' 'bs=1M' 'count=2'
[ 25.427347] random: crng init done
2+0 records in
2+0 records out
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ hexdump spi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0200000
+ sha256sum spi_read
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5 spi_read
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_read
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_test
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5 spi_read
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_test
+ mtd_debug info /dev/mtd0
mtd.type = MTD_NORFLASH
mtd.flags = MTD_CAP_NORFLASH
mtd.size = 67108864 (64M)
mtd.erasesize = 65536 (64K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0
+ alias 'show_sectors=grep -A4 "locked sectors" /sys/kernel/debug/spi-nor/spi0.0/params'
+ flash_lock -u /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: unlocked
Return code: 0
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_read
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_test
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03ffffff | unlocked | 1024
+ flash_lock -l /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: locked
Return code: 1
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_read
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_test
+ dd 'if=/dev/urandom' 'of=./spi_test2' 'bs=1M' 'count=2'
2+0 records in
2+0 records out
+ mtd_debug write /dev/mtd0 0 2097152 spi_test2
Copied 2097152 bytes from spi_test2 to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read2
Copied 2097152 bytes from address 0x00000000 in flash to spi_read2
+ sha256sum spi_read spi_read2 spi_test spi_test2
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_read
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_read2
cd699acc82353b7769b98547efed68d5300854ed0bbc84bf831b07937bf941a5 spi_test
f506020804f91fb750e8250e091fb2f732319221a645ed575e1b6ed3553d584c spi_test2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03ffffff | locked | 1024
+ flash_lock -u /dev/mtd0
+ cat /sys/class/mtd/mtd0/size
+ size=67108864
+ cat /sys/class/mtd/mtd0/erasesize
+ bs=65536
+ grep unlocked /sys/kernel/debug/spi-nor/spi0.0/params
+ sed -e 's/.*unlocked | //'
+ nsectors=1024
+ ss=65536
+ bps=1
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 66977792 2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03fdffff | unlocked | 1022
03fe0000-03ffffff | locked | 2
+ flash_lock -u /dev/mtd0 66977792 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03feffff | unlocked | 1023
03ff0000-03ffffff | locked | 1
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 58720256 128
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-037fffff | unlocked | 896
03800000-03ffffff | locked | 128
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 0 2
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0001ffff | locked | 2
00020000-03ffffff | unlocked | 1022
+ flash_lock -u /dev/mtd0 65536 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0000ffff | locked | 1
00010000-03ffffff | unlocked | 1023
+ all_but_one=1023
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 65536 1023
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0000ffff | unlocked | 1
00010000-03ffffff | locked | 1023
+ flash_lock -u /dev/mtd0 65536 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-0001ffff | unlocked | 2
00020000-03ffffff | locked | 1022
+ flash_lock -u /dev/mtd0
+ flash_lock -l /dev/mtd0 0 1023
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03feffff | locked | 1023
03ff0000-03ffffff | unlocked | 1
+ flash_lock -u /dev/mtd0 66977792 1
+ grep -A4 'locked sectors' /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03fdffff | locked | 1022
03fe0000-03ffffff | unlocked | 2
---
drivers/mtd/spi-nor/winbond.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 04ba8fd44a6a..689e5822b009 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -260,7 +260,7 @@ static const struct flash_info winbond_nor_parts[] = {
SPI_NOR_TB_SR_BIT6 | SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
.fixups = &w25q256_fixups,
}, {
- /* W25Q512JV-Q/N */
+ /* W25Q512JV-Q/N, W25Q51RV-Q/N */
.id = SNOR_ID(0xef, 0x40, 0x20),
.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
SPI_NOR_4BIT_BP | SPI_NOR_HAS_CMP,
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support
2026-05-29 16:05 ` [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support Miquel Raynal
@ 2026-07-07 9:43 ` Michael Walle
0 siblings, 0 replies; 7+ messages in thread
From: Michael Walle @ 2026-07-07 9:43 UTC (permalink / raw)
To: Miquel Raynal, Pratyush Yadav, Takahiro Kuwano,
Richard Weinberger, Vignesh Raghavendra, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Steam Lin, Hsin-Yi Wang, Thomas Petazzoni, linux-mtd,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 59259 bytes --]
Hi Miquel,
thanks for doing this (somewhat thankless) task. I briefly skimmed
over this patch.. [Ok coming back here, it's quite a bigger
feedback. And I'm sorry if it's sometime confusing, I was jumping
back and forth].
On Fri May 29, 2026 at 6:05 PM CEST, Miquel Raynal wrote:
> SPI NOR has a lot of history. Additions over additions, the subtleties
> of the JESD216 specification, their implementations by the manufacturers
> and the hardware mistakes have generated a gigantic maze, let's try to
> understand what is really needed.
>
> The specification explains the QE (Quad Enable) bitfield as describing
> where the bit to enable the Quad capability is, but also how to set
> it.
I think the standard just wrote down what all the different vendors
were doing. So it didn't really start as "one" standard but just
collected all the different variants from all the vendors. Also, it
seems that the standard is only interested in how to enable the
quad IO mode. Everything else is guesswork. But iff a given vendor
stick to its implementation, chances are that the QER will
correspond to how to read and write status registers. So for our
guesswork, we can use a QER-to-"sr access" mapping as a baseline.
We should map the QER enumeration to vendors to get a better
understanding now and in the future.
Also, I haven't found a vendor which is using the 3Eh/3Fh opcodes?!
> Unfortunately, the specification is not precise about what opcodes
> are supported exactly in all the cases. There is an introduction that
> basically states:
> - Opcode 0x05 reads SR1, and then SR2 if another byte is read
Where is that? It only states that 05 reads sr1, no?
> - Opcode 0x35 allows to read SR2
> - Opcode 0x01 writes SR1, and then SR2 if another byte is written
Ack.
But reading the individual QER methods below, I'm not sure, if that
is really the baseline here. I mean 101b corresponds to exactly
that, no?
> Then the bitfield, among indicating the location of the QE bit, may
> indicate:
> - Reading SR1 and SR2 in one operation is not possible (loops over the
> content of SR1)
> - Reading SR2 directly is possible
> - Writing SR1 smashes SR2
You mean writing just one byte with 01h, will set sr2 to zero. IOW,
you'll have to always write two bytes if using 01h.
What we also don't know is how many status registers there actually
are.
> One problem with the current implementation, is that it only focuses on
> the QE bit. A quad_enable function was created for each case, even
> though in practice, the logic was always the same: read, modify, write,
> read back and verify. One problem comes when other features need to play
> with the status registers, like software block protection or OTP: you
> never know how to properly handle the QE bit, nor where it is, nor how
> to read/write the Status registers. This lead to
> approximations/guessing (in swp.c, otp.c and obviously in legacy
> controller drivers like atmel.c) but also to the implementation of a
> gazillon of helpers for reading/writing/checking the status registers.
Mh? why do we have to know the QE bit in other parts? We shall just
read-modify-write the status registers, no?
> In addition, I believe some design decisions had a negative impact over
> the years.
>
> - All possible situations had to be flagged by the core. This is likely
> wrong, because we no longer know why we need specific quirks. It was
> ineherent to the state of the SPI NOR core before the great cleanup
> that had happened the past few years. I believe this creates confusion
> in the core today, and we should push this to vendor fixups
> instead. As an example, in 2023 Hsin-Yi was facing an issue because
> his chip was setting the wrong QER value, leading to RDCR being
> prevented, thus falling into a condition blindly setting a random QE
> bit (I strongy believe it is done like that for wrong reasons). His
> chip actually had RDCR support! The correct fix should have been to
> mark the capability in a device fixup instead of handling this in the
> core.
>
> Link: https://lore.kernel.org/lkml/CAJMQK-hR0eaO0b4Vd0U8_KAndLyZapqdHjVLAoe42rWi9rdLkA@mail.gmail.com/
You mean the "always set the QER bit"?
I agree, this should be pushed into the vendor fixups. But what I
really want to avoid is that we have to have a never ending list of
IDs for these fixups. So there shall be a generic vendor fixup.
As a side node, I really don't like how the fixups are handled at
the moment. I've proposed that way back but it would be awesome to
be able to just register a fixup with
register_spi_nor_fixup(SNOR_ID(0xef, 0x40, 0x16), winbond_w25q32_fixup);
register_spi_nor_fixup(SNOR_ID(0xef), winbond_default_properties);
Maybe you have some thoughts on that. I'm not sure we really need
the distinction between manufacturer fixups and per device fixups.
Now obviously, with our id reuse problem, that above isn't enough.
I've proposed to have a match function back that - by default - only
matches the id - but can be overwritten to match anything else. I
have also thought about just doing a checksum over the SFDP and
match that.
Any thoughts on that?
But I digress..
> - SFDP parsing is over cautious. I believe
> BFPT_DWORD15_QER_SR2_BIT1_NO_RD is abusive (nothing states that RDCR is
> not supported), and BFPT_DWORD15_QER_SR2_BIT1 is also out of
> specification when forcing 16-bit Status writes.
Also, I don't think BFPT_DWORD15_QER_SR2_BIT1_BUGGY is a good name
:)
> - SNOR_F_HAS_16BIT_SR is only imposing 16bit Status writes, whereas
> reads can still be 8-bit wides and even sometimes can only be 8-bit
> wide.
>
> - The usage of helpers verifying the writes was also spread for IMHO no
> really good reason. Why shouldn't we trust spi operations when it
> comes to Status Registers? We do not read back our page reads, so why
> status registers should be treated with so much care, if it's not
> because we are unsure of what is being done? My proposal includes a
> check when it comes to the QE bit (done once) but we don't need these
> checks otherwise. If the QE bit was written properly, there are high
> chances that the other register accesses will just be fine, no?
One thing that comes to mind is hardware write protection. If
there's nothing before that code which checks it, the verify might
fail if the hardware write protection is enabled. So we should
somehow check for that and drop the verify here.
> Asde from my main quest, I also observed no good reason to ask the
> read/write status register callers to use nor->bouncebuf while the
> low-level helpers could do it themselves (we are talking about one or
> two bytes being copied).
Totally agree!
> So after these observations, my proposal is the following:
> - Create private low level helpers that just read or write a status
> register. They are flexible, we can give the opcode (which varies
> based on the SFDP QER field) and the length (1 or 2).
> - Create public generic accessors which will be used to read/write sr1
> and/or sr2. This is where all the cleverness shall be. The helpers use
> the available opcodes for a given chip in order to fullfill the
> request.
> - Provide a single generic ->quad_enable() hook which generically does
> all the steps mentioned above (read, modify, write, read back and
> verify).
> - Create a list of opcodes for all 6 possible situations:
> {read, write} {sr1, sr2, sr1 and sr2}. These opcodes are
> filled/cleared based on the QER field. An opcode set to 0 indicates
> the absence of support (there is no 0x00 opcode in SPI NOR).
How would we fill these fields for flashes without SFDP? I guess
by vendor in the vendor modules.
> I tried my best to analyze the current behavior and to mimic it as much
> as possible, but this is a risky cleanup. However, if we go for this, it
> will be *much* easier in the future to handle all kind of chip
> variations. We won't be limited to a couple of flags anymore, but rather
> we'll be able to just disable a read or write capability using a single
> line.
I'd op for taking this early in the cycle for at least two cycles.
I.e. just let it sit in next and don't include it in the next PR,
but one after that.
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> This is a really hard to review patch, it is big, full of changes
> everywhere, but I cannot really split it further. The commit log is
> really important, it tries to explain the steps I went through when
> trying to refactor the whole QE/SR handling for which I received
> feedback from Michael, Tudor and Sashiko.
>
> Ideally we could expect test coverage of each case:
> - BFPT_DWORD15_QER_NONE: Probably not needed
> - BFPT_DWORD15_QER_SR2_BIT1_BUGGY: Expected from Hsin-Yi
> - BFPT_DWORD15_QER_SR2_BIT1_NO_RD: Tested with a chip re-enabling RDCR
> in a fixup
> - BFPT_DWORD15_QER_SR1_BIT6: TBD
> - BFPT_DWORD15_QER_SR2_BIT7: TBD
> - BFPT_DWORD15_QER_SR2_BIT1: TBD
> - BFPT_DWORD15_QER_SR2_BIT1_1B: Done.
> There will probably be breakages on older chips. These cannot be
> guessed because they are not properly listed in manufacturer
> fixups (yet). If we want a cleanup/simplification, we will have to cope
> with this risk.
>
> I hope the diff stats will motivate people to have a look and report
> their testing.
>
> I will also eagerly monitor Sashiko's output which will probably be
> very useful to catch niche weird cases where these changes might break.
> ---
> drivers/mtd/spi-nor/atmel.c | 50 +--
> drivers/mtd/spi-nor/core.c | 702 +++++++++++----------------------------
> drivers/mtd/spi-nor/core.h | 54 ++-
> drivers/mtd/spi-nor/debugfs.c | 1 -
> drivers/mtd/spi-nor/gigadevice.c | 6 +-
> drivers/mtd/spi-nor/issi.c | 3 +-
> drivers/mtd/spi-nor/macronix.c | 12 +-
> drivers/mtd/spi-nor/otp.c | 16 +-
> drivers/mtd/spi-nor/sfdp.c | 37 ++-
> drivers/mtd/spi-nor/spansion.c | 16 +-
> drivers/mtd/spi-nor/sst.c | 5 +-
> drivers/mtd/spi-nor/swp.c | 58 +---
> drivers/mtd/spi-nor/winbond.c | 4 +-
> include/linux/mtd/spi-nor.h | 4 -
> 14 files changed, 326 insertions(+), 642 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
> index 82c592f0a1e1..124ceb795f56 100644
> --- a/drivers/mtd/spi-nor/atmel.c
> +++ b/drivers/mtd/spi-nor/atmel.c
> @@ -23,18 +23,28 @@ static int at25fs_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>
> static int at25fs_nor_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
> {
> + /* Write 0x00 to the status register to disable write protection */
> + u8 sr = 0;
> int ret;
>
> /* We only support unlocking the whole flash array */
> if (ofs || len != nor->params->size)
> return -EINVAL;
>
> - /* Write 0x00 to the status register to disable write protection */
> - ret = spi_nor_write_sr_and_check(nor, 0);
> + ret = spi_nor_write_srs(nor, &sr, NULL);
> if (ret)
> - dev_dbg(nor->dev, "unable to clear BP bits, WP# asserted?\n");
> + return ret;
>
> - return ret;
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> + if (ret)
> + return ret;
> +
> + if (sr) {
> + dev_dbg(nor->dev, "unable to clear BP bits, WP# asserted?\n");
> + return -EIO;
> + }
> +
> + return 0;
> }
>
> static int at25fs_nor_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
> @@ -69,6 +79,7 @@ static const struct spi_nor_fixups at25fs_nor_fixups = {
> * Return: 0 on success, -error otherwise.
> */
> static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
> +
> u64 len, bool is_protect)
> {
> int ret;
> @@ -78,19 +89,24 @@ static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
> if (ofs || len != nor->params->size)
> return -EINVAL;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> if (ret)
> return ret;
>
> - sr = nor->bouncebuf[0];
> -
> /* SRWD bit needs to be cleared, otherwise the protection doesn't change */
> if (sr & SR_SRWD) {
> sr &= ~SR_SRWD;
> - ret = spi_nor_write_sr_and_check(nor, sr);
> - if (ret) {
> - dev_dbg(nor->dev, "unable to clear SRWD bit, WP# asserted?\n");
> + ret = spi_nor_write_srs(nor, &sr, NULL);
> + if (ret)
> return ret;
> +
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> + if (ret)
> + return ret;
> +
> + if (sr & SR_SRWD) {
> + dev_dbg(nor->dev, "unable to clear SRWD bit, WP# asserted?\n");
> + return -EIO;
> }
> }
>
> @@ -108,14 +124,7 @@ static int atmel_nor_set_global_protection(struct spi_nor *nor, loff_t ofs,
> sr &= ~ATMEL_SR_GLOBAL_PROTECT_MASK;
> }
>
> - nor->bouncebuf[0] = sr;
> -
> - /*
> - * We cannot use the spi_nor_write_sr_and_check() because this command
> - * isn't really setting any bits, instead it is an pseudo command for
> - * "Global Unprotect" or "Global Protect"
> - */
> - return spi_nor_write_sr(nor, nor->bouncebuf, 1);
> + return spi_nor_write_srs(nor, &sr, NULL);
> }
>
> static int atmel_nor_global_protect(struct spi_nor *nor, loff_t ofs, u64 len)
> @@ -131,16 +140,17 @@ static int atmel_nor_global_unprotect(struct spi_nor *nor, loff_t ofs, u64 len)
> static int atmel_nor_is_global_protected(struct spi_nor *nor, loff_t ofs,
> u64 len)
> {
> + u8 sr;
> int ret;
>
> if (ofs >= nor->params->size || (ofs + len) > nor->params->size)
> return -EINVAL;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> if (ret)
> return ret;
>
> - return ((nor->bouncebuf[0] & ATMEL_SR_GLOBAL_PROTECT_MASK) == ATMEL_SR_GLOBAL_PROTECT_MASK);
> + return ((sr & ATMEL_SR_GLOBAL_PROTECT_MASK) == ATMEL_SR_GLOBAL_PROTECT_MASK);
> }
>
> static const struct spi_nor_locking_ops atmel_nor_global_protection_ops = {
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index b25d1a870a22..ca66661a7f44 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -443,75 +443,6 @@ int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> return ret;
> }
>
> -/**
> - * spi_nor_read_sr() - Read the Status Register.
> - * @nor: pointer to 'struct spi_nor'.
> - * @sr: pointer to a DMA-able buffer where the value of the
> - * Status Register will be written. Should be at least 2 bytes.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = SPI_NOR_RDSR_OP(sr);
> -
> - if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
> - op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> - op.dummy.nbytes = nor->params->rdsr_dummy;
> - /*
> - * We don't want to read only one byte in DTR mode. So,
> - * read 2 and then discard the second byte.
> - */
> - op.data.nbytes = 2;
> - }
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDSR, sr,
> - 1);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d reading SR\n", ret);
> -
> - return ret;
> -}
> -
> -/**
> - * spi_nor_read_cr() - Read the Configuration Register using the
> - * SPINOR_OP_RDCR (35h) command.
> - * @nor: pointer to 'struct spi_nor'
> - * @cr: pointer to a DMA-able buffer where the value of the
> - * Configuration Register will be written.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = SPI_NOR_RDCR_OP(cr);
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDCR, cr,
> - 1);
> - }
> -
> - if (ret)
> - dev_dbg(nor->dev, "error %d reading CR\n", ret);
> -
> - return ret;
> -}
> -
> /**
> * spi_nor_set_4byte_addr_mode_en4b_ex4b() - Enter/Exit 4-byte address mode
> * using SPINOR_OP_EN4B/SPINOR_OP_EX4B. Typically used by
> @@ -617,12 +548,13 @@ int spi_nor_set_4byte_addr_mode_brwr(struct spi_nor *nor, bool enable)
> int spi_nor_sr_ready(struct spi_nor *nor)
> {
> int ret;
> + u8 sr;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> if (ret)
> return ret;
>
> - return !(nor->bouncebuf[0] & SR_WIP);
> + return !(sr & SR_WIP);
> }
>
> /**
> @@ -784,345 +716,210 @@ int spi_nor_global_block_unlock(struct spi_nor *nor)
> }
>
> /**
> - * spi_nor_write_sr() - Write the Status Register.
> + * spi_nor_read_srs_ll() - Low-level Status Registers read.
> * @nor: pointer to 'struct spi_nor'.
> - * @sr: pointer to DMA-able buffer to write to the Status Register.
> - * @len: number of bytes to write to the Status Register.
> + * @opcode: opcode for the status register read operation.
> + * @sr: pointer to buffer for storing the contnet of the status registers.
> + * @len: number of status registers to read.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
> +static int spi_nor_read_srs_ll(struct spi_nor *nor, u8 opcode, u8 *sr2,
> + unsigned int len)
> {
> - int ret;
> + int ret, i;
>
> - ret = spi_nor_write_enable(nor);
> - if (ret)
> - return ret;
> + for (i = 0; i < len; i++)
> + nor->bouncebuf[i] = 0;
>
> if (nor->spimem) {
> - struct spi_mem_op op = SPI_NOR_WRSR_OP(sr, len);
> + struct spi_mem_op op = SPI_NOR_RDSR_OP(opcode, nor->bouncebuf, len);
>
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>
> ret = spi_mem_exec_op(nor->spimem, &op);
> } else {
> - ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_WRSR, sr,
> - len);
> + ret = spi_nor_controller_ops_read_reg(nor, opcode, nor->bouncebuf, len);
> }
>
> - if (ret) {
> - dev_dbg(nor->dev, "error %d writing SR\n", ret);
> - return ret;
> - }
> -
> - return spi_nor_wait_till_ready(nor);
> -}
> -
> -/**
> - * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
> - * ensure that the byte written match the received value.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @sr1: byte value to be written to the Status Register.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
> -{
> - int ret;
> -
> - nor->bouncebuf[0] = sr1;
> -
> - ret = spi_nor_write_sr(nor, nor->bouncebuf, 1);
> - if (ret)
> - return ret;
> -
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - if (nor->bouncebuf[0] != sr1) {
> - dev_dbg(nor->dev, "SR1: read back test failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
> - * Status Register 2 in one shot. Ensure that the byte written in the Status
> - * Register 1 match the received value, and that the 16-bit Write did not
> - * affect what was already in the Status Register 2.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @sr1: byte value to be written to the Status Register 1.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
> -{
> - int ret;
> - u8 *sr_cr = nor->bouncebuf;
> - u8 cr_written;
> -
> - /* Make sure we don't overwrite the contents of Status Register 2. */
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - ret = spi_nor_read_cr(nor, &sr_cr[1]);
> - if (ret)
> - return ret;
> - } else if ((spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> - spi_nor_get_protocol_width(nor->write_proto) == 4) &&
> - nor->params->quad_enable) {
> - /*
> - * If the Status Register 2 Read command (35h) is not
> - * supported, we should at least be sure we don't
> - * change the value of the SR2 Quad Enable bit.
> - *
> - * When the Quad Enable method is set and the buswidth is 4, we
> - * can safely assume that the value of the QE bit is one, as a
> - * consequence of the nor->params->quad_enable() call.
> - *
> - * According to the JESD216 revB standard, BFPT DWORDS[15],
> - * bits 22:20, the 16-bit Write Status (01h) command is
> - * available just for the cases in which the QE bit is
> - * described in SR2 at BIT(1).
> - */
> - sr_cr[1] = SR2_QUAD_EN_BIT1;
> - } else {
> - sr_cr[1] = 0;
> - }
> -
> - sr_cr[0] = sr1;
> -
> - ret = spi_nor_write_sr(nor, sr_cr, 2);
> - if (ret)
> - return ret;
> -
> - ret = spi_nor_read_sr(nor, sr_cr);
> - if (ret)
> - return ret;
> -
> - if (sr1 != sr_cr[0]) {
> - dev_dbg(nor->dev, "SR: Read back test failed\n");
> - return -EIO;
> - }
> -
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - return 0;
> -
> - cr_written = sr_cr[1];
> -
> - ret = spi_nor_read_cr(nor, &sr_cr[1]);
> - if (ret)
> - return ret;
> -
> - if (cr_written != sr_cr[1]) {
> - dev_dbg(nor->dev, "CR: read back test failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * spi_nor_write_16bit_cr_and_check() - Write the Status Register 1 and the
> - * Configuration Register in one shot. Ensure that the byte written in the
> - * Configuration Register match the received value, and that the 16-bit Write
> - * did not affect what was already in the Status Register 1.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @cr: byte value to be written to the Configuration Register.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
> -{
> - int ret;
> - u8 *sr_cr = nor->bouncebuf;
> - u8 sr_written;
> -
> - /* Keep the current value of the Status Register 1. */
> - ret = spi_nor_read_sr(nor, sr_cr);
> - if (ret)
> - return ret;
> -
> - sr_cr[1] = cr;
> -
> - ret = spi_nor_write_sr(nor, sr_cr, 2);
> - if (ret)
> - return ret;
> -
> - sr_written = sr_cr[0];
> -
> - ret = spi_nor_read_sr(nor, sr_cr);
> - if (ret)
> - return ret;
> -
> - if (sr_written != sr_cr[0]) {
> - dev_dbg(nor->dev, "SR: Read back test failed\n");
> - return -EIO;
> - }
> -
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - return 0;
> -
> - ret = spi_nor_read_cr(nor, &sr_cr[1]);
> - if (ret)
> - return ret;
> -
> - if (cr != sr_cr[1]) {
> - dev_dbg(nor->dev, "CR: read back test failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * spi_nor_write_16bit_sr_cr_and_check() - Write the Status Register 1 and the
> - * Configuration Register in one shot. Ensure that the bytes written in both
> - * registers match the received value.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @regs: two-byte array with values to be written to the status and
> - * configuration registers.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_write_16bit_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
> -{
> - u8 written_regs[2];
> - int ret;
> -
> - written_regs[0] = regs[0];
> - written_regs[1] = regs[1];
> - nor->bouncebuf[0] = regs[0];
> - nor->bouncebuf[1] = regs[1];
> -
> - ret = spi_nor_write_sr(nor, nor->bouncebuf, 2);
> - if (ret)
> - return ret;
> -
> - ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
> - if (ret)
> - return ret;
> -
> - if (written_regs[0] != nor->bouncebuf[0]) {
> - dev_dbg(nor->dev, "SR: Read back test failed\n");
> - return -EIO;
> - }
> -
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - return 0;
> -
> - ret = spi_nor_read_cr(nor, &nor->bouncebuf[1]);
> - if (ret)
> - return ret;
> -
> - if (written_regs[1] != nor->bouncebuf[1]) {
> - dev_dbg(nor->dev, "CR: read back test failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
> - * the byte written match the received value without affecting other bits in the
> - * Status Register 1 and 2.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @sr1: byte value to be written to the Status Register.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
> -{
> - if (nor->flags & SNOR_F_HAS_16BIT_SR)
> - return spi_nor_write_16bit_sr_and_check(nor, sr1);
> -
> - return spi_nor_write_sr1_and_check(nor, sr1);
> -}
> -
> -/**
> - * spi_nor_write_sr_cr_and_check() - Write the Status Register 1 and ensure that
> - * the byte written match the received value. Same for the Control Register if
> - * available.
> - * @nor: pointer to a 'struct spi_nor'.
> - * @regs: byte array to be written to the registers.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
> -{
> - if (nor->flags & SNOR_F_HAS_16BIT_SR)
> - return spi_nor_write_16bit_sr_cr_and_check(nor, regs);
> -
> - return spi_nor_write_sr1_and_check(nor, regs[0]);
> -}
> -
> -/**
> - * spi_nor_write_sr2() - Write the Status Register 2 using the
> - * SPINOR_OP_WRSR2 (3eh) command.
> - * @nor: pointer to 'struct spi_nor'.
> - * @sr2: pointer to DMA-able buffer to write to the Status Register 2.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
> -{
> - int ret;
> -
> - ret = spi_nor_write_enable(nor);
> - if (ret)
> - return ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = SPI_NOR_WRSR2_OP(sr2);
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_WRSR2,
> - sr2, 1);
> - }
> -
> - if (ret) {
> - dev_dbg(nor->dev, "error %d writing SR2\n", ret);
> - return ret;
> - }
> -
> - return spi_nor_wait_till_ready(nor);
> -}
> -
> -/**
> - * spi_nor_read_sr2() - Read the Status Register 2 using the
> - * SPINOR_OP_RDSR2 (3fh) command.
> - * @nor: pointer to 'struct spi_nor'.
> - * @sr2: pointer to DMA-able buffer where the value of the
> - * Status Register 2 will be written.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
> -{
> - int ret;
> -
> - if (nor->spimem) {
> - struct spi_mem_op op = SPI_NOR_RDSR2_OP(sr2);
> -
> - spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> -
> - ret = spi_mem_exec_op(nor->spimem, &op);
> - } else {
> - ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDSR2, sr2,
> - 1);
> - }
> + memcpy(sr2, nor->bouncebuf, len);
>
> if (ret)
> - dev_dbg(nor->dev, "error %d reading SR2\n", ret);
> + dev_dbg(nor->dev, "Error %d reading Status Registers\n", ret);
>
> return ret;
> }
>
> +/**
> + * spi_nor_write_srs_ll() - Low-level Status Registers write.
> + * @nor: pointer to 'struct spi_nor'.
> + * @opcode: opcode for the status register write operation.
> + * @srs: pointer to status registers buffer to write.
> + * @len: number of status registers to write.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_write_srs_ll(struct spi_nor *nor, u8 opcode, const u8 *srs,
> + unsigned int len)
> +{
> + int ret, i;
> +
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < len; i++)
> + nor->bouncebuf[i] = srs[i];
> +
> + if (nor->spimem) {
> + struct spi_mem_op op = SPI_NOR_WRSR_OP(opcode,
> + nor->bouncebuf, len);
> +
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = spi_nor_controller_ops_write_reg(nor, opcode,
> + nor->bouncebuf, len);
> + }
> +
> + if (ret) {
> + dev_dbg(nor->dev, "Error %d writing Status Registers\n", ret);
> + return ret;
> + }
> +
> + return spi_nor_wait_till_ready(nor);
> +}
> +
> +int spi_nor_read_srs(struct spi_nor *nor, u8 *sr1, u8 *sr2)
nitpick, i think this is a weird name. But maybe it's just me.
What if we'd just make the one status register as u32? I mean in the
winbond datasheet it's called S7-S0 and S15-S8. That way we also
don't need a two byte qe_mask. To optimize the standard usecase to
poll the WIP bit, we could add a byte mask.
> +{
> + struct spi_nor_flash_parameter *params = nor->params;
> + int ret;
> +
> + if (params->read_srs_opcode &&
> + ((sr1 && !params->read_sr1_opcode) || (sr2 && !params->read_sr2_opcode))) {
> + u8 srs[2] = {};
> +
> + ret = spi_nor_read_srs_ll(nor, params->read_srs_opcode, srs, 2);
> + if (ret)
> + return ret;
> +
> + if (sr1)
> + *sr1 = srs[0];
> +
> + if (sr2)
> + *sr2 = srs[1];
> +
> + return 0;
> + }
> +
> + if (sr1) {
> + ret = spi_nor_read_srs_ll(nor, params->read_sr1_opcode, sr1, 1);
> + if (ret)
> + return ret;
> + }
> +
> + if (sr2) {
> + if (params->read_sr2_opcode) {
> + ret = spi_nor_read_srs_ll(nor, params->read_sr2_opcode, sr2, 1);
> + if (ret)
> + return ret;
> + } else {
> + /* Make sure the QE bit is persistently kept */
> + if (spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> + spi_nor_get_protocol_width(nor->write_proto) == 4)
> + *sr2 = params->qe_mask[1];
> + }
> + }
> +
> + return 0;
> +}
> +
> +int spi_nor_write_srs(struct spi_nor *nor, const u8 *sr1, const u8 *sr2)
> +{
> + struct spi_nor_flash_parameter *params = nor->params;
> + int ret;
> +
> + if (nor->flags & SNOR_F_HAS_16BIT_SR) {
> + u8 srs[2];
> +
> + if (!sr1 || !sr2) {
> + ret = spi_nor_read_srs_ll(nor, params->read_srs_opcode, srs, 2);
> + if (ret)
> + return ret;
> + }
> +
> + if (sr1)
> + srs[0] = *sr1;
> + if (sr2)
> + srs[1] = *sr2;
> +
> + return spi_nor_write_srs_ll(nor, params->write_srs_opcode, srs, 2);
> + }
> +
> + if (sr1) {
> + ret = spi_nor_write_srs_ll(nor, params->write_sr1_opcode, sr1, 1);
> + if (ret)
> + return ret;
> + }
> +
> + if (sr2 && params->read_sr2_opcode) {
> + ret = spi_nor_write_srs_ll(nor, params->write_sr2_opcode, sr2, 1);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * spi_nor_generic_quad_enable() - Read the status registers,
> + * apply the QE bit mask,
> + * write the status registers,
> + * read them back and check the content.
> + *
> + * @nor: pointer to 'struct spi_nor'
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_generic_quad_enable(struct spi_nor *nor)
> +{
> + u8 *qe_mask = nor->params->qe_mask;
> + u8 srs[2] = {}, tmp[2] = {};
> + int ret;
> +
> + if (!qe_mask[0] && !qe_mask[1])
> + return 0;
> +
> + ret = spi_nor_read_srs(nor, &srs[0], &srs[1]);
> + if (ret)
> + return ret;
> +
> + if (srs[0] & qe_mask[0] || srs[1] & qe_mask[1])
> + return 0;
> +
> + srs[0] |= qe_mask[0];
> + srs[1] |= qe_mask[1];
> +
> + if (nor->flags & SNOR_F_HAS_16BIT_SR)
> + ret = spi_nor_write_srs(nor, &srs[0], &srs[1]);
> + else
> + ret = spi_nor_write_srs(nor,
> + qe_mask[0] ? &srs[0] : NULL,
> + qe_mask[1] ? &srs[1] : NULL);
> + if (ret)
> + return ret;
> +
> + ret = spi_nor_read_srs(nor, &tmp[0], &tmp[1]);
> + if (ret)
> + return ret;
> +
> + if (srs[0] != tmp[0] || srs[1] != tmp[1])
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> /**
> * spi_nor_erase_die() - Erase the entire die.
> * @nor: pointer to 'struct spi_nor'.
> @@ -1904,106 +1701,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> return ret;
> }
>
> -/**
> - * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
> - * Register 1.
> - * @nor: pointer to a 'struct spi_nor'
> - *
> - * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
> -{
> - int ret;
> -
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
> - return 0;
> -
> - nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
> -
> - return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
> -}
> -
> -/**
> - * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
> - * Register 2.
> - * @nor: pointer to a 'struct spi_nor'.
> - *
> - * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
> -{
> - int ret;
> -
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
> -
> - ret = spi_nor_read_cr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
> - return 0;
> -
> - nor->bouncebuf[0] |= SR2_QUAD_EN_BIT1;
> -
> - return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
> -}
> -
> -/**
> - * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
> - * @nor: pointer to a 'struct spi_nor'
> - *
> - * Set the Quad Enable (QE) bit in the Status Register 2.
> - *
> - * This is one of the procedures to set the QE bit described in the SFDP
> - * (JESD216 rev B) specification but no manufacturer using this procedure has
> - * been identified yet, hence the name of the function.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
> -{
> - u8 *sr2 = nor->bouncebuf;
> - int ret;
> - u8 sr2_written;
> -
> - /* Check current Quad Enable bit value. */
> - ret = spi_nor_read_sr2(nor, sr2);
> - if (ret)
> - return ret;
> - if (*sr2 & SR2_QUAD_EN_BIT7)
> - return 0;
> -
> - /* Update the Quad Enable bit. */
> - *sr2 |= SR2_QUAD_EN_BIT7;
> -
> - ret = spi_nor_write_sr2(nor, sr2);
> - if (ret)
> - return ret;
> -
> - sr2_written = *sr2;
> -
> - /* Read back and check it. */
> - ret = spi_nor_read_sr2(nor, sr2);
> - if (ret)
> - return ret;
> -
> - if (*sr2 != sr2_written) {
> - dev_dbg(nor->dev, "SR2: Read back test failed\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> static const struct spi_nor_manufacturer *manufacturers[] = {
> &spi_nor_atmel,
> &spi_nor_eon,
> @@ -2494,6 +2191,7 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> {
> struct spi_nor_flash_parameter *params = nor->params;
> unsigned int cap;
> + u8 opcode;
>
> /* X-X-X modes are not supported yet, mask them all. */
> *hwcaps &= ~SNOR_HWCAPS_X_X_X;
> @@ -2525,14 +2223,14 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> *hwcaps &= ~BIT(cap);
> }
>
> - /* Some SPI controllers might not support CR read opcode. */
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - struct spi_mem_op op = SPI_NOR_RDCR_OP(nor->bouncebuf);
> -
> + /* Some SPI controllers might not support reading SR2 */
> + opcode = nor->params->read_sr2_opcode;
> + if (opcode) {
> + struct spi_mem_op op = SPI_NOR_RDSR_OP(opcode, nor->bouncebuf, 1);
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>
> if (!spi_mem_supports_op(nor->spimem, &op))
> - nor->flags |= SNOR_F_NO_READ_CR;
> + nor->params->read_sr2_opcode = 0;
> }
> }
>
> @@ -3106,11 +2804,14 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
> const struct flash_info *info = nor->info;
> struct device_node *np = spi_nor_get_flash_node(nor);
>
> - params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> - params->otp.org = info->otp;
> -
> - /* Default to 16-bit Write Status (01h) Command */
> + /* Default to 16-bit Read/Write Status commands */
> nor->flags |= SNOR_F_HAS_16BIT_SR;
> + params->read_srs_opcode = SPINOR_OP_RDSR;
> + params->write_srs_opcode = SPINOR_OP_WRSR;
> + params->quad_enable = spi_nor_generic_quad_enable;
> + params->qe_mask[1] = BIT(1);
Not sure this is a safe bet. I'd need to consult all datasheets of
the different vendors. Not now..
> +
> + params->otp.org = info->otp;
>
> /* Set SPI NOR sizes. */
> params->writesize = 1;
> @@ -3258,8 +2959,11 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
> return 0;
>
> if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> - spi_nor_get_protocol_width(nor->write_proto) == 4))
> + spi_nor_get_protocol_width(nor->write_proto) == 4)) {
> + nor->params->qe_mask[0] = 0;
> + nor->params->qe_mask[1] = 0;
Why setting the mask to 0? The mask is still valid, but we just
cannot use it. But it could still be a useful information, i.e. in
debugfs.
> return 0;
> + }
>
> return nor->params->quad_enable(nor);
> }
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index ba2d1a862c9d..d4aff914caf5 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -37,36 +37,18 @@
> SPI_MEM_OP_NO_DUMMY, \
> SPI_MEM_OP_NO_DATA)
>
> -#define SPI_NOR_RDSR_OP(buf) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR, 0), \
> +#define SPI_NOR_RDSR_OP(opcode, buf, len) \
> + SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0), \
> SPI_MEM_OP_NO_ADDR, \
> SPI_MEM_OP_NO_DUMMY, \
> - SPI_MEM_OP_DATA_IN(1, buf, 0))
> + SPI_MEM_OP_DATA_IN(len, buf, 0))
>
> -#define SPI_NOR_WRSR_OP(buf, len) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 0), \
> +#define SPI_NOR_WRSR_OP(opcode, buf, len) \
> + SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0), \
> SPI_MEM_OP_NO_ADDR, \
> SPI_MEM_OP_NO_DUMMY, \
> SPI_MEM_OP_DATA_OUT(len, buf, 0))
>
> -#define SPI_NOR_RDSR2_OP(buf) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 0), \
> - SPI_MEM_OP_NO_ADDR, \
> - SPI_MEM_OP_NO_DUMMY, \
> - SPI_MEM_OP_DATA_OUT(1, buf, 0))
> -
> -#define SPI_NOR_WRSR2_OP(buf) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 0), \
> - SPI_MEM_OP_NO_ADDR, \
> - SPI_MEM_OP_NO_DUMMY, \
> - SPI_MEM_OP_DATA_OUT(1, buf, 0))
> -
> -#define SPI_NOR_RDCR_OP(buf) \
> - SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR, 0), \
> - SPI_MEM_OP_NO_ADDR, \
> - SPI_MEM_OP_NO_DUMMY, \
> - SPI_MEM_OP_DATA_IN(1, buf, 0))
> -
> #define SPI_NOR_EN4B_EX4B_OP(enable) \
> SPI_MEM_OP(SPI_MEM_OP_CMD(enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B, 0), \
> SPI_MEM_OP_NO_ADDR, \
> @@ -130,7 +112,6 @@ enum spi_nor_option_flags {
> SNOR_F_HAS_4BAIT = BIT(4),
> SNOR_F_HAS_LOCK = BIT(5),
> SNOR_F_HAS_16BIT_SR = BIT(6),
> - SNOR_F_NO_READ_CR = BIT(7),
> SNOR_F_HAS_SR_TB_BIT6 = BIT(8),
> SNOR_F_HAS_4BIT_BP = BIT(9),
> SNOR_F_HAS_SR_BP3_BIT6 = BIT(10),
> @@ -375,6 +356,13 @@ struct spi_nor_otp {
> * @otp: SPI NOR OTP info.
> * @set_octal_dtr: enables or disables SPI NOR octal DTR mode.
> * @quad_enable: enables SPI NOR quad mode.
> + * @qe_mask: two bytes mask used to set/clear the QE bit
Two bytes? I assume that's one for each status register. That's not
really clear. But also see above, maybe make it u32.
> + * @read_srs_opcode: opcode used to read SR1 and SR2 in one operation
> + * @read_sr1_opcode: opcode used to read SR1 alone
> + * @read_sr2_opcode: opcode used to read SR2 alone
> + * @write_srs_opcode: opcode used to write SR1 and SR2 in one operation
> + * @write_sr1_opcode: opcode used to write SR1 alone
> + * @write_sr2_opcode: opcode used to write SR2 alone
> * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
> * @ready: (optional) flashes might use a different mechanism
> * than reading the status register to indicate they
> @@ -405,6 +393,13 @@ struct spi_nor_flash_parameter {
>
> int (*set_octal_dtr)(struct spi_nor *nor, bool enable);
> int (*quad_enable)(struct spi_nor *nor);
> + u8 qe_mask[2];
> + u8 read_srs_opcode;
> + u8 read_sr1_opcode;
> + u8 read_sr2_opcode;
> + u8 write_srs_opcode;
> + u8 write_sr1_opcode;
> + u8 write_sr2_opcode;
Can we move all the opcodes into one place? There is already a
die_erase_opcode. Though it might just be a good idea now to group
them into a subnode.
spi_nor_flash_opcodes {
u8 die_erase;
u8 rdsr;
u8 rdsr1;
u8 rdsr2;
u8 wrsr;
u8 wrsr1;
u8 wrsr2;
};
Also I'd make it a callback, so a it can be overwritten if there's
some weird flash.
int (*read_sr)(struct spi_nor *nor, u32 *sr, u8 bm)
int (*write_sr)(struct spi_nor *nor, u32 sr, u8 bm)
int spi_nor_generic_read_sr(struct spi_nor *nor, u32 *sr,
u8 byte_mask)
{
spi_nor_flash_parameter *params = nor->params;
if (params->flags & SNOR_F_SUPPORTS_SR_16BIT_READ)
...
}
int spi_nor_generic_write_sr(struct spi_nor *nor, u32 *sr,
u8 byte_mask)
{
if (params->flags & SNOR_F_HAS_16BIT_SR)
...
}
-michael
> int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
> int (*ready)(struct spi_nor *nor);
>
> @@ -633,18 +628,11 @@ int spi_nor_wait_till_ready(struct spi_nor *nor);
> int spi_nor_global_block_unlock(struct spi_nor *nor);
> int spi_nor_prep_and_lock(struct spi_nor *nor);
> void spi_nor_unlock_and_unprep(struct spi_nor *nor);
> -int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
> -int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
> -int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
> int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
> enum spi_nor_protocol reg_proto);
> -int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
> int spi_nor_sr_ready(struct spi_nor *nor);
> -int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
> -int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
> -int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
> -int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
> -int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs);
> +int spi_nor_read_srs(struct spi_nor *nor, u8 *sr1, u8 *sr2);
> +int spi_nor_write_srs(struct spi_nor *nor, const u8 *sr1, const u8 *sr2);
>
> ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
> u8 *buf);
> diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
> index fbefcef2d2e1..5d1c31edb417 100644
> --- a/drivers/mtd/spi-nor/debugfs.c
> +++ b/drivers/mtd/spi-nor/debugfs.c
> @@ -19,7 +19,6 @@ static const char *const snor_f_names[] = {
> SNOR_F_NAME(HAS_4BAIT),
> SNOR_F_NAME(HAS_LOCK),
> SNOR_F_NAME(HAS_16BIT_SR),
> - SNOR_F_NAME(NO_READ_CR),
> SNOR_F_NAME(HAS_SR_TB_BIT6),
> SNOR_F_NAME(HAS_4BIT_BP),
> SNOR_F_NAME(HAS_SR_BP3_BIT6),
> diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
> index ef1edd0add70..4070a692e968 100644
> --- a/drivers/mtd/spi-nor/gigadevice.c
> +++ b/drivers/mtd/spi-nor/gigadevice.c
> @@ -23,8 +23,10 @@ gd25q256_post_bfpt(struct spi_nor *nor,
> * GD25Q256E | SFDP_JESD216_MAJOR | SFDP_JESD216B_MINOR
> */
> if (bfpt_header->major == SFDP_JESD216_MAJOR &&
> - bfpt_header->minor == SFDP_JESD216_MINOR)
> - nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> + bfpt_header->minor == SFDP_JESD216_MINOR) {
> + nor->params->qe_mask[0] = BIT(6);
> + nor->params->qe_mask[1] = 0;
> + }
>
> return 0;
> }
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 18d9a00aa22e..04db42b5141e 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -131,7 +131,8 @@ static const struct flash_info issi_nor_parts[] = {
>
> static void issi_nor_default_init(struct spi_nor *nor)
> {
> - nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> + nor->params->qe_mask[0] = BIT(6);
> + nor->params->qe_mask[1] = 0;
> }
>
> static const struct spi_nor_fixups issi_fixups = {
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index e97f5cbd9aad..55612237c1ef 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -65,11 +65,12 @@ mx25l3255e_late_init_fixups(struct spi_nor *nor)
>
> /*
> * SFDP of MX25L3255E is JESD216, which does not include the Quad
> - * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> - * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> - * Therefore, it is necessary to correct this setting by late_init.
> + * Enable bit Requirement in BFPT. As a result, during BFPT parsing
> + * the quad_enable mask is reset. Therefore, it is necessary to
> + * correct this setting by late_init.
> */
> - params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> + params->qe_mask[0] = BIT(6);
> + params->qe_mask[1] = 0;
>
> /*
> * In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> @@ -314,7 +315,8 @@ static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
>
> static void macronix_nor_default_init(struct spi_nor *nor)
> {
> - nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> + nor->params->qe_mask[0] = BIT(6);
> + nor->params->qe_mask[1] = 0;
> }
>
> static int macronix_nor_late_init(struct spi_nor *nor)
> diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
> index 7d0b145d78d8..64686b904e24 100644
> --- a/drivers/mtd/spi-nor/otp.c
> +++ b/drivers/mtd/spi-nor/otp.c
> @@ -175,24 +175,24 @@ static int spi_nor_otp_lock_bit_cr(unsigned int region)
> */
> int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region)
> {
> - u8 *cr = nor->bouncebuf;
> int ret, lock_bit;
> + u8 sr2;
>
> lock_bit = spi_nor_otp_lock_bit_cr(region);
> if (lock_bit < 0)
> return lock_bit;
>
> - ret = spi_nor_read_cr(nor, cr);
> + ret = spi_nor_read_srs(nor, NULL, &sr2);
> if (ret)
> return ret;
>
> /* no need to write the register if region is already locked */
> - if (cr[0] & lock_bit)
> + if (sr2 & lock_bit)
> return 0;
>
> - cr[0] |= lock_bit;
> + sr2 |= lock_bit;
>
> - return spi_nor_write_16bit_cr_and_check(nor, cr[0]);
> + return spi_nor_write_srs(nor, NULL, &sr2);
> }
>
> /**
> @@ -207,18 +207,18 @@ int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region)
> */
> int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region)
> {
> - u8 *cr = nor->bouncebuf;
> int ret, lock_bit;
> + u8 sr2;
>
> lock_bit = spi_nor_otp_lock_bit_cr(region);
> if (lock_bit < 0)
> return lock_bit;
>
> - ret = spi_nor_read_cr(nor, cr);
> + ret = spi_nor_read_srs(nor, NULL, &sr2);
> if (ret)
> return ret;
>
> - return cr[0] & lock_bit;
> + return sr2 & lock_bit;
> }
>
> static loff_t spi_nor_otp_region_start(const struct spi_nor *nor, unsigned int region)
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 4600983cb579..23b809cc958f 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -561,10 +561,21 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> val >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> params->page_size = 1U << val;
>
> + /*
> + * The standard declares various read and write status methods, some of
> + * them will be overloaded based on the QER field.
> + */
> + params->read_srs_opcode = SPINOR_OP_RDSR;
Not sure, that's a sane default. I think it was always 05/35 for
reading. Some newer flashes might have mapped the sr2 to the second
byte for the 05 opcode.
> + params->read_sr1_opcode = SPINOR_OP_RDSR;
> + params->read_sr2_opcode = SPINOR_OP_RDCR;
> + params->write_srs_opcode = SPINOR_OP_WRSR;
> + params->write_sr1_opcode = SPINOR_OP_WRSR;
> +
> /* Quad Enable Requirements. */
> switch (bfpt.dwords[SFDP_DWORD(15)] & BFPT_DWORD15_QER_MASK) {
> case BFPT_DWORD15_QER_NONE:
> - params->quad_enable = NULL;
> + params->qe_mask[0] = 0;
> + params->qe_mask[1] = 0;
> break;
>
> case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
> @@ -572,23 +583,33 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> * Writing only one byte to the Status Register has the
> * side-effect of clearing Status Register 2.
> */
> + params->write_sr1_opcode = 0;
> + fallthrough;
> case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
> /*
> * Read Configuration Register (35h) instruction is not
> * supported.
> */
> - nor->flags |= SNOR_F_HAS_16BIT_SR | SNOR_F_NO_READ_CR;
> - params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> + params->read_sr2_opcode = 0;
> + nor->flags |= SNOR_F_HAS_16BIT_SR;
> + params->qe_mask[0] = 0;
> + params->qe_mask[1] = BIT(1);
> break;
>
> case BFPT_DWORD15_QER_SR1_BIT6:
> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> - params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> + params->qe_mask[0] = BIT(6);
> + params->qe_mask[1] = 0;
> break;
>
> case BFPT_DWORD15_QER_SR2_BIT7:
> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> - params->quad_enable = spi_nor_sr2_bit7_quad_enable;
> + params->qe_mask[0] = 0;
> + params->qe_mask[1] = BIT(7);
> + params->read_srs_opcode = 0;
> + params->read_sr2_opcode = SPINOR_OP_RDSR2;
> + params->write_srs_opcode = 0;
> + params->write_sr2_opcode = SPINOR_OP_WRSR2;
> break;
>
> case BFPT_DWORD15_QER_SR2_BIT1:
> @@ -599,8 +620,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> * assumption of a 16-bit Write Status (01h) command.
> */
> nor->flags |= SNOR_F_HAS_16BIT_SR;
> -
> - params->quad_enable = spi_nor_sr2_bit1_quad_enable;
> + params->write_sr1_opcode = 0;
> + params->read_srs_opcode = 0;
> + params->qe_mask[0] = 0;
> + params->qe_mask[1] = BIT(1);
> break;
>
> default:
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 19a0ad145599..2689ea21074c 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -399,8 +399,9 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
> nor->bouncebuf);
> bool is3byte, is4byte;
> int ret;
> + u8 sr;
>
> - ret = spi_nor_read_sr(nor, &nor->bouncebuf[1]);
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> if (ret)
> return ret;
>
> @@ -408,7 +409,7 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
> if (ret)
> return ret;
>
> - is3byte = (nor->bouncebuf[0] == nor->bouncebuf[1]);
> + is3byte = (nor->bouncebuf[0] == sr);
>
> op = (struct spi_mem_op)
> CYPRESS_NOR_RD_ANY_REG_OP(4, SPINOR_REG_CYPRESS_STR1V, 0,
> @@ -417,7 +418,7 @@ static int cypress_nor_determine_addr_mode_by_sr1(struct spi_nor *nor,
> if (ret)
> return ret;
>
> - is4byte = (nor->bouncebuf[0] == nor->bouncebuf[1]);
> + is4byte = (nor->bouncebuf[0] == sr);
>
> if (is3byte == is4byte)
> return -EIO;
> @@ -1098,13 +1099,14 @@ static const struct flash_info spansion_nor_parts[] = {
> static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
> {
> int ret;
> + u8 sr;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &sr, NULL);
> if (ret)
> return ret;
>
> - if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
> - if (nor->bouncebuf[0] & SR_E_ERR)
> + if (sr & (SR_E_ERR | SR_P_ERR)) {
> + if (sr & SR_E_ERR)
> dev_err(nor->dev, "Erase Error occurred\n");
> else
> dev_err(nor->dev, "Programming Error occurred\n");
> @@ -1124,7 +1126,7 @@ static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
> return -EIO;
> }
>
> - return !(nor->bouncebuf[0] & SR_WIP);
> + return !(sr & SR_WIP);
> }
>
> static int spansion_nor_late_init(struct spi_nor *nor)
> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
> index db02c14ba16f..15f8b2cbd497 100644
> --- a/drivers/mtd/spi-nor/sst.c
> +++ b/drivers/mtd/spi-nor/sst.c
> @@ -21,16 +21,17 @@ static int sst26vf_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
> static int sst26vf_nor_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
> {
> int ret;
> + u8 cr;
>
> /* We only support unlocking the entire flash array. */
> if (ofs != 0 || len != nor->params->size)
> return -EINVAL;
>
> - ret = spi_nor_read_cr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, NULL, &cr);
> if (ret)
> return ret;
>
> - if (!(nor->bouncebuf[0] & SST26VF_CR_BPNV)) {
> + if (!(cr & SST26VF_CR_BPNV)) {
> dev_dbg(nor->dev, "Any block has been permanently locked\n");
> return -EINVAL;
> }
> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
> index 235070b215d1..d746cb595b09 100644
> --- a/drivers/mtd/spi-nor/swp.c
> +++ b/drivers/mtd/spi-nor/swp.c
> @@ -36,7 +36,7 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
>
> static u8 spi_nor_get_sr_cmp_mask(struct spi_nor *nor)
> {
> - if (!(nor->flags & SNOR_F_NO_READ_CR) &&
> + if (nor->params->read_sr2_opcode &&
> nor->flags & SNOR_F_HAS_SR2_CMP_BIT6)
> return SR2_CMP_BIT6;
> else
> @@ -207,17 +207,9 @@ void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
>
>
> if (!sr) {
> - if (spi_nor_read_sr(nor, nor->bouncebuf))
> + if (spi_nor_read_srs(nor, &sr_cr[0], &sr_cr[1]))
> return;
>
> - sr_cr[0] = nor->bouncebuf[0];
> -
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - if (spi_nor_read_cr(nor, nor->bouncebuf))
> - return;
> - }
> -
> - sr_cr[1] = nor->bouncebuf[0];
> sr = sr_cr;
> }
>
> @@ -273,20 +265,10 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
> int ret;
> u8 pow;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &status_old[0], &status_old[1]);
> if (ret)
> return ret;
>
> - status_old[0] = nor->bouncebuf[0];
> -
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - ret = spi_nor_read_cr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - status_old[1] = nor->bouncebuf[0];
> - }
> -
> /* If nothing in our range is unlocked, we don't need to do anything */
> if (spi_nor_is_locked_sr(nor, ofs, len, status_old))
> return 0;
> @@ -378,10 +360,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
> (ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
> return -EINVAL;
>
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
> - else
> - ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
> + ret = spi_nor_write_srs(nor, &best_status_new[0], &best_status_new[1]);
> if (ret)
> return ret;
>
> @@ -409,20 +388,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
> int ret;
> u8 pow;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &status_old[0], &status_old[1]);
> if (ret)
> return ret;
>
> - status_old[0] = nor->bouncebuf[0];
> -
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - ret = spi_nor_read_cr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - status_old[1] = nor->bouncebuf[0];
> - }
> -
> /* If nothing in our range is locked, we don't need to do anything */
> if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old))
> return 0;
> @@ -512,10 +481,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
> (ofs_new < ofs_old || (ofs_old + len_old) < (ofs_new + len_new)))
> return -EINVAL;
>
> - if (nor->flags & SNOR_F_NO_READ_CR)
> - ret = spi_nor_write_sr_and_check(nor, best_status_new[0]);
> - else
> - ret = spi_nor_write_sr_cr_and_check(nor, best_status_new);
> + ret = spi_nor_write_srs(nor, &best_status_new[0], &best_status_new[1]);
> if (ret)
> return ret;
>
> @@ -536,20 +502,10 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
> u8 sr_cr[2] = {};
> int ret;
>
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> + ret = spi_nor_read_srs(nor, &sr_cr[0], &sr_cr[1]);
> if (ret)
> return ret;
>
> - sr_cr[0] = nor->bouncebuf[0];
> -
> - if (!(nor->flags & SNOR_F_NO_READ_CR)) {
> - ret = spi_nor_read_cr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - sr_cr[1] = nor->bouncebuf[0];
> - }
> -
> return spi_nor_is_locked_sr(nor, ofs, len, sr_cr);
> }
>
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index 60a7bb1f2ce4..f96a11531a09 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -481,13 +481,13 @@ static int winbond_nor_late_init(struct spi_nor *nor)
> * been to declare CR reads as unsupported, whereas the Jedec
> * specification doesn't clearly state that. In practice, all these
> * chips do support reading back the CR, which is needed for SWP support,
> - * so make sure that capability remains enabled (needed for SWP).
> + * so make sure that capability remains enabled.
> * In practice, only exclude the old W25X family (JEDEC ID: EF 30 xx)
> * which actually does not support this feature.
> */
> if (nor->info->id->bytes[0] == 0xef &&
> nor->info->id->bytes[1] > 0x30)
> - nor->flags &= ~SNOR_F_NO_READ_CR;
> + params->read_sr2_opcode = SPINOR_OP_RDCR;
>
> return 0;
> }
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4b92494827b1..f0b5a1989881 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -113,20 +113,16 @@
> #define SR_E_ERR BIT(5)
> #define SR_P_ERR BIT(6)
>
> -#define SR1_QUAD_EN_BIT6 BIT(6)
> -
> #define SR_BP_SHIFT 2
>
> /* Enhanced Volatile Configuration Register bits */
> #define EVCR_QUAD_EN_MICRON BIT(7) /* Micron Quad I/O */
>
> /* Status Register 2 bits. */
> -#define SR2_QUAD_EN_BIT1 BIT(1)
> #define SR2_LB1 BIT(3) /* Security Register Lock Bit 1 */
> #define SR2_LB2 BIT(4) /* Security Register Lock Bit 2 */
> #define SR2_LB3 BIT(5) /* Security Register Lock Bit 3 */
> #define SR2_CMP_BIT6 BIT(6)
> -#define SR2_QUAD_EN_BIT7 BIT(7)
>
> /* Supported SPI protocols */
> #define SNOR_PROTO_INST_MASK GENMASK(23, 16)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-07-07 9:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 16:05 [PATCH 0/5] mtd: spi-nor: Massive QE handling cleanup and Winbond RV chips addition Miquel Raynal
2026-05-29 16:05 ` [PATCH 1/5] mtd: spi-nor: Refactor Read Status/Write Status support Miquel Raynal
2026-07-07 9:43 ` Michael Walle
2026-05-29 16:05 ` [PATCH 2/5] mtd: spi-nor: Add support for the new JESD216 rev F QER field Miquel Raynal
2026-05-29 16:05 ` [PATCH 3/5] mtd: spi-nor: Move the SFDP header structure to a C header Miquel Raynal
2026-05-29 16:05 ` [PATCH 4/5] mtd: spi-nor: winbond: Add support for W25Q02RV-M Miquel Raynal
2026-05-29 16:05 ` [PATCH 5/5] mtd: spi-nor: winbond: Add support for W25Q51RV-M Miquel Raynal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox