[coreboot] [PATCH] v3: move mcp55 subsystem ID setting to v3 model
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Tue Aug 12 19:49:46 CEST 2008
On 12.08.2008 05:55, ron minnich wrote:
> I can not say I really like this idea at all.
>
> We have this ops_pci thing in the device, which I left in there
> probably by mistake in 2007.
>
> And for what reason? so we can do one pci operation.
>
AFAICS the infrastructure is generic to be able to set HT subsystem IDs
etc. No idea whether anything besides PCI has subsystem IDs.
> And on top of that, we need to set up an additional set of ops vectors
> with only one member:
>
Do you want to move the vector directly into struct device? Will this
really make stuff easier?
> +void mcp55_pci_dev_set_subsystem(struct device *dev, unsigned int vendor,
> + unsigned int device)
> +{
> + pci_write_config32(dev, PCI_CB_SUBSYSTEM_VENDOR_ID,
> + ((device & 0xffff) << 16) | (vendor & 0xffff));
> +}
> +
> +/** MCP55 specific device operation for PCI devices. */
> +struct pci_operations mcp55_pci_dev_ops_pci = {
> + .set_subsystem = mcp55_pci_dev_set_subsystem,
> +};
> +
>
> And what's that thing do anyway? Nothing special. It sets one
> register, the subsystem register, which most parts can handle in a
> generic way anyway.
We already have two incompatible subsystem setting functions in the
tree. The generic PCI one does not work on MCP55 and vice versa.
> And, we can do it in phase6 as I started to do --
> all it requires is that we add subsystem vendor and device id to the
> generic device. Given that the generic device already has some things
> in it that are pci-centric, I think we can afford two more u16s :-)
>
OK, then do it in phase6.
> Finally, we can do all this setting of the values of subsystem vendor
> and device id at compile time (or I thought we could) so why not do it
> then? Then, when we want to view the tree, we can see more
> information.
>
I still have trouble following the twisted maze called dtc.
> This patch is just bringing back the v2 way of doing things, and I
> would really hope we can avoid that.
>
> Can you take a look at my proposal to use the dtc to set this stuff
> and let me know if that might work instead? I really don't want to do
> this the v2 way.
>
> If you really want this mcp55_pci_dev_set_subsystem, let's just put it
> in the generic device and kill the pci_ops *. It's simpler.
>
Hm. New iteration/compromise follows. It also addresses the comments you
made in a private mail. My primary goal for this patch was to fix bugs
and point out the places where it can be changed to fit your suggestions.
Regards,
Carl-Daniel
Move mcp55 subsystem ID setting to the v3 model.
This fixes a genuine bug in the MCP55 code.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.h
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.h (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.h (Arbeitskopie)
@@ -22,6 +22,10 @@
#ifndef MCP55_H
#define MCP55_H
+#define PCI_MCP55_SUBSYSTEM_VENDOR_ID 0x40
+void mcp55_pci_dev_set_subsystem(struct device *dev, unsigned int vendor,
+ unsigned int device);
+struct pci_operations mcp55_pci_dev_ops_pci;
#endif /* MCP55_H */
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/ide.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/ide.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/ide.c (Arbeitskopie)
@@ -68,12 +68,6 @@
#ifdef CONFIG_PCI_ROM_RUN
pci_dev_init(dev);
#endif
-
-#warning set subsystem id on mcp55 ide
-#if 0
- pci_write_config32(dev, 0x40,
- ((device & 0xffff) << 16) | (vendor & 0xffff));
-#endif
}
struct device_operations mcp55_ide = {
@@ -86,6 +80,6 @@
.phase4_set_resources = pci_dev_set_resources,
.phase5_enable_resources = pci_dev_enable_resources,
.phase6_init = ide_init,
- .ops_pci = &pci_dev_ops_pci,
+ .ops_pci = &mcp55_pci_dev_ops_pci,
};
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb2.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb2.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb2.c (Arbeitskopie)
@@ -44,11 +44,6 @@
dword = pci_read_config32(dev, 0xf8);
dword |= 40;
pci_write_config32(dev, 0xf8, dword);
-#warning mange set subsystem in mcp55 usb2
-#if 0
- pci_write_config32(dev, 0x40,
- ((device & 0xffff) << 16) | (vendor & 0xffff));
-#endif
}
static void usb2_set_resources(struct device *dev)
@@ -84,5 +79,5 @@
.phase4_set_resources = usb2_set_resources,
.phase5_enable_resources = pci_dev_enable_resources,
.phase6_init = usb2_init,
- .ops_pci = &pci_dev_ops_pci,
+ .ops_pci = &mcp55_pci_dev_ops_pci,
};
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/sata.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/sata.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/sata.c (Arbeitskopie)
@@ -67,13 +67,6 @@
dword = pci_read_config32(dev, 0xf8);
dword |= 2;
pci_write_config32(dev, 0xf8, dword);
-
-
-#warning finish set subsystem in mcp55 sata
-#if 0
- pci_write_config32(dev, 0x40,
- ((device & 0xffff) << 16) | (vendor & 0xffff));
-#endif
}
struct device_operations mcp55_sata = {
@@ -86,5 +79,5 @@
.phase4_set_resources = pci_dev_set_resources,
.phase5_enable_resources = pci_dev_enable_resources,
.phase6_init = sata_init,
- .ops_pci = &pci_dev_ops_pci,
+ .ops_pci = &mcp55_pci_dev_ops_pci,
};
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/usb.c (Arbeitskopie)
@@ -34,11 +34,6 @@
static void usb_init(struct device * dev)
{
-#warning handle subsystem set in mcp55 usb
-#if 0
- pci_write_config32(dev, 0x40,
- ((device & 0xffff) << 16) | (vendor & 0xffff));
-#endif
}
struct device_operations mcp55_usb = {
@@ -51,5 +46,5 @@
.phase4_set_resources = pci_dev_set_resources,
.phase5_enable_resources = pci_dev_enable_resources,
.phase6_init = usb_init,
- .ops_pci = &pci_dev_ops_pci,
+ .ops_pci = &mcp55_pci_dev_ops_pci,
};
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/mcp55.c (Arbeitskopie)
@@ -248,6 +248,18 @@
}
+void mcp55_pci_dev_set_subsystem(struct device *dev, unsigned int vendor,
+ unsigned int device)
+{
+ pci_write_config32(dev, PCI_MCP55_SUBSYSTEM_VENDOR_ID,
+ ((device & 0xffff) << 16) | (vendor & 0xffff));
+}
+
+/** MCP55 specific device operation for PCI devices. */
+struct pci_operations mcp55_pci_dev_ops_pci = {
+ .set_subsystem = mcp55_pci_dev_set_subsystem,
+};
+
struct device_operations nvidia_ops = {
.id = {.type = DEVICE_ID_PCI,
{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
Index: corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/smbus.c
===================================================================
--- corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/smbus.c (Revision 749)
+++ corebootv3-mcp55_subsystem/southbridge/nvidia/mcp55/smbus.c (Arbeitskopie)
@@ -127,24 +127,19 @@
if (res)
pm_base = res->base;
#endif
-#warning finish subsystem set in mcp55 smbus
-#if 0
- pci_write_config32(dev, 0x40,
- ((device & 0xffff) << 16) | (vendor & 0xffff));
-#endif
}
struct device_operations mcp55_smbus = {
.id = {.type = DEVICE_ID_PCI,
{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
.device = PCI_DEVICE_ID_NVIDIA_MCP55_SM2}}},
- .constructor = default_device_constructor,
- .phase3_scan = scan_static_bus,
+ .constructor = default_device_constructor,
+ .phase3_scan = scan_static_bus,
.phase4_read_resources = mcp55_sm_read_resources,
.phase4_set_resources = pci_dev_set_resources,
.phase5_enable_resources = pci_dev_enable_resources,
- .phase6_init = mcp55_sm_init,
- .ops_pci = &pci_dev_ops_pci,
- .ops_smbus_bus = &lops_smbus_bus,
+ .phase6_init = mcp55_sm_init,
+ .ops_pci = &mcp55_pci_dev_ops_pci,
+ .ops_smbus_bus = &lops_smbus_bus,
};
Index: corebootv3-mcp55_subsystem/device/pci_device.c
===================================================================
--- corebootv3-mcp55_subsystem/device/pci_device.c (Revision 749)
+++ corebootv3-mcp55_subsystem/device/pci_device.c (Arbeitskopie)
@@ -620,26 +620,47 @@
printk(BIOS_SPEW, "Adding RAM resource (%lld bytes)\n", res->size);
}
-void pci_dev_enable_resources(struct device *dev)
+void pci_dev_set_subsystem_wrapper(struct device *dev)
{
const struct pci_operations *ops;
- u16 command;
+ u16 vendor = 0;
+ u16 device = 0;
+#warning Per-device subsystem ID has to be set here, but for that we have to call this function with the matching dts struct.
+
+#ifdef HAVE_MAINBOARD_PCI_SUBSYSTEM_ID
+ /* If there's no explicit subsystem ID for this device and the device
+ * and the device is onboard, use the board defaults. */
+ if (dev->on_mainboard) {
+ if (!vendor)
+ vendor = mainboard_pci_subsystem_vendor;
+ if (!device)
+ device = mainboard_pci_subsystem_device;
+ }
+#endif
/* Set the subsystem vendor and device ID for mainboard devices. */
ops = ops_pci(dev);
-#ifdef HAVE_MAINBOARD_PCI_SUBSYSTEM_ID
- if (dev->on_mainboard && ops && ops->set_subsystem) {
+ /* If either vendor or device is zero, we leave it as is. */
+ if (ops && ops->set_subsystem && vendor && device) {
printk(BIOS_DEBUG,
"%s: Setting subsystem VID/DID to %02x/%02x\n",
- dev_path(dev), mainboard_pci_subsystem_vendor,
- mainboard_pci_subsystem_device);
+ dev_path(dev), vendor, device);
- ops->set_subsystem(dev, mainboard_pci_subsystem_vendor,
- mainboard_pci_subsystem_device);
+ ops->set_subsystem(dev, vendor, device);
+ } else {
+ printk(BIOS_DEBUG, "%s: Not setting subsystem VID/DID\n",
+ dev_path(dev));
}
-#endif
+
+}
+void pci_dev_enable_resources(struct device *dev)
+{
+ u16 command;
+
+ pci_dev_set_subsystem_wrapper(dev);
+
command = pci_read_config16(dev, PCI_COMMAND);
command |= dev->command;
command |= (PCI_COMMAND_PARITY + PCI_COMMAND_SERR); // Error check.
--
http://www.hailfinger.org/
More information about the coreboot
mailing list