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

[Peter Stuge]

On Thu, Jan 17, 2008 at 12:55:22PM -0800, ron minnich wrote:
> +#define TYPENAME(a,b,c,d) ((a<<24)|(b<<16)|(c<<8)|(d))

Does each parameter need () in the macro definition here? And maybe
mask away overflowing bits?

#define TYPENAME(a,b,c,d) ((((a)&0xff)<<24)|(((b)&0xff)<<16)|(((c)&0xff)<<8)|((d)&0xff))

?
> DEVICE_ID_ROOT	= TYPENAME('R','O','O','T'),
> DEVICE_ID_PCI	= TYPENAME(' ','P','C','I'),
> DEVICE_ID_PNP	= TYPENAME(' ','P','N','P'),
> DEVICE_ID_I2C	= TYPENAME(' ','I','2','C'),
> DEVICE_ID_APIC	= TYPENAME('A','P','I','C'),
> DEVICE_ID_PCI_DOMAIN = TYPENAME('P','C','I','D'),
> DEVICE_ID_APIC_CLUSTER = TYPENAME('A','P','C','C'),
> DEVICE_ID_CPU = TYPENAME(' ','C','P','U'),
> DEVICE_ID_CPU_BUS =  TYPENAME(' ','B','U','S'),

Dunno..
> +{
>    constructor = "geodelx_north_constructors";
> domainid = "PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LXBRIDGE";
>
> +};

Different whitespace?
> /* note the flaw here: we're just taking the %s as an initialization string.
> Someone who is good at yacc might be able to put this stuff into the dts language.
> this streq stuff is a bit of a hack
> */
>
> if (streq(prop->name, "pcidomain")){
>    fprintf(f, "\t.path = {.type=DEVICE_PATH_PCI_DOMAIN,.u={.pci_domain={ .domain = %s }}},\n", 
>      prop->val.val);

uhm, what? Can you explain this a little further?

I like the general idea, but no ack just yet.

//Peter