RFC: Add support for limiting hw freq seeks to a certain band

RFC: Add support for limiting hw freq seeks to a certain band

[Hans de Goede]

This patchset, which builds on top of hverkuil's bands2 branch, which
adds the VIDIOC_ENUM_FREQ_BANDS API, add support for limiting
hw freq seeks to one of the bands from VIDIOC_ENUM_FREQ_BANDS, or a subset
there of.

The first patch introduces the new API and documents its, the other
patches are patches to the si470x radio driver, implementing the new API,
and removing the band module parameter as this is no longer needed
with this new API.

A git tree with all hverkuils patches included is here:
http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6-wip

A git tree of xawtv3 with its radio app modified to support the new
API is here:
http://git.linuxtv.org/hgoede/xawtv3.git/shortlog/refs/heads/band-support

Regards,

Hans

[PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans de Goede]

To allow apps to limit a hw-freq-seek to a specific band, for further
info see the documentation this patch adds for these new fields.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
 include/linux/videodev2.h                          |    5 ++-
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
index f4db44d..50dc9f8 100644
--- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
@@ -52,11 +52,21 @@
     <para>Start a hardware frequency seek from the current frequency.
 To do this applications initialize the <structfield>tuner</structfield>,
 <structfield>type</structfield>, <structfield>seek_upward</structfield>,
-<structfield>spacing</structfield> and
-<structfield>wrap_around</structfield> fields, and zero out the
-<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
-call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
-to this structure.</para>
+<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
+<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
+fields, and zero out the <structfield>reserved</structfield> array of a
+&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
+ioctl with a pointer to this structure.</para>
+
+    <para>The <structfield>rangelow</structfield> and
+<structfield>rangehigh</structfield> fields can be set to a non-zero value to
+tell the driver to search a specific band. If the &v4l2-tuner;
+<structfield>capability</structfield> field has the
+<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
+must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
+the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
+then these values must exactly match those of one of the bands returned by
+&VIDIOC-ENUM-FREQ-BANDS;.</para>
 
     <para>If an error is returned, then the original frequency will
     be restored.</para>
@@ -102,7 +112,23 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[7]</entry>
+	    <entry><structfield>rangelow</structfield></entry>
+	    <entry>If non-zero, the lowest tunable frequency of the band to
+search in units of 62.5 kHz, or if the &v4l2-tuner;
+<structfield>capability</structfield> field has the
+<constant>V4L2_TUNER_CAP_LOW</constant> flag set, in units of 62.5 Hz.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>rangehigh</structfield></entry>
+	    <entry>If non-zero, the highest tunable frequency of the band to
+search in units of 62.5 kHz, or if the &v4l2-tuner;
+<structfield>capability</structfield> field has the
+<constant>V4L2_TUNER_CAP_LOW</constant> flag set, in units of 62.5 Hz.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[5]</entry>
 	    <entry>Reserved for future extensions. Applications
 	    must set the array to zero.</entry>
 	  </row>
@@ -119,8 +145,10 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
 	<term><errorcode>EINVAL</errorcode></term>
 	<listitem>
 	  <para>The <structfield>tuner</structfield> index is out of
-bounds, the wrap_around value is not supported or the value in the <structfield>type</structfield> field is
-wrong.</para>
+bounds, the <structfield>wrap_around</structfield> value is not supported or
+one of the values in the <structfield>type</structfield>,
+<structfield>rangelow</structfield> or <structfield>rangehigh</structfield>
+fields is wrong.</para>
 	</listitem>
       </varlistentry>
       <varlistentry>
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 9fa822a..1c6aa63 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2029,6 +2029,7 @@ struct v4l2_modulator {
 #define V4L2_TUNER_CAP_RDS_BLOCK_IO	0x0100
 #define V4L2_TUNER_CAP_RDS_CONTROLS	0x0200
 #define V4L2_TUNER_CAP_FREQ_BANDS	0x0400
+#define V4L2_TUNER_CAP_HWSEEK_PROG_LIM	0x0800
 
 /*  Flags for the 'rxsubchans' field */
 #define V4L2_TUNER_SUB_MONO		0x0001
@@ -2074,7 +2075,9 @@ struct v4l2_hw_freq_seek {
 	__u32	seek_upward;
 	__u32	wrap_around;
 	__u32	spacing;
-	__u32	reserved[7];
+	__u32	rangelow;
+	__u32	rangehigh;
+	__u32	reserved[5];
 };
 
 /*
-- 
1.7.10.4

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans Verkuil]

Hi Hans,

Thanks for the patch.

I've CC-ed Halli as well.

On Wed July 11 2012 17:47:34 Hans de Goede wrote:
> To allow apps to limit a hw-freq-seek to a specific band, for further
> info see the documentation this patch adds for these new fields.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
>  include/linux/videodev2.h                          |    5 ++-
>  2 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> index f4db44d..50dc9f8 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> @@ -52,11 +52,21 @@
>      <para>Start a hardware frequency seek from the current frequency.
>  To do this applications initialize the <structfield>tuner</structfield>,
>  <structfield>type</structfield>, <structfield>seek_upward</structfield>,
> -<structfield>spacing</structfield> and
> -<structfield>wrap_around</structfield> fields, and zero out the
> -<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
> -call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
> -to this structure.</para>
> +<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
> +<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
> +fields, and zero out the <structfield>reserved</structfield> array of a
> +&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
> +ioctl with a pointer to this structure.</para>
> +
> +    <para>The <structfield>rangelow</structfield> and
> +<structfield>rangehigh</structfield> fields can be set to a non-zero value to
> +tell the driver to search a specific band. If the &v4l2-tuner;
> +<structfield>capability</structfield> field has the
> +<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
> +must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
> +the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
> +then these values must exactly match those of one of the bands returned by
> +&VIDIOC-ENUM-FREQ-BANDS;.</para>

OK, I have some questions here:

1) If you have a multiband tuner, what should happen if both low and high are
zero? Currently it is undefined, other than that the seek should start from
the current frequency until it reaches some limit.

Halli, what does your hardware do? In particular, is the hwseek limited by the
US/Europe or Japan band range or can it do the full range? If I'm not mistaken
it is the former, right?

If it is the former, then you need to explicitly set low + high to ensure that
the hwseek uses the correct range because the driver can't guess which of the
overlapping bands to use.

2) What happens if the current frequency is outside the low/high range? The
hwseek spec says that the seek starts from the current frequency, so that might
mean that hwseek returns -ERANGE in this case.

Regards,

	Hans

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans de Goede]

Hi Hans,

On 07/11/2012 08:01 PM, Hans Verkuil wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> I've CC-ed Halli as well.
>
> On Wed July 11 2012 17:47:34 Hans de Goede wrote:
>> To allow apps to limit a hw-freq-seek to a specific band, for further
>> info see the documentation this patch adds for these new fields.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
>>   include/linux/videodev2.h                          |    5 ++-
>>   2 files changed, 40 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> index f4db44d..50dc9f8 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> @@ -52,11 +52,21 @@
>>       <para>Start a hardware frequency seek from the current frequency.
>>   To do this applications initialize the <structfield>tuner</structfield>,
>>   <structfield>type</structfield>, <structfield>seek_upward</structfield>,
>> -<structfield>spacing</structfield> and
>> -<structfield>wrap_around</structfield> fields, and zero out the
>> -<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
>> -call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
>> -to this structure.</para>
>> +<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
>> +<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
>> +fields, and zero out the <structfield>reserved</structfield> array of a
>> +&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
>> +ioctl with a pointer to this structure.</para>
>> +
>> +    <para>The <structfield>rangelow</structfield> and
>> +<structfield>rangehigh</structfield> fields can be set to a non-zero value to
>> +tell the driver to search a specific band. If the &v4l2-tuner;
>> +<structfield>capability</structfield> field has the
>> +<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
>> +must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
>> +the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
>> +then these values must exactly match those of one of the bands returned by
>> +&VIDIOC-ENUM-FREQ-BANDS;.</para>
>
> OK, I have some questions here:
>
> 1) If you have a multiband tuner, what should happen if both low and high are
> zero? Currently it is undefined, other than that the seek should start from
> the current frequency until it reaches some limit.

That would be driver specific, we could add the same "If rangelow/high is zero
a reasonable default value is used." language as used for the spacing. For
example for the si470x if both are zero I simply switch to the "Japan wide"
band which covers all frequencies handled by the other bands, but if there
is no such covers all ranges band, then the logical thing todo would just keep
the band as is (so as determined by the last s_freq).

> Halli, what does your hardware do? In particular, is the hwseek limited by the
> US/Europe or Japan band range or can it do the full range? If I'm not mistaken
> it is the former, right?
>
> If it is the former, then you need to explicitly set low + high to ensure that
> the hwseek uses the correct range because the driver can't guess which of the
> overlapping bands to use.
>
> 2) What happens if the current frequency is outside the low/high range? The
> hwseek spec says that the seek starts from the current frequency, so that might
> mean that hwseek returns -ERANGE in this case.

What the si470x code currently does is just clamp the frequency to the new
range before seeking, but -ERANGE works for me too.

Regards,

Hans

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans Verkuil]

On Wed 11 July 2012 20:37:14 Hans de Goede wrote:
> Hi Hans,
> 
> On 07/11/2012 08:01 PM, Hans Verkuil wrote:
> > Hi Hans,
> >
> > Thanks for the patch.
> >
> > I've CC-ed Halli as well.
> >
> > On Wed July 11 2012 17:47:34 Hans de Goede wrote:
> >> To allow apps to limit a hw-freq-seek to a specific band, for further
> >> info see the documentation this patch adds for these new fields.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
> >>   include/linux/videodev2.h                          |    5 ++-
> >>   2 files changed, 40 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> >> index f4db44d..50dc9f8 100644
> >> --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
> >> @@ -52,11 +52,21 @@
> >>       <para>Start a hardware frequency seek from the current frequency.
> >>   To do this applications initialize the <structfield>tuner</structfield>,
> >>   <structfield>type</structfield>, <structfield>seek_upward</structfield>,
> >> -<structfield>spacing</structfield> and
> >> -<structfield>wrap_around</structfield> fields, and zero out the
> >> -<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
> >> -call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
> >> -to this structure.</para>
> >> +<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
> >> +<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
> >> +fields, and zero out the <structfield>reserved</structfield> array of a
> >> +&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
> >> +ioctl with a pointer to this structure.</para>
> >> +
> >> +    <para>The <structfield>rangelow</structfield> and
> >> +<structfield>rangehigh</structfield> fields can be set to a non-zero value to
> >> +tell the driver to search a specific band. If the &v4l2-tuner;
> >> +<structfield>capability</structfield> field has the
> >> +<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
> >> +must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
> >> +the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
> >> +then these values must exactly match those of one of the bands returned by
> >> +&VIDIOC-ENUM-FREQ-BANDS;.</para>
> >
> > OK, I have some questions here:
> >
> > 1) If you have a multiband tuner, what should happen if both low and high are
> > zero? Currently it is undefined, other than that the seek should start from
> > the current frequency until it reaches some limit.
> 
> That would be driver specific, we could add the same "If rangelow/high is zero
> a reasonable default value is used." language as used for the spacing. For
> example for the si470x if both are zero I simply switch to the "Japan wide"
> band which covers all frequencies handled by the other bands, but if there
> is no such covers all ranges band, then the logical thing todo would just keep
> the band as is (so as determined by the last s_freq).
> 
> > Halli, what does your hardware do? In particular, is the hwseek limited by the
> > US/Europe or Japan band range or can it do the full range? If I'm not mistaken
> > it is the former, right?
> >
> > If it is the former, then you need to explicitly set low + high to ensure that
> > the hwseek uses the correct range because the driver can't guess which of the
> > overlapping bands to use.
> >
> > 2) What happens if the current frequency is outside the low/high range? The
> > hwseek spec says that the seek starts from the current frequency, so that might
> > mean that hwseek returns -ERANGE in this case.
> 
> What the si470x code currently does is just clamp the frequency to the new
> range before seeking, but -ERANGE works for me too.

Clamping is a better idea IMHO as long as it is documented.

Regards,

	Hans

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans de Goede]

Hi,

On 07/12/2012 11:27 AM, Hans Verkuil wrote:
> On Wed 11 July 2012 20:37:14 Hans de Goede wrote:

<snip>

>>> 2) What happens if the current frequency is outside the low/high range? The
>>> hwseek spec says that the seek starts from the current frequency, so that might
>>> mean that hwseek returns -ERANGE in this case.
>>
>> What the si470x code currently does is just clamp the frequency to the new
>> range before seeking, but -ERANGE works for me too.
>
> Clamping is a better idea IMHO as long as it is documented.

Ok, I've respun this patch to improve the documentation in various parts, I'm
resending the entire set right after this email.

Regards,

Hans

p.s.

Tomorrow morning I'm leaving for a week of vacation, during which I won't be
reading my mail. If everybody agrees on the 2nd revision of this patchset
please add it to your bands2 branch, and if you agree that this seems to be
it wrt the API for tuner-bands, you could also consider sending a pull-req
for it to Mauro for 3.6 :)

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[halli manjunatha]

On Wed, Jul 11, 2012 at 1:01 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> I've CC-ed Halli as well.
>
> On Wed July 11 2012 17:47:34 Hans de Goede wrote:
>> To allow apps to limit a hw-freq-seek to a specific band, for further
>> info see the documentation this patch adds for these new fields.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
>>  include/linux/videodev2.h                          |    5 ++-
>>  2 files changed, 40 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> index f4db44d..50dc9f8 100644
>> --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>> @@ -52,11 +52,21 @@
>>      <para>Start a hardware frequency seek from the current frequency.
>>  To do this applications initialize the <structfield>tuner</structfield>,
>>  <structfield>type</structfield>, <structfield>seek_upward</structfield>,
>> -<structfield>spacing</structfield> and
>> -<structfield>wrap_around</structfield> fields, and zero out the
>> -<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
>> -call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
>> -to this structure.</para>
>> +<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
>> +<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
>> +fields, and zero out the <structfield>reserved</structfield> array of a
>> +&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
>> +ioctl with a pointer to this structure.</para>
>> +
>> +    <para>The <structfield>rangelow</structfield> and
>> +<structfield>rangehigh</structfield> fields can be set to a non-zero value to
>> +tell the driver to search a specific band. If the &v4l2-tuner;
>> +<structfield>capability</structfield> field has the
>> +<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
>> +must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
>> +the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
>> +then these values must exactly match those of one of the bands returned by
>> +&VIDIOC-ENUM-FREQ-BANDS;.</para>
>
> OK, I have some questions here:
>
> 1) If you have a multiband tuner, what should happen if both low and high are
> zero? Currently it is undefined, other than that the seek should start from
> the current frequency until it reaches some limit.
>
> Halli, what does your hardware do? In particular, is the hwseek limited by the
> US/Europe or Japan band range or can it do the full range? If I'm not mistaken
> it is the former, right?

You are right... my hardware seek is limited by the japan/US band range....

> If it is the former, then you need to explicitly set low + high to ensure that
> the hwseek uses the correct range because the driver can't guess which of the
> overlapping bands to use.

Yes in my driver I will take care of this :)....
>
> 2) What happens if the current frequency is outside the low/high range? The
> hwseek spec says that the seek starts from the current frequency, so that might
> mean that hwseek returns -ERANGE in this case.
>
> Regards,
>
>         Hans



-- 
Regards
Halli

Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct

[Hans de Goede]

Hi,

On 07/11/2012 08:37 PM, halli manjunatha wrote:
> On Wed, Jul 11, 2012 at 1:01 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> Hi Hans,
>>
>> Thanks for the patch.
>>
>> I've CC-ed Halli as well.
>>
>> On Wed July 11 2012 17:47:34 Hans de Goede wrote:
>>> To allow apps to limit a hw-freq-seek to a specific band, for further
>>> info see the documentation this patch adds for these new fields.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   44 ++++++++++++++++----
>>>   include/linux/videodev2.h                          |    5 ++-
>>>   2 files changed, 40 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>>> index f4db44d..50dc9f8 100644
>>> --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>>> +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
>>> @@ -52,11 +52,21 @@
>>>       <para>Start a hardware frequency seek from the current frequency.
>>>   To do this applications initialize the <structfield>tuner</structfield>,
>>>   <structfield>type</structfield>, <structfield>seek_upward</structfield>,
>>> -<structfield>spacing</structfield> and
>>> -<structfield>wrap_around</structfield> fields, and zero out the
>>> -<structfield>reserved</structfield> array of a &v4l2-hw-freq-seek; and
>>> -call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
>>> -to this structure.</para>
>>> +<structfield>wrap_around</structfield>, <structfield>spacing</structfield>,
>>> +<structfield>rangelow</structfield> and <structfield>rangehigh</structfield>
>>> +fields, and zero out the <structfield>reserved</structfield> array of a
>>> +&v4l2-hw-freq-seek; and call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant>
>>> +ioctl with a pointer to this structure.</para>
>>> +
>>> +    <para>The <structfield>rangelow</structfield> and
>>> +<structfield>rangehigh</structfield> fields can be set to a non-zero value to
>>> +tell the driver to search a specific band. If the &v4l2-tuner;
>>> +<structfield>capability</structfield> field has the
>>> +<constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag set, these values
>>> +must fall within one of the bands returned by &VIDIOC-ENUM-FREQ-BANDS;. If
>>> +the <constant>V4L2_TUNER_CAP_HWSEEK_PROG_LIM</constant> flag is not set,
>>> +then these values must exactly match those of one of the bands returned by
>>> +&VIDIOC-ENUM-FREQ-BANDS;.</para>
>>
>> OK, I have some questions here:
>>
>> 1) If you have a multiband tuner, what should happen if both low and high are
>> zero? Currently it is undefined, other than that the seek should start from
>> the current frequency until it reaches some limit.
>>
>> Halli, what does your hardware do? In particular, is the hwseek limited by the
>> US/Europe or Japan band range or can it do the full range? If I'm not mistaken
>> it is the former, right?
>
> You are right... my hardware seek is limited by the japan/US band range....
>
>> If it is the former, then you need to explicitly set low + high to ensure that
>> the hwseek uses the correct range because the driver can't guess which of the
>> overlapping bands to use.
>
> Yes in my driver I will take care of this :)....

I think you misunderstood Hans here, not the driver but userspace will need
to fill in the rangelow / rangehigh fields of struct v4l2_hw_freq_seek, because if
the current freq is in the overlapping area of the bands, the driver cannot know
which band to seek, so it will just have to guess, I think it is best to just leave
the band at its current setting in that case.

The way the new API works (which was done this way to preserve backward compat)
is that the bands returned from ENUM_BANDS are there as information only. userspace
never explicitly sets a band, so an old app will just see the entire 76-108 MHZ range
in the tuner struct and may do a S_FREQUENCY for any of those frequencies, and the
driver must automatically switch bands when necessary.

With S_HW_FREQ_SEEK we've the 2 new fields to indicate the band to seek for new apps,
but with old apps these fields will be 0, and the driver needs to just pick a band
to search on a best effort basis, for the si470x IE, if no band is specified
in struct v4l2_hw_freq_seek,  I simply always switch to the "Japan wide" band
of 76-108 Mhz as that includes all other bands supported by the si470x.

Regards,

Hans

[PATCH 2/5] radio-si470x: restore ctrl settings after suspend/resume

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/si470x/radio-si470x-usb.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index 40b963c..0204cf4 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -792,11 +792,16 @@ static int si470x_usb_driver_suspend(struct usb_interface *intf,
 static int si470x_usb_driver_resume(struct usb_interface *intf)
 {
 	struct si470x_device *radio = usb_get_intfdata(intf);
+	int ret;
 
 	dev_info(&intf->dev, "resuming now...\n");
 
 	/* start radio */
-	return si470x_start_usb(radio);
+	ret = si470x_start_usb(radio);
+	if (ret == 0)
+		v4l2_ctrl_handler_setup(&radio->hdl);
+
+	return ret;
 }
 
 
-- 
1.7.10.4

[PATCH 3/5] radio-si470x: Fix band selection

[Hans de Goede]

The mask was wrong resulting in band 0 and 1 always ending up as band 0
in the register.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/si470x/radio-si470x.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h
index b3b612f..11f14b6 100644
--- a/drivers/media/radio/si470x/radio-si470x.h
+++ b/drivers/media/radio/si470x/radio-si470x.h
@@ -87,7 +87,7 @@
 
 #define SYSCONFIG2		5	/* System Configuration 2 */
 #define SYSCONFIG2_SEEKTH	0xff00	/* bits 15..08: RSSI Seek Threshold */
-#define SYSCONFIG2_BAND		0x0080	/* bits 07..06: Band Select */
+#define SYSCONFIG2_BAND		0x00c0	/* bits 07..06: Band Select */
 #define SYSCONFIG2_SPACE	0x0030	/* bits 05..04: Channel Spacing */
 #define SYSCONFIG2_VOLUME	0x000f	/* bits 03..00: Volume */
 
-- 
1.7.10.4

[PATCH 4/5] radio-si470x: Add support for the new band APIs

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/si470x/radio-si470x-common.c |  215 +++++++++++++---------
 drivers/media/radio/si470x/radio-si470x-i2c.c    |    1 +
 drivers/media/radio/si470x/radio-si470x-usb.c    |    1 +
 drivers/media/radio/si470x/radio-si470x.h        |    1 +
 4 files changed, 126 insertions(+), 92 deletions(-)

diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c
index 84ab3d57..9e38132 100644
--- a/drivers/media/radio/si470x/radio-si470x-common.c
+++ b/drivers/media/radio/si470x/radio-si470x-common.c
@@ -4,6 +4,7 @@
  *  Driver for radios with Silicon Labs Si470x FM Radio Receivers
  *
  *  Copyright (c) 2009 Tobias Lorenz <tobias.lorenz@gmx.net>
+ *  Copyright (c) 2012 Hans de Goede <hdegoede@redhat.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -127,14 +128,6 @@ static unsigned short space = 2;
 module_param(space, ushort, 0444);
 MODULE_PARM_DESC(space, "Spacing: 0=200kHz 1=100kHz *2=50kHz*");
 
-/* Bottom of Band (MHz) */
-/* 0: 87.5 - 108 MHz (USA, Europe)*/
-/* 1: 76   - 108 MHz (Japan wide band) */
-/* 2: 76   -  90 MHz (Japan) */
-static unsigned short band = 1;
-module_param(band, ushort, 0444);
-MODULE_PARM_DESC(band, "Band: 0=87.5..108MHz *1=76..108MHz* 2=76..90MHz");
-
 /* De-emphasis */
 /* 0: 75 us (USA) */
 /* 1: 50 us (Europe, Australia, Japan) */
@@ -152,13 +145,61 @@ static unsigned int seek_timeout = 5000;
 module_param(seek_timeout, uint, 0644);
 MODULE_PARM_DESC(seek_timeout, "Seek timeout: *5000*");
 
-
+static const struct v4l2_frequency_band bands[] = {
+	{
+		.type = V4L2_TUNER_RADIO,
+		.index = 0,
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+			    V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO |
+			    V4L2_TUNER_CAP_HWSEEK_BOUNDED |
+			    V4L2_TUNER_CAP_HWSEEK_WRAP,
+		.rangelow   =  87500 * 16,
+		.rangehigh  = 108000 * 16,
+		.modulation = V4L2_BAND_MODULATION_FM,
+	},
+	{
+		.type = V4L2_TUNER_RADIO,
+		.index = 1,
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+			    V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO |
+			    V4L2_TUNER_CAP_HWSEEK_BOUNDED |
+			    V4L2_TUNER_CAP_HWSEEK_WRAP,
+		.rangelow   =  76000 * 16,
+		.rangehigh  = 108000 * 16,
+		.modulation = V4L2_BAND_MODULATION_FM,
+	},
+	{
+		.type = V4L2_TUNER_RADIO,
+		.index = 2,
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+			    V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO |
+			    V4L2_TUNER_CAP_HWSEEK_BOUNDED |
+			    V4L2_TUNER_CAP_HWSEEK_WRAP,
+		.rangelow   =  76000 * 16,
+		.rangehigh  =  90000 * 16,
+		.modulation = V4L2_BAND_MODULATION_FM,
+	},
+};
 
 /**************************************************************************
  * Generic Functions
  **************************************************************************/
 
 /*
+ * si470x_set_band - set the band
+ */
+static int si470x_set_band(struct si470x_device *radio, int band)
+{
+	if (radio->band == band)
+		return 0;
+
+	radio->band = band;
+	radio->registers[SYSCONFIG2] &= ~SYSCONFIG2_BAND;
+	radio->registers[SYSCONFIG2] |= radio->band << 6;
+	return si470x_set_register(radio, SYSCONFIG2);
+}
+
+/*
  * si470x_set_chan - set the channel
  */
 static int si470x_set_chan(struct si470x_device *radio, unsigned short chan)
@@ -194,48 +235,39 @@ done:
 	return retval;
 }
 
-
 /*
- * si470x_get_freq - get the frequency
+ * si470x_get_step - get channel spacing
  */
-static int si470x_get_freq(struct si470x_device *radio, unsigned int *freq)
+static unsigned int si470x_get_step(struct si470x_device *radio)
 {
-	unsigned int spacing, band_bottom;
-	unsigned short chan;
-	int retval;
-
 	/* Spacing (kHz) */
 	switch ((radio->registers[SYSCONFIG2] & SYSCONFIG2_SPACE) >> 4) {
 	/* 0: 200 kHz (USA, Australia) */
 	case 0:
-		spacing = 0.200 * FREQ_MUL; break;
+		return 200 * 16;
 	/* 1: 100 kHz (Europe, Japan) */
 	case 1:
-		spacing = 0.100 * FREQ_MUL; break;
+		return 100 * 16;
 	/* 2:  50 kHz */
 	default:
-		spacing = 0.050 * FREQ_MUL; break;
+		return 50 * 16;
 	};
+}
 
-	/* Bottom of Band (MHz) */
-	switch ((radio->registers[SYSCONFIG2] & SYSCONFIG2_BAND) >> 6) {
-	/* 0: 87.5 - 108 MHz (USA, Europe) */
-	case 0:
-		band_bottom = 87.5 * FREQ_MUL; break;
-	/* 1: 76   - 108 MHz (Japan wide band) */
-	default:
-		band_bottom = 76   * FREQ_MUL; break;
-	/* 2: 76   -  90 MHz (Japan) */
-	case 2:
-		band_bottom = 76   * FREQ_MUL; break;
-	};
+
+/*
+ * si470x_get_freq - get the frequency
+ */
+static int si470x_get_freq(struct si470x_device *radio, unsigned int *freq)
+{
+	int chan, retval;
 
 	/* read channel */
 	retval = si470x_get_register(radio, READCHAN);
 	chan = radio->registers[READCHAN] & READCHAN_READCHAN;
 
 	/* Frequency (MHz) = Spacing (kHz) x Channel + Bottom of Band (MHz) */
-	*freq = chan * spacing + band_bottom;
+	*freq = chan * si470x_get_step(radio) + bands[radio->band].rangelow;
 
 	return retval;
 }
@@ -246,44 +278,12 @@ static int si470x_get_freq(struct si470x_device *radio, unsigned int *freq)
  */
 int si470x_set_freq(struct si470x_device *radio, unsigned int freq)
 {
-	unsigned int spacing, band_bottom, band_top;
 	unsigned short chan;
 
-	/* Spacing (kHz) */
-	switch ((radio->registers[SYSCONFIG2] & SYSCONFIG2_SPACE) >> 4) {
-	/* 0: 200 kHz (USA, Australia) */
-	case 0:
-		spacing = 0.200 * FREQ_MUL; break;
-	/* 1: 100 kHz (Europe, Japan) */
-	case 1:
-		spacing = 0.100 * FREQ_MUL; break;
-	/* 2:  50 kHz */
-	default:
-		spacing = 0.050 * FREQ_MUL; break;
-	};
-
-	/* Bottom/Top of Band (MHz) */
-	switch ((radio->registers[SYSCONFIG2] & SYSCONFIG2_BAND) >> 6) {
-	/* 0: 87.5 - 108 MHz (USA, Europe) */
-	case 0:
-		band_bottom = 87.5 * FREQ_MUL;
-		band_top = 108 * FREQ_MUL;
-		break;
-	/* 1: 76   - 108 MHz (Japan wide band) */
-	default:
-		band_bottom = 76 * FREQ_MUL;
-		band_top = 108 * FREQ_MUL;
-		break;
-	/* 2: 76   -  90 MHz (Japan) */
-	case 2:
-		band_bottom = 76 * FREQ_MUL;
-		band_top = 90 * FREQ_MUL;
-		break;
-	};
-
-	freq = clamp(freq, band_bottom, band_top);
+	freq = clamp(freq, bands[radio->band].rangelow,
+			   bands[radio->band].rangehigh);
 	/* Chan = [ Freq (Mhz) - Bottom of Band (MHz) ] / Spacing (kHz) */
-	chan = (freq - band_bottom) / spacing;
+	chan = (freq - bands[radio->band].rangelow) / si470x_get_step(radio);
 
 	return si470x_set_chan(radio, chan);
 }
@@ -293,18 +293,43 @@ int si470x_set_freq(struct si470x_device *radio, unsigned int freq)
  * si470x_set_seek - set seek
  */
 static int si470x_set_seek(struct si470x_device *radio,
-		unsigned int wrap_around, unsigned int seek_upward)
+			   struct v4l2_hw_freq_seek *seek)
 {
-	int retval = 0;
+	int band, retval;
+	unsigned int freq;
 	bool timed_out = 0;
 
+	/* set band */
+	if (seek->rangelow || seek->rangehigh) {
+		for (band = 0; band < ARRAY_SIZE(bands); band++) {
+			if (bands[band].rangelow  == seek->rangelow &&
+			    bands[band].rangehigh == seek->rangehigh)
+				break;
+		}
+		if (band == ARRAY_SIZE(bands))
+			return -EINVAL; /* No matching band found */
+	} else
+		band = 1; /* If nothing is specified seek 76 - 108 Mhz */
+
+	if (radio->band != band) {
+		retval = si470x_get_freq(radio, &freq);
+		if (retval)
+			return retval;
+		retval = si470x_set_band(radio, band);
+		if (retval)
+			return retval;
+		retval = si470x_set_freq(radio, freq);
+		if (retval)
+			return retval;
+	}
+
 	/* start seeking */
 	radio->registers[POWERCFG] |= POWERCFG_SEEK;
-	if (wrap_around == 1)
+	if (seek->wrap_around)
 		radio->registers[POWERCFG] &= ~POWERCFG_SKMODE;
 	else
 		radio->registers[POWERCFG] |= POWERCFG_SKMODE;
-	if (seek_upward == 1)
+	if (seek->seek_upward)
 		radio->registers[POWERCFG] |= POWERCFG_SEEKUP;
 	else
 		radio->registers[POWERCFG] &= ~POWERCFG_SEEKUP;
@@ -360,7 +385,7 @@ int si470x_start(struct si470x_device *radio)
 	/* sysconfig 2 */
 	radio->registers[SYSCONFIG2] =
 		(0x1f  << 8) |				/* SEEKTH */
-		((band  << 6) & SYSCONFIG2_BAND)  |	/* BAND */
+		((radio->band << 6) & SYSCONFIG2_BAND) |/* BAND */
 		((space << 4) & SYSCONFIG2_SPACE) |	/* SPACE */
 		15;					/* VOLUME (max) */
 	retval = si470x_set_register(radio, SYSCONFIG2);
@@ -569,25 +594,8 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv,
 			    V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO |
 			    V4L2_TUNER_CAP_HWSEEK_BOUNDED |
 			    V4L2_TUNER_CAP_HWSEEK_WRAP;
-
-	/* range limits */
-	switch ((radio->registers[SYSCONFIG2] & SYSCONFIG2_BAND) >> 6) {
-	/* 0: 87.5 - 108 MHz (USA, Europe, default) */
-	default:
-		tuner->rangelow  =  87.5 * FREQ_MUL;
-		tuner->rangehigh = 108   * FREQ_MUL;
-		break;
-	/* 1: 76   - 108 MHz (Japan wide band) */
-	case 1:
-		tuner->rangelow  =  76   * FREQ_MUL;
-		tuner->rangehigh = 108   * FREQ_MUL;
-		break;
-	/* 2: 76   -  90 MHz (Japan) */
-	case 2:
-		tuner->rangelow  =  76   * FREQ_MUL;
-		tuner->rangehigh =  90   * FREQ_MUL;
-		break;
-	};
+	tuner->rangelow  =  76 * FREQ_MUL;
+	tuner->rangehigh = 108 * FREQ_MUL;
 
 	/* stereo indicator == stereo (instead of mono) */
 	if ((radio->registers[STATUSRSSI] & STATUSRSSI_ST) == 0)
@@ -670,10 +678,18 @@ static int si470x_vidioc_s_frequency(struct file *file, void *priv,
 		struct v4l2_frequency *freq)
 {
 	struct si470x_device *radio = video_drvdata(file);
+	int retval;
 
 	if (freq->tuner != 0)
 		return -EINVAL;
 
+	if (freq->frequency < bands[radio->band].rangelow ||
+	    freq->frequency > bands[radio->band].rangehigh) {
+		/* Switch to band 1 which covers everything we support */
+		retval = si470x_set_band(radio, 1);
+		if (retval)
+			return retval;
+	}
 	return si470x_set_freq(radio, freq->frequency);
 }
 
@@ -689,7 +705,21 @@ static int si470x_vidioc_s_hw_freq_seek(struct file *file, void *priv,
 	if (seek->tuner != 0)
 		return -EINVAL;
 
-	return si470x_set_seek(radio, seek->wrap_around, seek->seek_upward);
+	return si470x_set_seek(radio, seek);
+}
+
+/*
+ * si470x_vidioc_enum_freq_bands - enumerate supported bands
+ */
+static int si470x_vidioc_enum_freq_bands(struct file *file, void *priv,
+					 struct v4l2_frequency_band *band)
+{
+	if (band->tuner != 0)
+		return -EINVAL;
+	if (band->index >= ARRAY_SIZE(bands))
+		return -EINVAL;
+	*band = bands[band->index];
+	return 0;
 }
 
 const struct v4l2_ctrl_ops si470x_ctrl_ops = {
@@ -706,6 +736,7 @@ static const struct v4l2_ioctl_ops si470x_ioctl_ops = {
 	.vidioc_g_frequency	= si470x_vidioc_g_frequency,
 	.vidioc_s_frequency	= si470x_vidioc_s_frequency,
 	.vidioc_s_hw_freq_seek	= si470x_vidioc_s_hw_freq_seek,
+	.vidioc_enum_freq_bands = si470x_vidioc_enum_freq_bands,
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
index fb401a2..643a6ff 100644
--- a/drivers/media/radio/si470x/radio-si470x-i2c.c
+++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
@@ -350,6 +350,7 @@ static int __devinit si470x_i2c_probe(struct i2c_client *client,
 	}
 
 	radio->client = client;
+	radio->band = 1; /* Default to 76 - 108 MHz */
 	mutex_init(&radio->lock);
 	init_completion(&radio->completion);
 
diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index 0204cf4..146be42 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -597,6 +597,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
 	}
 	radio->usbdev = interface_to_usbdev(intf);
 	radio->intf = intf;
+	radio->band = 1; /* Default to 76 - 108 MHz */
 	mutex_init(&radio->lock);
 	init_completion(&radio->completion);
 
diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h
index 11f14b6..8e3a62f 100644
--- a/drivers/media/radio/si470x/radio-si470x.h
+++ b/drivers/media/radio/si470x/radio-si470x.h
@@ -147,6 +147,7 @@ struct si470x_device {
 	struct v4l2_device v4l2_dev;
 	struct video_device videodev;
 	struct v4l2_ctrl_handler hdl;
+	int band;
 
 	/* Silabs internal registers (0..15) */
 	unsigned short registers[RADIO_REGISTER_NUM];
-- 
1.7.10.4

[PATCH 5/5] radio-si470x: Lower firmware version requirements

[Hans de Goede]

Testing with a firmware version 12 usb radio stick has shown version 12
to work fine too.

Reported-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/si470x/radio-si470x.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h
index 8e3a62f..2f089b4 100644
--- a/drivers/media/radio/si470x/radio-si470x.h
+++ b/drivers/media/radio/si470x/radio-si470x.h
@@ -190,7 +190,7 @@ struct si470x_device {
  * Firmware Versions
  **************************************************************************/
 
-#define RADIO_FW_VERSION	14
+#define RADIO_FW_VERSION	12
 
 
 
-- 
1.7.10.4