[PATCH v2] mtd: spi-nor: clear the default RDCR opcode when entering Octal DTR
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Mon Sep 28 23:59:24 PDT 2026
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 at 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
More information about the linux-mtd
mailing list