[flashrom] [PATCH] Compile fixes

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Mar 8 01:21:16 CET 2011


Auf 07.03.2011 14:52, Michael Karcher schrieb:
> Am Montag, den 07.03.2011, 12:10 +0100 schrieb Carl-Daniel Hailfinger:
>   
>> Fix compilation if CONFIG_INTERNAL=no.
>> Fix compilation if everything except CONFIG_SATAMV is no.
>> Do not compile in PCI support for wiki printing if no PCI devices are
>> supported.
>>     
> I assume you tested the compilation. Your changes look sensible.
>
>   
>> Note: The flashrom.c hunk is ugly. Suggestions how to solve the ugliness
>> are appreciated.
>>     
> My suggestion: Remove the braces. Still kind-of ugly, but I also have no
> idea how to avoid that cleanly. An idea would be to define a dummy
> function physmap that does nothing in case we don't need the real
> physmap. But in fact we really want the whole if/snprintf go away, so it
> looks like we have to do the #ifdef stuff.
>   

Done. To be honest, I don't like the "at physical address" message
anymore. We could simply remove the word "physical" and mention the
address and the programmer name. Once external LPC/FWH programmer
support is merged, we will need that anyway. And my long-term goal is to
store programmer info in struct flashchip anyway to allow handling stuff
like multiple flash chips in a Dual BIOS scenario.


>> +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV >= 1
>>     
> We have this #if condition twice. Maybe we should define a
> "INCLUDE_PCIDEV" define from this condition in some header file?
>   

Hm. NEED_PCI almost fits the mandate, except that it also is active for
CONFIG_INTERNAL. So yes, your suggestion makes sense. Given the abuse of
NEED_PCI for anything which needs I/O, we should really clean up that mess.

What about
PCIDEV for all PCI-based programmers (not internal)
HWACCESS for programmers needing memory or I/O access
PCI for all programmers using PCI in any fashion
PCIDEV selects PCI
PCI selects HWACCESS

Pick any prefix for those names, e.g. NEED_ or INCLUDE_


>> +#if CONFIG_INTERNAL == 1
>>  	if (programmer_table[programmer].map_flash_region == physmap) {
>>  		snprintf(location, sizeof(location), "at physical address 0x%lx", base);
>> -	} else { 
>> +	} else
>> +#endif
>> +	{ 
>>  		snprintf(location, sizeof(location), "on %s", programmer_table[programmer].name);
>>     
> Acked-by: Michael Karcher <flashrom at mkarcher.dialup.fu-berlin.de>
>   

Thanks for your review! I removed the ugliness in flashrom.c and
committed in r1278.

Regards,
Carl-Daniel

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





More information about the flashrom mailing list