On 9/16/20 3:44 PM, Pratyush Yadav wrote: > Hi, Hello,
> > This series adds support for Octal DTR flashes in the SPI NOR framework, > and then adds hooks for the Cypress Semper and Micron Xcella flashes to > allow running them in Octal DTR mode. This series assumes that the flash > is handed to the kernel in Legacy SPI mode. > I like this series. There are some comments that can be addressed, no big deal though. I think that we shouldn't let the door open for users with flashes that enter in a X-X-X mode in a non-volatile way. Think of two flashes that have the same X-X-X mode enable sequence, but in which only the EN bit differs: for one the EN bit is volatile and for the other it is non-volatile. Users of the later flash that try to enable the X-X-X mode (using our code) will end up with the flash in a mode from which they can't recover. Thus my advice is to consider by default all the flashes, as X-X-X mode non-volatile flashes, and to not let them use the X-X-X mode enable methods. Flashes that can enter X-X-X modes in a volatile way, should discover this capability by parsing the optional SFDP SCCR Map. Those that don't define this table, should pass this capability as a flash_info flag when declaring the flash. With these, users should be conscious about the V or NV modes, and the risk to end up with flashes for which there is no software to recover is diminished. This is what I tried in RFC 1/3 and RFC 3/3. I think those 2 patches should be part of this series. 14/15 and 15/15 should be updated accordingly. RFC 2/3 has room for discussions because it provides access system-wise, while ideally would be to do it at flash granularity. I'll wait for your feedback on those. > Tested on TI J721E EVM with 1-bit ECC on the Cypress flash. As a tip, when introducing some big changes to the core, would be nice to be assured that things that worked previously are still working now. An erase-write-read-back test in Quad SPI would suffice. Probably you have already did that, but I haven't seen it mentioned. Cheers, ta
