From a7c850a07eddf0e0f515d57b2ce696a9779c6ed1 Mon Sep 17 00:00:00 2001 From: Tobias Lorenz Date: Sat, 31 May 2008 15:07:52 -0300 Subject: [PATCH] V4L/DVB (7995): si470x: a lot of small code cleanups MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/radio-si470x.c | 286 ++++++++++++++++++++--------- 1 file changed, 196 insertions(+), 90 deletions(-) diff --git a/drivers/media/radio/radio-si470x.c b/drivers/media/radio/radio-si470x.c index 7df1163d7097..954ba997895d 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 * 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; -- 2.20.1