lkml.org 
[lkml]   [2013]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] cciss: return 0 from driver probe function on success, not 1
On Fri, Nov 01, 2013 at 06:31:10AM -0700, Andrew Morton wrote:
> On Fri, 01 Nov 2013 14:06:45 +0100 Tomas Henzl <thenzl@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



\
 
 \ /
  Last update: 2013-11-01 15:21    [W:0.036 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site