Skip to contentSkip to conversation

radio-mr800: no need to pass curfreq value to amradio_setfreq()

PATCH In review Alexey Klimov 2 participants 2 messages Aug 8, 2009 – Aug 10, 2009 Media (V4L/DVB)

lore
Share & tools
lore

Alexey Klimov

author
#0 lore raw

PATCH 3/6 radio-mr800: no need to pass curfreq value to amradio_setfreq()

Small cleanup of amradio_setfreq(). No need to pass radio->curfreq value to this function.

Signed-off-by Alexey Klimov <[email protected]>

--

linux/drivers/media/radio/radio-mr800.c +3 -3
diff -r 5f3329bebfe4 linux/drivers/media/radio/radio-mr800.c
@@ -202,11 +202,11 @@
 }
 
 /* set a frequency, freq is defined by v4l's TUNER_LOW, i.e. 1/16th kHz */
-static int amradio_setfreq(struct amradio_device *radio, int freq)
+static int amradio_setfreq(struct amradio_device *radio)
 {
 	int retval;
 	int size;
-	unsigned short freq_send = 0x10 + (freq >> 3) / 25;
+	unsigned short freq_send = 0x10 + (radio->curfreq >> 3) / 25;
 
 	/* safety check */
 	if (radio->removed)

@@ -413,7 +413,7 @@
 	radio->curfreq = f->frequency;
 	mutex_unlock(&radio->lock);
 
-	retval = amradio_setfreq(radio, radio->curfreq);
+	retval = amradio_setfreq(radio);
 	if (retval < 0)
 		amradio_dev_warn(&radio->videodev->dev,
 			"set frequency failed\n");
Signature · 1 line
Best regards, Klimov Alexey
#1 lore raw
Quoted 10 lines · Alexey Klimov

Small cleanup of amradio_setfreq(). No need to pass radio->curfreq value to this function.

Signed-off-by Alexey Klimov <[email protected]>

--
diff -r 5f3329bebfe4 linux/drivers/media/radio/radio-mr800.c

linux/drivers/media/radio/radio-mr800.c   Wed Jul 29 12:41:51 2009 +0400 line 209

Quoted 10 lines
linux/drivers/media/radio/radio-mr800.c   Wed Jul 29 12:41:51 2009 +0400 renamed from linux/drivers/media/radio/radio-mr800.c   Wed Jul 29 12:36:46 2009 +0400 +2 -2
@@ -202,11 +202,11 @@
  }
 
  /* set a frequency, freq is defined by v4l's TUNER_LOW, i.e. 1/16th kHz */
-static int amradio_setfreq(struct amradio_device *radio, int freq)
+static int amradio_setfreq(struct amradio_device *radio)
  {
        int retval;
        int size;
-       unsigned short freq_send = 0x10 + (freq >> 3) / 25;
+       unsigned short freq_send = 0x10 + (radio->curfreq >> 3) / 25;

I suspect there may be race conditions here. Once again you're reading a value without first acquiring the lock. Since this is another utility function, the lock should probably be held _before_ calling this function and any locking in this function should be removed. Adding a BUG_ON(!is_mutex_locked(&radio->lock)) should probably be added as well.

Quoted 18 lines
linux/drivers/media/radio/radio-mr800.c   Wed Jul 29 12:41:51 2009 +0400 renamed from linux/drivers/media/radio/radio-mr800.c   Wed Jul 29 12:36:46 2009 +0400 +1 -2
        /* safety check */
        if (radio->removed)

@@ -413,7 +413,7 @@
        radio->curfreq = f->frequency;
        mutex_unlock(&radio->lock);
 
-       retval = amradio_setfreq(radio, radio->curfreq);
+       retval = amradio_setfreq(radio);
        if (retval < 0)
                amradio_dev_warn(&radio->videodev->dev,
                        "set frequency failed\n");
 
 
 
--

Best regards, Klimov Alexey

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,

David Ellingsworth