[flashrom] [PATCH] Move implicit erase out of chip drivers, clean up

[Carl-Daniel Hailfinger]

On 10.07.2010 20:16, Michael Karcher wrote:
> Am Samstag, den 22.05.2010, 03:26 +0200 schrieb Carl-Daniel Hailfinger:
>
> [as requested on IRC, this is not a full review, but two things not
> directly related to the patch stand out I don't want to leave
> uncommented]
>
>   
>>  int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
>>  {
>> -	int total_size = 1024 * flash->total_size;
>>  	int i;
>>  
>>  	/*
>>  	 * IT8716F only allows maximum of 512 kb SPI chip size for memory
>>  	 * mapped access.
>>  	 */
>> -	if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) {
>> +	if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) {
>>     
> why do you have to test for the programmer type here? It seems like
> it8716f* functions are only ever called if programmer is
> PROGRAMMER_IT87SPI.
>   

Historical baggage, and a corner case. PROGRAMMER_IT87SPI means someone
used "-p it87_spi" instead of "-p internal". That's for machines which
used it87_spi before we had autodetection (and used board enabled
instead), and it also is for machines where the IT87* Super I/O does not
perform flash translation, but where someone hooked up a flash chip
anyway and wants to use IT87 in pure command mode and avoid memmapped mode.
To be honest, I think we can kill that special case. I don't know of
anyone who actually uses it nowadays.


>> +/* real chunksize is 1, logical chunksize is 64k */
>>  int write_coreboot_m29f400bt(struct flashchip *flash, uint8_t *buf)
>>  {
>>  	chipaddr bios = flash->virtual_memory;
>>     
> The M29F400 stuff is completely broken. We use write_coreboot_m29f400bt
> everywhere and write_m29f400bt is dead code. But
> write_coreboot_m29f400bt does just write the lower half of the chip
> (below the dashed line in the diagram).
>   

I think I'll just kill that function.

Regards,
Carl-Daniel

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