V4L/DVB (7995): si470x: a lot of small code cleanups
This patch brings the following changes:
- comment on how to listen to an usb audio device
(i get so many questions about that...)
- code cleanup (error handling, more warnings, spacing, ...)
Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
diff --git a/drivers/media/radio/radio-si470x.c b/drivers/media/radio/radio-si470x.c
index 7df1163..954ba99 100644
--- a/drivers/media/radio/radio-si470x.c
+++ b/drivers/media/radio/radio-si470x.c
@@ -101,7 +101,7 @@
* - unplugging fixed
* 2008-05-07 Tobias Lorenz <tobias.lorenz@gmx.net>
* Version 1.0.8
- * - let si470x_get_freq return errno
+ * - more safety checks, let si470x_get_freq return errno
*
* ToDo:
* - add seeking support
@@ -487,11 +487,11 @@
USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
report[0], 2,
buf, size, usb_timeout);
+
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
": si470x_get_report: usb_control_msg returned %d\n",
retval);
-
return retval;
}
@@ -510,11 +510,11 @@
USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
report[0], 2,
buf, size, usb_timeout);
+
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
": si470x_set_report: usb_control_msg returned %d\n",
retval);
-
return retval;
}
@@ -623,24 +623,30 @@
radio->registers[CHANNEL] |= CHANNEL_TUNE | chan;
retval = si470x_set_register(radio, CHANNEL);
if (retval < 0)
- return retval;
+ goto done;
- /* wait till seek operation has completed */
+ /* wait till tune operation has completed */
timeout = jiffies + msecs_to_jiffies(tune_timeout);
do {
retval = si470x_get_register(radio, STATUSRSSI);
if (retval < 0)
- return retval;
+ goto stop;
timed_out = time_after(jiffies, timeout);
} while (((radio->registers[STATUSRSSI] & STATUSRSSI_STC) == 0) &&
(!timed_out));
+ if ((radio->registers[STATUSRSSI] & STATUSRSSI_STC) == 0)
+ printk(KERN_WARNING DRIVER_NAME ": tune does not complete\n");
if (timed_out)
printk(KERN_WARNING DRIVER_NAME
- ": seek does not finish after %u ms\n", tune_timeout);
+ ": tune timed out after %u ms\n", tune_timeout);
+stop:
/* stop tuning */
radio->registers[CHANNEL] &= ~CHANNEL_TUNE;
- return si470x_set_register(radio, CHANNEL);
+ retval = si470x_set_register(radio, CHANNEL);
+
+done:
+ return retval;
}
@@ -731,27 +737,30 @@
POWERCFG_DMUTE | POWERCFG_ENABLE | POWERCFG_RDSM;
retval = si470x_set_register(radio, POWERCFG);
if (retval < 0)
- return retval;
+ goto done;
/* sysconfig 1 */
radio->registers[SYSCONFIG1] = SYSCONFIG1_DE;
retval = si470x_set_register(radio, SYSCONFIG1);
if (retval < 0)
- return retval;
+ goto done;
/* sysconfig 2 */
radio->registers[SYSCONFIG2] =
- (0x3f << 8) | /* SEEKTH */
- (band << 6) | /* BAND */
- (space << 4) | /* SPACE */
- 15; /* VOLUME (max) */
+ (0x3f << 8) | /* SEEKTH */
+ ((band << 6) & SYSCONFIG2_BAND) | /* BAND */
+ ((space << 4) & SYSCONFIG2_SPACE) | /* SPACE */
+ 15; /* VOLUME (max) */
retval = si470x_set_register(radio, SYSCONFIG2);
if (retval < 0)
- return retval;
+ goto done;
/* reset last channel */
- return si470x_set_chan(radio,
+ retval = si470x_set_chan(radio,
radio->registers[CHANNEL] & CHANNEL_CHAN);
+
+done:
+ return retval;
}
@@ -766,13 +775,16 @@
radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_RDS;
retval = si470x_set_register(radio, SYSCONFIG1);
if (retval < 0)
- return retval;
+ goto done;
/* powercfg */
radio->registers[POWERCFG] &= ~POWERCFG_DMUTE;
/* POWERCFG_ENABLE has to automatically go low */
radio->registers[POWERCFG] |= POWERCFG_ENABLE | POWERCFG_DISABLE;
- return si470x_set_register(radio, POWERCFG);
+ retval = si470x_set_register(radio, POWERCFG);
+
+done:
+ return retval;
}
@@ -889,6 +901,7 @@
struct si470x_device *radio = container_of(work, struct si470x_device,
work.work);
+ /* safety checks */
if (radio->disconnected)
return;
if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0)
@@ -923,11 +936,15 @@
/* block if no new data available */
while (radio->wr_index == radio->rd_index) {
- if (file->f_flags & O_NONBLOCK)
- return -EWOULDBLOCK;
+ if (file->f_flags & O_NONBLOCK) {
+ retval = -EWOULDBLOCK;
+ goto done;
+ }
if (wait_event_interruptible(radio->read_queue,
- radio->wr_index != radio->rd_index) < 0)
- return -EINTR;
+ radio->wr_index != radio->rd_index) < 0) {
+ retval = -EINTR;
+ goto done;
+ }
}
/* calculate block count from byte count */
@@ -956,6 +973,7 @@
}
mutex_unlock(&radio->lock);
+done:
return retval;
}
@@ -967,6 +985,7 @@
struct poll_table_struct *pts)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
+ int retval = 0;
/* switch on rds reception */
if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0) {
@@ -978,9 +997,9 @@
poll_wait(file, &radio->read_queue, pts);
if (radio->rd_index != radio->wr_index)
- return POLLIN | POLLRDNORM;
+ retval = POLLIN | POLLRDNORM;
- return 0;
+ return retval;
}
@@ -997,17 +1016,18 @@
retval = usb_autopm_get_interface(radio->intf);
if (retval < 0) {
radio->users--;
- return -EIO;
+ retval = -EIO;
+ goto done;
}
if (radio->users == 1) {
retval = si470x_start(radio);
if (retval < 0)
usb_autopm_put_interface(radio->intf);
- return retval;
}
- return 0;
+done:
+ return retval;
}
@@ -1019,8 +1039,11 @@
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
int retval = 0;
- if (!radio)
- return -ENODEV;
+ /* safety check */
+ if (!radio) {
+ retval = -ENODEV;
+ goto done;
+ }
mutex_lock(&radio->disconnect_lock);
radio->users--;
@@ -1044,6 +1067,8 @@
unlock:
mutex_unlock(&radio->disconnect_lock);
+
+done:
return retval;
}
@@ -1131,7 +1156,7 @@
/*
* si470x_vidioc_g_input - get input
*/
-static int si470x_vidioc_g_input(struct file *filp, void *priv,
+static int si470x_vidioc_g_input(struct file *file, void *priv,
unsigned int *i)
{
*i = 0;
@@ -1143,12 +1168,18 @@
/*
* si470x_vidioc_s_input - set input
*/
-static int si470x_vidioc_s_input(struct file *filp, void *priv, unsigned int i)
+static int si470x_vidioc_s_input(struct file *file, void *priv, unsigned int i)
{
- if (i != 0)
- return -EINVAL;
+ int retval = 0;
- return 0;
+ /* safety checks */
+ if (i != 0)
+ retval = -EINVAL;
+
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": set input failed with %d\n", retval);
+ return retval;
}
@@ -1161,17 +1192,22 @@
unsigned char i;
int retval = -EINVAL;
+ /* safety checks */
+ if (!qc->id)
+ goto done;
+
for (i = 0; i < ARRAY_SIZE(si470x_v4l2_queryctrl); i++) {
- if (qc->id && qc->id == si470x_v4l2_queryctrl[i].id) {
+ if (qc->id == si470x_v4l2_queryctrl[i].id) {
memcpy(qc, &(si470x_v4l2_queryctrl[i]), sizeof(*qc));
retval = 0;
break;
}
}
+
+done:
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
- ": query control failed with %d\n", retval);
-
+ ": query controls failed with %d\n", retval);
return retval;
}
@@ -1183,9 +1219,13 @@
struct v4l2_control *ctrl)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
switch (ctrl->id) {
case V4L2_CID_AUDIO_VOLUME:
@@ -1196,9 +1236,15 @@
ctrl->value = ((radio->registers[POWERCFG] &
POWERCFG_DMUTE) == 0) ? 1 : 0;
break;
+ default:
+ retval = -EINVAL;
}
- return 0;
+done:
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": get control failed with %d\n", retval);
+ return retval;
}
@@ -1209,10 +1255,13 @@
struct v4l2_control *ctrl)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
- int retval;
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
switch (ctrl->id) {
case V4L2_CID_AUDIO_VOLUME:
@@ -1230,10 +1279,11 @@
default:
retval = -EINVAL;
}
+
+done:
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
": set control failed with %d\n", retval);
-
return retval;
}
@@ -1244,13 +1294,22 @@
static int si470x_vidioc_g_audio(struct file *file, void *priv,
struct v4l2_audio *audio)
{
- if (audio->index > 1)
- return -EINVAL;
+ int retval = 0;
+
+ /* safety checks */
+ if (audio->index != 0) {
+ retval = -EINVAL;
+ goto done;
+ }
strcpy(audio->name, "Radio");
audio->capability = V4L2_AUDCAP_STEREO;
- return 0;
+done:
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": get audio failed with %d\n", retval);
+ return retval;
}
@@ -1260,10 +1319,19 @@
static int si470x_vidioc_s_audio(struct file *file, void *priv,
struct v4l2_audio *audio)
{
- if (audio->index != 0)
- return -EINVAL;
+ int retval = 0;
- return 0;
+ /* safety checks */
+ if (audio->index != 0) {
+ retval = -EINVAL;
+ goto done;
+ }
+
+done:
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": set audio failed with %d\n", retval);
+ return retval;
}
@@ -1274,20 +1342,23 @@
struct v4l2_tuner *tuner)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
- int retval;
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
- if (tuner->index > 0)
- return -EINVAL;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
+ if ((tuner->index != 0) && (tuner->type != V4L2_TUNER_RADIO)) {
+ retval = -EINVAL;
+ goto done;
+ }
- /* read status rssi */
retval = si470x_get_register(radio, STATUSRSSI);
if (retval < 0)
- return retval;
+ goto done;
strcpy(tuner->name, "FM");
- tuner->type = V4L2_TUNER_RADIO;
switch (band) {
/* 0: 87.5 - 108 MHz (USA, Europe, default) */
default:
@@ -1321,7 +1392,11 @@
/* automatic frequency control: -1: freq to low, 1 freq to high */
tuner->afc = 0;
- return 0;
+done:
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": get tuner failed with %d\n", retval);
+ return retval;
}
@@ -1332,12 +1407,17 @@
struct v4l2_tuner *tuner)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
- int retval;
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
- if (tuner->index > 0)
- return -EINVAL;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
+ if ((tuner->index != 0) && (tuner->type != V4L2_TUNER_RADIO)) {
+ retval = -EINVAL;
+ goto done;
+ }
if (tuner->audmode == V4L2_TUNER_MODE_MONO)
radio->registers[POWERCFG] |= POWERCFG_MONO; /* force mono */
@@ -1345,10 +1425,11 @@
radio->registers[POWERCFG] &= ~POWERCFG_MONO; /* try stereo */
retval = si470x_set_register(radio, POWERCFG);
+
+done:
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
": set tuner failed with %d\n", retval);
-
return retval;
}
@@ -1360,12 +1441,25 @@
struct v4l2_frequency *freq)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
+ if ((freq->tuner != 0) && (freq->type != V4L2_TUNER_RADIO)) {
+ retval = -EINVAL;
+ goto done;
+ }
- freq->type = V4L2_TUNER_RADIO;
- return si470x_get_freq(radio, &radio->frequency);
+ retval = si470x_get_freq(radio, &freq->frequency);
+
+done:
+ if (retval < 0)
+ printk(KERN_WARNING DRIVER_NAME
+ ": get frequency failed with %d\n", retval);
+ return retval;
}
@@ -1376,19 +1470,25 @@
struct v4l2_frequency *freq)
{
struct si470x_device *radio = video_get_drvdata(video_devdata(file));
- int retval;
+ int retval = 0;
- if (radio->disconnected)
- return -EIO;
- if (freq->type != V4L2_TUNER_RADIO)
- return -EINVAL;
+ /* safety checks */
+ if (radio->disconnected) {
+ retval = -EIO;
+ goto done;
+ }
+ if ((freq->tuner != 0) && (freq->type != V4L2_TUNER_RADIO)) {
+ retval = -EINVAL;
+ goto done;
+ }
retval = si470x_set_freq(radio, freq->frequency);
+
+done:
if (retval < 0)
printk(KERN_WARNING DRIVER_NAME
": set frequency failed with %d\n", retval);
-
- return 0;
+ return retval;
}
@@ -1428,33 +1528,36 @@
const struct usb_device_id *id)
{
struct si470x_device *radio;
- int retval = -ENOMEM;
+ int retval = 0;
- /* private data allocation */
+ /* private data allocation and initialization */
radio = kzalloc(sizeof(struct si470x_device), GFP_KERNEL);
- if (!radio)
+ if (!radio) {
+ retval = -ENOMEM;
goto err_initial;
-
- /* video device allocation */
- radio->videodev = video_device_alloc();
- if (!radio->videodev)
- goto err_radio;
-
- /* initial configuration */
- memcpy(radio->videodev, &si470x_viddev_template,
- sizeof(si470x_viddev_template));
+ }
radio->users = 0;
radio->disconnected = 0;
radio->usbdev = interface_to_usbdev(intf);
radio->intf = intf;
mutex_init(&radio->disconnect_lock);
mutex_init(&radio->lock);
+
+ /* video device allocation and initialization */
+ radio->videodev = video_device_alloc();
+ if (!radio->videodev) {
+ retval = -ENOMEM;
+ goto err_radio;
+ }
+ memcpy(radio->videodev, &si470x_viddev_template,
+ sizeof(si470x_viddev_template));
video_set_drvdata(radio->videodev, radio);
/* show some infos about the specific device */
- retval = -EIO;
- if (si470x_get_all_registers(radio) < 0)
+ if (si470x_get_all_registers(radio) < 0) {
+ retval = -EIO;
goto err_all;
+ }
printk(KERN_INFO DRIVER_NAME ": DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
radio->registers[DEVICEID], radio->registers[CHIPID]);
@@ -1480,8 +1583,10 @@
/* rds buffer allocation */
radio->buf_size = rds_buf * 3;
radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
- if (!radio->buffer)
+ if (!radio->buffer) {
+ retval = -EIO;
goto err_all;
+ }
/* rds buffer configuration */
radio->wr_index = 0;
@@ -1493,6 +1598,7 @@
/* register video device */
if (video_register_device(radio->videodev, VFL_TYPE_RADIO, radio_nr)) {
+ retval = -EIO;
printk(KERN_WARNING DRIVER_NAME
": Could not register video device\n");
goto err_all;