<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 4, 2013 at 7:00 AM, Николай Николаев <span dir="ltr"><<a href="mailto:evrinoma@gmail.com" target="_blank">evrinoma@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br><div>Hi<br></div><div><div><br></div><div class="gmail_quote"><div><div>"Some additions of chips are non-functional yet... e.g. there is no<br>


support for more than 24 bits in addresses, but some chips require that.<br> Also, please do not send patches as base64 encoded attachments."<br></div></div><div> </div></div></div><div>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.</div>

<div>
<div><br></div><div>"I took a closer look at your patch. It contains a lot of white space<br>
errors. Please make sure that this won't happen again. Checking data<br>
sheets to review such patches is no fun, dealing with other's<br> white space errors is even worse ;)"</div><div><br></div></div><div>sorry i now rechecked my patches by linux <a href="http://checkpatch.pl" target="_blank">checkpatch.pl</a> script.</div>


<div><br></div><div>8)<br></div><div>-- <br><div>With best regards Nikolay Nikolaev</div></div><div>С Уважением Николаев Николай</div>
<br>_______________________________________________<br>
flashrom mailing list<br>
<a href="mailto:flashrom@flashrom.org" target="_blank">flashrom@flashrom.org</a><br>
<a href="http://www.flashrom.org/mailman/listinfo/flashrom" target="_blank">http://www.flashrom.org/mailman/listinfo/flashrom</a><br></blockquote><div>So, I've started reviewing some of this patch against the datasheets.<br>

</div><div>General questions to flashrom developers/things I haven't reviewed/non-specific comments:<br>.manufacture_id, model_id: Not sure how to check these.<br><br></div><div>.page_size: I assume they are all 256 as per point 6 here: <a href="http://flashrom.org/index.php?title=Development_Guidelines" target="_blank">http://flashrom.org/index.php?title=Development_Guidelines</a><br>

</div><div>.probe, .probe_timing: Make sure it has probe_spi_rdid & TIMING_ZERO if SPI chip and probe opcode 0x9F.<br></div><div>.printlock, .unlock, .write, .read : I'm not sure how to review any of these items.<br>

</div><div><br></div><div>Comments on specific chips:<br></div><div><br>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?<br>

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

</div><div><br></div><div>Eon EN25S16:<br>+                .eraseblocks = { {32 * 1024, 64} },<br>+                .block_erase = spi_block_erase_52,<br><br></div><div>It appears both the .eraseblocks and the .block_erase are wrong:<br>

</div><div>.eraseblocks should be  { {64  * 1024, 32} } - there are 32 blocks of 64 KBytes, not 64 blocks of 32 KBytes.<br></div><div>.block_erase uses spi_block_erase_52 when it should use spi_block_erase_d8.<br><br></div>

Eon EN25S32:<br>+                .eraseblocks = { {32 * 1024, 128} },<br>+                .block_erase = spi_block_erase_52,<br><br></div><div class="gmail_quote">I don't see where you are getting that .eraseblocks from- I can't find it in the datasheet.<br>

</div><div class="gmail_quote">I also don't know where the .block_erase came from- there is no 0x52 opcode in the datasheet.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">I'll send follow-up emails later for the next group of chips.<br>
</div><div class="gmail_quote">
<div><br></div><div> <br></div></div><br></div></div>