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

[Carl-Daniel Hailfinger]

On 06.11.2009 22:02, Myles Watson wrote:
> On Fri, Nov 6, 2009 at 1:14 PM, Myles Watson  wrote:
> > On Fri, Nov 6, 2009 at 12:56 PM, Myles Watson  wrote:
> > > On Fri, Nov 6, 2009 at 12:41 PM, Peter Stuge  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

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 
My comments are not strong requirements for a commit, I just would like
to know if some of this stuff was intentional.
> 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? Kill?
>         #end
>
> end
> chip southbridge/intel/i82801xx # Southbridge
>   			register "ide0_enable" = "1"
>   			register "ide1_enable" = "1"
> 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.
> 	        #end
> end
> device pci 1f.0 on  # ISA/LPC? Bridge
> 	chip superio/smsc/lpc47b272
> Index: svn/src/mainboard/asus/mew-vm/devicetree.cb
>
> --- svn.orig/src/mainboard/asus/mew-vm/devicetree.cb
> +++ svn/src/mainboard/asus/mew-vm/devicetree.cb
> @@ -2,18 +2,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

See above.
>         #end
>
> end
> chip southbridge/intel/i82801xx # Southbridge
>   			register "ide0_enable" = "1"
>   			register "ide1_enable" = "1"
> device pci 1e.0 on # PCI Bridge
> 		#chip drivers/pci/onboard
> 	#	device pci 1.0 on end

Dito.
> 	        #end
> end
> device pci 1f.0 on  # ISA/LPC? Bridge
> 	chip superio/smsc/lpc47b272
> 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.
> 			  end
> 		end
>         			device pci 2.0 on # LPC
> 			chip superio/ite/it8716f

Regards,
Carl-Daniel