V4L/DVB (7062): radio-si570x: Some fixes and new USB ID addition

- avoid poss. locking when doing copy_to_user which may sleep
- RDS is automatically activated on read now
- code cleaned of unnecessary rds_commands
- USB Vendor/Product ID for ADS/Tech FM Radio Receiver verified
  (thanks to Guillaume RAMOUSSE)

Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

1 files changed, 156 insertions, 135 deletions

diff --git a/drivers/media/radio/radio-si470x.c b/drivers/media/radio/radio-si470x.c
index d54fe6405351..8e4bd4769048 100644
--- a/drivers/media/radio/radio-si470x.c
+++ b/drivers/media/radio/radio-si470x.c
@@ -55,13 +55,17 @@
  *              - applied all checkpatch.pl v1.12 suggestions
  *                except the warning about the too long lines with bit comments
  *              - renamed FMRADIO to RADIO to cut line length (checkpatch.pl)
+ * 2008-01-22   Tobias Lorenz <tobias.lorenz@gmx.net>
+ *              Version 1.0.4
+ *              - avoid poss. locking when doing copy_to_user which may sleep
+ *              - RDS is automatically activated on read now
+ *              - code cleaned of unnecessary rds_commands
+ *              - USB Vendor/Product ID for ADS/Tech FM Radio Receiver verified
+ *                (thanks to Guillaume RAMOUSSE)
  *
  * ToDo:
- * - check USB Vendor/Product ID for ADS/Tech FM Radio Receiver
- *   (formerly Instant FM Music) (RDX-155-EF) is 06e1:a155
  * - add seeking support
  * - add firmware download/update support
- * - add possibility to switch off RDS
  * - RDS support: interrupt mode, instead of polling
  * - add LED status output (check if that's not already done in firmware)
  */
@@ -70,7 +74,7 @@
 /* driver definitions */
 #define DRIVER_AUTHOR "Tobias Lorenz <tobias.lorenz@gmx.net>"
 #define DRIVER_NAME "radio-si470x"
-#define DRIVER_VERSION KERNEL_VERSION(1, 0, 3)
+#define DRIVER_VERSION KERNEL_VERSION(1, 0, 4)
 #define DRIVER_CARD "Silicon Labs Si470x FM Radio Receiver"
 #define DRIVER_DESC "USB radio driver for Si470x FM Radio Receivers"
 
@@ -93,6 +97,8 @@
 static struct usb_device_id si470x_usb_driver_id_table[] = {
         /* Silicon Labs USB FM Radio Reference Design */
         { USB_DEVICE_AND_INTERFACE_INFO(0x10c4, 0x818a, USB_CLASS_HID, 0, 0) },
+        /* ADS/Tech FM Radio Receiver (formerly Instant FM Music) */
+        { USB_DEVICE_AND_INTERFACE_INFO(0x06e1, 0xa155, USB_CLASS_HID, 0, 0) },
         /* Terminating entry */
         { }
 };
@@ -159,6 +165,7 @@ MODULE_PARM_DESC(max_rds_errors, "RDS maximum block errors: *1*");
 /* RDS poll frequency */
 static int rds_poll_time = 40;
 /* 40 is used by the original USBRadio.exe */
+/* 50 is used by radio-cadet */
 /* 75 should be okay */
 /* 80 is the usual RDS receive interval */
 module_param(rds_poll_time, int, 0);
@@ -399,16 +406,13 @@ struct si470x_device {
 
         /* RDS receive buffer */
         struct work_struct work;
+        wait_queue_head_t read_queue;
         struct timer_list timer;
         spinlock_t lock;                /* buffer locking */
-        unsigned char *buffer;
+        unsigned char *buffer;          /* size is always multiple of three */
         unsigned int buf_size;
         unsigned int rd_index;
         unsigned int wr_index;
-        unsigned int block_count;
-        unsigned char last_blocknum;
-        wait_queue_head_t read_queue;
-        int data_available_for_read;
 };
 
 
@@ -658,8 +662,7 @@ static int si470x_start(struct si470x_device *radio)
                 return retval;
 
         /* sysconfig 1 */
-        radio->registers[SYSCONFIG1] =
-                SYSCONFIG1_DE | SYSCONFIG1_RDS;
+        radio->registers[SYSCONFIG1] = SYSCONFIG1_DE;
         retval = si470x_set_register(radio, SYSCONFIG1);
         if (retval < 0)
                 return retval;
@@ -685,6 +688,14 @@ static int si470x_start(struct si470x_device *radio)
  */
 static int si470x_stop(struct si470x_device *radio)
 {
+        int retval;
+
+        /* sysconfig 1 */
+        radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_RDS;
+        retval = si470x_set_register(radio, SYSCONFIG1);
+        if (retval < 0)
+                return retval;
+
         /* powercfg */
         radio->registers[POWERCFG] &= ~POWERCFG_DMUTE;
         /* POWERCFG_ENABLE has to automatically go low */
@@ -693,6 +704,17 @@ static int si470x_stop(struct si470x_device *radio)
 }
 
 
+/*
+ * si470x_rds_on - switch on rds reception
+ */
+static int si470x_rds_on(struct si470x_device *radio)
+{
+        /* sysconfig 1 */
+        radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDS;
+        return si470x_set_register(radio, SYSCONFIG1);
+}
+
+
 
 /**************************************************************************
  * RDS Driver Functions
@@ -703,15 +725,13 @@ static int si470x_stop(struct si470x_device *radio)
  */
 static void si470x_rds(struct si470x_device *radio)
 {
-        unsigned long flags;
         unsigned char tmpbuf[3];
         unsigned char blocknum;
-        unsigned char bler; /* RDS block errors */
+        unsigned char bler; /* rds block errors */
         unsigned short rds;
         unsigned int i;
 
-        if (radio->users == 0)
-                return;
+        /* get rds blocks */
         if (si470x_get_rds_registers(radio) < 0)
                 return;
         if ((radio->registers[STATUSRSSI] & STATUSRSSI_RDSR) == 0) {
@@ -723,63 +743,67 @@ static void si470x_rds(struct si470x_device *radio)
                 return;
         }
 
-        for (blocknum = 0; blocknum < 4; blocknum++) {
-                switch (blocknum) {
-                default:
-                        bler = (radio->registers[STATUSRSSI] &
-                                        STATUSRSSI_BLERA) >> 9;
-                        rds = radio->registers[RDSA];
-                        break;
-                case 1:
-                        bler = (radio->registers[READCHAN] &
-                                        READCHAN_BLERB) >> 14;
-                        rds = radio->registers[RDSB];
-                        break;
-                case 2:
-                        bler = (radio->registers[READCHAN] &
-                                        READCHAN_BLERC) >> 12;
-                        rds = radio->registers[RDSC];
-                        break;
-                case 3:
-                        bler = (radio->registers[READCHAN] &
-                                        READCHAN_BLERD) >> 10;
-                        rds = radio->registers[RDSD];
-                        break;
-                };
-
-                /* Fill the V4L2 RDS buffer */
-                tmpbuf[0] = rds & 0x00ff;       /* LSB */
-                tmpbuf[1] = (rds & 0xff00) >> 8;/* MSB */
-                tmpbuf[2] = blocknum;           /* offset name */
-                tmpbuf[2] |= blocknum << 3;     /* received offset */
-                if (bler > max_rds_errors)
-                        tmpbuf[2] |= 0x80;      /* uncorrectable errors */
-                else if (bler > 0)
-                        tmpbuf[2] |= 0x40;      /* corrected error(s) */
-
-                spin_lock_irqsave(&radio->lock, flags);
-
-                /* copy RDS block to internal buffer */
-                for (i = 0; i < 3; i++) {
-                        radio->buffer[radio->wr_index] = tmpbuf[i];
-                        radio->wr_index++;
+        /* copy four RDS blocks to internal buffer */
+        if (spin_trylock(&radio->lock)) {
+                /* process each rds block */
+                for (blocknum = 0; blocknum < 4; blocknum++) {
+                        switch (blocknum) {
+                        default:
+                                bler = (radio->registers[STATUSRSSI] &
+                                                STATUSRSSI_BLERA) >> 9;
+                                rds = radio->registers[RDSA];
+                                break;
+                        case 1:
+                                bler = (radio->registers[READCHAN] &
+                                                READCHAN_BLERB) >> 14;
+                                rds = radio->registers[RDSB];
+                                break;
+                        case 2:
+                                bler = (radio->registers[READCHAN] &
+                                                READCHAN_BLERC) >> 12;
+                                rds = radio->registers[RDSC];
+                                break;
+                        case 3:
+                                bler = (radio->registers[READCHAN] &
+                                                READCHAN_BLERD) >> 10;
+                                rds = radio->registers[RDSD];
+                                break;
+                        };
+
+                        /* Fill the V4L2 RDS buffer */
+                        tmpbuf[0] = rds & 0x00ff;       /* LSB */
+                        tmpbuf[1] = (rds & 0xff00) >> 8;/* MSB */
+                        tmpbuf[2] = blocknum;           /* offset name */
+                        tmpbuf[2] |= blocknum << 3;     /* received offset */
+                        if (bler > max_rds_errors)
+                                tmpbuf[2] |= 0x80; /* uncorrectable errors */
+                        else if (bler > 0)
+                                tmpbuf[2] |= 0x40; /* corrected error(s) */
+
+                        /* copy RDS block to internal buffer */
+                        for (i = 0; i < 3; i++) {
+                                radio->buffer[radio->wr_index] = tmpbuf[i];
+                                radio->wr_index++;
+                        }
+
+                        /* wrap write pointer */
+                        if (radio->wr_index >= radio->buf_size)
+                                radio->wr_index = 0;
+
+                        /* check for overflow */
+                        if (radio->wr_index == radio->rd_index) {
+                                /* increment and wrap read pointer */
+                                radio->rd_index += 3;
+                                if (radio->rd_index >= radio->buf_size)
+                                        radio->rd_index = 0;
+                        }
                 }
-
-                if (radio->wr_index >= radio->buf_size)
-                        radio->wr_index = 0;
-
-                if (radio->wr_index == radio->rd_index) {
-                        radio->rd_index += 3;
-                        if (radio->rd_index >= radio->buf_size)
-                                radio->rd_index = 0;
-                } else
-                        radio->block_count++;
-
-                spin_unlock_irqrestore(&radio->lock, flags);
+                spin_unlock(&radio->lock);
         }
 
-        radio->data_available_for_read = 1;
-        wake_up_interruptible(&radio->read_queue);
+        /* wake up read queue */
+        if (radio->wr_index != radio->rd_index)
+                wake_up_interruptible(&radio->read_queue);
 }
 
 
@@ -795,14 +819,14 @@ static void si470x_timer(unsigned long data)
 
 
 /*
- * si470x_timer - rds work function
+ * si470x_work - rds work function
  */
 static void si470x_work(struct work_struct *work)
 {
         struct si470x_device *radio = container_of(work, struct si470x_device,
                 work);
 
-        if (radio->users == 0)
+        if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0)
                 return;
 
         si470x_rds(radio);
@@ -822,53 +846,52 @@ static ssize_t si470x_fops_read(struct file *file, char __user *buf,
                 size_t count, loff_t *ppos)
 {
         struct si470x_device *radio = video_get_drvdata(video_devdata(file));
-        struct rds_command cmd;
-        unsigned long flags;
-        unsigned int i;
-        unsigned int rd_blocks;
-
-        cmd.block_count = count / 3; /* each RDS block needs 3 bytes */
-        cmd.result = 0;
-        cmd.buffer = buf;
-        cmd.instance = file;
-
-        /* copy RDS block out of internal buffer */
-        while (!radio->data_available_for_read) {
-                if (wait_event_interruptible(radio->read_queue,
-                                     radio->data_available_for_read) < 0)
-                        return -EINTR;
-        }
+        int retval = 0;
+        unsigned int block_count = 0;
 
-        spin_lock_irqsave(&radio->lock, flags);
-        rd_blocks = cmd.block_count;
-        if (rd_blocks > radio->block_count)
-                rd_blocks = radio->block_count;
+        /* switch on rds reception */
+        if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0) {
+                si470x_rds_on(radio);
+                schedule_work(&radio->work);
+        }
 
-        if (!rd_blocks) {
-                spin_unlock_irqrestore(&radio->lock, flags);
-                return cmd.result;
+        /* block if no new data available */
+        while (radio->wr_index == radio->rd_index) {
+                if (file->f_flags & O_NONBLOCK)
+                        return -EWOULDBLOCK;
+                interruptible_sleep_on(&radio->read_queue);
         }
 
-        for (i = 0; i < rd_blocks; i++) {
-                /* copy RDS block to user buffer */
-                if (radio->rd_index == radio->wr_index)
-                        break;
+        /* calculate block count from byte count */
+        count /= 3;
+
+        /* copy RDS block out of internal buffer and to user buffer */
+        if (spin_trylock(&radio->lock)) {
+                while (block_count < count) {
+                        if (radio->rd_index == radio->wr_index)
+                                break;
 
-                if (copy_to_user(buf, &radio->buffer[radio->rd_index], 3))
-                        break;
+                        /* always transfer rds complete blocks */
+                        if (copy_to_user(buf,
+                                        &radio->buffer[radio->rd_index], 3))
+                                /* retval = -EFAULT; */
+                                break;
 
-                radio->rd_index += 3;
-                if (radio->rd_index >= radio->buf_size)
-                        radio->rd_index = 0;
-                radio->block_count--;
+                        /* increment and wrap read pointer */
+                        radio->rd_index += 3;
+                        if (radio->rd_index >= radio->buf_size)
+                                radio->rd_index = 0;
+
+                        /* increment counters */
+                        block_count++;
+                        buf += 3;
+                        retval += 3;
+                }
 
-                buf += 3;
-                cmd.result += 3;
+                spin_unlock(&radio->lock);
         }
-        radio->data_available_for_read = (radio->block_count > 0);
-        spin_unlock_irqrestore(&radio->lock, flags);
 
-        return cmd.result;
+        return retval;
 }
 
 
@@ -879,14 +902,19 @@ 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;
 
-        retval = 0;
-        if (radio->data_available_for_read)
-                retval = POLLIN | POLLRDNORM;
+        /* switch on rds reception */
+        if ((radio->registers[SYSCONFIG1] & SYSCONFIG1_RDS) == 0) {
+                si470x_rds_on(radio);
+                schedule_work(&radio->work);
+        }
+
         poll_wait(file, &radio->read_queue, pts);
 
-        return retval;
+        if (radio->rd_index != radio->wr_index)
+                return POLLIN | POLLRDNORM;
+
+        return 0;
 }
 
 
@@ -895,17 +923,11 @@ static unsigned int si470x_fops_poll(struct file *file,
  */
 static int si470x_fops_open(struct inode *inode, struct file *file)
 {
-        int retval;
         struct si470x_device *radio = video_get_drvdata(video_devdata(file));
 
         radio->users++;
-        if (radio->users == 1) {
-                retval = si470x_start(radio);
-                if (retval < 0)
-                        return retval;
-
-                schedule_work(&radio->work);
-        }
+        if (radio->users == 1)
+                return si470x_start(radio);
 
         return 0;
 }
@@ -916,7 +938,6 @@ static int si470x_fops_open(struct inode *inode, struct file *file)
  */
 static int si470x_fops_release(struct inode *inode, struct file *file)
 {
-        int retval;
         struct si470x_device *radio = video_get_drvdata(video_devdata(file));
 
         if (!radio)
@@ -924,12 +945,14 @@ static int si470x_fops_release(struct inode *inode, struct file *file)
 
         radio->users--;
         if (radio->users == 0) {
-                radio->data_available_for_read = 1;             /* ? */
-                wake_up_interruptible(&radio->read_queue);      /* ? */
+                /* stop rds reception */
+                del_timer_sync(&radio->timer);
+                flush_scheduled_work();
 
-                retval = si470x_stop(radio);
-                if (retval < 0)
-                        return retval;
+                /* cancel read processes */
+                wake_up_interruptible(&radio->read_queue);
+
+                return si470x_stop(radio);
         }
 
         return 0;
@@ -1314,9 +1337,10 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
                         < RADIO_SW_VERSION_CURRENT)
                 printk(KERN_WARNING DRIVER_NAME
                         ": This driver is known to work with chip version %d, "
-                        "but the device has firmware %d. If you have some "
-                        "trouble using this driver, please report to V4L ML "
-                        "at video4linux-list@redhat.com\n",
+                        "but the device has firmware %d.\n"
+                        DRIVER_NAME
+                        "If you have some trouble using this driver, please "
+                        "report to V4L ML at video4linux-list@redhat.com\n",
                         radio->registers[CHIPID] & CHIPID_FIRMWARE,
                         RADIO_SW_VERSION_CURRENT);
 
@@ -1331,12 +1355,9 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
                 kfree(radio);
                 return -ENOMEM;
         }
-        radio->block_count = 0;
         radio->wr_index = 0;
         radio->rd_index = 0;
-        radio->last_blocknum = 0xff;
         init_waitqueue_head(&radio->read_queue);
-        radio->data_available_for_read = 0;
 
         /* prepare polling via eventd */
         INIT_WORK(&radio->work, si470x_work);
@@ -1408,4 +1429,4 @@ module_exit(si470x_module_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_VERSION("1.0.3");
+MODULE_VERSION("1.0.4");