提交 9a7b2d1f 编写于 作者: H Hans de Goede 提交者: Mauro Carvalho Chehab

[media] pwc: better usb disconnect handling

Unplugging a pwc cam while an app has the /dev/video# node open leads
to an oops in pwc_video_close when the app closes the node, because
the disconnect handler has free-ed the pdev struct pwc_video_close
tries to use. Instead of adding some sort of bandaid for this.
fix it properly using the v4l2 core's new(ish) behavior of keeping the
v4l2_dev structure around until both unregister has been called, and
all file handles referring  to it have been closed:

Embed the v4l2_dev structure in the pdev structure and define a v4l2 dev
release callback releasing the pdev structure (and thus also the embedded
v4l2 dev structure.
Signed-off-by: NHans de Goede <hdegoede@redhat.com>
Cc: stable@kernel.org
Signed-off-by: NMauro Carvalho Chehab <mchehab@redhat.com>
上级 e76e4706
...@@ -1414,7 +1414,7 @@ long pwc_ioctl(struct pwc_device *pdev, unsigned int cmd, void *arg) ...@@ -1414,7 +1414,7 @@ long pwc_ioctl(struct pwc_device *pdev, unsigned int cmd, void *arg)
{ {
ARG_DEF(struct pwc_probe, probe) ARG_DEF(struct pwc_probe, probe)
strcpy(ARGR(probe).name, pdev->vdev->name); strcpy(ARGR(probe).name, pdev->vdev.name);
ARGR(probe).type = pdev->type; ARGR(probe).type = pdev->type;
ARG_OUT(probe) ARG_OUT(probe)
break; break;
......
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
Oh yes, convention: to disctinguish between all the various pointers to Oh yes, convention: to disctinguish between all the various pointers to
device-structures, I use these names for the pointer variables: device-structures, I use these names for the pointer variables:
udev: struct usb_device * udev: struct usb_device *
vdev: struct video_device * vdev: struct video_device (member of pwc_dev)
pdev: struct pwc_devive * pdev: struct pwc_devive *
*/ */
...@@ -152,6 +152,7 @@ static ssize_t pwc_video_read(struct file *file, char __user *buf, ...@@ -152,6 +152,7 @@ static ssize_t pwc_video_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos); size_t count, loff_t *ppos);
static unsigned int pwc_video_poll(struct file *file, poll_table *wait); static unsigned int pwc_video_poll(struct file *file, poll_table *wait);
static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma); static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma);
static void pwc_video_release(struct video_device *vfd);
static const struct v4l2_file_operations pwc_fops = { static const struct v4l2_file_operations pwc_fops = {
.owner = THIS_MODULE, .owner = THIS_MODULE,
...@@ -164,41 +165,11 @@ static const struct v4l2_file_operations pwc_fops = { ...@@ -164,41 +165,11 @@ static const struct v4l2_file_operations pwc_fops = {
}; };
static struct video_device pwc_template = { static struct video_device pwc_template = {
.name = "Philips Webcam", /* Filled in later */ .name = "Philips Webcam", /* Filled in later */
.release = video_device_release, .release = pwc_video_release,
.fops = &pwc_fops, .fops = &pwc_fops,
.ioctl_ops = &pwc_ioctl_ops,
}; };
/***************************************************************************/
/* Okay, this is some magic that I worked out and the reasoning behind it...
The biggest problem with any USB device is of course: "what to do
when the user unplugs the device while it is in use by an application?"
We have several options:
1) Curse them with the 7 plagues when they do (requires divine intervention)
2) Tell them not to (won't work: they'll do it anyway)
3) Oops the kernel (this will have a negative effect on a user's uptime)
4) Do something sensible.
Of course, we go for option 4.
It happens that this device will be linked to two times, once from
usb_device and once from the video_device in their respective 'private'
pointers. This is done when the device is probed() and all initialization
succeeded. The pwc_device struct links back to both structures.
When a device is unplugged while in use it will be removed from the
list of known USB devices; I also de-register it as a V4L device, but
unfortunately I can't free the memory since the struct is still in use
by the file descriptor. This free-ing is then deferend until the first
opportunity. Crude, but it works.
A small 'advantage' is that if a user unplugs the cam and plugs it back
in, it should get assigned the same video device minor, but unfortunately
it's non-trivial to re-link the cam back to the video device... (that
would surely be magic! :))
*/
/***************************************************************************/ /***************************************************************************/
/* Private functions */ /* Private functions */
...@@ -1016,16 +987,15 @@ static ssize_t show_snapshot_button_status(struct device *class_dev, ...@@ -1016,16 +987,15 @@ static ssize_t show_snapshot_button_status(struct device *class_dev,
static DEVICE_ATTR(button, S_IRUGO | S_IWUSR, show_snapshot_button_status, static DEVICE_ATTR(button, S_IRUGO | S_IWUSR, show_snapshot_button_status,
NULL); NULL);
static int pwc_create_sysfs_files(struct video_device *vdev) static int pwc_create_sysfs_files(struct pwc_device *pdev)
{ {
struct pwc_device *pdev = video_get_drvdata(vdev);
int rc; int rc;
rc = device_create_file(&vdev->dev, &dev_attr_button); rc = device_create_file(&pdev->vdev.dev, &dev_attr_button);
if (rc) if (rc)
goto err; goto err;
if (pdev->features & FEATURE_MOTOR_PANTILT) { if (pdev->features & FEATURE_MOTOR_PANTILT) {
rc = device_create_file(&vdev->dev, &dev_attr_pan_tilt); rc = device_create_file(&pdev->vdev.dev, &dev_attr_pan_tilt);
if (rc) if (rc)
goto err_button; goto err_button;
} }
...@@ -1033,19 +1003,17 @@ static int pwc_create_sysfs_files(struct video_device *vdev) ...@@ -1033,19 +1003,17 @@ static int pwc_create_sysfs_files(struct video_device *vdev)
return 0; return 0;
err_button: err_button:
device_remove_file(&vdev->dev, &dev_attr_button); device_remove_file(&pdev->vdev.dev, &dev_attr_button);
err: err:
PWC_ERROR("Could not create sysfs files.\n"); PWC_ERROR("Could not create sysfs files.\n");
return rc; return rc;
} }
static void pwc_remove_sysfs_files(struct video_device *vdev) static void pwc_remove_sysfs_files(struct pwc_device *pdev)
{ {
struct pwc_device *pdev = video_get_drvdata(vdev);
if (pdev->features & FEATURE_MOTOR_PANTILT) if (pdev->features & FEATURE_MOTOR_PANTILT)
device_remove_file(&vdev->dev, &dev_attr_pan_tilt); device_remove_file(&pdev->vdev.dev, &dev_attr_pan_tilt);
device_remove_file(&vdev->dev, &dev_attr_button); device_remove_file(&pdev->vdev.dev, &dev_attr_button);
} }
#ifdef CONFIG_USB_PWC_DEBUG #ifdef CONFIG_USB_PWC_DEBUG
...@@ -1106,7 +1074,7 @@ static int pwc_video_open(struct file *file) ...@@ -1106,7 +1074,7 @@ static int pwc_video_open(struct file *file)
if (ret >= 0) if (ret >= 0)
{ {
PWC_DEBUG_OPEN("This %s camera is equipped with a %s (%d).\n", PWC_DEBUG_OPEN("This %s camera is equipped with a %s (%d).\n",
pdev->vdev->name, pdev->vdev.name,
pwc_sensor_type_to_string(i), i); pwc_sensor_type_to_string(i), i);
} }
} }
...@@ -1180,16 +1148,15 @@ static int pwc_video_open(struct file *file) ...@@ -1180,16 +1148,15 @@ static int pwc_video_open(struct file *file)
return 0; return 0;
} }
static void pwc_video_release(struct video_device *vfd)
static void pwc_cleanup(struct pwc_device *pdev)
{ {
pwc_remove_sysfs_files(pdev->vdev); struct pwc_device *pdev = container_of(vfd, struct pwc_device, vdev);
video_unregister_device(pdev->vdev); int hint;
#ifdef CONFIG_USB_PWC_INPUT_EVDEV /* search device_hint[] table if we occupy a slot, by any chance */
if (pdev->button_dev) for (hint = 0; hint < MAX_DEV_HINTS; hint++)
input_unregister_device(pdev->button_dev); if (device_hint[hint].pdev == pdev)
#endif device_hint[hint].pdev = NULL;
kfree(pdev); kfree(pdev);
} }
...@@ -1199,7 +1166,7 @@ static int pwc_video_close(struct file *file) ...@@ -1199,7 +1166,7 @@ static int pwc_video_close(struct file *file)
{ {
struct video_device *vdev = file->private_data; struct video_device *vdev = file->private_data;
struct pwc_device *pdev; struct pwc_device *pdev;
int i, hint; int i;
PWC_DEBUG_OPEN(">> video_close called(vdev = 0x%p).\n", vdev); PWC_DEBUG_OPEN(">> video_close called(vdev = 0x%p).\n", vdev);
...@@ -1234,12 +1201,6 @@ static int pwc_video_close(struct file *file) ...@@ -1234,12 +1201,6 @@ static int pwc_video_close(struct file *file)
} }
pdev->vopen--; pdev->vopen--;
PWC_DEBUG_OPEN("<< video_close() vopen=%d\n", pdev->vopen); PWC_DEBUG_OPEN("<< video_close() vopen=%d\n", pdev->vopen);
} else {
pwc_cleanup(pdev);
/* search device_hint[] table if we occupy a slot, by any chance */
for (hint = 0; hint < MAX_DEV_HINTS; hint++)
if (device_hint[hint].pdev == pdev)
device_hint[hint].pdev = NULL;
} }
return 0; return 0;
...@@ -1715,19 +1676,12 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1715,19 +1676,12 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
init_waitqueue_head(&pdev->frameq); init_waitqueue_head(&pdev->frameq);
pdev->vcompression = pwc_preferred_compression; pdev->vcompression = pwc_preferred_compression;
/* Allocate video_device structure */ /* Init video_device structure */
pdev->vdev = video_device_alloc(); memcpy(&pdev->vdev, &pwc_template, sizeof(pwc_template));
if (!pdev->vdev) { pdev->vdev.parent = &intf->dev;
PWC_ERROR("Err, cannot allocate video_device struture. Failing probe."); pdev->vdev.lock = &pdev->modlock;
rc = -ENOMEM; strcpy(pdev->vdev.name, name);
goto err_free_mem; video_set_drvdata(&pdev->vdev, pdev);
}
memcpy(pdev->vdev, &pwc_template, sizeof(pwc_template));
pdev->vdev->parent = &intf->dev;
pdev->vdev->lock = &pdev->modlock;
pdev->vdev->ioctl_ops = &pwc_ioctl_ops;
strcpy(pdev->vdev->name, name);
video_set_drvdata(pdev->vdev, pdev);
pdev->release = le16_to_cpu(udev->descriptor.bcdDevice); pdev->release = le16_to_cpu(udev->descriptor.bcdDevice);
PWC_DEBUG_PROBE("Release: %04x\n", pdev->release); PWC_DEBUG_PROBE("Release: %04x\n", pdev->release);
...@@ -1746,8 +1700,6 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1746,8 +1700,6 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
} }
} }
pdev->vdev->release = video_device_release;
/* occupy slot */ /* occupy slot */
if (hint < MAX_DEV_HINTS) if (hint < MAX_DEV_HINTS)
device_hint[hint].pdev = pdev; device_hint[hint].pdev = pdev;
...@@ -1759,16 +1711,16 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1759,16 +1711,16 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
pwc_set_leds(pdev, 0, 0); pwc_set_leds(pdev, 0, 0);
pwc_camera_power(pdev, 0); pwc_camera_power(pdev, 0);
rc = video_register_device(pdev->vdev, VFL_TYPE_GRABBER, video_nr); rc = video_register_device(&pdev->vdev, VFL_TYPE_GRABBER, video_nr);
if (rc < 0) { if (rc < 0) {
PWC_ERROR("Failed to register as video device (%d).\n", rc); PWC_ERROR("Failed to register as video device (%d).\n", rc);
goto err_video_release; goto err_free_mem;
} }
rc = pwc_create_sysfs_files(pdev->vdev); rc = pwc_create_sysfs_files(pdev);
if (rc) if (rc)
goto err_video_unreg; goto err_video_unreg;
PWC_INFO("Registered as %s.\n", video_device_node_name(pdev->vdev)); PWC_INFO("Registered as %s.\n", video_device_node_name(&pdev->vdev));
#ifdef CONFIG_USB_PWC_INPUT_EVDEV #ifdef CONFIG_USB_PWC_INPUT_EVDEV
/* register webcam snapshot button input device */ /* register webcam snapshot button input device */
...@@ -1776,7 +1728,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1776,7 +1728,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
if (!pdev->button_dev) { if (!pdev->button_dev) {
PWC_ERROR("Err, insufficient memory for webcam snapshot button device."); PWC_ERROR("Err, insufficient memory for webcam snapshot button device.");
rc = -ENOMEM; rc = -ENOMEM;
pwc_remove_sysfs_files(pdev->vdev); pwc_remove_sysfs_files(pdev);
goto err_video_unreg; goto err_video_unreg;
} }
...@@ -1794,7 +1746,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1794,7 +1746,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
if (rc) { if (rc) {
input_free_device(pdev->button_dev); input_free_device(pdev->button_dev);
pdev->button_dev = NULL; pdev->button_dev = NULL;
pwc_remove_sysfs_files(pdev->vdev); pwc_remove_sysfs_files(pdev);
goto err_video_unreg; goto err_video_unreg;
} }
#endif #endif
...@@ -1804,10 +1756,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1804,10 +1756,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
err_video_unreg: err_video_unreg:
if (hint < MAX_DEV_HINTS) if (hint < MAX_DEV_HINTS)
device_hint[hint].pdev = NULL; device_hint[hint].pdev = NULL;
video_unregister_device(pdev->vdev); video_unregister_device(&pdev->vdev);
pdev->vdev = NULL; /* So we don't try to release it below */
err_video_release:
video_device_release(pdev->vdev);
err_free_mem: err_free_mem:
kfree(pdev); kfree(pdev);
return rc; return rc;
...@@ -1816,10 +1765,8 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1816,10 +1765,8 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
/* The user yanked out the cable... */ /* The user yanked out the cable... */
static void usb_pwc_disconnect(struct usb_interface *intf) static void usb_pwc_disconnect(struct usb_interface *intf)
{ {
struct pwc_device *pdev; struct pwc_device *pdev = usb_get_intfdata(intf);
int hint;
pdev = usb_get_intfdata (intf);
mutex_lock(&pdev->modlock); mutex_lock(&pdev->modlock);
usb_set_intfdata (intf, NULL); usb_set_intfdata (intf, NULL);
if (pdev == NULL) { if (pdev == NULL) {
...@@ -1836,30 +1783,25 @@ static void usb_pwc_disconnect(struct usb_interface *intf) ...@@ -1836,30 +1783,25 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
} }
/* We got unplugged; this is signalled by an EPIPE error code */ /* We got unplugged; this is signalled by an EPIPE error code */
if (pdev->vopen) {
PWC_INFO("Disconnected while webcam is in use!\n");
pdev->error_status = EPIPE; pdev->error_status = EPIPE;
} pdev->unplugged = 1;
/* Alert waiting processes */ /* Alert waiting processes */
wake_up_interruptible(&pdev->frameq); wake_up_interruptible(&pdev->frameq);
/* Wait until device is closed */
if (pdev->vopen) {
pdev->unplugged = 1;
pwc_iso_stop(pdev);
} else {
/* Device is closed, so we can safely unregister it */
PWC_DEBUG_PROBE("Unregistering video device in disconnect().\n");
disconnect_out: /* No need to keep the urbs around after disconnection */
/* search device_hint[] table if we occupy a slot, by any chance */ pwc_isoc_cleanup(pdev);
for (hint = 0; hint < MAX_DEV_HINTS; hint++)
if (device_hint[hint].pdev == pdev)
device_hint[hint].pdev = NULL;
}
disconnect_out:
mutex_unlock(&pdev->modlock); mutex_unlock(&pdev->modlock);
pwc_cleanup(pdev);
pwc_remove_sysfs_files(pdev);
video_unregister_device(&pdev->vdev);
#ifdef CONFIG_USB_PWC_INPUT_EVDEV
if (pdev->button_dev)
input_unregister_device(pdev->button_dev);
#endif
} }
......
...@@ -162,9 +162,9 @@ struct pwc_imgbuf ...@@ -162,9 +162,9 @@ struct pwc_imgbuf
struct pwc_device struct pwc_device
{ {
struct video_device *vdev; struct video_device vdev;
/* Pointer to our usb_device */ /* Pointer to our usb_device, may be NULL after unplug */
struct usb_device *udev; struct usb_device *udev;
int type; /* type of cam (645, 646, 675, 680, 690, 720, 730, 740, 750) */ int type; /* type of cam (645, 646, 675, 680, 690, 720, 730, 740, 750) */
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册