On Mon, 3 Jun 2013 22:38:01 -0400 Steven Zakulec spzakulec@gmail.com wrote:
So, I think Stefan is going to handle the review of the S25FL128S / 256S Spansion chips since they're a bit unusual.
I'll handle those and maybe some other nitpicks when you two are done, yes.
The E and the uniform sector comment. This is because there are certain chips that once had (or still do) have hybrid sector setups- the top block of the chip could have one value, and the bottom block another.
There are 3 possible erase sector layouts in this chip family. 'E' has uniform sectors (meaning that every erase address range done by any one opcode is equal in size) and then there are two other variants that split the top or bottom block into a number of subdivisions. As explained on IRC these were invented and useful earlier in computer history for bootblocks. But since flash chips got bigger and usually include quite fine-grained erase opcodes today, I don't think many of those variants of this family are in the wild, but that might be overly naive and optimistic. :)
Flashrom can only handle uniform sectors right now.
That is incorrect. Flashrom *does* support them (e.g. Am29F002(N)BT). The problem is that with the current probing methods supported flashrom can not distinguish between those variants of the Micron chips.
See Numonxy datasheet of the N25Q128 with "subsectors" (Rev 7, May 2011), section 9.1.1 Read Identification (RDID) explains how to identify the (non-)uniform layouts (respectively) with the bits in the first EDID byte (the second one seems to be not specified btw). The problem is: currently flashrom does only read the first 3 bytes (which is everything there was before they (Atmel has the patent afaik (yes this seems to be really patentable in countries with ridiculous patent rules)) to extend RDID like that).
This will be solved by my in my GSoC project this summer. Until then I think it is ok to add them and expect them to be uniform. If there are really non-uniform variants out there the will be erase errors, but using another erase operation (which is done automatically) will work fine. But it is certainly a good idea to document that, hence the comments in flashchips.h.
+#define ST_N25Q256A13 0xBA19 /* N25Q256, 3.0V */ should be: +#define ST_N25Q256__3 0xBA19 /* N25Q256, 3.0V */
+#define ST_N25Q256A11 0xBB19 /* N25Q256, 1.8V */ should be: +#define ST_N25Q256__1 0xBB19 /* N25Q256, 1.8V */
+#define ST_N25Q512A13 0xBA20 /* N25Q512, 3.0V */ should be: +#define ST_N25Q512__3 0xBA20 /* N25Q512, 3.0V */
+#define ST_N25Q512A11 0xBB20 /* N25Q512, 1.8V */ should be: +#define ST_N25Q512__1 0xBB20 /* N25Q512, 1.8V */
+#define ST_N25Q00AA13 0xBA21 /* N25Q00A, 3.0V */ should be: +#define ST_N25Q00A__3 0xBA21 /* N25Q00A, 3.0V */
and please mention non-uniformness if you spot it in one of those datasheets (I have not checked that, but I guess Steven did so already).
I will handle any further iterations of this patch set (so please be sure to mail it to the mailing list, Nikolay).