Re: [PATCH v2 07/17] mtd: spi-nor: Fix WRSR with ODTR chips

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mtd: spi-nor: rejecting len != 1 in Octal DTR mode breaks 2-byte SR writes
- [High] mtd: spi-nor: hardware WEL state leak on early return in spi_nor_write_sr_ll()
--

commit 101ce767bdad155741f87e7d7cf56f7779db6402
Author: Miquel Raynal <miquel.raynal@bootlin.com>

mtd: spi-nor: Fix WRSR with ODTR chips

The ODTR interface requires an even number of bytes in each section of
the operation. spi_nor_spimem_setup_op() takes care of the opcode phase,
but we have to adapt the rest of the operation ourself.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cf23094021daa..381bbc13f82ff 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -791,6 +791,17 @@ static int spi_nor_write_sr_ll(struct spi_nor *nor, u8 opcode, const u8 *sr,
>  	if (nor->spimem) {
>  		struct spi_mem_op op = SPI_NOR_WRSR_OP(opcode,
>  						       nor->bouncebuf, len);
> +		if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
> +			if (len != 1)
> +				return -EOPNOTSUPP;

[Severity: High]
Does this erroneously reject legitimate 2-byte register writes required for
DTR-enabled chips?

Looking at spi_nor_write_sr1_and_sr2(), it passes len = 2 to
spi_nor_write_sr_ll(), which is now broken by this check.

This propagates -EOPNOTSUPP out to userspace via spi_nor_sr_unlock(), resulting
in a visible regression where Software Write Protection unlocking and flash
configuration fails for 8-8-8 DTR chips.

[Severity: High]
Does returning early here leave the Write Enable Latch (WEL) persistently active
on the flash device?

Since spi_nor_write_sr_ll() calls spi_nor_write_enable(nor) earlier in the
function, this early return happens without executing the SPI command that would
naturally clear it. Leaving the hardware write-enabled risks accidental data
corruption from stray writes.

> +
> +			/*
> +			 * We don't want to write only one byte in DTR mode. So,
> +			 * duplicate the buffer and write 2.
> +			 */
> +			op.data.nbytes = 2;
> +			nor->bouncebuf[1] = nor->bouncebuf[0];
> +		}
>  
>  		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-winbond-master-spi-nor-w35t-v2-0-592f3300a70f@bootlin.com?part=7

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/