Some basic changes to allocation bus range, MMIO resource for SR-IOV device.
And add new sysfs entry to hotplug core to pass parameter to a slot, which will be used by SR-IOV code.
Signed-off-by: Yu Zhao <yu.zhao@intel.com>
Signed-off-by: Eddie Dong <eddie.dong@intel.com>
---
drivers/pci/bus.c | 63 +++++++++++++-------------
drivers/pci/hotplug/pci_hotplug_core.c | 75 +++++++++++++++++++++++++++++---
drivers/pci/pci-sysfs.c | 4 +-
drivers/pci/pci.c | 68 +++++++++++++++++++++--------
drivers/pci/pci.h | 3 +
drivers/pci/probe.c | 37 ++++++++-------
drivers/pci/proc.c | 7 ++-
drivers/pci/remove.c | 3 +-
drivers/pci/setup-bus.c | 9 ++--
drivers/pci/setup-res.c | 29 ++++++------
include/linux/pci.h | 53 ++++++++++++++++-------
include/linux/pci_hotplug.h | 11 ++++-
12 files changed, 246 insertions(+), 116 deletions(-)
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 529d9d7..15f64c9 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -105,7 +105,7 @@ int pci_bus_add_device(struct pci_dev *dev)
void pci_bus_add_devices(struct pci_bus *bus)
{
struct pci_dev *dev;
- struct pci_bus *child_bus;
+ struct pci_bus *child;
int retval;
list_for_each_entry(dev, &bus->devices, bus_list) {
@@ -115,43 +115,42 @@ void pci_bus_add_devices(struct pci_bus *bus)
retval = pci_bus_add_device(dev);
if (retval)
dev_err(&dev->dev, "Error adding device, continuing\n");
- }
-
- list_for_each_entry(dev, &bus->devices, bus_list) {
- BUG_ON(!dev->is_added);
/*
* If there is an unattached subordinate bus, attach
* it and then scan for unattached PCI devices.
*/
- if (dev->subordinate) {
- if (list_empty(&dev->subordinate->node)) {
- down_write(&pci_bus_sem);
- ...I was reading this patch, expecting to see a change to the
hotplug core _API_ taking a param, not just a new sysfs entry.
I would suggest rewording this part of the changelog as:
Add new sysfs file 'param' to /sys/bus/pci/slots/.../
which allows the user to pass a parameter to a slot. This
parameter will be used by the SR-IOV code.
Please break the 80-column "rule" and make this all one line, to
help with greppability.
I know the prior version had it broken across two lines too, but
So is this new file only used for SR-IOV? Or do you imagine it to
be general-purpose in the future? If SR-IOV only, then I suggest
a different name other than 'param', since that's a very generic
name for a very specific feature.
If you do imagine 'param' to be generally useful, then ignore
CodingStyle?
if (slot->ops->set_param || slot->ops->get_param)
return 0;
Seems slightly more readable to me, but it's just a suggestion;
feel free to ignore.
I guess this comment applies to the above line too; you don't
Why not:
for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
Looks like the tradeoff is explicit mention of PCI_ROM_RESOURCE
vs using a non-standard C idiom (personally, I'm not a huge fan
of <= in a for loop, but ymmv).
Ouch, this enum makes my head hurt a little. Care to put in a
comment for PCI_BRIDGE_RES_END, saying that it has a value of 10?
Thanks,
/ac
--
It can be PCI_BRIDGE_RESOURCES, because there may be some non-standard resources following PCI_ROM_RESOURCE and before PCI_BRIDGE_RESOURCES.
For example, a standard PCI device has following resources:
0 - 5 BARs
6 ROM
7 - 10 Bridge
After SR-IOV is enabled, it becomes
0 - 5 standard BARs
6 Rom
7 - 12 SR-IOV BARs
Same as above, the PCI_BRIDGE_RES_END varies when some features is enabled or disabled.
--
Ok, makes sense now; was that documented somewhere else, and I Sure, you can Cc me next time too. Thanks. /ac --
