Re: [flashrom] Added chipsets

[Steven Zakulec]

On Mon, Mar 4, 2013 at 7:00 AM, Николай Николаев  wrote:
> Hi
>
> "Some additions of chips are non-functional yet... e.g. there is no
> support for more than 24 bits in addresses, but some chips require that.
>  Also, please do not send patches as base64 encoded attachments."
>
> All these chips has a 24 addressing limit, or three bytes. To addressing
> above 16Mbyte we must program special register - extend addressing space -
> it's look like a fetcher bit.
>
> "I took a closer look at your patch. It contains a lot of white space
> errors. Please make sure that this won't happen again. Checking data
> sheets to review such patches is no fun, dealing with other's
>  white space errors is even worse ;)"
>
> sorry i now rechecked my patches by linux checkpatch.pl script.
>
> flashrom mailing list
> [e-mail]
> http://www.flashrom.org/mailman/listinfo/flashrom

So, I've started reviewing some of this patch against the datasheets.
General questions to flashrom developers/things I haven't
reviewed/non-specific comments:
.manufacture_id, model_id: Not sure how to check these.

.page_size: I assume they are all 256 as per point 6 here:
http://flashrom.org/index.php?title=Development_Guidelines
.probe, .probe_timing: Make sure it has probe_spi_rdid & TIMING_ZERO if SPI
chip and probe opcode 0x9F.
.printlock, .unlock, .write, .read : I'm not sure how to review any of
these items.

Comments on specific chips:

AMIC A25LQ016: I couldn't find a datasheet for this chip available- I've
got a A25LQ032 datasheet, but not one for the A25LQ016. Could you point me
to that sheet, or provide me with a copy?

Eon EN25S10:
+                .eraseblocks = { {32 * 1024, 4} },
+                .block_erase = spi_block_erase_d8,
The .block_erase appears to be wrong here- it should be spi_block_erase_52
according to the datasheet.

Eon EN25S16:
+                .eraseblocks = { {32 * 1024, 64} },
+                .block_erase = spi_block_erase_52,

It appears both the .eraseblocks and the .block_erase are wrong:
.eraseblocks should be  { {64  * 1024, 32} } - there are 32 blocks of 64
KBytes, not 64 blocks of 32 KBytes.
.block_erase uses spi_block_erase_52 when it should use spi_block_erase_d8.

Eon EN25S32:
+                .eraseblocks = { {32 * 1024, 128} },
+                .block_erase = spi_block_erase_52,

I don't see where you are getting that .eraseblocks from- I can't find it
in the datasheet.
I also don't know where the .block_erase came from- there is no 0x52 opcode
in the datasheet.

I'll send follow-up emails later for the next group of chips.