Hello Bjorn,
Again, reviving this very old thread :-)
On Thu, 5 Oct 2017 12:23:30 -0500, Bjorn Helgaas wrote:
- if (PCI_SLOT(devfn) != 0) {
- if ((bus->number == pcie->root_bus_nr) && (PCI_SLOT(devfn) != 0)) {
I'm fine with this, but please take a look at these:
8e7ca8ca5fd8 PCI: xilinx: Relax device number checking to allow SR-IOV e18934b5e9c7 PCI: designware: Relax device number checking to allow SR-IOV d99e30b7936a PCI: altera: Relax device number checking to allow SR-IOV
and make sure that reasoning doesn't apply here, too.
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8e... http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e1... http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d9...
The original code for xilinx/designware/altera was doing:
if (bus->number == port->root_busno && devfn > 0) return false;
if (bus->primary == port->root_busno && devfn > 0) return false;
I.e, it was checking both if bus->number *and* bus->primary were equal to port->root_busno.
The commit you points removed the check on bus->primary, keeping the check on bus->number.
Your patch for the Aadvark driver only adds a check on bus->number, i.e exactly what the xilinx/designware/altera code is still doing today:
Altera:
/* access only one slot on each root port */ if (bus->number == pcie->root_bus_nr && dev > 0) return false;
Designware:
/* access only one slot on each root port */ if (bus->number == pp->root_bus_nr && dev > 0) return 0;
Xilinx:
/* Only one device down on each root port */ if (bus->number == port->root_busno && devfn > 0) return false;
Aardvark (with our patch):
if ((bus->number == pcie->root_bus_nr) && (PCI_SLOT(devfn) != 0)) { *val = 0xffffffff; return PCIBIOS_DEVICE_NOT_FOUND; }
So we're doing exactly the same thing.
Do you agree ?
Best regards,
Thomas Petazzoni