subsystem IDs

[Myles Watson]

This patch makes subsystem ids work.  Here are the changes by file:

device/pci_device.c:
    Only update IDs if:
    - The device is on the mainboard
    - The device has a Vendor ID and Device ID
    - The device has a set_subsystem function in ops_pci(dev)

util/dtc/flattree.c:
    Make devices from the dts be on_mainboard.
    If they're plugged in, they shouldn't be in the dts.

mainboard/amd/serengeti/dts:
    Add subsystem_vendor and subsystem_device.

Build tested on Serengeti.  Getting closer :)

Signed-off-by: Myles Watson

Thanks,
Myles

[Uwe Hermann]

On Tue, Nov 18, 2008 at 02:49:37PM -0700, Myles Watson wrote:
> This patch makes subsystem ids work.  Here are the changes by file:

Nice!

Acked-by: Uwe Hermann

So just to make sure -- we can now set per-board IDs _and_ override them
for individual PCI devices in the dts?
> Index: mainboard/amd/serengeti/dts
>
> --- mainboard/amd/serengeti/dts	(revision 1043)
> +++ mainboard/amd/serengeti/dts	(working copy)
> @@ -22,6 +22,8 @@
>    device_operations="serengeti";
>    mainboard_vendor = "AMD";
>    mainboard_name = "Serengeti";
> subsystem_vendor = "PCI_VENDOR_ID_AMD";

Nice, I didn't know that #defines could be used in dts.

Uwe.

[Myles Watson]

> -----Original Message-----
> From: Uwe Hermann [mailto:[e-mail]]
> Sent: Tuesday, November 18, 2008 3:47 PM
> To: Myles Watson
> Cc: Coreboot
> Subject: Re: [coreboot] subsystem IDs
>
> On Tue, Nov 18, 2008 at 02:49:37PM -0700, Myles Watson wrote:
> > This patch makes subsystem ids work.  Here are the changes by file:
>
> Nice!
>
> Acked-by: Uwe Hermann
>
> So just to make sure -- we can now set per-board IDs _and_ override them
> for individual PCI devices in the dts?

It should work.  You should just have to add the subsystem values to the dts
in the same way that it was added for the mainboard.  You'll have to add
them in the mainboard's dts, though, or override the function that sets it.
If you add them in the device's dts they end up in the device_configuration
structure.  That's part of the reason why the superio code ended up the way
it did.
> > Index: mainboard/amd/serengeti/dts
> >
> > --- mainboard/amd/serengeti/dts	(revision 1043)
> > +++ mainboard/amd/serengeti/dts	(working copy)
> > @@ -22,6 +22,8 @@
> >    device_operations="serengeti";
> >    mainboard_vendor = "AMD";
> >    mainboard_name = "Serengeti";
> > subsystem_vendor = "PCI_VENDOR_ID_AMD";
>
> Nice, I didn't know that #defines could be used in dts.

Yeah.  I like the dts.  The strings from here go straight into statictree.c,
so anything that compiles there is fine.

Rev 1045.

Thanks,
Myles

[Peter Stuge]

Uwe Hermann wrote:
> > +++ mainboard/amd/serengeti/dts	(working copy)
> > @@ -22,6 +22,8 @@
> >    device_operations="serengeti";
> >    mainboard_vendor = "AMD";
> >    mainboard_name = "Serengeti";
> > subsystem_vendor = "PCI_VENDOR_ID_AMD";
>
> Nice, I didn't know that #defines could be used in dts.

This is a good example of why NOT to use #defines for IDs:

1. The value could change unexpectedly
 * If the #define is changed in the source a new ID can end up being
   used even though the dts is unchanged. This is not obvious and IMO
   not acceptable.

2. It is redundant
 * mainboard_vendor already provides a friendly name for the vendor.

//Peter

[Myles Watson]

> Uwe Hermann wrote:
> > > +++ mainboard/amd/serengeti/dts	(working copy)
> > > @@ -22,6 +22,8 @@
> > >    device_operations="serengeti";
> > >    mainboard_vendor = "AMD";
> > >    mainboard_name = "Serengeti";
> > > subsystem_vendor = "PCI_VENDOR_ID_AMD";
> >
> > Nice, I didn't know that #defines could be used in dts.
>
> This is a good example of why NOT to use #defines for IDs:
> The value could change unexpectedly
> If the #define is changed in the source a new ID can end up being
> used even though the dts is unchanged. This is not obvious and IMO
> not acceptable.

If it does a lot more than this breaks.
> It is redundant
> mainboard_vendor already provides a friendly name for the vendor.

mainboard_vendor is friendly for printing, but it's hard to convert that
later into 0x1022.

Thanks,
Myles