[alsa-devel] [PATCH] tea575x: fix HW seek

[Hans Verkuil]

On Saturday, February 18, 2012 17:45:45 Ondrej Zary wrote:
> Fix HW seek in TEA575x to work properly:
>  - a delay must be present after search start and before first register read
>    or the seek does weird things
>  - when the search stops, the new frequency is not available immediately, we
>    must wait until it appears in the register (fortunately, we can clear the
>    frequency bits when starting the search as it starts at the frequency
>    currently set, not from the value written)
>  - sometimes, seek remains on the current frequency (or moves only a little),
>    so repeat it until it moves by at least 50 kHz
> 
> Signed-off-by: Ondrej Zary <linux at rainbow-software.org>
> 
> --- a/sound/i2c/other/tea575x-tuner.c
> +++ b/sound/i2c/other/tea575x-tuner.c
> @@ -89,7 +89,7 @@ static void snd_tea575x_write(struct snd_tea575x *tea, unsigned int val)
>  		tea->ops->set_pins(tea, 0);
>  }
>  
> -static unsigned int snd_tea575x_read(struct snd_tea575x *tea)
> +static u32 snd_tea575x_read(struct snd_tea575x *tea)
>  {
>  	u16 l, rdata;
>  	u32 data = 0;
> @@ -120,6 +120,27 @@ static unsigned int snd_tea575x_read(struct snd_tea575x *tea)
>  	return data;
>  }
>  
> +static void snd_tea575x_get_freq(struct snd_tea575x *tea)
> +{
> +	u32 freq = snd_tea575x_read(tea) & TEA575X_BIT_FREQ_MASK;
> +
> +	if (freq == 0) {
> +		tea->freq = 0;

Wouldn't it be better to return -EBUSY in this case? VIDIOC_G_FREQUENCY
should not return frequencies outside the valid frequency range. In this case
returning -EBUSY seems to make more sense to me.

I'm proposing adding this patch:

diff --git a/sound/i2c/other/tea575x-tuner.c b/sound/i2c/other/tea575x-tuner.c
index 474bb81..02da22f 100644
--- a/sound/i2c/other/tea575x-tuner.c
+++ b/sound/i2c/other/tea575x-tuner.c
@@ -120,14 +120,12 @@ static u32 snd_tea575x_read(struct snd_tea575x *tea)
 	return data;
 }
 
-static void snd_tea575x_get_freq(struct snd_tea575x *tea)
+static int snd_tea575x_get_freq(struct snd_tea575x *tea)
 {
 	u32 freq = snd_tea575x_read(tea) & TEA575X_BIT_FREQ_MASK;
 
-	if (freq == 0) {
-		tea->freq = 0;
-		return;
-	}
+	if (freq == 0)
+		return -EBUSY;
 
 	/* freq *= 12.5 */
 	freq *= 125;
@@ -139,6 +137,7 @@ static void snd_tea575x_get_freq(struct snd_tea575x *tea)
 		freq -= TEA575X_FMIF;
 
 	tea->freq = clamp(freq * 16, FREQ_LO, FREQ_HI); /* from kHz */
+	return 0;
 }
 
 static void snd_tea575x_set_freq(struct snd_tea575x *tea)
@@ -220,14 +219,15 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 					struct v4l2_frequency *f)
 {
 	struct snd_tea575x *tea = video_drvdata(file);
+	int ret = 0;
 
 	if (f->tuner != 0)
 		return -EINVAL;
 	f->type = V4L2_TUNER_RADIO;
 	if (!tea->cannot_read_data)
-		snd_tea575x_get_freq(tea);
+		ret = snd_tea575x_get_freq(tea);
 	f->frequency = tea->freq;
-	return 0;
+	return ret;
 }
 
 static int vidioc_s_frequency(struct file *file, void *priv,
@@ -250,11 +250,14 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
 	struct snd_tea575x *tea = video_drvdata(file);
 	int i, old_freq;
 	unsigned long timeout;
+	int ret;
 
 	if (tea->cannot_read_data)
 		return -ENOTTY;
 
-	snd_tea575x_get_freq(tea);
+	ret = snd_tea575x_get_freq(tea);
+	if (ret)
+		return ret;
 	old_freq = tea->freq;
 	/* clear the frequency, HW will fill it in */
 	tea->val &= ~TEA575X_BIT_FREQ_MASK;
@@ -278,8 +281,8 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
 			/* Found a frequency, wait until it can be read */
 			for (i = 0; i < 100; i++) {
 				msleep(10);
-				snd_tea575x_get_freq(tea);
-				if (tea->freq != 0) /* available */
+				ret = snd_tea575x_get_freq(tea);
+				if (ret == 0) /* available */
 					break;
 			}
 			/* if we moved by less than 50 kHz, continue seeking */

If you are OK with this, then I'll put everything together and make a final
pull request.

Regards,

	Hans