On Mon, 3 Jun 2013 22:38:01 -0400
Steven Zakulec <spzakulec(a)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).
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner