Add Tegra2 SPI controller.

processor_enable.c

Index: processor_enable.c
diff --git a/processor_enable.c b/processor_enable.c
index b14cd3381543cc684c88bfadb1746aa5b44d2b06..7c64e946d535505afb9541e8658b7a721dedeb8a 100644
--- a/processor_enable.c
+++ b/processor_enable.c
@@ -81,6 +81,45 @@ static int is_loongson(void)
 }
 #endif
 
+#if defined(__arm__)
+#include <stdio.h>
+#include <string.h>
+#include <ctype.h>
+
+static int is_tegra2(void)

[Micah C, 2011/03/28 02:08:22]

This function is complex enough to justify a comment explaining the goal. 
Something like this:

// Return true if executing on an Nvidia Tegra20 CPU, indicated when
/proc/cpuinfo contains a line like "CPU part : 0xc09"

In the future this will probably need to be extended to support a list of CPU
part identifiers, since future CPUs in the family will likely include a common
control interface.

[Louis, 2011/03/30 10:15:39]

Done. Good point.
On 2011/03/28 02:08:22, Micah C wrote:

+{
+	FILE *cpuinfo;
+	uint32_t impl = 0, architecture = 0, variant = 0, part = 0;
+
+	cpuinfo = fopen("/proc/cpuinfo", "rb");
+	if (!cpuinfo)
+		return 0;
+	while (!feof(cpuinfo)) {
+		char line[512], *ptr;
+		if (fgets(line, sizeof(line), cpuinfo) == NULL)
+			break;
+		ptr = line;
+		while (*ptr && isspace((unsigned char)*ptr))
+			ptr++;
+		if (strncmp(ptr, "CPU part", sizeof("CPU part") - 1) == 0)

[Micah C, 2011/03/28 02:08:22]

The appearance of the constant string "CPU part" 3 times in two lines suggests
that it might benefit from being a named constant.

How about declaring:
const char *field = "CPU part";
const char *value = "0xc09";

and then using the named constants for the comparison and the sizeof operators? 
 Some future implementation might replace the "value" with another loop over a
list of values, and using a named variable will make that be a smaller change.

[Louis, 2011/03/30 10:15:39]

Good idea. Done.

+			ptr += sizeof("CPU part") - 1;
+		while (*ptr && isspace((unsigned char)*ptr))
+			ptr++;
+		if (*ptr != ':')
+			continue;
+
+		fclose(cpuinfo);

[Micah C, 2011/03/28 02:08:22]

This seems like a surprising place to close the file descriptor, but I see that
you're doing this to make sure that it's not leaked when returning from within
this loop.

How about changing the looping structure so that the loop always ends, and there
is just 1 return path from the function?  Maybe use a local variable to
represent the return value, and then forcing it to 1 or 0 based on the loop exit
condition?

[Louis, 2011/03/30 10:15:39]

Done.

+		ptr++;
+		while (*ptr && isspace((unsigned char)*ptr))
+			ptr++;
+		return (strncmp(ptr, "0xc09",
+				 sizeof("0xc09") - 1) == 0);
+	}
+	fclose(cpuinfo);
+	return 0;
+}
+#endif
+
 int processor_flash_enable(void)
 {
 	/* FIXME: detect loongson on FreeBSD and OpenBSD as well.  */
@@ -90,6 +129,12 @@ int processor_flash_enable(void)
 		return 0;
 	}
 #endif
+#if defined (__arm__)
+	if (is_tegra2()) {
+		msg_pinfo("Detected NVIDIA Tegra 2.\n");

[Micah C, 2011/03/28 02:08:22]

Is this a temporary debugging message, or something that's valuable for the
final implementation?  Are there similar logging messages for the other chipsets
that flashrom supports?

[dhendrix, 2011/03/28 05:06:05]

Nice catch. We should suppress this for normal usage and use "msg_pdbg()"
instead.

There is a similar message for other chipsets, but I believe that is #ifdef'd
out since it's within a bunch of code that is more specific to x86...

[Louis, 2011/03/30 10:15:39]

Done.

[Louis, 2011/03/30 10:15:39]

Done.

+		return tegra2_spi_init();
+	}
+#endif
 	/* Not implemented yet. Oh well. */
 	return 1;
 }