[flashrom] [PATCH] Real partial writes

[Carl-Daniel Hailfinger]

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

Crap. Could you please retest http://patchwork.coreboot.org/patch/2155/ ?

If that one works, I am tempted to commit it, and send all other changes
as separate patches on top.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/