[coreboot] [PATCH] v3: move mcp55 subsystem ID setting to v3 model

ron minnich rminnich at gmail.com
Tue Aug 12 05:55:49 CEST 2008


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.

And on top of that, we need to set up an additional set of ops vectors
with onliy one member:
+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. 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 :-)

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.

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.

Thanks!

ron




More information about the coreboot mailing list