| Message ID | 20210519235426.99728-2-ameynarkhede03@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Expose and manage PCI device reset | expand |
Hi Amey, Thank you for working on this! Few comments and suggestions below. [...] > Link: https://lkml.org/lkml/2021/3/23/911 Linking to lkml.org is fine, however it became a canon now to link to lore, so this would be: https://lore.kernel.org/lkml/20210323100625.0021a943@omen.home.shazbot.org/ I personally find it a bit easier to read on lore compared to lkml.org when it goes to a large and long running threads. [...] > +int pci_reset_bus_function(struct pci_dev *dev, int probe) > +{ > + int rc = pci_dev_reset_slot_function(dev, probe); > + > + if (rc != -ENOTTY) > + return rc; > + return pci_parent_bus_reset(dev, probe); > +} Depends on the style, but I would suggest using a boolean type for the probe argument here and in the other functions that enable or disable something. I makes the intent clear, and this is also a popular pattern you can see throughout the PCI tree. Also, I would suggest adding a newline to separate final return, so that it's easier to read the code, and to keep things consistent. [...] > - rc = pci_dev_reset_slot_function(dev, 0); > - if (rc != -ENOTTY) > - return rc; > - return pci_parent_bus_reset(dev, 0); > + return pci_reset_bus_function(dev, 0); See above about using boolean type here. [...] > - rc = pci_dev_reset_slot_function(dev, 1); > if (rc != -ENOTTY) > return rc; > > - return pci_parent_bus_reset(dev, 1); > + return pci_reset_bus_function(dev, 1); Same as above. Krzysztof
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 16a17215f..a8f8dd588 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4982,6 +4982,15 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe) return pci_reset_hotplug_slot(dev->slot->hotplug, probe); } +int pci_reset_bus_function(struct pci_dev *dev, int probe) +{ + int rc = pci_dev_reset_slot_function(dev, probe); + + if (rc != -ENOTTY) + return rc; + return pci_parent_bus_reset(dev, probe); +} + static void pci_dev_lock(struct pci_dev *dev) { pci_cfg_access_lock(dev); @@ -5102,10 +5111,7 @@ int __pci_reset_function_locked(struct pci_dev *dev) rc = pci_pm_reset(dev, 0); if (rc != -ENOTTY) return rc; - rc = pci_dev_reset_slot_function(dev, 0); - if (rc != -ENOTTY) - return rc; - return pci_parent_bus_reset(dev, 0); + return pci_reset_bus_function(dev, 0); } EXPORT_SYMBOL_GPL(__pci_reset_function_locked); @@ -5135,13 +5141,10 @@ int pci_probe_reset_function(struct pci_dev *dev) if (rc != -ENOTTY) return rc; rc = pci_pm_reset(dev, 1); - if (rc != -ENOTTY) - return rc; - rc = pci_dev_reset_slot_function(dev, 1); if (rc != -ENOTTY) return rc; - return pci_parent_bus_reset(dev, 1); + return pci_reset_bus_function(dev, 1); } /** diff --git a/include/linux/pci.h b/include/linux/pci.h index 86c799c97..979d54335 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1228,6 +1228,7 @@ int pci_probe_reset_bus(struct pci_bus *bus); int pci_reset_bus(struct pci_dev *dev); void pci_reset_secondary_bus(struct pci_dev *dev); void pcibios_reset_secondary_bus(struct pci_dev *dev); +int pci_reset_bus_function(struct pci_dev *dev, int probe); void pci_update_resource(struct pci_dev *dev, int resno); int __must_check pci_assign_resource(struct pci_dev *dev, int i); int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);