[coreboot] [PATCH] superiotool: libsuperiodetect

[Stefan Reinauer]

On 6/30/10 4:16 PM, Carl-Daniel Hailfinger wrote:
>> 15kb lzma compressed is nothing.
>>   
>>     
> 8 kB more or less in a recovery payload of a coreboot image are not
> negligible IMHO.
>
> In normal OS environments people don't use compressed binaries, and
> there a 2 MB size increase for flashrom (98% of it for dead code which
> can't be optimized away by the compiler/linker) is definitely something
> people will object to.
>   
Well...

- once we have a recovery payload (which will have to load a 1MB rom
image from _somewhere_ as we speak of size) and
- once we have people complaining

we should definitely think about such optimizations. My bet is by then
anything below 8mbit flash parts has died out. :)


> IMHO the {EOT} markers should be killed in libsuperiodetect, at least
>>> for the LDNs. Such an operation would have increased the size of the
>>> patch and would have reduced readability as well. Due to that, I skipped
>>> it, but I plan to do that in a later operation.
>>>   
>>>     
>>>       
>> What would you use instead? The array members all have different sizes
>> as far as I can tell, so some kind of end marker will be needed.
>>   
>>     
> The libsuperiodetect case doesn't care about LDNs at all. Having exactly
> one LDN definition per Super I/O which is always EOT looks like dead
> code to me, especially if nothing is looking at the LDN.
>   
Maybe using the tables from superiotool is not the best way to solve the
problem you are trying to solve at all?

What is that problem anyways? Since flashrom has mechanisms to figure
out the board, and the super io is a hard component on the board, will
you need anything more but the numeric ID of the chip?

>>>  				     uint8_t revreg)
>>>  {
>>>  	uint8_t id, rev;
>>> +#ifndef LIBSUPERIODETECT
>>>  	uint16_t runtime_base;
>>> +#endif /* ! LIBSUPERIODETECT */
>>>   
>>>     
>>>       
>> @@ -765,7 +803,9 @@
>> Maybe this could be moved downwards to where it is used, to save an ifndef?
>>   
>>     
> IIRC this would break on some compilers which don't allow variable
> declarations in the middle of some code.
>
>   

Ok, which ones of them are supported for compiling flashrom?

The way superiotool defines functions also breaks K&R style.

Stefan