On Fri, Nov 01, 2013 at 06:31:10AM -0700, Andrew Morton wrote:
> On Fri, 01 Nov 2013 14:06:45 +0100 Tomas Henzl <the...@redhat.com> wrote:
>
> > The problem in kernel is that the error handling in local_pci_probe
> > and in __pci_device_probe is different for ret values > 0,
> > so we should fix it somewhere so it is in sync.
> > The documentation states that the probe function should return zero on
> > success
> > so what about this -
> >
> > This would bring the handling to sync
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 98f7b9b..200a071 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -317,8 +317,6 @@ __pci_device_probe(struct pci_driver *drv, struct
> > pci_dev *pci_dev)
> > id = pci_match_device(drv, pci_dev);
> > if (id)
> > error = pci_call_probe(drv, pci_dev, id);
> > - if (error >= 0)
> > - error = 0;
> > }
> > return error;
> > }
>
> ah, there it is.
>
> This change would turn semi-kaput drivers into kaput-kaput drivers. It
> would be better to add a runtime warning here so those drivers get
> fixed. Such a warning would need to reliably identify the offending
> probe function so a simple WARN_ON() wouldn't be sufficient.
>
FWIW, I just booted up with the following change:
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 98f7b9b..ef71bb5 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -264,9 +264,16 @@ 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 < 0) {
pci_dev->driver = NULL;
pm_runtime_put_sync(dev);
+ } else {
+ if (rc > 0) {
+ dev_warn(dev,
+ "Driver probe function returned %d, greater
than 0\n", rc);
+ rc = 0;
+
+ }
}
return rc;
}
And,
[scameron@localhost linux-3.12-rc6]$ dmesg | grep 'Driver probe'
[scameron@localhost linux-3.12-rc6]$
Not that it means all that much since I don't have hardware for
the majority of drivers, obviously.
I think the above should make drivers with probe functions
returning > 0 "work" again, but with a warning.
The question would be, are there drivers which return > 0 and
by this value intend to convey that the probe function has failed?
-- steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
