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

ron minnich rminnich at gmail.com
Fri Jan 18 01:30:13 CET 2008


On Jan 17, 2008 3:35 PM, Peter Stuge <peter at stuge.se> wrote:
> 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))

I'll fix this if we keep the change (see below).

>
> ?
>
>
> > +     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..

You mean you don't like it? I can always skip this patch. It is a
suggestion. I don't
know if I like it either, but on the other hand, it might be handy for
jtag debuggers.



>
>
> > +{
> > +        constructor = "geodelx_north_constructors";
> > +     domainid = "PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_LXBRIDGE";
> > +};
>
> Different whitespace?

fixed.

>
>
> > + /* 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?

Note that this code is not changed from a year ago. So I won't yank the code :-)

The comment is noteing that we're generating C code from property
names. So, for example, in
 mainboard/pcengines/alix1c/dts , we have this:
                pcidomain = "0";
That property name is picked up by the dtc code shown above and turned
into C code. The property name (pcidomain) matches in the streq; the
property value ("0") is taken as a domain name.

My comment represents some uncertainty on the design, but, in painful,
long, discussions on the v3 list we agreed to this -- so I should
probably just drop the comment. I had forgotten how long it took us to
iterate to this design, so I should not stir up discussion on it ...

That said, I don't see a good way to move forward absent this patch,
but I'm open to suggestion.

Thanks

ron




More information about the coreboot mailing list