Re: [PATCH] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unintended Register Corruption for Flashes with `qe_mask[0]` and an Active SR2.

Pre-existing issues:
- [High] spi_nor_write_sr1_and_sr2_and_check() still fails unconditionally for flashes without SR2 if the QE bit needs to be set.
--

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

mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

The patch modifies spi_nor_generic_quad_enable to only read SR1 when
qe_mask[0] is set. This aims to fix probe failures on flashes lacking
an SR2.

Closes: https://lore.kernel.org/linux-mtd/178876719232.3543902.14451625037676421254.b4-ty@b4/T/#m5bc4ba6776436f2870ced0eb5789d229037ad840

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index e2b6efafdd8d0..ac909dfb9657c 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -985,7 +985,10 @@ static int spi_nor_generic_quad_enable(struct spi_nor *nor)
>  	if (!qe_mask[0] && !qe_mask[1])
>  		return 0;
>  
> -	ret = spi_nor_read_sr1_and_sr2(nor, sr);
> +	if (qe_mask[0])
> +		ret = spi_nor_read_sr1(nor, sr);

[Severity: High]
If qe_mask[0] is set and the flash has an active SR2, does this leave sr[1]
unpopulated as 0x00 without ensuring the flash actually lacks a two-byte
write opcode?

Later in this function, if the QE bit is not already set, it calls:

    return spi_nor_write_sr1_and_sr2_and_check(nor, sr);

This will invoke the helper which executes the two-byte write_sr1_and_sr2
opcode if set, writing the unpopulated sr[1] (0x00) to the flash's SR2/CR.
Can this corrupt the flash state and fail the validation check if the second
byte reads back as non-zero?

> +	else
> +		ret = spi_nor_read_sr1_and_sr2(nor, sr);
>  	if (ret)
>  		return ret;
>  

[Severity: High]
This is a pre-existing issue, but does this still fail unconditionally for
flashes without SR2 if the QE bit needs to be set?

If the QE bit is not already set, spi_nor_generic_quad_enable() proceeds to
call spi_nor_write_sr1_and_sr2_and_check(), which reads both registers for
verification:

drivers/mtd/spi-nor/core.c:spi_nor_write_sr1_and_sr2_and_check() {
	...
	ret = spi_nor_read_sr1_and_sr2(nor, tmp);
	if (ret)
		return ret;
	...
}

Because read_sr2 is 0 for flashes lacking SR2, spi_nor_read_sr2() returns
-EINVAL. Doesn't this mean the generic helper still fails with -EINVAL during
verification?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-perso-fix-spi-nor-qe-mxic-v1-1-fd6d91416a2a@bootlin.com?part=1