1 Jun
2009
1 Jun
'09
10:20 p.m.
On 21:59 Mon 01 Jun , Stelian Pop wrote:
Hi Rémy,
I do have a few comments, see below:
On Mon, Jun 01, 2009 at 08:21:42PM +0200, Remy Bohmer wrote:
V2 reworked comments from Jean-Christophe PLAGNIOL-VILLARD
[...]
+COBJS-$(CONFIG_DISPLAY_CPUINFO) +=cpuinfo.o
missing space ?
- switch ((cidr & AT91_CIDR_EPROC) >> 5) {
/* 0x1 = ARM946ES, 0x2 = ARM7TDMI, 0x4 = ARM920T */- case 0x5: txt = "ARM926EJS"; break;
- default: txt = "undefined"; break;
since there are only four choices (until now at least...), why not put the 'txt' for all of them ?
We need to use the ifdef to reduce the size impact and not implement non-supported code is important
and the 'undefined' string is not a good choice: it represents a configuration not known to U-Boot, but its numeric value is well defined. So I suggest you change this to 'unknown', or even better, 'unknown: 0xfoo'.
I agree
Best Regards, J.