Re: [flashrom] [PATCH] Compile fixes, update README

[Carl-Daniel Hailfinger]

On 08.10.2010 12:44, Michael Karcher wrote:
> Am Freitag, den 08.10.2010, 04:30 +0200 schrieb Carl-Daniel Hailfinger:
> > In detail:
> > CONFIG_BITBANG_SPI was not selected if CONFIG_NICINTEL_SPI was on by
> > default.
>
> Patch is fine.
> > Wiki output was missing all flash chips if CONFIG_INTERNAL was not selected.
>
> Patch is fine if tested.
> > ft2232_spi used possibly negative indices in the toupper() function/array.
>
> Please also fix the other similar ctype issues in layout.c (two isprint
> invocations) and serial.c (three tolower invocations) at that time, too.
> The cast to "unsigned char" is exactly the right thing to do.

Done. I also fixed isspace invocations in processor_enable.c.
> > Software requirements on non-Linux/FreeBSD/OpenBSD operating systems
> > were missing.
>
> See comment below.
> > Non-x86 compilation does not work with the default programmer set, so
> > list the make parameters which result in a working build.
>
> I didn't verify the statement about what works on which architecture,
> but if the statements are true, the text is fine.

I tested them.
> > Index: flashrom-compilefixes/README
> >
> > --- flashrom-compilefixes/README	(revision 1201)
> > +++ flashrom-compilefixes/README	(working copy)
> > @@ -60,6 +60,11 @@
> > devel/gmake
> > sysutils/pciutils
> >
> > +On other operating systems, you need the following software:
> > pciutils+libpci (if you want support for mainboard or PCI device flashing)
> > libusb (if you want FT2232 or Dediprog support)
>
> The libusb requirements is general for all operating systems. I don't
> really like it in the "other operating systems" section.

I have moved that paragraph to the top and removed "other operating
systems".
> > Signed-off-by: Carl-Daniel Hailfinger
>
> With the comments addressed, this is
> Acked-by: Michael Karcher

Thanks for the review!

CONFIG_BITBANG_SPI was not selected if CONFIG_NICINTEL_SPI was on by
default.
Wiki output was missing all flash chips if CONFIG_INTERNAL was not selected.
Use correct type for toupper()/tolower()/isspace() functions.
Software requirements were not specified in a generic way.
Non-x86 compilation does not work with the default programmer set, so
list the make parameters which result in a working build.

Signed-off-by: Carl-Daniel Hailfinger 
Acked-by: Michael Karcher

Index: flashrom-compilefixes/Makefile
===================================================================
--- flashrom-compilefixes/Makefile	(Revision 1202)
+++ flashrom-compilefixes/Makefile	(Arbeitskopie)
@@ -115,21 +115,6 @@
 # RayeR SPIPGM hardware support
 CONFIG_RAYER_SPI ?= yes

-# Bitbanging SPI infrastructure, default off unless needed.
-ifeq ($(CONFIG_RAYER_SPI), yes)
-override CONFIG_BITBANG_SPI = yes
-else
-ifeq ($(CONFIG_INTERNAL), yes)
-override CONFIG_BITBANG_SPI = yes
-else
-ifeq ($(CONFIG_NICINTEL_SPI), yes)
-override CONFIG_BITBANG_SPI = yes
-else
-CONFIG_BITBANG_SPI ?= no
-endif
-endif
-endif
-
 # Always enable 3Com NICs for now.
 CONFIG_NIC3COM ?= yes

@@ -170,7 +155,22 @@
 # Disable wiki printing by default. It is only useful if you have wiki access.
 CONFIG_PRINT_WIKI ?= no

+# Bitbanging SPI infrastructure, default off unless needed.
+ifeq ($(CONFIG_RAYER_SPI), yes)
+override CONFIG_BITBANG_SPI = yes
+else
 ifeq ($(CONFIG_INTERNAL), yes)
+override CONFIG_BITBANG_SPI = yes
+else
+ifeq ($(CONFIG_NICINTEL_SPI), yes)
+override CONFIG_BITBANG_SPI = yes
+else
+CONFIG_BITBANG_SPI ?= no
+endif
+endif
+endif
+
+ifeq ($(CONFIG_INTERNAL), yes)
 FEATURE_CFLAGS += -D'CONFIG_INTERNAL=1'
 PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o dmi.o internal.o
 # FIXME: The PROGRAMMER_OBJS below should only be included on x86.
Index: flashrom-compilefixes/print_wiki.c
===================================================================
--- flashrom-compilefixes/print_wiki.c	(Revision 1202)
+++ flashrom-compilefixes/print_wiki.c	(Arbeitskopie)
@@ -268,8 +268,8 @@
    time_t t = time(NULL);

printf(wiki_header, ctime(&t), flashrom_version);
+	print_supported_chips_wiki(2);
 #if CONFIG_INTERNAL == 1
-	print_supported_chips_wiki(2);
    print_supported_chipsets_wiki(3);
    print_supported_boards_wiki();
 #endif
Index: flashrom-compilefixes/serial.c
===================================================================
--- flashrom-compilefixes/serial.c	(Revision 1202)
+++ flashrom-compilefixes/serial.c	(Arbeitskopie)
@@ -106,8 +106,9 @@
 #ifdef _WIN32
    HANDLE fd;
    char *dev2 = dev;
-	if ((strlen(dev) > 3) && (tolower(dev[0]) == 'c')
-	    && (tolower(dev[1]) == 'o') && (tolower(dev[2]) == 'm')) {
+	if ((strlen(dev) > 3) && (tolower((unsigned char)dev[0]) == 'c') &&
+	    (tolower((unsigned char)dev[1]) == 'o') &&
+	    (tolower((unsigned char)dev[2]) == 'm')) {
    	dev2 = malloc(strlen(dev) + 5);
    	strcpy(dev2, "\\.\");
    	strcpy(dev2 + 4, dev);
Index: flashrom-compilefixes/processor_enable.c
===================================================================
--- flashrom-compilefixes/processor_enable.c	(Revision 1202)
+++ flashrom-compilefixes/processor_enable.c	(Arbeitskopie)
@@ -53,22 +53,22 @@
    	if (fgets(line, sizeof(line), cpuinfo) == NULL)
    		break;
    	ptr = line;
-		while (*ptr && isspace(*ptr))
+		while (*ptr && isspace((unsigned char)*ptr))
    		ptr++;
    	/* "cpu" part appears only with some Linux versions.  */
    	if (strncmp(ptr, "cpu", sizeof("cpu") - 1) == 0)
    		ptr += sizeof("cpu") - 1;
-		while (*ptr && isspace(*ptr))
+		while (*ptr && isspace((unsigned char)*ptr))
    		ptr++;
    	if (strncmp(ptr, "model", sizeof("model") - 1) != 0)
    		continue;
    	ptr += sizeof("model") - 1;
-		while (*ptr && isspace(*ptr))
+		while (*ptr && isspace((unsigned char)*ptr))
    		ptr++;
    	if (*ptr != ':')
    		continue;
    	ptr++;
-		while (*ptr && isspace(*ptr))
+		while (*ptr && isspace((unsigned char)*ptr))
    		ptr++;
    	fclose(cpuinfo);
    	return (strncmp(ptr, "ICT Loongson-2 V0.3",
Index: flashrom-compilefixes/ft2232_spi.c
===================================================================
--- flashrom-compilefixes/ft2232_spi.c	(Revision 1202)
+++ flashrom-compilefixes/ft2232_spi.c	(Arbeitskopie)
@@ -148,7 +148,7 @@
    free(arg);
    arg = extract_programmer_param("port");
    if (arg) {
-		switch (toupper(*arg)) {
+		switch (toupper((unsigned char)*arg)) {
    	case 'A':
    		ft2232_interface = INTERFACE_A;
    		break;
Index: flashrom-compilefixes/README
===================================================================
--- flashrom-compilefixes/README	(Revision 1202)
+++ flashrom-compilefixes/README	(Arbeitskopie)
@@ -42,8 +42,12 @@
 Build Instructions
 ------------------

-To build flashrom you need to install the following packages or ports:
+To build flashrom you need to install the following software:

+ * pciutils+libpci (if you want support for mainboard or PCI device flashing)
+ * libusb (if you want FT2232 or Dediprog support)
+ * libftdi (if you want FT2232 support)
+
 Linux et al:

* pciutils / libpci
@@ -111,6 +115,18 @@
  http://homer.rice.edu/~sandmann/cwsdpmi/csdpmi7b.zip and make sure
  CWSDPMI.EXE is in the current directory.

+Processor architecture dependent features:
+
+ On non-x86 architectures you have to disable a few programmers because they
+ use port-based I/O which is not directly available on non-x86. Please add
+ CONFIG_RAYER_SPI=no CONFIG_NIC3COM=no CONFIG_ATAHPT=no CONFIG_NICREALTEK=no \
+ CONFIG_NICNATSEMI=no
+ as parameters to the "make" invocation.
+ Besides that, the internal programmer is only supported on x86 and MIPS. On
+ other architectures, please add
+ CONFIG_INTERNAL=no
+ as parameter to the "make" invocation.
+
 Installation
 ------------