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

[sashiko-bot@kernel.org]

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@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?

> +
>  out:
>  	kfree(dwords);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-spi-nor-fix-v3-1-bffa00b75c27@nxp.com?part=1