From patchwork Tue Jan 25 12:54:01 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Moved 'pci_set_subsystem' to a weak function: Support for Siemens
 Mainboard
Date: Tue, 25 Jan 2011 12:54:01 -0000
From: Patrick Georgi <Patrick.Georgi@secunet.com>
X-Patchwork-Id: 2556
Message-Id: <1295960041.2414.47.camel@linux-0a8x.site>
To: "Joseph Kellermann" <Joseph.Kellermann@heitec.de>
Cc: coreboot@coreboot.org

Am Dienstag, den 25.01.2011, 13:24 +0100 schrieb Georgi, Patrick:
> I'm not quite sure if using weak functions to wrap the subsystem
> CONFIG_* values is actually the right approach, I'll work on a patch to
> discuss.
Here it is.
I tested it by providing a mainboard_pci_subsystem_vendor_id for my
board in its mainboard.c which returned a different constant value -
this was picked up by properly and reported on boot.
mainboard_pci_subsystem_device_id will work just the same.

Joseph, will this suffice to help you implement your requirement?

Everyone, any opinion on the design?
My main issue is that other code can still use the CONFIG_* values
directly. Maybe our lint mechanism should look for that?
Any other issues you have with this?

Signed-off-by: Patrick Georgi <patrick.georgi@secunet.com>


Index: coreboot/src/devices/pci_device.c
===================================================================
--- coreboot.orig/src/devices/pci_device.c
+++ coreboot/src/devices/pci_device.c
@@ -586,6 +586,16 @@ void pci_dev_set_resources(struct device
 	pci_write_config8(dev, PCI_CACHE_LINE_SIZE, 64 >> 2);
 }
 
+unsigned __attribute__((weak)) mainboard_pci_subsystem_vendor_id(__attribute__((unused)) struct device *dev)
+{
+		return CONFIG_MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID;
+}
+
+unsigned __attribute__((weak)) mainboard_pci_subsystem_device_id(__attribute__((unused)) struct device *dev)
+{
+		return CONFIG_MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID;
+}
+
 void pci_dev_enable_resources(struct device *dev)
 {
 	const struct pci_operations *ops;
@@ -595,11 +605,11 @@ void pci_dev_enable_resources(struct dev
 	ops = ops_pci(dev);
 	if (dev->on_mainboard && ops && ops->set_subsystem) {
 		printk(BIOS_DEBUG, "%s subsystem <- %02x/%02x\n", dev_path(dev),
-		       CONFIG_MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID,
-		       CONFIG_MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID);
+		       mainboard_pci_subsystem_vendor_id(dev),
+		       mainboard_pci_subsystem_device_id(dev));
 		ops->set_subsystem(dev,
-				   CONFIG_MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID,
-				   CONFIG_MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID);
+				   mainboard_pci_subsystem_vendor_id(dev),
+				   mainboard_pci_subsystem_device_id(dev));
 	}
 	command = pci_read_config16(dev, PCI_COMMAND);
 	command |= dev->command;
Index: coreboot/src/include/device/pci.h
===================================================================
--- coreboot.orig/src/include/device/pci.h
+++ coreboot/src/include/device/pci.h
@@ -103,4 +103,7 @@ static inline const struct pci_bus_opera
 	return bops;
 }
 
+unsigned mainboard_pci_subsystem_vendor_id(struct device *dev);
+unsigned mainboard_pci_subsystem_device_id(struct device *dev);
+
 #endif /* PCI_H */
Index: coreboot/src/southbridge/intel/i82801gx/pci.c
===================================================================
--- coreboot.orig/src/southbridge/intel/i82801gx/pci.c
+++ coreboot/src/southbridge/intel/i82801gx/pci.c
@@ -73,11 +73,11 @@ static void ich_pci_dev_enable_resources
 	if (dev->on_mainboard && ops && ops->set_subsystem) {
 		printk(BIOS_DEBUG, "%s subsystem <- %02x/%02x\n",
 			dev_path(dev),
-			CONFIG_MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID,
-			CONFIG_MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID);
+			mainboard_pci_subsystem_vendor_id(dev),
+			mainboard_pci_subsystem_device_id(dev));
 		ops->set_subsystem(dev,
-			CONFIG_MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID,
-			CONFIG_MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID);
+			mainboard_pci_subsystem_vendor_id(dev),
+			mainboard_pci_subsystem_device_id(dev));
 	}
 
 	command = pci_read_config16(dev, PCI_COMMAND);