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 10/31/2013 11:06 PM, Andrew Morton wrote:
> On Thu, 31 Oct 2013 16:54:44 -0500 scameron@beardog.cce.hp.com wrote:
>
>>>> How did this ever work?
>>> Beats me. local_pci_probe() does
>>>
>>> rc = pci_drv->probe(pci_dev, ddi->id);
>>> if (rc) {
>>> pci_dev->driver = NULL;
>>> pm_runtime_put_sync(dev);
>>> }
>>> return rc;
>>>
>>> shrug, maybe this ->probe somehow has a different caller which checks
>>> for <0.
>> Older kernels (eg: http://lxr.linux.no/#linux+v2.6.32.61/drivers/pci/pci-driver.c )
>> had different code:
>>
>> 330__pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
>> 331{
>> 332 const struct pci_device_id *id;
>> 333 int error = 0;
>> 334
>> 335 if (!pci_dev->driver && drv->probe) {
>> 336 error = -ENODEV;
>> 337
>> 338 id = pci_match_device(drv, pci_dev);
>> 339 if (id)
>> 340 error = pci_call_probe(drv, pci_dev, id);
>> 341 if (error >= 0) {
>> 342 pci_dev->driver = drv;
>> 343 error = 0;
>> 344 }
>> 345 }
>> 346 return error;
>> 347}
> So cciss is presently kompletely kaput? If so, the kapputting code is
> present in 3.9 and probably earlier, so this patch is needed in 3.12 and
> -stable. Or if not, what?

Steve posted similar patch for hpsa too and I'm sure someone would notice
when the hpsa driver was kaput. If I understand it correctly the driver
works with some limitation - rmmod doesn't work for example.

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;
}

>
> (Playing question and answer like this is a bad way of writing a
> changelog btw - all this stuff should have been right there in the v1
> changelog).
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/



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