Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

Hi, Pratyush, Boris,

On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> This series adds support for octal DTR flashes in the spi-nor framework,

I'm still learning about this, but I can give you my 2 cents as of now, to 
open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous 
because the flash may not recover from unexpected resets. Entering one of 
these modes can be:
1/ volatile selectable, the device return to the 1-1-1 protocol after the next 
power-on. I guess this is conditioned by the optional RESET pin, but I'll have 
to check. Also the flash can return to the 1-1-1 mode using the software reset 
or through writing to its Configuration Register, without power-on or power-
off.
2/ non-volatile selectable in which RESET# and software reset are useless, the 
flash defaults to the mode selected in the non volatile Configuration Register 
bits. The only way to get back to 1-1-1 is to write to the Configuration 
Register.

Not recovering from unexpected resets is unacceptable. One should always 
prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the 
presence of the optional RESET pin.

For the unfortunate flashes that support just option 2/, we should not enter 
these modes on our own, just by discovering the capabilities from the SFDP 
tables or by the flags in the flash_info struct. The best we can do for them 
is to move the responsibility to the user. Maybe to add a Kconfig option that 
is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or 
8-8-8 modes. Once entered in one of these modes, if an unexpected reset comes, 
you most likely are doomed, because early stage bootloaders may not work in 
these modes and you'll not be able to boot the board. Assuming that one uses 
other environment to boot the board, we should at least make sure that the 
flash works in linux after an unexpected reset. We should try to determine in 
which mode we are at init, so maybe an extension of the default_init hook is 
needed. But all this looks like a BIG compromise, I'm not yet sure if we 
should adress 2/. Thoughts?

I'm still looking into this.

Cheers,
ta