[RFCv2 PATCH 0/5] Add hwseek caps and frequency bands

[RFCv2 PATCH 0/5] Add hwseek caps and frequency bands

[Hans Verkuil]

Changes since v1:

- Fixed typo in second patch
- Patch 5 now only contains the part about frequency bands
- Patch 6 contains only (I hope) a non-controversial clarification
regarding modulators (and a small change making a line more understandable).

Regards,

	Hans

This patch series adds improved hwseek support as discussed here:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg45957.html

and on irc:

http://linuxtv.org/irc/v4l/index.php?date=2012-05-26

>From the RFC I have implemented/documented items 1-4 and 6a. I decided
not to go with option 6b. This may be added in the future if there is a
clear need.

The addition of the frequency band came out of this discussion:

http://www.spinics.net/lists/linux-media/msg48272.html

Regards,

        Hans

[RFCv2 PATCH 1/6] videodev2.h: add new hwseek capability bits.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Tell the application whether the hardware seek is bounded and/or wraps around.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 include/linux/videodev2.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 370d111..2339678 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2039,6 +2039,8 @@ struct v4l2_modulator {
 /*  Flags for the 'capability' field */
 #define V4L2_TUNER_CAP_LOW		0x0001
 #define V4L2_TUNER_CAP_NORM		0x0002
+#define V4L2_TUNER_CAP_HWSEEK_BOUNDED	0x0004
+#define V4L2_TUNER_CAP_HWSEEK_WRAP	0x0008
 #define V4L2_TUNER_CAP_STEREO		0x0010
 #define V4L2_TUNER_CAP_LANG2		0x0020
 #define V4L2_TUNER_CAP_SAP		0x0020
-- 
1.7.10

[RFCv2 PATCH 2/6] v4l2 spec: document the new v4l2_tuner capabilities

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Update the spec with the new capabilities and specify new error codes for
S_HW_FREQ_SEEK.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 .../DocBook/media/v4l/vidioc-g-frequency.xml         |    6 ++++++
 Documentation/DocBook/media/v4l/vidioc-g-tuner.xml   |   12 ++++++++++++
 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml      |   18 +++++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
index 69c178a..40e58a4 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
@@ -135,6 +135,12 @@ bounds or the value in the <structfield>type</structfield> field is
 wrong.</para>
 	</listitem>
       </varlistentry>
+      <varlistentry>
+	<term><errorcode>EBUSY</errorcode></term>
+	<listitem>
+	  <para>A hardware seek is in progress.</para>
+	</listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
 </refentry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
index 62a1aa2..95d5371 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
@@ -276,6 +276,18 @@ can or must be switched. (B/G PAL tuners for example are typically not
       <constant>V4L2_TUNER_ANALOG_TV</constant> tuners can have this capability.</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_TUNER_CAP_HWSEEK_BOUNDED</constant></entry>
+	    <entry>0x0004</entry>
+	    <entry>If set, then this tuner supports the hardware seek functionality
+	    where the seek stops when it reaches the end of the frequency range.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_TUNER_CAP_HWSEEK_WRAP</constant></entry>
+	    <entry>0x0008</entry>
+	    <entry>If set, then this tuner supports the hardware seek functionality
+	    where the seek wraps around when it reaches the end of the frequency range.</entry>
+	  </row>
+	  <row>
 	<entry><constant>V4L2_TUNER_CAP_STEREO</constant></entry>
 	<entry>0x0010</entry>
 	<entry>Stereo audio reception is supported.</entry>
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 407dfce..f4db44d 100644
--- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
@@ -58,6 +58,9 @@ To do this applications initialize the <structfield>tuner</structfield>,
 call the <constant>VIDIOC_S_HW_FREQ_SEEK</constant> ioctl with a pointer
 to this structure.</para>
 
+    <para>If an error is returned, then the original frequency will
+    be restored.</para>
+
     <para>This ioctl is supported if the <constant>V4L2_CAP_HW_FREQ_SEEK</constant> capability is set.</para>
 
     <table pgwide="1" frame="none" id="v4l2-hw-freq-seek">
@@ -87,7 +90,10 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>wrap_around</structfield></entry>
-	    <entry>If non-zero, wrap around when at the end of the frequency range, else stop seeking.</entry>
+	    <entry>If non-zero, wrap around when at the end of the frequency range, else stop seeking.
+	    The &v4l2-tuner; <structfield>capability</structfield> field will tell you what the
+	    hardware supports.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -118,9 +124,15 @@ wrong.</para>
 	</listitem>
       </varlistentry>
       <varlistentry>
-	<term><errorcode>EAGAIN</errorcode></term>
+	<term><errorcode>ENODATA</errorcode></term>
+	<listitem>
+	  <para>The hardware seek found no channels.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><errorcode>EBUSY</errorcode></term>
 	<listitem>
-	  <para>The ioctl timed-out. Try again.</para>
+	  <para>Another hardware seek is already in progress.</para>
 	</listitem>
       </varlistentry>
     </variablelist>
-- 
1.7.10

[RFCv2 PATCH 3/6] S_HW_FREQ_SEEK: set capability flags and return ENODATA instead of EAGAIN.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Set the new capability flags in G_TUNER and return ENODATA if no channels
were found.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/radio/radio-mr800.c                |    5 +++--
 drivers/media/radio/radio-wl1273.c               |    3 ++-
 drivers/media/radio/si470x/radio-si470x-common.c |    6 ++++--
 drivers/media/radio/wl128x/fmdrv_rx.c            |    2 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.c          |    4 +++-
 sound/i2c/other/tea575x-tuner.c                  |    4 +++-
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/media/radio/radio-mr800.c b/drivers/media/radio/radio-mr800.c
index 94cb6bc..3182b26 100644
--- a/drivers/media/radio/radio-mr800.c
+++ b/drivers/media/radio/radio-mr800.c
@@ -295,7 +295,8 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 	v->type = V4L2_TUNER_RADIO;
 	v->rangelow = FREQ_MIN * FREQ_MUL;
 	v->rangehigh = FREQ_MAX * FREQ_MUL;
-	v->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO;
+	v->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+		V4L2_TUNER_CAP_HWSEEK_WRAP;
 	v->rxsubchans = is_stereo ? V4L2_TUNER_SUB_STEREO : V4L2_TUNER_SUB_MONO;
 	v->audmode = radio->stereo ?
 		V4L2_TUNER_MODE_STEREO : V4L2_TUNER_MODE_MONO;
@@ -372,7 +373,7 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *priv,
 	timeout = jiffies + msecs_to_jiffies(30000);
 	for (;;) {
 		if (time_after(jiffies, timeout)) {
-			retval = -EAGAIN;
+			retval = -ENODATA;
 			break;
 		}
 		if (schedule_timeout_interruptible(msecs_to_jiffies(10))) {
diff --git a/drivers/media/radio/radio-wl1273.c b/drivers/media/radio/radio-wl1273.c
index f1b6070..e8428f5 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1514,7 +1514,8 @@ static int wl1273_fm_vidioc_g_tuner(struct file *file, void *priv,
 	tuner->rangehigh = WL1273_FREQ(WL1273_BAND_OTHER_HIGH);
 
 	tuner->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_RDS |
-		V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS_BLOCK_IO;
+		V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS_BLOCK_IO |
+		V4L2_TUNER_CAP_HWSEEK_BOUNDED | V4L2_TUNER_CAP_HWSEEK_WRAP;
 
 	if (radio->stereo)
 		tuner->audmode = V4L2_TUNER_MODE_STEREO;
diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c
index 969cf49..d485b79 100644
--- a/drivers/media/radio/si470x/radio-si470x-common.c
+++ b/drivers/media/radio/si470x/radio-si470x-common.c
@@ -363,7 +363,7 @@ stop:
 
 	/* try again, if timed out */
 	if (retval == 0 && timed_out)
-		return -EAGAIN;
+		return -ENODATA;
 	return retval;
 }
 
@@ -596,7 +596,9 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv,
 	strcpy(tuner->name, "FM");
 	tuner->type = V4L2_TUNER_RADIO;
 	tuner->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
-			    V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO;
+			    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) {
diff --git a/drivers/media/radio/wl128x/fmdrv_rx.c b/drivers/media/radio/wl128x/fmdrv_rx.c
index 43fb722..3dd9fc0 100644
--- a/drivers/media/radio/wl128x/fmdrv_rx.c
+++ b/drivers/media/radio/wl128x/fmdrv_rx.c
@@ -251,7 +251,7 @@ again:
 	if (!timeleft) {
 		fmerr("Timeout(%d sec),didn't get tune ended int\n",
 			   jiffies_to_msecs(FM_DRV_RX_SEEK_TIMEOUT) / 1000);
-		return -ETIMEDOUT;
+		return -ENODATA;
 	}
 
 	int_reason = fmdev->irq_info.flag & (FM_TUNE_COMPLETE | FM_BAND_LIMIT);
diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index 080b96a..49a11ec 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -285,7 +285,9 @@ static int fm_v4l2_vidioc_g_tuner(struct file *file, void *priv,
 	tuner->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO |
 	((fmdev->rx.rds.flag == FM_RDS_ENABLE) ? V4L2_TUNER_SUB_RDS : 0);
 	tuner->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
-			    V4L2_TUNER_CAP_LOW;
+			    V4L2_TUNER_CAP_LOW |
+			    V4L2_TUNER_CAP_HWSEEK_BOUNDED |
+			    V4L2_TUNER_CAP_HWSEEK_WRAP;
 	tuner->audmode = (stereo_mono_mode ?
 			  V4L2_TUNER_MODE_MONO : V4L2_TUNER_MODE_STEREO);
 
diff --git a/sound/i2c/other/tea575x-tuner.c b/sound/i2c/other/tea575x-tuner.c
index 582aace..ba2bc51 100644
--- a/sound/i2c/other/tea575x-tuner.c
+++ b/sound/i2c/other/tea575x-tuner.c
@@ -191,6 +191,8 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 	strcpy(v->name, "FM");
 	v->type = V4L2_TUNER_RADIO;
 	v->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO;
+	if (!tea->cannot_read_data)
+		v->capability |= V4L2_TUNER_CAP_HWSEEK_BOUNDED;
 	v->rangelow = FREQ_LO;
 	v->rangehigh = FREQ_HI;
 	v->rxsubchans = tea->stereo ? V4L2_TUNER_SUB_STEREO : V4L2_TUNER_SUB_MONO;
@@ -299,7 +301,7 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *fh,
 	}
 	tea->val &= ~TEA575X_BIT_SEARCH;
 	snd_tea575x_set_freq(tea);
-	return -EAGAIN;
+	return -ENODATA;
 }
 
 static int tea575x_s_ctrl(struct v4l2_ctrl *ctrl)
-- 
1.7.10

[RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
---
 include/linux/videodev2.h |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 2339678..013ee46 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2023,7 +2023,8 @@ struct v4l2_tuner {
 	__u32			audmode;
 	__s32			signal;
 	__s32			afc;
-	__u32			reserved[4];
+	__u32			band;
+	__u32			reserved[3];
 };
 
 struct v4l2_modulator {
@@ -2033,7 +2034,8 @@ struct v4l2_modulator {
 	__u32			rangelow;
 	__u32			rangehigh;
 	__u32			txsubchans;
-	__u32			reserved[4];
+	__u32			band;
+	__u32			reserved[3];
 };
 
 /*  Flags for the 'capability' field */
@@ -2048,6 +2050,11 @@ struct v4l2_modulator {
 #define V4L2_TUNER_CAP_RDS		0x0080
 #define V4L2_TUNER_CAP_RDS_BLOCK_IO	0x0100
 #define V4L2_TUNER_CAP_RDS_CONTROLS	0x0200
+#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
+#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
+#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
+#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
+#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
 
 /*  Flags for the 'rxsubchans' field */
 #define V4L2_TUNER_SUB_MONO		0x0001
@@ -2065,6 +2072,14 @@ struct v4l2_modulator {
 #define V4L2_TUNER_MODE_LANG1		0x0003
 #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
 
+/*  Values for the 'band' field */
+#define V4L2_TUNER_BAND_DEFAULT       0
+#define V4L2_TUNER_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
+#define V4L2_TUNER_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
+#define V4L2_TUNER_BAND_FM_RUSSIAN    3       /* 65.8 MHz - 74 MHz */
+#define V4L2_TUNER_BAND_FM_WEATHER    4       /* 162.4 MHz - 162.55 MHz */
+#define V4L2_TUNER_BAND_AM_MW         5
+
 struct v4l2_frequency {
 	__u32		      tuner;
 	__u32		      type;	/* enum v4l2_tuner_type */
-- 
1.7.10

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Mauro Carvalho Chehab]

Em 28-05-2012 07:46, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   include/linux/videodev2.h |   19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 2339678..013ee46 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>   	__u32			audmode;
>   	__s32			signal;
>   	__s32			afc;
> -	__u32			reserved[4];
> +	__u32			band;
> +	__u32			reserved[3];
>   };
>   
>   struct v4l2_modulator {
> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>   	__u32			rangelow;
>   	__u32			rangehigh;
>   	__u32			txsubchans;
> -	__u32			reserved[4];
> +	__u32			band;
> +	__u32			reserved[3];
>   };
>   
>   /*  Flags for the 'capability' field */
> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>   #define V4L2_TUNER_CAP_RDS		0x0080
>   #define V4L2_TUNER_CAP_RDS_BLOCK_IO	0x0100
>   #define V4L2_TUNER_CAP_RDS_CONTROLS	0x0200
> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000

Frequency band is already specified by rangelow/rangehigh.

Why do you need to duplicate this information?


>   
>   /*  Flags for the 'rxsubchans' field */
>   #define V4L2_TUNER_SUB_MONO		0x0001
> @@ -2065,6 +2072,14 @@ struct v4l2_modulator {
>   #define V4L2_TUNER_MODE_LANG1		0x0003
>   #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
>   
> +/*  Values for the 'band' field */
> +#define V4L2_TUNER_BAND_DEFAULT       0

What does "default" mean?

> +#define V4L2_TUNER_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */

EUROPE_US is a bad name for this range. According with Wikipedia, this
range is used at "ITU region 1" (Europe/Africa), while America uses 
ITU region 2 (88-108).

In Brazil, the range from 87.5-88 were added several years ago, so it is
currently at the "ITU region 1" range, just like in US.

I don't doubt that there are still some places at the 88-108 MHz range.

> +#define V4L2_TUNER_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */

This is currently true, but wikipedia points that they may increase it 
(from 76MHz to 108MHz?) after the end of NTSC broadcast.

The DTV range there starts at channel 14 (473 MHz and upper). Maybe they
may reserve the channel 7-13 range (VHF High - starting at 177 MHz) like
Brazil for DTV. 

Anyway, what I mean is that calling a frequency range with a Country name
is dangerous, as frequency ranges can vary from time to time.

> +#define V4L2_TUNER_BAND_FM_RUSSIAN    3       /* 65.8 MHz - 74 MHz */

AFAIKT, this is wrong. The range used there is 65.8-104MHz.

It used to be 65.8 to 100 MHz.

Also, other ex-soviet countries are still using such range.

> +#define V4L2_TUNER_BAND_FM_WEATHER    4       /* 162.4 MHz - 162.55 MHz */
> +#define V4L2_TUNER_BAND_AM_MW         5
> +
>   struct v4l2_frequency {
>   	__u32		      tuner;
>   	__u32		      type;	/* enum v4l2_tuner_type */
> 

Regards,
Mauro

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>    include/linux/videodev2.h |   19 +++++++++++++++++--
>>    1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index 2339678..013ee46 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>    	__u32			audmode;
>>    	__s32			signal;
>>    	__s32			afc;
>> -	__u32			reserved[4];
>> +	__u32			band;
>> +	__u32			reserved[3];
>>    };
>>
>>    struct v4l2_modulator {
>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>    	__u32			rangelow;
>>    	__u32			rangehigh;
>>    	__u32			txsubchans;
>> -	__u32			reserved[4];
>> +	__u32			band;
>> +	__u32			reserved[3];
>>    };
>>
>>    /*  Flags for the 'capability' field */
>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>    #define V4L2_TUNER_CAP_RDS		0x0080
>>    #define V4L2_TUNER_CAP_RDS_BLOCK_IO	0x0100
>>    #define V4L2_TUNER_CAP_RDS_CONTROLS	0x0200
>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>
> Frequency band is already specified by rangelow/rangehigh.
>
> Why do you need to duplicate this information?

Because radio tuners may support multiple non overlapping
bands, this is why this patch also adds a band member
to the tuner struct, which can be used to set/get
the current band.

One example of this are the tea5757 / tea5759
radio tuner chips:

FM:
tea5757 87.5 - 108 MHz
tea5759 76 - 91 MHz

AM:
Both: 530 - 1710 kHz

So an app would set as band one of DEFAULT, EUROPE_US
(or JAPAN depending on the model) and AM_MW, and then
get the actual range supported reported in rangelow /
rangehigh on a subsequent G_TUNER.

Note that setting ie a band of FM_JAPAN on a 5757 would
result in the S_TUNER failing with -EINVAL.

>
>
>>
>>    /*  Flags for the 'rxsubchans' field */
>>    #define V4L2_TUNER_SUB_MONO		0x0001
>> @@ -2065,6 +2072,14 @@ struct v4l2_modulator {
>>    #define V4L2_TUNER_MODE_LANG1		0x0003
>>    #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
>>
>> +/*  Values for the 'band' field */
>> +#define V4L2_TUNER_BAND_DEFAULT       0
>
> What does "default" mean?

Default means default. This is for compatibility with
old apps which don't know about the new tuner band API
extension so they will set this field to 0 (as reserved
fields should be set to 0 by userspace). In this case
we don't want to fail with -EINVAL based on the band
value, so we need some value all tuners will accept.

Some tuners, ie the si470x support both selecting a
specific FM band, as well as selecting a "universal"
FM band of 76 - 108 MHz. For those default would be
the universal FM band. For the tea575x devices discussed
above default would have the range of whatever FM band
they support.

Note that even on devices with a universal band being
able to select a certain band is quite useful to limit
hardware freq-seek to this band since searching freqs
below 87.5 is useless in europe / US for example.

Thinking more about this I think we should rename
V4L2_TUNER_BAND_DEFAULT to V4L2_TUNER_BAND_FM_UNIVERSAL,
and document that this means the widest FM band the
device supports, with the actual limits being reported
in rangelow and rangehigh. Note that the mentioned ranges
by the bands are indications of the expected range only
the true range will still be reported through rangelow and
rangehigh, and this is what apps are expected to use.

Defining 0 as V4L2_TUNER_BAND_FM_UNIVERSAL does cause
a -EINVAL when doing a S_TUNER with a band value of 0
on AM only tuners, but:
1) We don't support AM only tuners atm, and I don't expect
we will in the future either
2) Non band aware apps don't work well with AM tuners anyways
(as they must take much smaller frequency steps for one).

>
>> +#define V4L2_TUNER_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
>
> EUROPE_US is a bad name for this range. According with Wikipedia, this
> range is used at "ITU region 1" (Europe/Africa), while America uses
> ITU region 2 (88-108).
>
> In Brazil, the range from 87.5-88 were added several years ago, so it is
> currently at the "ITU region 1" range, just like in US.
>
> I don't doubt that there are still some places at the 88-108 MHz range.

87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
creating 2 band defines for this.

>
>> +#define V4L2_TUNER_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
>
> This is currently true, but wikipedia points that they may increase it
> (from 76MHz to 108MHz?) after the end of NTSC broadcast.
>

This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
thus going all the way from 76 - 163 Mhz, so I guess we should add a
V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
supports this, and indeed calls it "Japan wide band"

> The DTV range there starts at channel 14 (473 MHz and upper). Maybe they
> may reserve the channel 7-13 range (VHF High - starting at 177 MHz) like
> Brazil for DTV.
>
> Anyway, what I mean is that calling a frequency range with a Country name
> is dangerous, as frequency ranges can vary from time to time.
>

So lets get back to the basis, for AM/FM switching / limiting hw-freq
seeking, and on some devices likely even just to be able to tune to
certain frequencies we need to select a band with various radio devices.

On some radio devices we may be able to just program the seek range, but on
most it is hardcoded based on a band selection register.

So we need some way of naming the bands, with approx. expected ranges
(the real range supported by the specific device will be reported on a
G_TUNER).

Looking at:
http://en.wikipedia.org/wiki/FM_broadcast_band

I suggest naming the bands after their standards, except for the Japanese
bands which are special and I suggest just naming them after their
country, resulting in:

#define V4L2_TUNER_BAND_FM_CCIR		1 /* 87.5 - 108 Mhz */
#define V4L2_TUNER_BAND_FM_OIRT		2 /* 65.8 MHz - 74 MHz */
#define V4L2_TUNER_BAND_JAPAN		3 /* 76 MHz - 90 MHz */
#define V4L2_TUNER_BAND_JAPAN_WIDE	4 /* 76 MHz - 108 MHz */
#define V4L2_TUNER_BAND_WEATHER		5 /* 162.4 MHz - 162.55 MHz */

Note for rationale of the weather band, see:
http://en.wikipedia.org/wiki/Weather_radio

Regards,

Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Mauro Carvalho Chehab]

Em 19-06-2012 05:27, Hans de Goede escreveu:
> Hi,
> 
> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>    include/linux/videodev2.h |   19 +++++++++++++++++--
>>>    1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 2339678..013ee46 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>        __u32            audmode;
>>>        __s32            signal;
>>>        __s32            afc;
>>> -    __u32            reserved[4];
>>> +    __u32            band;
>>> +    __u32            reserved[3];
>>>    };
>>>
>>>    struct v4l2_modulator {
>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>        __u32            rangelow;
>>>        __u32            rangehigh;
>>>        __u32            txsubchans;
>>> -    __u32            reserved[4];
>>> +    __u32            band;
>>> +    __u32            reserved[3];
>>>    };
>>>
>>>    /*  Flags for the 'capability' field */
>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>    #define V4L2_TUNER_CAP_RDS        0x0080
>>>    #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>    #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>
>> Frequency band is already specified by rangelow/rangehigh.
>>
>> Why do you need to duplicate this information?
> 
> Because radio tuners may support multiple non overlapping
> bands, this is why this patch also adds a band member
> to the tuner struct, which can be used to set/get
> the current band.
> 
> One example of this are the tea5757 / tea5759
> radio tuner chips:
> 
> FM:
> tea5757 87.5 - 108 MHz

	rangelow = 87.5 * 62500;
	rangehigh = 108 * 62500;

> tea5759 76 - 91 MHz

	rangelow = 76 * 62500;
	rangehigh = 91 * 62500;

> AM:
> Both: 530 - 1710 kHz

	rangelow = 0.530 * 62500;
	rangehigh = 0.1710 * 62500;


See radio-cadet.c:

static int vidioc_g_tuner(struct file *file, void *priv,
				struct v4l2_tuner *v)
{
	struct cadet *dev = video_drvdata(file);

	v->type = V4L2_TUNER_RADIO;
	switch (v->index) {
	case 0:
		strlcpy(v->name, "FM", sizeof(v->name));
		v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
			V4L2_TUNER_CAP_RDS_BLOCK_IO;
		v->rangelow = 1400;     /* 87.5 MHz */
		v->rangehigh = 1728;    /* 108.0 MHz */
		v->rxsubchans = cadet_getstereo(dev);
		switch (v->rxsubchans) {
		case V4L2_TUNER_SUB_MONO:
			v->audmode = V4L2_TUNER_MODE_MONO;
			break;
		case V4L2_TUNER_SUB_STEREO:
			v->audmode = V4L2_TUNER_MODE_STEREO;
			break;
		default:
			break;
		}
		v->rxsubchans |= V4L2_TUNER_SUB_RDS;
		break;
	case 1:
		strlcpy(v->name, "AM", sizeof(v->name));
		v->capability = V4L2_TUNER_CAP_LOW;
		v->rangelow = 8320;      /* 520 kHz */
		v->rangehigh = 26400;    /* 1650 kHz */
		v->rxsubchans = V4L2_TUNER_SUB_MONO;
		v->audmode = V4L2_TUNER_MODE_MONO;
		break;
	default:
		return -EINVAL;
	}
	v->signal = dev->sigstrength; /* We might need to modify scaling of this
 */
	return 0;
}
static int vidioc_s_tuner(struct file *file, void *priv,
				struct v4l2_tuner *v)
{
	struct cadet *dev = video_drvdata(file);

	if (v->index != 0 && v->index != 1)
		return -EINVAL;
	dev->curtuner = v->index;
	return 0;
}

Band switching are made via g_tuner/s_tuner calls. If a device have
several non-overlapping bands, just implement it there. There's no
need for a new API.

Also, this is generic enough to cover even devices with non-standard
frequency ranges.

All bands can easily be detected via a g_tuner loop, and band switching
is done via s_tuner.

Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
and this is a way more generic than what's being proposed.

It likely makes sense to standardize the band names inside the radio core,
in order to avoid having the same band called with two different names inside
the drivers.

It should also be noticed that each band may have different properties.
On the above, the FM band can do stereo/mono and RDS, while AM is just
mono So, a change like what's proposed would keep requiring two entries.

> So an app would set as band one of DEFAULT, EUROPE_US
> (or JAPAN depending on the model) and AM_MW, and then
> get the actual range supported reported in rangelow /
> rangehigh on a subsequent G_TUNER.
> 
> Note that setting ie a band of FM_JAPAN on a 5757 would
> result in the S_TUNER failing with -EINVAL.
> 
>>
>>
>>>
>>>    /*  Flags for the 'rxsubchans' field */
>>>    #define V4L2_TUNER_SUB_MONO        0x0001
>>> @@ -2065,6 +2072,14 @@ struct v4l2_modulator {
>>>    #define V4L2_TUNER_MODE_LANG1        0x0003
>>>    #define V4L2_TUNER_MODE_LANG1_LANG2    0x0004
>>>
>>> +/*  Values for the 'band' field */
>>> +#define V4L2_TUNER_BAND_DEFAULT       0
>>
>> What does "default" mean?
> 
> Default means default. This is for compatibility with
> old apps which don't know about the new tuner band API
> extension so they will set this field to 0 (as reserved
> fields should be set to 0 by userspace). In this case
> we don't want to fail with -EINVAL based on the band
> value, so we need some value all tuners will accept.
> 
> Some tuners, ie the si470x support both selecting a
> specific FM band, as well as selecting a "universal"
> FM band of 76 - 108 MHz. For those default would be
> the universal FM band. For the tea575x devices discussed
> above default would have the range of whatever FM band
> they support.
> 
> Note that even on devices with a universal band being
> able to select a certain band is quite useful to limit
> hardware freq-seek to this band since searching freqs
> below 87.5 is useless in europe / US for example.
> 
> Thinking more about this I think we should rename
> V4L2_TUNER_BAND_DEFAULT to V4L2_TUNER_BAND_FM_UNIVERSAL,
> and document that this means the widest FM band the
> device supports, with the actual limits being reported
> in rangelow and rangehigh. Note that the mentioned ranges
> by the bands are indications of the expected range only
> the true range will still be reported through rangelow and
> rangehigh, and this is what apps are expected to use.
> 
> Defining 0 as V4L2_TUNER_BAND_FM_UNIVERSAL does cause
> a -EINVAL when doing a S_TUNER with a band value of 0
> on AM only tuners, but:
> 1) We don't support AM only tuners atm, and I don't expect
> we will in the future either
> 2) Non band aware apps don't work well with AM tuners anyways
> (as they must take much smaller frequency steps for one).
> 
>>
>>> +#define V4L2_TUNER_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
>>
>> EUROPE_US is a bad name for this range. According with Wikipedia, this
>> range is used at "ITU region 1" (Europe/Africa), while America uses
>> ITU region 2 (88-108).
>>
>> In Brazil, the range from 87.5-88 were added several years ago, so it is
>> currently at the "ITU region 1" range, just like in US.
>>
>> I don't doubt that there are still some places at the 88-108 MHz range.
> 
> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
> creating 2 band defines for this.

Yes, it is very close, but Countries that added the extra 500 kHz bandwidth
added stations there. On those, older devices can't tune into the new channels.

In the city I used to live, two channels were added when the range got
extended, one at 87.5, and another at 87.9.

>>
>>> +#define V4L2_TUNER_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
>>
>> This is currently true, but wikipedia points that they may increase it
>> (from 76MHz to 108MHz?) after the end of NTSC broadcast.
>>
> 
> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
> thus going all the way from 76 - 163 Mhz, so I guess we should add a
> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
> supports this, and indeed calls it "Japan wide band"

That's why giving them name via defines is a bad thing: the concept of 
"universal" changes from time to time: 15 years ago, an "universal" radio
is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM (88-108MHz).

An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
it is now 76-163 Mhz.

If a band like that is described as "FM" with a frequency range from 76
to 163 MHz, this is clearer than calling it as "FM unversal".

>> The DTV range there starts at channel 14 (473 MHz and upper). Maybe they
>> may reserve the channel 7-13 range (VHF High - starting at 177 MHz) like
>> Brazil for DTV.
>>
>> Anyway, what I mean is that calling a frequency range with a Country name
>> is dangerous, as frequency ranges can vary from time to time.
>>
> 
> So lets get back to the basis, for AM/FM switching / limiting hw-freq
> seeking, and on some devices likely even just to be able to tune to
> certain frequencies we need to select a band with various radio devices.
> 
> On some radio devices we may be able to just program the seek range, but on
> most it is hardcoded based on a band selection register.

Except due to regulatory requirements, the driver could just expose the
broadest range. That's what I did with tea5767, as it allows using either
an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.

> So we need some way of naming the bands, with approx. expected ranges
> (the real range supported by the specific device will be reported on a
> G_TUNER).
> 
> Looking at:
> http://en.wikipedia.org/wiki/FM_broadcast_band
> 
> I suggest naming the bands after their standards, except for the Japanese
> bands which are special and I suggest just naming them after their
> country, resulting in:
> 
> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */

CCIR is a bad (and obsolete) name. 

It is a bad name because it is the name of the Radio committee of the ITU,
and this committee standardizes all radio ranges, not only the above.

It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].

Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several ranges there:
	87.5-108
	88-108
	88-100		(Norway)
	66-73		(Gambia)
	66-74		(Lithuania)
	87.8-108	(US)
	100-108		(India)
	76-90		(Japan)
	...

[1] http://en.wikipedia.org/wiki/ITU-R
[2] http://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.450-3-200111-I!!MSW-E.doc

> #define V4L2_TUNER_BAND_FM_OIRT        2 /* 65.8 MHz - 74 MHz */
> #define V4L2_TUNER_BAND_JAPAN        3 /* 76 MHz - 90 MHz */
> #define V4L2_TUNER_BAND_JAPAN_WIDE    4 /* 76 MHz - 108 MHz */
> #define V4L2_TUNER_BAND_WEATHER        5 /* 162.4 MHz - 162.55 MHz */
> 
> Note for rationale of the weather band, see:
> http://en.wikipedia.org/wiki/Weather_radio
> 
> Regards,
> 
> Hans

Regards,
Mauro

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
> Em 19-06-2012 05:27, Hans de Goede escreveu:
>> Hi,
>>
>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>     include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>> index 2339678..013ee46 100644
>>>> --- a/include/linux/videodev2.h
>>>> +++ b/include/linux/videodev2.h
>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>         __u32            audmode;
>>>>         __s32            signal;
>>>>         __s32            afc;
>>>> -    __u32            reserved[4];
>>>> +    __u32            band;
>>>> +    __u32            reserved[3];
>>>>     };
>>>>
>>>>     struct v4l2_modulator {
>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>         __u32            rangelow;
>>>>         __u32            rangehigh;
>>>>         __u32            txsubchans;
>>>> -    __u32            reserved[4];
>>>> +    __u32            band;
>>>> +    __u32            reserved[3];
>>>>     };
>>>>
>>>>     /*  Flags for the 'capability' field */
>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>     #define V4L2_TUNER_CAP_RDS        0x0080
>>>>     #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>     #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>
>>> Frequency band is already specified by rangelow/rangehigh.
>>>
>>> Why do you need to duplicate this information?
>>
>> Because radio tuners may support multiple non overlapping
>> bands, this is why this patch also adds a band member
>> to the tuner struct, which can be used to set/get
>> the current band.
>>
>> One example of this are the tea5757 / tea5759
>> radio tuner chips:
>>
>> FM:
>> tea5757 87.5 - 108 MHz
>
> 	rangelow = 87.5 * 62500;
> 	rangehigh = 108 * 62500;
>
>> tea5759 76 - 91 MHz
>
> 	rangelow = 76 * 62500;
> 	rangehigh = 91 * 62500;
>
>> AM:
>> Both: 530 - 1710 kHz
>
> 	rangelow = 0.530 * 62500;
> 	rangehigh = 0.1710 * 62500;
>
>
> See radio-cadet.c:
>
> static int vidioc_g_tuner(struct file *file, void *priv,
> 				struct v4l2_tuner *v)
> {
> 	struct cadet *dev = video_drvdata(file);
>
> 	v->type = V4L2_TUNER_RADIO;
> 	switch (v->index) {
> 	case 0:
> 		strlcpy(v->name, "FM", sizeof(v->name));
> 		v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
> 			V4L2_TUNER_CAP_RDS_BLOCK_IO;
> 		v->rangelow = 1400;     /* 87.5 MHz */
> 		v->rangehigh = 1728;    /* 108.0 MHz */
> 		v->rxsubchans = cadet_getstereo(dev);
> 		switch (v->rxsubchans) {
> 		case V4L2_TUNER_SUB_MONO:
> 			v->audmode = V4L2_TUNER_MODE_MONO;
> 			break;
> 		case V4L2_TUNER_SUB_STEREO:
> 			v->audmode = V4L2_TUNER_MODE_STEREO;
> 			break;
> 		default:
> 			break;
> 		}
> 		v->rxsubchans |= V4L2_TUNER_SUB_RDS;
> 		break;
> 	case 1:
> 		strlcpy(v->name, "AM", sizeof(v->name));
> 		v->capability = V4L2_TUNER_CAP_LOW;
> 		v->rangelow = 8320;      /* 520 kHz */
> 		v->rangehigh = 26400;    /* 1650 kHz */
> 		v->rxsubchans = V4L2_TUNER_SUB_MONO;
> 		v->audmode = V4L2_TUNER_MODE_MONO;
> 		break;
> 	default:
> 		return -EINVAL;
> 	}
> 	v->signal = dev->sigstrength; /* We might need to modify scaling of this
>   */
> 	return 0;
> }
> static int vidioc_s_tuner(struct file *file, void *priv,
> 				struct v4l2_tuner *v)
> {
> 	struct cadet *dev = video_drvdata(file);
>
> 	if (v->index != 0 && v->index != 1)
> 		return -EINVAL;
> 	dev->curtuner = v->index;
> 	return 0;
> }
>
> Band switching are made via g_tuner/s_tuner calls. If a device have
> several non-overlapping bands, just implement it there. There's no
> need for a new API.

<sigh>, this has been discussed extensively between me, Hans V and
Halli Manjunatha on both irc and on the list. What the cadet driver is
doing is an ugly hack, and really a poor match for what we want.

Not to mention that it is a clear violation of the v4l2 spec:
http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html

"Radio input devices have exactly one tuner with index zero, no video inputs."

So there is supposed to be only one tuner, and s_tuner / g_tuner
on radio devices always expect a tuner index of 0.

Also from the same page:
"Note that VIDIOC_S_TUNER does not switch the current tuner, when there is more than one at all."

So if we model discontinuous ranges as multiple tuners how do we
select the right tuner? Certainly *not* though s_tuner, as that would
violate the spec. Note that changing the spec here is not really an option,
S_TUNER is expected to change the properties of the tuner selected through
the index, and is *not* expected to change the active tuner , esp. since
changing the active tuner would raise the question, change the active tuner
for which input ? The spec is clear on this:
"The tuner is solely determined by the current video input."

iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
but it does not select a tuner. Making s_tuner actually select 1 of multiple
tuners for radio devices, would cause a large discrepancy between radio and
tv tuners.

For tv tuners we've a 1:1 mapping between tuners and inputs, which makes sense, because
there are actual dual tuner devices, and the purpose of those is to be able to watch /
record 2 "shows" at the same time. This is simply not the case with these radio devices,
they can tune both AM and FM but *not* at the same time, so they have a *single*
*multiple-band* tuner.

Modeling this as multiple tuners is just wrong. Not only have we already discussed
this in a long discussion, I've patches to extend the tea575x driver with AM support,
and the initial revision used the multiple tuner model, but that just does not work
well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's patchset for
limiting hw-freq seek ranges, after which all of this has been discussed extensively!

> Also, this is generic enough to cover even devices with non-standard
> frequency ranges.
>
> All bands can easily be detected via a g_tuner loop, and band switching
> is done via s_tuner.
>
> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
> and this is a way more generic than what's being proposed.

It is also very very wrong, there is only a single tuner on these devices,
modeling this as multiple tuners is just wrong!

> It likely makes sense to standardize the band names inside the radio core,
> in order to avoid having the same band called with two different names inside
> the drivers.
>
> It should also be noticed that each band may have different properties.
> On the above, the FM band can do stereo/mono and RDS, while AM is just
> mono So, a change like what's proposed would keep requiring two entries.

With FM we already have a situation where some channels are mono and other
stereo, with AM/FM the tuner capabilities would reflect what the tuner can
do on some bands-frequency combinations, just like it now reflects what
it can do on some frequencies.

<snip>

>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>> creating 2 band defines for this.
>
> Yes, it is very close, but Countries that added the extra 500 kHz bandwidth
> added stations there. On those, older devices can't tune into the new channels.

On those older devices rangelow would get reported as 88 rather then 87.5, the
band selection mechanism is there to select a certain range approximately,
the exact resulting range will be hw specific and reported in rangelow /'
rangehigh, as the patch documenting the new fields clearly documents.

<snip>

>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>> supports this, and indeed calls it "Japan wide band"
>
> That's why giving them name via defines is a bad thing: the concept of
> "universal" changes from time to time: 15 years ago, an "universal" radio
> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM (88-108MHz).
>
> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
> it is now 76-163 Mhz.
>
> If a band like that is described as "FM" with a frequency range from 76
> to 163 MHz, this is clearer than calling it as "FM unversal".

We will still have rangelow and rangehigh to report the actual implemented
band. So there is no problem here. An app can select universal and then
figure out what universal is on the specific device it is using with a
G_TUNER.

<snip>

>> So lets get back to the basis, for AM/FM switching / limiting hw-freq
>> seeking, and on some devices likely even just to be able to tune to
>> certain frequencies we need to select a band with various radio devices.
>>
>> On some radio devices we may be able to just program the seek range, but on
>> most it is hardcoded based on a band selection register.
>
> Except due to regulatory requirements, the driver could just expose the
> broadest range. That's what I did with tea5767, as it allows using either
> an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.
>
>> So we need some way of naming the bands, with approx. expected ranges
>> (the real range supported by the specific device will be reported on a
>> G_TUNER).
>>
>> Looking at:
>> http://en.wikipedia.org/wiki/FM_broadcast_band
>>
>> I suggest naming the bands after their standards, except for the Japanese
>> bands which are special and I suggest just naming them after their
>> country, resulting in:
>>
>> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */
>
> CCIR is a bad (and obsolete) name.

Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
be what most of the world is either using or moving too (most of the
former USSR has also moved to a range of 87.5 - 108, rather then the
OIRT bands).

> It is a bad name because it is the name of the Radio committee of the ITU,
> and this committee standardizes all radio ranges, not only the above.
>
> It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].
>
> Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several ranges there:
> 	87.5-108
> 	88-108
> 	88-100		(Norway)

Standard

> 	66-73		(Gambia)
> 	66-74		(Lithuania)

OIRT

> 	87.8-108	(US)
> 	100-108		(India)

Standard

> 	76-90		(Japan)

Japan

Note that currently several drivers already implement a band concept in some
way, ie in the tea5767 driver, you expose this through a config flag called japan_band,
and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
with the japan_band flag set to 0, resulting in not getting the wide band, but the
small band, and thus likely not working in japan. Also note that since the tea5767
radio tuner driver uses the standard tuner framework, it reports a hardcoded range
of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of the
japan_band parameter.

The si470x driver has a band *module* parameter instead, note though that in both cases
the (average) user ends up with a hardcoded band, where he should be able to adjust it
to match the country/regio he is in...

So we really need some way to enumerate and set radio-bands, not radio-tuners, but
radio-bands, and that is exactly what the proposed API gives us in a nice and simple
way.

Regards,

Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[halli manjunatha]

Hi Mauro,

Please take the Patch-set 7 which I submitted by removing my set_band
implementation (as per Hans V suggestion).

https://lkml.org/lkml/2012/5/21/294

Regards
Manju

On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
>>
>> Em 19-06-2012 05:27, Hans de Goede escreveu:
>>>
>>> Hi,
>>>
>>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>>>
>>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>>>
>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>
>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>    include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>>    1 file changed, 17 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>> index 2339678..013ee46 100644
>>>>> --- a/include/linux/videodev2.h
>>>>> +++ b/include/linux/videodev2.h
>>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>>        __u32            audmode;
>>>>>        __s32            signal;
>>>>>        __s32            afc;
>>>>> -    __u32            reserved[4];
>>>>> +    __u32            band;
>>>>> +    __u32            reserved[3];
>>>>>    };
>>>>>
>>>>>    struct v4l2_modulator {
>>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>>        __u32            rangelow;
>>>>>        __u32            rangehigh;
>>>>>        __u32            txsubchans;
>>>>> -    __u32            reserved[4];
>>>>> +    __u32            band;
>>>>> +    __u32            reserved[3];
>>>>>    };
>>>>>
>>>>>    /*  Flags for the 'capability' field */
>>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>>    #define V4L2_TUNER_CAP_RDS        0x0080
>>>>>    #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>>    #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>>
>>>>
>>>> Frequency band is already specified by rangelow/rangehigh.
>>>>
>>>> Why do you need to duplicate this information?
>>>
>>>
>>> Because radio tuners may support multiple non overlapping
>>> bands, this is why this patch also adds a band member
>>> to the tuner struct, which can be used to set/get
>>> the current band.
>>>
>>> One example of this are the tea5757 / tea5759
>>> radio tuner chips:
>>>
>>> FM:
>>> tea5757 87.5 - 108 MHz
>>
>>
>>        rangelow = 87.5 * 62500;
>>        rangehigh = 108 * 62500;
>>
>>> tea5759 76 - 91 MHz
>>
>>
>>        rangelow = 76 * 62500;
>>        rangehigh = 91 * 62500;
>>
>>> AM:
>>> Both: 530 - 1710 kHz
>>
>>
>>        rangelow = 0.530 * 62500;
>>        rangehigh = 0.1710 * 62500;
>>
>>
>> See radio-cadet.c:
>>
>> static int vidioc_g_tuner(struct file *file, void *priv,
>>                                struct v4l2_tuner *v)
>> {
>>        struct cadet *dev = video_drvdata(file);
>>
>>        v->type = V4L2_TUNER_RADIO;
>>        switch (v->index) {
>>        case 0:
>>                strlcpy(v->name, "FM", sizeof(v->name));
>>                v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS
>> |
>>                        V4L2_TUNER_CAP_RDS_BLOCK_IO;
>>                v->rangelow = 1400;     /* 87.5 MHz */
>>                v->rangehigh = 1728;    /* 108.0 MHz */
>>                v->rxsubchans = cadet_getstereo(dev);
>>                switch (v->rxsubchans) {
>>                case V4L2_TUNER_SUB_MONO:
>>                        v->audmode = V4L2_TUNER_MODE_MONO;
>>                        break;
>>                case V4L2_TUNER_SUB_STEREO:
>>                        v->audmode = V4L2_TUNER_MODE_STEREO;
>>                        break;
>>                default:
>>                        break;
>>                }
>>                v->rxsubchans |= V4L2_TUNER_SUB_RDS;
>>                break;
>>        case 1:
>>                strlcpy(v->name, "AM", sizeof(v->name));
>>                v->capability = V4L2_TUNER_CAP_LOW;
>>                v->rangelow = 8320;      /* 520 kHz */
>>                v->rangehigh = 26400;    /* 1650 kHz */
>>                v->rxsubchans = V4L2_TUNER_SUB_MONO;
>>                v->audmode = V4L2_TUNER_MODE_MONO;
>>                break;
>>        default:
>>                return -EINVAL;
>>        }
>>        v->signal = dev->sigstrength; /* We might need to modify scaling of
>> this
>>  */
>>        return 0;
>> }
>> static int vidioc_s_tuner(struct file *file, void *priv,
>>                                struct v4l2_tuner *v)
>> {
>>        struct cadet *dev = video_drvdata(file);
>>
>>        if (v->index != 0 && v->index != 1)
>>                return -EINVAL;
>>        dev->curtuner = v->index;
>>        return 0;
>> }
>>
>> Band switching are made via g_tuner/s_tuner calls. If a device have
>> several non-overlapping bands, just implement it there. There's no
>> need for a new API.
>
>
> <sigh>, this has been discussed extensively between me, Hans V and
> Halli Manjunatha on both irc and on the list. What the cadet driver is
> doing is an ugly hack, and really a poor match for what we want.
>
> Not to mention that it is a clear violation of the v4l2 spec:
> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
>
> "Radio input devices have exactly one tuner with index zero, no video
> inputs."
>
> So there is supposed to be only one tuner, and s_tuner / g_tuner
> on radio devices always expect a tuner index of 0.
>
> Also from the same page:
> "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is
> more than one at all."
>
> So if we model discontinuous ranges as multiple tuners how do we
> select the right tuner? Certainly *not* though s_tuner, as that would
> violate the spec. Note that changing the spec here is not really an option,
> S_TUNER is expected to change the properties of the tuner selected through
> the index, and is *not* expected to change the active tuner , esp. since
> changing the active tuner would raise the question, change the active tuner
> for which input ? The spec is clear on this:
> "The tuner is solely determined by the current video input."
>
> iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
> but it does not select a tuner. Making s_tuner actually select 1 of multiple
> tuners for radio devices, would cause a large discrepancy between radio and
> tv tuners.
>
> For tv tuners we've a 1:1 mapping between tuners and inputs, which makes
> sense, because
> there are actual dual tuner devices, and the purpose of those is to be able
> to watch /
> record 2 "shows" at the same time. This is simply not the case with these
> radio devices,
> they can tune both AM and FM but *not* at the same time, so they have a
> *single*
> *multiple-band* tuner.
>
> Modeling this as multiple tuners is just wrong. Not only have we already
> discussed
> this in a long discussion, I've patches to extend the tea575x driver with AM
> support,
> and the initial revision used the multiple tuner model, but that just does
> not work
> well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's
> patchset for
> limiting hw-freq seek ranges, after which all of this has been discussed
> extensively!
>
>
>> Also, this is generic enough to cover even devices with non-standard
>> frequency ranges.
>>
>> All bands can easily be detected via a g_tuner loop, and band switching
>> is done via s_tuner.
>>
>> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
>> and this is a way more generic than what's being proposed.
>
>
> It is also very very wrong, there is only a single tuner on these devices,
> modeling this as multiple tuners is just wrong!
>
>
>> It likely makes sense to standardize the band names inside the radio core,
>> in order to avoid having the same band called with two different names
>> inside
>> the drivers.
>>
>> It should also be noticed that each band may have different properties.
>> On the above, the FM band can do stereo/mono and RDS, while AM is just
>> mono So, a change like what's proposed would keep requiring two entries.
>
>
> With FM we already have a situation where some channels are mono and other
> stereo, with AM/FM the tuner capabilities would reflect what the tuner can
> do on some bands-frequency combinations, just like it now reflects what
> it can do on some frequencies.
>
> <snip>
>
>
>>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>>> creating 2 band defines for this.
>>
>>
>> Yes, it is very close, but Countries that added the extra 500 kHz
>> bandwidth
>> added stations there. On those, older devices can't tune into the new
>> channels.
>
>
> On those older devices rangelow would get reported as 88 rather then 87.5,
> the
> band selection mechanism is there to select a certain range approximately,
> the exact resulting range will be hw specific and reported in rangelow /'
> rangehigh, as the patch documenting the new fields clearly documents.
>
> <snip>
>
>
>>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather
>>> band,
>>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>>> supports this, and indeed calls it "Japan wide band"
>>
>>
>> That's why giving them name via defines is a bad thing: the concept of
>> "universal" changes from time to time: 15 years ago, an "universal" radio
>> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM
>> (88-108MHz).
>>
>> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
>> it is now 76-163 Mhz.
>>
>> If a band like that is described as "FM" with a frequency range from 76
>> to 163 MHz, this is clearer than calling it as "FM unversal".
>
>
> We will still have rangelow and rangehigh to report the actual implemented
> band. So there is no problem here. An app can select universal and then
> figure out what universal is on the specific device it is using with a
> G_TUNER.
>
> <snip>
>
>
>>> So lets get back to the basis, for AM/FM switching / limiting hw-freq
>>> seeking, and on some devices likely even just to be able to tune to
>>> certain frequencies we need to select a band with various radio devices.
>>>
>>> On some radio devices we may be able to just program the seek range, but
>>> on
>>> most it is hardcoded based on a band selection register.
>>
>>
>> Except due to regulatory requirements, the driver could just expose the
>> broadest range. That's what I did with tea5767, as it allows using either
>> an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.
>>
>>> So we need some way of naming the bands, with approx. expected ranges
>>> (the real range supported by the specific device will be reported on a
>>> G_TUNER).
>>>
>>> Looking at:
>>> http://en.wikipedia.org/wiki/FM_broadcast_band
>>>
>>> I suggest naming the bands after their standards, except for the Japanese
>>> bands which are special and I suggest just naming them after their
>>> country, resulting in:
>>>
>>> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */
>>
>>
>> CCIR is a bad (and obsolete) name.
>
>
> Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
> be what most of the world is either using or moving too (most of the
> former USSR has also moved to a range of 87.5 - 108, rather then the
> OIRT bands).
>
>
>> It is a bad name because it is the name of the Radio committee of the ITU,
>> and this committee standardizes all radio ranges, not only the above.
>>
>> It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].
>>
>> Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several
>> ranges there:
>>        87.5-108
>>        88-108
>>        88-100          (Norway)
>
>
> Standard
>
>>        66-73           (Gambia)
>>        66-74           (Lithuania)
>
>
> OIRT
>
>>        87.8-108        (US)
>>        100-108         (India)
>
>
> Standard
>
>>        76-90           (Japan)
>
>
> Japan
>
> Note that currently several drivers already implement a band concept in some
> way, ie in the tea5767 driver, you expose this through a config flag called
> japan_band,
> and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
> with the japan_band flag set to 0, resulting in not getting the wide band,
> but the
> small band, and thus likely not working in japan. Also note that since the
> tea5767
> radio tuner driver uses the standard tuner framework, it reports a hardcoded
> range
> of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of
> the
> japan_band parameter.
>
> The si470x driver has a band *module* parameter instead, note though that in
> both cases
> the (average) user ends up with a hardcoded band, where he should be able to
> adjust it
> to match the country/regio he is in...
>
> So we really need some way to enumerate and set radio-bands, not
> radio-tuners, but
> radio-bands, and that is exactly what the proposed API gives us in a nice
> and simple
> way.
>
> Regards,
>
> Hans
>
>
>
>
>
>
>



-- 
Regards
Halli

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Mauro Carvalho Chehab]

Em 19-06-2012 10:31, halli manjunatha escreveu:
> Hi Mauro,
> 
> Please take the Patch-set 7 which I submitted by removing my set_band
> implementation (as per Hans V suggestion).
> 
> https://lkml.org/lkml/2012/5/21/294

Manju,

That doesn't solve the issue.

As I pointed on my previous email, the ranges aren't consistent among the
radio devices. The best, IMHO, would be to use several g/s_tuner ranges,
one for each supported one.

An alternative would be to write a set of ioctls specific for radio that
would do the same that g/s_tuner does at radio-cadet, but, IMHO, this is
is overdesign.

In any case, we should not apply a patch for it without having a consensus
about the right way.

Regards,
Mauro

> 
> Regards
> Manju
> 
> On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
>>>
>>> Em 19-06-2012 05:27, Hans de Goede escreveu:
>>>>
>>>> Hi,
>>>>
>>>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>>>>
>>>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>>>>
>>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>>
>>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>>>> ---
>>>>>>     include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>>> index 2339678..013ee46 100644
>>>>>> --- a/include/linux/videodev2.h
>>>>>> +++ b/include/linux/videodev2.h
>>>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>>>         __u32            audmode;
>>>>>>         __s32            signal;
>>>>>>         __s32            afc;
>>>>>> -    __u32            reserved[4];
>>>>>> +    __u32            band;
>>>>>> +    __u32            reserved[3];
>>>>>>     };
>>>>>>
>>>>>>     struct v4l2_modulator {
>>>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>>>         __u32            rangelow;
>>>>>>         __u32            rangehigh;
>>>>>>         __u32            txsubchans;
>>>>>> -    __u32            reserved[4];
>>>>>> +    __u32            band;
>>>>>> +    __u32            reserved[3];
>>>>>>     };
>>>>>>
>>>>>>     /*  Flags for the 'capability' field */
>>>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>>>     #define V4L2_TUNER_CAP_RDS        0x0080
>>>>>>     #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>>>     #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>>>
>>>>>
>>>>> Frequency band is already specified by rangelow/rangehigh.
>>>>>
>>>>> Why do you need to duplicate this information?
>>>>
>>>>
>>>> Because radio tuners may support multiple non overlapping
>>>> bands, this is why this patch also adds a band member
>>>> to the tuner struct, which can be used to set/get
>>>> the current band.
>>>>
>>>> One example of this are the tea5757 / tea5759
>>>> radio tuner chips:
>>>>
>>>> FM:
>>>> tea5757 87.5 - 108 MHz
>>>
>>>
>>>         rangelow = 87.5 * 62500;
>>>         rangehigh = 108 * 62500;
>>>
>>>> tea5759 76 - 91 MHz
>>>
>>>
>>>         rangelow = 76 * 62500;
>>>         rangehigh = 91 * 62500;
>>>
>>>> AM:
>>>> Both: 530 - 1710 kHz
>>>
>>>
>>>         rangelow = 0.530 * 62500;
>>>         rangehigh = 0.1710 * 62500;
>>>
>>>
>>> See radio-cadet.c:
>>>
>>> static int vidioc_g_tuner(struct file *file, void *priv,
>>>                                 struct v4l2_tuner *v)
>>> {
>>>         struct cadet *dev = video_drvdata(file);
>>>
>>>         v->type = V4L2_TUNER_RADIO;
>>>         switch (v->index) {
>>>         case 0:
>>>                 strlcpy(v->name, "FM", sizeof(v->name));
>>>                 v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS
>>> |
>>>                         V4L2_TUNER_CAP_RDS_BLOCK_IO;
>>>                 v->rangelow = 1400;     /* 87.5 MHz */
>>>                 v->rangehigh = 1728;    /* 108.0 MHz */
>>>                 v->rxsubchans = cadet_getstereo(dev);
>>>                 switch (v->rxsubchans) {
>>>                 case V4L2_TUNER_SUB_MONO:
>>>                         v->audmode = V4L2_TUNER_MODE_MONO;
>>>                         break;
>>>                 case V4L2_TUNER_SUB_STEREO:
>>>                         v->audmode = V4L2_TUNER_MODE_STEREO;
>>>                         break;
>>>                 default:
>>>                         break;
>>>                 }
>>>                 v->rxsubchans |= V4L2_TUNER_SUB_RDS;
>>>                 break;
>>>         case 1:
>>>                 strlcpy(v->name, "AM", sizeof(v->name));
>>>                 v->capability = V4L2_TUNER_CAP_LOW;
>>>                 v->rangelow = 8320;      /* 520 kHz */
>>>                 v->rangehigh = 26400;    /* 1650 kHz */
>>>                 v->rxsubchans = V4L2_TUNER_SUB_MONO;
>>>                 v->audmode = V4L2_TUNER_MODE_MONO;
>>>                 break;
>>>         default:
>>>                 return -EINVAL;
>>>         }
>>>         v->signal = dev->sigstrength; /* We might need to modify scaling of
>>> this
>>>   */
>>>         return 0;
>>> }
>>> static int vidioc_s_tuner(struct file *file, void *priv,
>>>                                 struct v4l2_tuner *v)
>>> {
>>>         struct cadet *dev = video_drvdata(file);
>>>
>>>         if (v->index != 0 && v->index != 1)
>>>                 return -EINVAL;
>>>         dev->curtuner = v->index;
>>>         return 0;
>>> }
>>>
>>> Band switching are made via g_tuner/s_tuner calls. If a device have
>>> several non-overlapping bands, just implement it there. There's no
>>> need for a new API.
>>
>>
>> <sigh>, this has been discussed extensively between me, Hans V and
>> Halli Manjunatha on both irc and on the list. What the cadet driver is
>> doing is an ugly hack, and really a poor match for what we want.
>>
>> Not to mention that it is a clear violation of the v4l2 spec:
>> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
>>
>> "Radio input devices have exactly one tuner with index zero, no video
>> inputs."
>>
>> So there is supposed to be only one tuner, and s_tuner / g_tuner
>> on radio devices always expect a tuner index of 0.
>>
>> Also from the same page:
>> "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is
>> more than one at all."
>>
>> So if we model discontinuous ranges as multiple tuners how do we
>> select the right tuner? Certainly *not* though s_tuner, as that would
>> violate the spec. Note that changing the spec here is not really an option,
>> S_TUNER is expected to change the properties of the tuner selected through
>> the index, and is *not* expected to change the active tuner , esp. since
>> changing the active tuner would raise the question, change the active tuner
>> for which input ? The spec is clear on this:
>> "The tuner is solely determined by the current video input."
>>
>> iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
>> but it does not select a tuner. Making s_tuner actually select 1 of multiple
>> tuners for radio devices, would cause a large discrepancy between radio and
>> tv tuners.
>>
>> For tv tuners we've a 1:1 mapping between tuners and inputs, which makes
>> sense, because
>> there are actual dual tuner devices, and the purpose of those is to be able
>> to watch /
>> record 2 "shows" at the same time. This is simply not the case with these
>> radio devices,
>> they can tune both AM and FM but *not* at the same time, so they have a
>> *single*
>> *multiple-band* tuner.
>>
>> Modeling this as multiple tuners is just wrong. Not only have we already
>> discussed
>> this in a long discussion, I've patches to extend the tea575x driver with AM
>> support,
>> and the initial revision used the multiple tuner model, but that just does
>> not work
>> well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's
>> patchset for
>> limiting hw-freq seek ranges, after which all of this has been discussed
>> extensively!
>>
>>
>>> Also, this is generic enough to cover even devices with non-standard
>>> frequency ranges.
>>>
>>> All bands can easily be detected via a g_tuner loop, and band switching
>>> is done via s_tuner.
>>>
>>> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
>>> and this is a way more generic than what's being proposed.
>>
>>
>> It is also very very wrong, there is only a single tuner on these devices,
>> modeling this as multiple tuners is just wrong!
>>
>>
>>> It likely makes sense to standardize the band names inside the radio core,
>>> in order to avoid having the same band called with two different names
>>> inside
>>> the drivers.
>>>
>>> It should also be noticed that each band may have different properties.
>>> On the above, the FM band can do stereo/mono and RDS, while AM is just
>>> mono So, a change like what's proposed would keep requiring two entries.
>>
>>
>> With FM we already have a situation where some channels are mono and other
>> stereo, with AM/FM the tuner capabilities would reflect what the tuner can
>> do on some bands-frequency combinations, just like it now reflects what
>> it can do on some frequencies.
>>
>> <snip>
>>
>>
>>>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>>>> creating 2 band defines for this.
>>>
>>>
>>> Yes, it is very close, but Countries that added the extra 500 kHz
>>> bandwidth
>>> added stations there. On those, older devices can't tune into the new
>>> channels.
>>
>>
>> On those older devices rangelow would get reported as 88 rather then 87.5,
>> the
>> band selection mechanism is there to select a certain range approximately,
>> the exact resulting range will be hw specific and reported in rangelow /'
>> rangehigh, as the patch documenting the new fields clearly documents.
>>
>> <snip>
>>
>>
>>>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>>>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather
>>>> band,
>>>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>>>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>>>> supports this, and indeed calls it "Japan wide band"
>>>
>>>
>>> That's why giving them name via defines is a bad thing: the concept of
>>> "universal" changes from time to time: 15 years ago, an "universal" radio
>>> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM
>>> (88-108MHz).
>>>
>>> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
>>> it is now 76-163 Mhz.
>>>
>>> If a band like that is described as "FM" with a frequency range from 76
>>> to 163 MHz, this is clearer than calling it as "FM unversal".
>>
>>
>> We will still have rangelow and rangehigh to report the actual implemented
>> band. So there is no problem here. An app can select universal and then
>> figure out what universal is on the specific device it is using with a
>> G_TUNER.
>>
>> <snip>
>>
>>
>>>> So lets get back to the basis, for AM/FM switching / limiting hw-freq
>>>> seeking, and on some devices likely even just to be able to tune to
>>>> certain frequencies we need to select a band with various radio devices.
>>>>
>>>> On some radio devices we may be able to just program the seek range, but
>>>> on
>>>> most it is hardcoded based on a band selection register.
>>>
>>>
>>> Except due to regulatory requirements, the driver could just expose the
>>> broadest range. That's what I did with tea5767, as it allows using either
>>> an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.
>>>
>>>> So we need some way of naming the bands, with approx. expected ranges
>>>> (the real range supported by the specific device will be reported on a
>>>> G_TUNER).
>>>>
>>>> Looking at:
>>>> http://en.wikipedia.org/wiki/FM_broadcast_band
>>>>
>>>> I suggest naming the bands after their standards, except for the Japanese
>>>> bands which are special and I suggest just naming them after their
>>>> country, resulting in:
>>>>
>>>> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */
>>>
>>>
>>> CCIR is a bad (and obsolete) name.
>>
>>
>> Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
>> be what most of the world is either using or moving too (most of the
>> former USSR has also moved to a range of 87.5 - 108, rather then the
>> OIRT bands).
>>
>>
>>> It is a bad name because it is the name of the Radio committee of the ITU,
>>> and this committee standardizes all radio ranges, not only the above.
>>>
>>> It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].
>>>
>>> Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several
>>> ranges there:
>>>         87.5-108
>>>         88-108
>>>         88-100          (Norway)
>>
>>
>> Standard
>>
>>>         66-73           (Gambia)
>>>         66-74           (Lithuania)
>>
>>
>> OIRT
>>
>>>         87.8-108        (US)
>>>         100-108         (India)
>>
>>
>> Standard
>>
>>>         76-90           (Japan)
>>
>>
>> Japan
>>
>> Note that currently several drivers already implement a band concept in some
>> way, ie in the tea5767 driver, you expose this through a config flag called
>> japan_band,
>> and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
>> with the japan_band flag set to 0, resulting in not getting the wide band,
>> but the
>> small band, and thus likely not working in japan. Also note that since the
>> tea5767
>> radio tuner driver uses the standard tuner framework, it reports a hardcoded
>> range
>> of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of
>> the
>> japan_band parameter.
>>
>> The si470x driver has a band *module* parameter instead, note though that in
>> both cases
>> the (average) user ends up with a hardcoded band, where he should be able to
>> adjust it
>> to match the country/regio he is in...
>>
>> So we really need some way to enumerate and set radio-bands, not
>> radio-tuners, but
>> radio-bands, and that is exactly what the proposed API gives us in a nice
>> and simple
>> way.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>
>>
>>
> 
> 
> 

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[halli manjunatha]

On Tue, Jun 19, 2012 at 10:41 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 19-06-2012 10:31, halli manjunatha escreveu:
>> Hi Mauro,
>>
>> Please take the Patch-set 7 which I submitted by removing my set_band
>> implementation (as per Hans V suggestion).
>>
>> https://lkml.org/lkml/2012/5/21/294
>
> Manju,
>
> That doesn't solve the issue.
>
> As I pointed on my previous email, the ranges aren't consistent among the
> radio devices. The best, IMHO, would be to use several g/s_tuner ranges,
> one for each supported one.
>
> An alternative would be to write a set of ioctls specific for radio that
> would do the same that g/s_tuner does at radio-cadet, but, IMHO, this is
> is overdesign.
>
> In any case, we should not apply a patch for it without having a consensus
> about the right way.

I agree with you that we should not apply a patch till we come to a
conclusion about the design.

But the patches which I have sent (PATCH-SET 7) that doesn't deal with
FM band selection, instead it adds
few other features like below
        1) FM RX RDS AF turn ON/OFF
        2) FM RX De-Emphasis mode set/get
        3) FM TX Alternate Frequency set/get

So since these are other features which are not related to Band
selection I think you can merge these to K3.6 kernel.
>
> Regards,
> Mauro
>
>>
>> Regards
>> Manju
>>
>> On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>>
>>> On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
>>>>
>>>> Em 19-06-2012 05:27, Hans de Goede escreveu:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>>>>>
>>>>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>>>>>
>>>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>>>
>>>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>     include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>>>> index 2339678..013ee46 100644
>>>>>>> --- a/include/linux/videodev2.h
>>>>>>> +++ b/include/linux/videodev2.h
>>>>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>>>>         __u32            audmode;
>>>>>>>         __s32            signal;
>>>>>>>         __s32            afc;
>>>>>>> -    __u32            reserved[4];
>>>>>>> +    __u32            band;
>>>>>>> +    __u32            reserved[3];
>>>>>>>     };
>>>>>>>
>>>>>>>     struct v4l2_modulator {
>>>>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>>>>         __u32            rangelow;
>>>>>>>         __u32            rangehigh;
>>>>>>>         __u32            txsubchans;
>>>>>>> -    __u32            reserved[4];
>>>>>>> +    __u32            band;
>>>>>>> +    __u32            reserved[3];
>>>>>>>     };
>>>>>>>
>>>>>>>     /*  Flags for the 'capability' field */
>>>>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>>>>     #define V4L2_TUNER_CAP_RDS        0x0080
>>>>>>>     #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>>>>     #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>>>>
>>>>>>
>>>>>> Frequency band is already specified by rangelow/rangehigh.
>>>>>>
>>>>>> Why do you need to duplicate this information?
>>>>>
>>>>>
>>>>> Because radio tuners may support multiple non overlapping
>>>>> bands, this is why this patch also adds a band member
>>>>> to the tuner struct, which can be used to set/get
>>>>> the current band.
>>>>>
>>>>> One example of this are the tea5757 / tea5759
>>>>> radio tuner chips:
>>>>>
>>>>> FM:
>>>>> tea5757 87.5 - 108 MHz
>>>>
>>>>
>>>>         rangelow = 87.5 * 62500;
>>>>         rangehigh = 108 * 62500;
>>>>
>>>>> tea5759 76 - 91 MHz
>>>>
>>>>
>>>>         rangelow = 76 * 62500;
>>>>         rangehigh = 91 * 62500;
>>>>
>>>>> AM:
>>>>> Both: 530 - 1710 kHz
>>>>
>>>>
>>>>         rangelow = 0.530 * 62500;
>>>>         rangehigh = 0.1710 * 62500;
>>>>
>>>>
>>>> See radio-cadet.c:
>>>>
>>>> static int vidioc_g_tuner(struct file *file, void *priv,
>>>>                                 struct v4l2_tuner *v)
>>>> {
>>>>         struct cadet *dev = video_drvdata(file);
>>>>
>>>>         v->type = V4L2_TUNER_RADIO;
>>>>         switch (v->index) {
>>>>         case 0:
>>>>                 strlcpy(v->name, "FM", sizeof(v->name));
>>>>                 v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS
>>>> |
>>>>                         V4L2_TUNER_CAP_RDS_BLOCK_IO;
>>>>                 v->rangelow = 1400;     /* 87.5 MHz */
>>>>                 v->rangehigh = 1728;    /* 108.0 MHz */
>>>>                 v->rxsubchans = cadet_getstereo(dev);
>>>>                 switch (v->rxsubchans) {
>>>>                 case V4L2_TUNER_SUB_MONO:
>>>>                         v->audmode = V4L2_TUNER_MODE_MONO;
>>>>                         break;
>>>>                 case V4L2_TUNER_SUB_STEREO:
>>>>                         v->audmode = V4L2_TUNER_MODE_STEREO;
>>>>                         break;
>>>>                 default:
>>>>                         break;
>>>>                 }
>>>>                 v->rxsubchans |= V4L2_TUNER_SUB_RDS;
>>>>                 break;
>>>>         case 1:
>>>>                 strlcpy(v->name, "AM", sizeof(v->name));
>>>>                 v->capability = V4L2_TUNER_CAP_LOW;
>>>>                 v->rangelow = 8320;      /* 520 kHz */
>>>>                 v->rangehigh = 26400;    /* 1650 kHz */
>>>>                 v->rxsubchans = V4L2_TUNER_SUB_MONO;
>>>>                 v->audmode = V4L2_TUNER_MODE_MONO;
>>>>                 break;
>>>>         default:
>>>>                 return -EINVAL;
>>>>         }
>>>>         v->signal = dev->sigstrength; /* We might need to modify scaling of
>>>> this
>>>>   */
>>>>         return 0;
>>>> }
>>>> static int vidioc_s_tuner(struct file *file, void *priv,
>>>>                                 struct v4l2_tuner *v)
>>>> {
>>>>         struct cadet *dev = video_drvdata(file);
>>>>
>>>>         if (v->index != 0 && v->index != 1)
>>>>                 return -EINVAL;
>>>>         dev->curtuner = v->index;
>>>>         return 0;
>>>> }
>>>>
>>>> Band switching are made via g_tuner/s_tuner calls. If a device have
>>>> several non-overlapping bands, just implement it there. There's no
>>>> need for a new API.
>>>
>>>
>>> <sigh>, this has been discussed extensively between me, Hans V and
>>> Halli Manjunatha on both irc and on the list. What the cadet driver is
>>> doing is an ugly hack, and really a poor match for what we want.
>>>
>>> Not to mention that it is a clear violation of the v4l2 spec:
>>> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
>>>
>>> "Radio input devices have exactly one tuner with index zero, no video
>>> inputs."
>>>
>>> So there is supposed to be only one tuner, and s_tuner / g_tuner
>>> on radio devices always expect a tuner index of 0.
>>>
>>> Also from the same page:
>>> "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is
>>> more than one at all."
>>>
>>> So if we model discontinuous ranges as multiple tuners how do we
>>> select the right tuner? Certainly *not* though s_tuner, as that would
>>> violate the spec. Note that changing the spec here is not really an option,
>>> S_TUNER is expected to change the properties of the tuner selected through
>>> the index, and is *not* expected to change the active tuner , esp. since
>>> changing the active tuner would raise the question, change the active tuner
>>> for which input ? The spec is clear on this:
>>> "The tuner is solely determined by the current video input."
>>>
>>> iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
>>> but it does not select a tuner. Making s_tuner actually select 1 of multiple
>>> tuners for radio devices, would cause a large discrepancy between radio and
>>> tv tuners.
>>>
>>> For tv tuners we've a 1:1 mapping between tuners and inputs, which makes
>>> sense, because
>>> there are actual dual tuner devices, and the purpose of those is to be able
>>> to watch /
>>> record 2 "shows" at the same time. This is simply not the case with these
>>> radio devices,
>>> they can tune both AM and FM but *not* at the same time, so they have a
>>> *single*
>>> *multiple-band* tuner.
>>>
>>> Modeling this as multiple tuners is just wrong. Not only have we already
>>> discussed
>>> this in a long discussion, I've patches to extend the tea575x driver with AM
>>> support,
>>> and the initial revision used the multiple tuner model, but that just does
>>> not work
>>> well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's
>>> patchset for
>>> limiting hw-freq seek ranges, after which all of this has been discussed
>>> extensively!
>>>
>>>
>>>> Also, this is generic enough to cover even devices with non-standard
>>>> frequency ranges.
>>>>
>>>> All bands can easily be detected via a g_tuner loop, and band switching
>>>> is done via s_tuner.
>>>>
>>>> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
>>>> and this is a way more generic than what's being proposed.
>>>
>>>
>>> It is also very very wrong, there is only a single tuner on these devices,
>>> modeling this as multiple tuners is just wrong!
>>>
>>>
>>>> It likely makes sense to standardize the band names inside the radio core,
>>>> in order to avoid having the same band called with two different names
>>>> inside
>>>> the drivers.
>>>>
>>>> It should also be noticed that each band may have different properties.
>>>> On the above, the FM band can do stereo/mono and RDS, while AM is just
>>>> mono So, a change like what's proposed would keep requiring two entries.
>>>
>>>
>>> With FM we already have a situation where some channels are mono and other
>>> stereo, with AM/FM the tuner capabilities would reflect what the tuner can
>>> do on some bands-frequency combinations, just like it now reflects what
>>> it can do on some frequencies.
>>>
>>> <snip>
>>>
>>>
>>>>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>>>>> creating 2 band defines for this.
>>>>
>>>>
>>>> Yes, it is very close, but Countries that added the extra 500 kHz
>>>> bandwidth
>>>> added stations there. On those, older devices can't tune into the new
>>>> channels.
>>>
>>>
>>> On those older devices rangelow would get reported as 88 rather then 87.5,
>>> the
>>> band selection mechanism is there to select a certain range approximately,
>>> the exact resulting range will be hw specific and reported in rangelow /'
>>> rangehigh, as the patch documenting the new fields clearly documents.
>>>
>>> <snip>
>>>
>>>
>>>>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>>>>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather
>>>>> band,
>>>>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>>>>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>>>>> supports this, and indeed calls it "Japan wide band"
>>>>
>>>>
>>>> That's why giving them name via defines is a bad thing: the concept of
>>>> "universal" changes from time to time: 15 years ago, an "universal" radio
>>>> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM
>>>> (88-108MHz).
>>>>
>>>> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
>>>> it is now 76-163 Mhz.
>>>>
>>>> If a band like that is described as "FM" with a frequency range from 76
>>>> to 163 MHz, this is clearer than calling it as "FM unversal".
>>>
>>>
>>> We will still have rangelow and rangehigh to report the actual implemented
>>> band. So there is no problem here. An app can select universal and then
>>> figure out what universal is on the specific device it is using with a
>>> G_TUNER.
>>>
>>> <snip>
>>>
>>>
>>>>> So lets get back to the basis, for AM/FM switching / limiting hw-freq
>>>>> seeking, and on some devices likely even just to be able to tune to
>>>>> certain frequencies we need to select a band with various radio devices.
>>>>>
>>>>> On some radio devices we may be able to just program the seek range, but
>>>>> on
>>>>> most it is hardcoded based on a band selection register.
>>>>
>>>>
>>>> Except due to regulatory requirements, the driver could just expose the
>>>> broadest range. That's what I did with tea5767, as it allows using either
>>>> an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.
>>>>
>>>>> So we need some way of naming the bands, with approx. expected ranges
>>>>> (the real range supported by the specific device will be reported on a
>>>>> G_TUNER).
>>>>>
>>>>> Looking at:
>>>>> http://en.wikipedia.org/wiki/FM_broadcast_band
>>>>>
>>>>> I suggest naming the bands after their standards, except for the Japanese
>>>>> bands which are special and I suggest just naming them after their
>>>>> country, resulting in:
>>>>>
>>>>> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */
>>>>
>>>>
>>>> CCIR is a bad (and obsolete) name.
>>>
>>>
>>> Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
>>> be what most of the world is either using or moving too (most of the
>>> former USSR has also moved to a range of 87.5 - 108, rather then the
>>> OIRT bands).
>>>
>>>
>>>> It is a bad name because it is the name of the Radio committee of the ITU,
>>>> and this committee standardizes all radio ranges, not only the above.
>>>>
>>>> It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].
>>>>
>>>> Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several
>>>> ranges there:
>>>>         87.5-108
>>>>         88-108
>>>>         88-100          (Norway)
>>>
>>>
>>> Standard
>>>
>>>>         66-73           (Gambia)
>>>>         66-74           (Lithuania)
>>>
>>>
>>> OIRT
>>>
>>>>         87.8-108        (US)
>>>>         100-108         (India)
>>>
>>>
>>> Standard
>>>
>>>>         76-90           (Japan)
>>>
>>>
>>> Japan
>>>
>>> Note that currently several drivers already implement a band concept in some
>>> way, ie in the tea5767 driver, you expose this through a config flag called
>>> japan_band,
>>> and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
>>> with the japan_band flag set to 0, resulting in not getting the wide band,
>>> but the
>>> small band, and thus likely not working in japan. Also note that since the
>>> tea5767
>>> radio tuner driver uses the standard tuner framework, it reports a hardcoded
>>> range
>>> of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of
>>> the
>>> japan_band parameter.
>>>
>>> The si470x driver has a band *module* parameter instead, note though that in
>>> both cases
>>> the (average) user ends up with a hardcoded band, where he should be able to
>>> adjust it
>>> to match the country/regio he is in...
>>>
>>> So we really need some way to enumerate and set radio-bands, not
>>> radio-tuners, but
>>> radio-bands, and that is exactly what the proposed API gives us in a nice
>>> and simple
>>> way.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>
>



-- 
Regards
Halli

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Mauro Carvalho Chehab]

Em 19-06-2012 09:36, Hans de Goede escreveu:
> Hi,
> 
> On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
>> Em 19-06-2012 05:27, Hans de Goede escreveu:
>>> Hi,
>>>
>>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>
>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>     include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>> index 2339678..013ee46 100644
>>>>> --- a/include/linux/videodev2.h
>>>>> +++ b/include/linux/videodev2.h
>>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>>         __u32            audmode;
>>>>>         __s32            signal;
>>>>>         __s32            afc;
>>>>> -    __u32            reserved[4];
>>>>> +    __u32            band;
>>>>> +    __u32            reserved[3];
>>>>>     };
>>>>>
>>>>>     struct v4l2_modulator {
>>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>>         __u32            rangelow;
>>>>>         __u32            rangehigh;
>>>>>         __u32            txsubchans;
>>>>> -    __u32            reserved[4];
>>>>> +    __u32            band;
>>>>> +    __u32            reserved[3];
>>>>>     };
>>>>>
>>>>>     /*  Flags for the 'capability' field */
>>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>>     #define V4L2_TUNER_CAP_RDS        0x0080
>>>>>     #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>>     #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>>
>>>> Frequency band is already specified by rangelow/rangehigh.
>>>>
>>>> Why do you need to duplicate this information?
>>>
>>> Because radio tuners may support multiple non overlapping
>>> bands, this is why this patch also adds a band member
>>> to the tuner struct, which can be used to set/get
>>> the current band.
>>>
>>> One example of this are the tea5757 / tea5759
>>> radio tuner chips:
>>>
>>> FM:
>>> tea5757 87.5 - 108 MHz
>>
>>     rangelow = 87.5 * 62500;
>>     rangehigh = 108 * 62500;
>>
>>> tea5759 76 - 91 MHz
>>
>>     rangelow = 76 * 62500;
>>     rangehigh = 91 * 62500;
>>
>>> AM:
>>> Both: 530 - 1710 kHz
>>
>>     rangelow = 0.530 * 62500;
>>     rangehigh = 0.1710 * 62500;
>>
>>
>> See radio-cadet.c:
>>
>> static int vidioc_g_tuner(struct file *file, void *priv,
>>                 struct v4l2_tuner *v)
>> {
>>     struct cadet *dev = video_drvdata(file);
>>
>>     v->type = V4L2_TUNER_RADIO;
>>     switch (v->index) {
>>     case 0:
>>         strlcpy(v->name, "FM", sizeof(v->name));
>>         v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
>>             V4L2_TUNER_CAP_RDS_BLOCK_IO;
>>         v->rangelow = 1400;     /* 87.5 MHz */
>>         v->rangehigh = 1728;    /* 108.0 MHz */
>>         v->rxsubchans = cadet_getstereo(dev);
>>         switch (v->rxsubchans) {
>>         case V4L2_TUNER_SUB_MONO:
>>             v->audmode = V4L2_TUNER_MODE_MONO;
>>             break;
>>         case V4L2_TUNER_SUB_STEREO:
>>             v->audmode = V4L2_TUNER_MODE_STEREO;
>>             break;
>>         default:
>>             break;
>>         }
>>         v->rxsubchans |= V4L2_TUNER_SUB_RDS;
>>         break;
>>     case 1:
>>         strlcpy(v->name, "AM", sizeof(v->name));
>>         v->capability = V4L2_TUNER_CAP_LOW;
>>         v->rangelow = 8320;      /* 520 kHz */
>>         v->rangehigh = 26400;    /* 1650 kHz */
>>         v->rxsubchans = V4L2_TUNER_SUB_MONO;
>>         v->audmode = V4L2_TUNER_MODE_MONO;
>>         break;
>>     default:
>>         return -EINVAL;
>>     }
>>     v->signal = dev->sigstrength; /* We might need to modify scaling of this
>>   */
>>     return 0;
>> }
>> static int vidioc_s_tuner(struct file *file, void *priv,
>>                 struct v4l2_tuner *v)
>> {
>>     struct cadet *dev = video_drvdata(file);
>>
>>     if (v->index != 0 && v->index != 1)
>>         return -EINVAL;
>>     dev->curtuner = v->index;
>>     return 0;
>> }
>>
>> Band switching are made via g_tuner/s_tuner calls. If a device have
>> several non-overlapping bands, just implement it there. There's no
>> need for a new API.
> 
> <sigh>, this has been discussed extensively between me, Hans V and
> Halli Manjunatha on both irc and on the list. What the cadet driver is
> doing is an ugly hack, and really a poor match for what we want.
> 
> Not to mention that it is a clear violation of the v4l2 spec:
> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
> 
> "Radio input devices have exactly one tuner with index zero, no video inputs."
> 
> So there is supposed to be only one tuner, and s_tuner / g_tuner
> on radio devices always expect a tuner index of 0.
> 
> Also from the same page:
> "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is more than one at all."
> 
> So if we model discontinuous ranges as multiple tuners how do we
> select the right tuner? Certainly *not* though s_tuner, as that would
> violate the spec. Note that changing the spec here is not really an option,
> S_TUNER is expected to change the properties of the tuner selected through
> the index, and is *not* expected to change the active tuner , esp. since
> changing the active tuner would raise the question, change the active tuner
> for which input ? The spec is clear on this:
> "The tuner is solely determined by the current video input."

Well the specs need to be changed anyway, as there's no "video input" on a radio
device.

As I said several times, we need to have a "profiles" section at the V4L2 API
saying how ioctls should be implemented by each type of device: radio, tv tuners,
webcams, media-based cameras, etc. Without that, API compilance is not really
possible.

A clear example is that the omap3/s5p drivers don't work with an existent V4L2
API, as they don't implement video inputs via V4L2 API. The video input selection is
via the media API.

> iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
> but it does not select a tuner. Making s_tuner actually select 1 of multiple
> tuners for radio devices, would cause a large discrepancy between radio and
> tv tuners.

So what? This is a very small discrepancy if you compare with what we currently have
with omap3/s5p, where there's no way for a generic userspace application to work
with those devices, and a libv4l generic plugin to properly implement the video input
selection is still a dream.

It doesn't make any sense to implement video input selection on radio devices, as
radio doesn't have video. So, g_input/s_input should not be implemented there at all.

I think they're implemented with some bogus code, as otherwise some userspace apps would
break - but it doesn't make any sense to select a video input on a device without video!!!

Ok, hybrid radio/TV devices may have an "input" for FM, that actually selects the "no-video"
video input, but this is there only because, in the past, it was allowed to get radio using
the /dev/video0 device node. Thankfully, we get rid of this weird behavior on a few kernel
versions ago.

> For tv tuners we've a 1:1 mapping between tuners and inputs, which makes sense, because
> there are actual dual tuner devices, and the purpose of those is to be able to watch /
> record 2 "shows" at the same time. 

No, there isn't an 1:1 mapping. A typical TV tuner has several inputs (TV, S-Video,
Composite 1, ...) plus one radio "video" input.

They also have several audio inputs (managed via the audio routing ioctl's). On some
devices, it is even possible to select a TV channel while listening to the FM radio
(devices with tea5767 tuner). Of course, this is weird and never officially supported.

Most of the hybrid radio/TV devices actually have a single tuner that can be used 
at the VHF frequencies. So, they allow getting FM, as it is part of their tuning range.

Even so, they're mapped as 2 separate tuners, one for TV and another for radio.

> Modeling this as multiple tuners is just wrong.

It is just the way it is since the beginning of V4L2: TV range is mapped as one tuner;
FM radio is mapped as another one.

The cadet radio is one of the few devices that have FM and AM. It basically followed the
same model already adopted by bttv, cx88, saa7134, ... devices with radio support.

A change from that model will require changes at the radio implementation on all
TV drivers, in order to prevent them to use a separate tuner for radio.

The struct v4l2_tuner/v4l2_modulator structs would likely need to be converted into
something else or passed as an array, as each tuning band usage (TV, AM, FM, Weather,
digital FM, digital AM) can have different properties:
	- range low/high;
	- modulation (AM, FM, ...);
	- sub-carriers (mono, stereo, lang1, lang2);
	- properties (RDS, seek caps, ...).

> This is simply not the case with these radio devices,
> they can tune both AM and FM but *not* at the same time, so they have a *single*
> *multiple-band* tuner.

A chip with both AM and FM tuners are, internally, a dual tuner. Anyway, on most
devices, there's just a single dual-channel audio output. So, even on devices with
2 independent tuners, users can't really use both independently. There are, of course
exceptions (ivtv devices can likely record a TV show while listening to radio, as they
can use the MPEG encoder for TV).

> Not only have we already discussed
> this in a long discussion, I've patches to extend the tea575x driver with AM support,
> and the initial revision used the multiple tuner model, but that just does not work
> well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's patchset for
> limiting hw-freq seek ranges, after which all of this has been discussed extensively!

Sorry but I missed this discussion.

>> Also, this is generic enough to cover even devices with non-standard
>> frequency ranges.
>>
>> All bands can easily be detected via a g_tuner loop, and band switching
>> is done via s_tuner.
>>
>> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
>> and this is a way more generic than what's being proposed.
> 
> It is also very very wrong, there is only a single tuner on these devices,
> modeling this as multiple tuners is just wrong!
> 
>> It likely makes sense to standardize the band names inside the radio core,
>> in order to avoid having the same band called with two different names inside
>> the drivers.
>>
>> It should also be noticed that each band may have different properties.
>> On the above, the FM band can do stereo/mono and RDS, while AM is just
>> mono So, a change like what's proposed would keep requiring two entries.
> 
> With FM we already have a situation where some channels are mono and other
> stereo, with AM/FM the tuner capabilities would reflect what the tuner can
> do on some bands-frequency combinations, just like it now reflects what
> it can do on some frequencies.

Mixing an AM tuner with an FM tuner is really really wrong. Only the PLL
stage is identical.

The AM demodulator is generally just an envelope detector, while the FM 
demodulator is a way more complex and completely different from what's
done with AM.

Digital FM band and digital AM band radio is also completely different from
analog AM/analog FM. The only thing in comon with "standard" AM/FM is the
band.

The fact that all 5 types of tuner (TV, analog AM, analog FM, digital AM band,
digital FM band) are implemented by just one PLL or not is irrelevant. Each
one is a different tuner, as the tuning demodulation is different.

What I'm saying is that just adding a "band" field inside a single tuner
struct is plain wrong, as each type has different properties.

> 
> <snip>
> 
>>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>>> creating 2 band defines for this.
>>
>> Yes, it is very close, but Countries that added the extra 500 kHz bandwidth
>> added stations there. On those, older devices can't tune into the new channels.
> 
> On those older devices rangelow would get reported as 88 rather then 87.5, the
> band selection mechanism is there to select a certain range approximately,
> the exact resulting range will be hw specific and reported in rangelow /'
> rangehigh, as the patch documenting the new fields clearly documents.

Why to implement a "band" field that:
	1) can provide a wrong information (87.5 instead of 88);
	2) duplicates an existing information implemented at rangelow/rangehigh
?

> 
> <snip>
> 
>>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
>>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>>> supports this, and indeed calls it "Japan wide band"
>>
>> That's why giving them name via defines is a bad thing: the concept of
>> "universal" changes from time to time: 15 years ago, an "universal" radio
>> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM (88-108MHz).
>>
>> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
>> it is now 76-163 Mhz.
>>
>> If a band like that is described as "FM" with a frequency range from 76
>> to 163 MHz, this is clearer than calling it as "FM unversal".
> 
> We will still have rangelow and rangehigh to report the actual implemented
> band. So there is no problem here. An app can select universal and then
> figure out what universal is on the specific device it is using with a
> G_TUNER.

If rangelow/rangehigh is the actual band, why does it need something else?

Reusing G_TUNER/S_TUNER or not, the issue is that a bitfield parameter for
frequency range is not actually able to express what are the supported
ranges. As I said before, the tuner ranges can only be properly expressed 
by an array with:
	- range low/high;
	- modulation (AM, FM, ...);
	- sub-carriers (mono, stereo, lang1, lang2);
	- properties (RDS, seek caps, ...).

> 
> <snip>
> 
>>> So lets get back to the basis, for AM/FM switching / limiting hw-freq
>>> seeking, and on some devices likely even just to be able to tune to
>>> certain frequencies we need to select a band with various radio devices.
>>>
>>> On some radio devices we may be able to just program the seek range, but on
>>> most it is hardcoded based on a band selection register.
>>
>> Except due to regulatory requirements, the driver could just expose the
>> broadest range. That's what I did with tea5767, as it allows using either
>> an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.
>>
>>> So we need some way of naming the bands, with approx. expected ranges
>>> (the real range supported by the specific device will be reported on a
>>> G_TUNER).
>>>
>>> Looking at:
>>> http://en.wikipedia.org/wiki/FM_broadcast_band
>>>
>>> I suggest naming the bands after their standards, except for the Japanese
>>> bands which are special and I suggest just naming them after their
>>> country, resulting in:
>>>
>>> #define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */
>>
>> CCIR is a bad (and obsolete) name.
> 
> Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
> be what most of the world is either using or moving too (most of the
> former USSR has also moved to a range of 87.5 - 108, rather then the
> OIRT bands).
> 
>> It is a bad name because it is the name of the Radio committee of the ITU,
>> and this committee standardizes all radio ranges, not only the above.
>>
>> It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].
>>
>> Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several ranges there:
>>     87.5-108
>>     88-108
>>     88-100        (Norway)
> 
> Standard
> 
>>     66-73        (Gambia)
>>     66-74        (Lithuania)
> 
> OIRT
> 
>>     87.8-108    (US)
>>     100-108        (India)
> 
> Standard
> 
>>     76-90        (Japan)
> 
> Japan
> 
> Note that currently several drivers already implement a band concept in some
> way, ie in the tea5767 driver, you expose this through a config flag called japan_band,
> and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
> with the japan_band flag set to 0, resulting in not getting the wide band, but the
> small band, and thus likely not working in japan. Also note that since the tea5767
> radio tuner driver uses the standard tuner framework, it reports a hardcoded range
> of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of the
> japan_band parameter.
> 
> The si470x driver has a band *module* parameter instead, note though that in both cases
> the (average) user ends up with a hardcoded band, where he should be able to adjust it
> to match the country/regio he is in...
> 
> So we really need some way to enumerate and set radio-bands, not radio-tuners, but
> radio-bands, and that is exactly what the proposed API gives us in a nice and simple
> way.

I agree with the idea of reporting the supported bands, where it makes sense. I just 
don't agree with the proposed implementation that "rounds" the bandwidth into some 
bitfields that aren't capable of properly explaining what the hardware supports.

For example:

On tea5767, there are two supported ranges (using the datasheet names):
	"Japan range": 76-108 MHz
	"US/Europe range": 87.5-108 Mhz

On tea5761, there are also two supported ranges, but they're different:
	"Japan range": 76-91 MHz
	"US/Europe range": 87.5-108MHz

A FM1236 MK3 tuner supports 3 ranges:
	55.25 to 160 MHz
	160 MHz to 442 MHz
	442 MHz to 801.25 MHz

The selection between each range is done via 3 bits that selects the PLL range.

The datasheet says that the FM band there is 87.5-108 MHz, but it can actually accept
and it is tested/used in Russia for receiving the FM band - The PLL is just set to
the first range, e. g. 55.25 to 160 MHz. The only difference is that, with FM, the IF
frequency is 10.5 MHz, while with TV, it is standard-dependent).

While I never tested, I don't doubt that this device is capable of decoding FM on
all 3 PLL bands (so, from 55.25 to 801.25 MHz).

As you may see, on those 3 drivers, just one has a range that patches your definition,
and this is just for the "US/Europe" - e. g. the "standard" range.

The Japanese range doesn't match any of the above. Also, none of your above definitions
match the FM1236 MK3 tuner range.

That's why is is evil to use an enum/bitfield to map the range: it is really a
range low/range high pair of values, and the actual range is device-specific.

Regards,
Mauro

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

<snip long discussion about having a fixed set of bands versus
  a way to enumerate bands, including their rangelow, rangehigh
  and capabilities>

Ok, you've convinced me. I agree that having a way to actually
enumerate ranges, rather then having a fixed set of ranges, is
better.

Which brings us back many weeks to the proposal for making
it possible to enumerate bands on radio devices. Rather
then digging up the old mails lets start anew, I propose
the following API for this:

1. A radio device can have multiple tuners, but only 1 can
be active (streaming audio to the associated audio input)
at the same time.

2. Radio device tuners are enumerated by calling G_TUNER
with an increasing index until EINVAL gets returned

3. G_FREQUENCY will always return the frequency and index
of the currently active tuner

4. When calling S_TUNER on a radio device, the active
tuner will be set to the v4l2_tuner index field

5. When calling S_FREQUENCY on a radio device, the active
tuner will be set to the v4l2_frequency tuner field

6. On a G_TUNER call on a radio device the rxsubchans,
audmode, signal and afc v4l2_tuner fields are only
filled on for the active tuner (as returned by
G_FREQUENCY) for inactive tuners these fields are reported
as 0.


This is pretty close to what the cadet driver currently does,
the differences are point 5 and 6, currently wrt point 5 the
cadet driver just ignores the tuner field, and wrt point 6
it always tries to fill in these fields, reporting ie the
FM signal strength when the FM tuner is active as signal
for the AM tuner too.

Regards,

Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

On 06/19/2012 06:47 PM, Hans de Goede wrote:
> Hi,
>
> <snip long discussion about having a fixed set of bands versus
> a way to enumerate bands, including their rangelow, rangehigh
> and capabilities>
>
> Ok, you've convinced me. I agree that having a way to actually
> enumerate ranges, rather then having a fixed set of ranges, is
> better.
>
> Which brings us back many weeks to the proposal for making
> it possible to enumerate bands on radio devices. Rather
> then digging up the old mails lets start anew, I propose
> the following API for this:
>
> 1. A radio device can have multiple tuners, but only 1 can
> be active (streaming audio to the associated audio input)
> at the same time.
>
> 2. Radio device tuners are enumerated by calling G_TUNER
> with an increasing index until EINVAL gets returned
>
> 3. G_FREQUENCY will always return the frequency and index
> of the currently active tuner
>
> 4. When calling S_TUNER on a radio device, the active
> tuner will be set to the v4l2_tuner index field
>
> 5. When calling S_FREQUENCY on a radio device, the active
> tuner will be set to the v4l2_frequency tuner field
>
> 6. On a G_TUNER call on a radio device the rxsubchans,
> audmode, signal and afc v4l2_tuner fields are only
> filled on for the active tuner (as returned by
> G_FREQUENCY) for inactive tuners these fields are reported
> as 0.

p.s.

I forgot:

7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active
tuner will be set to the v4l2_hw_freq_seek tuner field

8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK,
the current frequency may be changed to fit in the range of the
new active tuner

9. For backwards compatibility reasons tuner 0 should be the tuner
with the broadest possible FM range

Regards,

Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[halli manjunatha]

On Tue, Jun 19, 2012 at 12:33 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 06/19/2012 06:47 PM, Hans de Goede wrote:
>>
>> Hi,
>>
>> <snip long discussion about having a fixed set of bands versus
>> a way to enumerate bands, including their rangelow, rangehigh
>> and capabilities>
>>
>> Ok, you've convinced me. I agree that having a way to actually
>> enumerate ranges, rather then having a fixed set of ranges, is
>> better.
>>
>> Which brings us back many weeks to the proposal for making
>> it possible to enumerate bands on radio devices. Rather
>> then digging up the old mails lets start anew, I propose
>> the following API for this:
>>
>> 1. A radio device can have multiple tuners, but only 1 can
>> be active (streaming audio to the associated audio input)
>> at the same time.
>>
>> 2. Radio device tuners are enumerated by calling G_TUNER
>> with an increasing index until EINVAL gets returned
>>
>> 3. G_FREQUENCY will always return the frequency and index
>> of the currently active tuner
>>
>> 4. When calling S_TUNER on a radio device, the active
>> tuner will be set to the v4l2_tuner index field
>>
>> 5. When calling S_FREQUENCY on a radio device, the active
>> tuner will be set to the v4l2_frequency tuner field
>>
>> 6. On a G_TUNER call on a radio device the rxsubchans,
>> audmode, signal and afc v4l2_tuner fields are only
>> filled on for the active tuner (as returned by
>> G_FREQUENCY) for inactive tuners these fields are reported
>> as 0.
>
>
> p.s.
>
> I forgot:
>
> 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active
> tuner will be set to the v4l2_hw_freq_seek tuner field
>
> 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK,
> the current frequency may be changed to fit in the range of the
> new active tuner
>
> 9. For backwards compatibility reasons tuner 0 should be the tuner
> with the broadest possible FM range

So with this approach every time during S_FREQ/S_HW_SEEK/S_TUNER
driver will check which tuner mode it is set to and change the tuner
mode (or band) according to tuner field.

So in my case I will have to support 5 tuner modes (EUROPE, JAPAN,
RUSSIAN, WEATHER and DEFAULT) just like bands.

This approach looks good to me.
>
> Regards,
>
> Hans



-- 
Regards
Halli

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

On 06/19/2012 07:43 PM, halli manjunatha wrote:
> On Tue, Jun 19, 2012 at 12:33 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 06/19/2012 06:47 PM, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> <snip long discussion about having a fixed set of bands versus
>>> a way to enumerate bands, including their rangelow, rangehigh
>>> and capabilities>
>>>
>>> Ok, you've convinced me. I agree that having a way to actually
>>> enumerate ranges, rather then having a fixed set of ranges, is
>>> better.
>>>
>>> Which brings us back many weeks to the proposal for making
>>> it possible to enumerate bands on radio devices. Rather
>>> then digging up the old mails lets start anew, I propose
>>> the following API for this:
>>>
>>> 1. A radio device can have multiple tuners, but only 1 can
>>> be active (streaming audio to the associated audio input)
>>> at the same time.
>>>
>>> 2. Radio device tuners are enumerated by calling G_TUNER
>>> with an increasing index until EINVAL gets returned
>>>
>>> 3. G_FREQUENCY will always return the frequency and index
>>> of the currently active tuner
>>>
>>> 4. When calling S_TUNER on a radio device, the active
>>> tuner will be set to the v4l2_tuner index field
>>>
>>> 5. When calling S_FREQUENCY on a radio device, the active
>>> tuner will be set to the v4l2_frequency tuner field
>>>
>>> 6. On a G_TUNER call on a radio device the rxsubchans,
>>> audmode, signal and afc v4l2_tuner fields are only
>>> filled on for the active tuner (as returned by
>>> G_FREQUENCY) for inactive tuners these fields are reported
>>> as 0.
>>
>>
>> p.s.
>>
>> I forgot:
>>
>> 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active
>> tuner will be set to the v4l2_hw_freq_seek tuner field
>>
>> 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK,
>> the current frequency may be changed to fit in the range of the
>> new active tuner
>>
>> 9. For backwards compatibility reasons tuner 0 should be the tuner
>> with the broadest possible FM range
>
> So with this approach every time during S_FREQ/S_HW_SEEK/S_TUNER
> driver will check which tuner mode it is set to and change the tuner
> mode (or band) according to tuner field.
>
> So in my case I will have to support 5 tuner modes (EUROPE, JAPAN,
> RUSSIAN, WEATHER and DEFAULT) just like bands.

Correct.

Regards,

Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans Verkuil]

On 19/06/12 19:33, Hans de Goede wrote:
> Hi,
>
> On 06/19/2012 06:47 PM, Hans de Goede wrote:
>> Hi,
>>
>> <snip long discussion about having a fixed set of bands versus
>> a way to enumerate bands, including their rangelow, rangehigh
>> and capabilities>
>>
>> Ok, you've convinced me. I agree that having a way to actually
>> enumerate ranges, rather then having a fixed set of ranges, is
>> better.
>>
>> Which brings us back many weeks to the proposal for making
>> it possible to enumerate bands on radio devices. Rather
>> then digging up the old mails lets start anew, I propose
>> the following API for this:
>>
>> 1. A radio device can have multiple tuners, but only 1 can
>> be active (streaming audio to the associated audio input)
>> at the same time.
>>
>> 2. Radio device tuners are enumerated by calling G_TUNER
>> with an increasing index until EINVAL gets returned
>>
>> 3. G_FREQUENCY will always return the frequency and index
>> of the currently active tuner
>>
>> 4. When calling S_TUNER on a radio device, the active
>> tuner will be set to the v4l2_tuner index field
>>
>> 5. When calling S_FREQUENCY on a radio device, the active
>> tuner will be set to the v4l2_frequency tuner field
>>
>> 6. On a G_TUNER call on a radio device the rxsubchans,
>> audmode, signal and afc v4l2_tuner fields are only
>> filled on for the active tuner (as returned by
>> G_FREQUENCY) for inactive tuners these fields are reported
>> as 0.
>
> p.s.
>
> I forgot:
>
> 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active
> tuner will be set to the v4l2_hw_freq_seek tuner field
>
> 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK,
> the current frequency may be changed to fit in the range of the
> new active tuner
>
> 9. For backwards compatibility reasons tuner 0 should be the tuner
> with the broadest possible FM range

I need to think about all these proposals. I know that when I worked with the
cadet driver I didn't like those multiple tuners at all.

But I need to read up on these new discussions and think about it. I doubt I'll
have time tomorrow, so it's probably going to be Thursday or Friday.

Regards,

	Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans Verkuil]

Sorry for the late reply, but I've been quite busy the last few days...

On Tue June 19 2012 16:14:26 Mauro Carvalho Chehab wrote:
> Em 19-06-2012 09:36, Hans de Goede escreveu:
> > Hi,
> > 
> > On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
> >> Em 19-06-2012 05:27, Hans de Goede escreveu:
> >>> Hi,
> >>>
> >>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
> >>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
> >>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>>>>
> >>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
> >>>>> ---
> >>>>>     include/linux/videodev2.h |   19 +++++++++++++++++--
> >>>>>     1 file changed, 17 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >>>>> index 2339678..013ee46 100644
> >>>>> --- a/include/linux/videodev2.h
> >>>>> +++ b/include/linux/videodev2.h
> >>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
> >>>>>         __u32            audmode;
> >>>>>         __s32            signal;
> >>>>>         __s32            afc;
> >>>>> -    __u32            reserved[4];
> >>>>> +    __u32            band;
> >>>>> +    __u32            reserved[3];
> >>>>>     };
> >>>>>
> >>>>>     struct v4l2_modulator {
> >>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
> >>>>>         __u32            rangelow;
> >>>>>         __u32            rangehigh;
> >>>>>         __u32            txsubchans;
> >>>>> -    __u32            reserved[4];
> >>>>> +    __u32            band;
> >>>>> +    __u32            reserved[3];
> >>>>>     };
> >>>>>
> >>>>>     /*  Flags for the 'capability' field */
> >>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
> >>>>>     #define V4L2_TUNER_CAP_RDS        0x0080
> >>>>>     #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
> >>>>>     #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
> >>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
> >>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
> >>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
> >>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
> >>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
> >>>>
> >>>> Frequency band is already specified by rangelow/rangehigh.
> >>>>
> >>>> Why do you need to duplicate this information?
> >>>
> >>> Because radio tuners may support multiple non overlapping
> >>> bands, this is why this patch also adds a band member
> >>> to the tuner struct, which can be used to set/get
> >>> the current band.
> >>>
> >>> One example of this are the tea5757 / tea5759
> >>> radio tuner chips:
> >>>
> >>> FM:
> >>> tea5757 87.5 - 108 MHz
> >>
> >>     rangelow = 87.5 * 62500;
> >>     rangehigh = 108 * 62500;
> >>
> >>> tea5759 76 - 91 MHz
> >>
> >>     rangelow = 76 * 62500;
> >>     rangehigh = 91 * 62500;
> >>
> >>> AM:
> >>> Both: 530 - 1710 kHz
> >>
> >>     rangelow = 0.530 * 62500;
> >>     rangehigh = 0.1710 * 62500;
> >>
> >>
> >> See radio-cadet.c:
> >>
> >> static int vidioc_g_tuner(struct file *file, void *priv,
> >>                 struct v4l2_tuner *v)
> >> {
> >>     struct cadet *dev = video_drvdata(file);
> >>
> >>     v->type = V4L2_TUNER_RADIO;
> >>     switch (v->index) {
> >>     case 0:
> >>         strlcpy(v->name, "FM", sizeof(v->name));
> >>         v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
> >>             V4L2_TUNER_CAP_RDS_BLOCK_IO;
> >>         v->rangelow = 1400;     /* 87.5 MHz */
> >>         v->rangehigh = 1728;    /* 108.0 MHz */
> >>         v->rxsubchans = cadet_getstereo(dev);
> >>         switch (v->rxsubchans) {
> >>         case V4L2_TUNER_SUB_MONO:
> >>             v->audmode = V4L2_TUNER_MODE_MONO;
> >>             break;
> >>         case V4L2_TUNER_SUB_STEREO:
> >>             v->audmode = V4L2_TUNER_MODE_STEREO;
> >>             break;
> >>         default:
> >>             break;
> >>         }
> >>         v->rxsubchans |= V4L2_TUNER_SUB_RDS;
> >>         break;
> >>     case 1:
> >>         strlcpy(v->name, "AM", sizeof(v->name));
> >>         v->capability = V4L2_TUNER_CAP_LOW;
> >>         v->rangelow = 8320;      /* 520 kHz */
> >>         v->rangehigh = 26400;    /* 1650 kHz */
> >>         v->rxsubchans = V4L2_TUNER_SUB_MONO;
> >>         v->audmode = V4L2_TUNER_MODE_MONO;
> >>         break;
> >>     default:
> >>         return -EINVAL;
> >>     }
> >>     v->signal = dev->sigstrength; /* We might need to modify scaling of this
> >>   */
> >>     return 0;
> >> }
> >> static int vidioc_s_tuner(struct file *file, void *priv,
> >>                 struct v4l2_tuner *v)
> >> {
> >>     struct cadet *dev = video_drvdata(file);
> >>
> >>     if (v->index != 0 && v->index != 1)
> >>         return -EINVAL;
> >>     dev->curtuner = v->index;
> >>     return 0;
> >> }
> >>
> >> Band switching are made via g_tuner/s_tuner calls. If a device have
> >> several non-overlapping bands, just implement it there. There's no
> >> need for a new API.
> > 
> > <sigh>, this has been discussed extensively between me, Hans V and
> > Halli Manjunatha on both irc and on the list. What the cadet driver is
> > doing is an ugly hack, and really a poor match for what we want.
> > 
> > Not to mention that it is a clear violation of the v4l2 spec:
> > http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
> > 
> > "Radio input devices have exactly one tuner with index zero, no video inputs."
> > 
> > So there is supposed to be only one tuner, and s_tuner / g_tuner
> > on radio devices always expect a tuner index of 0.
> > 
> > Also from the same page:
> > "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is more than one at all."
> > 
> > So if we model discontinuous ranges as multiple tuners how do we
> > select the right tuner? Certainly *not* though s_tuner, as that would
> > violate the spec. Note that changing the spec here is not really an option,
> > S_TUNER is expected to change the properties of the tuner selected through
> > the index, and is *not* expected to change the active tuner , esp. since
> > changing the active tuner would raise the question, change the active tuner
> > for which input ? The spec is clear on this:
> > "The tuner is solely determined by the current video input."
> 
> Well the specs need to be changed anyway, as there's no "video input" on a radio
> device.
> 
> As I said several times, we need to have a "profiles" section at the V4L2 API
> saying how ioctls should be implemented by each type of device: radio, tv tuners,
> webcams, media-based cameras, etc. Without that, API compilance is not really
> possible.
> 
> A clear example is that the omap3/s5p drivers don't work with an existent V4L2
> API, as they don't implement video inputs via V4L2 API. The video input selection is
> via the media API.
> 
> > iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
> > but it does not select a tuner. Making s_tuner actually select 1 of multiple
> > tuners for radio devices, would cause a large discrepancy between radio and
> > tv tuners.
> 
> So what? This is a very small discrepancy if you compare with what we currently have
> with omap3/s5p, where there's no way for a generic userspace application to work
> with those devices, and a libv4l generic plugin to properly implement the video input
> selection is still a dream.
> 
> It doesn't make any sense to implement video input selection on radio devices, as
> radio doesn't have video. So, g_input/s_input should not be implemented there at all.
> 
> I think they're implemented with some bogus code, as otherwise some userspace apps would
> break - but it doesn't make any sense to select a video input on a device without video!!!

v4l2-compliance will complain about radio devices implementing any of the input/output
ioctls. I've removed them from most of those drivers by now.

> Ok, hybrid radio/TV devices may have an "input" for FM, that actually selects the "no-video"
> video input, but this is there only because, in the past, it was allowed to get radio using
> the /dev/video0 device node. Thankfully, we get rid of this weird behavior on a few kernel
> versions ago.
> 
> > For tv tuners we've a 1:1 mapping between tuners and inputs, which makes sense, because
> > there are actual dual tuner devices, and the purpose of those is to be able to watch /
> > record 2 "shows" at the same time. 
> 
> No, there isn't an 1:1 mapping. A typical TV tuner has several inputs (TV, S-Video,
> Composite 1, ...) plus one radio "video" input.
> 
> They also have several audio inputs (managed via the audio routing ioctl's). On some
> devices, it is even possible to select a TV channel while listening to the FM radio
> (devices with tea5767 tuner). Of course, this is weird and never officially supported.
> 
> Most of the hybrid radio/TV devices actually have a single tuner that can be used 
> at the VHF frequencies. So, they allow getting FM, as it is part of their tuning range.
> 
> Even so, they're mapped as 2 separate tuners, one for TV and another for radio.

We *fake* them as two tuners. And that's fairly painful as you have to keep track
of the last-used frequency for each faked tuner and check what mode the tuner is in.

Now, this is understandable given the clear difference between radio and TV, but
that doesn't make it a smart choice when you are talking about discrete frequency
ranges within a single faked tuner (radio/TV).

> > Modeling this as multiple tuners is just wrong.
> 
> It is just the way it is since the beginning of V4L2: TV range is mapped as one tuner;
> FM radio is mapped as another one.
> 
> The cadet radio is one of the few devices that have FM and AM. It basically followed the
> same model already adopted by bttv, cx88, saa7134, ... devices with radio support.

The problem is that it doesn't really work. For each fake tuner you add you need to
keep track of the last-used frequency at the very least. And users should also
be able to detect whether the e.g. signal strength they get back from a tuner is
valid for the current band or is unavailable because another fake tuner is actually
selected.

> A change from that model will require changes at the radio implementation on all
> TV drivers, in order to prevent them to use a separate tuner for radio.
> 
> The struct v4l2_tuner/v4l2_modulator structs would likely need to be converted into
> something else or passed as an array, as each tuning band usage (TV, AM, FM, Weather,
> digital FM, digital AM) can have different properties:
> 	- range low/high;
> 	- modulation (AM, FM, ...);
> 	- sub-carriers (mono, stereo, lang1, lang2);
> 	- properties (RDS, seek caps, ...).
> 
> > This is simply not the case with these radio devices,
> > they can tune both AM and FM but *not* at the same time, so they have a *single*
> > *multiple-band* tuner.
> 
> A chip with both AM and FM tuners are, internally, a dual tuner.

No, it is one tuner with two modes. When in one mode you can't get any information
from the other mode. A true dual tuner would be able to set each tuner block to it's
own frequency and get back signal strength information etc. from each of them.

AFAIK some car radios can do this: the main tuner is used for audio and RDS, and it
collects from RDS the alternate frequencies the current station is transmitting on,
and a second tuner is used to try those frequencies and determine which has the
strongest signal. That's something you would model as two tuners.

> Anyway, on most
> devices, there's just a single dual-channel audio output. So, even on devices with
> 2 independent tuners, users can't really use both independently. There are, of course
> exceptions (ivtv devices can likely record a TV show while listening to radio, as they
> can use the MPEG encoder for TV).

If I remember correctly ivtv can't due to other limitations. It can select the audio from
the radio instead of the audio from the TV, though :-)

> 
> > Not only have we already discussed
> > this in a long discussion, I've patches to extend the tea575x driver with AM support,
> > and the initial revision used the multiple tuner model, but that just does not work
> > well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's patchset for
> > limiting hw-freq seek ranges, after which all of this has been discussed extensively!
> 
> Sorry but I missed this discussion.
> 
> >> Also, this is generic enough to cover even devices with non-standard
> >> frequency ranges.
> >>
> >> All bands can easily be detected via a g_tuner loop, and band switching
> >> is done via s_tuner.
> >>
> >> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
> >> and this is a way more generic than what's being proposed.
> > 
> > It is also very very wrong, there is only a single tuner on these devices,
> > modeling this as multiple tuners is just wrong!
> > 
> >> It likely makes sense to standardize the band names inside the radio core,
> >> in order to avoid having the same band called with two different names inside
> >> the drivers.
> >>
> >> It should also be noticed that each band may have different properties.
> >> On the above, the FM band can do stereo/mono and RDS, while AM is just
> >> mono So, a change like what's proposed would keep requiring two entries.
> > 
> > With FM we already have a situation where some channels are mono and other
> > stereo, with AM/FM the tuner capabilities would reflect what the tuner can
> > do on some bands-frequency combinations, just like it now reflects what
> > it can do on some frequencies.
> 
> Mixing an AM tuner with an FM tuner is really really wrong. Only the PLL
> stage is identical.
> 
> The AM demodulator is generally just an envelope detector, while the FM 
> demodulator is a way more complex and completely different from what's
> done with AM.
> 
> Digital FM band and digital AM band radio is also completely different from
> analog AM/analog FM. The only thing in comon with "standard" AM/FM is the
> band.
> 
> The fact that all 5 types of tuner (TV, analog AM, analog FM, digital AM band,
> digital FM band) are implemented by just one PLL or not is irrelevant. Each
> one is a different tuner, as the tuning demodulation is different.

The struct v4l2_tuner basically abstracts a PLL: it has an associated frequency,
signal strength and audio mode detection. So it is IMHO a crucially important
fact that these bands are all implemented by one PLL: it means that it has to
be represented by one tuner struct.

The radio-cadet driver fakes with with two structs, and I've worked with that
for a bit and it is just a poor mapping. Using one struct makes it all fall
neatly into place.

Frankly, we never got the radio/tv fake tuner mapping working well. I actually
think that faking it like it is today is a bad idea as well. Instead it should
be seen as a single resource, and if it is in use by the TV, then any attempt
to access it from the radio side should return -EBUSY and vice versa.

Anyway, that's a discussion for a later time.

> What I'm saying is that just adding a "band" field inside a single tuner
> struct is plain wrong, as each type has different properties.

See my proposal below.

> 
> > 
> > <snip>
> > 
> >>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
> >>> creating 2 band defines for this.
> >>
> >> Yes, it is very close, but Countries that added the extra 500 kHz bandwidth
> >> added stations there. On those, older devices can't tune into the new channels.
> > 
> > On those older devices rangelow would get reported as 88 rather then 87.5, the
> > band selection mechanism is there to select a certain range approximately,
> > the exact resulting range will be hw specific and reported in rangelow /'
> > rangehigh, as the patch documenting the new fields clearly documents.
> 
> Why to implement a "band" field that:
> 	1) can provide a wrong information (87.5 instead of 88);
> 	2) duplicates an existing information implemented at rangelow/rangehigh
> ?

I agree here. Using fixed bands is too limiting.

> 
> > 
> > <snip>
> > 
> >>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
> >>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
> >>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
> >>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
> >>> supports this, and indeed calls it "Japan wide band"
> >>
> >> That's why giving them name via defines is a bad thing: the concept of
> >> "universal" changes from time to time: 15 years ago, an "universal" radio
> >> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM (88-108MHz).
> >>
> >> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
> >> it is now 76-163 Mhz.
> >>
> >> If a band like that is described as "FM" with a frequency range from 76
> >> to 163 MHz, this is clearer than calling it as "FM unversal".
> > 
> > We will still have rangelow and rangehigh to report the actual implemented
> > band. So there is no problem here. An app can select universal and then
> > figure out what universal is on the specific device it is using with a
> > G_TUNER.
> 
> If rangelow/rangehigh is the actual band, why does it need something else?
> 
> Reusing G_TUNER/S_TUNER or not, the issue is that a bitfield parameter for
> frequency range is not actually able to express what are the supported
> ranges. As I said before, the tuner ranges can only be properly expressed 
> by an array with:
> 	- range low/high;
> 	- modulation (AM, FM, ...);
> 	- sub-carriers (mono, stereo, lang1, lang2);
> 	- properties (RDS, seek caps, ...).

Agreed.

So, in my opinion we still need the band field, but instead of this being a
fixed band it is an index.

In order to enumerate over all bands I propose a new ioctl:

VIDIOC_ENUM_TUNER_BAND

with struct:

struct v4l2_tuner_band {
	__u32 tuner;
	__u32 index;
	char name[32];
	__u32 capability;	/* The same as in v4l2_tuner */
	__u32 rangelow;
	__u32 rangehigh;
	__u32 reserved[7];
};

It enumerates the supported bands by the tuner, each with a human readable name,
frequency range and capabilities.

If you change the band using S_TUNER, then G_TUNER will return the frequency range
and capabilities from the corresponding v4l2_tuner_band struct.

The only capability that needs to be added is one that tells the application that
the tuner has the capability to do band enumeration (V4L2_TUNER_CAP_HAS_BANDS or
something).

I am not 100% certain about the name field: it is very nice for apps, but we do
need some guidelines here.

A similar struct would be needed for modulators if we ever need to add support for
modulators with multiple bands.

We could perhaps rename v4l2_tuner_band to just v4l2_band to make it tuner/modulator
agnostic.

Regards,

	Hans

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Mauro Carvalho Chehab]

Em 22-06-2012 11:07, Hans Verkuil escreveu:
> Sorry for the late reply, but I've been quite busy the last few days...
> 
> On Tue June 19 2012 16:14:26 Mauro Carvalho Chehab wrote:
>> Em 19-06-2012 09:36, Hans de Goede escreveu:
>>> Hi,
>>>
>>> On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:
>>>> Em 19-06-2012 05:27, Hans de Goede escreveu:
>>>>> Hi,
>>>>>
>>>>> On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:
>>>>>> Em 28-05-2012 07:46, Hans Verkuil escreveu:
>>>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>>>
>>>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>>> Acked-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>      include/linux/videodev2.h |   19 +++++++++++++++++--
>>>>>>>      1 file changed, 17 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>>>> index 2339678..013ee46 100644
>>>>>>> --- a/include/linux/videodev2.h
>>>>>>> +++ b/include/linux/videodev2.h
>>>>>>> @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
>>>>>>>          __u32            audmode;
>>>>>>>          __s32            signal;
>>>>>>>          __s32            afc;
>>>>>>> -    __u32            reserved[4];
>>>>>>> +    __u32            band;
>>>>>>> +    __u32            reserved[3];
>>>>>>>      };
>>>>>>>
>>>>>>>      struct v4l2_modulator {
>>>>>>> @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
>>>>>>>          __u32            rangelow;
>>>>>>>          __u32            rangehigh;
>>>>>>>          __u32            txsubchans;
>>>>>>> -    __u32            reserved[4];
>>>>>>> +    __u32            band;
>>>>>>> +    __u32            reserved[3];
>>>>>>>      };
>>>>>>>
>>>>>>>      /*  Flags for the 'capability' field */
>>>>>>> @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
>>>>>>>      #define V4L2_TUNER_CAP_RDS        0x0080
>>>>>>>      #define V4L2_TUNER_CAP_RDS_BLOCK_IO    0x0100
>>>>>>>      #define V4L2_TUNER_CAP_RDS_CONTROLS    0x0200
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US     0x00010000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
>>>>>>> +#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000
>>>>>>
>>>>>> Frequency band is already specified by rangelow/rangehigh.
>>>>>>
>>>>>> Why do you need to duplicate this information?
>>>>>
>>>>> Because radio tuners may support multiple non overlapping
>>>>> bands, this is why this patch also adds a band member
>>>>> to the tuner struct, which can be used to set/get
>>>>> the current band.
>>>>>
>>>>> One example of this are the tea5757 / tea5759
>>>>> radio tuner chips:
>>>>>
>>>>> FM:
>>>>> tea5757 87.5 - 108 MHz
>>>>
>>>>      rangelow = 87.5 * 62500;
>>>>      rangehigh = 108 * 62500;
>>>>
>>>>> tea5759 76 - 91 MHz
>>>>
>>>>      rangelow = 76 * 62500;
>>>>      rangehigh = 91 * 62500;
>>>>
>>>>> AM:
>>>>> Both: 530 - 1710 kHz
>>>>
>>>>      rangelow = 0.530 * 62500;
>>>>      rangehigh = 0.1710 * 62500;
>>>>
>>>>
>>>> See radio-cadet.c:
>>>>
>>>> static int vidioc_g_tuner(struct file *file, void *priv,
>>>>                  struct v4l2_tuner *v)
>>>> {
>>>>      struct cadet *dev = video_drvdata(file);
>>>>
>>>>      v->type = V4L2_TUNER_RADIO;
>>>>      switch (v->index) {
>>>>      case 0:
>>>>          strlcpy(v->name, "FM", sizeof(v->name));
>>>>          v->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
>>>>              V4L2_TUNER_CAP_RDS_BLOCK_IO;
>>>>          v->rangelow = 1400;     /* 87.5 MHz */
>>>>          v->rangehigh = 1728;    /* 108.0 MHz */
>>>>          v->rxsubchans = cadet_getstereo(dev);
>>>>          switch (v->rxsubchans) {
>>>>          case V4L2_TUNER_SUB_MONO:
>>>>              v->audmode = V4L2_TUNER_MODE_MONO;
>>>>              break;
>>>>          case V4L2_TUNER_SUB_STEREO:
>>>>              v->audmode = V4L2_TUNER_MODE_STEREO;
>>>>              break;
>>>>          default:
>>>>              break;
>>>>          }
>>>>          v->rxsubchans |= V4L2_TUNER_SUB_RDS;
>>>>          break;
>>>>      case 1:
>>>>          strlcpy(v->name, "AM", sizeof(v->name));
>>>>          v->capability = V4L2_TUNER_CAP_LOW;
>>>>          v->rangelow = 8320;      /* 520 kHz */
>>>>          v->rangehigh = 26400;    /* 1650 kHz */
>>>>          v->rxsubchans = V4L2_TUNER_SUB_MONO;
>>>>          v->audmode = V4L2_TUNER_MODE_MONO;
>>>>          break;
>>>>      default:
>>>>          return -EINVAL;
>>>>      }
>>>>      v->signal = dev->sigstrength; /* We might need to modify scaling of this
>>>>    */
>>>>      return 0;
>>>> }
>>>> static int vidioc_s_tuner(struct file *file, void *priv,
>>>>                  struct v4l2_tuner *v)
>>>> {
>>>>      struct cadet *dev = video_drvdata(file);
>>>>
>>>>      if (v->index != 0 && v->index != 1)
>>>>          return -EINVAL;
>>>>      dev->curtuner = v->index;
>>>>      return 0;
>>>> }
>>>>
>>>> Band switching are made via g_tuner/s_tuner calls. If a device have
>>>> several non-overlapping bands, just implement it there. There's no
>>>> need for a new API.
>>>
>>> <sigh>, this has been discussed extensively between me, Hans V and
>>> Halli Manjunatha on both irc and on the list. What the cadet driver is
>>> doing is an ugly hack, and really a poor match for what we want.
>>>
>>> Not to mention that it is a clear violation of the v4l2 spec:
>>> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html
>>>
>>> "Radio input devices have exactly one tuner with index zero, no video inputs."
>>>
>>> So there is supposed to be only one tuner, and s_tuner / g_tuner
>>> on radio devices always expect a tuner index of 0.
>>>
>>> Also from the same page:
>>> "Note that VIDIOC_S_TUNER does not switch the current tuner, when there is more than one at all."
>>>
>>> So if we model discontinuous ranges as multiple tuners how do we
>>> select the right tuner? Certainly *not* though s_tuner, as that would
>>> violate the spec. Note that changing the spec here is not really an option,
>>> S_TUNER is expected to change the properties of the tuner selected through
>>> the index, and is *not* expected to change the active tuner , esp. since
>>> changing the active tuner would raise the question, change the active tuner
>>> for which input ? The spec is clear on this:
>>> "The tuner is solely determined by the current video input."
>>
>> Well the specs need to be changed anyway, as there's no "video input" on a radio
>> device.
>>
>> As I said several times, we need to have a "profiles" section at the V4L2 API
>> saying how ioctls should be implemented by each type of device: radio, tv tuners,
>> webcams, media-based cameras, etc. Without that, API compilance is not really
>> possible.
>>
>> A clear example is that the omap3/s5p drivers don't work with an existent V4L2
>> API, as they don't implement video inputs via V4L2 API. The video input selection is
>> via the media API.
>>
>>> iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
>>> but it does not select a tuner. Making s_tuner actually select 1 of multiple
>>> tuners for radio devices, would cause a large discrepancy between radio and
>>> tv tuners.
>>
>> So what? This is a very small discrepancy if you compare with what we currently have
>> with omap3/s5p, where there's no way for a generic userspace application to work
>> with those devices, and a libv4l generic plugin to properly implement the video input
>> selection is still a dream.
>>
>> It doesn't make any sense to implement video input selection on radio devices, as
>> radio doesn't have video. So, g_input/s_input should not be implemented there at all.
>>
>> I think they're implemented with some bogus code, as otherwise some userspace apps would
>> break - but it doesn't make any sense to select a video input on a device without video!!!
> 
> v4l2-compliance will complain about radio devices implementing any of the input/output
> ioctls. I've removed them from most of those drivers by now.

This should be easy to fix.

>> Ok, hybrid radio/TV devices may have an "input" for FM, that actually selects the "no-video"
>> video input, but this is there only because, in the past, it was allowed to get radio using
>> the /dev/video0 device node. Thankfully, we get rid of this weird behavior on a few kernel
>> versions ago.
>>
>>> For tv tuners we've a 1:1 mapping between tuners and inputs, which makes sense, because
>>> there are actual dual tuner devices, and the purpose of those is to be able to watch /
>>> record 2 "shows" at the same time.
>>
>> No, there isn't an 1:1 mapping. A typical TV tuner has several inputs (TV, S-Video,
>> Composite 1, ...) plus one radio "video" input.
>>
>> They also have several audio inputs (managed via the audio routing ioctl's). On some
>> devices, it is even possible to select a TV channel while listening to the FM radio
>> (devices with tea5767 tuner). Of course, this is weird and never officially supported.
>>
>> Most of the hybrid radio/TV devices actually have a single tuner that can be used
>> at the VHF frequencies. So, they allow getting FM, as it is part of their tuning range.
>>
>> Even so, they're mapped as 2 separate tuners, one for TV and another for radio.
> 
> We *fake* them as two tuners. And that's fairly painful as you have to keep track
> of the last-used frequency for each faked tuner and check what mode the tuner is in.

There was never a 1:1 relation between the number of tuners and how they're exposed on
userspace. See for example the cases of video devices with FM: devices with a separate
radio tuner are exported the same way as devices with a single tuner by tuner-core.

There are a few reasons for that, but the main one is that it doesn't make any practical
sense to use both tuners at the same time, as, on almost all board design, the FM audio
output is wired to the same dual-channel output as the TV one. Besides that, who would
listen to FM radio while seeing/listening TV?

> Now, this is understandable given the clear difference between radio and TV, but
> that doesn't make it a smart choice when you are talking about discrete frequency
> ranges within a single faked tuner (radio/TV).
> 
>>> Modeling this as multiple tuners is just wrong.
>>
>> It is just the way it is since the beginning of V4L2: TV range is mapped as one tuner;
>> FM radio is mapped as another one.
>>
>> The cadet radio is one of the few devices that have FM and AM. It basically followed the
>> same model already adopted by bttv, cx88, saa7134, ... devices with radio support.
> 
> The problem is that it doesn't really work. For each fake tuner you add you need to
> keep track of the last-used frequency at the very least.

Well, the per-frequency range last-used frequency makes sense: if the last frequency at
the AM range is 570 kHz, and the radio is switched to the Japan range, the 570kHz doesn't
make any sense there.

The same is true if the radio changes from the Japan range (76-90 MHz) to the European
Range, as, if the frequency is below 87.5 MHz, it is invalid at the European range.

> And users should also
> be able to detect whether the e.g. signal strength they get back from a tuner is
> valid for the current band or is unavailable because another fake tuner is actually
> selected.

It also makes sense: if the range is switched, the tuning frequency is switched to
the new one, and signal strength should get the signal at the new range, not at the
old one.

>> A change from that model will require changes at the radio implementation on all
>> TV drivers, in order to prevent them to use a separate tuner for radio.
>>
>> The struct v4l2_tuner/v4l2_modulator structs would likely need to be converted into
>> something else or passed as an array, as each tuning band usage (TV, AM, FM, Weather,
>> digital FM, digital AM) can have different properties:
>> 	- range low/high;
>> 	- modulation (AM, FM, ...);
>> 	- sub-carriers (mono, stereo, lang1, lang2);
>> 	- properties (RDS, seek caps, ...).
>>
>>> This is simply not the case with these radio devices,
>>> they can tune both AM and FM but *not* at the same time, so they have a *single*
>>> *multiple-band* tuner.
>>
>> A chip with both AM and FM tuners are, internally, a dual tuner.
> 
> No, it is one tuner with two modes. When in one mode you can't get any information
> from the other mode. A true dual tuner would be able to set each tuner block to it's
> own frequency and get back signal strength information etc. from each of them.

A device with true dual-mode tuners should be mapped as two independent radio nodes.
AFAIKT, g_tuner/s_tuner API were never meant to be used by such devices, as they're
independent nodes.

> AFAIK some car radios can do this: the main tuner is used for audio and RDS, and it
> collects from RDS the alternate frequencies the current station is transmitting on,
> and a second tuner is used to try those frequencies and determine which has the
> strongest signal. That's something you would model as two tuners.

Ok, this is a new use-case. Is there any driver implementing it? If not, let's only
take care of it when the first use case for it arrives.

>> Anyway, on most
>> devices, there's just a single dual-channel audio output. So, even on devices with
>> 2 independent tuners, users can't really use both independently. There are, of course
>> exceptions (ivtv devices can likely record a TV show while listening to radio, as they
>> can use the MPEG encoder for TV).
> 
> If I remember correctly ivtv can't due to other limitations. It can select the audio from
> the radio instead of the audio from the TV, though :-)

cx88 devices with a separate audio tuner could do that, but this is something that is 
not officially supported by the device manufacturer. As it will power more devices that
planned, if the board is not well-designed, it may overheat the device or cause some other
problems there. I don't think we should officially support it. 

> 
>>
>>> Not only have we already discussed
>>> this in a long discussion, I've patches to extend the tea575x driver with AM support,
>>> and the initial revision used the multiple tuner model, but that just does not work
>>> well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's patchset for
>>> limiting hw-freq seek ranges, after which all of this has been discussed extensively!
>>
>> Sorry but I missed this discussion.
>>
>>>> Also, this is generic enough to cover even devices with non-standard
>>>> frequency ranges.
>>>>
>>>> All bands can easily be detected via a g_tuner loop, and band switching
>>>> is done via s_tuner.
>>>>
>>>> Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
>>>> and this is a way more generic than what's being proposed.
>>>
>>> It is also very very wrong, there is only a single tuner on these devices,
>>> modeling this as multiple tuners is just wrong!
>>>
>>>> It likely makes sense to standardize the band names inside the radio core,
>>>> in order to avoid having the same band called with two different names inside
>>>> the drivers.
>>>>
>>>> It should also be noticed that each band may have different properties.
>>>> On the above, the FM band can do stereo/mono and RDS, while AM is just
>>>> mono So, a change like what's proposed would keep requiring two entries.
>>>
>>> With FM we already have a situation where some channels are mono and other
>>> stereo, with AM/FM the tuner capabilities would reflect what the tuner can
>>> do on some bands-frequency combinations, just like it now reflects what
>>> it can do on some frequencies.
>>
>> Mixing an AM tuner with an FM tuner is really really wrong. Only the PLL
>> stage is identical.
>>
>> The AM demodulator is generally just an envelope detector, while the FM
>> demodulator is a way more complex and completely different from what's
>> done with AM.
>>
>> Digital FM band and digital AM band radio is also completely different from
>> analog AM/analog FM. The only thing in comon with "standard" AM/FM is the
>> band.
>>
>> The fact that all 5 types of tuner (TV, analog AM, analog FM, digital AM band,
>> digital FM band) are implemented by just one PLL or not is irrelevant. Each
>> one is a different tuner, as the tuning demodulation is different.
> 
> The struct v4l2_tuner basically abstracts a PLL: it has an associated frequency,
> signal strength and audio mode detection. So it is IMHO a crucially important
> fact that these bands are all implemented by one PLL: it means that it has to
> be represented by one tuner struct.

No. It abstracts the PLL plus the demod. Audio mode detection is part of the demod
block. For example, on analog FM, stereo is indicated only if the 19 kHz pilot
is present and it is above a certain threshold. RDS will require a 57kHz sub-carrier.

If/when digital radio is implemented, in order to get the audio mode, decoding
the sub-carriers will also be needed.

So, what I'm saying is that, despite its name, v4l2_tuner struct is not a per-tuner
data. It is something else. It sets/gets the tuner+demod data.

> The radio-cadet driver fakes with with two structs, and I've worked with that
> for a bit and it is just a poor mapping. Using one struct makes it all fall
> neatly into place.

No. Using just one struct is WRONG: The tuner range for AM is completely different
from the one for FM; there's no support for stereo on AM; tuning name is different
(one is "AM"; the other one is "FM"), etc.

> Frankly, we never got the radio/tv fake tuner mapping working well. I actually
> think that faking it like it is today is a bad idea as well. Instead it should
> be seen as a single resource, and if it is in use by the TV, then any attempt
> to access it from the radio side should return -EBUSY and vice versa.

Returning -EBUSY when TV is streaming and radio tries to use a shared resource
makes sense, but a shared resource is not just a tuner (or a PLL).

> Anyway, that's a discussion for a later time.
> 
>> What I'm saying is that just adding a "band" field inside a single tuner
>> struct is plain wrong, as each type has different properties.
> 
> See my proposal below.
> 
>>
>>>
>>> <snip>
>>>
>>>>> 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
>>>>> creating 2 band defines for this.
>>>>
>>>> Yes, it is very close, but Countries that added the extra 500 kHz bandwidth
>>>> added stations there. On those, older devices can't tune into the new channels.
>>>
>>> On those older devices rangelow would get reported as 88 rather then 87.5, the
>>> band selection mechanism is there to select a certain range approximately,
>>> the exact resulting range will be hw specific and reported in rangelow /'
>>> rangehigh, as the patch documenting the new fields clearly documents.
>>
>> Why to implement a "band" field that:
>> 	1) can provide a wrong information (87.5 instead of 88);
>> 	2) duplicates an existing information implemented at rangelow/rangehigh
>> ?
> 
> I agree here. Using fixed bands is too limiting.
> 
>>
>>>
>>> <snip>
>>>
>>>>> This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
>>>>> on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather band,
>>>>> thus going all the way from 76 - 163 Mhz, so I guess we should add a
>>>>> V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
>>>>> supports this, and indeed calls it "Japan wide band"
>>>>
>>>> That's why giving them name via defines is a bad thing: the concept of
>>>> "universal" changes from time to time: 15 years ago, an "universal" radio
>>>> is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM (88-108MHz).
>>>>
>>>> An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
>>>> it is now 76-163 Mhz.
>>>>
>>>> If a band like that is described as "FM" with a frequency range from 76
>>>> to 163 MHz, this is clearer than calling it as "FM unversal".
>>>
>>> We will still have rangelow and rangehigh to report the actual implemented
>>> band. So there is no problem here. An app can select universal and then
>>> figure out what universal is on the specific device it is using with a
>>> G_TUNER.
>>
>> If rangelow/rangehigh is the actual band, why does it need something else?
>>
>> Reusing G_TUNER/S_TUNER or not, the issue is that a bitfield parameter for
>> frequency range is not actually able to express what are the supported
>> ranges. As I said before, the tuner ranges can only be properly expressed
>> by an array with:
>> 	- range low/high;
>> 	- modulation (AM, FM, ...);
>> 	- sub-carriers (mono, stereo, lang1, lang2);
>> 	- properties (RDS, seek caps, ...).
> 
> Agreed.
> 
> So, in my opinion we still need the band field, but instead of this being a
> fixed band it is an index.
> 
> In order to enumerate over all bands I propose a new ioctl:
> 
> VIDIOC_ENUM_TUNER_BAND
> 
> with struct:
> 
> struct v4l2_tuner_band {
> 	__u32 tuner;
> 	__u32 index;
> 	char name[32];
> 	__u32 capability;	/* The same as in v4l2_tuner */
> 	__u32 rangelow;
> 	__u32 rangehigh;
> 	__u32 reserved[7];
> };
> 
> It enumerates the supported bands by the tuner, each with a human readable name,
> frequency range and capabilities.
> 
> If you change the band using S_TUNER, then G_TUNER will return the frequency range
> and capabilities from the corresponding v4l2_tuner_band struct.
> 
> The only capability that needs to be added is one that tells the application that
> the tuner has the capability to do band enumeration (V4L2_TUNER_CAP_HAS_BANDS or
> something).
> 
> I am not 100% certain about the name field: it is very nice for apps, but we do
> need some guidelines here.
> 
> A similar struct would be needed for modulators if we ever need to add support for
> modulators with multiple bands.
> 
> We could perhaps rename v4l2_tuner_band to just v4l2_band to make it tuner/modulator
> agnostic.

The above proposal would be great if we were starting to write the radio API today, but
your proposal is not backward compatible with the status quo.

Regards,
Mauro

Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

[Hans de Goede]

Hi,

On 06/22/2012 06:15 PM, Mauro Carvalho Chehab wrote:
> Em 22-06-2012 11:07, Hans Verkuil escreveu:

<snip>

>>> Reusing G_TUNER/S_TUNER or not, the issue is that a bitfield parameter for
>>> frequency range is not actually able to express what are the supported
>>> ranges. As I said before, the tuner ranges can only be properly expressed
>>> by an array with:
>>> 	- range low/high;
>>> 	- modulation (AM, FM, ...);
>>> 	- sub-carriers (mono, stereo, lang1, lang2);
>>> 	- properties (RDS, seek caps, ...).
>>
>> Agreed.
>>
>> So, in my opinion we still need the band field, but instead of this being a
>> fixed band it is an index.
>>
>> In order to enumerate over all bands I propose a new ioctl:
>>
>> VIDIOC_ENUM_TUNER_BAND
>>
>> with struct:
>>
>> struct v4l2_tuner_band {
>> 	__u32 tuner;
>> 	__u32 index;
>> 	char name[32];
>> 	__u32 capability;	/* The same as in v4l2_tuner */
>> 	__u32 rangelow;
>> 	__u32 rangehigh;
>> 	__u32 reserved[7];
>> };
>>
>> It enumerates the supported bands by the tuner, each with a human readable name,
>> frequency range and capabilities.
>>
>> If you change the band using S_TUNER, then G_TUNER will return the frequency range
>> and capabilities from the corresponding v4l2_tuner_band struct.
>>
>> The only capability that needs to be added is one that tells the application that
>> the tuner has the capability to do band enumeration (V4L2_TUNER_CAP_HAS_BANDS or
>> something).
>>
>> I am not 100% certain about the name field: it is very nice for apps, but we do
>> need some guidelines here.
>>
>> A similar struct would be needed for modulators if we ever need to add support for
>> modulators with multiple bands.
>>
>> We could perhaps rename v4l2_tuner_band to just v4l2_band to make it tuner/modulator
>> agnostic.

I've not replied before because I've been thinking about Hans V's proposal for a bit,
I've come to the conclusion that Hans V's proposal is better, because it avoids a
discrepancy in how tuners work between tv and radio, which is something which worried
me about my own proposal. Hans V's proposal also has the benefit that it will work fine
for tv-tuners too, so if we ever need bands for tv tuners we can use the *same* API.

> The above proposal would be great if we were starting to write the radio API today, but
> your proposal is not backward compatible with the status quo.

Huh? Hans V. is proposing adding a band field to the tuner struct (using one of the
reserved fields) and adding a new ioctl to enumerate bands. Old apps will have that field
set to 0 on a S_TUNER, selecting band 0, and G_TUNER will give info on the current band,
where-as S/G_FREQ will be unmodified (they will work on the current band). I don't see how
this breaks current apps...

Anyways both proposals seem workable to me, although I prefer Hans V.'s one. Lets just pick
one and get on with this.

Regards,

Hans




>
> Regards,
> Mauro
>

[RFCv2 PATCH 5/6] V4L2 spec: add frequency band documentation.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Based in part on an earlier patch from <hallimanju@gmail.com>.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 .../DocBook/media/v4l/vidioc-g-modulator.xml       |   38 +++++---
 Documentation/DocBook/media/v4l/vidioc-g-tuner.xml |   97 +++++++++++++++++---
 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |    3 +-
 3 files changed, 112 insertions(+), 26 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
index 7f4ac7e..713ba06 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml
@@ -68,17 +68,17 @@ to this structure. Drivers fill the rest of the structure or return an
 applications shall begin at index zero, incrementing by one until the
 driver returns <errorcode>EINVAL</errorcode>.</para>
 
-    <para>Modulators have two writable properties, an audio
-modulation set and the radio frequency. To change the modulated audio
-subprograms, applications initialize the <structfield>index
-</structfield> and <structfield>txsubchans</structfield> fields and the
-<structfield>reserved</structfield> array and call the
-<constant>VIDIOC_S_MODULATOR</constant> ioctl. Drivers may choose a
-different audio modulation if the request cannot be satisfied. However
-this is a write-only ioctl, it does not return the actual audio
+    <para>Modulators have three writable properties, an audio
+modulation set, the frequency band and the radio frequency. To change the
+modulated audio subprograms or frequency band, applications initialize the
+<structfield>index</structfield>, <structfield>band</structfield>,
+<structfield>txsubchans</structfield> and <structfield>reserved</structfield>
+fields and call the <constant>VIDIOC_S_MODULATOR</constant> ioctl. Drivers
+may choose a different audio modulation if the request cannot be satisfied.
+However this is a write-only ioctl, it does not return the actual audio
 modulation selected.</para>
 
-    <para>To change the radio frequency the &VIDIOC-S-FREQUENCY; ioctl
+    <para>To change the frequency the &VIDIOC-S-FREQUENCY; ioctl
 is available.</para>
 
     <table pgwide="1" frame="none" id="v4l2-modulator">
@@ -110,16 +110,16 @@ change for example with the current video standard.</entry>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>rangelow</structfield></entry>
-	    <entry>The lowest tunable frequency in units of 62.5
-KHz, or if the <structfield>capability</structfield> flag
+	    <entry>The lowest tunable frequency of the current frequency band
+in units of 62.5 kHz, or if the <structfield>capability</structfield> flag
 <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
 Hz.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>rangehigh</structfield></entry>
-	    <entry>The highest tunable frequency in units of 62.5
-KHz, or if the <structfield>capability</structfield> flag
+	    <entry>The highest tunable frequency of the current frequency band
+in units of 62.5 kHz, or if the <structfield>capability</structfield> flag
 <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
 Hz.</entry>
 	  </row>
@@ -138,7 +138,17 @@ indicator, for example a stereo pilot tone.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[4]</entry>
+	    <entry><structfield>band</structfield></entry>
+	    <entry spanname="hspan">The frequency band. The available bands are
+	    defined in the <structfield>capability</structfield> field. The band
+	    <constant>V4L2_TUNER_BAND_DEFAULT</constant> is always available. After changing
+	    the band the current frequency will be clamped to the new frequency range.
+	    See <xref linkend="radio-bands" /> for valid band values.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[3]</entry>
 	    <entry>Reserved for future extensions. Drivers and
 applications must set the array to zero.</entry>
 	  </row>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
index 95d5371..27a8916 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
@@ -68,10 +68,10 @@ structure. Drivers fill the rest of the structure or return an
 applications shall begin at index zero, incrementing by one until the
 driver returns <errorcode>EINVAL</errorcode>.</para>
 
-    <para>Tuners have two writable properties, the audio mode and
-the radio frequency. To change the audio mode, applications initialize
-the <structfield>index</structfield>,
-<structfield>audmode</structfield> and
+    <para>Tuners have three writable properties, the audio mode, the frequency
+band and the radio frequency. To change the audio mode and band, applications
+initialize the <structfield>index</structfield>,
+<structfield>audmode</structfield>, <structfield>band</structfield> and
 <structfield>reserved</structfield> fields and call the
 <constant>VIDIOC_S_TUNER</constant> ioctl. This will
 <emphasis>not</emphasis> change the current tuner, which is determined
@@ -80,7 +80,7 @@ if the requested mode is invalid or unsupported. Since this is a
 <!-- FIXME -->write-only ioctl, it does not return the actually
 selected audio mode.</para>
 
-    <para>To change the radio frequency the &VIDIOC-S-FREQUENCY; ioctl
+    <para>To change the frequency the &VIDIOC-S-FREQUENCY; ioctl
 is available.</para>
 
     <table pgwide="1" frame="none" id="v4l2-tuner">
@@ -127,16 +127,16 @@ the structure refers to a radio tuner only the
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>rangelow</structfield></entry>
-	    <entry spanname="hspan">The lowest tunable frequency in
-units of 62.5 kHz, or if the <structfield>capability</structfield>
+	    <entry spanname="hspan">The lowest tunable frequency of the current
+frequency band in units of 62.5 kHz, or if the <structfield>capability</structfield>
 flag <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
 Hz.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>rangehigh</structfield></entry>
-	    <entry spanname="hspan">The highest tunable frequency in
-units of 62.5 kHz, or if the <structfield>capability</structfield>
+	    <entry spanname="hspan">The highest tunable frequency of the current
+frequency band in units of 62.5 kHz, or if the <structfield>capability</structfield>
 flag <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
 Hz.</entry>
 	  </row>
@@ -226,7 +226,17 @@ settles at zero, &ie; range is what? --></entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[4]</entry>
+	    <entry><structfield>band</structfield></entry>
+	    <entry spanname="hspan">The frequency band. The available bands are
+	    defined in the <structfield>capability</structfield> field. The band
+	    <constant>V4L2_TUNER_BAND_DEFAULT</constant> is always available. After changing
+	    the band the current frequency will be clamped to the new frequency range.
+	    See <xref linkend="radio-bands" /> for valid band values.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[3]</entry>
 	    <entry spanname="hspan">Reserved for future extensions. Drivers and
 applications must set the array to zero.</entry>
 	  </row>
@@ -340,6 +350,31 @@ radio tuners.</entry>
 	<entry>0x0200</entry>
 	<entry>The RDS data is parsed by the hardware and set via controls.</entry>
 	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_FM_EUROPE_US</constant></entry>
+	<entry>0x010000</entry>
+	<entry>FM radio European or US band (87.5 Mhz - 108 MHz, exact range is hardware dependent).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_FM_JAPAN</constant></entry>
+	<entry>0x020000</entry>
+	<entry>FM radio Japan band (76 MHz - 90 MHz, exact range is hardware dependent).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_FM_RUSSIAN</constant></entry>
+	<entry>0x040000</entry>
+	<entry>FM radio OIRT or Russian band (65.8 MHz - 74 MHz, exact range is hardware dependent).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_FM_WEATHER</constant></entry>
+	<entry>0x080000</entry>
+	<entry>FM radio weather band (162.4 MHz - 162.55 MHz, exact range is hardware dependent).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_AM_MW</constant></entry>
+	<entry>0x100000</entry>
+	<entry>AM radio medium wave band (520 kHz - 1710 kHz, exact range is hardware dependent).</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
@@ -532,6 +567,39 @@ Lang1/Lang1</entry>
       </tgroup>
     </table>
   </refsect1>
+    <table pgwide="1" frame="none" id="radio-bands">
+      <title>Radio Band Types</title>
+      <tgroup cols="2">
+	&cs-str;
+	<tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_DEFAULT</constant>&nbsp;</entry>
+		      <entry>This is the default band, which should be the widest frequency range supported by
+		      the hardware. This band is always available.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_FM_EUROPE_US</constant>&nbsp;</entry>
+		      <entry>FM radio European or US band (87.5 Mhz - 108 MHz, exact range is hardware dependent).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_FM_JAPAN</constant>&nbsp;</entry>
+		      <entry>FM radio Japan band (76 MHz - 90 MHz, exact range is hardware dependent).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_FM_RUSSIAN</constant>&nbsp;</entry>
+		      <entry>FM radio OIRT or Russian band (65.8 MHz - 74 MHz, exact range is hardware dependent).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_FM_WEATHER</constant>&nbsp;</entry>
+		      <entry>FM radio weather band (162.4 MHz - 162.55 MHz, exact range is hardware dependent).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_TUNER_BAND_AM_MW</constant>&nbsp;</entry>
+		      <entry>AM radio medium wave band (520 kHz - 1710 kHz, exact range is hardware dependent).</entry>
+		    </row>
+	</tbody>
+      </tgroup>
+    </table>
 
   <refsect1>
     &return-value;
@@ -541,7 +609,14 @@ Lang1/Lang1</entry>
 	<term><errorcode>EINVAL</errorcode></term>
 	<listitem>
 	  <para>The &v4l2-tuner; <structfield>index</structfield> is
-out of bounds.</para>
+out of bounds or the <structfield>band</structfield> is invalid.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><errorcode>EBUSY</errorcode></term>
+	<listitem>
+	  <para>An attempt was made to change the frequency band while a hardware
+frequency seek was in progress.</para>
 	</listitem>
       </varlistentry>
     </variablelist>
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..0d684d4 100644
--- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
@@ -49,7 +49,8 @@
   <refsect1>
     <title>Description</title>
 
-    <para>Start a hardware frequency seek from the current frequency.
+    <para>Start a hardware frequency seek from the current frequency covering
+the current frequency band.
 To do this applications initialize the <structfield>tuner</structfield>,
 <structfield>type</structfield>, <structfield>seek_upward</structfield>,
 <structfield>spacing</structfield> and
-- 
1.7.10

[RFCv2 PATCH 6/6] V4L2 spec: clarify a few modulator issues.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/common.xml |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml
index 4101aeb..b91d253 100644
--- a/Documentation/DocBook/media/v4l/common.xml
+++ b/Documentation/DocBook/media/v4l/common.xml
@@ -464,14 +464,14 @@ The <structfield>type</structfield> field of the respective
 <structfield>tuner</structfield> field contains the index number of
 the tuner.</para>
 
-      <para>Radio devices have exactly one tuner with index zero, no
+      <para>Radio input devices have exactly one tuner with index zero, no
 video inputs.</para>
 
       <para>To query and change tuner properties applications use the
 &VIDIOC-G-TUNER; and &VIDIOC-S-TUNER; ioctl, respectively. The
 &v4l2-tuner; returned by <constant>VIDIOC_G_TUNER</constant> also
 contains signal status information applicable when the tuner of the
-current video input, or a radio tuner is queried. Note that
+current video or radio input is queried. Note that
 <constant>VIDIOC_S_TUNER</constant> does not switch the current tuner,
 when there is more than one at all. The tuner is solely determined by
 the current video input. Drivers must support both ioctls and set the
@@ -491,8 +491,17 @@ the modulator. The <structfield>type</structfield> field of the
 respective &v4l2-output; returned by the &VIDIOC-ENUMOUTPUT; ioctl is
 set to <constant>V4L2_OUTPUT_TYPE_MODULATOR</constant> and its
 <structfield>modulator</structfield> field contains the index number
-of the modulator. This specification does not define radio output
-devices.</para>
+of the modulator.</para>
+
+      <para>Radio output devices have exactly one modulator with index
+zero, no video outputs.</para>
+
+      <para>A video or radio device cannot support both a tuner and a
+modulator. Two separate device nodes will have to be used for such
+hardware, one that supports the tuner functionality and one that supports
+the modulator functionality. The reason is a limitation with the
+&VIDIOC-S-FREQUENCY; ioctl where you cannot specify whether the frequency
+is for a tuner or a modulator.</para>
 
       <para>To query and change modulator properties applications use
 the &VIDIOC-G-MODULATOR; and &VIDIOC-S-MODULATOR; ioctl. Note that
-- 
1.7.10

Re: [RFCv2 PATCH 0/5] Add hwseek caps and frequency bands

[Hans de Goede]

Hi,

Looks good, the entire series is:

Acked-by: Hans de Goede <hdegoede@redhat.com>

I was thinking that it would be a good idea to add a:
#define V4L2_TUNER_CAP_BANDS_MASK 0x001f0000

to videodev2.h, which apps can then easily use to test
if the driver supports any bands other then the default,
and decide to show band selection elements of the UI or
not based on a test on the tuner-caps using that mask.

This can be done in a separate patch, or merged into
"PATCH 4/6 videodev2.h: add frequency band information"

Regards,

Hans



 >
 >


On 05/28/2012 12:46 PM, Hans Verkuil wrote:
> Changes since v1:
>
> - Fixed typo in second patch
> - Patch 5 now only contains the part about frequency bands
> - Patch 6 contains only (I hope) a non-controversial clarification
> regarding modulators (and a small change making a line more understandable).
>
> Regards,
>
> 	Hans
>
> This patch series adds improved hwseek support as discussed here:
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg45957.html
>
> and on irc:
>
> http://linuxtv.org/irc/v4l/index.php?date=2012-05-26
>
>  From the RFC I have implemented/documented items 1-4 and 6a. I decided
> not to go with option 6b. This may be added in the future if there is a
> clear need.
>
> The addition of the frequency band came out of this discussion:
>
> http://www.spinics.net/lists/linux-media/msg48272.html
>
> Regards,
>
>          Hans
>

Re: [RFCv2 PATCH 0/5] Add hwseek caps and frequency bands

[Hans Verkuil]

On Mon May 28 2012 13:20:47 Hans de Goede wrote:
> Hi,
> 
> Looks good, the entire series is:
> 
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> 
> I was thinking that it would be a good idea to add a:
> #define V4L2_TUNER_CAP_BANDS_MASK 0x001f0000
> 
> to videodev2.h, which apps can then easily use to test
> if the driver supports any bands other then the default,
> and decide to show band selection elements of the UI or
> not based on a test on the tuner-caps using that mask.
> 
> This can be done in a separate patch, or merged into
> "PATCH 4/6 videodev2.h: add frequency band information"

Good idea, I've merged it into patch 4 and 5 (documenting it).

It's here:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/bands

Regards,

	Hans

Re: [RFCv2 PATCH 0/5] Add hwseek caps and frequency bands

[Hans de Goede]

Hi,

On 05/28/2012 01:58 PM, Hans Verkuil wrote:
> On Mon May 28 2012 13:20:47 Hans de Goede wrote:
>> Hi,
>>
>> Looks good, the entire series is:
>>
>> Acked-by: Hans de Goede<hdegoede@redhat.com>
>>
>> I was thinking that it would be a good idea to add a:
>> #define V4L2_TUNER_CAP_BANDS_MASK 0x001f0000
>>
>> to videodev2.h, which apps can then easily use to test
>> if the driver supports any bands other then the default,
>> and decide to show band selection elements of the UI or
>> not based on a test on the tuner-caps using that mask.
>>
>> This can be done in a separate patch, or merged into
>> "PATCH 4/6 videodev2.h: add frequency band information"
>
> Good idea, I've merged it into patch 4 and 5 (documenting it).
>
> It's here:
>
> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/bands

New version still look good, so the entire series still is:

Acked-by: Hans de Goede<hdegoede@redhat.com>

:)

Regards,

Hans