[v4,1/5] V4L2: Add seek spacing and FM RX class.

Message ID 1275647663-20650-2-git-send-email-matti.j.aaltonen@nokia.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

m7aalton June 4, 2010, 10:34 a.m. UTC
  Add spacing field to v4l2_hw_freq_seek and also add FM RX class to
control classes.

Signed-off-by: Matti J. Aaltonen <matti.j.aaltonen@nokia.com>
---
 include/linux/videodev2.h |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
  

Comments

Mauro Carvalho Chehab July 6, 2010, 2:03 a.m. UTC | #1
Em 04-06-2010 07:34, Matti J. Aaltonen escreveu:
> Add spacing field to v4l2_hw_freq_seek and also add FM RX class to
> control classes.

If you're adding new stuff to API, you should also patch Documentation/DocBook/v4l stuff.
> 
> Signed-off-by: Matti J. Aaltonen <matti.j.aaltonen@nokia.com>
> ---
>  include/linux/videodev2.h |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 418dacf..95675cd 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -935,6 +935,7 @@ struct v4l2_ext_controls {
>  #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
>  #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
>  #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
> +#define V4L2_CTRL_CLASS_FM_RX 0x009c0000	/* FM Tuner control class */
>  
>  #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
>  #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
> @@ -1313,6 +1314,17 @@ enum v4l2_preemphasis {
>  #define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 113)
>  #define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 114)
>  
> +/* FM Tuner class control IDs */
> +#define V4L2_CID_FM_RX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_RX | 0x900)
> +#define V4L2_CID_FM_RX_CLASS			(V4L2_CTRL_CLASS_FM_RX | 1)
> +
> +#define V4L2_CID_FM_RX_BAND			(V4L2_CID_FM_TX_CLASS_BASE + 1)
> +enum v4l2_fm_rx_band {
> +	V4L2_FM_BAND_OTHER		= 0,
> +	V4L2_FM_BAND_JAPAN		= 1,
> +	V4L2_FM_BAND_OIRT		= 2
> +};


You don't need anything special to define the bandwidth. VIDIOC_G_TUNER/VIDIOC_S_TUNER allows
negotiating rangelow/rangehigh.

> +
>  /*
>   *	T U N I N G
>   */
> @@ -1377,7 +1389,8 @@ struct v4l2_hw_freq_seek {
>  	enum v4l2_tuner_type  type;
>  	__u32		      seek_upward;
>  	__u32		      wrap_around;
> -	__u32		      reserved[8];
> +	__u32		      spacing;
> +	__u32		      reserved[7];
>  };
>  
>  /*

I can't comment on your other API proposals, as you didn't send a patch documenting it
at the API spec.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hans Verkuil July 9, 2010, 11:06 a.m. UTC | #2
On Friday 04 June 2010 12:34:19 Matti J. Aaltonen wrote:
> Add spacing field to v4l2_hw_freq_seek and also add FM RX class to
> control classes.
> 
> Signed-off-by: Matti J. Aaltonen <matti.j.aaltonen@nokia.com>
> ---
>  include/linux/videodev2.h |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 418dacf..95675cd 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -935,6 +935,7 @@ struct v4l2_ext_controls {
>  #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
>  #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
>  #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
> +#define V4L2_CTRL_CLASS_FM_RX 0x009c0000	/* FM Tuner control class */
>  
>  #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
>  #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
> @@ -1313,6 +1314,17 @@ enum v4l2_preemphasis {
>  #define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 113)
>  #define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 114)
>  
> +/* FM Tuner class control IDs */
> +#define V4L2_CID_FM_RX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_RX | 0x900)
> +#define V4L2_CID_FM_RX_CLASS			(V4L2_CTRL_CLASS_FM_RX | 1)
> +
> +#define V4L2_CID_FM_RX_BAND			(V4L2_CID_FM_TX_CLASS_BASE + 1)
> +enum v4l2_fm_rx_band {
> +	V4L2_FM_BAND_OTHER		= 0,
> +	V4L2_FM_BAND_JAPAN		= 1,
> +	V4L2_FM_BAND_OIRT		= 2
> +};

I've been thinking about this a bit more. Would it be possible to do this automatically
in the driver? I.e. based on the frequency you switch the device into the appropriate
band?

If that is not possible, then you shouldn't forget to document this new control in the spec.
When you document it you should give some background information as well: the freq ranges of
these bands and roughly where they are used.

Regards,

	Hans

> +
>  /*
>   *	T U N I N G
>   */
> @@ -1377,7 +1389,8 @@ struct v4l2_hw_freq_seek {
>  	enum v4l2_tuner_type  type;
>  	__u32		      seek_upward;
>  	__u32		      wrap_around;
> -	__u32		      reserved[8];
> +	__u32		      spacing;
> +	__u32		      reserved[7];
>  };
>  
>  /*
>
  
m7aalton July 19, 2010, 7:55 a.m. UTC | #3
Hello.


On Tue, 2010-07-06 at 04:03 +0200, ext Mauro Carvalho Chehab wrote:
> Em 04-06-2010 07:34, Matti J. Aaltonen escreveu:
> > +
> > +#define V4L2_CID_FM_RX_BAND			(V4L2_CID_FM_TX_CLASS_BASE + 1)
> > +enum v4l2_fm_rx_band {
> > +	V4L2_FM_BAND_OTHER		= 0,
> > +	V4L2_FM_BAND_JAPAN		= 1,
> > +	V4L2_FM_BAND_OIRT		= 2
> > +};
> 
> 
> You don't need anything special to define the bandwidth. VIDIOC_G_TUNER/VIDIOC_S_TUNER allows
> negotiating rangelow/rangehigh.

I don't quite get how you want this to be done... Are you saying that
this enum should not be there? At least the wl1273 chip actually
supports two bands so it would seem more straightforward to start by
defining the band and working from there than the other way round. 

B.R. 
Matti A.

> 
> > +
> >  /*
> >   *	T U N I N G
> >   */
> > @@ -1377,7 +1389,8 @@ struct v4l2_hw_freq_seek {
> >  	enum v4l2_tuner_type  type;
> >  	__u32		      seek_upward;
> >  	__u32		      wrap_around;
> > -	__u32		      reserved[8];
> > +	__u32		      spacing;
> > +	__u32		      reserved[7];
> >  };
> >  
> >  /*
> 
> I can't comment on your other API proposals, as you didn't send a patch documenting it
> at the API spec.
> 
> Cheers,
> Mauro.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 418dacf..95675cd 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -935,6 +935,7 @@  struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
 #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
 #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
+#define V4L2_CTRL_CLASS_FM_RX 0x009c0000	/* FM Tuner control class */
 
 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1313,6 +1314,17 @@  enum v4l2_preemphasis {
 #define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 113)
 #define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 114)
 
+/* FM Tuner class control IDs */
+#define V4L2_CID_FM_RX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_RX | 0x900)
+#define V4L2_CID_FM_RX_CLASS			(V4L2_CTRL_CLASS_FM_RX | 1)
+
+#define V4L2_CID_FM_RX_BAND			(V4L2_CID_FM_TX_CLASS_BASE + 1)
+enum v4l2_fm_rx_band {
+	V4L2_FM_BAND_OTHER		= 0,
+	V4L2_FM_BAND_JAPAN		= 1,
+	V4L2_FM_BAND_OIRT		= 2
+};
+
 /*
  *	T U N I N G
  */
@@ -1377,7 +1389,8 @@  struct v4l2_hw_freq_seek {
 	enum v4l2_tuner_type  type;
 	__u32		      seek_upward;
 	__u32		      wrap_around;
-	__u32		      reserved[8];
+	__u32		      spacing;
+	__u32		      reserved[7];
 };
 
 /*