| Message ID | 20220422132858.1213022-2-schnelle@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: Rework pci_scan_slot() and isolated PCI functions | expand |
On Fri, 2022-04-22 at 15:28 +0200, Niklas Schnelle wrote: > While determining the next PCI function is factored out of > pci_scan_slot() into next_fn() the former still handles the first > function as a special case. This duplicates the code from the scan loop. > > Furthermore the non ARI branch of next_fn() is generally hard to > understand and especially the check for multifunction devices is hidden > in the handling of NULL devices for non-contiguous multifunction. It > also signals that no further functions need to be scanned by returning > 0 via wraparound and this is a valid function number. > > Improve upon this by transforming the conditions in next_fn() to be > easier to understand. > > By changing next_fn() to return -ENODEV instead of 0 when there is no > next function we can then handle the initial function inside the loop > and deduplicate the shared handling. This also makes it more explicit > that only function 0 must exist. > > No functional change is intended. > > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > drivers/pci/probe.c | 39 ++++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 17a969942d37..2000e9858f12 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2579,8 +2579,7 @@ struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn) > } > EXPORT_SYMBOL(pci_scan_single_device); > > -static unsigned int next_fn(struct pci_bus *bus, struct pci_dev *dev, > - unsigned int fn) > +static int next_fn(struct pci_bus *bus, struct pci_dev *dev, int fn) > { > int pos; > u16 cap = 0; > @@ -2588,24 +2587,27 @@ static unsigned int next_fn(struct pci_bus *bus, struct pci_dev *dev, > > if (pci_ari_enabled(bus)) { > if (!dev) > - return 0; > + return -ENODEV; > + > Sorry the whitespace damage was of course not intended, fixed locally.
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 17a969942d37..2000e9858f12 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2579,8 +2579,7 @@ struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn) } EXPORT_SYMBOL(pci_scan_single_device); -static unsigned int next_fn(struct pci_bus *bus, struct pci_dev *dev, - unsigned int fn) +static int next_fn(struct pci_bus *bus, struct pci_dev *dev, int fn) { int pos; u16 cap = 0; @@ -2588,24 +2587,27 @@ static unsigned int next_fn(struct pci_bus *bus, struct pci_dev *dev, if (pci_ari_enabled(bus)) { if (!dev) - return 0; + return -ENODEV; + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI); if (!pos) - return 0; + return -ENODEV; pci_read_config_word(dev, pos + PCI_ARI_CAP, &cap); next_fn = PCI_ARI_CAP_NFN(cap); if (next_fn <= fn) - return 0; /* protect against malformed list */ + return -ENODEV; /* protect against malformed list */ return next_fn; } + if (fn >= 7) + return -ENODEV; - /* dev may be NULL for non-contiguous multifunction devices */ - if (!dev || dev->multifunction) - return (fn + 1) % 8; + /* only multifunction devices may have more functions */ + if (dev && !dev->multifunction) + return -ENODEV; - return 0; + return fn + 1; } static int only_one_child(struct pci_bus *bus) @@ -2643,26 +2645,25 @@ static int only_one_child(struct pci_bus *bus) */ int pci_scan_slot(struct pci_bus *bus, int devfn) { - unsigned int fn, nr = 0; struct pci_dev *dev; + int fn = 0, nr = 0; if (only_one_child(bus) && (devfn > 0)) return 0; /* Already scanned the entire slot */ - dev = pci_scan_single_device(bus, devfn); - if (!dev) - return 0; - if (!pci_dev_is_added(dev)) - nr++; - - for (fn = next_fn(bus, dev, 0); fn > 0; fn = next_fn(bus, dev, fn)) { + do { dev = pci_scan_single_device(bus, devfn + fn); if (dev) { if (!pci_dev_is_added(dev)) nr++; - dev->multifunction = 1; + if (fn > 0) + dev->multifunction = 1; + } else if (fn == 0) { + /* function 0 is required */ + break; } - } + fn = next_fn(bus, dev, fn); + } while (fn >= 0); /* Only one slot has PCIe device */ if (bus->self && nr)
While determining the next PCI function is factored out of pci_scan_slot() into next_fn() the former still handles the first function as a special case. This duplicates the code from the scan loop. Furthermore the non ARI branch of next_fn() is generally hard to understand and especially the check for multifunction devices is hidden in the handling of NULL devices for non-contiguous multifunction. It also signals that no further functions need to be scanned by returning 0 via wraparound and this is a valid function number. Improve upon this by transforming the conditions in next_fn() to be easier to understand. By changing next_fn() to return -ENODEV instead of 0 when there is no next function we can then handle the initial function inside the loop and deduplicate the shared handling. This also makes it more explicit that only function 0 must exist. No functional change is intended. Cc: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> --- drivers/pci/probe.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)