On Thu, Jan 17, 2008 at 12:55:22PM -0800, ron minnich wrote: > +/* here is a simple macro for creating 32-bit int values from > + * 4 chars. I am putting it here as I am not yet sure where to > + * put it! > + */ > +
Here's a slighly better comment, IMHO: /** * Create a 32-bit value from four characters. This is better * than the usual enum values when using (JTAG) debuggers. */ #define TYPENAME(a,b,c,d) ((a<<24)|(b<<16)|(c<<8)|(d)) > + /* get the properties out that are generic device props */ Please try to write all code comments (at least full sentences) correctly, i.e. starting with capital letter and ending in a full stop. I'm fixing this from time to time in the code, but it keeps coming back... > @@ -1349,7 +1364,7 @@ > > fix_next(bi->dt); > /* emit any includes that we need -- TODO: ONLY ONCE PER TYPE*/ > - fprintf(f, "#include <device/device.h>\n#include <device/pci.h>\n"); > + fprintf(f, "#include <device/device.h>\n#include > <device/pci.h>\n#include <device/pci_ids.h>\n"); Shouldn't we just #include pci_ids.h from pci.h? It's tedious and useless to #include pci_ids.h manually all over the place. We need it in pretty much every file in v3 (at least in all files which also need pci.h). Uwe. -- http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org -- coreboot mailing list [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
