[PATCH] Drop drivers/pci/onboard

[Myles Watson]

PCI onboard was only used for ROM images at fixed offsets in flash.  Now
that we have CBFS, there is no use for it.

Fix qemu, since it is a special case where the ROM image ends up at 0xc0000
without any help from coreboot.

Signed-off-by: Myles Watson

Abuild tested.  Boot tested on qemu.

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 06.11.2009 18:14, 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.
>
> Fix qemu, since it is a special case where the ROM image ends up at 0xc0000
> without any help from coreboot.
>
> Signed-off-by: Myles Watson

The patch is missing.

Regards,
Carl-Daniel

[Myles Watson]

On Fri, Nov 6, 2009 at 11:41 AM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> On 06.11.2009 18:14, 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.
> >
> > Fix qemu, since it is a special case where the ROM image ends up at
>
> 0xc0000
> > without any help from coreboot.
> >
> > Signed-off-by: Myles Watson
>
> The patch is missing.

Thanks.

Myles

[Peter Stuge]

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.
> > >
> > > Fix qemu, since it is a special case where the ROM image ends up
> > > at 0xc0000 without any help from coreboot.
> > >
> > > Signed-off-by: Myles Watson

I am inclined to say:

Acked-by: Peter Stuge

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?

//Peter

[Myles Watson]

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.

Thanks,
Myles

[Myles Watson]

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

Thanks,
Myles

[Myles Watson]

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

Thanks,
Myles

[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

[Myles Watson]

On Fri, Nov 6, 2009 at 3:34 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> 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

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

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 06.11.2009 22:47, Myles Watson wrote:
> On Fri, Nov 6, 2009 at 3:34 PM, Carl-Daniel Hailfinger 
>  wrote:
> > 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.

Ah.
> > 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.

Right. Good goal.
> I'm not sure what looks fishy here.  This is the way I understand it:
> 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.

Thanks for the explanation.
Go ahead!

Regards,
Carl-Daniel

[Myles Watson]

> Go ahead!

Rev 4925.

Thanks,
Myles

[Stefan Reinauer]

Myles Watson wrote:
> On Fri, Nov 6, 2009 at 3:34 PM, Carl-Daniel Hailfinger
> <[e-mail]
> > wrote:
> On 06.11.2009 22:02, Myles Watson wrote:
> > On Fri, Nov 6, 2009 at 1:14 PM, Myles Watson <[e-mail]
> <mailto:[e-mail]>> wrote:
> >
> >
> >> On Fri, Nov 6, 2009 at 12:56 PM, Myles Watson
> <[e-mail] <mailto:[e-mail]>> wrote:
> >>
> >>
> >>> On Fri, Nov 6, 2009 at 12:41 PM, Peter Stuge <[e-mail]
> <mailto:[e-mail]>> 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 <[e-mail]
> <mailto:[e-mail]>>
> >
>
> 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
> <[e-mail]
> <mailto:[e-mail]>>
>
> 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:
> 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

[Myles Watson]

> > > 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:
> > 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.

Yeah.  I thought maybe it was on-mainboard AGP.
> Generally, the static tree should only contain devices that can not be
> removed

Agreed.  That gets a little problematic with some of the "stuff options"
where some of the boards have SCSI or an extra nic or...

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.

Sounds good.

Thanks,
Myles

[Stefan Reinauer]

Myles Watson wrote:
> Signed-off-by: Myles Watson <[e-mail] >

Acked-by: Stefan Reinauer