On Mon, Mar 4, 2013 at 7:00 AM, Николай Николаев <evrinoma@gmail.com> 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.

8)
--
With best regards Nikolay Nikolaev
С Уважением Николаев Николай

_______________________________________________
flashrom mailing list
flashrom@flashrom.org
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.