<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, May 27, 2013 at 5:25 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">Hi steven,<br><br><div class="gmail_quote"><div class="im">2013/5/27 Steven Zakulec <span dir="ltr"><<a href="mailto:spzakulec@gmail.com" target="_blank">spzakulec@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"><div><div><div><div><div><div><div><div><div><div>Before I get started on the review, just a general comment or two.<br></div><div>If you plan to contribute in the future (which I certainly hope you will), please break submissions down into blocks of 5-10 chips.  This makes it so much easier for me, and any other reviewers, to quickly check the patch and provide feedback in a timely manner.<br>

</div></div></div></div></div></div></div></div></div></div></div></blockquote></div><div>Sorry, this is my first patch. My next patch i wrote with you wish. (<a href="http://patchwork.coreboot.org/patch/3896/" target="_blank">http://patchwork.coreboot.org/patch/3896/</a>) 8)</div>
<div class="im">
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div><div><div><div>In the last review, I mentioned the Numonyx M45PE series (10, 20, 40, 80, 16), should only have <br>

+        .feature_bits    = FEATURE_WRSR_WREN, </div>
<div>set when the command is there.  It appears I may have been unclear there, because instead of removing just the WREN flag, you removed the whole line (the chips do have a Write Status Register- WRSR).<br></div></div>

</div></div></div></div></div></div></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div>
<div><div><div><div>
<div><div>

<br>Continuing the review from Numonyx M45PE16.<br></div></div></div></div></div></div></div></div></div></div></div></blockquote></div><div>Sorry i don't know which flag must be used. You right these chips have a only WREN instruction, and i think if these chips doesn't have a feature that i exclude feature_bits. Maybe we will be able to specify the details.</div>
</div></blockquote><div>Other people can provide more details on the general case, but in this instance, with only a WREN instruction, the FEATURE line should be:<br> +        .feature_bits    = FEATURE_WRSR</div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote"><div class="im">
<div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div><div><div><div><div><br></div>Numonyx M25PX80-<br>
+        .voltage    = {2700, 3600},<br>
</div>The datasheets I've seen have it as 2.3-3.6V (rev 4 Dec 2008 and Rev B 3/2013)<br>
<br></div><div>+        .tested        = TEST_OK_PREW,<br></div>Unless
 I'm reading it wrong, it looks like you're adding this chip as a fully 
tested one- would you mind supplying some logs showing that it 
successfully does these operations? (write logs shouldn't contain all :S
 (for skipped blocks) next to the addresses).<br></div></div></div></div></div></div></div></div></blockquote></div><div>You right this is my typos, and i fixed them.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div dir="ltr"><div><div><div><div><div><div>Numonyx N25Q00AA13-<br></div>I'll admit I don't really know how this chip works, but it seems there should be a 3rd eraseblock from the description of the memory organization (datasheet is Rev E, 2/12):<br>



<br>Memory Configuration and Block Diagram<br>The memory is a stacked device comprised of four 256Mb chips. Each chip is internally<br>partitioned into two 128Mb segments. Each page of memory can be individually pro-<br>


grammed. Bits are programmed from one through zero. The device is subsector, sector,<br>
or single 256Mb chip erasable, but not page-erasable. Bits are erased from zero through<br>one. The memory is configured as 134,217,728 bytes (8 bits each); 2048 sectors (64KB<br>each); 32,768 subsectors (4KB each); and 524,288 pages (256 bytes each); and 64 OTP<br>



bytes are located outside the main memory array. <br><br></div>You've got:<br>+                .eraseblocks = { {4 * 1024, 32768 } },<br>+               .block_erase = spi_block_erase_20,<br>+            }, {<br>+                .eraseblocks = { {64 * 1024, 2048 } },<br>



+               .block_erase = spi_block_erase_d8,<br><br></div>There's a die erase opcode, which I am unfamiliar with- maybe it handles erasing a single 256Mb chip at a time?<br></div></div></div></div></blockquote>

<div> </div></div><div>You right this chip used a four 256 Mb chips. And 256 Mb chip has a three instruction subsector 20(hex) sector D8(hex) bulk C7(hex) erase commands. But chip N25Q00AA13 has a other op-code to initiate die erase command C4 (hex)</div>
<div class="im">
<div><br></div></div></div></blockquote><div>So, how do you do a single-chip erase for this chip? <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote"><div class="im"><div></div><div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><br>
</div>Sanyo LF25FW203A / LE25FW203A-<br>

</div>Thanks so much for getting to the bottom of this chip, and figuring out which chip it actually is.<br><br>         .probe        = probe_spi_rdid,<br></div><div>Maybe I'm reading the patch wrong, but you appear to be missing the .probe_timing line for this chip.<br>

</div></div></blockquote></div><div>You reading patch wrong.  The probe_timing line for this chip is present below in source code.</div></div></blockquote><div>Okay- thanks for checking. <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote"><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"><div>
<br>LE25FW418A, LE25FW808  and LE25FW806 -<br></div><div><div><div><div><div>For the LE25FW806A you included the small sector erase, but left it out for the rest of the chips- any particular reason for this?<br></div></div>

</div></div></div></div></blockquote></div><div>Because only LE25FW806A has a small sector erase op-code instruction, other chips didn't have it.<br></div><div class="im"><div><br></div></div></div></blockquote><div>
What data sheets do you have for those chips?  The ones below list the small sector erase op-code as D7.<br></div><div>LE25FW418A: 61709 SY 20090428-S00004 No.A1432<br></div><div>LE25FW808: 61009 SY IM 20090319-S00003 No.A0839<br>
</div><div>LE25FW806: 70208 SY IM 20070628-S00004 No.A0838<br><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div class="im">
<div></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"><div><div><div><div><div><br></div>
<div><br>I'll cover from Spansion S25FL128S to the end tomorrow.<br clear="all"></div></div></div></div></div></div></blockquote></div><div>Excellent  </div></div><div><br></div><div><br></div><div>how to delete all versions of the patch and leave only comments in one thread?<br>
</div></blockquote><div>Don't change what you're doing for this patch. <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>
</div><div><br></div><div>I send a new version patch</div><div class=""><div class="h5"><div><br></div>-- <br><div>With best regards Nikolay Nikolaev</div><div>С Уважением Николаев Николай</div>
</div></div></blockquote></div><br></div></div>