All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] Add frequency band information
@ 2012-07-02 14:15 Hans Verkuil
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
  2012-07-02 14:42 ` [RFC PATCH 0/6] Add frequency band information Hans de Goede
  0 siblings, 2 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media; +Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab

Hi all,

This patch series adds support for the new VIDIOC_ENUM_FREQ_BANDS ioctl that
tells userspace if a tuner supports multiple frequency bands.

This API is based on earlier attempts:

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

And an irc discussion:

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

This irc discussion also talked about adding rangelow/high to the v4l2_hw_freq_seek
struct, but I decided not to do that. The hwseek additions are independent to this
patch series, and I think it is best if Hans de Goede does that part so that that
API change can be made together with a driver that actually uses it.

In order to show how the new ioctl is used, this patch series adds support for it
to the very, very old radio-cadet driver.

Comments are welcome!

Regards,

	Hans

PS: Mauro, I haven't been able to work on the radio profile yet, so that's not
included.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-02 14:15 [RFC PATCH 0/6] Add frequency band information Hans Verkuil
@ 2012-07-02 14:15 ` Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 2/6] v4l2: add core support for the new VIDIOC_ENUM_FREQ_BANDS ioctl Hans Verkuil
                     ` (5 more replies)
  2012-07-02 14:42 ` [RFC PATCH 0/6] Add frequency band information Hans de Goede
  1 sibling, 6 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

Add a new ioctl to enumerate the supported frequency bands of a tuner.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index f79d0cc..d54ec6e 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
 
 /*  Flags for the 'rxsubchans' field */
 #define V4L2_TUNER_SUB_MONO		0x0001
@@ -2066,19 +2067,30 @@ struct v4l2_modulator {
 #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
 
 struct v4l2_frequency {
-	__u32		      tuner;
-	__u32		      type;	/* enum v4l2_tuner_type */
-	__u32		      frequency;
-	__u32		      reserved[8];
+	__u32	tuner;
+	__u32	type;	/* enum v4l2_tuner_type */
+	__u32	frequency;
+	__u32	reserved[8];
+};
+
+struct v4l2_frequency_band {
+	__u32	tuner;
+	__u32	type;	/* enum v4l2_tuner_type */
+	__u32	index;
+	__u32	capability;
+	__u32	rangelow;
+	__u32	rangehigh;
+	__u8	name[32];
+	__u32	reserved[6];
 };
 
 struct v4l2_hw_freq_seek {
-	__u32		      tuner;
-	__u32		      type;	/* enum v4l2_tuner_type */
-	__u32		      seek_upward;
-	__u32		      wrap_around;
-	__u32		      spacing;
-	__u32		      reserved[7];
+	__u32	tuner;
+	__u32	type;	/* enum v4l2_tuner_type */
+	__u32	seek_upward;
+	__u32	wrap_around;
+	__u32	spacing;
+	__u32	reserved[7];
 };
 
 /*
@@ -2646,6 +2658,10 @@ struct v4l2_create_buffers {
 #define VIDIOC_QUERY_DV_TIMINGS  _IOR('V', 99, struct v4l2_dv_timings)
 #define VIDIOC_DV_TIMINGS_CAP   _IOWR('V', 100, struct v4l2_dv_timings_cap)
 
+/* Experimental, this ioctl may change over the next couple of kernel
+   versions. */
+#define VIDIOC_ENUM_FREQ_BANDS	_IOWR('V', 101, struct v4l2_frequency_band)
+
 /* Reminder: when adding new ioctls please add support for them to
    drivers/media/video/v4l2-compat-ioctl32.c as well! */
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 2/6] v4l2: add core support for the new VIDIOC_ENUM_FREQ_BANDS ioctl.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
@ 2012-07-02 14:15   ` Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 3/6] v4l2 spec: add VIDIOC_ENUM_FREQ_BANDS documentation Hans Verkuil
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

This adds the usual core support code for this new ioctl.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/v4l2-compat-ioctl32.c |    1 +
 drivers/media/video/v4l2-dev.c            |    1 +
 drivers/media/video/v4l2-ioctl.c          |   21 +++++++++++++++++++++
 include/media/v4l2-ioctl.h                |    2 ++
 4 files changed, 25 insertions(+)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 5327ad3..81e5129 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -1026,6 +1026,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_ENUM_DV_TIMINGS:
 	case VIDIOC_QUERY_DV_TIMINGS:
 	case VIDIOC_DV_TIMINGS_CAP:
+	case VIDIOC_ENUM_FREQ_BANDS:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;
 
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 83dbb2d..4e34808 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -663,6 +663,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
 	SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
 	SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+	SET_VALID_IOCTL(ops, VIDIOC_ENUM_FREQ_BANDS, vidioc_enum_freq_bands);
 	SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
 	SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
 #ifdef CONFIG_VIDEO_ADV_DEBUG
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index d7fa896..b8e5ef7 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -284,6 +284,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO(VIDIOC_ENUM_DV_TIMINGS, 0),
 	IOCTL_INFO(VIDIOC_QUERY_DV_TIMINGS, 0),
 	IOCTL_INFO(VIDIOC_DV_TIMINGS_CAP, 0),
+	IOCTL_INFO(VIDIOC_ENUM_FREQ_BANDS, 0),
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
@@ -1765,6 +1766,26 @@ static long __video_do_ioctl(struct file *file,
 			ret = ops->vidioc_s_frequency(file, fh, p);
 		break;
 	}
+	case VIDIOC_ENUM_FREQ_BANDS:
+	{
+		struct v4l2_frequency_band *p = arg;
+		enum v4l2_tuner_type type;
+
+		type = (vfd->vfl_type == VFL_TYPE_RADIO) ?
+			V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
+		if (p->type != type)
+			ret = -EINVAL;
+		else
+			ret = ops->vidioc_enum_freq_bands(file, fh, p);
+		if (!ret)
+			dbgarg(cmd, "tuner=%d, index=%d, name=%s, "
+					"capability=0x%x, rangelow=%d, "
+					"rangehigh=%d\n",
+					p->tuner, p->index, p->name,
+					p->capability, p->rangelow,
+					p->rangehigh);
+		break;
+	}
 	case VIDIOC_G_SLICED_VBI_CAP:
 	{
 		struct v4l2_sliced_vbi_cap *p = arg;
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index d8b76f7..6d706ab 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -230,6 +230,8 @@ struct v4l2_ioctl_ops {
 					struct v4l2_frequency *a);
 	int (*vidioc_s_frequency)      (struct file *file, void *fh,
 					struct v4l2_frequency *a);
+	int (*vidioc_enum_freq_bands) (struct file *file, void *fh,
+				    struct v4l2_frequency_band *band);
 
 	/* Sliced VBI cap */
 	int (*vidioc_g_sliced_vbi_cap) (struct file *file, void *fh,
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 3/6] v4l2 spec: add VIDIOC_ENUM_FREQ_BANDS documentation.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 2/6] v4l2: add core support for the new VIDIOC_ENUM_FREQ_BANDS ioctl Hans Verkuil
@ 2012-07-02 14:15   ` Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 4/6] radio-cadet: upgrade to latest frameworks Hans Verkuil
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/compat.xml         |   12 ++
 Documentation/DocBook/media/v4l/v4l2.xml           |    6 +
 .../DocBook/media/v4l/vidioc-enum-freq-bands.xml   |  152 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-g-frequency.xml       |    7 +-
 Documentation/DocBook/media/v4l/vidioc-g-tuner.xml |   26 +++-
 5 files changed, 194 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index ea42ef8..2ec8000 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2458,6 +2458,15 @@ details.</para>
       </orderedlist>
     </section>
 
+    <section>
+      <title>V4L2 in Linux 3.6</title>
+      <orderedlist>
+        <listitem>
+	  <para>Added support for frequency band enumerations: &VIDIOC-ENUM-FREQ-BANDS;.</para>
+        </listitem>
+      </orderedlist>
+    </section>
+
     <section id="other">
       <title>Relation of V4L2 to other Linux multimedia APIs</title>
 
@@ -2587,6 +2596,9 @@ ioctls.</para>
 	  <para><link linkend="v4l2-auto-focus-area"><constant>
 	  V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para>
         </listitem>
+        <listitem>
+	  <para>Support for frequency band enumeration: &VIDIOC-ENUM-FREQ-BANDS; ioctl.</para>
+        </listitem>
       </itemizedlist>
     </section>
 
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 008c2d7..c6e307b 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -140,6 +140,11 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>3.6</revnumber>
+	<date>2012-07-02</date>
+	<authorinitials>hv</authorinitials>
+	<revremark>Added VIDIOC_ENUM_FREQ_BANDS.
+	</revremark>
 	<revnumber>3.5</revnumber>
 	<date>2012-05-07</date>
 	<authorinitials>sa, sn</authorinitials>
@@ -534,6 +539,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-enum-fmt;
     &sub-enum-framesizes;
     &sub-enum-frameintervals;
+    &sub-enum-freq-bands;
     &sub-enuminput;
     &sub-enumoutput;
     &sub-enumstd;
diff --git a/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml b/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml
new file mode 100644
index 0000000..27e264f
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml
@@ -0,0 +1,152 @@
+<refentry id="vidioc-enum-freq-bands">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_ENUM_FREQ_BANDS</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_ENUM_FREQ_BANDS</refname>
+    <refpurpose>Enumerate supported frequency bands</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_frequency_band
+*<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_ENUM_FREQ_BANDS</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <note>
+      <title>Experimental</title>
+      <para>This is an <link linkend="experimental"> experimental </link>
+      interface and may change in the future.</para>
+    </note>
+
+    <para>Enumerates the frequency bands that a tuner or modulator supports.
+To do this applications initialize the <structfield>tuner</structfield>,
+<structfield>type</structfield> and <structfield>index</structfield> fields,
+and zero out the <structfield>reserved</structfield> array of a &v4l2-frequency-band; and
+call the <constant>VIDIOC_ENUM_FREQ_BANDS</constant> ioctl with a pointer
+to this structure.</para>
+
+    <para>This ioctl is supported if the <constant>V4L2_TUNER_CAP_FREQ_BANDS</constant> capability
+    of the corresponding tuner/modulator is set.</para>
+
+    <table pgwide="1" frame="none" id="v4l2-frequency-band">
+      <title>struct <structname>v4l2_frequency_band</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>tuner</structfield></entry>
+	    <entry>The tuner or modulator index number. This is the
+same value as in the &v4l2-input; <structfield>tuner</structfield>
+field and the &v4l2-tuner; <structfield>index</structfield> field, or
+the &v4l2-output; <structfield>modulator</structfield> field and the
+&v4l2-modulator; <structfield>index</structfield> field.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+	    <entry>The tuner type. This is the same value as in the
+&v4l2-tuner; <structfield>type</structfield> field. The type must be set
+to <constant>V4L2_TUNER_RADIO</constant> for <filename>/dev/radioX</filename>
+device nodes, and to <constant>V4L2_TUNER_ANALOG_TV</constant>
+for all others. Set this field to <constant>V4L2_TUNER_RADIO</constant> for
+modulators (currently only radio modulators are supported).
+See <xref linkend="v4l2-tuner-type" /></entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>index</structfield></entry>
+	    <entry>Identifies the frequency band, set by the application.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>capability</structfield></entry>
+	    <entry spanname="hspan">The tuner/modulator capability flags for
+this frequency band, see <xref linkend="tuner-capability" />. The <constant>V4L2_TUNER_CAP_LOW</constant>
+capability must be the same for all frequency bands of the selected tuner/modulator.
+So either all bands have that capability set, or none of them have that capability.</entry>
+	  </row>
+	  <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>
+flag <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
+Hz, for this frequency band.</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>
+flag <constant>V4L2_TUNER_CAP_LOW</constant> is set, in units of 62.5
+Hz, for this frequency band.</entry>
+	  </row>
+	  <row>
+	    <entry>__u8</entry>
+	    <entry><structfield>name</structfield>[32]</entry>
+	    <entry spanname="hspan">The name of this frequency band. This name
+can be used in user interfaces.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[6]</entry>
+	    <entry>Reserved for future extensions. Applications and drivers
+	    must set the array to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>The <structfield>tuner</structfield> or <structfield>index</structfield>
+is out of bounds or the <structfield>type</structfield> field is wrong.</para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
index 40e58a4..c7a1c46 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml
@@ -98,11 +98,12 @@ the &v4l2-output; <structfield>modulator</structfield> field and the
 	    <entry>__u32</entry>
 	    <entry><structfield>type</structfield></entry>
 	    <entry>The tuner type. This is the same value as in the
-&v4l2-tuner; <structfield>type</structfield> field. See The type must be set
+&v4l2-tuner; <structfield>type</structfield> field. The type must be set
 to <constant>V4L2_TUNER_RADIO</constant> for <filename>/dev/radioX</filename>
 device nodes, and to <constant>V4L2_TUNER_ANALOG_TV</constant>
-for all others. The field is not applicable to modulators, &ie; ignored
-by drivers. See <xref linkend="v4l2-tuner-type" /></entry>
+for all others. Set this field to <constant>V4L2_TUNER_RADIO</constant> for
+modulators (currently only radio modulators are supported).
+See <xref linkend="v4l2-tuner-type" /></entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
index 95d5371..288b319 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
@@ -119,10 +119,14 @@ field is not quite clear.--></para></entry>
 <xref linkend="tuner-capability" />. Audio flags indicate the ability
 to decode audio subprograms. They will <emphasis>not</emphasis>
 change, for example with the current video standard.</para><para>When
-the structure refers to a radio tuner only the
-<constant>V4L2_TUNER_CAP_LOW</constant>,
-<constant>V4L2_TUNER_CAP_STEREO</constant> and
-<constant>V4L2_TUNER_CAP_RDS</constant> flags can be set.</para></entry>
+the structure refers to a radio tuner the
+<constant>V4L2_TUNER_CAP_LANG1</constant>,
+<constant>V4L2_TUNER_CAP_LANG2</constant> and
+<constant>V4L2_TUNER_CAP_NORM</constant> flags can't be used.</para>
+<para>If multiple frequency bands are supported (<constant>V4L2_TUNER_CAP_FREQ_BANDS</constant>
+is set), then <structfield>capability</structfield> is the union of all
+<structfield>capability></structfield> fields of each &v4l2-frequency-band;.
+</para></entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -130,7 +134,9 @@ the structure refers to a radio tuner only the
 	    <entry spanname="hspan">The lowest tunable frequency 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>
+Hz. If multiple frequency bands are supported (<constant>V4L2_TUNER_CAP_FREQ_BANDS</constant>
+is set), then <structfield>rangelow</structfield> is the lowest frequency
+of all the frequency bands.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -138,7 +144,9 @@ Hz.</entry>
 	    <entry spanname="hspan">The highest tunable frequency 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>
+Hz. If multiple frequency bands are supported (<constant>V4L2_TUNER_CAP_FREQ_BANDS</constant>
+is set), then <structfield>rangehigh</structfield> is the highest frequency
+of all the frequency bands.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -340,6 +348,12 @@ 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_FREQ_BANDS</constant></entry>
+	<entry>0x0400</entry>
+	<entry>There are multiple frequency bands and the &VIDIOC-ENUM-FREQ-BANDS;
+	ioctl can be used to enumerate them.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 4/6] radio-cadet: upgrade to latest frameworks.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 2/6] v4l2: add core support for the new VIDIOC_ENUM_FREQ_BANDS ioctl Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 3/6] v4l2 spec: add VIDIOC_ENUM_FREQ_BANDS documentation Hans Verkuil
@ 2012-07-02 14:15   ` Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 5/6] radio-cadet: fix RDS handling Hans Verkuil
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

- add control framework
- use core locking
- use V4L2_TUNER_CAP_LOW
- remove volume support: there is no hardware volume control

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/radio/radio-cadet.c |  243 +++++++++++++------------------------
 1 file changed, 83 insertions(+), 160 deletions(-)

diff --git a/drivers/media/radio/radio-cadet.c b/drivers/media/radio/radio-cadet.c
index 16a089f..93536b7 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -41,6 +41,9 @@
 #include <linux/io.h>		/* outb, outb_p			*/
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 
 MODULE_AUTHOR("Fred Gleason, Russell Kroll, Quay Lu, Donald Song, Jason Lewis, Scott McGrath, William McGrath");
 MODULE_DESCRIPTION("A driver for the ADS Cadet AM/FM/RDS radio card.");
@@ -61,8 +64,8 @@ module_param(radio_nr, int, 0);
 struct cadet {
 	struct v4l2_device v4l2_dev;
 	struct video_device vdev;
+	struct v4l2_ctrl_handler ctrl_handler;
 	int io;
-	int users;
 	int curtuner;
 	int tunestat;
 	int sigstrength;
@@ -94,11 +97,9 @@ static int cadet_getstereo(struct cadet *dev)
 	if (dev->curtuner != 0)	/* Only FM has stereo capability! */
 		return V4L2_TUNER_SUB_MONO;
 
-	mutex_lock(&dev->lock);
 	outb(7, dev->io);          /* Select tuner control */
 	if ((inb(dev->io + 1) & 0x40) == 0)
 		ret = V4L2_TUNER_SUB_STEREO;
-	mutex_unlock(&dev->lock);
 	return ret;
 }
 
@@ -111,8 +112,6 @@ static unsigned cadet_gettune(struct cadet *dev)
 	 * Prepare for read
 	 */
 
-	mutex_lock(&dev->lock);
-
 	outb(7, dev->io);       /* Select tuner control */
 	curvol = inb(dev->io + 1); /* Save current volume/mute setting */
 	outb(0x00, dev->io + 1);  /* Ensure WRITE-ENABLE is LOW */
@@ -134,8 +133,6 @@ static unsigned cadet_gettune(struct cadet *dev)
 	 * Restore volume/mute setting
 	 */
 	outb(curvol, dev->io + 1);
-	mutex_unlock(&dev->lock);
-
 	return fifo;
 }
 
@@ -161,7 +158,7 @@ static unsigned cadet_getfreq(struct cadet *dev)
 			fifo = fifo >> 1;
 		}
 		freq -= 10700000;           /* IF frequency is 10.7 MHz */
-		freq = (freq * 16) / 1000000;   /* Make it 1/16 MHz */
+		freq = (freq * 16) / 1000;   /* Make it 1/16 kHz */
 	}
 	if (dev->curtuner == 1)    /* AM */
 		freq = ((fifo & 0x7fff) - 2010) * 16;
@@ -174,8 +171,6 @@ static void cadet_settune(struct cadet *dev, unsigned fifo)
 	int i;
 	unsigned test;
 
-	mutex_lock(&dev->lock);
-
 	outb(7, dev->io);                /* Select tuner control */
 	/*
 	 * Write the shift register
@@ -194,7 +189,6 @@ static void cadet_settune(struct cadet *dev, unsigned fifo)
 		test = 0x1c | ((fifo >> 23) & 0x02);
 		outb(test, dev->io + 1);
 	}
-	mutex_unlock(&dev->lock);
 }
 
 static void cadet_setfreq(struct cadet *dev, unsigned freq)
@@ -209,7 +203,7 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 	fifo = 0;
 	if (dev->curtuner == 0) {    /* FM */
 		test = 102400;
-		freq = (freq * 1000) / 16;       /* Make it kHz */
+		freq = freq / 16;       /* Make it kHz */
 		freq += 10700;               /* IF is 10700 kHz */
 		for (i = 0; i < 14; i++) {
 			fifo = fifo << 1;
@@ -229,10 +223,8 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 	 * Save current volume/mute setting
 	 */
 
-	mutex_lock(&dev->lock);
 	outb(7, dev->io);                /* Select tuner control */
 	curvol = inb(dev->io + 1);
-	mutex_unlock(&dev->lock);
 
 	/*
 	 * Tune the card
@@ -240,10 +232,8 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 	for (j = 3; j > -1; j--) {
 		cadet_settune(dev, fifo | (j << 16));
 
-		mutex_lock(&dev->lock);
 		outb(7, dev->io);         /* Select tuner control */
 		outb(curvol, dev->io + 1);
-		mutex_unlock(&dev->lock);
 
 		msleep(100);
 
@@ -257,32 +247,6 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 }
 
 
-static int cadet_getvol(struct cadet *dev)
-{
-	int ret = 0;
-
-	mutex_lock(&dev->lock);
-
-	outb(7, dev->io);                /* Select tuner control */
-	if ((inb(dev->io + 1) & 0x20) != 0)
-		ret = 0xffff;
-
-	mutex_unlock(&dev->lock);
-	return ret;
-}
-
-
-static void cadet_setvol(struct cadet *dev, int vol)
-{
-	mutex_lock(&dev->lock);
-	outb(7, dev->io);                /* Select tuner control */
-	if (vol > 0)
-		outb(0x20, dev->io + 1);
-	else
-		outb(0x00, dev->io + 1);
-	mutex_unlock(&dev->lock);
-}
-
 static void cadet_handler(unsigned long data)
 {
 	struct cadet *dev = (void *)data;
@@ -337,18 +301,19 @@ static ssize_t cadet_read(struct file *file, char __user *data, size_t count, lo
 		add_timer(&dev->readtimer);
 	}
 	if (dev->rdsin == dev->rdsout) {
-		mutex_unlock(&dev->lock);
-		if (file->f_flags & O_NONBLOCK)
-			return -EWOULDBLOCK;
+		if (file->f_flags & O_NONBLOCK) {
+			i = -EWOULDBLOCK;
+			goto unlock;
+		}
 		interruptible_sleep_on(&dev->read_queue);
-		mutex_lock(&dev->lock);
 	}
 	while (i < count && dev->rdsin != dev->rdsout)
 		readbuf[i++] = dev->rdsbuf[dev->rdsout++];
-	mutex_unlock(&dev->lock);
 
 	if (copy_to_user(data, readbuf, i))
-		return -EFAULT;
+		i = -EFAULT;
+unlock:
+	mutex_unlock(&dev->lock);
 	return i;
 }
 
@@ -359,8 +324,9 @@ static int vidioc_querycap(struct file *file, void *priv,
 	strlcpy(v->driver, "ADS Cadet", sizeof(v->driver));
 	strlcpy(v->card, "ADS Cadet", sizeof(v->card));
 	strlcpy(v->bus_info, "ISA", sizeof(v->bus_info));
-	v->capabilities = V4L2_CAP_TUNER | V4L2_CAP_RADIO |
+	v->device_caps = V4L2_CAP_TUNER | V4L2_CAP_RADIO |
 			  V4L2_CAP_READWRITE | V4L2_CAP_RDS_CAPTURE;
+	v->capabilities = v->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
 
@@ -374,20 +340,11 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 	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 */
+			V4L2_TUNER_CAP_RDS_BLOCK_IO | V4L2_TUNER_CAP_LOW;
+		v->rangelow = 1400000;     /* 87.5 MHz */
+		v->rangehigh = 1728000;    /* 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->audmode = V4L2_TUNER_MODE_STEREO;
 		v->rxsubchans |= V4L2_TUNER_SUB_RDS;
 		break;
 	case 1:
@@ -408,11 +365,8 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 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;
 }
 
@@ -421,7 +375,8 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 {
 	struct cadet *dev = video_drvdata(file);
 
-	f->tuner = dev->curtuner;
+	if (f->tuner > 1)
+		return -EINVAL;
 	f->type = V4L2_TUNER_RADIO;
 	f->frequency = cadet_getfreq(dev);
 	return 0;
@@ -435,101 +390,52 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	if (f->type != V4L2_TUNER_RADIO)
 		return -EINVAL;
-	if (dev->curtuner == 0 && (f->frequency < 1400 || f->frequency > 1728))
-		return -EINVAL;
-	if (dev->curtuner == 1 && (f->frequency < 8320 || f->frequency > 26400))
+	if (f->tuner == 0) {
+		if (f->frequency < 1400000)
+			f->frequency = 1400000;
+		else if (f->frequency > 1728000)
+			f->frequency = 1728000;
+	} else if (f->tuner == 1) {
+		if (f->frequency < 8320)
+			f->frequency = 8320;
+		else if (f->frequency > 26400)
+			f->frequency = 26400;
+	} else
 		return -EINVAL;
 	cadet_setfreq(dev, f->frequency);
 	return 0;
 }
 
-static int vidioc_queryctrl(struct file *file, void *priv,
-				struct v4l2_queryctrl *qc)
+static int cadet_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	switch (qc->id) {
-	case V4L2_CID_AUDIO_MUTE:
-		return v4l2_ctrl_query_fill(qc, 0, 1, 1, 1);
-	case V4L2_CID_AUDIO_VOLUME:
-		return v4l2_ctrl_query_fill(qc, 0, 0xff, 1, 0xff);
-	}
-	return -EINVAL;
-}
-
-static int vidioc_g_ctrl(struct file *file, void *priv,
-				struct v4l2_control *ctrl)
-{
-	struct cadet *dev = video_drvdata(file);
+	struct cadet *dev = container_of(ctrl->handler, struct cadet, ctrl_handler);
 
 	switch (ctrl->id) {
-	case V4L2_CID_AUDIO_MUTE: /* TODO: Handle this correctly */
-		ctrl->value = (cadet_getvol(dev) == 0);
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-		ctrl->value = cadet_getvol(dev);
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int vidioc_s_ctrl(struct file *file, void *priv,
-				struct v4l2_control *ctrl)
-{
-	struct cadet *dev = video_drvdata(file);
-
-	switch (ctrl->id){
-	case V4L2_CID_AUDIO_MUTE: /* TODO: Handle this correctly */
-		if (ctrl->value)
-			cadet_setvol(dev, 0);
+	case V4L2_CID_AUDIO_MUTE:
+		outb(7, dev->io);                /* Select tuner control */
+		if (ctrl->val)
+			outb(0x00, dev->io + 1);
 		else
-			cadet_setvol(dev, 0xffff);
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-		cadet_setvol(dev, ctrl->value);
-		break;
-	default:
-		return -EINVAL;
+			outb(0x20, dev->io + 1);
+		return 0;
 	}
-	return 0;
-}
-
-static int vidioc_g_input(struct file *filp, void *priv, unsigned int *i)
-{
-	*i = 0;
-	return 0;
-}
-
-static int vidioc_s_input(struct file *filp, void *priv, unsigned int i)
-{
-	return i ? -EINVAL : 0;
-}
-
-static int vidioc_g_audio(struct file *file, void *priv,
-				struct v4l2_audio *a)
-{
-	a->index = 0;
-	strlcpy(a->name, "Radio", sizeof(a->name));
-	a->capability = V4L2_AUDCAP_STEREO;
-	return 0;
-}
-
-static int vidioc_s_audio(struct file *file, void *priv,
-				struct v4l2_audio *a)
-{
-	return a->index ? -EINVAL : 0;
+	return -EINVAL;
 }
 
 static int cadet_open(struct file *file)
 {
 	struct cadet *dev = video_drvdata(file);
+	int err;
 
 	mutex_lock(&dev->lock);
-	dev->users++;
-	if (1 == dev->users)
+	err = v4l2_fh_open(file);
+	if (err)
+		goto fail;
+	if (v4l2_fh_is_singular_file(file))
 		init_waitqueue_head(&dev->read_queue);
+fail:
 	mutex_unlock(&dev->lock);
-	return 0;
+	return err;
 }
 
 static int cadet_release(struct file *file)
@@ -537,11 +443,11 @@ static int cadet_release(struct file *file)
 	struct cadet *dev = video_drvdata(file);
 
 	mutex_lock(&dev->lock);
-	dev->users--;
-	if (0 == dev->users) {
+	if (v4l2_fh_is_singular_file(file) && dev->rdsstat) {
 		del_timer_sync(&dev->readtimer);
 		dev->rdsstat = 0;
 	}
+	v4l2_fh_release(file);
 	mutex_unlock(&dev->lock);
 	return 0;
 }
@@ -549,11 +455,12 @@ static int cadet_release(struct file *file)
 static unsigned int cadet_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct cadet *dev = video_drvdata(file);
+	unsigned int res = v4l2_ctrl_poll(file, wait);
 
 	poll_wait(file, &dev->read_queue, wait);
 	if (dev->rdsin != dev->rdsout)
-		return POLLIN | POLLRDNORM;
-	return 0;
+		res |= POLLIN | POLLRDNORM;
+	return res;
 }
 
 
@@ -572,13 +479,13 @@ static const struct v4l2_ioctl_ops cadet_ioctl_ops = {
 	.vidioc_s_tuner     = vidioc_s_tuner,
 	.vidioc_g_frequency = vidioc_g_frequency,
 	.vidioc_s_frequency = vidioc_s_frequency,
-	.vidioc_queryctrl   = vidioc_queryctrl,
-	.vidioc_g_ctrl      = vidioc_g_ctrl,
-	.vidioc_s_ctrl      = vidioc_s_ctrl,
-	.vidioc_g_audio     = vidioc_g_audio,
-	.vidioc_s_audio     = vidioc_s_audio,
-	.vidioc_g_input     = vidioc_g_input,
-	.vidioc_s_input     = vidioc_s_input,
+	.vidioc_log_status  = v4l2_ctrl_log_status,
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+};
+
+static const struct v4l2_ctrl_ops cadet_ctrl_ops = {
+	.s_ctrl = cadet_s_ctrl,
 };
 
 #ifdef CONFIG_PNP
@@ -648,7 +555,8 @@ static int __init cadet_init(void)
 {
 	struct cadet *dev = &cadet_card;
 	struct v4l2_device *v4l2_dev = &dev->v4l2_dev;
-	int res;
+	struct v4l2_ctrl_handler *hdl;
+	int res = -ENODEV;
 
 	strlcpy(v4l2_dev->name, "cadet", sizeof(v4l2_dev->name));
 	mutex_init(&dev->lock);
@@ -680,23 +588,37 @@ static int __init cadet_init(void)
 		goto fail;
 	}
 
+	hdl = &dev->ctrl_handler;
+	v4l2_ctrl_handler_init(hdl, 2);
+	v4l2_ctrl_new_std(hdl, &cadet_ctrl_ops,
+			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+	v4l2_dev->ctrl_handler = hdl;
+	if (hdl->error) {
+		res = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register controls\n");
+		goto err_hdl;
+	}
+
 	strlcpy(dev->vdev.name, v4l2_dev->name, sizeof(dev->vdev.name));
 	dev->vdev.v4l2_dev = v4l2_dev;
 	dev->vdev.fops = &cadet_fops;
 	dev->vdev.ioctl_ops = &cadet_ioctl_ops;
 	dev->vdev.release = video_device_release_empty;
+	dev->vdev.lock = &dev->lock;
+	set_bit(V4L2_FL_USE_FH_PRIO, &dev->vdev.flags);
 	video_set_drvdata(&dev->vdev, dev);
 
-	if (video_register_device(&dev->vdev, VFL_TYPE_RADIO, radio_nr) < 0) {
-		v4l2_device_unregister(v4l2_dev);
-		release_region(dev->io, 2);
-		goto fail;
-	}
+	if (video_register_device(&dev->vdev, VFL_TYPE_RADIO, radio_nr) < 0)
+		goto err_hdl;
 	v4l2_info(v4l2_dev, "ADS Cadet Radio Card at 0x%x\n", dev->io);
 	return 0;
+err_hdl:
+	v4l2_ctrl_handler_free(hdl);
+	v4l2_device_unregister(v4l2_dev);
+	release_region(dev->io, 2);
 fail:
 	pnp_unregister_driver(&cadet_pnp_driver);
-	return -ENODEV;
+	return res;
 }
 
 static void __exit cadet_exit(void)
@@ -704,6 +626,7 @@ static void __exit cadet_exit(void)
 	struct cadet *dev = &cadet_card;
 
 	video_unregister_device(&dev->vdev);
+	v4l2_ctrl_handler_free(&dev->ctrl_handler);
 	v4l2_device_unregister(&dev->v4l2_dev);
 	release_region(dev->io, 2);
 	pnp_unregister_driver(&cadet_pnp_driver);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 5/6] radio-cadet: fix RDS handling.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
                     ` (2 preceding siblings ...)
  2012-07-02 14:15   ` [RFC PATCH 4/6] radio-cadet: upgrade to latest frameworks Hans Verkuil
@ 2012-07-02 14:15   ` Hans Verkuil
  2012-07-02 14:15   ` [RFC PATCH 6/6] radio-cadet: implement frequency band enumeration Hans Verkuil
  2012-07-02 17:42   ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Mauro Carvalho Chehab
  5 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

The current RDS code suffered from bit rot. Clean it up and make it work again.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/radio/radio-cadet.c |   56 ++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/media/radio/radio-cadet.c b/drivers/media/radio/radio-cadet.c
index 93536b7..d1fb427 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -71,7 +71,7 @@ struct cadet {
 	int sigstrength;
 	wait_queue_head_t read_queue;
 	struct timer_list readtimer;
-	__u8 rdsin, rdsout, rdsstat;
+	u8 rdsin, rdsout, rdsstat;
 	unsigned char rdsbuf[RDS_BUFFER];
 	struct mutex lock;
 	int reading;
@@ -85,8 +85,8 @@ static struct cadet cadet_card;
  * strength value.  These values are in microvolts of RF at the tuner's input.
  */
 static __u16 sigtable[2][4] = {
-	{  5, 10, 30,  150 },
-	{ 28, 40, 63, 1000 }
+	{ 2185, 4369, 13107, 65535 },
+	{ 1835, 2621,  4128, 65535 }
 };
 
 
@@ -240,10 +240,13 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 		cadet_gettune(dev);
 		if ((dev->tunestat & 0x40) == 0) {   /* Tuned */
 			dev->sigstrength = sigtable[dev->curtuner][j];
-			return;
+			goto reset_rds;
 		}
 	}
 	dev->sigstrength = 0;
+reset_rds:
+	outb(3, dev->io);
+	outb(inb(dev->io + 1) & 0x7f, dev->io + 1);
 }
 
 
@@ -259,7 +262,7 @@ static void cadet_handler(unsigned long data)
 		outb(0x80, dev->io);      /* Select RDS fifo */
 		while ((inb(dev->io) & 0x80) != 0) {
 			dev->rdsbuf[dev->rdsin] = inb(dev->io + 1);
-			if (dev->rdsin == dev->rdsout)
+			if (dev->rdsin + 1 == dev->rdsout)
 				printk(KERN_WARNING "cadet: RDS buffer overflow\n");
 			else
 				dev->rdsin++;
@@ -278,11 +281,21 @@ static void cadet_handler(unsigned long data)
 	 */
 	init_timer(&dev->readtimer);
 	dev->readtimer.function = cadet_handler;
-	dev->readtimer.data = (unsigned long)0;
+	dev->readtimer.data = data;
 	dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
 	add_timer(&dev->readtimer);
 }
 
+static void cadet_start_rds(struct cadet *dev)
+{
+	dev->rdsstat = 1;
+	outb(0x80, dev->io);        /* Select RDS fifo */
+	init_timer(&dev->readtimer);
+	dev->readtimer.function = cadet_handler;
+	dev->readtimer.data = (unsigned long)dev;
+	dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
+	add_timer(&dev->readtimer);
+}
 
 static ssize_t cadet_read(struct file *file, char __user *data, size_t count, loff_t *ppos)
 {
@@ -291,26 +304,21 @@ static ssize_t cadet_read(struct file *file, char __user *data, size_t count, lo
 	int i = 0;
 
 	mutex_lock(&dev->lock);
-	if (dev->rdsstat == 0) {
-		dev->rdsstat = 1;
-		outb(0x80, dev->io);        /* Select RDS fifo */
-		init_timer(&dev->readtimer);
-		dev->readtimer.function = cadet_handler;
-		dev->readtimer.data = (unsigned long)dev;
-		dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
-		add_timer(&dev->readtimer);
-	}
+	if (dev->rdsstat == 0)
+		cadet_start_rds(dev);
 	if (dev->rdsin == dev->rdsout) {
 		if (file->f_flags & O_NONBLOCK) {
 			i = -EWOULDBLOCK;
 			goto unlock;
 		}
+		mutex_unlock(&dev->lock);
 		interruptible_sleep_on(&dev->read_queue);
+		mutex_lock(&dev->lock);
 	}
 	while (i < count && dev->rdsin != dev->rdsout)
 		readbuf[i++] = dev->rdsbuf[dev->rdsout++];
 
-	if (copy_to_user(data, readbuf, i))
+	if (i && copy_to_user(data, readbuf, i))
 		i = -EFAULT;
 unlock:
 	mutex_unlock(&dev->lock);
@@ -345,7 +353,12 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 		v->rangehigh = 1728000;    /* 108.0 MHz */
 		v->rxsubchans = cadet_getstereo(dev);
 		v->audmode = V4L2_TUNER_MODE_STEREO;
-		v->rxsubchans |= V4L2_TUNER_SUB_RDS;
+		outb(3, dev->io);
+		outb(inb(dev->io + 1) & 0x7f, dev->io + 1);
+		mdelay(100);
+		outb(3, dev->io);
+		if (inb(dev->io + 1) & 0x80)
+			v->rxsubchans |= V4L2_TUNER_SUB_RDS;
 		break;
 	case 1:
 		strlcpy(v->name, "AM", sizeof(v->name));
@@ -455,9 +468,16 @@ static int cadet_release(struct file *file)
 static unsigned int cadet_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct cadet *dev = video_drvdata(file);
+	unsigned long req_events = poll_requested_events(wait);
 	unsigned int res = v4l2_ctrl_poll(file, wait);
 
 	poll_wait(file, &dev->read_queue, wait);
+	if (dev->rdsstat == 0 && (req_events & (POLLIN | POLLRDNORM))) {
+		mutex_lock(&dev->lock);
+		if (dev->rdsstat == 0)
+			cadet_start_rds(dev);
+		mutex_unlock(&dev->lock);
+	}
 	if (dev->rdsin != dev->rdsout)
 		res |= POLLIN | POLLRDNORM;
 	return res;
@@ -628,6 +648,8 @@ static void __exit cadet_exit(void)
 	video_unregister_device(&dev->vdev);
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
 	v4l2_device_unregister(&dev->v4l2_dev);
+	outb(7, dev->io);	/* Mute */
+	outb(0x00, dev->io + 1);
 	release_region(dev->io, 2);
 	pnp_unregister_driver(&cadet_pnp_driver);
 }
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 6/6] radio-cadet: implement frequency band enumeration.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
                     ` (3 preceding siblings ...)
  2012-07-02 14:15   ` [RFC PATCH 5/6] radio-cadet: fix RDS handling Hans Verkuil
@ 2012-07-02 14:15   ` Hans Verkuil
  2012-07-02 17:42   ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Mauro Carvalho Chehab
  5 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2012-07-02 14:15 UTC (permalink / raw)
  To: linux-media
  Cc: Hans de Goede, halli manjunatha, Mauro Carvalho Chehab, Hans Verkuil

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

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/radio/radio-cadet.c |  129 +++++++++++++++++++++----------------
 1 file changed, 72 insertions(+), 57 deletions(-)

diff --git a/drivers/media/radio/radio-cadet.c b/drivers/media/radio/radio-cadet.c
index d1fb427..946a4d7 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -66,7 +66,8 @@ struct cadet {
 	struct video_device vdev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	int io;
-	int curtuner;
+	bool is_fm_band;
+	u32 curfreq;
 	int tunestat;
 	int sigstrength;
 	wait_queue_head_t read_queue;
@@ -84,9 +85,9 @@ static struct cadet cadet_card;
  * The V4L API spec does not define any particular unit for the signal
  * strength value.  These values are in microvolts of RF at the tuner's input.
  */
-static __u16 sigtable[2][4] = {
+static u16 sigtable[2][4] = {
+	{ 1835, 2621,  4128, 65535 },
 	{ 2185, 4369, 13107, 65535 },
-	{ 1835, 2621,  4128, 65535 }
 };
 
 
@@ -94,7 +95,7 @@ static int cadet_getstereo(struct cadet *dev)
 {
 	int ret = V4L2_TUNER_SUB_MONO;
 
-	if (dev->curtuner != 0)	/* Only FM has stereo capability! */
+	if (!dev->is_fm_band)	/* Only FM has stereo capability! */
 		return V4L2_TUNER_SUB_MONO;
 
 	outb(7, dev->io);          /* Select tuner control */
@@ -149,20 +150,18 @@ static unsigned cadet_getfreq(struct cadet *dev)
 	/*
 	 * Convert to actual frequency
 	 */
-	if (dev->curtuner == 0) {    /* FM */
-		test = 12500;
-		for (i = 0; i < 14; i++) {
-			if ((fifo & 0x01) != 0)
-				freq += test;
-			test = test << 1;
-			fifo = fifo >> 1;
-		}
-		freq -= 10700000;           /* IF frequency is 10.7 MHz */
-		freq = (freq * 16) / 1000;   /* Make it 1/16 kHz */
+	if (!dev->is_fm_band)    /* AM */
+		return ((fifo & 0x7fff) - 2010) * 16;
+
+	test = 12500;
+	for (i = 0; i < 14; i++) {
+		if ((fifo & 0x01) != 0)
+			freq += test;
+		test = test << 1;
+		fifo = fifo >> 1;
 	}
-	if (dev->curtuner == 1)    /* AM */
-		freq = ((fifo & 0x7fff) - 2010) * 16;
-
+	freq -= 10700000;           /* IF frequency is 10.7 MHz */
+	freq = (freq * 16) / 1000;   /* Make it 1/16 kHz */
 	return freq;
 }
 
@@ -197,11 +196,12 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 	int i, j, test;
 	int curvol;
 
+	dev->curfreq = freq;
 	/*
 	 * Formulate a fifo command
 	 */
 	fifo = 0;
-	if (dev->curtuner == 0) {    /* FM */
+	if (dev->is_fm_band) {    /* FM */
 		test = 102400;
 		freq = freq / 16;       /* Make it kHz */
 		freq += 10700;               /* IF is 10700 kHz */
@@ -213,10 +213,9 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 			}
 			test = test >> 1;
 		}
-	}
-	if (dev->curtuner == 1) {    /* AM */
-		fifo = (freq / 16) + 2010;            /* Make it kHz */
-		fifo |= 0x100000;            /* Select AM Band */
+	} else {	/* AM */
+		fifo = (freq / 16) + 450;	/* Make it kHz */
+		fifo |= 0x100000;		/* Select AM Band */
 	}
 
 	/*
@@ -239,7 +238,7 @@ static void cadet_setfreq(struct cadet *dev, unsigned freq)
 
 		cadet_gettune(dev);
 		if ((dev->tunestat & 0x40) == 0) {   /* Tuned */
-			dev->sigstrength = sigtable[dev->curtuner][j];
+			dev->sigstrength = sigtable[dev->is_fm_band][j];
 			goto reset_rds;
 		}
 	}
@@ -338,39 +337,50 @@ static int vidioc_querycap(struct file *file, void *priv,
 	return 0;
 }
 
+static const struct v4l2_frequency_band bands[] = {
+	{
+		.index = 0,
+		.name = "AM MW",
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_FREQ_BANDS,
+		.rangelow = 8320,      /* 520 kHz */
+		.rangehigh = 26400,    /* 1650 kHz */
+	}, {
+		.index = 1,
+		.name = "FM",
+		.capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
+			V4L2_TUNER_CAP_RDS_BLOCK_IO | V4L2_TUNER_CAP_LOW |
+			V4L2_TUNER_CAP_FREQ_BANDS,
+		.rangelow = 1400000,   /* 87.5 MHz */
+		.rangehigh = 1728000,  /* 108.0 MHz */
+	},
+};
+
 static int vidioc_g_tuner(struct file *file, void *priv,
 				struct v4l2_tuner *v)
 {
 	struct cadet *dev = video_drvdata(file);
 
+	if (v->index)
+		return -EINVAL;
 	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 | V4L2_TUNER_CAP_LOW;
-		v->rangelow = 1400000;     /* 87.5 MHz */
-		v->rangehigh = 1728000;    /* 108.0 MHz */
+	strlcpy(v->name, "Radio", sizeof(v->name));
+	v->capability = bands[0].capability | bands[1].capability;
+	v->rangelow = bands[0].rangelow;	   /* 520 kHz (start of AM band) */
+	v->rangehigh = bands[1].rangehigh;    /* 108.0 MHz (end of FM band) */
+	if (dev->is_fm_band) {
 		v->rxsubchans = cadet_getstereo(dev);
-		v->audmode = V4L2_TUNER_MODE_STEREO;
 		outb(3, dev->io);
 		outb(inb(dev->io + 1) & 0x7f, dev->io + 1);
 		mdelay(100);
 		outb(3, dev->io);
 		if (inb(dev->io + 1) & 0x80)
 			v->rxsubchans |= V4L2_TUNER_SUB_RDS;
-		break;
-	case 1:
-		strlcpy(v->name, "AM", sizeof(v->name));
-		v->capability = V4L2_TUNER_CAP_LOW;
+	} else {
 		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->audmode = V4L2_TUNER_MODE_STEREO;
 	v->signal = dev->sigstrength; /* We might need to modify scaling of this */
 	return 0;
 }
@@ -378,8 +388,17 @@ static int vidioc_g_tuner(struct file *file, void *priv,
 static int vidioc_s_tuner(struct file *file, void *priv,
 				struct v4l2_tuner *v)
 {
-	if (v->index != 0 && v->index != 1)
+	return v->index ? -EINVAL : 0;
+}
+
+static int vidioc_enum_freq_bands(struct file *file, void *priv,
+				struct v4l2_frequency_band *band)
+{
+	if (band->tuner)
+		return -EINVAL;
+	if (band->index >= ARRAY_SIZE(bands))
 		return -EINVAL;
+	*band = bands[band->index];
 	return 0;
 }
 
@@ -388,10 +407,10 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 {
 	struct cadet *dev = video_drvdata(file);
 
-	if (f->tuner > 1)
+	if (f->tuner)
 		return -EINVAL;
 	f->type = V4L2_TUNER_RADIO;
-	f->frequency = cadet_getfreq(dev);
+	f->frequency = dev->curfreq;
 	return 0;
 }
 
@@ -401,20 +420,12 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 {
 	struct cadet *dev = video_drvdata(file);
 
-	if (f->type != V4L2_TUNER_RADIO)
-		return -EINVAL;
-	if (f->tuner == 0) {
-		if (f->frequency < 1400000)
-			f->frequency = 1400000;
-		else if (f->frequency > 1728000)
-			f->frequency = 1728000;
-	} else if (f->tuner == 1) {
-		if (f->frequency < 8320)
-			f->frequency = 8320;
-		else if (f->frequency > 26400)
-			f->frequency = 26400;
-	} else
+	if (f->tuner)
 		return -EINVAL;
+	dev->is_fm_band =
+		f->frequency >= (bands[0].rangehigh + bands[1].rangelow) / 2;
+	clamp(f->frequency, bands[dev->is_fm_band].rangelow,
+			    bands[dev->is_fm_band].rangehigh);
 	cadet_setfreq(dev, f->frequency);
 	return 0;
 }
@@ -499,6 +510,7 @@ static const struct v4l2_ioctl_ops cadet_ioctl_ops = {
 	.vidioc_s_tuner     = vidioc_s_tuner,
 	.vidioc_g_frequency = vidioc_g_frequency,
 	.vidioc_s_frequency = vidioc_s_frequency,
+	.vidioc_enum_freq_bands = vidioc_enum_freq_bands,
 	.vidioc_log_status  = v4l2_ctrl_log_status,
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
@@ -555,8 +567,8 @@ static void cadet_probe(struct cadet *dev)
 	for (i = 0; i < 8; i++) {
 		dev->io = iovals[i];
 		if (request_region(dev->io, 2, "cadet-probe")) {
-			cadet_setfreq(dev, 1410);
-			if (cadet_getfreq(dev) == 1410) {
+			cadet_setfreq(dev, bands[1].rangelow);
+			if (cadet_getfreq(dev) == bands[1].rangelow) {
 				release_region(dev->io, 2);
 				return;
 			}
@@ -619,6 +631,9 @@ static int __init cadet_init(void)
 		goto err_hdl;
 	}
 
+	dev->is_fm_band = true;
+	dev->curfreq = bands[dev->is_fm_band].rangelow;
+	cadet_setfreq(dev, dev->curfreq);
 	strlcpy(dev->vdev.name, v4l2_dev->name, sizeof(dev->vdev.name));
 	dev->vdev.v4l2_dev = v4l2_dev;
 	dev->vdev.fops = &cadet_fops;
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/6] Add frequency band information
  2012-07-02 14:15 [RFC PATCH 0/6] Add frequency band information Hans Verkuil
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
@ 2012-07-02 14:42 ` Hans de Goede
  2012-07-02 15:24   ` halli manjunatha
  1 sibling, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2012-07-02 14:42 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, halli manjunatha, Mauro Carvalho Chehab

Hi,

Series looks good to me, ack series:
Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

On 07/02/2012 04:15 PM, Hans Verkuil wrote:
> Hi all,
>
> This patch series adds support for the new VIDIOC_ENUM_FREQ_BANDS ioctl that
> tells userspace if a tuner supports multiple frequency bands.
>
> This API is based on earlier attempts:
>
> http://www.spinics.net/lists/linux-media/msg48391.html
> http://www.spinics.net/lists/linux-media/msg48435.html
>
> And an irc discussion:
>
> http://linuxtv.org/irc/v4l/index.php?date=2012-06-26
>
> This irc discussion also talked about adding rangelow/high to the v4l2_hw_freq_seek
> struct, but I decided not to do that. The hwseek additions are independent to this
> patch series, and I think it is best if Hans de Goede does that part so that that
> API change can be made together with a driver that actually uses it.
>
> In order to show how the new ioctl is used, this patch series adds support for it
> to the very, very old radio-cadet driver.
>
> Comments are welcome!
>
> Regards,
>
> 	Hans
>
> PS: Mauro, I haven't been able to work on the radio profile yet, so that's not
> included.
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/6] Add frequency band information
  2012-07-02 14:42 ` [RFC PATCH 0/6] Add frequency band information Hans de Goede
@ 2012-07-02 15:24   ` halli manjunatha
  0 siblings, 0 replies; 16+ messages in thread
From: halli manjunatha @ 2012-07-02 15:24 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Hans Verkuil, linux-media, Mauro Carvalho Chehab

Hi,

This looks good to me

Acked-by: Manjunatha Halli <manjunatha_halli@ti.com>

Regards
Manju

On Mon, Jul 2, 2012 at 9:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> Series looks good to me, ack series:
> Acked-by: Hans de Goede <hdegoede@redhat.com>
>
> Regards,
>
> Hans
>
>
> On 07/02/2012 04:15 PM, Hans Verkuil wrote:
>>
>> Hi all,
>>
>> This patch series adds support for the new VIDIOC_ENUM_FREQ_BANDS ioctl that
>> tells userspace if a tuner supports multiple frequency bands.
>>
>> This API is based on earlier attempts:
>>
>> http://www.spinics.net/lists/linux-media/msg48391.html
>> http://www.spinics.net/lists/linux-media/msg48435.html
>>
>> And an irc discussion:
>>
>> http://linuxtv.org/irc/v4l/index.php?date=2012-06-26
>>
>> This irc discussion also talked about adding rangelow/high to the v4l2_hw_freq_seek
>> struct, but I decided not to do that. The hwseek additions are independent to this
>> patch series, and I think it is best if Hans de Goede does that part so that that
>> API change can be made together with a driver that actually uses it.
>>
>> In order to show how the new ioctl is used, this patch series adds support for it
>> to the very, very old radio-cadet driver.
>>
>> Comments are welcome!
>>
>> Regards,
>>
>>         Hans
>>
>> PS: Mauro, I haven't been able to work on the radio profile yet, so that's not
>> included.
>>
>



--
Regards
Halli

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
                     ` (4 preceding siblings ...)
  2012-07-02 14:15   ` [RFC PATCH 6/6] radio-cadet: implement frequency band enumeration Hans Verkuil
@ 2012-07-02 17:42   ` Mauro Carvalho Chehab
  2012-07-03  7:19     ` Hans Verkuil
  5 siblings, 1 reply; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2012-07-02 17:42 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

Em 02-07-2012 11:15, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add a new ioctl to enumerate the supported frequency bands of a tuner.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>   include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
>   1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index f79d0cc..d54ec6e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
>   
>   /*  Flags for the 'rxsubchans' field */
>   #define V4L2_TUNER_SUB_MONO		0x0001
> @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
>   #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
>   
>   struct v4l2_frequency {
> -	__u32		      tuner;
> -	__u32		      type;	/* enum v4l2_tuner_type */
> -	__u32		      frequency;
> -	__u32		      reserved[8];
> +	__u32	tuner;
> +	__u32	type;	/* enum v4l2_tuner_type */
> +	__u32	frequency;
> +	__u32	reserved[8];
> +};
> +
> +struct v4l2_frequency_band {
> +	__u32	tuner;
> +	__u32	type;	/* enum v4l2_tuner_type */
> +	__u32	index;
> +	__u32	capability;
> +	__u32	rangelow;
> +	__u32	rangehigh;
> +	__u8	name[32];

As we've discussed, band name can be inferred from the frequency.
Also, there are more than one name for the same band (it could be
named based on the wavelength or frequency - also, some bands or
band segments may have special names, like Tropical Wave).
Let's userspace just call it whatever it wants. So, I'll just
drop it. 

On the other hand, the modulation is independent on the band, and
ITU-R and regulator agencies may allow more than one modulation type
and usage for the same frequency (like primary and secondary usage).

So, it makes sense to have an enum here to describe the modulation type
(currenly, AM, FM and VSB).

> +	__u32	reserved[6];
>   };
>   
>   struct v4l2_hw_freq_seek {
> -	__u32		      tuner;
> -	__u32		      type;	/* enum v4l2_tuner_type */
> -	__u32		      seek_upward;
> -	__u32		      wrap_around;
> -	__u32		      spacing;
> -	__u32		      reserved[7];
> +	__u32	tuner;
> +	__u32	type;	/* enum v4l2_tuner_type */
> +	__u32	seek_upward;
> +	__u32	wrap_around;
> +	__u32	spacing;
> +	__u32	reserved[7];
>   };
>   
>   /*
> @@ -2646,6 +2658,10 @@ struct v4l2_create_buffers {
>   #define VIDIOC_QUERY_DV_TIMINGS  _IOR('V', 99, struct v4l2_dv_timings)
>   #define VIDIOC_DV_TIMINGS_CAP   _IOWR('V', 100, struct v4l2_dv_timings_cap)
>   
> +/* Experimental, this ioctl may change over the next couple of kernel
> +   versions. */
> +#define VIDIOC_ENUM_FREQ_BANDS	_IOWR('V', 101, struct v4l2_frequency_band)
> +
>   /* Reminder: when adding new ioctls please add support for them to
>      drivers/media/video/v4l2-compat-ioctl32.c as well! */
>   
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-02 17:42   ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Mauro Carvalho Chehab
@ 2012-07-03  7:19     ` Hans Verkuil
  2012-07-03 16:01       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2012-07-03  7:19 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

On Mon 2 July 2012 19:42:33 Mauro Carvalho Chehab wrote:
> Em 02-07-2012 11:15, Hans Verkuil escreveu:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Add a new ioctl to enumerate the supported frequency bands of a tuner.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >   include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
> >   1 file changed, 26 insertions(+), 10 deletions(-)
> > 
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index f79d0cc..d54ec6e 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
> >   
> >   /*  Flags for the 'rxsubchans' field */
> >   #define V4L2_TUNER_SUB_MONO		0x0001
> > @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
> >   #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
> >   
> >   struct v4l2_frequency {
> > -	__u32		      tuner;
> > -	__u32		      type;	/* enum v4l2_tuner_type */
> > -	__u32		      frequency;
> > -	__u32		      reserved[8];
> > +	__u32	tuner;
> > +	__u32	type;	/* enum v4l2_tuner_type */
> > +	__u32	frequency;
> > +	__u32	reserved[8];
> > +};
> > +
> > +struct v4l2_frequency_band {
> > +	__u32	tuner;
> > +	__u32	type;	/* enum v4l2_tuner_type */
> > +	__u32	index;
> > +	__u32	capability;
> > +	__u32	rangelow;
> > +	__u32	rangehigh;
> > +	__u8	name[32];
> 
> As we've discussed, band name can be inferred from the frequency.
> Also, there are more than one name for the same band (it could be
> named based on the wavelength or frequency - also, some bands or
> band segments may have special names, like Tropical Wave).
> Let's userspace just call it whatever it wants. So, I'll just
> drop it.

That will lead to chaos IMHO: one application will call it one thing,
the other something else. Since the frequency band boundaries will
generally be slightly different between different products it is even
not so easy to map a frequency to a particular name. Not to mention
the simple fact that most apps will only ever see FM since the number of
products that support other bands is very, very small.

Sure, an application can just print the frequency range and use that
as the name, but how many end-users would know how to interpret that as
FM or AM MW, etc.? Very few indeed.

> 
> On the other hand, the modulation is independent on the band, and
> ITU-R and regulator agencies may allow more than one modulation type
> and usage for the same frequency (like primary and secondary usage).

But the actual tuner/demod in question will support only one modulation
type per frequency range. It's not something you can change in our API. So
what's the use of such a modulation type? What would an application do with
it? I want to avoid adding a field for which there is no practical use.

This API is used to show a combobox or similar to the end-user allowing him/her
to select a frequency band that the radio application will use. So you need
human-readable names for the frequency bands that are understandable for
your average human being. Frequency ranges or talk about ITU standards are
NOT suitable for that.

Prior to me becoming involved in this discussion the only names I would have
understood are FM and AM SW/MW/LW and I would have no idea what the frequency
ranges for the AM bands were.

Regards,

	Hans

> So, it makes sense to have an enum here to describe the modulation type
> (currenly, AM, FM and VSB).
> 
> > +	__u32	reserved[6];
> >   };
> >   
> >   struct v4l2_hw_freq_seek {
> > -	__u32		      tuner;
> > -	__u32		      type;	/* enum v4l2_tuner_type */
> > -	__u32		      seek_upward;
> > -	__u32		      wrap_around;
> > -	__u32		      spacing;
> > -	__u32		      reserved[7];
> > +	__u32	tuner;
> > +	__u32	type;	/* enum v4l2_tuner_type */
> > +	__u32	seek_upward;
> > +	__u32	wrap_around;
> > +	__u32	spacing;
> > +	__u32	reserved[7];
> >   };
> >   
> >   /*
> > @@ -2646,6 +2658,10 @@ struct v4l2_create_buffers {
> >   #define VIDIOC_QUERY_DV_TIMINGS  _IOR('V', 99, struct v4l2_dv_timings)
> >   #define VIDIOC_DV_TIMINGS_CAP   _IOWR('V', 100, struct v4l2_dv_timings_cap)
> >   
> > +/* Experimental, this ioctl may change over the next couple of kernel
> > +   versions. */
> > +#define VIDIOC_ENUM_FREQ_BANDS	_IOWR('V', 101, struct v4l2_frequency_band)
> > +
> >   /* Reminder: when adding new ioctls please add support for them to
> >      drivers/media/video/v4l2-compat-ioctl32.c as well! */
> >   
> > 
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-03  7:19     ` Hans Verkuil
@ 2012-07-03 16:01       ` Mauro Carvalho Chehab
  2012-07-03 16:47         ` Hans Verkuil
  0 siblings, 1 reply; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2012-07-03 16:01 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

Em 03-07-2012 04:19, Hans Verkuil escreveu:
> On Mon 2 July 2012 19:42:33 Mauro Carvalho Chehab wrote:
>> Em 02-07-2012 11:15, Hans Verkuil escreveu:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Add a new ioctl to enumerate the supported frequency bands of a tuner.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> ---
>>>    include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
>>>    1 file changed, 26 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index f79d0cc..d54ec6e 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
>>>    
>>>    /*  Flags for the 'rxsubchans' field */
>>>    #define V4L2_TUNER_SUB_MONO		0x0001
>>> @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
>>>    #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
>>>    
>>>    struct v4l2_frequency {
>>> -	__u32		      tuner;
>>> -	__u32		      type;	/* enum v4l2_tuner_type */
>>> -	__u32		      frequency;
>>> -	__u32		      reserved[8];
>>> +	__u32	tuner;
>>> +	__u32	type;	/* enum v4l2_tuner_type */
>>> +	__u32	frequency;
>>> +	__u32	reserved[8];
>>> +};
>>> +
>>> +struct v4l2_frequency_band {
>>> +	__u32	tuner;
>>> +	__u32	type;	/* enum v4l2_tuner_type */
>>> +	__u32	index;
>>> +	__u32	capability;
>>> +	__u32	rangelow;
>>> +	__u32	rangehigh;
>>> +	__u8	name[32];
>>
>> As we've discussed, band name can be inferred from the frequency.
>> Also, there are more than one name for the same band (it could be
>> named based on the wavelength or frequency - also, some bands or
>> band segments may have special names, like Tropical Wave).
>> Let's userspace just call it whatever it wants. So, I'll just
>> drop it.
> 
> That will lead to chaos IMHO: one application will call it one thing,
> the other something else. Since the frequency band boundaries will
> generally be slightly different between different products it is even
> not so easy to map a frequency to a particular name. Not to mention
> the simple fact that most apps will only ever see FM since the number of
> products that support other bands is very, very small.
> 
> Sure, an application can just print the frequency range and use that
> as the name, but how many end-users would know how to interpret that as
> FM or AM MW, etc.? Very few indeed.

AM or FM can be retrieved from a modulation field. The band range is:
	1) Country-dependent, e. g. they're defined by the regulator's
	   agency on each Country and standardized on ITU-R;

	2) Per-country regulatory restrictions may apply, as it may be
illegal or it may be required an special license to operate outside the 
public services range. Some of the supported devices for can be used
at the amateur radio range

	3) requires locale support. For example, in Brazil:
		short wave is OC
		medium wave is OM
		part of the OC band is called Tropical wave
		...

Devices with dual TV/FM tuners allows a band that it is larger than SW+MW+LW.
How would you call such band?

What I'm saying is that an application that would properly implement radio
support will need to have a per-Country regulatory data, in order to name
a band, using the Country's denomination for that band.

It is not a Kernel's task to keep such database. It may be added on a library,
through.

>> On the other hand, the modulation is independent on the band, and
>> ITU-R and regulator agencies may allow more than one modulation type
>> and usage for the same frequency (like primary and secondary usage).
> 
> But the actual tuner/demod in question will support only one modulation
> type per frequency range. It's not something you can change in our API. So
> what's the use of such a modulation type? What would an application do with
> it? I want to avoid adding a field for which there is no practical use.

Devices like bttv and cx88 with a TV/FM tuner allow at least 2 modulation types:
FM and SDR (Software Delivered Radio), as the internal RISC processor can deliver
the IF samples though the DMA engine, allowing demodulation in userspace.

> This API is used to show a combobox or similar to the end-user allowing him/her
> to select a frequency band that the radio application will use. So you need
> human-readable names for the frequency bands that are understandable for
> your average human being. Frequency ranges or talk about ITU standards are
> NOT suitable for that.

It is not a Kernel's task to present a combobox. Also, converting the radio
band names into a combobox will require converting the band names into locale
data, with is more complex, less portable than to compare the band ranges with 
the ITU-R tables.

That's said, let's suppose an application that would allow to select between:
	- FM Europe/America;
	- FM Japan;
	- FM Russia

And let's suppose 2 different drivers:
	- driver 1: bttv + TV/FM tuner - band from 56 MHz to 165 MHz;
	- driver 2: tea5767 - japan band from 76 to 108 MHz;
		    european band from 87.5 to 108 MHz;

For driver 1, the band will be bigger than all 3 FM ranges. Userspace will need
to use S_TUNER to adjust the single band to the selected one, or to prevent
using a frequency outside band;

For driver 2, for euro band, it can just select the second band. However, for
band 1, it will need to use S_TUNER to restrict the maximum frequency to 90 MHz,
in order to match the regulatory band.

So, whatever "name" would be used, the userspace will need to know what are the
regulatory standards and use some logic to make sure that the proper band will
be used.

Of course, as such logic is common for all radio applications, it makes sense to
add it into a radio v4l-utils library.

> Prior to me becoming involved in this discussion the only names I would have
> understood are FM and AM SW/MW/LW and I would have no idea what the frequency
> ranges for the AM bands were.

Well, the sort wave range is actually 15 bands, each with their own regulations.
For example, In Brazil, SW is used by broadcast and amateur radio. For amateur
radio, those are the used ranges:
	160m, 80m, 40m, 30m, 20m, 17m, 15m, 12m, 10m

In summary, band names can't be properly represented in Kernel, as this is not
a trivial issue, nor Kernel should bother about localized names or per-Country
data.

Regards,
Mauro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-03 16:01       ` Mauro Carvalho Chehab
@ 2012-07-03 16:47         ` Hans Verkuil
  2012-07-03 20:35           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2012-07-03 16:47 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

On Tue July 3 2012 18:01:51 Mauro Carvalho Chehab wrote:
> Em 03-07-2012 04:19, Hans Verkuil escreveu:
> > On Mon 2 July 2012 19:42:33 Mauro Carvalho Chehab wrote:
> >> Em 02-07-2012 11:15, Hans Verkuil escreveu:
> >>> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>>
> >>> Add a new ioctl to enumerate the supported frequency bands of a tuner.
> >>>
> >>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>> ---
> >>>    include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
> >>>    1 file changed, 26 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >>> index f79d0cc..d54ec6e 100644
> >>> --- a/include/linux/videodev2.h
> >>> +++ b/include/linux/videodev2.h
> >>> @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
> >>>    
> >>>    /*  Flags for the 'rxsubchans' field */
> >>>    #define V4L2_TUNER_SUB_MONO		0x0001
> >>> @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
> >>>    #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
> >>>    
> >>>    struct v4l2_frequency {
> >>> -	__u32		      tuner;
> >>> -	__u32		      type;	/* enum v4l2_tuner_type */
> >>> -	__u32		      frequency;
> >>> -	__u32		      reserved[8];
> >>> +	__u32	tuner;
> >>> +	__u32	type;	/* enum v4l2_tuner_type */
> >>> +	__u32	frequency;
> >>> +	__u32	reserved[8];
> >>> +};
> >>> +
> >>> +struct v4l2_frequency_band {
> >>> +	__u32	tuner;
> >>> +	__u32	type;	/* enum v4l2_tuner_type */
> >>> +	__u32	index;
> >>> +	__u32	capability;
> >>> +	__u32	rangelow;
> >>> +	__u32	rangehigh;
> >>> +	__u8	name[32];
> >>
> >> As we've discussed, band name can be inferred from the frequency.
> >> Also, there are more than one name for the same band (it could be
> >> named based on the wavelength or frequency - also, some bands or
> >> band segments may have special names, like Tropical Wave).
> >> Let's userspace just call it whatever it wants. So, I'll just
> >> drop it.
> > 
> > That will lead to chaos IMHO: one application will call it one thing,
> > the other something else. Since the frequency band boundaries will
> > generally be slightly different between different products it is even
> > not so easy to map a frequency to a particular name. Not to mention
> > the simple fact that most apps will only ever see FM since the number of
> > products that support other bands is very, very small.
> > 
> > Sure, an application can just print the frequency range and use that
> > as the name, but how many end-users would know how to interpret that as
> > FM or AM MW, etc.? Very few indeed.
> 
> AM or FM can be retrieved from a modulation field. The band range is:
> 	1) Country-dependent, e. g. they're defined by the regulator's
> 	   agency on each Country and standardized on ITU-R;
> 
> 	2) Per-country regulatory restrictions may apply, as it may be
> illegal or it may be required an special license to operate outside the 
> public services range. Some of the supported devices for can be used
> at the amateur radio range
> 
> 	3) requires locale support. For example, in Brazil:
> 		short wave is OC
> 		medium wave is OM
> 		part of the OC band is called Tropical wave
> 		...
> 
> Devices with dual TV/FM tuners allows a band that it is larger than SW+MW+LW.
> How would you call such band?
> 
> What I'm saying is that an application that would properly implement radio
> support will need to have a per-Country regulatory data, in order to name
> a band, using the Country's denomination for that band.
> 
> It is not a Kernel's task to keep such database. It may be added on a library,
> through.
> 
> >> On the other hand, the modulation is independent on the band, and
> >> ITU-R and regulator agencies may allow more than one modulation type
> >> and usage for the same frequency (like primary and secondary usage).
> > 
> > But the actual tuner/demod in question will support only one modulation
> > type per frequency range. It's not something you can change in our API. So
> > what's the use of such a modulation type? What would an application do with
> > it? I want to avoid adding a field for which there is no practical use.
> 
> Devices like bttv and cx88 with a TV/FM tuner allow at least 2 modulation types:
> FM and SDR (Software Delivered Radio), as the internal RISC processor can deliver
> the IF samples though the DMA engine, allowing demodulation in userspace.
> 
> > This API is used to show a combobox or similar to the end-user allowing him/her
> > to select a frequency band that the radio application will use. So you need
> > human-readable names for the frequency bands that are understandable for
> > your average human being. Frequency ranges or talk about ITU standards are
> > NOT suitable for that.
> 
> It is not a Kernel's task to present a combobox. Also, converting the radio
> band names into a combobox will require converting the band names into locale
> data, with is more complex, less portable than to compare the band ranges with 
> the ITU-R tables.
> 
> That's said, let's suppose an application that would allow to select between:
> 	- FM Europe/America;
> 	- FM Japan;
> 	- FM Russia
> 
> And let's suppose 2 different drivers:
> 	- driver 1: bttv + TV/FM tuner - band from 56 MHz to 165 MHz;
> 	- driver 2: tea5767 - japan band from 76 to 108 MHz;
> 		    european band from 87.5 to 108 MHz;
> 
> For driver 1, the band will be bigger than all 3 FM ranges. Userspace will need
> to use S_TUNER to adjust the single band to the selected one, or to prevent
> using a frequency outside band;
> 
> For driver 2, for euro band, it can just select the second band. However, for
> band 1, it will need to use S_TUNER to restrict the maximum frequency to 90 MHz,
> in order to match the regulatory band.
> 
> So, whatever "name" would be used, the userspace will need to know what are the
> regulatory standards and use some logic to make sure that the proper band will
> be used.
> 
> Of course, as such logic is common for all radio applications, it makes sense to
> add it into a radio v4l-utils library.
> 
> > Prior to me becoming involved in this discussion the only names I would have
> > understood are FM and AM SW/MW/LW and I would have no idea what the frequency
> > ranges for the AM bands were.
> 
> Well, the sort wave range is actually 15 bands, each with their own regulations.
> For example, In Brazil, SW is used by broadcast and amateur radio. For amateur
> radio, those are the used ranges:
> 	160m, 80m, 40m, 30m, 20m, 17m, 15m, 12m, 10m
> 
> In summary, band names can't be properly represented in Kernel, as this is not
> a trivial issue, nor Kernel should bother about localized names or per-Country
> data.

You clearly have way too much knowledge on this topic :-)

OK, name is out, a modulation field is in.

This promptly leads to the next problem: there is no modulation field in v4l2_tuner,
so what to do for existing drivers.

It makes sense to have the tuner capability field be a union of the caps of all bands,
but you can't do the same for a modulation field (unless you make it a bitfield, but
that's just weird IMHO).

So perhaps we should add compat code like what I suggested earlier in a private email
where, if no enum_freq_bands op is defined but g_tuner is, then the core will provide
a enum_freq_bands version that uses the caps/rangelow/high from g_tuner and that can
fill in the modulation type based on the video node (i.e. if called from radio, then
the modulation is FM, else VSB).

This is true for all currently available drivers, except for radio-cadet. But the
latter will be updated with proper enum_freq_bands support, so it won't use the
compat code.

As a result of this, an application can always find the modulation of a particular
frequency band by calling VIDIOC_ENUM_FREQ_BANDS.

Note that for the sake of brevity I'm ignoring the modulator in this discussion, but
the same principle applies there.

BTW, if I understand it correctly SDR isn't a modulation type as such, it's the raw
output from the tuner and it is up to the software to decide whether to demodulate
as FM or AM (in the case of analog radio), right? Although in practice I expect it
to be obvious what should be used.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-03 16:47         ` Hans Verkuil
@ 2012-07-03 20:35           ` Mauro Carvalho Chehab
  2012-07-04  8:35             ` Hans Verkuil
  0 siblings, 1 reply; 16+ messages in thread
From: Mauro Carvalho Chehab @ 2012-07-03 20:35 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

Em 03-07-2012 13:47, Hans Verkuil escreveu:
> On Tue July 3 2012 18:01:51 Mauro Carvalho Chehab wrote:
>> Em 03-07-2012 04:19, Hans Verkuil escreveu:
>>> On Mon 2 July 2012 19:42:33 Mauro Carvalho Chehab wrote:
>>>> Em 02-07-2012 11:15, Hans Verkuil escreveu:
>>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>>
>>>>> Add a new ioctl to enumerate the supported frequency bands of a tuner.
>>>>>
>>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>>> ---
>>>>>     include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
>>>>>     1 file changed, 26 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>>>> index f79d0cc..d54ec6e 100644
>>>>> --- a/include/linux/videodev2.h
>>>>> +++ b/include/linux/videodev2.h
>>>>> @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
>>>>>     
>>>>>     /*  Flags for the 'rxsubchans' field */
>>>>>     #define V4L2_TUNER_SUB_MONO		0x0001
>>>>> @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
>>>>>     #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
>>>>>     
>>>>>     struct v4l2_frequency {
>>>>> -	__u32		      tuner;
>>>>> -	__u32		      type;	/* enum v4l2_tuner_type */
>>>>> -	__u32		      frequency;
>>>>> -	__u32		      reserved[8];
>>>>> +	__u32	tuner;
>>>>> +	__u32	type;	/* enum v4l2_tuner_type */
>>>>> +	__u32	frequency;
>>>>> +	__u32	reserved[8];
>>>>> +};
>>>>> +
>>>>> +struct v4l2_frequency_band {
>>>>> +	__u32	tuner;
>>>>> +	__u32	type;	/* enum v4l2_tuner_type */
>>>>> +	__u32	index;
>>>>> +	__u32	capability;
>>>>> +	__u32	rangelow;
>>>>> +	__u32	rangehigh;
>>>>> +	__u8	name[32];
>>>>
>>>> As we've discussed, band name can be inferred from the frequency.
>>>> Also, there are more than one name for the same band (it could be
>>>> named based on the wavelength or frequency - also, some bands or
>>>> band segments may have special names, like Tropical Wave).
>>>> Let's userspace just call it whatever it wants. So, I'll just
>>>> drop it.
>>>
>>> That will lead to chaos IMHO: one application will call it one thing,
>>> the other something else. Since the frequency band boundaries will
>>> generally be slightly different between different products it is even
>>> not so easy to map a frequency to a particular name. Not to mention
>>> the simple fact that most apps will only ever see FM since the number of
>>> products that support other bands is very, very small.
>>>
>>> Sure, an application can just print the frequency range and use that
>>> as the name, but how many end-users would know how to interpret that as
>>> FM or AM MW, etc.? Very few indeed.
>>
>> AM or FM can be retrieved from a modulation field. The band range is:
>> 	1) Country-dependent, e. g. they're defined by the regulator's
>> 	   agency on each Country and standardized on ITU-R;
>>
>> 	2) Per-country regulatory restrictions may apply, as it may be
>> illegal or it may be required an special license to operate outside the
>> public services range. Some of the supported devices for can be used
>> at the amateur radio range
>>
>> 	3) requires locale support. For example, in Brazil:
>> 		short wave is OC
>> 		medium wave is OM
>> 		part of the OC band is called Tropical wave
>> 		...
>>
>> Devices with dual TV/FM tuners allows a band that it is larger than SW+MW+LW.
>> How would you call such band?
>>
>> What I'm saying is that an application that would properly implement radio
>> support will need to have a per-Country regulatory data, in order to name
>> a band, using the Country's denomination for that band.
>>
>> It is not a Kernel's task to keep such database. It may be added on a library,
>> through.
>>
>>>> On the other hand, the modulation is independent on the band, and
>>>> ITU-R and regulator agencies may allow more than one modulation type
>>>> and usage for the same frequency (like primary and secondary usage).
>>>
>>> But the actual tuner/demod in question will support only one modulation
>>> type per frequency range. It's not something you can change in our API. So
>>> what's the use of such a modulation type? What would an application do with
>>> it? I want to avoid adding a field for which there is no practical use.
>>
>> Devices like bttv and cx88 with a TV/FM tuner allow at least 2 modulation types:
>> FM and SDR (Software Delivered Radio), as the internal RISC processor can deliver
>> the IF samples though the DMA engine, allowing demodulation in userspace.
>>
>>> This API is used to show a combobox or similar to the end-user allowing him/her
>>> to select a frequency band that the radio application will use. So you need
>>> human-readable names for the frequency bands that are understandable for
>>> your average human being. Frequency ranges or talk about ITU standards are
>>> NOT suitable for that.
>>
>> It is not a Kernel's task to present a combobox. Also, converting the radio
>> band names into a combobox will require converting the band names into locale
>> data, with is more complex, less portable than to compare the band ranges with
>> the ITU-R tables.
>>
>> That's said, let's suppose an application that would allow to select between:
>> 	- FM Europe/America;
>> 	- FM Japan;
>> 	- FM Russia
>>
>> And let's suppose 2 different drivers:
>> 	- driver 1: bttv + TV/FM tuner - band from 56 MHz to 165 MHz;
>> 	- driver 2: tea5767 - japan band from 76 to 108 MHz;
>> 		    european band from 87.5 to 108 MHz;
>>
>> For driver 1, the band will be bigger than all 3 FM ranges. Userspace will need
>> to use S_TUNER to adjust the single band to the selected one, or to prevent
>> using a frequency outside band;
>>
>> For driver 2, for euro band, it can just select the second band. However, for
>> band 1, it will need to use S_TUNER to restrict the maximum frequency to 90 MHz,
>> in order to match the regulatory band.
>>
>> So, whatever "name" would be used, the userspace will need to know what are the
>> regulatory standards and use some logic to make sure that the proper band will
>> be used.
>>
>> Of course, as such logic is common for all radio applications, it makes sense to
>> add it into a radio v4l-utils library.
>>
>>> Prior to me becoming involved in this discussion the only names I would have
>>> understood are FM and AM SW/MW/LW and I would have no idea what the frequency
>>> ranges for the AM bands were.
>>
>> Well, the sort wave range is actually 15 bands, each with their own regulations.
>> For example, In Brazil, SW is used by broadcast and amateur radio. For amateur
>> radio, those are the used ranges:
>> 	160m, 80m, 40m, 30m, 20m, 17m, 15m, 12m, 10m
>>
>> In summary, band names can't be properly represented in Kernel, as this is not
>> a trivial issue, nor Kernel should bother about localized names or per-Country
>> data.
> 
> You clearly have way too much knowledge on this topic :-)

:)

> OK, name is out, a modulation field is in.
> 
> This promptly leads to the next problem: there is no modulation field in v4l2_tuner,
> so what to do for existing drivers.
> 
> It makes sense to have the tuner capability field be a union of the caps of all bands,
> but you can't do the same for a modulation field (unless you make it a bitfield, but
> that's just weird IMHO).

Well, DVB exposes modulation as caps. Not sure if this is the best thing to do there,
but it could make sense to have something like:
	HAVE_AM
	HAVE_FM
	HAVE_SDR
	HAVE_VSB
...

for modulation caps.

> 
> So perhaps we should add compat code like what I suggested earlier in a private email
> where, if no enum_freq_bands op is defined but g_tuner is, then the core will provide
> a enum_freq_bands version that uses the caps/rangelow/high from g_tuner and that can
> fill in the modulation type based on the video node (i.e. if called from radio, then
> the modulation is FM, else VSB).

That would be an alternative. I don't have a strong opinion here, but I think that
modulation caps could work better, IMHO.
> 
> This is true for all currently available drivers, except for radio-cadet. But the
> latter will be updated with proper enum_freq_bands support, so it won't use the
> compat code.
> 
> As a result of this, an application can always find the modulation of a particular
> frequency band by calling VIDIOC_ENUM_FREQ_BANDS.
> 
> Note that for the sake of brevity I'm ignoring the modulator in this discussion, but
> the same principle applies there.
> 
> BTW, if I understand it correctly SDR isn't a modulation type as such, it's the raw
> output from the tuner and it is up to the software to decide whether to demodulate
> as FM or AM (in the case of analog radio), right?

Yes.

> Although in practice I expect it to be obvious what should be used.

Some bands could allow more than one modulation type. FYI, regulatory agencies define
two types of usage for a frequency band: a "primary usage" and a "secondary usage".

Being simplistic, primary usage is generally for public or government services. Secondary
usage is generally for personal/private usage. The usages that don't require special
licenses to transmit (like wifi) are secondary usage. Remote-controlled toys and walk talks
at 27MHz range is an example of such secondary usage. AFAIKT, several of those devices use FM
modulation, but AM may also be used on those devices.

An SDR implementation may decode either AM or FM (or it could also decode other types,
like SSB - used on several amateur radio bands).

Regards,
Mauro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-03 20:35           ` Mauro Carvalho Chehab
@ 2012-07-04  8:35             ` Hans Verkuil
  2012-07-04  9:18               ` Hans de Goede
  0 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2012-07-04  8:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Hans de Goede, halli manjunatha, Hans Verkuil

On Tue 3 July 2012 22:35:54 Mauro Carvalho Chehab wrote:
> Em 03-07-2012 13:47, Hans Verkuil escreveu:
> > On Tue July 3 2012 18:01:51 Mauro Carvalho Chehab wrote:
> >> Em 03-07-2012 04:19, Hans Verkuil escreveu:
> >>> On Mon 2 July 2012 19:42:33 Mauro Carvalho Chehab wrote:
> >>>> Em 02-07-2012 11:15, Hans Verkuil escreveu:
> >>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
> >>>>>
> >>>>> Add a new ioctl to enumerate the supported frequency bands of a tuner.
> >>>>>
> >>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>>>> ---
> >>>>>     include/linux/videodev2.h |   36 ++++++++++++++++++++++++++----------
> >>>>>     1 file changed, 26 insertions(+), 10 deletions(-)
> >>>>>
> >>>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >>>>> index f79d0cc..d54ec6e 100644
> >>>>> --- a/include/linux/videodev2.h
> >>>>> +++ b/include/linux/videodev2.h
> >>>>> @@ -2048,6 +2048,7 @@ 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_FREQ_BANDS	0x0400
> >>>>>     
> >>>>>     /*  Flags for the 'rxsubchans' field */
> >>>>>     #define V4L2_TUNER_SUB_MONO		0x0001
> >>>>> @@ -2066,19 +2067,30 @@ struct v4l2_modulator {
> >>>>>     #define V4L2_TUNER_MODE_LANG1_LANG2	0x0004
> >>>>>     
> >>>>>     struct v4l2_frequency {
> >>>>> -	__u32		      tuner;
> >>>>> -	__u32		      type;	/* enum v4l2_tuner_type */
> >>>>> -	__u32		      frequency;
> >>>>> -	__u32		      reserved[8];
> >>>>> +	__u32	tuner;
> >>>>> +	__u32	type;	/* enum v4l2_tuner_type */
> >>>>> +	__u32	frequency;
> >>>>> +	__u32	reserved[8];
> >>>>> +};
> >>>>> +
> >>>>> +struct v4l2_frequency_band {
> >>>>> +	__u32	tuner;
> >>>>> +	__u32	type;	/* enum v4l2_tuner_type */
> >>>>> +	__u32	index;
> >>>>> +	__u32	capability;
> >>>>> +	__u32	rangelow;
> >>>>> +	__u32	rangehigh;
> >>>>> +	__u8	name[32];
> >>>>
> >>>> As we've discussed, band name can be inferred from the frequency.
> >>>> Also, there are more than one name for the same band (it could be
> >>>> named based on the wavelength or frequency - also, some bands or
> >>>> band segments may have special names, like Tropical Wave).
> >>>> Let's userspace just call it whatever it wants. So, I'll just
> >>>> drop it.
> >>>
> >>> That will lead to chaos IMHO: one application will call it one thing,
> >>> the other something else. Since the frequency band boundaries will
> >>> generally be slightly different between different products it is even
> >>> not so easy to map a frequency to a particular name. Not to mention
> >>> the simple fact that most apps will only ever see FM since the number of
> >>> products that support other bands is very, very small.
> >>>
> >>> Sure, an application can just print the frequency range and use that
> >>> as the name, but how many end-users would know how to interpret that as
> >>> FM or AM MW, etc.? Very few indeed.
> >>
> >> AM or FM can be retrieved from a modulation field. The band range is:
> >> 	1) Country-dependent, e. g. they're defined by the regulator's
> >> 	   agency on each Country and standardized on ITU-R;
> >>
> >> 	2) Per-country regulatory restrictions may apply, as it may be
> >> illegal or it may be required an special license to operate outside the
> >> public services range. Some of the supported devices for can be used
> >> at the amateur radio range
> >>
> >> 	3) requires locale support. For example, in Brazil:
> >> 		short wave is OC
> >> 		medium wave is OM
> >> 		part of the OC band is called Tropical wave
> >> 		...
> >>
> >> Devices with dual TV/FM tuners allows a band that it is larger than SW+MW+LW.
> >> How would you call such band?
> >>
> >> What I'm saying is that an application that would properly implement radio
> >> support will need to have a per-Country regulatory data, in order to name
> >> a band, using the Country's denomination for that band.
> >>
> >> It is not a Kernel's task to keep such database. It may be added on a library,
> >> through.
> >>
> >>>> On the other hand, the modulation is independent on the band, and
> >>>> ITU-R and regulator agencies may allow more than one modulation type
> >>>> and usage for the same frequency (like primary and secondary usage).
> >>>
> >>> But the actual tuner/demod in question will support only one modulation
> >>> type per frequency range. It's not something you can change in our API. So
> >>> what's the use of such a modulation type? What would an application do with
> >>> it? I want to avoid adding a field for which there is no practical use.
> >>
> >> Devices like bttv and cx88 with a TV/FM tuner allow at least 2 modulation types:
> >> FM and SDR (Software Delivered Radio), as the internal RISC processor can deliver
> >> the IF samples though the DMA engine, allowing demodulation in userspace.
> >>
> >>> This API is used to show a combobox or similar to the end-user allowing him/her
> >>> to select a frequency band that the radio application will use. So you need
> >>> human-readable names for the frequency bands that are understandable for
> >>> your average human being. Frequency ranges or talk about ITU standards are
> >>> NOT suitable for that.
> >>
> >> It is not a Kernel's task to present a combobox. Also, converting the radio
> >> band names into a combobox will require converting the band names into locale
> >> data, with is more complex, less portable than to compare the band ranges with
> >> the ITU-R tables.
> >>
> >> That's said, let's suppose an application that would allow to select between:
> >> 	- FM Europe/America;
> >> 	- FM Japan;
> >> 	- FM Russia
> >>
> >> And let's suppose 2 different drivers:
> >> 	- driver 1: bttv + TV/FM tuner - band from 56 MHz to 165 MHz;
> >> 	- driver 2: tea5767 - japan band from 76 to 108 MHz;
> >> 		    european band from 87.5 to 108 MHz;
> >>
> >> For driver 1, the band will be bigger than all 3 FM ranges. Userspace will need
> >> to use S_TUNER to adjust the single band to the selected one, or to prevent
> >> using a frequency outside band;
> >>
> >> For driver 2, for euro band, it can just select the second band. However, for
> >> band 1, it will need to use S_TUNER to restrict the maximum frequency to 90 MHz,
> >> in order to match the regulatory band.
> >>
> >> So, whatever "name" would be used, the userspace will need to know what are the
> >> regulatory standards and use some logic to make sure that the proper band will
> >> be used.
> >>
> >> Of course, as such logic is common for all radio applications, it makes sense to
> >> add it into a radio v4l-utils library.
> >>
> >>> Prior to me becoming involved in this discussion the only names I would have
> >>> understood are FM and AM SW/MW/LW and I would have no idea what the frequency
> >>> ranges for the AM bands were.
> >>
> >> Well, the sort wave range is actually 15 bands, each with their own regulations.
> >> For example, In Brazil, SW is used by broadcast and amateur radio. For amateur
> >> radio, those are the used ranges:
> >> 	160m, 80m, 40m, 30m, 20m, 17m, 15m, 12m, 10m
> >>
> >> In summary, band names can't be properly represented in Kernel, as this is not
> >> a trivial issue, nor Kernel should bother about localized names or per-Country
> >> data.
> > 
> > You clearly have way too much knowledge on this topic :-)
> 
> :)
> 
> > OK, name is out, a modulation field is in.
> > 
> > This promptly leads to the next problem: there is no modulation field in v4l2_tuner,
> > so what to do for existing drivers.
> > 
> > It makes sense to have the tuner capability field be a union of the caps of all bands,
> > but you can't do the same for a modulation field (unless you make it a bitfield, but
> > that's just weird IMHO).
> 
> Well, DVB exposes modulation as caps. Not sure if this is the best thing to do there,
> but it could make sense to have something like:
> 	HAVE_AM
> 	HAVE_FM
> 	HAVE_SDR
> 	HAVE_VSB
> ...
> 
> for modulation caps.

Can we have a (hopefully short) irc discussion today? I'd really like to get this API
finalized.

> 
> > 
> > So perhaps we should add compat code like what I suggested earlier in a private email
> > where, if no enum_freq_bands op is defined but g_tuner is, then the core will provide
> > a enum_freq_bands version that uses the caps/rangelow/high from g_tuner and that can
> > fill in the modulation type based on the video node (i.e. if called from radio, then
> > the modulation is FM, else VSB).
> 
> That would be an alternative. I don't have a strong opinion here, but I think that
> modulation caps could work better, IMHO.
>
> > This is true for all currently available drivers, except for radio-cadet. But the
> > latter will be updated with proper enum_freq_bands support, so it won't use the
> > compat code.
> > 
> > As a result of this, an application can always find the modulation of a particular
> > frequency band by calling VIDIOC_ENUM_FREQ_BANDS.
> > 
> > Note that for the sake of brevity I'm ignoring the modulator in this discussion, but
> > the same principle applies there.
> > 
> > BTW, if I understand it correctly SDR isn't a modulation type as such, it's the raw
> > output from the tuner and it is up to the software to decide whether to demodulate
> > as FM or AM (in the case of analog radio), right?
> 
> Yes.

In that case, shouldn't SDR be signalled as a tuner capability? It's not a modulator
as such. If set, then you can read from the radio device to get the raw samples.

This assumes that the SDR capability and the RDS_BLOCK_IO capability are mutually exclusive.
Or at the very least that RDS is disabled if the SDR functionality is enabled and vice
versa.

> > Although in practice I expect it to be obvious what should be used.
> 
> Some bands could allow more than one modulation type. FYI, regulatory agencies define
> two types of usage for a frequency band: a "primary usage" and a "secondary usage".
> 
> Being simplistic, primary usage is generally for public or government services. Secondary
> usage is generally for personal/private usage. The usages that don't require special
> licenses to transmit (like wifi) are secondary usage. Remote-controlled toys and walk talks
> at 27MHz range is an example of such secondary usage. AFAIKT, several of those devices use FM
> modulation, but AM may also be used on those devices.
> 
> An SDR implementation may decode either AM or FM (or it could also decode other types,
> like SSB - used on several amateur radio bands).

Based on this I am inclined to go with a bitfield. If the tuner supports only SDR, then
the bitfield is 0 (since it is software that decides the demodulation). If there are one
or more hardware demodulators, then the appropriate flags are set. Right now there is
only one hardware demodulator per frequency band but future hardware may have more.

I still don't think it is a good idea to add the same modulation field to v4l2_tuner:
leave that to v4l2_frequency_band. A modulation field in v4l2_tuner should instead be
used to select what demodulation to use if there are more hardware demodulators. We
don't have that at the moment, so I think we shouldn't add it yet.

So my current proposal is: use a bitfield in v4l2_frequency_band to describe possible
(de)modulators and add compat code to the v4l2-ioctl.c to automatically create a
vidioc_enum_freq_bands op if no such op was supplied, using the data from g_tuner or
g_modulator and which device node was used to fill in the fields.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
  2012-07-04  8:35             ` Hans Verkuil
@ 2012-07-04  9:18               ` Hans de Goede
  0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2012-07-04  9:18 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, linux-media, halli manjunatha, Hans Verkuil

Hi,

On 07/04/2012 10:35 AM, Hans Verkuil wrote:

<snip snip>

> Can we have a (hopefully short) irc discussion today? I'd really like to get this API
> finalized.

+1, I'm available the entire day (CET office hours + evening if needed that is)

<snip snip>

> So my current proposal is: use a bitfield in v4l2_frequency_band to describe possible
> (de)modulators and add compat code to the v4l2-ioctl.c to automatically create a
> vidioc_enum_freq_bands op if no such op was supplied, using the data from g_tuner or
> g_modulator and which device node was used to fill in the fields.

+1

Regards,

The other Hans :)

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-07-04  9:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-02 14:15 [RFC PATCH 0/6] Add frequency band information Hans Verkuil
2012-07-02 14:15 ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Hans Verkuil
2012-07-02 14:15   ` [RFC PATCH 2/6] v4l2: add core support for the new VIDIOC_ENUM_FREQ_BANDS ioctl Hans Verkuil
2012-07-02 14:15   ` [RFC PATCH 3/6] v4l2 spec: add VIDIOC_ENUM_FREQ_BANDS documentation Hans Verkuil
2012-07-02 14:15   ` [RFC PATCH 4/6] radio-cadet: upgrade to latest frameworks Hans Verkuil
2012-07-02 14:15   ` [RFC PATCH 5/6] radio-cadet: fix RDS handling Hans Verkuil
2012-07-02 14:15   ` [RFC PATCH 6/6] radio-cadet: implement frequency band enumeration Hans Verkuil
2012-07-02 17:42   ` [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS Mauro Carvalho Chehab
2012-07-03  7:19     ` Hans Verkuil
2012-07-03 16:01       ` Mauro Carvalho Chehab
2012-07-03 16:47         ` Hans Verkuil
2012-07-03 20:35           ` Mauro Carvalho Chehab
2012-07-04  8:35             ` Hans Verkuil
2012-07-04  9:18               ` Hans de Goede
2012-07-02 14:42 ` [RFC PATCH 0/6] Add frequency band information Hans de Goede
2012-07-02 15:24   ` halli manjunatha

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.