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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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?

Thanks,
Miquèl