RE: [PATCH 1/2] staging/comedi/drivers: release allocated I/O region if alloc_private fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Monday, July 02, 2012 1:41 AM, Ian Abbott wrote:
> No.  The I/O region will be deallocated in fl512_detach() because 
> dev->iobase has been set non-zero.  fl512_detach() will be called by the 
> comedi core if fl512_attach() returns an error.  This is an unusual 
> aspect of the comedi drivers.

I have been wondering if that aspect should be "fixed".

It's more typical for kernel drivers to clean up after themselves if
the probe/attach/init/etc. fails. And the release/detach/exit/etc.
is only called if the driver has successfully loaded.

With the comedi drivers, the "detach" is always called if the "attach"
failed. And, of course the "detach" is called when the device is removed.

Because of this the "detach" routines need to do all the checks to
see what needs to be cleaned up. Not a big deal but it does create
some confusion as this patch shows.

Ian, what's your opinion on this? Do you think we should refactor all
the driver "attach" routines so they clean up on failure and fix the
core so the "detach" is only called after a successful "attach"?

This would be a pretty big patch since it affects every driver as well
as the core. 

We could break it up by introducing a temporary flag in the comedi_driver
struct that indicates if the driver has been "fixed". The core could then
work as-is for non-updated drivers. Once all the drivers have been updated
we then fix the core and remove the flag from all the drivers.

Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux