[coreboot] [PATCH] flashrom: More SPI erase functions

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Nov 3 01:04:41 CET 2008


On 03.11.2008 00:39, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> On 05.08.2008 01:58, Carl-Daniel Hailfinger wrote:
>>   
>>     
>>> As promised to Stefan, here's the flashrom patch I've been talking about.
>>>
>>> Add additional SPI sector erase and chip erase command functions to
>>> flashrom. Not all chips support all commands, so allow the implementer
>>> to select the matching function.
>>> Fix a layering violation in ICH SPI code to be less bad. Still not
>>> perfect, but the new code is shorter, more generic and architecturally
>>> more sound.
>>> TODO (in a separate patch):
>>> - move the generic sector erase code to spi.c
>>> - decide which erase command to use based on info about the chip
>>> - create a generic spi_erase_all_sectors function which calls the
>>> generic sector erase function
>>>   
>>>     
>>>       
>> Thanks to Stefan for reviewing and commenting. New patch follows.
>>
>> Regards,
>> Carl-Daniel
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>     
> Acked-by: Stefan Reinauer <stepan at coresystems.de>
>   

Thanks, committed in r3722.


>> -	if (run_opcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) {
>>     
> This won't apply cleanly with the version in the tree though, I dropped
> the index thing as per our discussion earlier.
>   

Yes, I updated the patch.


>> -		rc = ich_spi_erase_block(flash, i * erase_size);
>> +		/* FIMXE: call the chip-specific spi_block_erase_XX instead.
>> +		 * For this, we need to add a block erase function to
>> +		 * struct flashchip.
>> +		 */
>> +		rc = spi_block_erase_d8(flash, i * erase_size);
>>     
>
> I agree with the observation. But I'm not sure what the struct flashchip
> should look like.
>
> When I wrote /dev/bios in 1998 I had both a page size (128-256 bytes for
> example) and a block size (8-192k for example) and /dev/bios would try
> to choose the appropriate function for your chip. Some chips have
> non-linear block sizes, so I also added an array with the size of each
> block. Plus a bitfield describing whether to use page size or block size
> erases and writes.
>
> So, for some random intel chip, the block sizes would look like this:
>
> { "28F001BX-T", 0x9489, 12,  128, 1, 128, 3, (int []) {
> 0,112,116,120,128 } },
>
> For some ancient source code browsing fun go to
> http://tracker.coreboot.org/trac/openbios/browser/openbios-devel/utils/devbios
>   

Let's continue the discussion about per-sector flashing in the existing
thread about it with subject
[coreboot] [RFC] flashrom: sector-based flashing

Regards,
Carl-Daniel

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





More information about the coreboot mailing list