mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes
@ 2026-10-22 10:45 haibo.chen
  2026-10-22 10:51 ` Miquel Raynal
  0 siblings, 1 reply; 2+ messages in thread
From: haibo.chen @ 2026-10-22 10:45 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra
  Cc: linux-mtd, linux-kernel, imx, michael, Haibo Chen

From: Haibo Chen <haibo.chen@nxp.com>

The core defaults opcodes.read_sr2 to the legacy RDCR opcode (0x35), and
spi_nor_cache_sr_lock_bits() reads SR2 during spi_nor_init(), i.e. after
the flash has been switched to its runtime protocol. On an octal flash
running in 8D-8D-8D mode the opcode is extended per cmd_ext_type (e.g.
0x3535), but RDCR is not a valid register read there: these parts access
their status/configuration registers through a vendor-specific indirect
space, if at all. The flash does not answer, the register read times out
and probe (or resume) fails (on i.MX FlexSPI: -ETIMEDOUT and a controller
WARN(), seen with a Micron MT35XU01G and a Macronix MX25UW51345G).

Clear read_sr2 on every path that enables 8D-8D-8D, so that
spi_nor_read_sr1_and_sr2() degrades to an SR1-only read and no
unsupported command is issued. A manufacturer driver that genuinely
needs an SR2 read in this mode can still install its own opcode in a
later fixup.

- spi_nor_parse_profile1(): the common case. The xSPI Profile 1.0 table
  does not describe any SR2/RDCR read, and it runs before the
  manufacturer late_init.
- mt35xu512aba_post_sfdp_fixup(): some mt35xu512aba batches lack the
  Profile 1.0 table and enable 8D here instead, with the BFPT QER field
  left at a reserved value so read_sr2 keeps its RDCR default.
- spi_nor_no_sfdp_init_params(): non-SFDP parts that enable 8D via
  SPI_NOR_OCTAL_DTR_READ keep the RDCR default too.

CMP (the only user of SR2 beyond quad-enable in the locking code) is
Winbond-specific and is not placed in SR2 on their 8D-capable parts, so
no octal flash loses a usable SR2 read because of this change.

Fixes: b7b63475903c ("mtd: spi-nor: Create a local SR cache")
Fixes: 63489002d397 ("mtd: spi-nor: Refactor Read Status/Write Status support")
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
Changes in v4:
- Following the Sashiko review and Miquel's preference, keep the
  capability-based clear but cover every path that enables 8D-8D-8D, not
  only the xSPI Profile 1.0 one:
    - spi_nor_parse_profile1() (the common case, already in v3),
    - mt35xu512aba_post_sfdp_fixup(): some batches lack the 8D command
      info in SFDP so Profile 1.0 parsing may not clear it,
    - spi_nor_no_sfdp_init_params(): non-SFDP parts using
      SPI_NOR_OCTAL_DTR_READ.
- Checked all vendor drivers; Micron is the only one that enables 8D
  outside Profile 1.0 parsing. Macronix and Cypress rely on Profile 1.0.
- Link to v3: https://lore.kernel.org/r/20261022-spi-nor-fix-v3-1-bffa00b75c27@nxp.com

Changes in v3:
- Clear opcodes.read_sr2 in spi_nor_parse_profile1() (capability based)
  instead of in spi_nor_set_octal_dtr() (runtime-protocol based), as
  suggested by Miquel and Michael. It runs for octal-capable parts and
  before the manufacturer late_init, so a driver can still install its
  own SR2 opcode afterwards.
- Confirmed on a Micron MT35XU01G (i.MX95) and a Macronix MX25UW51345G
  (i.MX8ULP): both publish the xSPI Profile 1.0 table, and 0x35 is not a
  valid SR2 read in 8D mode on either.
- Drop the Assisted-by tag (removed upstream for 7.3).
- Link to v2: https://lore.kernel.org/r/20260929-spi-nor-fix-v2-1-75440cfe4e76@nxp.com

Changes in v2:
- Rework the fix following review: instead of guarding the read at runtime
  in spi_nor_read_sr2(), clear the default RDCR opcode.read_sr2 once, at the
  point the switch to Octal DTR succeeds (spi_nor_set_octal_dtr()).
- Key the clear off the real runtime protocol so a flash that advertises
  Octal DTR but runs in (x)STR (host without 8D support) keeps its RDCR.
- Drop the swp.c change and the -EOPNOTSUPP handling; a cleared read_sr2
  already means "no SR2 read", handled by the existing debugfs caching path.
- Trim the code comment; move the details to the commit message.
- Link to v1: https://lore.kernel.org/r/20260928-spi-nor-fix-v1-1-ce65ba90cfbd@nxp.com
---
 drivers/mtd/spi-nor/core.c      |  6 ++++++
 drivers/mtd/spi-nor/micron-st.c | 10 ++++++++++
 drivers/mtd/spi-nor/sfdp.c      | 11 +++++++++++
 3 files changed, 27 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 90524c58602e1c27cd83951ab0cdd3669a2ebfd6..8ded5cd12669161a95d7c08773b5a3a9a5f9b3ac 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2711,6 +2711,12 @@ static void spi_nor_no_sfdp_init_params(struct spi_nor *nor)
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
 					  0, 20, SPINOR_OP_READ_FAST,
 					  SNOR_PROTO_8_8_8_DTR);
+		/*
+		 * The default SR2 read opcode(35h) is not usable in
+		 * 8D-8D-8D mode; drop it so spi_nor_init() does not issue an
+		 * unsupported command.
+		 */
+		params->opcodes.read_sr2 = 0;
 	}
 
 	if (no_sfdp_flags & SPI_NOR_OCTAL_DTR_PP) {
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index f97f2dead8c2f175564410871c896260dc31aad3..cff774a83d7656ca2d7dc5af13a78c8c520b1dd9 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -181,6 +181,16 @@ static int mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
 	nor->params->rdsr_dummy = 8;
 	nor->params->rdsr_addr_nbytes = 0;
 
+	/*
+	 * Some batches do not expose the 8D-8D-8D command information through
+	 * SFDP (see above), so spi_nor_parse_profile1() may not run and clear
+	 * the default SR2 read opcode(35h). Drop it here as well: it is not a
+	 * valid register read in 8D-8D-8D mode and would time out during
+	 * spi_nor_init(). Clearing it again when it was already cleared is
+	 * harmless.
+	 */
+	nor->params->opcodes.read_sr2 = 0;
+
 	/*
 	 * The BFPT quad enable field is set to a reserved value so the quad
 	 * enable function is ignored by spi_nor_parse_bfpt(). Make sure we
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 4a3f82e97cef8a76c9704918aa6558ca4e2f2b97..722eeff71d0df8c47ec7bdd795c08ba1bdf5e9cc 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1344,6 +1344,17 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
 	 */
 	nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
 
+	/*
+	 * The xSPI Profile 1.0 table does not describe a Read Configuration
+	 * Register (RDCR, 35h) instruction, and the single-byte RDCR default
+	 * is not usable in 8D-8D-8D mode. Drop the SR2 read opcode for these
+	 * octal-capable flashes so spi_nor_read_sr1_and_sr2() degrades to an
+	 * SR1-only read and spi_nor_init() does not issue an unsupported
+	 * command that the flash never answers. A manufacturer driver that
+	 * actually needs SR2 can install its own opcode in a later fixup.
+	 */
+	nor->params->opcodes.read_sr2 = 0;
+
 out:
 	kfree(dwords);
 	return ret;

---
base-commit: 9a482184d910775cf648863148944945b74e2815
change-id: 20260928-spi-nor-fix-eb7ebe22a740

Best regards,
-- 
Haibo Chen <haibo.chen@nxp.com>


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

* Re: [PATCH v4] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes
  2026-10-22 10:45 [PATCH v4] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes haibo.chen
@ 2026-10-22 10:51 ` Miquel Raynal
  0 siblings, 0 replies; 2+ messages in thread
From: Miquel Raynal @ 2026-10-22 10:51 UTC (permalink / raw)
  To: haibo.chen
  Cc: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd, linux-kernel,
	imx, michael, Haibo Chen

On 22/10/2026 at 18:45:16 +08, haibo.chen@oss.nxp.com wrote:

> From: Haibo Chen <haibo.chen@nxp.com>
>
> The core defaults opcodes.read_sr2 to the legacy RDCR opcode (0x35), and
> spi_nor_cache_sr_lock_bits() reads SR2 during spi_nor_init(), i.e. after
> the flash has been switched to its runtime protocol. On an octal flash
> running in 8D-8D-8D mode the opcode is extended per cmd_ext_type (e.g.
> 0x3535), but RDCR is not a valid register read there: these parts access
> their status/configuration registers through a vendor-specific indirect
> space, if at all. The flash does not answer, the register read times out
> and probe (or resume) fails (on i.MX FlexSPI: -ETIMEDOUT and a controller
> WARN(), seen with a Micron MT35XU01G and a Macronix MX25UW51345G).
>
> Clear read_sr2 on every path that enables 8D-8D-8D, so that
> spi_nor_read_sr1_and_sr2() degrades to an SR1-only read and no
> unsupported command is issued. A manufacturer driver that genuinely
> needs an SR2 read in this mode can still install its own opcode in a
> later fixup.
>
> - spi_nor_parse_profile1(): the common case. The xSPI Profile 1.0 table
>   does not describe any SR2/RDCR read, and it runs before the
>   manufacturer late_init.
> - mt35xu512aba_post_sfdp_fixup(): some mt35xu512aba batches lack the
>   Profile 1.0 table and enable 8D here instead, with the BFPT QER field
>   left at a reserved value so read_sr2 keeps its RDCR default.
> - spi_nor_no_sfdp_init_params(): non-SFDP parts that enable 8D via
>   SPI_NOR_OCTAL_DTR_READ keep the RDCR default too.
>
> CMP (the only user of SR2 beyond quad-enable in the locking code) is
> Winbond-specific and is not placed in SR2 on their 8D-capable parts, so
> no octal flash loses a usable SR2 read because of this change.
>
> Fixes: b7b63475903c ("mtd: spi-nor: Create a local SR cache")

Is this still relevant?

> Fixes: 63489002d397 ("mtd: spi-nor: Refactor Read Status/Write Status
> support")

Will be merged in the next cycle, so Cc: stable is probably required.

> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

> Changes in v4:
> - Following the Sashiko review and Miquel's preference, keep the
>   capability-based clear but cover every path that enables 8D-8D-8D, not
>   only the xSPI Profile 1.0 one:
>     - spi_nor_parse_profile1() (the common case, already in v3),
>     - mt35xu512aba_post_sfdp_fixup(): some batches lack the 8D command
>       info in SFDP so Profile 1.0 parsing may not clear it,
>     - spi_nor_no_sfdp_init_params(): non-SFDP parts using
>       SPI_NOR_OCTAL_DTR_READ.
> - Checked all vendor drivers; Micron is the only one that enables 8D
>   outside Profile 1.0 parsing. Macronix and Cypress rely on Profile
> 1.0.

Winbond as well (see my W25QxxPW series).

Thanks,
Miquèl

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

end of thread, other threads:[~2026-10-22 10:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 10:45 [PATCH v4] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes haibo.chen
2026-10-22 10:51 ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®