So, I think Stefan is going to handle the review of the S25FL128S / 256S Spansion chips since they're a bit unusual.
I'm going to cover the flashchips.h part of the patch, and some issues with chip names/IDs that came up in review of the flashchips.h part.
In general, chip names should contain only important characters- unimportant characters should be marked with a . or an _ depending on the field in flashchips.c and with an _ for names in flashchips.h. See
http://www.flashrom.org/pipermail/flashrom/2012-July/009599.html for the reasoning here, and N25Q064..1E for an example.
The Spansion chips you've added seem to have an extra letter at the end that's not important, so I would remove it.
From flashchips.h:
#define ST_N25Q032__1E 0xBB16 /* N25Q032, 1.8V, (uniform sectors expected) */
#define ST_N25Q064__3E 0xBA17 /* N25Q064, 3.0V, (uniform sectors expected) */
#define ST_N25Q064__1E 0xBB17 /* N25Q064, 1.8V, (uniform sectors expected) */
Here's examples of chips with names that have been shortened due to unimportant characters. The underscore ( _ ) is used to replace the unimportant characters.
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.
Flashrom can only handle uniform sectors right now.
The N25Q128A13 is a great example of this: the Numonyx datasheet from Feb 2011 mentions it has uniform sectors, but the May 2011 sheet says it has top or bottom sectors
+#define ST_N25Q128A13 0xBA18 /* N25Q128, 3.0V */
should be:
+#define ST_N25Q128__3E 0xBA18 /* N25Q128, 3.0V, (uniform sectors expected) */
+#define ST_N25Q128A11 0xBB18 /* N25Q128, 1.8V */
should be:
+#define ST_N25Q128__1E 0xBB18 /* N25Q128, 1.8V, (uniform sectors expected) */
+#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 */
Please adjust the .name and .model_id fields in flashchips.c so the names are properly shortened as done above:
.name uses periods in place of characters
.model_id uses underscores ( _ ) in place of characters
Please see N25Q064..3E for an example of this in flashchips.c
I realize this may be a lot to take in, so ask questions, and if I can't answer it, hopefully someone else on the list can.