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.
Note: The flashrom.c hunk is ugly. Suggestions how to solve the ugliness are appreciated.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-compilefixes/print_wiki.c =================================================================== --- flashrom-compilefixes/print_wiki.c (Revision 1275) +++ flashrom-compilefixes/print_wiki.c (Arbeitskopie) @@ -246,6 +246,8 @@ printf("\n|}\n\n|}\n"); }
+/* Not needed for CONFIG_INTERNAL, but for all other PCI-based programmers. */ +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV >= 1 static void print_supported_pcidevs_wiki(const struct pcidev_status *devs) { int i = 0; @@ -262,6 +264,7 @@ (devs[i].status == NT) ? "?3" : "OK"); } } +#endif
void print_supported_wiki(void) { Index: flashrom-compilefixes/flashrom.c =================================================================== --- flashrom-compilefixes/flashrom.c (Revision 1275) +++ flashrom-compilefixes/flashrom.c (Arbeitskopie) @@ -1186,9 +1186,12 @@ if (!flash || !flash->name) return NULL;
+#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); }
Index: flashrom-compilefixes/programmer.h =================================================================== --- flashrom-compilefixes/programmer.h (Revision 1275) +++ flashrom-compilefixes/programmer.h (Arbeitskopie) @@ -229,7 +229,7 @@ #endif
/* print.c */ -#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI >= 1 +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV >= 1 void print_supported_pcidevs(const struct pcidev_status *devs); #endif
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.
+#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?
+#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@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
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@mkarcher.dialup.fu-berlin.de
Thanks for your review! I removed the ugliness in flashrom.c and committed in r1278.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Michael Karcher