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

[Bough Chen <haibo.chen@oss.nxp.com>]

On Thu, Oct 15, 2026 at 12:23:38PM +0200, Miquel Raynal wrote:
> On 15/10/2026 at 11:10:17 +08, Bough Chen <haibo.chen@oss.nxp.com> wrote:
> 
> > On Tue, Sep 29, 2026 at 09:10:37AM +0000, Bough Chen (OSS) wrote:
> >> > -----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?
> >
> > Hi Miquel,
> >
> > Any suggestion?
> >
> > Regards
> > Haibo Chen
> >
> >> > 
> >> > 
> >> > > - 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).
> 
> I am sorry but I failed to find devices acting like you describe. Your
> commit log mentions Micron MT35X and Macronix devices. I checked the
> datasheets:
> - Micron octal devices do not seem to use SR2 at all, they have a flag
>   register, but that's something else (like Winbond) and they have
>   similar support for it both in ODTR and in SSDR mode.
> - Macronix octal products feature several SR/CR. The sr1_and_sr2 opcode
>   exist but I would like to stop using it since there is no more need
>   for it and it just complexifies everything for no good reason. There
>   is no difference between the SSDR and the ODTR instruction sets, they
>   both support the same opcodes for reading and writing those registers
>   (with the subtlety that the repeated opcode should be a bitwise
>   complement).
> 
> Which device do you use exactly? Why is it not working? Are you sure it
> is not a problem with your controller or the SFDP tables instead?

Hi Miquel,

The device is a Micron MT35XU01GBBA (JEDEC 2c 5b 1b) on an i.MX95 EVK, and
a Macronix MX25UW51345G(JEDEC c2 84 3a) on an i.MX8ULP EVK.

In 8D mode RDID (9f) and RDSR1 (05) are read back fine. Only RDSR2 (0x35, 
extended to 0x3535 or 0x35ca) times out. So the controller and the 8D sampling
are OK — the flash simply does not answer a 0x35 in 8D mode.

Root cause is in the core, not the SFDP/controller: spi_nor_init_default_params()
sets opcodes.read_sr2 = RDCR (0x35) unconditionally, the Micron driver never touches
it, and spi_nor_cache_sr_lock_bits() then reads SR2 during init, after the switch
to 8D. As you noted, Micron octal parts have no SR2 (they use the flag register),
so 0x35 is simply not a valid command there. Same for Macronix, 0x35 is not valid
in 8D mode.

For spi-nor device which support 8D, it should contain the xSPI Profile 1.0 table,
And Profile 1.0 does not describe any SR2/RDCR read. Per your suggestion, I can
clear opcodes.read_sr2 in spi_nor_parse_profile1(). It runs for octal-capable
parts and before the manufacturer late_init, so a driver can still install its
own SR2 opcode afterwards. I test this method, it can works, but I have some concern: 
though flexspi(controller) and spi-nor device both support 8D mode, but user
still can force the mode to 1s-1s-1s mode by setting the spi-t(or r)x-bus-width=1
in dts. Like the Macronix on i.MX8ULP, in 1s-1s-1s mode, it dose issue 0x35, but
it has no HAS_SR2_CMP_BIT6 flag, so cmp_mask is 0 and the SR2 value is discarded
anyway — clearing read_sr2 causes no regression there. 
But if an 8D-capable part declared SPI_NOR_HAS_CMP in the kernel and were restricted
to (x)STR by the host, clearing read_sr2 by capability in Profile 1.0 parsing would
drop its usable SR2-CMP read in STR mode. This is my concern. So I switch to the
runtime mode, clear the SR2 only when mode actually in 8D mode.
I'm not sure whether such device really exist.

Which method do you prefer? If you prefer clear SR2 in spi_nor_parse_profile1(), I
can send V3 patch.


In the end of this reply, I list the SFDP of these two spi-nor devices. 

Micron:
root@imx95evk:/sys/bus/spi/devices/spi1.0/spi-nor# hexdump -C sfdp
00000000  53 46 44 50 0a 01 03 ff  00 08 01 17 30 00 00 ff  |SFDP........0...|
00000010  84 00 01 02 90 00 00 ff  05 01 01 06 a0 00 00 ff  |................|
00000020  0a 00 01 08 b0 00 00 ff  ff ff ff ff ff ff ff ff  |................|
00000030  e5 20 8a ff ff ff ff 3f  00 00 00 00 00 00 00 00  |. .....?........|
00000040  ee ff ff ff ff ff 00 00  ff ff 00 00 0c 20 11 d8  |............. ..|
00000050  0f 52 00 00 39 61 99 00  87 8e 03 d3 ac a1 27 3d  |.R..9a........'=|
00000060  7a 75 7a 75 fb bd d5 5c  00 00 70 ff 81 50 f8 a1  |zuzu...\..p..P..|
00000070  2f cb 27 8b 00 00 04 01  00 06 01 00 ff ff ff 8e  |/.'.............|
00000080  00 00 00 00 00 00 00 00  00 00 00 00 ff ff ff ff  |................|
00000090  43 0e ff ff 21 dc 5c ff  ff ff ff ff ff ff ff ff  |C...!.\.........|
000000a0  00 0b 80 9e b1 81 b5 85  00 f0 ff 9f 00 0a 00 00  |................|
000000b0  00 0a 1a 88 10 00 00 00  ff ff ff ff ff ff ff ff  |................|
000000c0  00 00 06 01 00 00 00 00  14 01 81 03 00 00 00 00  |................|
000000d0
root@imx95evk:/sys/bus/spi/devices/spi1.0/spi-nor# ls
jedec_id  manufacturer  sfdp
root@imx95evk:/sys/bus/spi/devices/spi1.0/spi-nor# cat jedec_id
2c5b1b
root@imx95evk:/sys/bus/spi/devices/spi1.0/spi-nor# cat manufacturer
micron
root@imx95evk:/sys/kernel/debug/spi-nor/spi1.0# cat params
name            (null)
id              2c 5b 1b 10 41 00
size            128 MiB
write size      1
page size       256
address nbytes  4
flags           4B_OPCODES | HAS_4BAIT | HAS_LOCK | IO_MODE_EN_VOLATILE | SOFT_RESET

opcodes
 read           0xfd
   dummy cycles  20
 erase          0x21
 program        0x12
 SR1 read       0x05
 SR2 read       0x35
 SR1 write      0x01
 SR1+SR2 write  0x01
 8D extension   repeat

protocols
 read           8D-8D-8D
 write          8D-8D-8D
 register       8D-8D-8D

erase commands
 21 (4.00 KiB) [1]
 5c (32.0 KiB) [2]
 dc (128 KiB) [3]
 c4 (128 MiB)

sector map
 region (in hex)   | erase mask | overlaid
 ------------------+------------+---------
 00000000-07ffffff |     [ 1  ] | no

locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-07ffffff | unlocked | 64

root@imx95evk:/sys/kernel/debug/spi-nor/spi1.0# cat capabilities
Supported read modes by the flash
 1S-1S-1S
  opcode        0x13
  mode cycles   0
  dummy cycles  0
 1S-1S-8S
  opcode        0x7c
  mode cycles   1
  dummy cycles  7
 1S-8S-8S
  opcode        0xcc
  mode cycles   1
  dummy cycles  15
 8D-8D-8D
  opcode        0xfd
  mode cycles   0
  dummy cycles  20

Supported page program modes by the flash
 1S-1S-1S
  opcode        0x12
 1S-1S-8S
  opcode        0x84
 1S-8S-8S
  opcode        0x8e
 8D-8D-8D
  opcode        0x12

Macronix:
root@imx8ulpevk:/sys/bus/spi/devices/spi2.0/spi-nor# ls
jedec_id  manufacturer  sfdp
root@imx8ulpevk:/sys/bus/spi/devices/spi2.0/spi-nor# cat jedec_id
c2
root@imx8ulpevk:/sys/bus/spi/devices/spi2.0/spi-nor# cat manufacturer
macronix
root@imx8ulpevk:/sys/bus/spi/devices/spi2.0/spi-nor# hexdump -C sfdp
00000000  53 46 44 50 08 01 04 fd  00 07 01 14 40 00 00 ff  |SFDP........@...|
00000010  87 01 01 1c 90 00 00 ff  0a 00 01 08 00 01 00 ff  |................|
00000020  05 00 01 05 20 01 00 ff  84 00 01 02 34 01 00 ff  |.... .......4...|
00000030  00 00 00 00 00 00 00 00  ff ff ff ff ff ff ff ff  |................|
00000040  e5 20 8a ff ff ff ff 1f  00 ff 00 ff 00 ff 00 ff  |. ..............|
00000050  ee ff ff ff ff ff 00 ff  ff ff 00 ff 0c 20 10 d8  |............. ..|
00000060  00 ff 00 ff 87 79 01 00  84 12 00 e2 cc 04 67 46  |.....y........gF|
00000070  30 b0 30 b0 f4 bd d5 5c  00 00 00 ff 10 10 00 20  |0.0....\....... |
00000080  00 00 00 00 00 00 7c 23  48 00 00 00 00 00 88 88  |......|#H.......|
00000090  00 00 00 00 00 00 00 40  0f d1 ff f3 0f d1 ff f3  |.......@........|
000000a0  00 05 00 90 00 05 00 b1  00 2b 00 95 00 2b 00 96  |.........+...+..|
000000b0  72 71 03 b8 72 71 03 b8  00 00 00 00 90 a3 18 82  |rq..rq..........|
000000c0  00 c0 69 96 00 00 00 00  00 00 00 00 72 71 00 98  |..i.........rq..|
000000d0  72 71 00 b8 72 71 00 99  00 00 00 00 72 71 00 98  |rq..rq......rq..|
000000e0  72 71 00 f8 72 71 00 99  72 71 00 f9 00 00 00 00  |rq..rq..rq......|
000000f0  00 00 00 00 01 15 01 d0  72 71 06 d8 00 00 86 50  |........rq.....P|
00000100  00 00 06 01 00 00 00 00  02 00 01 03 00 02 00 00  |................|
00000110  00 00 06 01 00 00 00 00  00 00 72 06 00 02 00 00  |..........r.....|
00000120  00 ee c0 69 72 72 71 71  00 d8 f7 f6 00 0a 00 00  |...irrqq........|
00000130  14 45 98 80 43 06 0f 00  21 dc ff ff              |.E..C...!...|
0000013c

root@imx8ulpevk:/sys/kernel/debug/spi-nor/spi2.0# cat params
name            (null)
id              c2 84 3a c2 84 3a
size            64.0 MiB
write size      1
page size       256
address nbytes  4
flags           4B_OPCODES | HAS_4BAIT | IO_MODE_EN_VOLATILE | SOFT_RESET

opcodes
 read           0xee
  dummy cycles  20
 erase          0x21
 program        0x12
 SR1 read       0x05
 SR2 read       0x35
 SR1 write      0x01
 SR1+SR2 write  0x01
 8D extension   invert

protocols
 read           8D-8D-8D
 write          8D-8D-8D
 register       8D-8D-8D

erase commands
 21 (4.00 KiB) [2]
 dc (64.0 KiB) [3]
 c7 (64.0 MiB)

sector map
 region (in hex)   | erase mask | overlaid
 ------------------+------------+---------
 00000000-03ffffff |     [  2 ] | no

root@imx8ulpevk:/sys/kernel/debug/spi-nor/spi2.0# cat capabilities
Supported read modes by the flash
 1S-1S-1S
  opcode        0x13
  mode cycles   0
  dummy cycles  0
 8D-8D-8D
  opcode        0xee
  mode cycles   0
  dummy cycles  20

Supported page program modes by the flash
 1S-1S-1S
  opcode        0x12
 8D-8D-8D
  opcode        0x12

Regards
Haibo Chen
> 
> Thanks,
> Miquèl