Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > These patches improve the tea575x-tuner module to make it up to date
> > > with the latest V4L2 frameworks.
> > >
> > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > and I've used that card to test it.
> > >
> > > Unfortunately, this card can't read the data pin, so the new hardware
> > > seek functionality has been tested only partially (yes, it seeks, but
> > > when it finds a channel I can't read back the frequency).
> > >
> > > Ondrej, are you able to test these patches for the sound cards that use
> > > this tea575x tuner?
> > >
> > > Note that these two patches rely on other work that I did and that
> > > hasn't been merged yet. So it is best to pull from my git tree:
> > >
> > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > >o-pc i2
> > >
> > > You can use the v4l-utils repository
> > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > hardware seek:
> > >
> > > To seek down:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > >
> > > To seek up:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > >
> > > To do the compliance test:
> > >
> > > v4l2-compliance -r /dev/radio0
> >
> > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > severely broken. Reading the frequency immediately after seek does not
> > work, it always returns the old value (haven't found a delay that works).
> > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > wrong place (only noise) or even outside the FM range.
> >
> > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > be completely broken (unless there's some weird bug in the code).
>
> Well, it seemed like a good idea at the time :-) I'll remove this
> 'feature', it's really not worth our time to try and make this work for
> these old cards.

I fixed it somehow. Now it works most of the time.
The important things:
 - 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

--- 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;
+		return;
+	}
+
+	/* freq *= 12.5 */
+	freq *= 125;
+	freq /= 10;
+	/* crystal fixup */
+	if (tea->tea5759)
+		freq += TEA575X_FMIF;
+	else
+		freq -= TEA575X_FMIF;
+
+	tea->freq = clamp(freq * 16, FREQ_LO, FREQ_HI); /* from kHz */
+}
+
 static void snd_tea575x_set_freq(struct snd_tea575x *tea)
 {
 	u32 freq = tea->freq;
@@ -203,6 +224,8 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 	if (f->tuner != 0)
 		return -EINVAL;
 	f->type = V4L2_TUNER_RADIO;
+	if (!tea->cannot_read_data)
+		snd_tea575x_get_freq(tea);
 	f->frequency = tea->freq;
 	return 0;
 }
@@ -225,36 +248,50 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
 					struct v4l2_hw_freq_seek *a)
 {
 	struct snd_tea575x *tea = video_drvdata(file);
+	int i, old_freq;
+	unsigned long timeout;
 
 	if (tea->cannot_read_data)
 		return -ENOTTY;
+
+	snd_tea575x_get_freq(tea);
+	old_freq = tea->freq;
+	/* clear the frequency, HW will fill it in */
+	tea->val &= ~TEA575X_BIT_FREQ_MASK;
 	tea->val |= TEA575X_BIT_SEARCH;
-	tea->val &= ~TEA575X_BIT_UPDOWN;
 	if (a->seek_upward)
 		tea->val |= TEA575X_BIT_UPDOWN;
+	else
+		tea->val &= ~TEA575X_BIT_UPDOWN;
 	snd_tea575x_write(tea, tea->val);
+	timeout = jiffies + msecs_to_jiffies(10000);
 	for (;;) {
-		unsigned val = snd_tea575x_read(tea);
-
-		if (!(val & TEA575X_BIT_SEARCH)) {
-			/* Found a frequency */
-			val &= TEA575X_BIT_FREQ_MASK;
-			val = (val * 10) / 125;
-			if (tea->tea5759)
-				val += TEA575X_FMIF;
-			else
-				val -= TEA575X_FMIF;
-			tea->freq = clamp(val * 16, FREQ_LO, FREQ_HI);
-			return 0;
-		}
+		if (time_after(jiffies, timeout))
+			break;
 		if (schedule_timeout_interruptible(msecs_to_jiffies(10))) {
 			/* some signal arrived, stop search */
 			tea->val &= ~TEA575X_BIT_SEARCH;
 			snd_tea575x_write(tea, tea->val);
 			return -ERESTARTSYS;
 		}
+		if (!(snd_tea575x_read(tea) & TEA575X_BIT_SEARCH)) {
+			/* 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 */
+					break;
+			}
+			/* if we moved by less than 50 kHz, continue seeking */
+			if (abs(old_freq - tea->freq) < 16 * 500) {
+				snd_tea575x_write(tea, tea->val);
+				continue;
+			}
+			tea->val &= ~TEA575X_BIT_SEARCH;
+			return 0;
+		}
 	}
-	return 0;
+	return -ETIMEDOUT;
 }
 
 static int tea575x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -318,7 +355,7 @@ int snd_tea575x_init(struct snd_tea575x *tea)
 			return -ENODEV;
 	}
 
-	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
+	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_5_28;
 	tea->freq = 90500 * 16;		/* 90.5Mhz default */
 	snd_tea575x_set_freq(tea);
 



-- 
Ondrej Zary

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Hans Verkuil]

On Friday, February 17, 2012 21:13:40 Ondrej Zary wrote:
> On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> > On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > > These patches improve the tea575x-tuner module to make it up to date
> > > > with the latest V4L2 frameworks.
> > > >
> > > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > > and I've used that card to test it.
> > > >
> > > > Unfortunately, this card can't read the data pin, so the new hardware
> > > > seek functionality has been tested only partially (yes, it seeks, but
> > > > when it finds a channel I can't read back the frequency).
> > > >
> > > > Ondrej, are you able to test these patches for the sound cards that use
> > > > this tea575x tuner?
> > > >
> > > > Note that these two patches rely on other work that I did and that
> > > > hasn't been merged yet. So it is best to pull from my git tree:
> > > >
> > > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > > >o-pc i2
> > > >
> > > > You can use the v4l-utils repository
> > > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > > hardware seek:
> > > >
> > > > To seek down:
> > > >
> > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > > >
> > > > To seek up:
> > > >
> > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > > >
> > > > To do the compliance test:
> > > >
> > > > v4l2-compliance -r /dev/radio0
> > >
> > > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > > severely broken. Reading the frequency immediately after seek does not
> > > work, it always returns the old value (haven't found a delay that works).
> > > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > > wrong place (only noise) or even outside the FM range.
> > >
> > > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > > be completely broken (unless there's some weird bug in the code).
> >
> > Well, it seemed like a good idea at the time :-) I'll remove this
> > 'feature', it's really not worth our time to try and make this work for
> > these old cards.
> 
> I fixed it somehow. Now it works most of the time.
> The important things:
>  - 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

Thanks! And I'm impressed that you spent all that time on this old hardware :-)

Can I get your Signed-off-by line for this patch?

Regards,

	Hans

> 
> --- 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;
> +		return;
> +	}
> +
> +	/* freq *= 12.5 */
> +	freq *= 125;
> +	freq /= 10;
> +	/* crystal fixup */
> +	if (tea->tea5759)
> +		freq += TEA575X_FMIF;
> +	else
> +		freq -= TEA575X_FMIF;
> +
> +	tea->freq = clamp(freq * 16, FREQ_LO, FREQ_HI); /* from kHz */
> +}
> +
>  static void snd_tea575x_set_freq(struct snd_tea575x *tea)
>  {
>  	u32 freq = tea->freq;
> @@ -203,6 +224,8 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>  	if (f->tuner != 0)
>  		return -EINVAL;
>  	f->type = V4L2_TUNER_RADIO;
> +	if (!tea->cannot_read_data)
> +		snd_tea575x_get_freq(tea);
>  	f->frequency = tea->freq;
>  	return 0;
>  }
> @@ -225,36 +248,50 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
>  					struct v4l2_hw_freq_seek *a)
>  {
>  	struct snd_tea575x *tea = video_drvdata(file);
> +	int i, old_freq;
> +	unsigned long timeout;
>  
>  	if (tea->cannot_read_data)
>  		return -ENOTTY;
> +
> +	snd_tea575x_get_freq(tea);
> +	old_freq = tea->freq;
> +	/* clear the frequency, HW will fill it in */
> +	tea->val &= ~TEA575X_BIT_FREQ_MASK;
>  	tea->val |= TEA575X_BIT_SEARCH;
> -	tea->val &= ~TEA575X_BIT_UPDOWN;
>  	if (a->seek_upward)
>  		tea->val |= TEA575X_BIT_UPDOWN;
> +	else
> +		tea->val &= ~TEA575X_BIT_UPDOWN;
>  	snd_tea575x_write(tea, tea->val);
> +	timeout = jiffies + msecs_to_jiffies(10000);
>  	for (;;) {
> -		unsigned val = snd_tea575x_read(tea);
> -
> -		if (!(val & TEA575X_BIT_SEARCH)) {
> -			/* Found a frequency */
> -			val &= TEA575X_BIT_FREQ_MASK;
> -			val = (val * 10) / 125;
> -			if (tea->tea5759)
> -				val += TEA575X_FMIF;
> -			else
> -				val -= TEA575X_FMIF;
> -			tea->freq = clamp(val * 16, FREQ_LO, FREQ_HI);
> -			return 0;
> -		}
> +		if (time_after(jiffies, timeout))
> +			break;
>  		if (schedule_timeout_interruptible(msecs_to_jiffies(10))) {
>  			/* some signal arrived, stop search */
>  			tea->val &= ~TEA575X_BIT_SEARCH;
>  			snd_tea575x_write(tea, tea->val);
>  			return -ERESTARTSYS;
>  		}
> +		if (!(snd_tea575x_read(tea) & TEA575X_BIT_SEARCH)) {
> +			/* 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 */
> +					break;
> +			}
> +			/* if we moved by less than 50 kHz, continue seeking */
> +			if (abs(old_freq - tea->freq) < 16 * 500) {
> +				snd_tea575x_write(tea, tea->val);
> +				continue;
> +			}
> +			tea->val &= ~TEA575X_BIT_SEARCH;
> +			return 0;
> +		}
>  	}
> -	return 0;
> +	return -ETIMEDOUT;
>  }
>  
>  static int tea575x_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -318,7 +355,7 @@ int snd_tea575x_init(struct snd_tea575x *tea)
>  			return -ENODEV;
>  	}
>  
> -	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
> +	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_5_28;
>  	tea->freq = 90500 * 16;		/* 90.5Mhz default */
>  	snd_tea575x_set_freq(tea);
>  
> 
> 
> 
> 

tea575x-tuner improvements & use in maxiradio

[Hans Verkuil]

These patches improve the tea575x-tuner module to make it up to date with
the latest V4L2 frameworks.

The maxiradio driver has also been converted to use the tea575x-tuner and
I've used that card to test it.

Unfortunately, this card can't read the data pin, so the new hardware seek
functionality has been tested only partially (yes, it seeks, but when it finds
a channel I can't read back the frequency).

Ondrej, are you able to test these patches for the sound cards that use this
tea575x tuner?

Note that these two patches rely on other work that I did and that hasn't been
merged yet. So it is best to pull from my git tree:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radio-pci2

You can use the v4l-utils repository (http://git.linuxtv.org/v4l-utils.git) to
test the drivers: the v4l2-compliance test should succeed and with v4l2-ctl you
can test the hardware seek:

To seek down:

v4l2-ctl -d /dev/radio0 --freq-seek=dir=0

To seek up:

v4l2-ctl -d /dev/radio0 --freq-seek=dir=1

To do the compliance test:

v4l2-compliance -r /dev/radio0

Regards,

	Hans

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> These patches improve the tea575x-tuner module to make it up to date with
> the latest V4L2 frameworks.
>
> The maxiradio driver has also been converted to use the tea575x-tuner and
> I've used that card to test it.
>
> Unfortunately, this card can't read the data pin, so the new hardware seek
> functionality has been tested only partially (yes, it seeks, but when it
> finds a channel I can't read back the frequency).
>
> Ondrej, are you able to test these patches for the sound cards that use
> this tea575x tuner?
>
> Note that these two patches rely on other work that I did and that hasn't
> been merged yet. So it is best to pull from my git tree:
>
> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radio-pc
>i2
>
> You can use the v4l-utils repository (http://git.linuxtv.org/v4l-utils.git)
> to test the drivers: the v4l2-compliance test should succeed and with
> v4l2-ctl you can test the hardware seek:
>
> To seek down:
>
> v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
>
> To seek up:
>
> v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
>
> To do the compliance test:
>
> v4l2-compliance -r /dev/radio0

It seems to work (tested with SF64-PCR - snd_fm801) but the seek is severely 
broken. Reading the frequency immediately after seek does not work, it always 
returns the old value (haven't found a delay that works). Reading it later 
(copied back snd_tea575x_get_freq function) works. The chip seeks randomly up 
or down, ignoring UP/DOWN flag and often stops at wrong place (only noise) or 
even outside the FM range.

So I strongly suggest not to enable this (mis-)feature. The HW seems to be 
completely broken (unless there's some weird bug in the code).


-- 
Ondrej Zary

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Hans Verkuil]

On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > These patches improve the tea575x-tuner module to make it up to date with
> > the latest V4L2 frameworks.
> >
> > The maxiradio driver has also been converted to use the tea575x-tuner and
> > I've used that card to test it.
> >
> > Unfortunately, this card can't read the data pin, so the new hardware seek
> > functionality has been tested only partially (yes, it seeks, but when it
> > finds a channel I can't read back the frequency).
> >
> > Ondrej, are you able to test these patches for the sound cards that use
> > this tea575x tuner?
> >
> > Note that these two patches rely on other work that I did and that hasn't
> > been merged yet. So it is best to pull from my git tree:
> >
> > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radio-pc
> >i2
> >
> > You can use the v4l-utils repository (http://git.linuxtv.org/v4l-utils.git)
> > to test the drivers: the v4l2-compliance test should succeed and with
> > v4l2-ctl you can test the hardware seek:
> >
> > To seek down:
> >
> > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> >
> > To seek up:
> >
> > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> >
> > To do the compliance test:
> >
> > v4l2-compliance -r /dev/radio0
> 
> It seems to work (tested with SF64-PCR - snd_fm801) but the seek is severely 
> broken. Reading the frequency immediately after seek does not work, it always 
> returns the old value (haven't found a delay that works). Reading it later 
> (copied back snd_tea575x_get_freq function) works. The chip seeks randomly up 
> or down, ignoring UP/DOWN flag and often stops at wrong place (only noise) or 
> even outside the FM range.
> 
> So I strongly suggest not to enable this (mis-)feature. The HW seems to be 
> completely broken (unless there's some weird bug in the code).

Well, it seemed like a good idea at the time :-) I'll remove this 'feature',
it's really not worth our time to try and make this work for these old cards.

I wonder if you are able to test the ISA radio-sf16fmr2.c driver? I'm not
sure if you have the hardware, but since I changed this driver to use the
proper isa kernel framework I'd like to have this tested if possible.

Regards,

	Hans

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Wednesday 08 February 2012, you wrote:
> On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > These patches improve the tea575x-tuner module to make it up to date
> > > with the latest V4L2 frameworks.
> > >
> > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > and I've used that card to test it.
> > >
> > > Unfortunately, this card can't read the data pin, so the new hardware
> > > seek functionality has been tested only partially (yes, it seeks, but
> > > when it finds a channel I can't read back the frequency).
> > >
> > > Ondrej, are you able to test these patches for the sound cards that use
> > > this tea575x tuner?
> > >
> > > Note that these two patches rely on other work that I did and that
> > > hasn't been merged yet. So it is best to pull from my git tree:
> > >
> > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > >o-pc i2
> > >
> > > You can use the v4l-utils repository
> > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > hardware seek:
> > >
> > > To seek down:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > >
> > > To seek up:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > >
> > > To do the compliance test:
> > >
> > > v4l2-compliance -r /dev/radio0
> >
> > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > severely broken. Reading the frequency immediately after seek does not
> > work, it always returns the old value (haven't found a delay that works).
> > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > wrong place (only noise) or even outside the FM range.
> >
> > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > be completely broken (unless there's some weird bug in the code).
>
> Well, it seemed like a good idea at the time :-) I'll remove this
> 'feature', it's really not worth our time to try and make this work for
> these old cards.

I'll test it with other cards, maybe it will work at least a bit. And also 
with original (Windows) software - it has some seek functionality, IIRC.

> I wonder if you are able to test the ISA radio-sf16fmr2.c driver? I'm not
> sure if you have the hardware, but since I changed this driver to use the
> proper isa kernel framework I'd like to have this tested if possible.

Yes, I have the card so I'll test it. All I can say right now is that the 
driver did not build yesterday (missing #include <linux/slab.h>).


-- 
Ondrej Zary

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > These patches improve the tea575x-tuner module to make it up to date
> > > with the latest V4L2 frameworks.
> > >
> > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > and I've used that card to test it.
> > >
> > > Unfortunately, this card can't read the data pin, so the new hardware
> > > seek functionality has been tested only partially (yes, it seeks, but
> > > when it finds a channel I can't read back the frequency).
> > >
> > > Ondrej, are you able to test these patches for the sound cards that use
> > > this tea575x tuner?
> > >
> > > Note that these two patches rely on other work that I did and that
> > > hasn't been merged yet. So it is best to pull from my git tree:
> > >
> > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > >o-pc i2
> > >
> > > You can use the v4l-utils repository
> > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > hardware seek:
> > >
> > > To seek down:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > >
> > > To seek up:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > >
> > > To do the compliance test:
> > >
> > > v4l2-compliance -r /dev/radio0
> >
> > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > severely broken. Reading the frequency immediately after seek does not
> > work, it always returns the old value (haven't found a delay that works).
> > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > wrong place (only noise) or even outside the FM range.
> >
> > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > be completely broken (unless there's some weird bug in the code).
>
> Well, it seemed like a good idea at the time :-) I'll remove this
> 'feature', it's really not worth our time to try and make this work for
> these old cards.
>
> I wonder if you are able to test the ISA radio-sf16fmr2.c driver? I'm not
> sure if you have the hardware, but since I changed this driver to use the
> proper isa kernel framework I'd like to have this tested if possible.

The driver works, provided that linux/slab.h is included. It oopses on rmmod
because dev_set_drvdata() call is missing from init.
This patch fixes all of that and also removes (now useless) static fmr2_card:

--- a/drivers/media/radio/radio-sf16fmr2.c
+++ b/drivers/media/radio/radio-sf16fmr2.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/module.h>	/* Modules 			*/
 #include <linux/init.h>		/* Initdata			*/
+#include <linux/slab.h>
 #include <linux/ioport.h>	/* request_region		*/
 #include <linux/io.h>		/* outb, outb_p			*/
 #include <linux/isa.h>
@@ -32,7 +33,6 @@ struct fmr2 {
 
 /* the port is hardwired so no need to support multiple cards */
 #define FMR2_PORT	0x384
-static struct fmr2 fmr2_card;
 
 /* TEA575x tuner pins */
 #define STR_DATA	(1 << 0)
@@ -188,7 +188,7 @@ static int fmr2_tea_ext_init(struct snd_tea575x *tea)
 
 static int __init fmr2_probe(struct device *pdev, unsigned int dev)
 {
-	struct fmr2 *fmr2 = &fmr2_card;
+	struct fmr2 *fmr2;
 	int err;
 
 	fmr2 = kzalloc(sizeof(*fmr2), GFP_KERNEL);
@@ -229,6 +229,7 @@ static int __init fmr2_probe(struct device *pdev, unsigned int dev)
 	}
 
 	printk(KERN_INFO "radio-sf16fmr2: SF16-FMR2 radio card at 0x%x.\n", fmr2->io);
+	dev_set_drvdata(pdev, fmr2);
 	return 0;
 }
 



-- 
Ondrej Zary

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Hans Verkuil]

On Wednesday, February 08, 2012 20:57:43 Ondrej Zary wrote:
> On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> > On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > > These patches improve the tea575x-tuner module to make it up to date
> > > > with the latest V4L2 frameworks.
> > > >
> > > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > > and I've used that card to test it.
> > > >
> > > > Unfortunately, this card can't read the data pin, so the new hardware
> > > > seek functionality has been tested only partially (yes, it seeks, but
> > > > when it finds a channel I can't read back the frequency).
> > > >
> > > > Ondrej, are you able to test these patches for the sound cards that use
> > > > this tea575x tuner?
> > > >
> > > > Note that these two patches rely on other work that I did and that
> > > > hasn't been merged yet. So it is best to pull from my git tree:
> > > >
> > > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > > >o-pc i2
> > > >
> > > > You can use the v4l-utils repository
> > > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > > hardware seek:
> > > >
> > > > To seek down:
> > > >
> > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > > >
> > > > To seek up:
> > > >
> > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > > >
> > > > To do the compliance test:
> > > >
> > > > v4l2-compliance -r /dev/radio0
> > >
> > > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > > severely broken. Reading the frequency immediately after seek does not
> > > work, it always returns the old value (haven't found a delay that works).
> > > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > > wrong place (only noise) or even outside the FM range.
> > >
> > > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > > be completely broken (unless there's some weird bug in the code).
> >
> > Well, it seemed like a good idea at the time :-) I'll remove this
> > 'feature', it's really not worth our time to try and make this work for
> > these old cards.
> >
> > I wonder if you are able to test the ISA radio-sf16fmr2.c driver? I'm not
> > sure if you have the hardware, but since I changed this driver to use the
> > proper isa kernel framework I'd like to have this tested if possible.
> 
> The driver works, provided that linux/slab.h is included. It oopses on rmmod
> because dev_set_drvdata() call is missing from init.
> This patch fixes all of that and also removes (now useless) static fmr2_card:

Thanks! I'll apply the changes for the final pull request. That's probably
going to be in a week or two.

BTW, do you perhaps have a mirosound pcm20 card (or know someone who does)?
I'd like to update the radio-miropcm20 driver as well and it would be nice if
my changes can be tested.

Regards,

	Hans

> 
> --- a/drivers/media/radio/radio-sf16fmr2.c
> +++ b/drivers/media/radio/radio-sf16fmr2.c
> @@ -9,6 +9,7 @@
>  #include <linux/delay.h>
>  #include <linux/module.h>	/* Modules 			*/
>  #include <linux/init.h>		/* Initdata			*/
> +#include <linux/slab.h>
>  #include <linux/ioport.h>	/* request_region		*/
>  #include <linux/io.h>		/* outb, outb_p			*/
>  #include <linux/isa.h>
> @@ -32,7 +33,6 @@ struct fmr2 {
>  
>  /* the port is hardwired so no need to support multiple cards */
>  #define FMR2_PORT	0x384
> -static struct fmr2 fmr2_card;
>  
>  /* TEA575x tuner pins */
>  #define STR_DATA	(1 << 0)
> @@ -188,7 +188,7 @@ static int fmr2_tea_ext_init(struct snd_tea575x *tea)
>  
>  static int __init fmr2_probe(struct device *pdev, unsigned int dev)
>  {
> -	struct fmr2 *fmr2 = &fmr2_card;
> +	struct fmr2 *fmr2;
>  	int err;
>  
>  	fmr2 = kzalloc(sizeof(*fmr2), GFP_KERNEL);
> @@ -229,6 +229,7 @@ static int __init fmr2_probe(struct device *pdev, unsigned int dev)
>  	}
>  
>  	printk(KERN_INFO "radio-sf16fmr2: SF16-FMR2 radio card at 0x%x.\n", fmr2->io);
> +	dev_set_drvdata(pdev, fmr2);
>  	return 0;
>  }
>  
> 
> 
> 
> 

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Wednesday 08 February 2012 21:30:07 Hans Verkuil wrote:
> On Wednesday, February 08, 2012 20:57:43 Ondrej Zary wrote:
> > On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> > > On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > > > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > > > These patches improve the tea575x-tuner module to make it up to
> > > > > date with the latest V4L2 frameworks.
> > > > >
> > > > > The maxiradio driver has also been converted to use the
> > > > > tea575x-tuner and I've used that card to test it.
> > > > >
> > > > > Unfortunately, this card can't read the data pin, so the new
> > > > > hardware seek functionality has been tested only partially (yes, it
> > > > > seeks, but when it finds a channel I can't read back the
> > > > > frequency).
> > > > >
> > > > > Ondrej, are you able to test these patches for the sound cards that
> > > > > use this tea575x tuner?
> > > > >
> > > > > Note that these two patches rely on other work that I did and that
> > > > > hasn't been merged yet. So it is best to pull from my git tree:
> > > > >
> > > > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/
> > > > >radi o-pc i2
> > > > >
> > > > > You can use the v4l-utils repository
> > > > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > > > v4l2-compliance test should succeed and with v4l2-ctl you can test
> > > > > the hardware seek:
> > > > >
> > > > > To seek down:
> > > > >
> > > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > > > >
> > > > > To seek up:
> > > > >
> > > > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > > > >
> > > > > To do the compliance test:
> > > > >
> > > > > v4l2-compliance -r /dev/radio0
> > > >
> > > > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > > > severely broken. Reading the frequency immediately after seek does
> > > > not work, it always returns the old value (haven't found a delay that
> > > > works). Reading it later (copied back snd_tea575x_get_freq function)
> > > > works. The chip seeks randomly up or down, ignoring UP/DOWN flag and
> > > > often stops at wrong place (only noise) or even outside the FM range.
> > > >
> > > > So I strongly suggest not to enable this (mis-)feature. The HW seems
> > > > to be completely broken (unless there's some weird bug in the code).
> > >
> > > Well, it seemed like a good idea at the time :-) I'll remove this
> > > 'feature', it's really not worth our time to try and make this work for
> > > these old cards.
> > >
> > > I wonder if you are able to test the ISA radio-sf16fmr2.c driver? I'm
> > > not sure if you have the hardware, but since I changed this driver to
> > > use the proper isa kernel framework I'd like to have this tested if
> > > possible.
> >
> > The driver works, provided that linux/slab.h is included. It oopses on
> > rmmod because dev_set_drvdata() call is missing from init.
> > This patch fixes all of that and also removes (now useless) static
> > fmr2_card:
>
> Thanks! I'll apply the changes for the final pull request. That's probably
> going to be in a week or two.
>
> BTW, do you perhaps have a mirosound pcm20 card (or know someone who does)?
> I'd like to update the radio-miropcm20 driver as well and it would be nice
> if my changes can be tested.

Unfortunately not. I only have SF16-FMI, SF16-FMP, SF16-FMR2, SF64-PCR, 
SF64-PCE2, SF256-PCP. Another card should be on the way, looks like AOpen 
FX-3D Pro Radio on photo.

-- 
Ondrej Zary

Re: [alsa-devel] tea575x-tuner improvements & use in maxiradio

[Ondrej Zary]

On Wednesday 08 February 2012 08:29:25 Hans Verkuil wrote:
> On Tuesday, February 07, 2012 23:20:19 Ondrej Zary wrote:
> > On Sunday 05 February 2012 14:17:05 Hans Verkuil wrote:
> > > These patches improve the tea575x-tuner module to make it up to date
> > > with the latest V4L2 frameworks.
> > >
> > > The maxiradio driver has also been converted to use the tea575x-tuner
> > > and I've used that card to test it.
> > >
> > > Unfortunately, this card can't read the data pin, so the new hardware
> > > seek functionality has been tested only partially (yes, it seeks, but
> > > when it finds a channel I can't read back the frequency).
> > >
> > > Ondrej, are you able to test these patches for the sound cards that use
> > > this tea575x tuner?
> > >
> > > Note that these two patches rely on other work that I did and that
> > > hasn't been merged yet. So it is best to pull from my git tree:
> > >
> > > http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/radi
> > >o-pc i2
> > >
> > > You can use the v4l-utils repository
> > > (http://git.linuxtv.org/v4l-utils.git) to test the drivers: the
> > > v4l2-compliance test should succeed and with v4l2-ctl you can test the
> > > hardware seek:
> > >
> > > To seek down:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=0
> > >
> > > To seek up:
> > >
> > > v4l2-ctl -d /dev/radio0 --freq-seek=dir=1
> > >
> > > To do the compliance test:
> > >
> > > v4l2-compliance -r /dev/radio0
> >
> > It seems to work (tested with SF64-PCR - snd_fm801) but the seek is
> > severely broken. Reading the frequency immediately after seek does not
> > work, it always returns the old value (haven't found a delay that works).
> > Reading it later (copied back snd_tea575x_get_freq function) works. The
> > chip seeks randomly up or down, ignoring UP/DOWN flag and often stops at
> > wrong place (only noise) or even outside the FM range.
> >
> > So I strongly suggest not to enable this (mis-)feature. The HW seems to
> > be completely broken (unless there's some weird bug in the code).
>
> Well, it seemed like a good idea at the time :-) I'll remove this
> 'feature', it's really not worth our time to try and make this work for
> these old cards.

I fixed it somehow. Now it works most of the time.
The important things:
 - 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

--- 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;
+		return;
+	}
+
+	/* freq *= 12.5 */
+	freq *= 125;
+	freq /= 10;
+	/* crystal fixup */
+	if (tea->tea5759)
+		freq += TEA575X_FMIF;
+	else
+		freq -= TEA575X_FMIF;
+
+	tea->freq = clamp(freq * 16, FREQ_LO, FREQ_HI); /* from kHz */
+}
+
 static void snd_tea575x_set_freq(struct snd_tea575x *tea)
 {
 	u32 freq = tea->freq;
@@ -203,6 +224,8 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 	if (f->tuner != 0)
 		return -EINVAL;
 	f->type = V4L2_TUNER_RADIO;
+	if (!tea->cannot_read_data)
+		snd_tea575x_get_freq(tea);
 	f->frequency = tea->freq;
 	return 0;
 }
@@ -225,36 +248,50 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
 					struct v4l2_hw_freq_seek *a)
 {
 	struct snd_tea575x *tea = video_drvdata(file);
+	int i, old_freq;
+	unsigned long timeout;
 
 	if (tea->cannot_read_data)
 		return -ENOTTY;
+
+	snd_tea575x_get_freq(tea);
+	old_freq = tea->freq;
+	/* clear the frequency, HW will fill it in */
+	tea->val &= ~TEA575X_BIT_FREQ_MASK;
 	tea->val |= TEA575X_BIT_SEARCH;
-	tea->val &= ~TEA575X_BIT_UPDOWN;
 	if (a->seek_upward)
 		tea->val |= TEA575X_BIT_UPDOWN;
+	else
+		tea->val &= ~TEA575X_BIT_UPDOWN;
 	snd_tea575x_write(tea, tea->val);
+	timeout = jiffies + msecs_to_jiffies(10000);
 	for (;;) {
-		unsigned val = snd_tea575x_read(tea);
-
-		if (!(val & TEA575X_BIT_SEARCH)) {
-			/* Found a frequency */
-			val &= TEA575X_BIT_FREQ_MASK;
-			val = (val * 10) / 125;
-			if (tea->tea5759)
-				val += TEA575X_FMIF;
-			else
-				val -= TEA575X_FMIF;
-			tea->freq = clamp(val * 16, FREQ_LO, FREQ_HI);
-			return 0;
-		}
+		if (time_after(jiffies, timeout))
+			break;
 		if (schedule_timeout_interruptible(msecs_to_jiffies(10))) {
 			/* some signal arrived, stop search */
 			tea->val &= ~TEA575X_BIT_SEARCH;
 			snd_tea575x_write(tea, tea->val);
 			return -ERESTARTSYS;
 		}
+		if (!(snd_tea575x_read(tea) & TEA575X_BIT_SEARCH)) {
+			/* 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 */
+					break;
+			}
+			/* if we moved by less than 50 kHz, continue seeking */
+			if (abs(old_freq - tea->freq) < 16 * 500) {
+				snd_tea575x_write(tea, tea->val);
+				continue;
+			}
+			tea->val &= ~TEA575X_BIT_SEARCH;
+			return 0;
+		}
 	}
-	return 0;
+	return -ETIMEDOUT;
 }
 
 static int tea575x_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -318,7 +355,7 @@ int snd_tea575x_init(struct snd_tea575x *tea)
 			return -ENODEV;
 	}
 
-	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
+	tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_5_28;
 	tea->freq = 90500 * 16;		/* 90.5Mhz default */
 	snd_tea575x_set_freq(tea);
 



-- 
Ondrej Zary