On Tue, Apr 04, 2006 at 09:35:40PM -0400, Dmitry Torokhov wrote:
> On Tuesday 04 April 2006 17:45, Greg KH wrote:
> > Ah, I see what you are saying now. Yeah, we should still add the
> > default attributes for the bus and create the bus link even if some
> > random driver had problems. But then, we should still propagate the
> > error back up, right?
>
> I don't think so because device creation did not fail. Otherwise how
> would you as a caller of device_register() distinguish between the
> following 2 scenarios:
>
> - you got -ENOMEM (or other error code) because device creation
> indeed failed;
> - you got -ENOMEM because some odd driver could not allocate 4MB
> of memory.
>
> IOW you trying to propagate driver error to device creation code...
>
> Also result of device_register() should not depend on whether
> driver_register() was called earlier or not.
Indeed. Greg - this patch is bogus.
When device_register() returns a negative number, many device drivers
assume (correctly) that the device was _not_ registered and they will
take whatever cleanup action is necessary to free the memory associated
with that device, believing that sysfs never used it.
If we start propagating driver probe error codes up through device_register()
we then have a big headache. Is this -ENOMEM error a result of not being
able to register the struct device, or is it because something failed in
the driver.
This will also impact hotplug and PCMCIA. For example, you have a device
driver loaded, but are short on memory, and you insert a PCMCIA device.
That creates a struct device. Let's say that the driver fails to allocate
enough memory, and returns -ENOMEM. With this patch, that'll kill off
the struct device. Now, when the device is pulled, we could end up calling
driver_unregister on an unregistered struct device.
The expectations by current subsystems is that struct device registration
does _NOT_ depend on the currently loaded set of device drivers not
returning errors.
The thing is that this is all corner case stuff - you can apply this patch
and apparantly have a working system. It's the corner cases where some
driver returns a failure which are the problem case.
Note also that this patch will not do what the ALSA folk want - they want
to know if the device was found or whether the probe returned -ENODEV.
We knock -ENODEV and -ENXIO to zero in driver_probe_device(), so they
won't even see that.
There is also no 1:1 relationship between platform device drivers and
platform devices. You can have multiple platform devices driven by one
platform device driver. See the plethora of platform devices in the
ARM sub-tree. Or serial sub-tree.
So, all round this patch seems wrong.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
