[cpia] USB anyone?

[sbertin@mindspring.com]

--1037094400-1804289383-948322032=:816
Content-Type: TEXT/plain; charset=us-ascii

On 19 Jan, Jochen Scharrlach wrote:
> sbertin@mindspring.com writes:
>  > I hadn't looked at what v4l does.  The video_unregister_device call
>  > in cpia_unregister_camera definately should not happen if the
>  > camera is open.  Just put an if(cam->open_count == 0) in front of
>  > it.
> 
> That's still not enough - the driver must not unregister an open
> video_device, even if the module should be unloaded.

That should be enough.  It will delay the call to
video_unregister_device until the device is closed.  Ioctls and mmap
will fail (oops, there's a bug.  They should return -ENODEV if cam->ops
is NULL). /proc/cpia/videox will go away. cpia_read will return
-ENODEV.  No need to go through the gymnastics you describe below.

> I'm not familiar
> with the module system, but isn't it possible to reject an
> rmmod-request? I'd propose the following: whenever an open device
> needs to be unregistered (e.g. because of a physical disconnect), the
> vectors of the corresponding video_device are overwritten: a special
> close() cares about the cleanup, while all other calls simply return
> -ENODEV. In addition to this the cam_data-struct will be added to a
> cleanup-list - the module won't unload before this list is empty,
> i.e. all devices are closed.

rmmod will fail if the usage count is > 0.  This means that it is not
possible to remove the module if a camera is open.

I've attached a patch that should fix these issues with removing an open
camera.  If you're happy with it, I'll put together a patch combining
this and the memory management changes needed for 2.3 kernels and we can
call it 0.5.1.

Scott J. Bertin
sbertin@mindspring.com


--1037094400-1804289383-948322032=:816
Content-Type: TEXT/plain; CHARSET=US-ASCII
Content-Description: unregister.patch

diff -r -c --exclude=linux cpia-0.5.0/module/cpia.c cpia-0.5.1/module/cpia.c
*** cpia-0.5.0/module/cpia.c	Wed Jan 12 16:04:26 2000
--- cpia-0.5.1/module/cpia.c	Wed Jan 19 17:39:11 2000
***************
*** 2738,2743 ****
--- 2738,2747 ----
  	struct cam_data *cam = dev->priv;
  	int retval = 0;
  
+ 	if(!cam || !cam->ops) {
+ 		return -ENODEV;
+ 	}
+ 	
  	/* make this _really_ smp-safe */
  	if( down_interruptible(&cam->busy_lock) ) {
  		return -EINTR;
***************
*** 3084,3100 ****
  {
  	unsigned long start = (unsigned long)adr;
  	unsigned long page, pos;
! 	struct cam_data *cam;
  	int retval;
  
  	DBG("cpia_mmap: %ld\n", size);
  
  	if(size > FRAME_NUM*CPIA_MAX_FRAME_SIZE){
  		return -EINVAL;
  	}
  
- 	cam = dev->priv;
- 
  	/* make this _really_ smp-safe */
  	if( down_interruptible(&cam->busy_lock) ) {
  		return -EINTR;
--- 3088,3106 ----
  {
  	unsigned long start = (unsigned long)adr;
  	unsigned long page, pos;
! 	struct cam_data *cam = dev->priv;
  	int retval;
  
+ 	if(!cam || !cam->ops) {
+ 		return -ENODEV;
+ 	}
+ 	
  	DBG("cpia_mmap: %ld\n", size);
  
  	if(size > FRAME_NUM*CPIA_MAX_FRAME_SIZE){
  		return -EINVAL;
  	}
  
  	/* make this _really_ smp-safe */
  	if( down_interruptible(&cam->busy_lock) ) {
  		return -EINTR;
***************
*** 3347,3355 ****
  	if(camnr >= CPIA_MAXCAMS || camera[camnr] == NULL) return;
  	cam = camera[camnr];
  	
! 	DBG("unregistering video\n");
! 	/* FIXME: Is this safe if the device is open? */
! 	video_unregister_device(&cam->vdev);
  	
  	/* Need a lock when adding/removing cameras.  This doesn't happen
  	 * often and doesn't take very long, so grabbing the kernel lock
--- 3353,3365 ----
  	if(camnr >= CPIA_MAXCAMS || camera[camnr] == NULL) return;
  	cam = camera[camnr];
  	
! 	if(cam->open_count == 0) {
! 		DBG("unregistering video\n");
! 		video_unregister_device(&cam->vdev);
! 	} else {
! 		LOG("/dev/video%d removed while open, "
! 		    "deferring video_unregister_device\n", cam->vdev.minor);
! 	}
  	
  	/* Need a lock when adding/removing cameras.  This doesn't happen
  	 * often and doesn't take very long, so grabbing the kernel lock

--1037094400-1804289383-948322032=:816--