[flashrom] [PATCH] Convert chips to block erasers: SST

[Carl-Daniel Hailfinger]

On 20.01.2010 13:46, Michael Karcher wrote:
> Am Mittwoch, den 20.01.2010, 03:55 -0800 schrieb Sean Nelson:
>   
>> On 1/20/10 2:16 AM, Michael Karcher wrote:
>>     
>>> This chip supports erase_chip only in A/A mux mode (although the
>>> datasheet calls it "parallel". Do we really want to have that listed?
>>> This also applies to:
>>>       
>> I don't think we can determine if the chip is in LPC or A/A mux mode.
>>     
> Is there *any* available chipset that uses A/A mux? I think this mode is
> only usable by external programmers, and I also don't think that
> flashrom is supporting any external programmer that does A/A mux.
>   

AFAIK no chipset uses A/A mux mode. It won't be listed in
.buses_supported (see the comments about A/A mux in flashchips.c), so it
is a safe assumption that commands for A/A mux mode are unavailable for
all practical purposes. My suggestion is to set .block_erase=NULL until
we handle this differently.


>>>> + .block_erasers =
>>>> + {
>>>> + {
>>>> + .eraseblocks = { {4 * 1024, 512} },
>>>> + .block_erase = erase_sector_49lfxxxc,
>>>> + }, {
>>>> + .eraseblocks = {
>>>> + {64 * 1024, 31},
>>>> + {32 * 1024, 1},
>>>> + {8 * 1024, 2},
>>>> + {16 * 1024, 1},
>>>> + },
>>>> + .block_erase = erase_block_49lfxxxc,
>>>> + }, {
>>>> + .eraseblocks = { {2 * 1024 * 1024, 1} },
>>>> + .block_erase = erase_chip_49lfxxxc,
>>>> + }
>>>> + },
>>>>         
>>> erase_chip_49lfxxxc is just calling erase_sector_49lfxxxc. No need to
>>> add it as chip erase function.
>>>       
>> erase_chip_49lfxxc is needed to convert to block_eraser(struct flashrom 
>> *flash, unsigned int addr, unsigned int size),
>> its also needed because of unlocking.
>>     
>
> Right. So erase_sector_49lfxxxc and your new erase_block_49lfxxxc don't
> work if you don't unlock beforehands. You might try sector erase anyway
> (the chip could be unlocked already), but I would add unlocking stuff
> into erase_block_49lfxxxc, as unlocking is done on block level. As the
> chip-erase has to be emulated by walking either sectors or blocks, I
> don't see the point of doing the walk yourself if the generic eraseblock
> walker we are trying to use does exactly that. And after adding
> blockwise unlock to erase_block_49lfxxxc, you don't need the unlocking
> part of erase_49lfxxxc anymore.
>
> And then you can also kill erase_49lfxxxc completely.
>   

The problem with locking is that locking will always have boundaries
independent of eraseblock structure. My patch at
http://patchwork.coreboot.org/patch/581/ tries to decouple locking from
all other functions, and AFAICS it would help here a lot.

Regards,
Carl-Daniel

-- 
Developer quote of the year:
"We are juggling too many chainsaws and flaming arrows and tigers."