On Mon, 5 May 2014 14:17:05 -0700 Wei Hu wei@arista.com wrote:
On Sun, May 4, 2014 at 1:06 PM, Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
On Mon, 31 Mar 2014 22:18:48 -0700 Wei Hu wei@aristanetworks.com wrote:
Datasheet available at http://ww1.microchip.com/downloads/en/DeviceDoc/20005119D.pdf
Hi,
thank you for your patch! I have reviewed it and I think you got a few things wrong:
- Most importantly, I think the block erase sizes are non-uniform (for opcode 0xD8). See note 11 of Table 5.1 and Section 3.0 etc.
Thanks for fixing the non-uniform layout! Learned something new today.
This became rather uncommon with SPI chips but still there are even new chips with this feature. It was very common in older flash chips. We currently support over 100 chips with such a layout (look for ".eraseblocks = {\n" in flashchips.c).
- Also, the ULBPR requires a WREN opcode sent before it. OTOH your spi_disable_blockprotect_generic() calls seems useless (all bits of the status register are read-only).
I know spi_disable_blockprotect_generic() is useless here, but I thought it was a good idea to call the generic function first in general? As a side effect of calling this function I'm sending WREN, so I didn't send on explicitly.
There is no "generic" function that always should be called. The spi_disable_blockprotect_generic() function is just generic in the sense that most unprotect schemes can be implemented with it by supplying the respective parameters. Also, the WREN command is only active for one matching operation, and in this case this is the WRSR command that immediately follows it within spi_write_status_register_flag(). So your version would not do what you expect.
- The spi_prettyprint_status_register_sst25 function is completely wrong for these chips.
- The OTP memory, and other advanced features were not documented at all.
What's OTP? I didn't see it in the datasheet. Why do you want that?
One-time-programmable memory, it is noted on the front page of the datahsset and is explained in more detail in 5.27 "Program Security ID". If the flag FEATURE_OTP is set for a chip, we produce the following message: "This chip may contain one-time programmable memory. flashrom cannot read and may never be able to write it, hence it may not be able to completely clone the contents of this chip (see man page for details)."
Most users seem to not understand that, but I think nevertheless that it is useful for those that do :)