[PATCH v3] mtd: spi-nor: clear the default read_sr2 opcode for xSPI Profile 1.0 flashes

[Bough Chen (OSS)]

> -----Original Message-----
> From: sashiko-bot at kernel.org <sashiko-bot at kernel.org>
> Sent: Thursday, October 22, 2026 4:21 PM
> To: Bough Chen (OSS) <haibo.chen at oss.nxp.com>; Pratyush Yadav
> <pratyush at kernel.org>; Michael Walle <mwalle at kernel.org>; Takahiro
> Kuwano <takahiro.kuwano at infineon.com>; Miquel Raynal
> <miquel.raynal at bootlin.com>; Richard Weinberger <richard at nod.at>;
> Vignesh Raghavendra <vigneshr at ti.com>
> Cc: linux-mtd at lists.infradead.org; imx at lists.linux.dev; michael at walle.cc;
> Bough Chen <haibo.chen at nxp.com>; Frank.Li at kernel.org
> Subject: Re: [PATCH v3] mtd: spi-nor: clear the default read_sr2 opcode for
> xSPI Profile 1.0 flashes
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] mtd: spi-nor: read_sr2 left set for 8D-8D-8D flashes without
> Profile 1.0
> --
> 
> commit ef4079e3aea2014890e08df31ea4d8775a392f81
> Author: Haibo Chen <haibo.chen at nxp.com>
> 
> mtd: spi-nor: clear the default read_sr2 opcode for xSPI Profile 1.0 flashes
> 
> This commit clears params->opcodes.read_sr2 in spi_nor_parse_profile1() so
> that flashes with an xspi profile 1.0 table do not issue an unsupported
> 0x35 register read when running in octal dtr mode.
> 
> > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> > index 4a3f82e97cef8..722eeff71d0df 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;
> 
> [Severity: Medium]
> What happens on octal dtr flashes that do not publish an xspi profile 1.0
> table, so spi_nor_parse_profile1() is never called?
> 
> Specifically, some batches of the Micron mt35xu512aba lack the profile 1.0
> table and instead configure SNOR_PROTO_8_8_8_DTR in
> drivers/mtd/spi-nor/micron-st.c:mt35xu512aba_post_sfdp_fixup():
> 
> 	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
> 	spi_nor_set_read_settings(&nor->params-
> >reads[SNOR_CMD_READ_8_8_8_DTR],
> 				  0, 20, SPINOR_OP_MT_DTR_RD,
> 				  SNOR_PROTO_8_8_8_DTR);
> 	...
> 	nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
> 	spi_nor_set_pp_settings(&nor->params-
> >page_programs[SNOR_CMD_PP_8_8_8_DTR],
> 				SPINOR_OP_PP_4B,
> SNOR_PROTO_8_8_8_DTR);
> 
> In drivers/mtd/spi-nor/sfdp.c:spi_nor_parse_bfpt(), params-
> >opcodes.read_sr2 is initialized to SPINOR_OP_RDCR (0x35) and remains set
> when the quad enable requirements field contains a reserved value (which
> mt35xu512aba_post_sfdp_fixup() notes is the case on this part) or
> BFPT_DWORD15_QER_NONE:
> 
> 	params->opcodes.read_sr1 = SPINOR_OP_RDSR;
> 	params->opcodes.read_sr2 = SPINOR_OP_RDCR;
> 	...
> 	default:
> 		dev_dbg(nor->dev, "BFPT QER reserved value used\n");
> 		break;
> 
> Similarly, non-sfdp flashes initialized with SPI_NOR_OCTAL_DTR_READ in
> drivers/mtd/spi-nor/core.c:spi_nor_no_sfdp_init_params() enable
> SNOR_HWCAPS_READ_8_8_8_DTR while leaving params->opcodes.read_sr2
> set to SPINOR_OP_RDCR from spi_nor_init_default_params().
> 
> When spi_nor_init() switches these flashes to SNOR_PROTO_8_8_8_DTR via
> spi_nor_set_octal_dtr() and then runs:
> 
>   spi_nor_init()
>     -> spi_nor_cache_sr_lock_bits()
>       -> spi_nor_read_sr1_and_sr2_careful()
>         -> spi_nor_read_sr2_careful()
>           -> spi_nor_read_sr_ll()
> 
> wouldn't this still issue the unsupported 0x3535 register read in
> SNOR_PROTO_8_8_8_DTR mode and fail probe or resume with a controller
> timeout?

Hi Miquel,

The Sashiko AI review raised a valid point on v3.

I see three ways to go, which do you prefer?

1, Keep the v3 approach (clear in spi_nor_parse_profile1()) and fix the remaining
    cases one by one, clearing read_sr2 in each relevant fixup 
    (e.g. mt35xu512aba_post_sfdp_fixup) / no-SFDP path.

2, Move the clear into spi_nor_late_init_params(), gated on the 8D read capability
    (hwcaps & SNOR_HWCAPS_READ_8_8_8_DTR) and read_sr2 == RDCR, so all 8D 
    paths are covered in one place. 
    The downside: late_init runs after the manufacturer fixups, so if a vendor later
    wants to set its own SR2 opcode for 8D it would be clobbered here and overriding
     it cleanly becomes awkward.

3, Go back to the v2 approach (clear in spi_nor_set_octal_dtr() when the switch to 8D
    actually succeeds). This covers every path that ends up in 8D regardless of how the 
    capability was discovered, but it is runtime-protocol based rather than capability 
    based, which you didn't prefer earlier. And this method has the same downside as
    method 2.

Regards
Haibo Chen
> 
> > +
> >  out:
> >  	kfree(dwords);
> >  	return ret;
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261022-spi-nor-fix-v3-1-
> bffa00b75c27 at nxp.com?part=1