[coreboot] next set of patches: modify the dtc and device code to support IDs

Uwe Hermann uwe at hermann-uwe.de
Fri Jan 18 16:09:05 CET 2008

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).

http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

More information about the coreboot mailing list