2010/10/29 Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span><br><div class="gmail_quote"><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><div class="h5">On 26.10.2010 19:22, Carl-Daniel Hailfinger wrote:<br>
> On 26.10.2010 16:27, Idwer Vollering wrote:<br>
><br>
>> 2010/10/22 Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
>><br>
>><br>
>><br>
>>> Given that the version which addresses the review comments apparently is<br>
>>> buggy, I hereby propose to merge the earlier version which worked. It is<br>
>>> here again for your reference with some slight cosmetic fixes which<br>
>>> should not impact functionality.<br>
>>> Error recovery still needs work (see the FIXME comment). It will work in<br>
>>> most cases, but if a given erase command for a block only erased parts<br>
>>> of the block (partial write lock), the code will make incorrect<br>
>>> assumptions.<br>
>>><br>
>>> If anyone feels adventurous, I would love to see logs on Intel/VIA<br>
>>> chipsets with SPI (preferably locked down chipsets or with r1193 reverted).<br>
>>><br>
>>> If you're going to review this, make sure you keep a stack of bananas<br>
>>> (quickly mobilized carbohydrates for your brain), a bucket of ice (to<br>
>>> prevent brain overheating) and a bottle of aspirin handy. If any code is<br>
>>> unclear, please tell me and I'll try to add comments to improve<br>
>>> readability.<br>
>>><br>
>>> This code has been tested. Testing erase (and checking with a separate<br>
>>> readback that erase actually worked) and write (same test with separate<br>
>>> readback) would be highly appreciated. Verbose logs are even more<br>
>>> appreciated.<br>
>>><br>
>>> I think the code is ready for merge if you trust write/erase to never<br>
>>> fail. The error cases still need to be tested. Should we reread the<br>
>>> whole chip if write/erase failed to make sure our current view of the<br>
>>> chip contents is not stale?<br>
>>><br>
>>> This patch makes flashrom use real partial writes. If you write an image<br>
>>> full of 0xff, flashrom will erase and detect that no write is needed. If<br>
>>> you write an image which differs only in some parts from the current<br>
>>> flash contents, flashrom will detect that and not touch unchanged areas.<br>
>>><br>
>>> Fix a long-standing bug in need_erase() for 256 byte granularity as well.<br>
>>><br>
>>> Nice side benefit: Detailed progress printing.<br>
>>> S means skipped<br>
>>> E means erased<br>
>>> W means written<br>
>>><br>
>>> Thanks to Andrew Morgan for testing countless iterations of this patch.<br>
>>> Thanks to Richard A. Smith for testing on Dediprog.<br>
>>> Thanks to David Hendricks for the review (will be addressed later).<br>
>>><br>
>>> Signed-off-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
>>><br>
>>><br>
>>><br>
>> With your latest patch (<a href="http://patchwork.coreboot.org/patch/2160/" target="_blank">http://patchwork.coreboot.org/patch/2160/</a>) applied:<br>
>><br>
>> $ sudo ./flashrom -p nicintel_spi -V -w nicintel_spi.rom -c M25P10.RES<br>
>> flashrom v0.9.3-r1215 on FreeBSD 8.1-RELEASE (i386), built with libpci<br>
>> 3.1.7, GCC 4.2.1 20070719  [FreeBSD], little endian<br>
>> Calibrating delay loop... OK.<br>
>> Initializing nicintel_spi programmer<br>
>> Found "Intel 82541PI Gigabit Ethernet Controller" (8086:107c, BDF 01:03.0).<br>
>> Probing for ST M25P10.RES, 128 KB: probe_spi_res1: id 0x10<br>
>> Chip status register is 00<br>
>> Found chip "ST M25P10.RES" (128 KB, SPI) at physical address 0xfffe0000.<br>
>> Reading old flash chip contents...<br>
>> Erasing and writing flash chip... Looking at blockwise erase function 0...<br>
>> trying... 0x000000-0x007fff:W, 0x008000-0x00ffff:W, 0x010000-0x017fff:W,<br>
>> 0x018000-0x01ffff:S<br>
>> Done.<br>
>> Verifying flash... VERIFY FAILED at 0x00008000! Expected=0xc6, Read=0x15,<br>
>> failed byte count from 0x00000000-0x0001ffff: 0x8d75<br>
>><br>
>><br>
><br>
> Crap. Could you please retest <a href="http://patchwork.coreboot.org/patch/2155/" target="_blank">http://patchwork.coreboot.org/patch/2155/</a> ?<br>
><br>
> If that one works, I am tempted to commit it, and send all other changes<br>
> as separate patches on top.<br>
><br>
<br>
</div></div>Idwer tested this patch together with patch 2155</blockquote><div><br>No. That was 2160, like you asked me to test on IRC.<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;">
from patchwork and it<br>
worked.<br>
<a href="http://paste.flashrom.org/view.php?id=138" target="_blank">http://paste.flashrom.org/view.php?id=138</a><br>
<div><div></div><div class="h5"><br>
Regards,<br>
Carl-Daniel<br>
<br>
--<br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a><br>
<br>
</div></div></blockquote></div><br>