[coreboot] [PATCH] Drop drivers/pci/onboard

[Stefan Reinauer]

Myles Watson wrote:
>
>
> On Fri, Nov 6, 2009 at 3:34 PM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at gmx.net
> <mailto:c-d.hailfinger.devel.2006 at gmx.net>> wrote:
>
>     On 06.11.2009 22:02, Myles Watson wrote:
>     > On Fri, Nov 6, 2009 at 1:14 PM, Myles Watson <mylesgw at gmail.com
>     <mailto:mylesgw at gmail.com>> wrote:
>     >
>     >
>     >> On Fri, Nov 6, 2009 at 12:56 PM, Myles Watson
>     <mylesgw at gmail.com <mailto:mylesgw at gmail.com>> wrote:
>     >>
>     >>
>     >>> On Fri, Nov 6, 2009 at 12:41 PM, Peter Stuge <peter at stuge.se
>     <mailto:peter at stuge.se>> wrote:
>     >>>
>     >>>
>     >>>>  Myles Watson wrote:
>     >>>>
>     >>>>>>> PCI onboard was only used for ROM images at fixed offsets in
>     >>>>>>> flash.  Now that we have CBFS, there is no use for it.
>     >>>>>>>
>     >>>>  But what about the PCI device ids which are being taken out
>     of the
>     >>>> device tree? Are they guaranteed to be discovered through
>     automatic
>     >>>> scanning?
>     >>>>
>     >>>>
>     >>> Yes.  If the couldn't be probed, they would have been disabled
>     even though
>     >>> they were found in the tree.
>     >>>
>     >>> I wonder a little about the on_mainboard flag.  It's possible
>     that some of
>     >>> these devices would have had that flag set if they didn't need
>     to have a
>     >>> ROM.  I don't know of a good way to automatically figure out
>     which devices
>     >>> that would apply to.
>     >>>
>     >>>
>     >> The rest of the story :)
>     >>
>     >> The on_mainboard flag gets used to set subsystem IDs and
>     >> CONFIG_CONSOLE_VGA_ONBOARD_AT_FIRST
>     >>
>     >>
>     > I put all the devices back in the tree.  So they show up in
>     static.c like
>     > they should, they just don't have the onboard driver any more.
>     >
>     > Signed-off-by: Myles Watson <mylesgw at gmail.com
>     <mailto:mylesgw at gmail.com>>
>     >
>
>     I really like this patch, and the effort you put into removing
>     commented
>     out variants as well. A few minor cosmetic points, but other than
>     that it is
>     Acked-by: Carl-Daniel Hailfinger
>     <c-d.hailfinger.devel.2006 at gmx.net
>     <mailto:c-d.hailfinger.devel.2006 at gmx.net>>
>
> Thanks.
>  
>
>     > Index: svn/src/mainboard/asus/mew-vm/Config.lb
>     > ===================================================================
>     > --- svn.orig/src/mainboard/asus/mew-vm/Config.lb
>     > +++ svn/src/mainboard/asus/mew-vm/Config.lb
>     > @@ -97,18 +97,14 @@ chip northbridge/intel/i82810
>     >       device pci_domain 0 on
>     >               device pci 0.0 on end # Host bridge
>     >               device pci 1.0 on # Onboard Video
>     > -                     #chip drivers/pci/onboard
>     >                       #       device pci 1.0 on end
>     >
>
>     A device which hangs off itself?
>
> I couldn't tell.  The PCI devfn (1.0) doesn't help because a child
> device will be on a different bus, so it's allowed to have the same
> devfn as its parent.  That's why I left it.
I was about to write the same comment as Carl-Daniel. But, on the i945
0:1.0 is a bridge of which the (on i945 external PCIe) graphics hangs off.
So, the second 1.0 is indeed most likely supposed to be a device hanging
off a bridge at 0:1.0.


>  
>
>
>     >                       device pci 1e.0 on # PCI Bridge
>     > -                             #chip drivers/pci/onboard
>     >                               #       device pci 1.0 on end
>     >
>
>     Hm. Kill the above line? Could be a botched cut-n-paste.
>
> Could be.  Again, I couldn't tell.  Anyone with the board would know
> the first time they booted, so it could be removed.  I was trying to
> be minimally invasive.
Good.


>
>     > Index: svn/src/mainboard/gigabyte/ga_2761gxdk/Config.lb
>     > ===================================================================
>     > --- svn.orig/src/mainboard/gigabyte/ga_2761gxdk/Config.lb
>     > +++ svn/src/mainboard/gigabyte/ga_2761gxdk/Config.lb
>     > @@ -178,9 +178,7 @@ chip northbridge/amd/amdk8/root_complex
>     >                               chip southbridge/sis/sis966
>     >                                       device pci 0.0 on end   #
>     Northbridge
>     >                                       device pci 1.0 on        
>           # AGP bridge
>     > -                                       chip drivers/pci/onboard
>          # Integrated VGA
>     >                                               device pci 0.0 on end
>     >
>
>     This looks fishy, but then again, I never understood the v2 device
>     tree
>     syntax completely.
>

> I'm not sure what looks fishy here.  This is the way I understand it:
> 1. Devices between the 'on' and 'end' tokens are children (or children
> of children) of the device.
> 2. The 'chip' token assigns the driver for devices inside it.
>
> So in this case, device 1.0 has an AGP bus with 0.0 hanging off of it.
In which case the inner device pci 0.0 on end should be removed.
Otherwise it gets assigned an on_mainboard = 1, and be treated as an
on-mainboard graphics card even though it's an AGP plugin card.

Generally, the static tree should only contain devices that can not be
removed

But, to be sure we might want to trust the original authors of the
device trees that they knew what they were doing unless someone reports
a problem or fixes one.

Stefan