Commit 03dea86d authored by Tobias Lorenz's avatar Tobias Lorenz Committed by Mauro Carvalho Chehab

V4L/DVB (7401): radio-si470x: unplugging fixed

This patch fixes several kernel oops, when unplugging device while it is in
use:

Basically the patch delays freeing of the internal variables in
si470x_usb_driver_disconnect, until the the last user closed the device in
si470x_fops_release. This was implemented a while ago with the help of Oliver
Neukum.

I tested the patch five times (unplugging while in use) without oops coming
from the radio-si470x driver anymore. A remaining oops was coming from the
usbaudio driver, but this is someone else task. Hopefully this fixed all
unplugging issues.
Signed-off-by: default avatarTobias Lorenz <tobias.lorenz@gmx.net>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@infradead.org>
parent b412ba78
...@@ -85,6 +85,7 @@ ...@@ -85,6 +85,7 @@
* Oliver Neukum <oliver@neukum.org> * Oliver Neukum <oliver@neukum.org>
* Version 1.0.7 * Version 1.0.7
* - usb autosuspend support * - usb autosuspend support
* - unplugging fixed
* *
* ToDo: * ToDo:
* - add seeking support * - add seeking support
...@@ -97,10 +98,10 @@ ...@@ -97,10 +98,10 @@
/* driver definitions */ /* driver definitions */
#define DRIVER_AUTHOR "Tobias Lorenz <tobias.lorenz@gmx.net>" #define DRIVER_AUTHOR "Tobias Lorenz <tobias.lorenz@gmx.net>"
#define DRIVER_NAME "radio-si470x" #define DRIVER_NAME "radio-si470x"
#define DRIVER_KERNEL_VERSION KERNEL_VERSION(1, 0, 6) #define DRIVER_KERNEL_VERSION KERNEL_VERSION(1, 0, 7)
#define DRIVER_CARD "Silicon Labs Si470x FM Radio Receiver" #define DRIVER_CARD "Silicon Labs Si470x FM Radio Receiver"
#define DRIVER_DESC "USB radio driver for Si470x FM Radio Receivers" #define DRIVER_DESC "USB radio driver for Si470x FM Radio Receivers"
#define DRIVER_VERSION "1.0.6" #define DRIVER_VERSION "1.0.7"
/* kernel includes */ /* kernel includes */
...@@ -424,6 +425,7 @@ struct si470x_device { ...@@ -424,6 +425,7 @@ struct si470x_device {
/* driver management */ /* driver management */
unsigned int users; unsigned int users;
unsigned char disconnected;
/* Silabs internal registers (0..15) */ /* Silabs internal registers (0..15) */
unsigned short registers[RADIO_REGISTER_NUM]; unsigned short registers[RADIO_REGISTER_NUM];
...@@ -439,6 +441,12 @@ struct si470x_device { ...@@ -439,6 +441,12 @@ struct si470x_device {
}; };
/*
* Lock to prevent kfree of data before all users have releases the device.
*/
static DEFINE_MUTEX(open_close_lock);
/* /*
* The frequency is set in units of 62.5 Hz when using V4L2_TUNER_CAP_LOW, * The frequency is set in units of 62.5 Hz when using V4L2_TUNER_CAP_LOW,
* 62.5 kHz otherwise. * 62.5 kHz otherwise.
...@@ -577,7 +585,7 @@ static int si470x_get_rds_registers(struct si470x_device *radio) ...@@ -577,7 +585,7 @@ static int si470x_get_rds_registers(struct si470x_device *radio)
usb_rcvintpipe(radio->usbdev, 1), usb_rcvintpipe(radio->usbdev, 1),
(void *) &buf, sizeof(buf), &size, usb_timeout); (void *) &buf, sizeof(buf), &size, usb_timeout);
if (size != sizeof(buf)) if (size != sizeof(buf))
printk(KERN_WARNING DRIVER_NAME ": si470x_get_rds_register: " printk(KERN_WARNING DRIVER_NAME ": si470x_get_rds_registers: "
"return size differs: %d != %zu\n", size, sizeof(buf)); "return size differs: %d != %zu\n", size, sizeof(buf));
if (retval < 0) if (retval < 0)
printk(KERN_WARNING DRIVER_NAME ": si470x_get_rds_registers: " printk(KERN_WARNING DRIVER_NAME ": si470x_get_rds_registers: "
...@@ -875,6 +883,8 @@ static void si470x_work(struct work_struct *work) ...@@ -875,6 +883,8 @@ static void si470x_work(struct work_struct *work)
struct si470x_device *radio = container_of(work, struct si470x_device, struct si470x_device *radio = container_of(work, struct si470x_device,
work.work); work.work);
if (radio->disconnected)
return;
if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0) if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0)
return; return;
...@@ -1001,13 +1011,21 @@ static int si470x_fops_open(struct inode *inode, struct file *file) ...@@ -1001,13 +1011,21 @@ static int si470x_fops_open(struct inode *inode, struct file *file)
static int si470x_fops_release(struct inode *inode, struct file *file) static int si470x_fops_release(struct inode *inode, struct file *file)
{ {
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval; int retval = 0;
if (!radio) if (!radio)
return -ENODEV; return -ENODEV;
mutex_lock(&open_close_lock);
radio->users--; radio->users--;
if (radio->users == 0) { if (radio->users == 0) {
if (radio->disconnected) {
video_unregister_device(radio->videodev);
kfree(radio->buffer);
kfree(radio);
goto done;
}
/* stop rds reception */ /* stop rds reception */
cancel_delayed_work_sync(&radio->work); cancel_delayed_work_sync(&radio->work);
...@@ -1016,10 +1034,11 @@ static int si470x_fops_release(struct inode *inode, struct file *file) ...@@ -1016,10 +1034,11 @@ static int si470x_fops_release(struct inode *inode, struct file *file)
retval = si470x_stop(radio); retval = si470x_stop(radio);
usb_autopm_put_interface(radio->intf); usb_autopm_put_interface(radio->intf);
return retval;
} }
return 0; done:
mutex_unlock(&open_close_lock);
return retval;
} }
...@@ -1157,6 +1176,9 @@ static int si470x_vidioc_g_ctrl(struct file *file, void *priv, ...@@ -1157,6 +1176,9 @@ static int si470x_vidioc_g_ctrl(struct file *file, void *priv,
{ {
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
if (radio->disconnected)
return -EIO;
switch (ctrl->id) { switch (ctrl->id) {
case V4L2_CID_AUDIO_VOLUME: case V4L2_CID_AUDIO_VOLUME:
ctrl->value = radio->registers[SYSCONFIG2] & ctrl->value = radio->registers[SYSCONFIG2] &
...@@ -1181,6 +1203,9 @@ static int si470x_vidioc_s_ctrl(struct file *file, void *priv, ...@@ -1181,6 +1203,9 @@ static int si470x_vidioc_s_ctrl(struct file *file, void *priv,
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval; int retval;
if (radio->disconnected)
return -EIO;
switch (ctrl->id) { switch (ctrl->id) {
case V4L2_CID_AUDIO_VOLUME: case V4L2_CID_AUDIO_VOLUME:
radio->registers[SYSCONFIG2] &= ~SYSCONFIG2_VOLUME; radio->registers[SYSCONFIG2] &= ~SYSCONFIG2_VOLUME;
...@@ -1243,6 +1268,8 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv, ...@@ -1243,6 +1268,8 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv,
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval; int retval;
if (radio->disconnected)
return -EIO;
if (tuner->index > 0) if (tuner->index > 0)
return -EINVAL; return -EINVAL;
...@@ -1299,6 +1326,8 @@ static int si470x_vidioc_s_tuner(struct file *file, void *priv, ...@@ -1299,6 +1326,8 @@ static int si470x_vidioc_s_tuner(struct file *file, void *priv,
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval; int retval;
if (radio->disconnected)
return -EIO;
if (tuner->index > 0) if (tuner->index > 0)
return -EINVAL; return -EINVAL;
...@@ -1324,6 +1353,9 @@ static int si470x_vidioc_g_frequency(struct file *file, void *priv, ...@@ -1324,6 +1353,9 @@ static int si470x_vidioc_g_frequency(struct file *file, void *priv,
{ {
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
if (radio->disconnected)
return -EIO;
freq->type = V4L2_TUNER_RADIO; freq->type = V4L2_TUNER_RADIO;
freq->frequency = si470x_get_freq(radio); freq->frequency = si470x_get_freq(radio);
...@@ -1340,6 +1372,8 @@ static int si470x_vidioc_s_frequency(struct file *file, void *priv, ...@@ -1340,6 +1372,8 @@ static int si470x_vidioc_s_frequency(struct file *file, void *priv,
struct si470x_device *radio = video_get_drvdata(video_devdata(file)); struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval; int retval;
if (radio->disconnected)
return -EIO;
if (freq->type != V4L2_TUNER_RADIO) if (freq->type != V4L2_TUNER_RADIO)
return -EINVAL; return -EINVAL;
...@@ -1510,11 +1544,16 @@ static void si470x_usb_driver_disconnect(struct usb_interface *intf) ...@@ -1510,11 +1544,16 @@ static void si470x_usb_driver_disconnect(struct usb_interface *intf)
{ {
struct si470x_device *radio = usb_get_intfdata(intf); struct si470x_device *radio = usb_get_intfdata(intf);
mutex_lock(&open_close_lock);
radio->disconnected = 1;
cancel_delayed_work_sync(&radio->work); cancel_delayed_work_sync(&radio->work);
usb_set_intfdata(intf, NULL); usb_set_intfdata(intf, NULL);
if (radio->users == 0) {
video_unregister_device(radio->videodev); video_unregister_device(radio->videodev);
kfree(radio->buffer); kfree(radio->buffer);
kfree(radio); kfree(radio);
}
mutex_unlock(&open_close_lock);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment