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>

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 @@ static int si470x_get_report(struct si470x_device *radio, void *buf, int size)
 		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 @@ static int si470x_set_report(struct si470x_device *radio, void *buf, int size)
 		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 @@ static int si470x_set_chan(struct si470x_device *radio, unsigned short chan)
 	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 @@ static int si470x_start(struct si470x_device *radio)
 		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 @@ static int si470x_stop(struct si470x_device *radio)
 	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 @@ static void si470x_work(struct work_struct *work)
 	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 @@ static ssize_t si470x_fops_read(struct file *file, char __user *buf,
 
 	/* 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 @@ static ssize_t si470x_fops_read(struct file *file, char __user *buf,
 	}
 	mutex_unlock(&radio->lock);
 
+done:
 	return retval;
 }
 
@@ -967,6 +985,7 @@ static unsigned int si470x_fops_poll(struct file *file,
 		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 @@ static unsigned int si470x_fops_poll(struct file *file,
 	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 @@ static int si470x_fops_open(struct inode *inode, struct file *file)
 	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 @@ static int si470x_fops_release(struct inode *inode, struct file *file)
 	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 @@ static int si470x_fops_release(struct inode *inode, struct file *file)
 
 unlock:
 	mutex_unlock(&radio->disconnect_lock);
+
+done:
 	return retval;
 }
 
@@ -1131,7 +1156,7 @@ static int si470x_vidioc_querycap(struct file *file, void *priv,
 /*
  * 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 @@ static int si470x_vidioc_g_input(struct file *filp, void *priv,
 /*
  * 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)
 {
+	int retval = 0;
+
+	/* safety checks */
 	if (i != 0)
-		return -EINVAL;
+		retval = -EINVAL;
 
-	return 0;
+	if (retval < 0)
+		printk(KERN_WARNING DRIVER_NAME
+			": set input failed with %d\n", retval);
+	return retval;
 }
 
 
@@ -1161,17 +1192,22 @@ static int si470x_vidioc_queryctrl(struct file *file, void *priv,
 	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 @@ static int si470x_vidioc_g_ctrl(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_g_ctrl(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_s_ctrl(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_s_ctrl(struct file *file, void *priv,
 	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_s_ctrl(struct file *file, void *priv,
 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_g_audio(struct file *file, void *priv,
 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 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv,
 	/* 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 @@ static int si470x_vidioc_s_tuner(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_s_tuner(struct file *file, void *priv,
 		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 @@ static int si470x_vidioc_g_frequency(struct file *file, void *priv,
 		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;
+	}
+
+	retval = si470x_get_freq(radio, &freq->frequency);
 
-	freq->type = V4L2_TUNER_RADIO;
-	return si470x_get_freq(radio, &radio->frequency);
+done:
+	if (retval < 0)
+		printk(KERN_WARNING DRIVER_NAME
+			": get frequency failed with %d\n", retval);
+	return retval;
 }
 
 
@@ -1376,19 +1470,25 @@ static int si470x_vidioc_s_frequency(struct file *file, void *priv,
 		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 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 		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 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 	/* 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 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 
 	/* 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;