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

[Miquel Raynal]

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

There is no "default" hardware state regarding dummy cycles. This is a
state that must be negociated at every "boot" (when relevant), warm or
not, so there is no breakage here since next warm boot the same dance
will happen again from scratch. In case octal is not reachable we
already recalculate the dummy cycles for the SSDR case when relevant.

So: won't fix either.

Miquèl