mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes
@ 2026-10-23  7:11 haibo.chen
  2026-11-10  8:13 ` Bough Chen
  0 siblings, 1 reply; 2+ messages in thread
From: haibo.chen @ 2026-10-23  7:11 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, stable

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: 63489002d397 ("mtd: spi-nor: Refactor Read Status/Write Status support")
Cc: stable@vger.kernel.org
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v5:
- No code change; address review feedback from Miquel on v4:
    - Drop the Fixes: b7b63475903c tag, keep only the Refactor one.
    - Add Cc: stable.
    - Collect Reviewed-by: Miquel Raynal.
- Link to v4: https://lore.kernel.org/r/20261022-spi-nor-fix-v4-1-fba3ec3d2b5e@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 v5] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes
  2026-10-23  7:11 [PATCH v5] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes haibo.chen
@ 2026-11-10  8:13 ` Bough Chen
  0 siblings, 0 replies; 2+ messages in thread
From: Bough Chen @ 2026-11-10  8:13 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, stable

Hello,

Just a gentle ping to the MTD spi nor maintainers.

Please let me know if any changes are still needed. Otherwise, could it be picked up?

Thanks
Haibo Chen

On Fri, Oct 23, 2026 at 03:11:54PM +0800, 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: 63489002d397 ("mtd: spi-nor: Refactor Read Status/Write Status support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> Changes in v5:
> - No code change; address review feedback from Miquel on v4:
>     - Drop the Fixes: b7b63475903c tag, keep only the Refactor one.
>     - Add Cc: stable.
>     - Collect Reviewed-by: Miquel Raynal.
> - Link to v4: https://lore.kernel.org/r/20261022-spi-nor-fix-v4-1-fba3ec3d2b5e@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

end of thread, other threads:[~2026-11-10  8:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-23  7:11 [PATCH v5] mtd: spi-nor: clear the default read_sr2 opcode for Octal DTR flashes haibo.chen
2026-11-10  8:13 ` Bough Chen

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®