[PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[haibo.chen]

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

The core defaults opcodes.read_sr2 to the legacy RDCR opcode (0x35). In
8D-8D-8D mode the opcode is extended to two bytes per cmd_ext_type, but
the resulting command is not a valid SR2 read for these flashes, which
access their status/config registers through a vendor-specific indirect
register space. Since spi_nor_cache_sr_lock_bits() now reads SR2 during
init, i.e. after the switch to Octal DTR, the extended RDCR gets no data
back and the read times out (on i.MX FlexSPI: -ETIMEDOUT and a controller
WARN() during probe with Micron MT35xU and Macronix octal parts).

Clear read_sr2 when the switch to Octal DTR actually succeeds, so the SR2
read is skipped (a cleared opcode already means "unsupported"). Doing it
in spi_nor_set_octal_dtr() keys off the real runtime protocol: a flash
that advertises Octal DTR but runs in (x)STR because the host lacks
support keeps its usable RDCR.

Assisted-by: LLM
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 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 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 90524c58602e1c27cd83951ab0cdd3669a2ebfd6..5c0985491ade59cab317b74aab8572d7caedb64a 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3014,10 +3014,16 @@ static int spi_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
 	if (ret)
 		return ret;
 
-	if (enable)
+	if (enable) {
 		nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
-	else
+		/*
+		 * The default RDCR is not a valid SR2 read in Octal DTR mode;
+		 * clear it so SR2 reads are skipped instead of being issued.
+		 */
+		nor->params->opcodes.read_sr2 = 0;
+	} else {
 		nor->reg_proto = SNOR_PROTO_1_1_1;
+	}
 
 	return 0;
 }

---
base-commit: ba52b770f89bd2f3771b98edb92c4fde68ca8cc4
change-id: 20260928-spi-nor-fix-eb7ebe22a740

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

Re: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mtd: spi-nor: compilation error accessing non-existent opcodes.read_sr2
- [High] mtd: spi-nor: modifying hallucinated opcode does not skip SR2 reads
- [Medium] mtd: spi-nor: permanent breakage of SR2 reads due to missing state restore
--

commit b3ebcb1264c5eaab828caba13b5a6ee56af80aa5
Author: Haibo Chen <haibo.chen@nxp.com>

mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

This commit attempts to skip invalid SR2 reads when switching to Octal DTR
mode by clearing a default RDCR opcode. It updates spi_nor_set_octal_dtr()
to clear read_sr2 on success.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 8bc117b46e028..0f47d869cbb48 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3238,10 +3238,16 @@ static int spi_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
>  	if (ret)
>  		return ret;
>  
> -	if (enable)
> +	if (enable) {
>  		nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> -	else
> +		/*
> +		 * The default RDCR is not a valid SR2 read in Octal DTR mode;
> +		 * clear it so SR2 reads are skipped instead of being issued.
> +		 */
> +		nor->params->opcodes.read_sr2 = 0;

[Severity: High]
Can this code compile successfully? struct spi_nor_flash_parameter (which
nor->params points to) does not appear to contain an opcodes field, which
would cause a build failure here.

[Severity: High]
If this dynamic opcode field did exist, would modifying it actually prevent
the SR2 reads?

Looking at spi_nor_read_sr2(), it seems the reads rely entirely on statically
hardcoded macros rather than dynamic configuration fields:

drivers/mtd/spi-nor/core.c:spi_nor_read_sr2() {
    ...
	if (nor->spimem) {
		struct spi_mem_op op = SPI_NOR_RDSR2_OP(sr2);
    ...
	} else {
		ret = spi_nor_controller_ops_read_reg(nor, SPINOR_OP_RDSR2, sr2,
						      1);
	}
    ...
}

Since SPI_NOR_RDSR2_OP and SPINOR_OP_RDSR2 are static constants, wouldn't
the reads still be issued regardless of this variable's state?

> +	} else {
>  		nor->reg_proto = SNOR_PROTO_1_1_1;
> +	}

[Severity: Medium]
When disabling Octal DTR, does this path also need to restore the cleared
opcode state?

If the opcode was zeroed when enabling the mode, dropping back to
SNOR_PROTO_1_1_1 without restoring it might leave SR2 reads permanently
disabled for the remaining lifetime of the device.

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-spi-nor-fix-v2-1-75440cfe4e76@nxp.com?part=1

Re: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[Miquel Raynal]

On 29/09/2026 at 06:59:24 GMT, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] mtd: spi-nor: compilation error accessing non-existent opcodes.read_sr2
> - [High] mtd: spi-nor: modifying hallucinated opcode does not skip SR2 reads
> - [Medium] mtd: spi-nor: permanent breakage of SR2 reads due to
> missing state restore

Please ignore these. Sashiko did not apply the patch to the correct tree
(-rcX instead of -next).

Re: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[Miquel Raynal]

On 29/09/2026 at 14:53:07 +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). In
> 8D-8D-8D mode the opcode is extended to two bytes per cmd_ext_type, but
> the resulting command is not a valid SR2 read for these flashes, which
> access their status/config registers through a vendor-specific indirect
> register space. Since spi_nor_cache_sr_lock_bits() now reads SR2 during
> init, i.e. after the switch to Octal DTR, the extended RDCR gets no data
> back and the read times out (on i.MX FlexSPI: -ETIMEDOUT and a controller
> WARN() during probe with Micron MT35xU and Macronix octal parts).
>
> Clear read_sr2 when the switch to Octal DTR actually succeeds, so the SR2
> read is skipped (a cleared opcode already means "unsupported"). Doing it
> in spi_nor_set_octal_dtr() keys off the real runtime protocol: a flash
> that advertises Octal DTR but runs in (x)STR because the host lacks
> support keeps its usable RDCR.
>
> Assisted-by: LLM
> 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 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()).

I don't get that choice. Why switching when Octal DTR succeeds only? SR2
is either supported or not supported, I don't think it is anyway
different when entering octal DTR mode, is it? So I would expect SR2 to
be cleared earlier than that, once we know the chip is octal DTR
capable. And this must be early enough so that manufacturer drivers can
still set their own value.

I am wondering whether we should simply drop sr2 opcode in the QER SFDP
parsing entirely for octal DTR devices (Michael?).


> - 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.

Are you sure this is a valid case?

Thanks,
Miquèl

RE: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[Bough Chen (OSS)]

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, September 29, 2026 4:23 PM
> To: Bough Chen (OSS) <haibo.chen@oss.nxp.com>
> Cc: Pratyush Yadav <pratyush@kernel.org>; Michael Walle
> <mwalle@kernel.org>; Takahiro Kuwano <takahiro.kuwano@infineon.com>;
> Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; michael@walle.cc; Bough Chen
> <haibo.chen@nxp.com>
> Subject: Re: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when
> entering Octal DTR
> 
> On 29/09/2026 at 14:53:07 +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).
> > In 8D-8D-8D mode the opcode is extended to two bytes per cmd_ext_type,
> > but the resulting command is not a valid SR2 read for these flashes,
> > which access their status/config registers through a vendor-specific
> > indirect register space. Since spi_nor_cache_sr_lock_bits() now reads
> > SR2 during init, i.e. after the switch to Octal DTR, the extended RDCR
> > gets no data back and the read times out (on i.MX FlexSPI: -ETIMEDOUT
> > and a controller
> > WARN() during probe with Micron MT35xU and Macronix octal parts).
> >
> > Clear read_sr2 when the switch to Octal DTR actually succeeds, so the
> > SR2 read is skipped (a cleared opcode already means "unsupported").
> > Doing it in spi_nor_set_octal_dtr() keys off the real runtime
> > protocol: a flash that advertises Octal DTR but runs in (x)STR because
> > the host lacks support keeps its usable RDCR.
> >
> > Assisted-by: LLM
> > 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 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()).
> 
> I don't get that choice. Why switching when Octal DTR succeeds only? SR2 is
> either supported or not supported, I don't think it is anyway different when
> entering octal DTR mode, is it? So I would expect SR2 to be cleared earlier than
> that, once we know the chip is octal DTR capable. And this must be early
> enough so that manufacturer drivers can still set their own value.
> 
> I am wondering whether we should simply drop sr2 opcode in the QER SFDP
> parsing entirely for octal DTR devices (Michael?).

The reason I clear it at the point Octal DTR actually succeeds rather than
 "once the chip is octal-DTR-capable" is that capability != running mode: 
a chip can advertise Octal DTR while still running in (x)STR because the host 
controller does not support 8D (see reply below). In that (x)STR case RDCR is
 valid and needed, so keying off capability alone would wrongly drop a usable
 SR2 read.

That said, I agree the current spot is awkward and does not let manufacturer 
drivers override the value. I'm happy to move it earlier. Two options:
1, Clear it during SFDP QER parsing (as you suggest) for octal-DTR-capable devices.
     This is the natural place next to the existing read_sr2 = 0 QER cases, and it runs 
      before the manufacturer late_init/fixups, so a driver can still install its own SR2
      opcode afterwards. 
     My only concern is the capability-vs-running-mode point above — we'd be 
     clearing based on advertised 8D capability, not on whether 8D is actually entered.

2, Clear it in spi_nor_late_init_params() after the manufacturer hooks, gated on 
     octal-DTR capability, so vendor values set in their late_init are preserved.

If the capability-based approach is acceptable (i.e. we accept that a chip advertising
 8D but forced to STR by the host loses its RDCR), I'll go with dropping it in the QER SFDP
 parsing. Michael, do you have a preference?
> 
> 
> > - 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.
> 
> Are you sure this is a valid case?

Yes. spi_nor_setup() computes shared_mask = hwcaps->mask & params->hwcaps.mask, 
i.e. the intersection of flash and controller capabilities. If the host does not support 8D, 
SNOR_HWCAPS_READ_8_8_8_DTR is not in shared_mask, spi_nor_select_read() picks a 
non-8D read_proto/write_proto, and spi_nor_set_octal_dtr() bails out early at:

if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
      nor->write_proto == SNOR_PROTO_8_8_8_DTR))
	return 0;

So the chip stays in (x)STR with reg_proto == SNOR_PROTO_1_1_1, and RDCR is still a valid 
single-byte SR2 read there. That is the case I wanted to avoid breaking by keying the clear
 off the negotiated runtime protocol rather than the advertised capability. If we move the 
clear to capability-based SFDP/late_init as discussed, this is exactly the scenario we need to 
be comfortable regressing (an 8D-capable part on a non-8D host would lose its RDCR read of SR2).

Regards
Haibo Chen

> 
> Thanks,
> Miquèl

Re: [PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR

[Michael Walle]

On Tue Sep 29, 2026 at 10:23 AM CEST, Miquel Raynal wrote:
> On 29/09/2026 at 14:53:07 +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). In
>> 8D-8D-8D mode the opcode is extended to two bytes per cmd_ext_type, but
>> the resulting command is not a valid SR2 read for these flashes, which
>> access their status/config registers through a vendor-specific indirect
>> register space. Since spi_nor_cache_sr_lock_bits() now reads SR2 during
>> init, i.e. after the switch to Octal DTR, the extended RDCR gets no data
>> back and the read times out (on i.MX FlexSPI: -ETIMEDOUT and a controller
>> WARN() during probe with Micron MT35xU and Macronix octal parts).
>>
>> Clear read_sr2 when the switch to Octal DTR actually succeeds, so the SR2
>> read is skipped (a cleared opcode already means "unsupported"). Doing it
>> in spi_nor_set_octal_dtr() keys off the real runtime protocol: a flash
>> that advertises Octal DTR but runs in (x)STR because the host lacks
>> support keeps its usable RDCR.
>>
>> Assisted-by: LLM
>> 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 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()).
>
> I don't get that choice. Why switching when Octal DTR succeeds only? SR2
> is either supported or not supported, I don't think it is anyway
> different when entering octal DTR mode, is it? So I would expect SR2 to
> be cleared earlier than that, once we know the chip is octal DTR
> capable. And this must be early enough so that manufacturer drivers can
> still set their own value.
>
> I am wondering whether we should simply drop sr2 opcode in the QER SFDP
> parsing entirely for octal DTR devices (Michael?).

This should probably be put into spi_nor_parse_profile1() and check
if command 15h (read configuration register according to JESD251D)
is supported or not. Would that work, Haibo Chen?

Honestly, I haven't done much with these high density NOR flashes,
so my knowledge is rather sparse. I've just skimmed over the JEDEC
docs.

-michael

>> - 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.
>
> Are you sure this is a valid case?