From d5d3a7cc127d096c22ad63c4ff72970e1beb22ef Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 11 Mar 2013 04:48:30 -0300 Subject: [PATCH] [media] go7007: fix unregister/disconnect handling - use the v4l2_device's release() callback - remove the unnecessary ref_count - don't free usb data structures on disconnect, only do that in the final release callback. This is the correct way in order to safely handle disconnect and removal of modules. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/go7007/go7007-driver.c | 63 ++++++++++-------- drivers/staging/media/go7007/go7007-priv.h | 4 +- drivers/staging/media/go7007/go7007-usb.c | 64 +++++++++++-------- drivers/staging/media/go7007/go7007-v4l2.c | 22 +------ drivers/staging/media/go7007/saa7134-go7007.c | 7 +- drivers/staging/media/go7007/snd-go7007.c | 5 +- 6 files changed, 85 insertions(+), 80 deletions(-) diff --git a/drivers/staging/media/go7007/go7007-driver.c b/drivers/staging/media/go7007/go7007-driver.c index db9a4b3a7113..dca85d8edd9d 100644 --- a/drivers/staging/media/go7007/go7007-driver.c +++ b/drivers/staging/media/go7007/go7007-driver.c @@ -220,6 +220,31 @@ static int init_i2c_module(struct i2c_adapter *adapter, const struct go_i2c *con return -EINVAL; } +/* + * Detach and unregister the encoder. The go7007 struct won't be freed + * until v4l2 finishes releasing its resources and all associated fds are + * closed by applications. + */ +static void go7007_remove(struct v4l2_device *v4l2_dev) +{ + struct go7007 *go = container_of(v4l2_dev, struct go7007, v4l2_dev); + + v4l2_device_unregister(v4l2_dev); + if (go->hpi_ops->release) + go->hpi_ops->release(go); + if (go->i2c_adapter_online) { + if (i2c_del_adapter(&go->i2c_adapter) == 0) + go->i2c_adapter_online = 0; + else + v4l2_err(&go->v4l2_dev, + "error removing I2C adapter!\n"); + } + + kfree(go->boot_fw); + go7007_v4l2_remove(go); + kfree(go); +} + /* * Finalize the GO7007 hardware setup, register the on-board I2C adapter * (if used on this board), load the I2C client driver for the sensor @@ -234,17 +259,17 @@ int go7007_register_encoder(struct go7007 *go, unsigned num_i2c_devs) dev_info(go->dev, "go7007: registering new %s\n", go->name); + go->v4l2_dev.release = go7007_remove; + ret = v4l2_device_register(go->dev, &go->v4l2_dev); + if (ret < 0) + return ret; + mutex_lock(&go->hw_lock); ret = go7007_init_encoder(go); mutex_unlock(&go->hw_lock); if (ret < 0) return -1; - /* v4l2 init must happen before i2c subdevs */ - ret = go7007_v4l2_init(go); - if (ret < 0) - return ret; - if (!go->i2c_adapter_online && go->board_info->flags & GO7007_BOARD_USE_ONBOARD_I2C) { if (go7007_i2c_init(go) < 0) @@ -269,6 +294,11 @@ int go7007_register_encoder(struct go7007 *go, unsigned num_i2c_devs) v4l2_subdev_call(go->sd_video, video, s_routing, 0, 0, go->channel_number + 1); } + + ret = go7007_v4l2_init(go); + if (ret < 0) + return ret; + if (go->board_info->flags & GO7007_BOARD_HAS_AUDIO) { go->audio_enabled = 1; go7007_snd_init(go); @@ -608,7 +638,6 @@ struct go7007 *go7007_alloc(struct go7007_board_info *board, struct device *dev) init_waitqueue_head(&go->frame_waitq); spin_lock_init(&go->spinlock); go->video_dev = NULL; - go->ref_count = 0; go->status = STATUS_INIT; memset(&go->i2c_adapter, 0, sizeof(go->i2c_adapter)); go->i2c_adapter_online = 0; @@ -658,26 +687,4 @@ struct go7007 *go7007_alloc(struct go7007_board_info *board, struct device *dev) } EXPORT_SYMBOL(go7007_alloc); -/* - * Detach and unregister the encoder. The go7007 struct won't be freed - * until v4l2 finishes releasing its resources and all associated fds are - * closed by applications. - */ -void go7007_remove(struct go7007 *go) -{ - if (go->i2c_adapter_online) { - if (i2c_del_adapter(&go->i2c_adapter) == 0) - go->i2c_adapter_online = 0; - else - v4l2_err(&go->v4l2_dev, - "error removing I2C adapter!\n"); - } - - if (go->audio_enabled) - go7007_snd_remove(go); - kfree(go->boot_fw); - go7007_v4l2_remove(go); -} -EXPORT_SYMBOL(go7007_remove); - MODULE_LICENSE("GPL v2"); diff --git a/drivers/staging/media/go7007/go7007-priv.h b/drivers/staging/media/go7007/go7007-priv.h index d390120e4db4..54cfc0aa9826 100644 --- a/drivers/staging/media/go7007/go7007-priv.h +++ b/drivers/staging/media/go7007/go7007-priv.h @@ -117,6 +117,7 @@ struct go7007_hpi_ops { int (*stream_stop)(struct go7007 *go); int (*send_firmware)(struct go7007 *go, u8 *data, int len); int (*send_command)(struct go7007 *go, unsigned int cmd, void *arg); + void (*release)(struct go7007 *go); }; /* The video buffer size must be a multiple of PAGE_SIZE */ @@ -181,7 +182,6 @@ struct go7007 { void *boot_fw; unsigned boot_fw_len; struct v4l2_device v4l2_dev; - int ref_count; enum { STATUS_INIT, STATUS_ONLINE, STATUS_SHUTDOWN } status; spinlock_t spinlock; struct mutex hw_lock; @@ -287,8 +287,6 @@ int go7007_start_encoder(struct go7007 *go); void go7007_parse_video_stream(struct go7007 *go, u8 *buf, int length); struct go7007 *go7007_alloc(struct go7007_board_info *board, struct device *dev); -void go7007_remove(struct go7007 *go); - /* go7007-fw.c */ int go7007_construct_fw_image(struct go7007 *go, u8 **fw, int *fwlen); diff --git a/drivers/staging/media/go7007/go7007-usb.c b/drivers/staging/media/go7007/go7007-usb.c index f49672064c06..2d349f49fb45 100644 --- a/drivers/staging/media/go7007/go7007-usb.c +++ b/drivers/staging/media/go7007/go7007-usb.c @@ -617,6 +617,8 @@ static int go7007_usb_interface_reset(struct go7007 *go) struct go7007_usb *usb = go->hpi_context; u16 intr_val, intr_data; + if (go->status == STATUS_SHUTDOWN) + return -1; /* Reset encoder */ if (go7007_write_interrupt(go, 0x0001, 0x0001) < 0) return -1; @@ -886,6 +888,35 @@ static int go7007_usb_send_firmware(struct go7007 *go, u8 *data, int len) &transferred, timeout); } +static void go7007_usb_release(struct go7007 *go) +{ + struct go7007_usb *usb = go->hpi_context; + struct urb *vurb, *aurb; + int i; + + usb_kill_urb(usb->intr_urb); + + /* Free USB-related structs */ + for (i = 0; i < 8; ++i) { + vurb = usb->video_urbs[i]; + if (vurb) { + usb_kill_urb(vurb); + kfree(vurb->transfer_buffer); + usb_free_urb(vurb); + } + aurb = usb->audio_urbs[i]; + if (aurb) { + usb_kill_urb(aurb); + kfree(aurb->transfer_buffer); + usb_free_urb(aurb); + } + } + kfree(usb->intr_urb->transfer_buffer); + usb_free_urb(usb->intr_urb); + + kfree(go->hpi_context); +} + static struct go7007_hpi_ops go7007_usb_ezusb_hpi_ops = { .interface_reset = go7007_usb_interface_reset, .write_interrupt = go7007_usb_ezusb_write_interrupt, @@ -893,6 +924,7 @@ static struct go7007_hpi_ops go7007_usb_ezusb_hpi_ops = { .stream_start = go7007_usb_stream_start, .stream_stop = go7007_usb_stream_stop, .send_firmware = go7007_usb_send_firmware, + .release = go7007_usb_release, }; static struct go7007_hpi_ops go7007_usb_onboard_hpi_ops = { @@ -902,6 +934,7 @@ static struct go7007_hpi_ops go7007_usb_onboard_hpi_ops = { .stream_start = go7007_usb_stream_start, .stream_stop = go7007_usb_stream_stop, .send_firmware = go7007_usb_send_firmware, + .release = go7007_usb_release, }; /********************* Driver for EZ-USB I2C adapter *********************/ @@ -1279,34 +1312,15 @@ allocfail: static void go7007_usb_disconnect(struct usb_interface *intf) { struct go7007 *go = to_go7007(usb_get_intfdata(intf)); - struct go7007_usb *usb = go->hpi_context; - struct urb *vurb, *aurb; - int i; - - usb_kill_urb(usb->intr_urb); - - /* Free USB-related structs */ - for (i = 0; i < 8; ++i) { - vurb = usb->video_urbs[i]; - if (vurb) { - usb_kill_urb(vurb); - kfree(vurb->transfer_buffer); - usb_free_urb(vurb); - } - aurb = usb->audio_urbs[i]; - if (aurb) { - usb_kill_urb(aurb); - kfree(aurb->transfer_buffer); - usb_free_urb(aurb); - } - } - kfree(usb->intr_urb->transfer_buffer); - usb_free_urb(usb->intr_urb); - kfree(go->hpi_context); + if (go->audio_enabled) + go7007_snd_remove(go); go->status = STATUS_SHUTDOWN; - go7007_remove(go); + v4l2_device_disconnect(&go->v4l2_dev); + video_unregister_device(go->video_dev); + + v4l2_device_put(&go->v4l2_dev); } static struct usb_driver go7007_usb_driver = { diff --git a/drivers/staging/media/go7007/go7007-v4l2.c b/drivers/staging/media/go7007/go7007-v4l2.c index 6cb6f43a1aa5..216c94a45f6f 100644 --- a/drivers/staging/media/go7007/go7007-v4l2.c +++ b/drivers/staging/media/go7007/go7007-v4l2.c @@ -100,7 +100,6 @@ static int go7007_open(struct file *file) gofh = kzalloc(sizeof(struct go7007_file), GFP_KERNEL); if (gofh == NULL) return -ENOMEM; - ++go->ref_count; gofh->go = go; mutex_init(&gofh->lock); gofh->buf_count = 0; @@ -120,8 +119,6 @@ static int go7007_release(struct file *file) gofh->buf_count = 0; } kfree(gofh); - if (--go->ref_count == 0) - kfree(go); file->private_data = NULL; return 0; } @@ -1772,11 +1769,7 @@ static unsigned int go7007_poll(struct file *file, poll_table *wait) static void go7007_vfl_release(struct video_device *vfd) { - struct go7007 *go = video_get_drvdata(vfd); - video_device_release(vfd); - if (--go->ref_count == 0) - kfree(go); } static struct v4l2_file_operations go7007_fops = { @@ -1844,7 +1837,8 @@ int go7007_v4l2_init(struct go7007 *go) if (go->video_dev == NULL) return -ENOMEM; *go->video_dev = go7007_template; - go->video_dev->parent = go->dev; + video_set_drvdata(go->video_dev, go); + go->video_dev->v4l2_dev = &go->v4l2_dev; rv = video_register_device(go->video_dev, VFL_TYPE_GRABBER, -1); if (rv < 0) { video_device_release(go->video_dev); @@ -1856,14 +1850,6 @@ int go7007_v4l2_init(struct go7007 *go) v4l2_disable_ioctl(go->video_dev, VIDIOC_S_AUDIO); v4l2_disable_ioctl(go->video_dev, VIDIOC_ENUMAUDIO); } - rv = v4l2_device_register(go->dev, &go->v4l2_dev); - if (rv < 0) { - video_device_release(go->video_dev); - go->video_dev = NULL; - return rv; - } - video_set_drvdata(go->video_dev, go); - ++go->ref_count; dev_info(go->dev, "registered device %s [v4l2]\n", video_device_node_name(go->video_dev)); @@ -1883,8 +1869,4 @@ void go7007_v4l2_remove(struct go7007 *go) spin_unlock_irqrestore(&go->spinlock, flags); } mutex_unlock(&go->hw_lock); - if (go->video_dev) - video_unregister_device(go->video_dev); - if (go->status != STATUS_SHUTDOWN) - v4l2_device_unregister(&go->v4l2_dev); } diff --git a/drivers/staging/media/go7007/saa7134-go7007.c b/drivers/staging/media/go7007/saa7134-go7007.c index afe21f3f0959..4c73945172c8 100644 --- a/drivers/staging/media/go7007/saa7134-go7007.c +++ b/drivers/staging/media/go7007/saa7134-go7007.c @@ -499,12 +499,17 @@ static int saa7134_go7007_fini(struct saa7134_dev *dev) return 0; go = video_get_drvdata(dev->empress_dev); + if (go->audio_enabled) + go7007_snd_remove(go); + saa = go->hpi_context; go->status = STATUS_SHUTDOWN; free_page((unsigned long)saa->top); free_page((unsigned long)saa->bottom); kfree(saa); - go7007_remove(go); + video_unregister_device(&go->vdev); + + v4l2_device_put(&go->v4l2_dev); dev->empress_dev = NULL; return 0; diff --git a/drivers/staging/media/go7007/snd-go7007.c b/drivers/staging/media/go7007/snd-go7007.c index 6f6271ec558c..4be0fa40a39a 100644 --- a/drivers/staging/media/go7007/snd-go7007.c +++ b/drivers/staging/media/go7007/snd-go7007.c @@ -221,8 +221,6 @@ static int go7007_snd_free(struct snd_device *device) kfree(go->snd_context); go->snd_context = NULL; - if (--go->ref_count == 0) - kfree(go); return 0; } @@ -285,8 +283,8 @@ int go7007_snd_init(struct go7007 *go) gosnd->substream = NULL; go->snd_context = gosnd; + v4l2_device_get(&go->v4l2_dev); ++dev; - ++go->ref_count; return 0; } @@ -298,6 +296,7 @@ int go7007_snd_remove(struct go7007 *go) snd_card_disconnect(gosnd->card); snd_card_free_when_closed(gosnd->card); + v4l2_device_put(&go->v4l2_dev); return 0; } EXPORT_SYMBOL(go7007_snd_remove); -- 2.39.5