pci_bus conversion to struct device
RFC 1/1 pci_bus conversion to struct device
Hi Matt,
I'm trying to clean up all the usages of struct class_device to use struct device, and I ran into the pci_bus code. Right now you create a symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory to the pci device that is the bridge.
This is messy to try to convert to struct device, but I have hack of a patch below. I had some questions for you:
- is there userspace tools that use the 'bridge' symlink?
- do we really need a separate device here? If I just create a tree of symlinks to the pci devices that the bridges are, and add the sysfs attributes that are currently in the class_device, to the pci_device location, will that be acceptable?
Any thoughts you have about this would be appreciated.
thanks,
greg k-h
--- drivers/pci/bus.c | 10 ++++----- drivers/pci/pci.h | 2 - drivers/pci/probe.c | 54 +++++++++++++++++++++++++-------------------------- drivers/pci/remove.c | 7 ++---- include/linux/pci.h | 4 +-- 5 files changed, 38 insertions(+), 39 deletions(-)
drivers/pci/bus.c +5 -5
@@ -138,11 +138,11 @@ void __devinit pci_bus_add_devices(strucup_write (&pci_bus_sem); }pci_bus_add_devices (dev->subordinate); - retval =sysfs_create_link (&dev->subordinate->class_dev.kobj, - &dev->dev.kobj,"bridge" ); -if (retval) -dev_err (&dev->dev,"Error creating sysfs " -"bridge symlink, continuing...\n" ); +// retval = sysfs_create_link(&dev->subordinate->class_dev.kobj, +// &dev->dev.kobj, "bridge"); +// if (retval) +// dev_err(&dev->dev, "Error creating sysfs " +// "bridge symlink, continuing...\n"); } } }
drivers/pci/pci.h +1 -1
@@ -78,7 +78,7 @@ static inline int pci_no_d1d2(struct pci }extern int pcie_mch_quirk;extern struct device_attribute pci_dev_attrs[]; -extern struct class_device_attribute class_device_attr_cpuaffinity; +extern struct device_attribute dev_attr_cpuaffinity;/** * pci_match_one_device - Tell if a PCI device structure has a matching
drivers/pci/probe.c +27 -27
132 lines not on this page · raw message
drivers/pci/remove.c +3 -4
@@ -74,10 +74,9 @@ void pci_remove_bus(struct pci_bus *pci_list_del (&pci_bus->node);up_write (&pci_bus_sem);pci_remove_legacy_files (pci_bus); -class_device_remove_file (&pci_bus->class_dev, - &class_device_attr_cpuaffinity); -sysfs_remove_link (&pci_bus->class_dev.kobj,"bridge" ); -class_device_unregister (&pci_bus->class_dev); +device_remove_file (&pci_bus->dev, &dev_attr_cpuaffinity); +// sysfs_remove_link(&pci_bus->class_dev.kobj, "bridge"); +device_unregister (&pci_bus->dev); }EXPORT_SYMBOL (pci_remove_bus);
include/linux/pci.h +2 -2
@@ -251,13 +251,13 @@ struct pci_bus {unsigned short bridge_ctl;/* manage NO_ISA/FBB/et al behaviors */ pci_bus_flags_t bus_flags;/* Inherited by child busses */ struct device *bridge; -struct class_device class_dev; +struct device dev;struct bin_attribute *legacy_io;/* legacy I/O for this bus */ struct bin_attribute *legacy_mem;/* legacy mem */ };#define pci_bus_b (n) list_entry(n, struct pci_bus, node) -#define to_pci_bus (n) container_of(n, struct pci_bus, class_dev) +#define to_pci_bus (n) container_of(n, struct pci_bus, dev)/* * Error values that may be returned by PCI functions.
Quoted 4 lines · Greg KH
I'm trying to clean up all the usages of struct
class_deviceto use struct device, and I ran into thepci_buscode. Right now you create a symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory to the pci device that is the bridge.
I recommend we just delete the pci_bus class. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
Quoted 11 lines · Matthew Wilcox
Quoted 4 lines · Greg KH
I'm trying to clean up all the usages of struct
class_deviceto use struct device, and I ran into thepci_buscode. Right now you create a symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory to the pci device that is the bridge.I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
I would like to do that, but I want to make sure that no userspace tools are using this information.
But, as Matt Dobson is now gone off somewhere in Europe, not doing Linux stuff anymore, he's not going to answer this. So I'll just make up a removal patch and let it sit in -mm for a while to see if anything breaks.
I really only think the big NUMA boxes care about that information, if anything.
thanks,
greg k-h
Hello!
Quoted 5 lines
I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
It doesn't seem to serve any useful purpose other than the affinity now, but I still think that it conceptually belongs there, because it makes sense to have per-bus attributes. For example, in the future we could show data width and signalling speed.
Have a nice fortnight
Signature · 3 lines
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth "I invented the term Object-Oriented, and I can tell you I did not have C++ in mind." -- Alan Kay
Quoted 8 lines · Martin Mares
Hello!
Quoted 5 lines
I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
Quoted 4 lines
It doesn't seem to serve any useful purpose other than the affinity now, but I still think that it conceptually belongs there, because it makes sense to have per-bus attributes. For example, in the future we could show data width and signalling speed.
So, if it were to stay, where in the tree should it be? Hanging off of the pci device that is the bridge? Or just placing these files within the pci device directory itself, as it is the bridge.
There are also some "legacy io" binary sysfs files in these directories for those platforms that support it (#ifdef HAVE_PCI_LEGACY), and I'm guessing that there is some user for them out there, otherwise they would not have been added...
Hm, only ia64 enables that option. Matthew, do you care about those files?
thanks,
greg k-h
Quoted 19 lines · Greg KH
Quoted 9 lines · Martin Mares
Hello!
Quoted 6 lines
I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.Quoted 4 lines
It doesn't seem to serve any useful purpose other than the affinity now, but I still think that it conceptually belongs there, because it makes sense to have per-bus attributes. For example, in the future we could show data width and signalling speed.
So, if it were to stay, where in the tree should it be? Hanging off of the pci device that is the bridge? Or just placing these files within the pci device directory itself, as it is the bridge.
Quoted 7 lines
There are also some "legacy io" binary sysfs files in these directories for those platforms that support it (#ifdef
HAVE_PCI_LEGACY), and I'm guessing that there is some user for them out there, otherwise they would not have been added...Hm, only ia64 enables that option. Matthew, do you care about those files?
Those interfaces could be supported on other platforms (iirc benh has been planning to add legacy_* support to ppc for awhile). It would be great if they were supported everywhere to provide userspace with a fairly sane way to get at this stuff on all Linux systems...
Should be trivial on i386 I think.
Jesse
Hello!
Quoted 3 lines
So, if it were to stay, where in the tree should it be? Hanging off of the pci device that is the bridge? Or just placing these files within the pci device directory itself, as it is the bridge.
I originally didn't realize that we already represent devices on the subordinate bus as subdirectories of the bridge device's directory, without any extra level of abstraction. So it probably makes sense to put the bus-specific files there or, in case of a root bus, to /sys/devices/pciXXXX:YY/.
Have a nice fortnight
Signature · 3 lines
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/ Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth "It is only with the heart that one can see rightly; What is essential is invisible to the eye." -- The Little Prince
Quoted 2 lines · Greg KH
Hm, only ia64 enables that option. Matthew, do you care about those files?
Given the ia64 nature, unless benh was truly wanting to do something or ppc64, IBM's big NUMA boxes are pretty unlikely to care.
Tim
Quoted 7 lines · Tim Pepper
Quoted 2 lines · Greg KH
Hm, only ia64 enables that option. Matthew, do you care about those files?
Given the ia64 nature, unless benh was truly wanting to do something or ppc64, IBM's big NUMA boxes are pretty unlikely to care.
I'd still like to do something at one point for ppc and ppc64... or others. It's a generic problem with non-x86 machines.
In addition, there is the problem of legacy VGA arbitration
Part of the problem with legacy VGA accesses is also that it needs some level of arbitration which we currently don't do in the kernel at all. X does some stuff, but it doesn't necessarily arbitrate well with other things for obvious reasons :-)
I wrote some code for that a little while ago but never quite finished, and X would have had to be ported over. I might give it a go again one of these days though. Now that X is slowly moving over some sane PCI access library, there is a good opportunity to slap in support for a nice kernel based VGA access arbitration interface.
Ben.
Quoted 9 lines · Greg KH
Quoted 8 lines · Martin Mares
Hello!
Quoted 5 lines
I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
Quoted 4 lines
Quoted 4 lines
It doesn't seem to serve any useful purpose other than the affinity now, but I still think that it conceptually belongs there, because it makes sense to have per-bus attributes. For example, in the future we could show data width and signalling speed.
Data width is kind of hard to figure out since it's negotiated per transaction. One could conceive of a device which only does 32-bit transactions to some addresses, and 64-bit transactions to others.
What I've done in recent patches is make these kinds of attributes available per slot.
Quoted 3 lines
So, if it were to stay, where in the tree should it be? Hanging off of the pci device that is the bridge? Or just placing these files within the pci device directory itself, as it is the bridge.
/sys/bus/pci/busses ?
Quoted 7 lines
There are also some "legacy io" binary sysfs files in these directories for those platforms that support it (#ifdef
HAVE_PCI_LEGACY), and I'm guessing that there is some user for them out there, otherwise they would not have been added...Hm, only ia64 enables that option. Matthew, do you care about those files?
I think they were added for Altix ... not sure who uses them. Maybe X?
Quoted 10 lines · Matthew Wilcox
Quoted 7 lines
There are also some "legacy io" binary sysfs files in these directories for those platforms that support it (#ifdef
HAVE_PCI_LEGACY), and I'm guessing that there is some user for them out there, otherwise they would not have been added...Hm, only ia64 enables that option. Matthew, do you care about those files?
I think they were added for Altix ... not sure who uses them. Maybe X?
Yes. Though like I said, those interfaces make sense on other platforms too, with weird ISA I/O and memory space mapping requirements.
Jesse
Quoted 8 lines · Martin Mares
Hello!
Quoted 5 lines
I recommend we just delete the
pci_busclass. I don't think it serves any useful purpose. The bridge can be inferred frmo the sysfs hierarchy (not to mention lspci will tell you). The cpuaffinity file should be moved from the bus to the device -- it really doesn't make any sense to talk about which cpu a bus is affine to, only a device.
Quoted 4 lines
It doesn't seem to serve any useful purpose other than the affinity now, but I still think that it conceptually belongs there, because it makes sense to have per-bus attributes. For example, in the future we could show data width and signalling speed.
Other per bus attributes might be address routing, VGA routing enabled, Fast-back-to-back enabled. PCI-X bridges and PCI-e bridges might also advertise data related to MMRBC and similar onboard buffer mgt behaviors.
ISTR, IBM PCI-X bridge works better with 512 "block" (data xfer size) than larger sizes becuase it internally allocates buffer space in 512B chunks. It would be useful to know along with downstream device MMRBC. Not sure this all has to come from /sys though.
grant
Quoted 8 lines · Grant Grundler
Other per bus attributes might be address routing, VGA routing enabled, Fast-back-to-back enabled. PCI-X bridges and PCI-e bridges might also advertise data related to MMRBC and similar onboard buffer mgt behaviors.
ISTR, IBM PCI-X bridge works better with 512 "block" (data xfer size) than larger sizes becuase it internally allocates buffer space in 512B chunks. It would be useful to know along with downstream device MMRBC. Not sure this all has to come from /sys though.
Most of this comes best from lspci / setpci.