| Message ID | 20131101193455.18686.92302.stgit@beardog.cce.hp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, Nov 01, 2013 at 02:34:55PM -0500, Stephen M. Cameron wrote: > From: Stephen M. Cameron <scameron@beardog.cce.hp.com> > > Ages ago, drivers could return values greater than zero from > their probe function and this would be regarded as success. > Commit f3ec4f87d607f40497 "PCI: change device runtime PM > settings for probe and remove" slightly altered this in 2010, > and commit 967577b062417b4e4b8e27b "PCI/PM: Keep runtime PM > enabled for unbound PCI devices" in late 2012 altered it more > signficantly, setting pci_dev->driver to NULL if the driver's > probe function returned a value greater than zero, which would > for example prevent the driver's remove function from being > called on rmmod. > > Neither of those changes would necessarily make the driver fail > in an obvious way though, and so at least a couple drivers (cciss, > hpsa) fell into this hole since they were returning 1, and this > situation went unnoticed for quite some time. > > If a driver's probe function returns a value greater than zero, > issue a warning, but otherwise treat this as success. > > Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com> Applied to my pci/misc branch for v3.13, thanks. Bjorn > --- > drivers/pci/pci-driver.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 98f7b9b..7fbe343 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -264,11 +264,19 @@ static long local_pci_probe(void *_ddi) > pm_runtime_get_sync(dev); > pci_dev->driver = pci_drv; > rc = pci_drv->probe(pci_dev, ddi->id); > - if (rc) { > + if (!rc) > + return rc; > + if (rc < 0) { > pci_dev->driver = NULL; > pm_runtime_put_sync(dev); > + return rc; > } > - return rc; > + /* > + * Probe function should return < 0 for failure, 0 for success > + * Treat values > 0 as success, but warn. > + */ > + dev_warn(dev, "Driver probe function unexpectedly returned %d\n", rc); > + return 0; > } > > static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev, > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 98f7b9b..7fbe343 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -264,11 +264,19 @@ static long local_pci_probe(void *_ddi) pm_runtime_get_sync(dev); pci_dev->driver = pci_drv; rc = pci_drv->probe(pci_dev, ddi->id); - if (rc) { + if (!rc) + return rc; + if (rc < 0) { pci_dev->driver = NULL; pm_runtime_put_sync(dev); + return rc; } - return rc; + /* + * Probe function should return < 0 for failure, 0 for success + * Treat values > 0 as success, but warn. + */ + dev_warn(dev, "Driver probe function unexpectedly returned %d\n", rc); + return 0; } static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,