Re: [PATCH v3 09/17] mtd: spi-nor: Allow configuring the actual number of dummy cycles

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: missing hardware state rollback on probe failure
--

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

mtd: spi-nor: Allow configuring the actual number of dummy cycles

The SPI NOR core selects the fastest read variant advertised by the SFDP
tables. This may or may not be a wise choice but at least it is simpler
as any bus frequency will just work. There is however a downside: the
number of dummy cycles is maximized (in case the bus frequency is really
fast, this is how the chip enforces a sufficient delay). Maximizing the
number of dummy cycles means that, if the amount of cycles is
configurable in the chip, we might not take its default value. When this
happens, we need to make sure that the chip is configured accordingly.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index a91e47eae2789..359df29c4ca6f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c

[ ... ]

> @@ -3100,6 +3118,10 @@ static int spi_nor_init(struct spi_nor *nor)
>  	struct spi_nor_flash_parameter *params = nor->params;
>  	int err;
>  
> +	err = spi_nor_set_dummy(nor);
> +	if (err)
> +		return err;
> +
>  	err = spi_nor_set_octal_dtr(nor, true);
>  	if (err) {
>  		dev_dbg(nor->dev, "octal mode not supported\n");
>  		return err;
>  	}

[Severity: High]
If spi_nor_set_octal_dtr() returns an error, does this leave the flash chip
in a non-default hardware state?

It appears that if entering octal mode fails, the function returns immediately
without undoing the just-applied spi_nor_set_dummy() configuration.

Similarly, if a memory allocation fails later during driver probe after
spi_nor_scan() has successfully mutated the flash state:

drivers/mtd/spi-nor/core.c:spi_nor_probe() {
    ...
	err = spi_nor_scan(nor, name, &hwcaps);
	if (err)
		return err;

	if (nor->bouncebuf_size) {
		nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
					      GFP_KERNEL);
		if (!nor->bouncebuf)
			return -ENOMEM;
    ...
}

Does the driver return here without calling spi_nor_restore() to undo the
dummy cycles configuration?

If the flash chip is left expecting the newly configured dummy cycles,
reads using default SPI parameters (e.g., standard dummy cycles) during a warm
reboot might return garbage, potentially hanging the boot process.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-winbond-master-spi-nor-w35t-v3-0-6dde1d513df8@bootlin.com?part=9

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